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

Secrets should be predeployed before tasks #209

Closed
agilgur5 opened this issue Nov 22, 2017 · 11 comments
Closed

Secrets should be predeployed before tasks #209

agilgur5 opened this issue Nov 22, 2017 · 11 comments

Comments

@agilgur5
Copy link

I have a task / bare pod that has part of its environment set from a Secret. These pods fail right now because Secrets are deployed after them

@KnVerey
Copy link
Contributor

KnVerey commented Nov 22, 2017

The supported way to manage secrets with kubernetes-deploy is to use EJSON to store them in encrypted form: https://github.com/Shopify/kubernetes-deploy#deploying-kubernetes-secrets-from-ejson. Secrets managed this way will be correctly created/updated before anything else. I would not recommend keeping kubernetes secrets unencrypted in your source.

@agilgur5
Copy link
Author

agilgur5 commented Nov 22, 2017

I'm currently inserting Secrets at deploy time only, so they're of course not in source control (if I had to choose, I would prefer to use Vault or Bitnami's Sealed Secrets so they're not so easily accessible in Kubernetes as well). I wouldn't think it would be difficult to support regular Secrets as you'd just skip the decryption part. Since Secrets are a known / larger problem in Kubernetes, there are many ways to deal with it out there in general, so I'm just not sure that not supporting them outright is a best practice (I'm assuming that was an intentional design decision)

@KnVerey
Copy link
Contributor

KnVerey commented Jan 9, 2019

I'm assuming that was an intentional design decision

Correct, we intentionally added ejson-to-secret support instead to encourage safe secret management. It wouldn't be difficult to predeploy regular secrets, but I'm hesitant to support that for best-practice reasons as you said. We can revisit this if there is more demand for it.

@KnVerey KnVerey closed this as completed Jan 9, 2019
@agilgur5
Copy link
Author

agilgur5 commented Jan 9, 2019

Er, I said that I didn't believe that's a best practice. Unless there are plans to support all sorts of secrets providers and mechanisms (I don't believe there are), regular Secrets should be supported -- it's up to the developer to handle them correctly. The current approach creates lock-in to EJSON and does not allow other secret mechanisms.

A good alternative that would give developers freedom would be to hide regular Secrets behind some flag, thereby requiring explicit confirmation of potential pitfalls.

@agilgur5
Copy link
Author

agilgur5 commented Jan 9, 2019

The current approach is: "You don't know what you're doing, so you must use our preferred method, EJSON" vs. a flag would be: "This is not recommended, only use this if you know what you're doing".

The underlying assumptions are quite different between the two.

@KnVerey
Copy link
Contributor

KnVerey commented Jan 10, 2019

Sorry for my misreading of your initial comment. We're open to your idea of adding a flag that enables predeployment of raw secrets. Then, if the flag isn't provided and we discover a raw secret among the resources, we can fail validation early in the process with a helpful message about the risks and the flag instead of just printing the unsupported resource warning as we do today. Implementing this is not a priority for us, but we would accept a PR.

@KnVerey KnVerey reopened this Jan 10, 2019
@asabourin
Copy link

We're in a similar situation where we've got a secrets.yml.erb with the following:

apiVersion: v1
data:
  RAILS_MASTER_KEY: <%= Base64.strict_encode64(File.read("#{Dir.pwd}/config/credentials.yml.enc").strip) %>
kind: Secret
metadata:
  name: secrets

It works well with Rails encrypted credentials system and is simple and easy to set up for developers. The only issue is that it's not deployed first by kubernetes-deploy. Instead of logic specific to secrets perhaps there could be a way to force/override the order of deployment of a given resource?

@KnVerey
Copy link
Contributor

KnVerey commented Feb 14, 2019

After further discussion among the maintainers and conversations with internal users, we have decided to change our position. We will plan to support Secrets like any other resource, no flags required. We will also look into adjusting our Ejson secret provisioner to stop managing secret lifecycles (e.g. pruning) itself, for consistency between the two secret sources.

@dturn
Copy link
Contributor

dturn commented Feb 25, 2019

@DazWorrall is your PR fixing this?

@DazWorrall
Copy link
Member

Looking at the OP and Katrina’s last comment: yes I think #424 will resolve this issue.

@KnVerey
Copy link
Contributor

KnVerey commented Mar 1, 2019

We just merged #424, which adds support for plain Secrets and makes secrets whose data we generated from Ejson get deployed the same way as other resources (they are now applyd). This means there's no longer an "unsupported resource" warning when you deploy a secret, and secrets are both predeployed and pruned.

Please note that this may require an upgrade process for some organizations. As documented in our changelog:

  • If you previously used this gem to deploy secrets from EJSON and the first time commit you deploy using this version removes one or more of them, they will not be pruned.
  • If you previously manually kubectl apply'd secrets that are not passed to kubernetes-deploy, your first deploy using this version is going to delete them.
  • If you previously passed secrets manifests to kubernetes-deploy and they are no longer in the set you pass to the first deploy using this version, it will delete them.

Secrets are now like any other supported resource: if you want kubernetes-deploy to ignore one, you must create it not apply it. If you have secrets you've already applied out of band, you can simply remove the kubectl.kubernetes.io/last-applied-configuration annotation from them.

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

No branches or pull requests

5 participants