Skip to content

Export credentialUnavailableError constructor#19733

Merged
chlowell merged 7 commits intoAzure:mainfrom
chlowell:credential-unavailable
Jan 6, 2023
Merged

Export credentialUnavailableError constructor#19733
chlowell merged 7 commits intoAzure:mainfrom
chlowell:credential-unavailable

Conversation

@chlowell
Copy link
Copy Markdown
Member

@chlowell chlowell commented Dec 29, 2022

This error type exists so a credential that can't acquire a token due to some non-fatal reason can signal ChainedTokenCredential to try another credential. For example, ManagedIdentityCredential returns this error type when managed identity isn't available in the runtime environment. Today, because this type isn't exported, ChainedTokenCredential treats all errors from custom credential implementations as fatal and therefore never tries the next credential in the chain. #19699 (comment) describes a scenario blocked by that behavior.

I also updated ChainedTokenCredential to behave the same whether a credentialUnavailableError is wrapped or not, so custom credentials can propagate other errors more easily.

@JeffreyRichter
Copy link
Copy Markdown
Member

We need to start with a very super clear definition of what "non-fatal" means exactly.
What I'm worried about is unpredictable behavior where a credential considered something non-fatal and so the chain picks the next one but this non-fatal error is successful in the future and now a different credential is in effect. It is critical that behavior be consistent & predictable all the time once the process starts.

So, I assume a network error is non-fatal because another attempt may succeed? But some EnvironmentCredential always fails if the env var doesn't exist or has a bad value? But then, what if the process adds the env var or changes the value? Then this EnvCred in the chain will now work and be used in a new client?

Let's discuss this at our next scrum meeting.

@chlowell
Copy link
Copy Markdown
Member Author

What I'm worried about is unpredictable behavior where a credential considered something non-fatal and so the chain picks the next one but this non-fatal error is successful in the future and now a different credential is in effect.

I agree that would be bad. Fortunately, the default behavior of a chain is to reuse the first successful credential for all subsequent authentications. If the chain is A -> B and A returns CredentialUnavailableError while B succeeds, the chain won't try A again.

I assume a network error is non-fatal because another attempt may succeed?

No, network errors during authentication are fatal in that they cause credentials to return other error types. CredentialUnavailableError means a credential can't attempt to authenticate because the runtime environment lacks some necessary data or state.

But some EnvironmentCredential always fails if the env var doesn't exist or has a bad value? But then, what if the process adds the env var or changes the value? Then this EnvCred in the chain will now work and be used in a new client?

This doesn't arise because only the constructor reads environment variables, and it returns an error when it doesn't find a complete set. If the configuration is complete but incorrect, the credential will attempt to authenticate--no CredentialUnavailableError--and ultimately return an Azure AD error.

Comment thread sdk/azidentity/errors.go Outdated
@chlowell chlowell requested a review from jhendrixMSFT January 6, 2023 20:49
Comment thread sdk/azidentity/errors.go
@chlowell chlowell changed the title Export CredentialUnavailableError Export credentialUnavailableError constructor Jan 6, 2023
@chlowell chlowell merged commit e29b926 into Azure:main Jan 6, 2023
@chlowell chlowell deleted the credential-unavailable branch January 6, 2023 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants