auth/azure: use ACR-scoped token for registry authentication#1130
auth/azure: use ACR-scoped token for registry authentication#1130matheuscscp merged 1 commit intofluxcd:mainfrom
Conversation
|
We need an Azure engineer to review this change. @dipti-pai can you please take a look? |
|
From the docs, this scenario is supported. Validated from azure-go-sdk that the constant is defined here and that the value is same for the 3 Azure clouds. @kukacz, Can you confirm that you tested this with container registry accepting the ACR-scoped token only and the broader ARM-scoped token (default). It will help if you paste the screenshot from your test results. For custom configuration, I did not find the override, we can skip the changes there, i.e fallback to ARM scope and take the ACR scope change as a separate change if/when this is requested. The primary reason for this is it is hard to validate the correctness without running it in the custom cloud environment. |
3853030 to
4664b1d
Compare
|
@dipti-pai Thanks for your comments! I removed the custom configuration part and substituted it with fall back to ARM-scoped token for backwards compatibility.
Sure, attaching 3 screenshots illustrating different test scenarios:
|
|
Thank you @kukacz. The changes LGTM. Another small request for completeness - could you run the integration tests defined here, workload identity test scenarios are covered for Azure in these tests. The pre-requisites for Azure and the steps to run the test are described in the README. Let us know if you hit any issues running these. Thanks again! |
I cloned the branch and pushed to the |
Sure, I ran the integration tests with workload identity enabled - few of them skipped, the rest passed: |
|
@kukacz Please rebase and force-push 🙏 @dipti-pai Is this something we should backport to a Flux 2.8 patch? |
4664b1d to
c8ad0ff
Compare
Rebased. |
matheuscscp
left a comment
There was a problem hiding this comment.
LGTM! 🚀
@kukacz Thanks very much!
@dipti-pai @stefanprodan I think this should be backported to 2.8 in the next patch, let me know your thoughts 🙏
stefanprodan
left a comment
There was a problem hiding this comment.
LGTM
We'll backport this and include it in the next Flux 2.8 patch release
These passed. Thanks @kukacz for running them as well 👍 |
|
Let's wait for @dipti-pai to stamp this PR, then we do the backport. |
|
LGTM. Thanks @kukacz |
|
@kukacz one final rebase please, so we can merge |
c8ad0ff to
ac2e88e
Compare
The access token scope for Azure Container Registry was derived from cloud.ResourceManager, which resolves to the ARM endpoint (e.g. https://management.azure.com/.default\). This is an ARM-scoped token, not an ACR-scoped one. Microsoft recommends disabling ARM audience authentication on ACR registries for enhanced security and least-privilege compliance. When organizations follow this recommendation and disable authentication-as-arm, Flux's ARM-scoped tokens are rejected, causing unauthorized errors for workload identity users. Use azcontainerregistry.ServiceName audience instead, which resolves to https://containerregistry.azure.net/.default -- the correct ACR scope as documented by Microsoft: https://learn.microsoft.com/en-us/azure/container-registry/container-registry-disable-authentication-as-arm Signed-off-by: Lukáš Kubín <lukas.kubin@gmail.com>
ac2e88e to
68b8866
Compare
Of course. Rebased now, with commit message update only. |
|
Successfully created backport PR for |
|
@kukacz This fix is scheduled for a Flux patch release on March 12 |
Summary
This PR fixes Azure Container Registry authentication for workload identity scenarios by requesting an ACR-scoped token instead of an ARM-scoped token.
Problem
The token audience was derived from ARM (
https://management.azure.com/.default), which is rejected when ACRauthentication-as-armis disabled (recommended by Microsoft for least-privilege security).Change
https://containerregistry.azure.net/.default.Populate ACR service config in cloud environment setup so custom environment file paths also resolve the correct ACR scope.Impact
Prevents unauthorized errors against ACR when ARM audience authentication is disabled.
Notes
First contribution from me to this repository. I used AI assistance to help construct the fix, and I manually reviewed and tested the final changes.