Repository navigation
Improve Azure token logic - #1321
Conversation
ghost
added
There was a problem hiding this comment.
I think having the AzureTokenCredentialProvider as a token cache is a great idea.
I have a general comment below - you can take or leave this idea :)
I think all of the commands already use their auth right away - the only reason we had failures in the BuildCommand is because ExecuteWithCredentialsAsync(...) and the ManifestService were getting their own credentials, right? (one at the start, and the other after the build finished). If AzureTokenCredentialProvider were just a general cache for token(s) instead of trying to predict which tokens we may need, I think it might still solve the problem while being simpler. When we decide to get the tokens from the AzureTokenCredentialProvider at any point in a command could be a separate consideration. As long as the build command uses its auth right away and the token is cached, we wouldn't have to do anything fancy there.
This reverts commit 43f971b.
Previously, AzureTokenCredentialProvider would use `WorkloadIdentityCredential` during builds. In order to work around Workload Identity Credential lifetimes, we implemented a StaticTokenCredential that immediately fetched the requested token and cached it forever (added with #1321). WIF token lifetimes are documented [here](https://learn.microsoft.com/en-us/azure/devops/pipelines/release/troubleshoot-workload-identity?view=azure-devops#error-messages). Since #1712, we use `AzurePipelinesCredential` instead. `AzurePipelinesCredential` handles all of the lifetime/expiration concerns. We can use it directly and allow the Azure SDK to handle caching and automatic refresh when tokens are close to expiring. I have not found any documented restrictions on its lifetime. The System.AccessToken and the resulting AzurePipelinesCredential *should* be valid for the entire length of the pipeline job and no longer. I have built a test version of ImageBuilder with this change and validated it against our longest running build job, here: https://dev.azure.com/dnceng/internal/_build/results?buildId=2854918&view=logs&j=5b10b89b-008d-5f76-ef8b-7fc595d90c00&t=a505c4d8-3f6e-56eb-f81a-e4be2494dd9a It happily spends over 3 hours building images and then has no trouble pushing them.
Fixes #1316
The original changes from #1310 and #1314 were hacks in order to deal with token expiration that occurs with a long-running command. If execution of the command takes longer than an hour and then an attempt is made to retrieve an Azure access token, it will fail. This is resolved by retrieving the token at the beginning of the command instead.
This defines a provider component that returns an Azure credential that's configured with a pre-cached access token. The access tokens are retrieved during the startup of the command during component composition.
In order to get the tokens, we have to know the scope ahead of time. So these are explicitly defined and a token is retrieved for each one of them. When the caller needs the credential, it specifies which scope is needed and the appropriate credential for that scope is returned.