support for `fromConnectionSecretKey` patch

[haarchri]

What problem are you facing?

we want to have the option to patch in compositions - connectionDetails from one mr to other mr as input in provider-aws we have a few live examples for it:

• https://github.com/crossplane/provider-aws/issues/955#issuecomment-980136699
• https://github.com/crossplane/provider-aws/pull/966
• use provider-aws Secrets.v1alpha1.secretsmanager.aws.crossplane.io as input for other mr

How could Crossplane help solve your problem?

implement patch fromConnectionSecretKey

[negz]

Would this have security implications? I know sometimes the connection details we write to a secret aren't necessarily sensitive, but often they are. I wonder if we could address this by ensuring when designing MRs that any non-sensitive connection details are both in the connection secret and in the status of the resource, that way you could patch from the status instead.

[janwillies]

I believe there's room for interpretation for certain type of information wether they are sensitive or not. IMO it's best to leave this to the user instead of to the developer of the respective provider.

[shailsirohi]

I believe there is perhaps a possibility to generalize this by allowing patching from any kubernetes secret. One of the use case we have right now: We are using central cred store for maintaining all secrets and don't want to store any secrets in git. The secrets are synced inside namespace using a custom operator. This means for a RDS instance, a secret is created first in the cred store for database password and synced to namespace. By enabling patching this secret data directly, we can automate the entire resource creation through GitOps

[hferentschik]

I believe there is perhaps a possibility to generalize this by allowing patching from any kubernetes secret.

This is exactly what I am looking for as well. My use case is the following. I want to create and manage multiple GKE clusters via Crossplane. I also want to create for each cluster a CloudSQLInstance. It's pretty easy to achieve the above with Crossplane and a composite, however, the connection secret for the DB then lives on the Crossplane cluster. I need to mirror/sync it into the corresponding GKE cluster. Currently, there is no easy way to do this. Being able to patch from any connection secret would solve my problem. Then I could use the Kubernetes provider to create a Secret in the target GKE cluster patching the required fields from the DB connection secret.

@haarchri pointed out that there is a current workaround by using a dummy Helm chart in conjunction with the Crossplane Helm provider, but that feels very hacky.

[shailsirohi]

Yes and this can further enable daisy chaining of multiple managed resources inside composition (i.e output field of 1 mr becomes input of another mr) and simplify provider creation. Currently, the dependency between 2 managed resources has to be defined explicitly inside the provider. Allowing this field will be a more flexible way of chaining managed resources together and avoid opinionated dependency creations inside provider

[shailsirohi]

Also, @haarchri did point to the chat in slack channel.. our use case is little more than that. For example: we also have requirements where the input spec of a mr is derived from a secret. For example: UAA tenant (custom provider) instance creation requires users to provide the creds they would like to apply beforehand. Regarding security implications pointed by @negz , in a central control plane model, it's relatively easy to enforce stringent controls through RBAC or even better allowing only GitOps based approach for resource creation through ArgoCD. No direct access to k8s clusters through kubectl.

[hferentschik]

Yes and this can further enable daisy chaining of multiple managed resources inside composition (i.e output field of 1 mr becomes input of another mr) and simplify provider creation.

Agreed.

Ideally, one would patch between managed resources directly. ATM, you need to make the detour via the XR status. It kind of works, but for secrets, it is really a no go.

I am wondering why patching atm is only allowed from and to the XR? Is this a design decision or are there technical reasons as well? I would love to hear what someone with a better understanding of the inner workings of Crossplane has to say to this.

[haarchri]

interesting PR in crossplane eco-system in provider-kubernetes regarding this topic... https://github.com/crossplane-contrib/provider-kubernetes/pull/21/files

[negz]

Continuing on my speculation from a previous comment, I could imagine tackling this by updating Crossplane such that all connection details were written to status. Something like:

apiVersion: example.crossplane.io/v1
kind: SomeManagedResource
metadata:
  name: example
spec:
  forProvider:
    size: large
status:
  atProvider:
    width: 42
    length: 20
  connectionDetails:
    location: Europe
    username: admin
    password: SENSITIVE

If we included sensitive keys this could help make make it a little more obvious what connection details exist. On the other hand it leaks information (e.g. that a username and password even exist). It would also need us to somehow annotate certain keys as sensitive and others not.

All that said though, patching from a connection secret key seems fine. We should make sure to consider https://github.com/crossplane/crossplane/blob/master/design/design-doc-external-secret-stores.md.

[ChrisJBurns]

This would help me in my case also.

Currently, I want to be able to share values between composition resources, and currently some values are output into a connection secret, which I cannot access in another managed resource.

So in my case, I have a terraform resource, and inside it I use the tls terraform provider to generate a public_key and private_key.

          ...
          forProvider:
            source: Inline
            module: |
              // Outputs are written to the connection secret.
              output "private_key" {
                value       = tls_private_key.flux_deploy_keys.private_key_pem
                sensitive = true
              }
              output "public_key" {
                value       = tls_private_key.flux_deploy_keys.public_key_pem
                sensitive = false
              }
              resource "tls_private_key" "flux_deploy_keys" {
                algorithm   = "ECDSA"
                ecdsa_curve = "P256"
              }
          ...

The public_key is fine because I can use the ToCompositeFieldPath to assign it to the XR and then use it in my Helm resource via the FromCompositeFieldPath. However because the tls terraform provider marks the private_key as sensitive, the terraform provider doesn't allow me to output it in the same way as the public_key, which means the private_key gets output to a connection secret instead - which is fine. But in my case, I need to retrieve that private_key value and patch it into my Helm resource where I need both keys - which currently I can't / don't know how to do. I assume this issue ticket is a way of making that possible? Which if it is, is really helpful because not being able to share data between composition resources is quite a big deal I'd imagine.

