hashicorp / faas-nomad

OpenFaaS plugin for Nomad
https://www.openfaas.com
MIT License
254 stars 46 forks source link

Limited Ability to use Vault Secrets. #60

Open ryanmickler opened 5 years ago

ryanmickler commented 5 years ago

I've been trying to integrate openfaas-nomad into our stack, but a lot of our functions require access to various AWS S3 buckets to read/write data. The problem is that the openfaas-nomad vault integration only can access secrets at secrets/${secrets_dir}/${secret_name}. However, for our AWS secrets, we'd need to read secrets at aws/creds/${secrets_name}. Can i propose that we include the ability to customize this vault policy, instead of VaultDefaultPolicy: "openfaas"

Secondly, it would also be required to be able to specify the template that is rendered, as each read of the aws secrets generates a new set of two keys that need to be rendered simultaneously. The default template format {{with secret "secret/${vault_prefix}/${secret_name}"}}{{.Data.${name}}}{{end}} isn't suitable for this purpose.

So i propose that we also provide the ability to specify the policy and pass in a template block

functions:
  facedetect:
    lang: go-opencv
    handler: ./facedetect
    image: nicholasjackson/func_facedetect
    secrets:
      - mysecret 
        vault_policy: my_s3_secret_vault_policy
        template: |
[default] {{with secret "aws/creds/my_s3_secret"}}
aws_access_key_id={{.Data.access_key}}
aws_secret_access_key={{.Data.secret_key}}{{end}}

the vault policy my_s3_secret_vault_policy would just allow read access to aws/creds/my_s3_secret.

This example would generate a suitable AWS_CREDENTIALS_FILE at /etc/openfaas/secrets/mysecret that could be used to provide AWS api access.

alexellis commented 5 years ago

Hi @ryanmickler I'd suggest bubbling this issue up to the upstream repo on openfaas/faas where we could discuss your needs and requirements in a bit more detail.

If you'd like to bind to environmental variables there are some ways to do that, but AWS doesn't strictly need to use env-vars to read secrets. cc @ewilde

Alex

ryanmickler commented 5 years ago

Thanks for the quick reply. However, after looking through the openfaas-nomad source, i believe that my desired functionality could be accomplished entirely within this repo, without having to modify the core openfaas source.

The only modifications would be to the openfaas function definition yaml, by including the vault_policy and template strings.

In fact, i believe it could only be done in this repo, as we'd need to directly interact with the nomad API.

acornies commented 5 years ago

Hi Ryan, thanks for opening this issue. I think part of what you're asking for is already implemented: vault_default_policy and vault_secret_path_prefix can be overridden. However, there are no function-specific overrides and it does currently expect this template format:

{{with secret "%s"}}{{.Data.%s}}{{end}}

translates to

{{with secret "secret/openfaas/facedetect"}}{{.Data.mysecret}}{{end}}

My plan was to implement function-specific overrides for this exact purpose, but it would take the form of adding annotations to a function rather than change the behaviour of secrets in the faas-cli.

ryanmickler commented 5 years ago

hmmm. im not sure how the annotations to a function would work, but it sounds interesting. But you are spot on, a function-specific override of vault_default_policy, vault_secret_path_prefix, and the template would satisfy my use case. Its just a question of how that would be specified.

acornies commented 5 years ago

My idea was this:

functions:
  facedetect:
    lang: go-opencv
    handler: ./facedetect
    image: nicholasjackson/func_facedetect
    secrets:
      - aws_access_key_id
      - aws_secret_access_key
    annotations:
      vault_policy: my_s3_secret_vault_policy
      vault_path_prefix: aws/creds

this would get you what you want using openfaas' standard format for secrets. I'll have to think on how to specify a whole secret template so that it gels nicely with the rest of that faas ecosystem.

ryanmickler commented 5 years ago

The problem still remains that the aws_access_key_id and aws_secret_access_key have to be accessed on the same vault read call, i.e. {{ with secret .. }}. So they should really be treated as a single secret with two values, rather than a pair.

ryanmickler commented 5 years ago

Furthermore, for injecting multiple secrets, you'd want at least to have a separate vault_path_prefix per secret. And probably have a separate vault_policy too, however thats not a critical requirement.

acornies commented 5 years ago

Remembering what @nicholasjackson was talking about earlier, how about this:

functions:
  facedetect:
    lang: go-opencv
    handler: ./facedetect
    image: nicholasjackson/func_facedetect
    secrets:
      - aws/creds/my_s3_secret <-- with vault key prefix
    annotations:
      vault_policy: my_s3_secret_vault_policy
      vault_template: |
        aws_access_key_id={{.Data.access_key}}aws_secret_access_key={{.Data.secret_key}}

We could detect the path in the secret name and use that to override the defaultkey prefix. The only thing I don't like about this is how it starts to deviate from how OpenFaaS manages secrets in other providers. Thoughts @alexellis ?

ryanmickler commented 5 years ago

Yeah, however, just for a bit of future proofing, we want to inject two secrets that require separate custom templates. Is there no way to embed the extra data (policy, template) as a child of the ‘secret’ node in the yaml without violating the openfaas standards for secrets, as in my original post? I think using these annotations seems confusing.

acornies commented 5 years ago

I'm thinking of the broader OpenFaaS ecosystem with my suggestion of annotations. Changes would have to be made to upstream faas gateway implementation to do what you're talking about in the original issue. Those changes wouldn't exactly be relevant for other providers, although we are currently developing new secret management tooling for faas-cli/gateway components.

We can support what you want by adding additional functionality through some sort of annotation convention on a function by function basis.

Maybe: vault_template_{secret_name}

secrets:
  - aws/creds/my_s3_secret <-- with vault key prefix
  - another_secret <-- default prefix
annotations:
  vault_policy: my_s3_secret_vault_policy
  vault_template_my_s3_secret: "aws_access_key_id={{.Data.access_key}}aws_secret_access_key={{.Data.secret_key}},aws/creds/my_s3_secret"
  vault_template_another_secret: "{{.Data.value}}"
ryanmickler commented 5 years ago

I was going to suggest something pretty much identical to this solution. This would meet my use case. Thanks!

ryanmickler commented 5 years ago

So can i suggest just a slight modification to your solution

functions:
  facedetect:
    lang: go-opencv
    handler: ./facedetect
    image: nicholasjackson/func_facedetect
    secrets:
      - mysecret 
    annotations:
        vault_policy_mysecret: my_s3_secret_vault_policy
        vault_template_mysecret: |
[default] {{with secret "aws/creds/my_s3_secret"}}
aws_access_key_id={{.Data.access_key}}
aws_secret_access_key={{.Data.secret_key}}{{end}}

The template file will be rendered to /etc/openfaas/secrets/mysecret. This seems to be a bit cleaner.

nicholasjackson commented 5 years ago

@ryanmickler, I am aware that this has been open for a number of days, really do want to get a resolution for this but don't want to break OpenFaaS standard compatibility. Will try and see if we can get a proposal into the main core project to support Vault secrets. @alexellis are you free to chat about this maybe at KubeCon next week?

ryanmickler commented 5 years ago

Thanks for the update. Let me know if i can help.

acornies commented 5 years ago

@ryanmickler circling back on this - I haven't had a chance to implement what we talked about here since we needed to support the secrets api faas-cli secret ... interaction first. Have you found a workaround? I'm also open to PRs if you've got some spare cycles.

ryanmickler commented 5 years ago

@acornies, sorry i haven't had a chance, and we've been using a local gateway for s3 access without auth as a workaround. I might be able to put one of my team onto it in a few weeks though. Were we happy with my proposed format for the yaml as above?

acornies commented 5 years ago

I think the format above is still valid since we're talking about providing a custom consul-template to the function. Since faas-nomad now supports faas-cli secret ... commands, these get submitted to vault using {secret_prefix}/{policy_name}/mysecret {"value": "secretvalue"}. If you want to use this format above, you'd still have to manage secrets using Vault directly and then reference the policy defined above in the function yaml or deploy args.

Revamping this provider with Nomad 0.9 deps and a few other things are further up on my priority list, so if you could get one of your team members to spend some, I'll test out the PR when ready.