Add support for Authentication against Azure Key Vault using Client Certificate by lieberlois · Pull Request #3469 · external-secrets/external-secrets

lieberlois · 2024-05-08T09:04:20Z

Problem Statement

I find that the operator should be able to authenticate via ClientCredentials. In the perfect world, it should be possible to configure a SecretStore and a ClusterSecretStore to fetch the clientCertificate key from a secret and use that for authentication against Azure KV rather than the clientSecret field.

Related Issue

Fixes #3461

Proposed Changes

Extend the CRD to not only accept clientSecret but also clientCertificate

Checklist

I have read the contribution guidelines
All commits are signed with git commit --signoff
My changes have reasonable test coverage
All tests pass with make test -- main branch is currently failing, the tests that i changed are all green ✅
I ensured my PR is ready for review with make reviewable

Signed-off-by: Luis Schweigard <[email protected]>

Implementation of Certificate Based Authz against Azure Key Vault

Signed-off-by: Luis Schweigard <[email protected]>

pkg/provider/azure/keyvault/keyvault.go

gusfcarvalho · 2024-05-10T08:13:44Z

hi @lieberlois thanks for your contribution! Just some nits in the PR - plus, if you can run make reviewable - some linting tests are failing 😄

Signed-off-by: Luis Schweigard <[email protected]>

lieberlois · 2024-05-11T09:21:23Z

@gusfcarvalho Thanks for the review! Maybe we need to extend the developer docs - for MacOS (at least arm64 chips) you need to have gnu-utils installed and cannot use the MacOS native sed etc. - this is why i couldnt run make reviewable before ^^

I applied the reviewed changes 👍

gusfcarvalho · 2024-05-11T10:33:07Z

@gusfcarvalho Thanks for the review! Maybe we need to extend the developer docs - for MacOS (at least arm64 chips) you need to have gnu-utils installed and cannot use the MacOS native sed etc. - this is why i couldnt run make reviewable before ^^

I applied the reviewed changes 👍

We used to have as part of our make files the detection of arm64 platforms and bump of the appropriate sed commands, install of needed tools, etc, to use. Maybe this needs to be reviewed once again 🤔

gusfcarvalho · 2024-05-11T10:35:02Z

/ok-to-test sha=b0be5a1f5e86acf5cd8f1d0b8c042f01efd5c985

gusfcarvalho · 2024-05-11T13:59:47Z

hi @lieberlois ! I think we only need now to fix the message from that unit test: https://github.com/external-secrets/external-secrets/actions/runs/9042682072/job/24850275469?pr=3469#step:7:940

Signed-off-by: Luis Schweigard <[email protected]>

lieberlois · 2024-05-11T16:21:53Z

Hey @gusfcarvalho 😉 yeah, that was a search and replace error, I corrected it. Tests are now running 😄 Sorry for missing that !

sonarqubecloud · 2024-05-11T16:22:45Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

lieberlois · 2024-05-11T16:26:50Z

@gusfcarvalho Thanks for the review! Maybe we need to extend the developer docs - for MacOS (at least arm64 chips) you need to have gnu-utils installed and cannot use the MacOS native sed etc. - this is why i couldnt run make reviewable before ^^
I applied the reviewed changes 👍

We used to have as part of our make files the detection of arm64 platforms and bump of the appropriate sed commands, install of needed tools, etc, to use. Maybe this needs to be reviewed once again 🤔

Regarding this topic: yes, the detection works, i just didnt have "gnu-sed" on my mac (which basically aliases "gsed" to the gnu-sed version). The script however requires this.

lieberlois · 2024-05-13T12:19:12Z

@gusfcarvalho Any insights on the next release date? 😃 we plan to use this change in our project

