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

docs: add instructions to authenticate to Azure Container Registry with workload identity #676

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

etiennetremel
Copy link

Add instructions to authenticate to Azure Container Registry with workload identity.

Closes #586, #550 and #473

@codecov-commenter
Copy link

codecov-commenter commented Feb 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.27%. Comparing base (7d93c7a) to head (92459cd).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #676   +/-   ##
=======================================
  Coverage   66.27%   66.27%           
=======================================
  Files          22       22           
  Lines        2150     2150           
=======================================
  Hits         1425     1425           
  Misses        591      591           
  Partials      134      134           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@etiennetremel etiennetremel force-pushed the docs/azure-workload-identity branch 2 times, most recently from 236b924 to 92459cd Compare February 23, 2024 13:27
@Pionerd
Copy link

Pionerd commented Apr 10, 2024

I just tested this, but the credentials are not picked up (argocd-image-updater test <private_image> fails). How can I know the script is actually ran? I can see the script present, the registries.conf is OK, the /var/run/secrets/azure/tokens/azure-identity-token is set, what am I missing?

Edit:
Three observations:

  • I think it makes sense to document that the ConfigMap should be mounted in such a way that the script becomes executable (e.g. defaultMode: 493).
  • The documentation should be clear in the ACR_NAME is expected to be including azurecr.io.
  • Although the script gives a good token when run manually (I can use it with docker login and docker pull), it still does not work when used in argocd-image-updater test. Please see: Cannot pull images from Azure Container Registry #550 (comment)

@vepetkov
Copy link

I just tested this, but the credentials are not picked up (argocd-image-updater test <private_image> fails). How can I know the script is actually ran? I can see the script present, the registries.conf is OK, the /var/run/secrets/azure/tokens/azure-identity-token is set, what am I missing?

Edit: Three observations:

  • I think it makes sense to document that the ConfigMap should be mounted in such a way that the script becomes executable (e.g. defaultMode: 493).
  • The documentation should be clear in the ACR_NAME is expected to be including azurecr.io.
  • Although the script gives a good token when run manually (I can use it with docker login and docker pull), it still does not work when used in argocd-image-updater test. Please see: Cannot pull images from Azure Container Registry #550 (comment)

I was able to test it successfully but I in addition to the 2 things you mentioned above about the defaultMode and the ACR_NAME, I also had to add the shebang #!/bin/sh on top of the auth.sh file in the configmap.
Without it the script would run manually but fail when the image updater tries to run it with the error Could not set registry endpoint credentials: error executing /app/auth/auth.sh: fork/exec /app/auth/auth.sh: exec format error.

To test it within the pod, I had to explicitly specify the path to the registries.conf file as it did not pick it up (
argocd-image-updater --registries-conf-path /app/config/registries.conf test <private_image>).
However, it all works fine when deployed and the workload identity is used for get the available tags from ACR.

@etiennetremel
Copy link
Author

Thanks for testing out @vepetkov and @Pionerd, I was using the official helm chart which does include some of these settings by itself.

@etiennetremel etiennetremel force-pushed the docs/azure-workload-identity branch 2 times, most recently from a9703eb to 194a433 Compare May 14, 2024 05:58
@avo-sepp
Copy link

avo-sepp commented Aug 19, 2024

This works if you only have a single ACR, it relies on the ACR being name being hard coded into a single Environment variable $ACR_NAME. This is not a viable solution for the project as a whole. As it's pretty common to have more that one Container Registry at an Org, usually with the same vendor.

We need true support for Workload Identities in the project or Azure Container Registry push secrets.

@cveld
Copy link

cveld commented Sep 26, 2024

@etiennetremel where can I find the specific code that is responsible reacting upon the seemingly magic prefix ext? Maybe we can easily extend the logic and include commandline parameters to the script? In that case we can leverage one general token script with the container registry as the parameter.

Although if a code change is required we possibly can better include the azure workload identity logic.

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.

7 participants