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.

…th workload identity

Signed-off-by: Etienne Tremel <995474+etiennetremel@users.noreply.github.com>
Signed-off-by: Etienne Tremel <995474+etiennetremel@users.noreply.github.com>
@etiennetremel etiennetremel force-pushed the docs/azure-workload-identity branch 2 times, most recently from a9703eb to 194a433 Compare May 14, 2024 05:58
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.

None yet

4 participants