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

Manage secrets with sealed secrets #462

Merged
merged 11 commits into from May 1, 2023
Merged

Manage secrets with sealed secrets #462

merged 11 commits into from May 1, 2023

Conversation

jjnesbitt
Copy link
Collaborator

@jjnesbitt jjnesbitt commented Apr 12, 2023

Currently, secrets are managed in an ad-hoc way, where the secret is created in the cluster, and then added here in a completely commented file named secrets-dummy, purely for reference. Sealed Secrets provide a way for us to securely commit our secrets into our codebase, without the need for any workarounds.

This PR consists of the following:

  1. The necessary helm repositories/charts for sealed secrets
  2. SealedSecret resource defintions for all of the secrets which had a secrets-dummy file committed. These contain the encrypted data pulled from the live cluster
  3. Scripts to help facilitate the management of secrets
  4. Documentation detailing our use of sealed secrets

For all of the secrets encoded in the sealed-secrets.yaml files included in this PR, I've added the "sealedsecrets.bitnami.com/managed" = "true" annotation, which will allow this change to take effect correctly.

@jjnesbitt jjnesbitt force-pushed the sealed-secrets branch 14 times, most recently from 05e1f75 to 058311b Compare April 17, 2023 16:47
@jjnesbitt
Copy link
Collaborator Author

jjnesbitt commented Apr 19, 2023

Waiting to merge this until #404 is merged. I'll then rebase this off of that and deal with conflicts, as well as configure staging, since staging isn't currently covered by this PR.

UPDATE: This has been completed.

@jjnesbitt jjnesbitt force-pushed the sealed-secrets branch 4 times, most recently from 3bf19e1 to 187d6a8 Compare April 20, 2023 00:04
@jjnesbitt jjnesbitt marked this pull request as ready for review April 20, 2023 18:29
mvandenburgh
mvandenburgh previously approved these changes Apr 25, 2023
Copy link
Collaborator

@mvandenburgh mvandenburgh left a comment

Choose a reason for hiding this comment

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

Looks good to me, although i haven't tested it.

@mvandenburgh
Copy link
Collaborator

Should we disable flux in production, merge this, and re-enable it when we're sure it works ok in staging? I'm not sure the best way to test all of this tbh, unless you've already done so.

@jjnesbitt
Copy link
Collaborator Author

Should we disable flux in production, merge this, and re-enable it when we're sure it works ok in staging? I'm not sure the best way to test all of this tbh, unless you've already done so.

Just to clarify, I have already tested this to the best of my ability by doing the following:

  1. Applied one or more of these files (under a different name) to the cluster, checking the contents of the unsealed secret and verifying that it's correct
  2. Verified that the private and public key pair match by temporarily storing the private key locally and using the kubeseal recovery mode to decrypt the secret files locally, verifying that their contents are what they should be
  3. Applied the "sealedsecrets.bitnami.com/managed" = "true" annotation to all existing secrets that have corresponding sealed-secrets.yaml files in this PR
  4. Verified that an existing secret which has the above annotation can be correctly "taken control of" by the sealed secrets manager (i.e. that the sealed secret would be unsealed and the regular secret data updated without issue, even though all sealed data in this PR is the same as the existing secrets).

I'm still happy to do a test in staging first, but I don't think it'll really be testing what we care about, since the staging cluster uses a different key pair, and doesn't contain most of the secrets we're encoding here. Essentially what I'm saying is, I think the most meaningful test is verifying that we've sealed all of the existing secret data correctly, which I've done to the best of my knowledge.

@mvandenburgh
Copy link
Collaborator

I'm still happy to do a test in staging first, but I don't think it'll really be testing what we care about, since the staging cluster uses a different key pair, and doesn't contain most of the secrets we're encoding here. Essentially what I'm saying is, I think the most meaningful test is verifying that we've sealed all of the existing secret data correctly, which I've done to the best of my knowledge.

Agreed, sounds like you've tested this reasonably so I think it's good to merge 👍

@jjnesbitt jjnesbitt merged commit ea5e819 into main May 1, 2023
1 check passed
@jjnesbitt jjnesbitt deleted the sealed-secrets branch May 1, 2023 17:02
@mvandenburgh mvandenburgh mentioned this pull request Jun 20, 2023
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

2 participants