Code review comment for lp:~jtv/juju-core/session-certificate

LGTM with some minor suggestions below.

https://codereview.appspot.com/10447043/diff/1/environs/azure/environ.go
File environs/azure/environ.go (right):

https://codereview.appspot.com/10447043/diff/1/environs/azure/environ.go#newcode153
environs/azure/environ.go:153: certFile, err :=
newTempCertFile([]byte(certData))
i hope we manage to avoid the need for these temporary files sooner
rather than later.

https://codereview.appspot.com/10447043/diff/1/environs/azure/environ.go#newcode158
environs/azure/environ.go:158: // make sure we clean up that certificate
file.
rather than recovering, i think a better approach might be something
like this:
func (env *azureEnviron) getManagementAPI() (_ *azureManagementContext,
retErr) {
      ...
       certFile, err := newTempCertFile([]byte(certData))
       if err != nil {
           return nil, err
       }
       defer func() {
           if retErr != nil {
               certFile.Delete()
           }
       }()

which amounts to much the same thing except you don't
need to call certFile.Delete if NewManagementAPI fails.

To be honest though, we're not gaining that much
by deleting the file even when we panic. That
will only happen if NewManagementAPI itself panics,
not if any other goroutine panics.

I'd probably just delete the defer and leave the
Delete in the NewManagementAPI error case.
Anyone with access to read the cert file
probably has access to get the environment
config anyway, so it's not a huge security issue.

https://codereview.appspot.com/10447043/diff/1/environs/azure/environ.go#newcode178
environs/azure/environ.go:178: // releaseManagementAPI frees up a
context object for interfacing with Azure's
// releaseManagementAPI frees up an Azure management context.
?

https://codereview.appspot.com/10447043/diff/1/environs/azure/environ_test.go
File environs/azure/environ_test.go (right):

https://codereview.appspot.com/10447043/diff/1/environs/azure/environ_test.go#newcode104
environs/azure/environ_test.go:104: }
i wouldn't mind seeing a test that checks that the temporary cert file
is deleted when NewManagementAPI fails.

https://codereview.appspot.com/10447043/