-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Using ManagedIdentityCredential in Microsoft.DotNet.Monitoring.Sdk #4172
base: main
Are you sure you want to change the base?
Conversation
} | ||
} | ||
|
||
ChainedTokenCredential tokenCredential = TokenCredentialHelper.GetChainedTokenCredential(ManagedIdentityId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It depends on what the token credential is actually used for here. @garath may know more, but it looks like it's used to access a KV and replace secret sentinels in a config file with the actual secrets.
If this is the case, then most likely we want to use workload identity federation. This is the pattern where you execute the deployment set in AzureCLI with a service connection that is attached to a managed identity that has access to the vault. Looks sorta like: https://github.com/dotnet/arcade/blob/main/eng/publishing/v3/publish.yml#L87-L95
In that case, then I think the managed identity would be the value provided by the AzureCLI: https://github.com/dotnet/arcade/blob/main/eng/publishing/v3/publish.yml#L111
You may not need that though. You may just need AzureCliCredential and _defaultCredential as part of the chain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The user assigned managed identity would be placed alongside the service in Azure, and FR knows how to set up the service connection using federated credentials if you don't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I believe you want to keep the AzurePipelinesCredential
use. This tool is most often run during an Azure Pipeline run, so managed identities won't work.
The DefaultAzureCredential
is meant to cover use by a dev on their own machine (which is also a common way its run).
If you actually need to configure use for managed identity, I recommend following the current pattern of seeing which flags are passed in to the app and then configuring a Credential instance specifically for it. (Though, I'm not sure about cases where MI would be helpful here. Please describe the use case.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Marking as "Request Changes" just to prevent merging these changes as it will break existing builds.
Fixes #2702