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

SecretTemplate supports ytt templating #70

Open
Samze opened this issue May 31, 2022 · 8 comments
Open

SecretTemplate supports ytt templating #70

Samze opened this issue May 31, 2022 · 8 comments
Labels
awaiting-input carvel-accepted This issue should be considered for future work and that the triage process has been completed enhancement This issue is a feature request priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.

Comments

@Samze
Copy link
Contributor

Samze commented May 31, 2022

Describe the problem/challenge you have

#66 Introduced the SecretTemplate API. This allows you to define arbitrary input resources that are used to generate a new Kubernetes Secret based on templating.

For a first pass, we just supported a simple jsonpath based template. However for more dynamic cases we should support a ytt templating. This would allow the entire generated secret to be templated.

Describe the solution you'd like
We should introduce an optional ytt block as an alternative to template that loads input resources as data.values. So this might look like:

---
apiVersion: secretgen.k14s.io/v1alpha1
kind: SecretTemplate
metadata:
  name: generated-secret
spec:
  serviceAccountName: my-resource-reader
  inputResources:
  - name: rds
    ref:
      apiVersion: rds.services.k8s.aws/v1alpha1
      kind: DBInstance
      name: my-rds-instance
  - name: creds
    ref:
      apiVersion: v1
      kind: Secret
      name: {.rds.masterPassword.name}
      namespace: rds-services
  ytt: | 
    #@ load("@ytt:data", "data")
    
    # Store the input resource as data values with the key being 
    # the specified reference name.
    #@ rds = data.values.rds
    #@ creds = data.values.creds
    #@ endpoint = rds.status.endpoint
    metadata:
      name: db-instance-secret
    stringData:
      type: postgresql
      database: #@ rds.spec.dbName
      port: #@ endpoint.port
      host: #@ endpoint.address
      # Example of defaulting.
      username: #@ rds.spec.masterUsername if rds.spec.masterUsername != "" else "admin"

    data:
      # Example of dynamic key loading.
      password: #@ creds.data[rds.masterUserPassword.get("key")]

Anything else you would like to add:
n/a


Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible"
👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you would like to work on this issue.

@Samze Samze added carvel-triage This issue has not yet been reviewed for validity enhancement This issue is a feature request labels May 31, 2022
@joe-kimmel-vmw joe-kimmel-vmw added good first issue An issue that will be a good candidate for a new contributor carvel-accepted This issue should be considered for future work and that the triage process has been completed priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed carvel-triage This issue has not yet been reviewed for validity carvel-accepted This issue should be considered for future work and that the triage process has been completed good first issue An issue that will be a good candidate for a new contributor labels Jul 20, 2022
@joe-kimmel-vmw
Copy link
Contributor

@Samze This does raise some new scope for Secretgen-controller, because currently it does not depend on any external binaries such as ytt. If you want to cross-compare with kapp-controller for the complexity this introduces, see e.g. :
https://github.com/vmware-tanzu/carvel-kapp-controller/blob/develop/cmd/controller/sidecarexec.go
https://github.com/vmware-tanzu/carvel-kapp-controller/blob/develop/cmd/controller/main.go#L34
https://github.com/vmware-tanzu/carvel-kapp-controller/blob/develop/config/deployment.yml#L61
https://github.com/vmware-tanzu/carvel-kapp-controller/blob/develop/hack/dependencies.yml#L8

Is there a way to solve the problem you're posing by using -- maybe? -- kapp controller's existing templating to output something that the secret templates could consume? We're trying to imagine how an AppCR could fit into this flow and tbh it's pretty awkward :) But it does feel appealing to imagine how else we could solve this other than by adding complexity to the sg-c build and deploy process before starting on this path.

@Samze
Copy link
Contributor Author

Samze commented Aug 3, 2022

@Samze This does raise some new scope for Secretgen-controller, because currently it does not depend on any external binaries such as ytt. If you want to cross-compare with kapp-controller for the complexity this introduces, see e.g. : https://github.com/vmware-tanzu/carvel-kapp-controller/blob/develop/cmd/controller/sidecarexec.go https://github.com/vmware-tanzu/carvel-kapp-controller/blob/develop/cmd/controller/main.go#L34 https://github.com/vmware-tanzu/carvel-kapp-controller/blob/develop/config/deployment.yml#L61 https://github.com/vmware-tanzu/carvel-kapp-controller/blob/develop/hack/dependencies.yml#L8

Is there a way to solve the problem you're posing by using -- maybe? -- kapp controller's existing templating to output something that the secret templates could consume? We're trying to imagine how an AppCR could fit into this flow and tbh it's pretty awkward :) But it does feel appealing to imagine how else we could solve this other than by adding complexity to the sg-c build and deploy process before starting on this path.

Apologies for the late response!

@joe-kimmel-vmw if there are concerns around including an external binary, can we not just consume ytt as a go-module as described here? Or are safety concerns you have with this approach?

I'm not sure I fully understand the suggestion of using AppCR (or other carvel tools) to solve this problem. Could you elaborate further?

@joe-kimmel-vmw
Copy link
Contributor

sorry , yup - you're right. just consume it as a go module.

@joe-kimmel-vmw joe-kimmel-vmw added the carvel-accepted This issue should be considered for future work and that the triage process has been completed label Aug 3, 2022
@Samze
Copy link
Contributor Author

Samze commented Aug 10, 2022

Looking at this again, I think it maybe a bad idea to just inline the template on the .spec.ytt key or if we should allow more extension in the future.

Originally this:

