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

Vault Jsonnet Native Function #101

Closed
forestsword opened this issue Oct 30, 2019 · 15 comments
Closed

Vault Jsonnet Native Function #101

forestsword opened this issue Oct 30, 2019 · 15 comments
Labels
component/jsonnet Everything regarding the jsonnet language kind/feature Something new should be added stale

Comments

@forestsword
Copy link

I'd love to be able to pull secrets from vault when I apply. We do this already with tooling we have but would like to start using tanka. Any reason why we shouldn't or can't have this? Or how are others handling their secrets with tanka?

For example when I apply this jsonnet tanka would use the standard vault envvars VAULT_ADDR and VAULT_TOKEN to create a client for use with the native jsonnet function:

{
  secret: {
    apiVersion: 'v1',
    kind: 'Secret',
    type: 'Opaque',
    metadata: {
      name: 'secret',
    },
    data: {
      password: std.native('vault')('secret/path', 'password'),
    },
  },
}
@issue-label-bot issue-label-bot bot added the kind/feature Something new should be added label Oct 30, 2019
@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the label kind/feature to this issue, with a confidence of 0.88. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

@sh0rez
Copy link
Member

sh0rez commented Oct 30, 2019

I like that idea!

Internally at Grafana, we use git-crypt to store secrets gpg-encrypted in the git repository. However, we have already hit the limitations of this concept (hard to rotate secrets, no tight access control).

I think we can definitely implement a native function for pulling secrets from vault as long as it is documented thoroughly :D

@sh0rez sh0rez added the component/jsonnet Everything regarding the jsonnet language label Oct 30, 2019
@sbarzowski
Copy link

(Upstream Jsonnet dev chiming in.)

Perhaps these can be passed as regular extvars? Tanka could have an integration with the Vault, have the keys to pull and connection info specified somewhere and fetched before the evaluation of the script. They could then be passed to Jsonnet as ExtVars, which is one of the intended mechanism of passing data to Jsonnet code.

@sh0rez
Copy link
Member

sh0rez commented Oct 30, 2019

I have thought about this idea as well ... However, I didn't come up with a nice way of specifying which secrets to obtain from vault.

Vault is basically a very secure key-value store. Tanka on the other hand uses Jsonnet code to define Kubernetes objects where the Jsonnet is the single source of truth.

As far as I understand, extVars need to be statically resolved at the time the Jsonnet evaluation is invoked. However, at that point we do not yet know which secrets the user might need. Letting them specify in advance feels overcomplicated, as there would be another source of truth involved which needs to be maintained. Another option would be to pull all secrets from vault into a dict, but as secrets should be accessed sparingly I would vote against that as well.

Unless I miss something, it looks as if pulling dynamically during evaluation has the least drawbacks. WDYT?

@forestsword
Copy link
Author

I agree @sbarzowski I would use extvars if I were using the jsonnet binary and I'd wrap it all up into a Makefile. But I see that as the enticing part of tanka and in general any tool using the jsonnet go library. That we can create tools for very specific workflows, like tanka is doing for us with k8s, with all the things we need in one binary. Ok two here if you count kubectl.

A drawback I also see is that the configuration of all of those secrets is another set of configs I have to maintain. And probably I'd have to maintain those in a different fashion then the jsonnet that I'm using to render the k8s resources. Like @sh0rez I also don't see a nice way to do this. Plus I'll need an additional script or something to take that input then actually call vault so that I finally get it all in the form that I need to finally put it in the extvars.

If it's here it's all nice and clean. Or should I open a pull-request on https://github.com/google/jsonnet 🤔

@sbarzowski
Copy link

sbarzowski commented Oct 30, 2019

@sh0rez

As far as I understand, extVars need to be statically resolved at the time the Jsonnet evaluation is invoked.

That's right.

However, at that point we do not yet know which secrets the user might need.

I think this is something that a user may want to statically find out. Being able to tell what is calculated based on what data is kinda what Jsonnet is about :-). Having some deep part of the code fetch arbitrary secrets - and not even knowing what secrets are needed for the whole config to run - sounds not ideal.

However, specifically with Vault, I think this is not so bad. Vault has its own concept of roles and grouping of who-can-access-what, so being pedantic about it on Jsonnet side may be not worth it.

Letting them specify in advance feels overcomplicated

Well, that would require some additional manifest (probably in JSON or Jsonnet) which needs to be processed before the main evaluation.

I would consider that the most "proper" solution. But I don't know if it's pragmatically worth it.

Unless I miss something, it looks as if pulling dynamically during evaluation has the least drawbacks.

Hmmm... FYI native functions were designed for pure computation and not IO, which means quite a lot of things would need to be done on Tanka side to provide the optimal experience.

I would recommend doing the following if native functions are going to be used for fetching values from Vault (or other external datasource):

  • Cache request results within one evaluation (ensure purity within one execution, even if a secret changes during the evaluation).
  • Have a way to produce a log of accessed secrets.
  • Have a way of specifying these secret values from file (e.g. for development and debugging purposes). Ideally adding other key-value secret backends would be possible in the future.
  • In the user documentation, recommend to create separate file(s) which assign secrets to object fields instead of sprinkling the calls to tanka.secret('some-random-string') over the codebase.

@forestsword

If it's here it's all nice and clean. Or should I open a pull-request on https://github.com/google/jsonnet 🤔

More convenient upstream support for data input (including loading of secrets) would definitely be a good thing. This issue is definitely one of the sources of inspiration for that. Still, I think it's best to start with Tanka.

@shubhanshus
Copy link

I would like to pick this up. @sh0rez Do we have any Contribution guidelines?

@sh0rez
Copy link
Member

sh0rez commented Nov 1, 2019

I would like to pick this up. @sh0rez Do we have any Contribution guidelines?

Awesome! No, we do not really have any specific rules on contributing .. try to keep your changes as small and focused as possible, use good commit messages (what and why), address linter suggestions (golangci-lint) if they make sense, everything else will resolve during review :D

Regarding the design of this feature, I like what @sbarzowski outlined above and I would still go with a std.native func.

One thing that seems nice is that the parameters to this func are kept general enough to be swapped out with other kv stores (not only vault), to allow easier local development, e.g:

// local:
std.native("gpgSecret")("/home/user/.secrets/supersecret.yaml.gpg", "field")

// production:
std.native("vaultSecret")("path/to/secret", "field")

I look forward to see your solution. Reach out to me here or on Slack if there are any questions / open points!

@stale
Copy link

stale bot commented Dec 1, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 1, 2019
@sh0rez sh0rez removed the stale label Dec 1, 2019
@stale
Copy link

stale bot commented Dec 31, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 31, 2019
@stale stale bot closed this as completed Jan 7, 2020
@sh0rez sh0rez reopened this Jan 7, 2020
@stale stale bot removed the stale label Jan 7, 2020
@sh0rez
Copy link
Member

sh0rez commented Jan 7, 2020

Reopening, as we are still planning to ship this, even though it might be delayed further because of our focus being put on other things at the moment

@davidovich
Copy link
Contributor

Instead of binding to a specific secret management solution (and implementing the plethora of different combinations), would it be possible to consider delegating to something like sops (which already has many secret backends, and Hashicorp Vault support is being developed currently).

@stale
Copy link

stale bot commented Mar 28, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 28, 2020
@stale stale bot closed this as completed Apr 4, 2020
@davidovich
Copy link
Contributor

@sh0rez this was closed without any comments (after removing the keepalive label), could you talk about the rationale ?

@rrichardson
Copy link

Is there a new recommended approach for managing secrets? Or are we still stuck with pulling down ExtVars from Vault?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/jsonnet Everything regarding the jsonnet language kind/feature Something new should be added stale
Projects
None yet
Development

No branches or pull requests

6 participants