IMDS retry exponential backoff

[4gust]

Description

This pull request introduces enhancements to the IMDS (Instance Metadata Service) retry logic in the Microsoft Authentication Library for Go (MSAL Go). The updates aim to improve the reliability and resilience of token acquisition when interacting with Azure IMDS endpoints, particularly under transient network failures or service unavailability scenarios.

Key changes:

• Adjusted backoff intervals and retry limits for improved fault tolerance.
• Added or updated corresponding unit tests to ensure correct retry behavior.

These improvements help ensure more consistent authentication experiences for applications running in Azure environments that rely on managed identities.

---

Please review the changes and provide feedback or suggestions for further improvements.

[sonarqubecloud]

Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

[chlowell]

This is less backoff than recommended: https://learn.microsoft.com/entra/identity/managed-identities-azure-resources/how-to-use-vm-token#retry-guidance

[4gust]

Updated the time to reflect the time.

[Robbie-Microsoft]

Still not addressed. The current code starts at 1<<0 = 1s for the first attempt (1s, 2s, 4s, 8s…). The IMDS retry guidance recommends starting at 2s (2^retry), so the series should be 2s, 4s, 8s, 16s…. Also worth noting: there's currently no cap on the wait duration — at attempt 10 the code would wait 1024s. Recommend something like:

waitTime := time.Duration(2<<uint(attempt)) * time.Second
if waitTime > 60*time.Second {
    waitTime = 60 * time.Second
}

[chlowell]

if A {
    B
} else {
    B
}

[Robbie-Microsoft]

Still present in the updated diff. Lines 91–95 of the test have identical branches:

if i < len(tt.mockResponses)-1 {
    mockClient.AppendResponse(..., mock.WithCallback(callback))
} else {
    mockClient.AppendResponse(..., mock.WithCallback(callback)) // identical
}

The condition does nothing — both branches call the same thing. Should just be:

mockClient.AppendResponse(mock.WithBody(body.Bytes()), mock.WithHTTPStatusCode(resp.statusCode), mock.WithCallback(callback))

[chlowell]

This makes the tests slow. Consider enabling them to skip the wait, for example by adding a hook like this:

var after = func(d time.Duration) <-chan time.Time {
	return time.After(d)
}

// in a test case
after = func(d time.Duration) <-chan time.Time {
    // TODO: validate d
    ch := make(chan time.Time, 1)
    ch <- time.Now()
    return ch
}

(has the additional benefit of preventing flakiness due to unpredictable scheduling)

[4gust]

Updated, and added after.
This feels like we should always have a wrapper for time for testing all the time related things.

[Robbie-Microsoft]

Still not addressed. The current diff still has case <-time.After(waitTime): with no overrideable hook. The new test cases actually sleep for real (1s + 2s + 4s = 7s for the IMDS exponential backoff case alone) and then validate wall-clock timing with ±500ms tolerance — exactly the flakiness concern raised here.