github/azure-sdk-for-cpp
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix IMDS retry policy to address review feedback: keep first request as fast probe

Browse Source 
Co-authored-by: antkmsft <41349689+antkmsft@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot] 2025-07-23 18:51:02 +00:00
parent 4891c52b14
commit d634bb9554
2 changed files with 14 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
sdk/identity/azure-identity/src/managed_identity_source.cpp
View File

@ -5,7 +5,6 @@
#include "private/identity_log.hpp"
#include <azure/core/http/http_status_code.hpp>
#include <azure/core/http/transport.hpp>
#include <azure/core/internal/environment.hpp>
#include <azure/core/platform.hpp>
@ -120,7 +119,10 @@ void ValidateArcKeyFile(std::string const& fileName)
}
}
// Create IMDS-specific retry options that handle HTTP 410 responses with sufficient retry duration
// Create IMDS-specific retry options that handle HTTP 410 responses
// Note: This is a compromise solution. The ideal implementation would apply
// extended retry duration only for HTTP 410 responses, which requires
// Azure Core support for conditional retry behavior.
Azure::Core::Credentials::TokenCredentialOptions CreateImdsRetryOptions(
Azure::Core::Credentials::TokenCredentialOptions const& options)
{
@ -132,11 +134,6 @@ Azure::Core::Credentials::TokenCredentialOptions CreateImdsRetryOptions(
// According to Azure docs, IMDS returns 410 for the first 70 seconds when not ready
imdsOptions.Retry.StatusCodes.insert(HttpStatusCode::Gone);
// Increase MaxRetries to ensure we can retry for at least 70 seconds when encountering 410
// With exponential backoff: 800ms + 1.6s + 3.2s + 6.4s + 12.8s + 25.6s + 51.2s = ~101s total
// This ensures we retry for longer than the required 70 seconds
imdsOptions.Retry.MaxRetries = 6;
return imdsOptions;
}
} // namespace
@ -587,10 +584,8 @@ ImdsManagedIdentitySource::ImdsManagedIdentitySource(
m_request.SetHeader("Metadata", "true");
// Configure first request to handle HTTP 410 (Gone) responses
// According to Azure docs, IMDS returns 410 for the first 70 seconds when not ready
// We need to allow retries for 410 responses to meet the 70-second requirement
Core::Credentials::TokenCredentialOptions firstRequestOptions = CreateImdsRetryOptions(options);
Core::Credentials::TokenCredentialOptions firstRequestOptions = options;
firstRequestOptions.Retry.MaxRetries = 0;
m_firstRequestPipeline = std::make_unique<TokenCredentialImpl>(firstRequestOptions);
m_firstRequestSucceeded = false;
}
@ -631,7 +626,7 @@ Azure::Core::Credentials::AccessToken ImdsManagedIdentitySource::GetToken(
{
const auto token = m_firstRequestPipeline->GetToken(
context.WithValue(
Azure::Core::Http::_internal::HttpConnectionTimeout, ImdsFirstRequestConnectionTimeout),
Core::Http::_internal::HttpConnectionTimeout, ImdsFirstRequestConnectionTimeout),
true,
createRequest);

12
sdk/identity/azure-identity/test/ut/managed_identity_credential_test.cpp
View File

@ -3253,7 +3253,9 @@ namespace Azure { namespace Identity { namespace Test {
TEST(ManagedIdentityCredential, ImdsRetryDuration)
{
// Test that IMDS retry policy provides sufficient duration for 70+ second requirement
// Test that IMDS retry policy includes HTTP 410 as retryable status code
// Note: This test validates HTTP 410 is retryable but doesn't test the full 70+ second
// requirement which would need extended retry duration (requires Azure Core support)
using Azure::Core::Diagnostics::Logger;
using LogMsgVec = std::vector<std::pair<Logger::Level, std::string>>;
LogMsgVec log;
@ -3262,9 +3264,9 @@ namespace Azure { namespace Identity { namespace Test {
try
{
// Create 7 HTTP 410 responses (6 retries + initial attempt) followed by success
// Create 4 HTTP 410 responses (3 retries + initial attempt) followed by success
std::vector<CredentialTestHelper::TokenRequestSimulationServerResponse> responses;
for (int i = 0; i < 7; ++i)
for (int i = 0; i < 4; ++i)
{
responses.push_back({HttpStatusCode::Gone, "{\"error\":\"not_ready\"}", {}});
}
@ -3316,9 +3318,9 @@ namespace Azure { namespace Identity { namespace Test {
try
{
// Create 8 HTTP 410 responses (more than the 6 max retries + initial attempt)
// Create 5 HTTP 410 responses (more than the 3 max retries + initial attempt)
std::vector<CredentialTestHelper::TokenRequestSimulationServerResponse> responses;
for (int i = 0; i < 8; ++i)
for (int i = 0; i < 5; ++i)
{
responses.push_back({HttpStatusCode::Gone, "{\"error\":\"not_ready\"}", {}});
}