Skip to content
This repository has been archived by the owner on Jul 26, 2022. It is now read-only.

feat(secretsManager): ✨ Introduce dataFromWithOptions #846

Merged

Conversation

adutchak-x
Copy link
Contributor

This PR resolves this request: #823.

Use case:
While following GitOps practice, we try to keep our deployments immutable and when it comes to secrets management - we always specify the versionId of any secret we use in Git.

In the same time, we do really like existent dataFrom feature, which releases us from going thought each secret's property in order to get it populated in the Kubernetes secret.
However, with the existent implementation, dataFrom retrieves only the latest secret version, which breaks our GitOps & deployments immutability approach.

dataFromWithOptions allows getting all the values in bulk (as it is now with simple dataFrom option) but gives the possibility to specify the secret version we want to retrieve.

dataFromWithOptions allows to get values in bulk of a specific version
@adutchak-x adutchak-x marked this pull request as draft October 15, 2021 18:18
@adutchak-x adutchak-x marked this pull request as ready for review October 16, 2021 05:58
@Flydiverny
Copy link
Member

Flydiverny commented Oct 19, 2021

I've been considering if we should have added this.
One thought I had is whether or not it would be better to have dataFrom be more flexible.
So you could either specify a key as today, or a full item like in your PR here so instead of having both dataFrom and dataFromWithOptions, you could do for example:

apiVersion: kubernetes-client.io/v1
kind: ExternalSecret
metadata:
  name: hello-service
spec:
  backendType: secretsManager
  region: us-east-1
  dataFrom:
    - some-secret-key
    - key: hello-service/credentials
      versionStage: AWSPREVIOUS
    - hello-service/credentials

Not sure if this is better or worse 😄

So this means

  dataFrom:
    - hello-service/credentials

would be the same as

  dataFrom:
    - key: hello-service/credentials

@adutchak-x
Copy link
Contributor Author

adutchak-x commented Oct 19, 2021

I've been considering if we should have added this. One thought I had is whether or not it would be better to have dataFrom be more flexible. So you could either specify a key as today, or a full item like in your PR here so instead of having both dataFrom and dataFromWithOptions, you could do for example:

apiVersion: kubernetes-client.io/v1
kind: ExternalSecret
metadata:
  name: hello-service
spec:
  backendType: secretsManager
  region: us-east-1
  dataFrom:
    - some-secret-key
    - key: hello-service/credentials
      versionStage: AWSPREVIOUS
    - hello-service/credentials

Not sure if this is better or worse 😄

So this means

  dataFrom:
    - hello-service/credentials

would be the same as

  dataFrom:
    - key: hello-service/credentials

This was my desired initial implementation, but the problem is with current CRD implementation - currently dataFrom is defined as list of strings, and I could make it mixed type with oneOf or anyOf declaration if we didn't have to specify structural schema only (introduced in kubernetes > v1.15).
This means that dataFrom list items can only be of one type unfortunately, converting it to list of objects will break the backward compatibility.

@Flydiverny
Copy link
Member

Ah. Fair point, I guess bumping the CRD version would be the only way around that, which doesn't sound all fun 😄 🤔

@ghostsquad
Copy link

Per https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/2335-vanilla-crd-openapi-subset-structural-schemas

Note: we explicitly accept that polymorphic types (like IntOrString) are an anti-pattern for Kubernetes-like types and we do not want to encourage its use beyond IntOrString.

A new field must be introduced in order to allow backwards compatibility and to conform with the Structural Schema validation in K8s 1.15+

@Flydiverny Flydiverny merged commit 4dbb6dd into external-secrets:master Nov 17, 2021
@Flydiverny
Copy link
Member

Released as 8.4.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants