Skip to content

ImdsManagedIdentitySource handles 502 response with CredentialUnavailableException#18479

Merged
christothes merged 3 commits intoAzure:masterfrom
christothes:users/chriss/IMDS502
Feb 23, 2021
Merged

ImdsManagedIdentitySource handles 502 response with CredentialUnavailableException#18479
christothes merged 3 commits intoAzure:masterfrom
christothes:users/chriss/IMDS502

Conversation

@christothes
Copy link
Copy Markdown
Member

fixes #18362

@christothes christothes requested a review from schaabs as a code owner February 5, 2021 17:13
@ghost ghost added the Azure.Identity label Feb 5, 2021
@christothes christothes self-assigned this Feb 5, 2021
@christothes christothes added the Client This issue is related to a non-management package label Feb 5, 2021

var options = new TokenCredentialOptions() { Transport = mockTransport };

options.Retry.MaxRetries = 0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still want to retry on 502 or should it be excluded in the response classifier for the ManagedIdentityCredential? I'm ok with still retrying if we think that's the right thing to do here, but I want to make sure it's an explicit choice.


[NonParallelizable]
[Test]
public async Task VerifyAppService2017RequestMockAsync()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this test a duplicate?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, dupe of VerifyAppService2017RequestWithClientIdMockAsync

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like VerifyAppService2017RequestWithClientIdMockAsync always requests a user assigned id. I think we should test with this parameter an without. Perhaps we can add a parameter to VerifyAppService2017RequestWithClientIdMockAsync so we can test with a clientId and with specifying null for the clientId?

@schaabs schaabs self-requested a review February 23, 2021 22:59
@christothes christothes merged commit 0f2b212 into Azure:master Feb 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Azure.Identity Client This issue is related to a non-management package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] ManagedIdentityCredential fails with AuthenticationFailedException for a 502 response

2 participants