[bobh66]

A workaround for this was discussed in https://github.com/crossplane-contrib/provider-kubernetes/issues/41 where you can use managementPolicy: Observe in the kubernetes provider to "import" a Secret and copy any data you need from the Secret, even using the FromBase64 / ToBase64 transforms as needed to manipulate the data,

It's definitely not ideal to have to observe a Secret that contains the connectionDetails, but it does work as an option until a more elegant/official solution is available.

[github-actions[bot]]

Crossplane does not currently have enough maintainers to address every issue and pull request. This issue has been automatically marked as stale because it has had no activity in the last 90 days. It will be closed in 7 days if no further activity occurs. Leaving a comment starting with /fresh will mark this issue as not stale.

[haarchri]

/fresh we have a lot of folks for this feature

[negz]

/fresh

https://github.com/crossplane/crossplane/runs/8063904781?check_suite_focus=true - Looks like I need to fix this action.

[github-actions[bot]]

Crossplane does not currently have enough maintainers to address every issue and pull request. This issue has been automatically marked as stale because it has had no activity in the last 90 days. It will be closed in 7 days if no further activity occurs. Leaving a comment starting with /fresh will mark this issue as not stale.

[jbw976]

/fresh

[pedjak]

@hferentschik it seems that your usecase can be meanwhile supported by external secret store.

@haarchri - instead of supporting yet another patching strategy, the behavior can be implemented through a composition function. Namely, the declared function receives as the input a FunctionIO resource containing all objects involved in a composition. Among others connection details for each managed resource:

apiVersion: apiextensions.crossplane.io/v1alpha1
kind: FunctionIO
observed:
  resources:
    - connectionDetails:
      - name: password
        value: foo
      - name: username
        value: admin
      name: my-db
      resource:
         apiVersion: rds.aws.upbound.io/v1beta1
         kind: Instance
        .
        .

In the function it would be possible to read appropriate key/values from connection details and update the relevant managed resources.

Let me know if there are some concerns with the proposed approach.

@plumbis how about that we document this usecase as the part of Composition Function sections?

[haarchri]

yes fair - but today composition functions not running for all environments like bottlerocket in aws eks - so this issue is still valid

[pedjak]

composition functions not running for all environments like bottlerocket in aws eks

This is reported in #3899

[turkenh]

Rather than extending the composition API further, I would also prefer to lean on the composition functions as a solution here especially given we already have a workaround whenever that does not work.

In Crossplane, we usually avoid having multiple ways of doing the same thing, but I can also understand that it is a bit controversial here since the composition function could do anything :)

@pedjak I am wondering if we could develop a generic xfn for this case here? I mean, instead of everyone building their function, having a generic one and leveraging the config block to specify how patching from connection secret should be done. What could the API look like for such an xfn?

[pedjak]

I am wondering if we could develop a generic xfn for this case here? I mean, instead of everyone building their function, having a generic one and leveraging the config block to specify how patching from connection secret should be done. What could the API look like for such an xfn?

Sure, I do think it would be doable. In order to discuss further its API, perhaps it would make sense to open a new issue for it?

[turkenh]

In order to discuss further its API, perhaps it would make sense to open a new issue for it?

I was more interested in how a possible API could look like, so we can keep using this issue until we get a sense. But feel free to open another issue if you think it would make more sense.

[pedjak]

@turkenh In order to generalize the thing even further and enable patching from any to any resource, we could actually create a function that is a wrapper around well-known yq. With that, we can expose the very well-known and powerful yq syntax to users, so that they can do any sort of manipulation on the passed FunctionIO resource.

How a composition that uses such function can look like? Let create an example where two resources are managed:

apiVersion: apiextensions.crossplane.io/v1
kind: Composition
metadata:
  name: example
spec:
  .
  .
  resources:
    - name: mr1
      base:
        apiVersion: example.crossplane.io/v1
        kind: SomeManagedResource
        spec:
          forProvider:
            .
            .
    - name: mr2
      base:
        apiVersion: example.crossplane.io/v1
        kind: SomeOtherManagedResource
        spec:
          forProvider:
            .
            .
  functions:
    - name: yq-fn
      type: Container
      container:
        image: xpkg.upbound.io/xfns/yq
      config:
        apiVersion: yq.xfn.upbound.io/v1alpha1
        kind: Config
        spec:
          - ignoreOnError: true
            expression: >
            ((.observed.resources.[] | select (.name == "mr1")) | .connectionDetails.[] | select (.name == "username") | .value) as $v 
            | (.desired.resources.[] | select (.name == "mr2") | .resource.spec.forProvider.user ) = $v
            

The details unnecessary for this discussion are omitted from the above example. Let's imaging that mr1 connection details contain some username that is required for creation of mr2 resource. In order to patch it, we are going to provide a yq expression within Config function resource. The example expression from above selects the connection details of mr1 resource, retrieves the value of username key and sets it as spec.forProvider.user on mr2 resource.

We should be able to provide multiple expressions that are executed on the same FunctionIO in sequence and should be able to control how an error is treated.

Wrapping yq in a composition functions gives us endless patching possibilities on resources involved in a given composition.

[github-actions[bot]]

Crossplane does not currently have enough maintainers to address every issue and pull request. This issue has been automatically marked as stale because it has had no activity in the last 90 days. It will be closed in 14 days if no further activity occurs. Leaving a comment starting with /fresh will mark this issue as not stale.

[ytsarev]

/fresh still valid request, maybe it can go to a new P&T function

[pedjak]

@ytsarev would be a creating a dedicated function as sketched above acceptable

[ytsarev]

@pedjak yq function seems cool, but it has much broader scope than the original request imho :)