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

Ability to encrypt only values that are actually secret #202

Closed
TBeijen opened this issue May 21, 2019 · 4 comments
Closed

Ability to encrypt only values that are actually secret #202

TBeijen opened this issue May 21, 2019 · 4 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@TBeijen
Copy link

TBeijen commented May 21, 2019

As one of the benefits of a gitops style handling of app definition and configuration, is the ability to review any changes, it would be nice if only the values that are actually secret need to be encrypted.

To illustrate, part of one of our applications .env file contents:

# could be human readable
DEBUG=False
GIGYA_TIMEOUT=0.3
GIGYA_DOMAIN=eu1.gigya.com

# obviously a secret
GIGYA_SECRET_KEY="SECRETSECRETSECRET"

# contains secrets, but also parts that would be nice to read normally
DATABASE_URL=mysql://read_user:SECRETSECRET@compl-ex-rds-cluster-name-f039-ro.cluster-abc123fgh987.eu-west-1.rds.amazonaws.com/mydb

Some possible approaches that spring to mind:

Adding ability to init container to combine encrypted and plain configmaps

  • Would go well (needs to be tested) with Kustomize's ConfigMapGenerator mechanism, that appends hash to the resulting ConfigMap name based on contents, implicitly triggering new pods if the config changes.
  • Will not allow parts of config value to be secret.
  • Merging of configs needs to support all available config types (json, cfg, cfg-strict, etc.)
  • Not applicable to KamusSecrets.

Allow opt-in for only decrypting certain values

  • Example: {kamus{SECRETSECRET}}, choosing delimiters that go well with base64 encrypted data, and also shouldn't interfere with common templating delimiters.
    • Delimiters could be configurable as well, if default is not suitable
  • Would affect the init container and the crd controller.
  • Opt-in by means of annotation on KamusSecret or command-line arg on init-container.
  • Would allow parts of value to be secret (the DATABASE_URL example)
@omerlh
Copy link
Contributor

omerlh commented May 21, 2019

Hey @TBeijen! Thanks for raising this issue. I think this is similar to #141 - see the discussion there. I think we should add support for templating mechanism in the init container, similar to Consul Template mechanism. It should solve both that issue and your issue if I understand it correctly. I'll be happy to help you start with a PR :)
With that being said, we used 2 separate config maps - one for the encrypted configs and one for the plain text configs and just merge them in code. This has some advantage as it gives you a clear way to understand what is sensitive and what isn't.
Can you elaborate what you said about adding an annotation to KamusSecret? why would you like to have both clear text and encrypted data in a KamusSecret?

@omerlh omerlh added enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed labels May 21, 2019
@TBeijen
Copy link
Author

TBeijen commented May 24, 2019

Hi @omerlh,

I was aware of #141 but considered it a separate issue, based on the assumption that you would typically like to have the list of key-value pairs that configure an application in the same place (12-factor apps: environment vars or .env files).

If #141 would allow specifying template file and destination, taking key-value pairs as input, you have great flexibility to generate basically any type of config file, while still allowing the input of said templating to be lists of key-value pairs. Such key-value pairs in turn, can be easily modified per destination deployment, using e.g. Kustomize (configMapGenerator) or Helmfile. And with this feature you would have maximum readability in your development workflow.

I suppose for KamusSecret the use case is less obvious, so maybe best to treat that as a separate issue if a use case pops up.

Summarizing: If KamusSecret is out of scope, and we focus on just the init container. Do you think this has value, also if #141 would also be implemented? My argument for would be that it allows to keep all configuration that constitutes a configMap in one place in the manifest, while allowing for easier code-reviews.

@omerlh
Copy link
Contributor

omerlh commented May 26, 2019

Sorry, I'm not following - if you'll look on the PR (#204) you'll see that the templating mechanism allows you to do that. Something like:

{
  "SomeKey": "I'm not sensitive"
  "SomeOtherKey": <%= secrets["key1"] %>
}

Isn't that what you're looking for? What else is missing?

I'll be happy to understand more about the KamusSecret flow - please open a new issue for that.

@TBeijen
Copy link
Author

TBeijen commented May 26, 2019

Aha, I missed that PR on #141. Looking at the templating docs there, that looks quite nice indeed.

Thanks for taking time to discuss this and the #204 PR! I'll be closing this one.

Note to self: Don't discuss GH issues over the course of days in short fragments without proper focus.

@TBeijen TBeijen closed this as completed May 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants