Skip to content
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

ClientID should be optional when using UserAssignedManagedIdentity #2237

Closed
amirschw opened this issue Feb 26, 2023 · 4 comments
Closed

ClientID should be optional when using UserAssignedManagedIdentity #2237

amirschw opened this issue Feb 26, 2023 · 4 comments
Assignees
Labels
feature-request New feature requests

Comments

@amirschw
Copy link
Contributor

amirschw commented Feb 26, 2023

Proposal

When using UserAssignedManagedIdentity for auth, the ClientID of the Managed Identity (aka identityId) is currently required:

else if (authenticationConfiguration.Mode == AuthenticationMode.UserAssignedManagedIdentity)
{
if (string.IsNullOrWhiteSpace(identityId))
{
throw new AuthenticationException("No identity was configured for user-assigned managed identity");
}
}

This becomes an issue at scale, because it requires some work to get the ClientID from the resource and put it in the configuration when deploying Promitor. Therefore, since ManagedIdentityCredential doesn't require ClientID (it's only required by IMDS if the VM has multiple user-assigned managed identities, but can be added by NMI when using aad-pod-identity), I suggest making it optional.

Alternatively, ResourceID should be supported in addition to ClientID (it is supported by IMDS and ManagedIdentityCredential). ResourceID is easier to pass in configuration since its value is known even prior to the creation of the resource (/subscriptions/<subscription_id>/resourceGroups/<resource_group_name>/providers/Microsoft.ManagedIdentity/userAssignedIdentities/<identity_name>).

Related to #2218

Component

Resource Discovery, Scraper

Contact Details

Teams :)

@amirschw amirschw added the feature-request New feature requests label Feb 26, 2023
@ravidbro
Copy link

+1

@tomkerkhove
Copy link
Owner

tomkerkhove commented Feb 26, 2023

Hm, it's been a while since I looked at user-defined managed identities but isn't the client ID required?

@amirschw
Copy link
Contributor Author

It is not required, it is only an issue when there's more than one user-assigned managed identity attached to the VM. And even in this case, when aad-pod-identity is used, NMI will add the client id to the request before passing it on to IMDS.

@tomkerkhove
Copy link
Owner

THank you for the contribution!

@github-project-automation github-project-automation bot moved this from Proposed to Ready To Ship in Promitor Roadmap Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature requests
Projects
Status: Ready To Ship
Development

No branches or pull requests

3 participants