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

fix: Invalid Azure Scope #231

Merged
merged 1 commit into from
Mar 18, 2022
Merged

fix: Invalid Azure Scope #231

merged 1 commit into from
Mar 18, 2022

Conversation

s-bauer
Copy link
Contributor

@s-bauer s-bauer commented Feb 14, 2022

The scope is missing a /.default at the end and thereby prevented me from acquiring a valid access token.

Azure requires a full scope and not only the URL to be able to acquire a token. As the flow uses application permissions and not delegate permissions, the scope needs to end in "/.default" as described in https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-permissions-and-consent#request-the-permissions-in-the-app-registration-portal

@relu
Copy link
Member

relu commented Feb 14, 2022

Thank you, @s-bauer! Would you mind amending your commit to resolve the DCO check? Maybe also add some more information about this fix, like a link to some relevant documentation.

@s-bauer
Copy link
Contributor Author

s-bauer commented Feb 14, 2022

@relu Added more information and my sign-off.

@somtochiama
Copy link
Member

Hey @s-bauer,

The scope works okay. What is needed is to install AAD Pod Identity, create Azure identity and azure identity binding, then add a aadpodidbinding:<name-of-az-identity> label to the image-reflector-controller deployment podspec. I am in the process of writing and updating the docs for this.
My apologies that this wasn;t clear from the start🙏🏾

@s-bauer
Copy link
Contributor Author

s-bauer commented Feb 14, 2022

@somtochiama Yea I know this is supposed to be used with the pod identity, but as it has several limitations and there is a pod identity v2 coming, I want to avoid using it at the moment. I've applied a Kustomize patch to the image-reflector-controller to add the AZURE_CLIENT_ID AZURE_TENANT_ID and AZURE_CLIENT_SECRET environment variables. The DefaultAzureCredentials use the EnvironmentCredentials as the first priority and therefore picks up the credentials correctly.

The only thing that didn't work is the scope as the url ("https://management.azure.com/") is simply not a valid scope for AAD. I'm not sure why pod identity works with that scope.

If you have an environment with pod-identity, can you please check if it is also working correctly with the "/.default" suffix?

@somtochiama
Copy link
Member

somtochiama commented Feb 14, 2022

I have verified that this scope works with aad pod identity too

@s-bauer
Copy link
Contributor Author

s-bauer commented Feb 14, 2022

Awesome! BTW, just seen about "Azure Workload Identity" which is replacing pod identity and now available in preview: https://azure.github.io/azure-workload-identity/docs/introduction.html

@s-bauer
Copy link
Contributor Author

s-bauer commented Feb 15, 2022

I've done some tests with the new Azure Workload Identity package and unfortunately it's not yet working with the image-reflector-controller. This is due to the reason that the azure identity library for go does not yet support authentication using federated identities yet (Tracked in Azure/azure-sdk-for-go#15615). A workaround would be to use msal like the example at https://github.com/Azure/azure-workload-identity/tree/main/examples/msal-go or to simply wait until the azure identity sdk for go is updated.

@hiddeco hiddeco self-assigned this Mar 2, 2022
@pjbgf
Copy link
Member

pjbgf commented Mar 18, 2022

@s-bauer I caught up with @hiddeco on this and we agreed on merging this PR to fix the Azure Scope issue, and then add support for Azure Workload Identity as a separate issue.

Can you rebase this PR please?

Azure requires a full scope and not only the URL to be able to acquire a token. As the flow uses application permissions and not delegate permissions, the scope needs to end in "/.default" as described in https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-permissions-and-consent#request-the-permissions-in-the-app-registration-portal

Signed-off-by: Simon Bauer <bauer-s-public@outlook.com>
@s-bauer
Copy link
Contributor Author

s-bauer commented Mar 18, 2022

Done!

@hiddeco hiddeco added the bug Something isn't working label Mar 18, 2022
@hiddeco hiddeco merged commit a417312 into fluxcd:main Mar 18, 2022
@hiddeco hiddeco removed their assignment Mar 18, 2022
@hiddeco
Copy link
Member

hiddeco commented Mar 18, 2022

Thank you @s-bauer 🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants