external-secrets icon indicating copy to clipboard operation
external-secrets copied to clipboard

Design/target custom resources

Open gusfcarvalho opened this issue 1 year ago • 16 comments

Problem Statement

What is the problem you're trying to solve?

Related Issue

Proposal for #2503

Proposed Changes

This is a design document for change proposal on spec.target behavior. Allowing it to target multiple manifests, and a change on spec.target.template.target behavior, also allowing it to target multiple keys.

Checklist

  • [x] I have read the contribution guidelines
  • [X] All commits are signed with git commit --signoff
  • N/A My changes have reasonable test coverage
  • N/A All tests pass with make test
  • [x] I ensured my PR is ready for review with make reviewable

gusfcarvalho avatar May 05 '24 11:05 gusfcarvalho

Great proposal, I lot of end users are looking to use this feature make their k8s configuration more secure ability to rotate sensitive values stored in other kind of resources (ie configMaps) values using ESO.

csantanapr avatar May 05 '24 12:05 csantanapr

This would be useful for OpenShift Routes which can directly contain certificates, including the private key.

gnunn1 avatar May 05 '24 14:05 gnunn1

Or you can use template like

    name: target-custom
    objectTemplate: |
        apiVersion: v1
        kind: CustomUserOject
        metadata:
            annotations:
            name: {{ ."{{ .user | toString }}"   | lower }}

gautsing avatar May 06 '24 00:05 gautsing

Adding ConfigMaps would be a much appreciated feature!

christianh814 avatar May 06 '24 14:05 christianh814

I was recently designing an infra-dependency management strategy based on ESO and was using Secrets. However, ConfigMaps is the ideal construct for that, so this feature would be super useful.

asanka-x avatar May 07 '24 01:05 asanka-x

This is well overdue ❤️! However, I think that this could use some way for a cluster operator to restrict the use of target!= Secret on a case-by-case basis.

I could see scenarios where dev-teams inadvertently leak secrets into places where they should not be. My concerns are perhaps misplaced, since in practice there's not a huge difference between having an ExternalSecret target of Secret vs a ConfigMap (or any other CR for that matter).

Perhaps restricting the use of ExternalSecrets in that way is not in the scope of ESO, but rather something like Kyverno?

blakepettersson avatar May 07 '24 08:05 blakepettersson

This is well overdue ❤️! However, I think that this could use some way for a cluster operator to restrict the use of target!= Secret on a case-by-case basis.

I could see scenarios where dev-teams inadvertently leak secrets into places where they should not be. My concerns are perhaps misplaced, since in practice there's not a huge difference between having an ExternalSecret target of Secret vs a ConfigMap (or any other CR for that matter).

Perhaps restricting the use of ExternalSecrets in that way is not in the scope of ESO, but rather something like Kyverno?

You’re not entirely wrong. Secrets are normally the only resource that are kms encrypted, so there is a risk there. That is why as part of this proposal, RBAC needs to be added by the admin, as well as the flag unsafe-allow-non-secret-target

gusfcarvalho avatar May 07 '24 10:05 gusfcarvalho

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

sonarqubecloud[bot] avatar May 07 '24 23:05 sonarqubecloud[bot]

@shuheiktgw I would also like to know your input on this one - if it is not much of a bother 😄

gusfcarvalho avatar May 15 '24 08:05 gusfcarvalho

@gusfcarvalho Thank you for putting together such an impressive proposal! However, I do have one concern regarding the decision to extend the ExternalSecret resource rather than introducing a new kind. Given that the ExternalSecret resource includes some fields specific to Secrets and that the controller logic is already quite complex, I would personally advocate for creating a new kind rather than expanding the existing one. Although I recognize that there might be some redundancy between the external secret controller and a new one, sharing logic between different controllers should not be overly challenging.

shuheiktgw avatar May 16 '24 00:05 shuheiktgw

@gusfcarvalho Thank you for putting together such an impressive proposal! However, I do have one concern regarding the decision to extend the ExternalSecret resource rather than introducing a new kind. Given that the ExternalSecret resource includes some fields specific to Secrets and that the controller logic is already quite complex, I would personally advocate for creating a new kind rather than expanding the existing one. Although I recognize that there might be some redundancy between the external secret controller and a new one, sharing logic between different controllers should not be overly challenging.

Wouldn't that be worse to maintain? as in - if we just duplicate the code base and find a bug in creationPolicy, we necessarily need to propagate it, no?

On the other hand - to implement a new manifests reusing functions is pretty much the effor to implement this template bit 😄 - I mean - the effort of implementing manifest etc code wise is the same effort refactoring the code and existing functions to support the new manifest - code wise I believe adding a new manifest would be more content to maintain.

Sharing some examples I could remember here needed to implement this feature as part of ExternalSecrets:

  • Templates will need to stop supporting only Secrets to support anything
  • Reconciler call would need to stop supporting only secrets to support anything
  • Data hashes would need to support anything

The only thing that we do not have to change is the Reconciler, which would still be code that exists - just on a different folder. Plus we get more API code and validation to maintain.

Am I not seeing something here? 🤔

gusfcarvalho avatar May 16 '24 08:05 gusfcarvalho

Yes, I agree that extending ExternalSecret may be easier in the short term, but I believe the proposal has much broader implications for the project. It essentially redefines the entire scope of the project—from provisioning Kubernetes Secrets from external sources to provisioning any Kubernetes resource from these sources. If that is the direction we want to pursue, I'm not entirely sure if simply extending the current ExternalSecret with additional fields is the best approach for the long term. For instance, it might be more logical and coherent to have a single interface that can provision any Kubernetes resource, including Secrets. To achieve this, it might actually be simpler to introduce a new resource rather than extending ExternalSecret.

I absolutely agree with the proposed goal of extending the project to support other Kubernetes resources, and I appreciate all the effort that has gone into this proposal. However, I am not entirely convinced that this is the best approach for supporting those resources just yet 🙇

shuheiktgw avatar May 17 '24 00:05 shuheiktgw

I absolutely agree with the proposed goal of extending the project to support other Kubernetes resources, and I appreciate all the effort that has gone into this proposal. However, I am not entirely convinced that this is the best approach for supporting those resources just yet 🙇

Do not worry about this - I asked your opinion even because I thought you would have these concerns from other issues 😄

Yes, I agree that extending ExternalSecret may be easier in the short term, but I believe the proposal has much broader implications for the project. It essentially redefines the entire scope of the project—from provisioning Kubernetes Secrets from external sources to provisioning any Kubernetes resource from these sources. If that is the direction we want to pursue, I'm not entirely sure if simply extending the current ExternalSecret with additional fields is the best approach for the long term. For instance, it might be more logical and coherent to have a single interface that can provision any Kubernetes resource, including Secrets. To achieve this, it might actually be simpler to introduce a new resource rather than extending ExternalSecret.

Do you envision it being a new CRD with the same features ExternalSecrets currently has, except for maybe potentially more fields to control the manifest generation? something like:

apiVersion: external.external-secrets.io/v1beta1
kind: Configmap
metadata:
  name: example
spec:
  refreshInterval: 1h 
  secretStoreRef:
    kind: SecretStore
    name: example              
  target:
    creationPolicy: Owner
     ## Something different here
  dataFrom:
  - extract:
      key: all-keys-example-secret 

?

or would the kind be something like kind=ExternalResource under the same apiVersion?

gusfcarvalho avatar May 17 '24 19:05 gusfcarvalho

Do you envision it being a new CRD with the same features ExternalSecrets currently has, except for maybe potentially more fields to control the manifest generation? or would the kind be something like kind=ExternalResource under the same apiVersion?

I'm envisioning a completely new type of resource. Let me to briefly outline what I have in mind! I believe the new resource should consist of three key components:

  1. (Secret) Data Sources
  2. Transformers
  3. Provisioners

(Secret) Data Sources The role of (Secret) Data Sources is to provide the data that we base our resource creation upon. This could include SecretStores and ClusterSecretStores. A Generator should also be considered as a data source. I refer to these as "(Secret) Data Sources" because it’s important that we can utilize data sources that don't contain sensitive information, like a ConfigMap, as well.

Transformers Transformers are responsible for altering the data supplied by the Data Sources according to user specifications. We already support Go template-based modification, but we might also explore utilizing CEL (Common Expression Language) or WebAssembly as one of the transformers.

Provisioners Provisioners then create resources provided by the transformed data. They are not limited to provisioning Kubernetes resources such as Secrets, ConfigMaps, or CRDs; they can also manage external resources like the GCP secret manager. Thus, it could cover the use cases that currently PushSecrets do.

Is this proposed design too idealistic? Possibly yes, but considering that provisioning CRDs or ConfigMaps is a brand-new feature, the good news is we can start small, and covering some of the major use cases may not be too challenging 🙂

shuheiktgw avatar May 18 '24 05:05 shuheiktgw

As an end-user, this is probably the most exciting OSS proposal I have seen in a while. IMHO, enabling use cases listed in this PR would significantly extend ESO useability and adoption 🚀

I know at least in my case it will streamline some K8s deployments away from hacks and workarounds (we do inject not so secret values from SSM into CRs which can’t reference secret values).

mubarak-j avatar Jun 25 '24 17:06 mubarak-j

Hi. Is this PR still undergoing discussion/development? This is really a gamechanger for many scenarios. In our case, we are already using external-secrets as a pre-processing template tool to generate data for flux-cd valuesFrom. Being able to generate directly a flux-cd/HelmRelease or an argo-cd/Application (https://github.com/argoproj/argo-cd/issues/1786) would be amazing and would avoid many complex setups. Thanks!

joaocc avatar Sep 22 '24 12:09 joaocc