---
apiVersion: secretgen.k14s.io/v1alpha1
kind: SecretTemplate
metadata:
  name: generated-secret
spec:
  serviceAccountName: my-resource-reader
  inputResources:
     ....
  ytt: | 
    #@ load("@ytt:data", "data")

Should become this:

---
apiVersion: secretgen.k14s.io/v1alpha1
kind: SecretTemplate
metadata:
  name: generated-secret
spec:
  serviceAccountName: my-resource-reader
  inputResources:
     ....
  ytt:
    template: |
       #@ load("@ytt:data", "data")

For example, perhaps we need to provide other metadata like ytt options, for example we could introduce .spec.ytt.strict or maybe loading template from a file via .spec.ytt.templateRef.secretRef.

The slight downside of this is that we already introduced jsonpath as .spec.template so .spec.ytt.template is slightly misaligned but more flexible going forward.

@cppforlife
Copy link
Contributor

@Samze take a look at ytt interface for rebase rules within kapp -- https://carvel.dev/kapp/docs/v0.51.0/config/#rebaserules. one thing that i think we need to be mindful about is that execution environment (e.g. format of well known data values, etc.) for configuration might change over time and that's why in kapp for example it's marked as V1. goal would be to avoid breaking any configurations that have been already written, but also allow to transition to newer contract version.

@davewalter
Copy link

In our ytt templates, we are currently creating a dockerconfigjson secret based on user-provided username/password data values file like this:

#@ load("@ytt:base64", "base64")
#@ load("@ytt:data", "data")
#@ load("@ytt:json", "json")

---
apiVersion: v1
kind: Secret
metadata:
  name: reg-creds
  namespace: target-ns
type: kubernetes.io/dockerconfigjson
data:
  #@ docker_auth = base64.encode("{}:{}".format(data.values.registry.credentials.username, data.values.registry.credentials.password))
  #@ docker_creds = {"username": data.values.registry.credentials.username, "password": data.values.registry.credentials.password, "auth": docker_auth}
  .dockerconfigjson: #@ base64.encode(json.encode({"auths": {data.values.registry.hostname: docker_creds}}))

I am working to change this to allow the user to create their own generic secret containing the username and password, and then simply include the secret name and namespace in the values file. This is what I want the user-created secret to look like:

apiVersion: v1
kind: Secret
metadata:
  name: example-registry-creds
  namespace: user-ns
type: Opaque
stringData:
  hostname: registry.example.com
  username: joeuser
  password: supersecret

So far, my SecretTemplate looks like this:

#@ load("@ytt:base64", "base64")
#@ load("@ytt:data", "data")
#@ load("@ytt:json", "json")

---
apiVersion: secretgen.carvel.dev/v1alpha1
kind: SecretTemplate
metadata:
  name: reg-creds
  namespace: #@ data.values.registry.credentials.namespace
spec:
  inputResources:
  - name: user-provided-secret
    ref:
      apiVersion: v1
      kind: Secret
      name: #@ data.values.registry.credentials.secret_name
  template:
    type: kubernetes.io/dockerconfigjson
    stringData:
      .dockerconfigjson: |
        {
          "auths": {
            "index.docker.io": {
              "username": $(.user-provided-secret.stringData.username),
              "password": $(.user-provided-secret.stringData.password),
              "auth": ""
            }
          }
        }

and the user-provided values file would be:

#@data/values
---
registry:
  credentials:
    secret_name: example-registry-creds
    namespace: user-ns

For this to work,

  • I need to be able to inject templated values into the string data, which does not seem to be possible with current functionality.
  • I need to be able to access the base64-decoded data from the user-provided secret. I do not know if $(.user-provided-secret.stringData.username) would work.
  • I need to be able to replace the "index.docker.io" key with the appropriate data value.

I think all of this would be possible if I could define a ytt template like this:

#@ load("@ytt:data", "data")

---
apiVersion: secretgen.carvel.dev/v1alpha1
kind: SecretTemplate
metadata:
  name: reg-creds
  namespace: #@ data.values.app_registry.credentials.namespace
spec:
  inputResources:
  - name: user-provided-secret
    ref:
      apiVersion: v1
      kind: Secret
      name: #@ data.values.app_registry.credentials.secret_name
  ytt: |
    #@ load("@ytt:base64", "base64")
    #@ load("@ytt:data", "data")
    #@ load("@ytt:json", "json")

    #@ hostname = base64.decode($(.user-provided-secret.stringData.hostname))
    #@ username = base64.decode($(.user-provided-secret.stringData.username))
    #@ password = base64.decode($(.user-provided-secret.stringData.password))
    #@ docker_auth = base64.encode("{}:{}".format(username, password))
    #@ docker_creds = {"username": username, "password": password, "auth": docker_auth}

    type: kubernetes.io/dockerconfigjson
    data:
      .dockerconfigjson: #@ base64.encode(json.encode({"auths": {hostname: docker_creds}}))

Ideally, the user would not need to include the hostname in the secret with the username and password, but I think that would require me to be able to define an input source other than an existing resource on the cluster so that I could inject it at templating time from the data values input.

@ramonskie
Copy link

wondering if there has been activity on this in the background?

@neil-hickey
Copy link
Contributor

I don't think there has been any updates on this, @Samze is that fair? Do you think this is something you all would be prioritizing at some point? Or shall I assume that this is fairgame to any contributor and we should not expect this to be done by you all?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-input carvel-accepted This issue should be considered for future work and that the triage process has been completed enhancement This issue is a feature request priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Projects
Status: Unprioritized
Development

No branches or pull requests

6 participants