Skip to content

Conversation

natescherer
Copy link
Contributor

@natescherer natescherer commented Jul 10, 2025

Apologies for the confusion on opening this PR earlier than I intended to.

This PR adds support for being able to choose which authentication method you use for Azure Key Vault, as well as correcting the documentation which incorrectly described how the azidentity Go module currently handled authentication.

Please let me know if you want me to update or change anything!

@natescherer
Copy link
Contributor Author

This also fixes #408

@yxxhero
Copy link
Member

yxxhero commented Jul 11, 2025

@natescherer natescherer force-pushed the azure-key-vault-auth branch from a56d0a1 to e4ffdcb Compare July 12, 2025 06:44
@natescherer
Copy link
Contributor Author

}
chain = append(chain, cred)
default:
panic("Environment variable 'AZKV_AUTH' is set to an unsupported value!")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not return error?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

@natescherer natescherer Jul 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My assumption was we should cause the entire rendering to fail immediately if someone provides a malformed variable value as vals (and any tools calling it) won't produce the desired output if the auth type can't be determined.

If you disagree, though, I'm happy to refactor to throw an error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@natescherer we should not use panic as mush as we can.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I refactored, built and tested, let me know what you think.

@natescherer natescherer force-pushed the azure-key-vault-auth branch from 208f6ea to 545e58b Compare July 14, 2025 00:17
@natescherer natescherer force-pushed the azure-key-vault-auth branch from 545e58b to 1700ba2 Compare July 14, 2025 00:56
@yxxhero yxxhero merged commit 1fca1e9 into helmfile:main Jul 14, 2025
5 checks passed
@yxxhero yxxhero linked an issue Jul 14, 2025 that may be closed by this pull request
CorentinPtrl pushed a commit to CorentinPtrl/vals that referenced this pull request Oct 8, 2025
* feat: add AZKV_AUTH env var for azure key vault

Signed-off-by: Nate Scherer <[email protected]>

* fix: switch to using NewChainedTokenCredential

Signed-off-by: Nate Scherer <[email protected]>

* fix: append to slices

Signed-off-by: Nate Scherer <[email protected]>

* fix: properly append to slices

Signed-off-by: Nate Scherer <[email protected]>

* docs: update azure key vault docs

Signed-off-by: Nate Scherer <[email protected]>

* refactor: return error instead of panicking

Signed-off-by: Nate Scherer <[email protected]>

---------

Signed-off-by: Nate Scherer <[email protected]>
Signed-off-by: CorentinPtrl <[email protected]>
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.

AZURE_USE_MSI switch does not do anything

2 participants