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

Add support for Azure AD Workload Identity. #548

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

YvesZelros
Copy link
Contributor

Description

Add support for Azure AD Workload Identity.
https://learn.microsoft.com/en-us/azure/aks/workload-identity-overview

Replace low level Azure mod autorest that is deprecated by more higth level Azure Api
https://github.com/Azure/go-autorest/blob/autorest/azure/auth/v0.5.12/README.md

Replace old PR:

Fixes:

Checklist

Please make sure that your PR fulfills the following requirements:

  • Reviewed the guidelines for contributing to this repository
  • The commit message follows the Conventional Commits Guidelines.
  • Tests for the changes have been updated
  • Are you adding dependencies? If so, please run go mod tidy -compat=1.17 to ensure only the minimum is pulled in.
  • Docs have been added / updated
  • Optional. My organization is added to USERS.md.

Type of Change

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • New tests
  • Build/CI related changes
  • Documentation content changes
  • Other (please describe)

Other information

Update Go from 17 to 18 because new tests required Go 18 as Azure function use generic

@YvesZelros YvesZelros changed the title Feat/azure workload identity Add support for Azure AD Workload Identity. Sep 1, 2023
@YvesZelros YvesZelros force-pushed the feat/azure__workload_identity branch 2 times, most recently from c501834 to 7a83912 Compare September 4, 2023 12:37
@YvesZelros
Copy link
Contributor Author

@werne2j It's possible to re-run Workflows.
I have fix the test that was fail and the linux go image sha1.
Thx

@YvesZelros
Copy link
Contributor Author

@jkayani Can you run the ci please, thanks

@codecov-commenter
Copy link

Codecov Report

Merging #548 (fd625b2) into main (77b07b1) will increase coverage by 0.41%.
The diff coverage is 88.63%.

@@            Coverage Diff             @@
##             main     #548      +/-   ##
==========================================
+ Coverage   71.29%   71.71%   +0.41%     
==========================================
  Files          26       26              
  Lines        1951     1962      +11     
==========================================
+ Hits         1391     1407      +16     
+ Misses        460      458       -2     
+ Partials      100       97       -3     
Files Changed Coverage Δ
pkg/backends/azurekeyvault.go 92.00% <88.09%> (+14.58%) ⬆️
pkg/config/config.go 83.06% <100.00%> (-1.34%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@YvesZelros
Copy link
Contributor Author

@werne2j @jkayani
Please review this change when you have some time.
On my side I test this change on our Aks cluster.

  • Authentification with AZURE_CLIENT_SECRET continu to be supported
  • Azure managed identity is now supported

@YvesZelros
Copy link
Contributor Author

@werne2j Can you look on please.

@Stolz
Copy link

Stolz commented Nov 10, 2023

Also interested in this getting merged

@lukipro
Copy link

lukipro commented Nov 28, 2023

Please merge this life saving PR :)

@Stolz
Copy link

Stolz commented Nov 28, 2023

@YvesZelros would it be possible for you to release the binary from your fork?

@werne2j
Copy link
Member

werne2j commented Nov 28, 2023

Will look to get this merged for the next release. @YvesZelros can you rebase the branch? I can then kick off the CI and do a review

@YvesZelros YvesZelros force-pushed the feat/azure__workload_identity branch 3 times, most recently from d90cc4b to 4a2fca9 Compare November 28, 2023 16:43
@YvesZelros
Copy link
Contributor Author

Will look to get this merged for the next release. @YvesZelros can you rebase the branch? I can then kick off the CI and do a review

@werne2j Rebase is done

@YvesZelros
Copy link
Contributor Author

YvesZelros commented Nov 28, 2023

@YvesZelros would it be possible for you to release the binary from your fork?

@Stolz

We have push temporary a amd64 binary on our Github, you can test it WITHOUT any guaranty ;-)

We plan to remove this binary soon as this this PR will be merged !

curl -L https://github.com/zelros/argocd-vault-plugin/raw/azure_workload_identity_linux_amd64_beta_v3/argocd-vault-plugin -o /custom-tools/argocd-vault-plugin 

@werne2j
Copy link
Member

werne2j commented Nov 30, 2023

@YvesZelros this is backwards compatible? Still works with client and tenant Id as before?

@YvesZelros
Copy link
Contributor Author

YvesZelros commented Dec 1, 2023

@YvesZelros this is backwards compatible? Still works with client and tenant Id as before?

@werne2j

Yes, the Azure SDK method NewDefaultAzureCredential that I use is described here

As documented on Azure it support these types of auth =>

Option 1: Define environment variables (as the pervious version of this plugin)

  • Service principal with a secret (AZURE_CLIENT_ID, AZURE_TENANT_ID, AZURE_CLIENT_SECRET)
  • Service principal with certificate (AZURE_CLIENT_ID, AZURE_TENANT_ID, AZURE_CLIENT_CERTIFICATE_PATH)
  • Username and password (AZURE_CLIENT_ID, AZURE_USERNAME, AZURE_PASSWORD)

Option 2: 🥇🥇 Use Workload Identity

  • Enables pods in a Kubernetes cluster to use a Kubernetes identity (service account)
    Based on the Kubernetes service account, Azure will automatically inject env variables =>

    • AZURE_TENANT_ID=xxx

    • AZURE_FEDERATED_TOKEN_FILE=/var/run/secrets/azure/tokens/azure-identity-token

    • AZURE_AUTHORITY_HOST=https://login.microsoftonline.com/

    • AZURE_CLIENT_ID=yyyy

    • And mount a secret token on /var/run/secrets/azure/tokens/azure-identity-token that have a limited validity and is automatically rotated by Azure.

Option 3: Use a managed identity

As resume, yes AZURE_CLIENT_SECRET still work but using Azure AD Workload Identity is a better way for service authentification in Azure.

Signed-off-by: Yves Galante <yves.galante@zelros.com>
@YvesZelros YvesZelros force-pushed the feat/azure__workload_identity branch from 87aa110 to c9604b0 Compare January 8, 2024 16:03
@YvesZelros
Copy link
Contributor Author

@werne2j Any chance to merge it in 2024 ;-) ?

@Eneuman
Copy link

Eneuman commented Jan 16, 2024

This really needs to be merged. Todays code is using the deprecated (Since June 30, 2023) ADAL lib that does not recive any more security updates.

@werne2j werne2j merged commit 64935da into argoproj-labs:main Jan 17, 2024
3 checks passed
@SylwiaBrant
Copy link

Hello. When can we expect this to be released?

@werne2j
Copy link
Member

werne2j commented Jan 30, 2024

Will try to get it released soon

@singh-balraj
Copy link

Hello @werne2j Any tentative date for release?

@arunalakmal
Copy link

When can we expect this to be released?

@yyvess
Copy link

yyvess commented Mar 15, 2024

@werne2j Can you communicate about AgroCd plan related to this project #618 ?

@Eneuman
Copy link

Eneuman commented Apr 14, 2024

@werne2j Any news? We really need a new release.

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

10 participants