-
Notifications
You must be signed in to change notification settings - Fork 90
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
Fallback to AZURE_CLIENT_ID env var if no client_id
query param in token request
#145
Comments
I realize this issue is a bit old, but are there any further thoughts on this? The current behavior is making it challenging to move certain third-party applications (such as Pulumi) from aad-pod-identity to Workflow Identity. |
@polivbr Can you provide more details on why this is blocking your move to workload identity? |
In the particular case of Pulumi, the Azure provider currently doesn't handle OIDC auth, so I have to use MSI. When configured to use MSI, rather than looking for an AZURE_CLIENT_ID environment variable, it looks for ARM_CLIENT_ID. If it's not set, it doesn't pass the client_id query param, as it assumes it's not needed, which worked fine with aad-pod-identity but breaks with workload identity. This isn't a showstopper as I can work around it, but it cost me many, many hours trying to debug exactly why it was failing. |
Am currently stuck on this. Tried to integrate fluxcd Image update automation but |
This is now causing me issues with services that use the rust sdk. Its identity library doesn't currently support OIDC and code that uses its default token credentials won't pick up a client_id. Is there a reason to not enable this fallback? Seems like not doing so causes way more issues than it could possibly be solving. It has literally broken every app I've attempted to deploy and now I'm faced with one for which I don't have a workaround. |
I'm ok with adding the fallback to |
Excellent. Thank you! |
This is still up for debate so I didn't want to add this as part of this PR. I'll add a follow-up PR once we make a decision on this.
Originally posted by @aramase in #142 (comment)
The text was updated successfully, but these errors were encountered: