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

[FEATURE] Multiple vault objects into single K8s secret #36

Closed
jozala-work opened this issue Feb 24, 2020 · 11 comments
Closed

[FEATURE] Multiple vault objects into single K8s secret #36

jozala-work opened this issue Feb 24, 2020 · 11 comments
Labels
enhancement New feature or request
Milestone

Comments

@jozala-work
Copy link

jozala-work commented Feb 24, 2020

Is your feature request related to a problem? Please describe.
I would like to use azure-key-vault-to-kubernetes together with Flux feature "valuesFrom".
To do that, I need to read multiple secrets from Azure KeyVault and put it into a single Kubernetes secret, under single key. The values is read later by Flux and used as additional values file when deploying Helm release.

Describe the solution you'd like
I would like to be able to specify multiple objects in AzureKeyVaultSecret (or new CRD) so it would look like this:

apiVersion: spv.no/v1alpha1
kind: AzureKeyVaultSecret
metadata:
  name: secret-sync
  namespace: default
spec:
  vault:
    name: my-keyvault
    objects:
      - name: test-secret
        type: secret
      - name: another-secret
        type: secret
  output:
    secret:
      name: app-secret # kubernetes secret name
      dataKey: values.yaml # key to store object value in kubernetes secret

And the output secret in K8s would be:

apiVersion: v1
kind: Secret
type: Opaque
metadata:
  name: app-secret
  namespace: default
data:
  values.yaml: | # The value would be base64 encoded
    test-secret: test-secret-value
    another-secret: another-secret-value

Describe alternatives you've considered
One alternative could be to have customer templating support so it would be possible to achieve the same with templating. There is an existing issue about templating #18.

Additional context
Would that feature make sense for others? Is anyone using azure-key-vault-to-kubernetes with Flux or ArgoCD?
I may try to implement it, but I would like to know what would be feasible solution for this project.

@jozala-work jozala-work added the enhancement New feature or request label Feb 24, 2020
@jozala-work
Copy link
Author

@torresdal Does this feature would make sense in your opinion? As I wrote - I could try to implement this, but I would like to know if this has a chance to be accepted.

@torresdal
Copy link
Collaborator

@MariuszJozala-Avaleo it does make sense 👍 Need to think through if we can reuse the AzureKeyVaultSecret CRD or if we have to create a new one. Mostly semantic really. AzureKeyVaultSecret is singular and I would expect it to represent a single secret in Azure Key Vault. Ignoring semantics, the easiest would be to reuse.

Another alternative would be to define/use multiple AzureKeyVaultSecret's - pointing to the same Kubernetes Secret - effectively adding multiple elements to one single Kubernetes Secret. E.g. adding a new Type of secret-multi or similar.

We are also using Flux, but not currently using .valuesFrom - but we probably should/will.