…ertificate (external-secrets#3469) * Implementation of Certificate Based Authz against Azure Key Vault Signed-off-by: Luis Schweigard <[email protected]> * Add tests for new Azure certificate auth functionality Signed-off-by: Luis Schweigard <[email protected]> * Add documentation for Azure Cert based Auth Signed-off-by: Luis Schweigard <[email protected]> * Generate spec.md Signed-off-by: Luis Schweigard <[email protected]> * Add changes from code review Signed-off-by: Luis Schweigard <[email protected]> * Fix naming in test error case Signed-off-by: Luis Schweigard <[email protected]> --------- Signed-off-by: Luis Schweigard <[email protected]>

…ertificate (external-secrets#3469) * Implementation of Certificate Based Authz against Azure Key Vault Signed-off-by: Luis Schweigard <[email protected]> * Add tests for new Azure certificate auth functionality Signed-off-by: Luis Schweigard <[email protected]> * Add documentation for Azure Cert based Auth Signed-off-by: Luis Schweigard <[email protected]> * Generate spec.md Signed-off-by: Luis Schweigard <[email protected]> * Add changes from code review Signed-off-by: Luis Schweigard <[email protected]> * Fix naming in test error case Signed-off-by: Luis Schweigard <[email protected]> --------- Signed-off-by: Luis Schweigard <[email protected]> Signed-off-by: Bude8 <[email protected]>

…ertificate (external-secrets#3469) * Implementation of Certificate Based Authz against Azure Key Vault Signed-off-by: Luis Schweigard <[email protected]> * Add tests for new Azure certificate auth functionality Signed-off-by: Luis Schweigard <[email protected]> * Add documentation for Azure Cert based Auth Signed-off-by: Luis Schweigard <[email protected]> * Generate spec.md Signed-off-by: Luis Schweigard <[email protected]> * Add changes from code review Signed-off-by: Luis Schweigard <[email protected]> * Fix naming in test error case Signed-off-by: Luis Schweigard <[email protected]> --------- Signed-off-by: Luis Schweigard <[email protected]> Signed-off-by: Bill Hamilton <[email protected]>

lieberlois added 5 commits May 8, 2024 09:44

Implementation of Certificate Based Authz against Azure Key Vault

9ab0213

Signed-off-by: Luis Schweigard <[email protected]>

Merge pull request #1 from lieberlois/feature/azure-key-vault-cert

94bb3be

Implementation of Certificate Based Authz against Azure Key Vault

Add tests for new Azure certificate auth functionality

1c1cdbc

Signed-off-by: Luis Schweigard <[email protected]>

Add documentation for Azure Cert based Auth

941ef68

Signed-off-by: Luis Schweigard <[email protected]>

Generate spec.md

8f198d0

Signed-off-by: Luis Schweigard <[email protected]>

lieberlois requested a review from a team as a code owner May 8, 2024 09:04

lieberlois requested a review from gusfcarvalho May 8, 2024 09:04

Merge branch 'main' into main

9c12ec8

lieberlois mentioned this pull request May 8, 2024

Support for Azure KV Authentication with ClientCertificate #3461

Closed

gusfcarvalho reviewed May 10, 2024

View reviewed changes

pkg/provider/azure/keyvault/keyvault.go Outdated Show resolved Hide resolved

gusfcarvalho reviewed May 10, 2024

View reviewed changes

pkg/provider/azure/keyvault/keyvault.go Show resolved Hide resolved

Add changes from code review

cd043e5

Signed-off-by: Luis Schweigard <[email protected]>

Merge branch 'main' into main

b0be5a1

lieberlois requested a review from gusfcarvalho May 11, 2024 09:22

Fix naming in test error case

dbb952f

Signed-off-by: Luis Schweigard <[email protected]>

Merge branch 'main' into main

6045529

gusfcarvalho approved these changes May 13, 2024

View reviewed changes

gusfcarvalho merged commit 0abb3e9 into external-secrets:main May 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

Add support for Authentication against Azure Key Vault using Client Certificate#3469

Add support for Authentication against Azure Key Vault using Client Certificate#3469
gusfcarvalho merged 10 commits intoexternal-secrets:mainfrom
lieberlois:main

lieberlois commented May 8, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

gusfcarvalho commented May 10, 2024

Uh oh!

lieberlois commented May 11, 2024 •

edited

Loading

Uh oh!

gusfcarvalho commented May 11, 2024 •

edited

Loading

Uh oh!

gusfcarvalho commented May 11, 2024

Uh oh!

gusfcarvalho commented May 11, 2024

Uh oh!

lieberlois commented May 11, 2024

Uh oh!

sonarqubecloud bot commented May 11, 2024

Uh oh!

lieberlois commented May 11, 2024

Uh oh!

lieberlois commented May 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Comments

Conversation

lieberlois commented May 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem Statement

Related Issue

Proposed Changes

Checklist

Uh oh!

Uh oh!

Uh oh!

gusfcarvalho commented May 10, 2024

Uh oh!

lieberlois commented May 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gusfcarvalho commented May 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gusfcarvalho commented May 11, 2024

Uh oh!

gusfcarvalho commented May 11, 2024

Uh oh!

lieberlois commented May 11, 2024

Uh oh!

sonarqubecloud bot commented May 11, 2024

Quality Gate passed

Uh oh!

lieberlois commented May 11, 2024

Uh oh!

lieberlois commented May 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lieberlois commented May 8, 2024 •

edited

Loading

lieberlois commented May 11, 2024 •

edited

Loading

gusfcarvalho commented May 11, 2024 •

edited

Loading