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

helm secrets with age #301

Merged
merged 11 commits into from
Dec 10, 2021
Merged

helm secrets with age #301

merged 11 commits into from
Dec 10, 2021

Conversation

gerardcl
Copy link
Member

@gerardcl gerardcl commented Nov 23, 2021

Fixes #293

Tasks:

  • Updated design documents in docs/design directory or not applicable
  • Updated user-facing documentation in docs directory or not applicable
  • Ran tests (e.g. make test) or not applicable
  • Updated changelog or not applicable

@gerardcl
Copy link
Member Author

gerardcl commented Nov 23, 2021

first commit with changes on the Dockerfile.helm file -> helm-secrets to latest version, add sops and age explicitly
missing: docs update (followup from #292), change wherever pgp is used to age (secrets in tests, etc), something else?

@michaelsauter is this first commit ok? I did this way since we only want the binaries at the end...

@michaelsauter
Copy link
Member

Yep, that looks about right. Keep in mind that this commit should also update https://github.com/opendevstack/ods-pipeline/blob/master/docs/design/software-design-specification.adoc. That document tracks what gets installed and in which version.

@gerardcl gerardcl force-pushed the feature/helm-secrets-with-age branch 3 times, most recently from 32625bf to f2c6e6e Compare November 29, 2021 06:46
@gerardcl gerardcl marked this pull request as ready for review November 29, 2021 06:47
@gerardcl
Copy link
Member Author

@michaelsauter from what I have understood, until now we had kinda not mandatory having the helm-private-key-secret in place in the tests and so on, but it looks that anyhow it is required by design, or am I wrong? Do we make it required so the tests do not fail in the case it is not existing (by making it always to exist)? or we should keep it being not mandatory?

@michaelsauter
Copy link
Member

@gerardcl See https://github.com/opendevstack/ods-pipeline/blob/master/test/tasks/ods-deploy-helm_test.go#L62. That sets up the private key. I think you need to exchange that for age.

@gerardcl
Copy link
Member Author

@michaelsauter I thought about that but was not sure, so in this case I understand instead of importing via executing gpg import I will need to make it available in the context env when executing helm command. ok! let's see!

@gerardcl gerardcl force-pushed the feature/helm-secrets-with-age branch from 13ecc2f to 4d895db Compare November 30, 2021 09:43
@gerardcl
Copy link
Member Author

@renedupont I updated user docs from your update (added multiple recipients note) and updated related part on admin installation too 👍

matching a fingerprint listed in `.sops.yaml`.
identified by the `age-key-secret` parameter exists, and contains an age secret key
which public key was used as one of the recipients to encrypt. The age-key file location
is known via `SOPS_AGE_KEY_FILE` environment variable (e.g.: `SOPS_AGE_KEY_FILE=/sops/age/keys.txt`)
Copy link
Member Author

Choose a reason for hiding this comment

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

that is under construction still, that comment there explains the current approach just for the task in here:

anyhow, I am not sure if that will be the final approach yet

@gerardcl
Copy link
Member Author

@michaelsauter ready for review 👍

@gerardcl gerardcl requested review from henrjk and renedupont and removed request for renedupont December 1, 2021 10:10
Copy link
Member

@henrjk henrjk left a comment

Choose a reason for hiding this comment

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

This looks good to me. Note that I did not actually try it out though.

@gerardcl gerardcl self-assigned this Dec 4, 2021
Copy link
Member

@michaelsauter michaelsauter left a comment

Choose a reason for hiding this comment

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

Awesome! This looks really good. Thanks!

I had a few nitpicks (sorry!), and wanted to ask if you could also update docs/design/software-design-specification.adoc? It would be good to record the updated versions and add a new software item for age.

build/package/Dockerfile.helm Outdated Show resolved Hide resolved
cmd/deploy-with-helm/main.go Outdated Show resolved Hide resolved
docs/helm-secrets.adoc Outdated Show resolved Hide resolved
docs/helm-secrets.adoc Outdated Show resolved Hide resolved
@gerardcl gerardcl force-pushed the feature/helm-secrets-with-age branch from b9b4db4 to b8972f0 Compare December 9, 2021 17:00
Copy link
Member

@michaelsauter michaelsauter left a comment

Choose a reason for hiding this comment

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

👌

@michaelsauter michaelsauter merged commit 692ac4e into master Dec 10, 2021
@michaelsauter michaelsauter deleted the feature/helm-secrets-with-age branch December 10, 2021 07:30
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.

ods-deploy-helm support age for secret management
4 participants