Regarding the alternative (#18) with templating, in your case you would still need multiple secrets as input. Therefore my interpretation is that your feature request is mainly about querying multiple objects in Azure Key Vault and output into one single secret.

Just some initial thoughts. Anyone feel free to chip into this discussion with your preferences and suggestions.

@robinmanuelthiel
Copy link

robinmanuelthiel commented Mar 1, 2020

Actually, I would prefer not to change the AzureKeyVaultSecret CRD at all. Defining multiple AzureKeyVaultSecret objects is fine, as long as they can all point to the same Kubernetes Secret.

So defining two seperate AzureKeyVaultSecret objects like this:

apiVersion: spv.no/v1alpha1
kind: AzureKeyVaultSecret
metadata:
  name: keyvault-secret-one
spec:
  vault:
    name: mykeyvault
    object:
      name: SecretOne
      type: secret
  output:
    secret:
      name: my-only-kuberentes-secret
      dataKey: secret-one
---
apiVersion: spv.no/v1alpha1
kind: AzureKeyVaultSecret
metadata:
  name: keyvault-secret-two
spec:
  vault:
    name: mykeyvault
    object:
      name: SecretTwo
      type: secret
  output:
    secret:
      name: my-only-kuberentes-secret
      dataKey: secret-two

would result in one single Kubernetes Secretlike this:

apiVersion: v1
kind: Secret
type: Opaque
metadata:
  name: my-only-kuberentes-secret
data:
  secret-one: XXXXXXXX
  secret-two: XXXXXXXX

What do you think?

BTW: Currently, when defining two AzureKeyVaultSecret secrets targeting the same Kubernetes Secret as shown above, they are overwriting each other, which is a bug in my option. I will file a separate Bug report for this.

@torresdal
Copy link
Collaborator

I'm thinking overwriting the Secret should be the default behavior, given a AzureKeyVaultSecret owns the Secret (ref #37). If not we will get in trouble when/if you change the name of the dataKey in AzureKeyVaultSecret - it will add a new, instead of replacing.

To enable multiple AzureKeyVaultSecret's writing different keys to the same Secret, I think we should introduce a new output type like secret-multi-keys (naming suggestions welcome!!). Using your example this would look like this:

apiVersion: spv.no/v1alpha1
kind: AzureKeyVaultSecret
metadata:
  name: keyvault-secret-one
spec:
  vault:
    name: mykeyvault
    object:
      name: SecretOne
      type: secret
  output:
    secret-multi-keys:
      name: my-only-kuberentes-secret # <-- Same Kubernetes Secret targeted
      dataKey: secret-one
---
apiVersion: spv.no/v1alpha1
kind: AzureKeyVaultSecret
metadata:
  name: keyvault-secret-two
spec:
  vault:
    name: mykeyvault
    object:
      name: SecretTwo
      type: secret
  output:
    secret-multi-keys:
      name: my-only-kuberentes-secret # <-- Same Kubernetes Secret targeted
      dataKey: secret-two

...or maybe secret-allow-multi-keys or existing-secret or secret-shared... Suggestions welcome 😄

What do you think @robinmanuelthiel ?

@robinmanuelthiel
Copy link

I would definitely prefer camelCase over of kebab-case for the new field, as it seems to be the standard in other Kubernetes objects. What about

  • sharedSecret
  • multiKeySecret

Can you explain, why we need a different extra property for this? Why can't we just reuse the existing secret field? I would expect it to just add keys to an existing secret. If two different AzureKeyVaultSecret target the same Secret name and data key, they overwrite each other, which is okay, I think.

I would honestly expect it to "just work" when I define two AzureKeyVaultSecrets that both target the same Secret name but different keys. Would also solve #37 IMHO

@torresdal
Copy link
Collaborator

We’ll have a go at the most intuitive solution as you describe, and see where it goes from there 👍

@jozala-work
Copy link
Author

jozala-work commented Mar 18, 2020

Thanks for getting involved and all proposed solutions. Sorry for late reply, but I had to leave the topic aside for a while.
I think I oversimplified the example in my initial comment. That might have caused one important bit overlooked. It may influence how CRD should look like.
To make new feature work with "valuesFrom" feature from Flux, secret in K8s should have yaml stuctured values under single data key (values.yaml by default):

apiVersion: v1
kind: Secret
type: Opaque
metadata:
  name: app-secret
  namespace: apps-ns
data:
  values.yaml: | # value of this field will be passed as additional values file to Helm (it needs to be YAML).
    some:
      value:
        one: test-secret-value
        two: another-secret value

If the way is to go with multiple AzureKeyVaultSecret resources, these resources would need to have not only the same spec.output.multiKeySecret, but also spec.output.multiKeySecret.dataKey. Plus additional field to tell where in the secret value should be put. So it would look like this:

apiVersion: spv.no/v1alpha1
kind: AzureKeyVaultSecret
metadata:
  name: keyvault-secret-one
spec:
  vault:
    name: mykeyvault
    object:
      name: SecretOne
      type: secret
  output:
    multiKeySecret:
      name: my-only-kuberentes-secret # <-- Same Kubernetes Secret targeted
      dataKey: values.yaml # <-- Same data field in K8s secret
      secretKey: some.value.one # <-- Additional to know where to put value
---
apiVersion: spv.no/v1alpha1
kind: AzureKeyVaultSecret
metadata:
  name: keyvault-secret-two
spec:
  vault:
    name: mykeyvault
    object:
      name: SecretTwo
      type: secret
  output:
    multiKeySecret:
      name: my-only-kuberentes-secret # <-- Same Kubernetes Secret targeted
      dataKey: values.yaml # <-- Same data field in K8s secret
      secretKey: some.value.two # <-- Additional to know where to put value

I was also thinking about how usage will look like.
What we usually have is an application that has multiple secrets which needs to be applied through environment variables.

Some KeyVault secrets are shared by multiple applications, but we would still like to have separate Kubernetes secrets per application with all KeyVault secrets that application needs.

Example:
Application "Foo" needs following values (required by Helm chart):

  • database.password (read from KeyVault database_foo_password)
  • rabbitmq.password (read from KeyVault rabbitmq_foo_password)
  • someintegration.key (read from KeyVault someintegration_key and may be read by multiple apps)

I would like to have single secret which I could use in my HelmRelease:

apiVersion: v1
kind: Secret
type: Opaque
metadata:
  name: foo-app-secrets
  namespace: apps-ns
data:
  values.yaml: |
    database:
      password: dbPassValue
    rabbitmq:
      password: rabbitmqPassValue
    someintegration:
      key: keyValue123456789

It would be possible to use it in HelmRelease like this (some fields omitted for clarity):

apiVersion: helm.fluxcd.io/v1
kind: HelmRelease
metadata:
  name: foo-app
  namespace: apps-ns
spec:
  releaseName: foo-app
  valuesFrom:
  - secretKeyRef:
      name: foo-app-secrets # <-- this is where I tell Flux to add K8s secret as value file
  values:
    image:
      tag: 1.10.0

With all of this, I am asking myself a question if defining three AzureKeyVaultSecret resources, for this case, isn't too much work and duplication.
This is getting so far from original AzureKeyVaultSecret CRD that I would say it requires completely separate one. This is what I was able to come up with:

apiVersion: spv.no/v1alpha1
kind: AzureKeyVaultSecretsMapping
metadata:
  name: foo-app-secrets
  namespace: apps-ns
spec:
  vault:
    name: my-keyvault
    objects:
      - name: database_foo_password
        type: secret
        outputKey: database.password # <-- this is different for each secret from KeyVault
      - name: rabbitmq_foo_password
        type: secret
        outputKey: rabbitmq.password 
      - name: someintegration_key
        type: secret
        outputKey: someintegration.key
  output:
    secret:
      name: foo-app-secrets # <-- these are the same for all secrets from Keyvault so I've put them separately
      dataKey: values.yaml

From my perspective important part is that there is not much duplication and it will be pretty easy to define based on what application needs from Azure KeyVault. Naming is probably rubbish. Forgive me, I wanted to focus on the use cases and structure.

Please let me know what you think.

@torresdal torresdal added this to the Version 1.2.0 milestone Mar 20, 2020
@jozala-work
Copy link
Author

I am trying to implement this, but I am pretty new to Kubernetes controllers.
I have problem with k8s.io/code-generator. After client regeneration I am not able to build project because of error:

pkg/k8s/client/clientset/versioned/typed/azurekeyvault/v1/azurekeyvault_client.go:76:45: scheme.Codecs.WithoutConversion undefined (type serializer.CodecFactory has no field or method WithoutConversion)

I'v tried with code-generation from tag "kubernetes-1.17.4".

I believe I am using wrong version of it.
Could you tell me which version of code-generator you use in the project?

@torresdal
Copy link
Collaborator

@torresdal
Copy link
Collaborator

This is now supported in 1.2-beta - soon to be released

@torresdal
Copy link
Collaborator

As mentioned this will be available in 1.2. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants