Skip to content

Conversation

bragi92
Copy link
Contributor

@bragi92 bragi92 commented Apr 10, 2025

Fixes #13500.

As promised I have created a PR for this. I looked over the comment of adding a type field for System-Assigned versus User-Assigned Identities but I have not added it in this PR for the following reasons :

  • We require client_id for User-Assigned, and skip it for System-Assigned. This pattern is unambiguous and widely used in Azure SDKs and tooling so we're consistent with that.
  • The Go SDK (via azidentity.NewManagedIdentityCredential) just uses presence/absence of client_id to decide the identity type. It doesn’t expose or require a separate type enum.
  • Simple is better for Prometheus users, adding a type field increases verbosity with little gain.

* remote-write: allow empty azure client_id to support system assigned managed identity

* add blank line for tests

* remote-write: allow empty azure client_id to support system assigned managed identity

Signed-off-by: Kaveesh Dubey <kadubey@microsoft.com>

* add blank line for tests

Signed-off-by: Kaveesh Dubey <kadubey@microsoft.com>

---------

Signed-off-by: Kaveesh Dubey <kadubey@microsoft.com>
Signed-off-by: Kaveesh Dubey <kadubey@microsoft.com>
@bwplotka bwplotka changed the title remote_write : allow empty client_id to suppport system assigned managed identity remote_write azure auth : allow empty client_id to suppport system assigned managed identity May 6, 2025
@bwplotka
Copy link
Member

bwplotka commented May 6, 2025

That generally make sense. Looking on the CI errors, did you forget to git add a test file?

https://github.com/prometheus/prometheus/actions/runs/14388718386/job/40350397383?pr=16421#step:5:71

Signed-off-by: bragi92 <kadubey@microsoft.com>
@bragi92
Copy link
Contributor Author

bragi92 commented May 14, 2025

@bwplotka Yes, I had accidently named the file azuread_good_specificmanagedidentity.yaml.yaml instead of azuread_good_specificmanagedidentity.yaml. I've updated the PR with the correct name. Please review and merge it in if it looks good to you.

@bragi92
Copy link
Contributor Author

bragi92 commented May 23, 2025

@bwplotka bumping this up. Please have a look at it when you get the chance.

Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@bwplotka bwplotka merged commit 14fc57e into prometheus:main May 24, 2025
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Azure AD system managed identity auth support for remote write
2 participants