crossplane icon indicating copy to clipboard operation
crossplane copied to clipboard

Adds JSON string encoding for `transforms` API

Open jpmcb opened this issue 2 years ago • 7 comments

Description of your changes

👋🏼 - first time Crossplane contributor here! Feel free to let me know if this is missing anything!

  • Accepts a transform with the encoding specified as "JSON" in the format:
transforms:
- type: string
   string:
    type: Encode
    encode: JSON
  • Takes in the raw string, marshals it to ensure it's valid JSON and than returns the valid formatted JSON string

Fixes #2943

I have:

  • [x] Read and followed Crossplane's [contribution process].
  • [x] Run make reviewable to ensure this PR is ready for review.
  • [ ] Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

Added two unit tests to cover this. Still spinning up an environment to give this a full e2e spin.

jpmcb avatar Jun 08 '22 17:06 jpmcb

cc @AaronME - would be great if you had eyes on this and confirmed this is what you imagined for your feature request!

jpmcb avatar Jun 08 '22 17:06 jpmcb

Thanks @jpmcb! This looks good, but similar to the thread at https://github.com/crossplane/crossplane/pull/3099#discussion_r881080373 you'll need to reflect these changes in a few other places to account for our v1alpha1 CompositionRevision feature.

negz avatar Jun 08 '22 21:06 negz

Done! Also updated the docs in regards to this new transform as well!

jpmcb avatar Jun 09 '22 02:06 jpmcb

I was just about to approve this, but it occurred to me that we already support ToBase64 in the "convert" type of string transform. I think this might be a better fit there for consistency rather than adding another new type of string transform (even though I think "encode" is more accurate for both this case and ToBase64).

So this would instead make the API:

transforms:
- type: string
  string:
    type: Convert
    convert: ToJSON

negz avatar Jun 30 '22 08:06 negz

Ah - there's also (unfortunately) currently a bit of boilerplate conversion code that needs to be updated whenever we add a new transform or patch type. See https://github.com/crossplane/crossplane/pull/3158 for an example - specifically the code that transforms a Composition to/from a CompositionRevision.

negz avatar Jun 30 '22 08:06 negz

Can you please provide an example composition as well on how to use this json transformation to solve the example given in the issue? I must be missing something obvious, but I don't understand how to apply this json transformation to the connection details from the connection secret.

Example based upon upbound/platform-ref-gcp:

apiVersion: apiextensions.crossplane.io/v1
kind: Composition
metadata:
  name: gke.gcp.platformref.crossplane.io
  labels:
    provider: GCP
spec:
  writeConnectionSecretsToNamespace: upbound-system
  compositeTypeRef:
    apiVersion: gcp.platformref.crossplane.io/v1alpha1
    kind: GKE
  resources:
    - base:
        apiVersion: container.gcp.crossplane.io/v1beta2
        kind: Cluster
        spec:
          forProvider:
            ...
      connectionDetails:
        - fromConnectionSecretKey: clusterCA
        - fromConnectionSecretKey: endpoint
        - fromConnectionSecretKey: kubeconfig
    - base:
        apiVersion:
        kind:
        spec:
          forProvider:
             jsonData: # patched from connection secret
        patches:
          - type: ?
             fromFieldPath: <<< how to reference the connection secret key 'clusterCA'?
             toFieldPath: spec.forProvider.jsonData
             transforms:
              - type: string
                 string:
                   fmt: '{"clusterCA":"%s"}'
              - type: string
                 string:
                   type: Convert
                   convert: ToJSON

The examples which reference connection secrets I can find rely on the provider implementing a xxxSecretRef field, e.g.

apiVersion: helm.crossplane.io/v1beta1
kind: ProviderConfig
spec:
  credentials:
    source: Secret
    secretRef:
      key: kubeconfig

or

apiVersion: generic.vault.jet.crossplane.io/v1alpha1
kind: Secret
metadata:
  name: example
spec:
  forProvider:
    path: "secret/foo"
    dataJsonSecretRef:
      key: data_json
      name: example-data
      namespace: default

This SecretRef reference seems to be bypassing the patch+transform flow.

An example on how to construct a custom json blob from connection secret details and normalize it with ToJSON would be much appreciated.

sboschman avatar Jul 22 '22 12:07 sboschman

Thanks @negz for the detailed review and thoughts! I'm unfortunately no longer in a position to dedicate my own personal time or resources to moving this forward. I'd be happy for anyone else on the team in in the community to pick up my contribution and finish it. Please feel free to close this once a replacement PR is open! ❤️

jpmcb avatar Aug 08 '22 03:08 jpmcb

@negz I think it would be a nice feature to add to the crossplane functionalities, what do you think?

braghettos avatar Oct 09 '22 14:10 braghettos