Azure reacts to invalid cloud credentials. #8989
Conversation
| ) | ||
|
|
||
| var logger = loggo.GetLogger("juju.provider.azure") |
There was a problem hiding this comment.
juju.provider.azure.errorutils ?
There was a problem hiding this comment.
I have thought about it but I am on the fence. I'd rather leave it as-is simply because our logging is not generally grouped by package but by functional area and here the functional area is azure provider. So all the logging for azure provider is either desired or not :)
I have changed the logging of error to Warning although, I do not know what an operator can do with it once the failure occurs.
Great pickup overall though, as I was not logging the actual failure :) Fixed now.
|
|
||
| // HasCredentialError determines if given error has authorisation denial codes embedded. | ||
| // If a code related to an invalid credential is found, the credential is invalidated as well. | ||
| func HasCredentialError(err error, ctx context.ProviderCallContext) bool { |
There was a problem hiding this comment.
The name HasCredentialError confused me initially when I was reading the code which calls this because the name implies a non mutating method. Since it returns a bool, a name like MaybeInvalidateCredential() would be better as it tells the user that mutating operation could offer and it's much easier to read the code.
|
|
||
| invalidateErr := ctx.InvalidateCredential("azure cloud denied access") | ||
| if invalidateErr != nil { | ||
| logger.Infof("could not invalidate stored azure cloud credential on the controller") |
provider/azure/environ.go
Outdated
| @@ -1232,6 +1241,7 @@ func (env *azureEnviron) deleteVirtualMachine( | |||
| logger.Debugf("- deleting virtual machine (%s)", vmName) | |||
| vmResultCh, errCh := vmClient.Delete(env.resourceGroup, vmName, nil) | |||
| if result, err := <-vmResultCh, <-errCh; err != nil { | |||
| errorutils.HandleCredentialError(err, ctx) | |||
There was a problem hiding this comment.
If there's a credential error here, any subsequent call to GetContainerReference below will also fail right? Did we need to do something like:
if maybeStorageClient && validCredential {
of something
provider/azure/environ_test.go
Outdated
| env := prepareForBootstrap(c, ctx, s.provider, &s.sender) | ||
|
|
||
| s.createSenderWithUnauthorisedStatusCode(c) | ||
| // s.sender = append(s.sender,s.initResourceGroupSenders()) |
… that we'd fail due to the invalid credential.
|
|
Description of change
This PR ensures that juju azure provider is responsive to azure cloud un-happinness with provided credentials.
Both situations where an invalid credential and invalidated (say, expired) credential is used to communicate with azure cloud is catered for here.
The most interesting bits are contained in ~/provider/azure/internal/errorutils package. This is the code that examines azure errors and determines whether the error is related to credential and worth reacting to.
All cloud calls that throw errors have now been modified to go through this logic to ensure that if the cloud complained about a credential, juju marks it as invalid.
Unit tests have been added to provide adequate initial coverage.