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

SOPS: add support for Azure Key Vault credentials using SecretRef #495

Merged
merged 7 commits into from
Mar 14, 2022
Merged

SOPS: add support for Azure Key Vault credentials using SecretRef #495

merged 7 commits into from
Mar 14, 2022

Conversation

dquagebeur
Copy link
Contributor

@dquagebeur dquagebeur commented Nov 19, 2021

This PR is to support Azure Keyvault for SOPS.
Azure credentials have to be provided in the SOPS secret under the key sops.azure-kv.

@dquagebeur
Copy link
Contributor Author

I have rebased my commit on the main branch to remove the preBuild dependency.
As we already use it, could you give your feedback on this PR too (#430). If you agree with it, after this PR, I will rebase the other one.

Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

Will try to get this reviewed on Monday, thanks in advance @dquagebeur 🙇

.gitignore Outdated Show resolved Hide resolved
@stefanprodan
Copy link
Member

This PR is to support Azure Keyvault for SOPS.

This is misleading, Flux does supports Azure Key Vault, here is the e2e tests that proves this https://github.com/fluxcd/flux2/blob/main/tests/azure/azure_test.go#L525

@hiddeco hiddeco self-assigned this Nov 19, 2021
@stefanprodan stefanprodan changed the title Add support for Azure Keyvault SOPS: Add support for Azure Key Vault using security principal auth Nov 19, 2021
@dquagebeur
Copy link
Contributor Author

Yes, it's true.
To be exact, this PR adds a per namespace support for Azure Key Vault .
With it, because credentials are provided as a namespace's secret, each namespace can have its own key vault and be sure that other namespaces cannot decrypt its secrets.

@stefanprodan
Copy link
Member

To be exact, this PR adds a per namespace support for Azure Key Vault .

Right! This makes Flux usable on multi-tenant AKS clusters, it does solve the issue raised in #324 but only for Azure Key Vault.

@stefanprodan
Copy link
Member

main.go Outdated Show resolved Hide resolved
@dquagebeur
Copy link
Contributor Author

hi @hiddeco, do you have any more changes before validating the PR ?

@hiddeco hiddeco added area/sops SOPS related issues and pull requests enhancement New feature or request labels Dec 7, 2021
internal/sops/azkv/keysource.go Outdated Show resolved Hide resolved
internal/sops/azkv/keysource.go Outdated Show resolved Hide resolved
@hiddeco
Copy link
Member

hiddeco commented Dec 9, 2021

I need to have another round on Microsoft internals on how they present things to their (groups) of users, as I am getting the sense that there are multiple instructions out there that people may use as a starting point. Meaning that we either need to guide them to not do this, or support multiple JSON structures.

In addition, I need to have another look at the (various) SDK(s) we built upon, to ensure that besides rate limiting, I am not missing another setting to facilitate smooth operations. Thanks for making me aware of this @phillebaba 🙇

In any case, and given this touches the area of security, I do not want to rush this. Which means that with an eye on the holiday period that's arriving soon, many of us enjoying some well deserved time off then, and my current pile of TODOs. I think this is more likely to land at the beginning of 2022.

dquagebeur and others added 5 commits March 14, 2022 10:27
Signed-off-by: David Quagebeur <david.quagebeur@worldline.com>
- Ensure key source follows upstream SOPS contracts as closely as
  possible (e.g. `MasterKey` interface).
- Prevent unnecesary FS operations by allowing token creation and
  and authorizer configuration to be factored from file bytes.
- Ensure a limited number of configuration option is taken into
  account, excluding e.g. file path references.
- Ensure server maintains backwards compatibility with previously
  supported "global" Azure configuration, _without_ relying on file
  assumptions and/or inspections (but rather, server configurations).

Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Hidde Beydals <hello@hidde.co>
This updates to the `github.com/Azure/azure-sdk-for-go` SDK, which is
the (apparent) successor of the previous SDK, and allows for easier
configuration of credentials through the `azidentity` package.

Signed-off-by: Hidde Beydals <hello@hidde.co>
This supports the fields as documented in the AKS documentation:
https://docs.microsoft.com/en-us/azure/aks/kubernetes-service-principal?tabs=azure-cli#manually-create-a-service-principal

Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Hidde Beydals <hello@hidde.co>
This includes a refactor of the other entries, to start moving guides
to the website while containing minimal technical (instructions)
in-spec.

Signed-off-by: Hidde Beydals <hello@hidde.co>
@hiddeco hiddeco changed the title SOPS: Add support for Azure Key Vault using security principal auth SOPS: add support for Azure Key Vault credentials using SecretRef Mar 14, 2022
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @hiddeco and @dquagebeur 🏅

@hiddeco hiddeco merged commit 423cdde into fluxcd:main Mar 14, 2022
@marshallford
Copy link

Out of curiosity, what was the reasoning behind changing the pinned version of kustomize from v4.5.2 to v4.4.1? I often look to that comment to determine which version of kustomize to use elsewhere in my CI/testing process to best replicate how the controller will render manifests.

@stefanprodan
Copy link
Member

@marshallford the version got bork during merge, we’ll do a patch release today with the right one, thanks for reporting this.

@dquagebeur dquagebeur deleted the feature/azurekv branch May 20, 2022 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/sops SOPS related issues and pull requests enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants