Patching from environment patches does not work with Pipeline mode

[lajchon]

What happened?

When switching to mode: Pipeline in Composition and utilizing function-patch-and-transform, spec.environment.patches are never applied. Switching to spec.resources, the composite resource is successfully patched.

How can we reproduce it?

Create new kind cluster:

kind create cluster

Deploy Crossplane 1.14.5:

helm upgrade --install crossplane --namespace crossplane-system --create-namespace --set args={"--enable-environment-configs", "--enable-composition-functions=true"} --version 1.14.5 crossplane-stable/crossplane

Deploy Provider, Composition and CompositeResourceDefinition:

---
apiVersion: pkg.crossplane.io/v1
kind: Provider
metadata:
  name: provider-nop
spec:
  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.0
  ignoreCrossplaneConstraints: true
---
apiVersion: apiextensions.crossplane.io/v1
kind: Composition
metadata:
  name: nop.sqlinstances.example.org
spec:
  compositeTypeRef:
    apiVersion: example.org/v1alpha1
    kind: XSQLInstance
  environment:
    environmentConfigs:
      - ref:
          name: example-environment-1
        type: Reference
    patches:
      - type: ToCompositeFieldPath
        fromFieldPath: labels.environment
        toFieldPath: metadata.annotations[valueFromEnv]
      - type: ToCompositeFieldPath
        fromFieldPath: annotations.environment
        toFieldPath: metadata.labels[valueFromEnv]
  mode: Pipeline
  pipeline:
    - step: patches
      functionRef:
        name: function-patch-and-transform
      input:
        apiVersion: pt.fn.crossplane.io/v1beta1
        kind: Resources
        resources:
          - name: nop
            base:
              apiVersion: nop.crossplane.io/v1alpha1
              kind: NopResource
              spec:
                forProvider:
                  fields:
                    fromNopResource: fromNopResource
                  conditionAfter:
                    - conditionType: Ready
                      conditionStatus: "False"
                      time: 0s
                    - conditionType: Ready
                      conditionStatus: "True"
                      time: 10s
            patches:
              - type: ToCompositeFieldPath
                fromFieldPath: spec.forProvider.fields.fromNopResource
                toFieldPath: status.valueFromNopResource
---
apiVersion: apiextensions.crossplane.io/v1
kind: CompositeResourceDefinition
metadata:
  name: xsqlinstances.example.org
spec:
  group: example.org
  names:
    kind: XSQLInstance
    plural: xsqlinstances
  claimNames:
    kind: SQLInstance
    plural: sqlinstances
  defaultCompositionRef:
    name: nop.sqlinstances.example.org
  defaultCompositeDeletePolicy: Background
  defaultCompositionUpdatePolicy: Automatic
  versions:
    - name: v1alpha1
      served: true
      referenceable: true
      schema:
        openAPIV3Schema:
          type: object
          properties:
            spec:
              type: object
              properties:
                parameters:
                  type: object
                  properties:
                    storageGB:
                      type: integer
                  required:
                    - storageGB
              required:
                - parameters
            status:
              type: object
              properties:
                address:
                  description: Address of this MySQL server.
                  type: string
                valueFromEnv:
                  description: ValueFromEnv is the value of the environment variable.
                  type: string
                valueFromNopResource:
                  description: valueFromNopResource is the value of field in the nop resource.
                  type: string

Deploy EnvironmentConfig:

---
apiVersion: apiextensions.crossplane.io/v1alpha1
kind: EnvironmentConfig
metadata:
  name: example-environment-1
data:
  labels:
    environment: develop
  annotations:
    environment: develop

Deploy Claim:

---
apiVersion: example.org/v1alpha1
kind: SQLInstance
metadata:
  name: example
  namespace: default
  labels:
    stage: develop
spec:
  parameters:
    storageGB: 10

Output resources:

kubectl get sqlinstances.example.org,xsqlinstances.example.org,nopresources.nop.crossplane.io -o yaml

Results:

apiVersion: v1
kind: List
metadata:
  resourceVersion: ""
items:
- apiVersion: example.org/v1alpha1
  kind: SQLInstance
  metadata:
    annotations:
      kubectl.kubernetes.io/last-applied-configuration: |
        {"apiVersion":"example.org/v1alpha1","kind":"SQLInstance","metadata":{"annotations":{},"labels":{"stage":"develop"},"name":"example","namespace":"default"},"spec":{"parameters":{"storageGB":10}}}
    finalizers:
    - finalizer.apiextensions.crossplane.io
    labels:
      stage: develop
    name: example
    namespace: default
  spec:
    ...
    resourceRef:
      apiVersion: example.org/v1alpha1
      kind: XSQLInstance
      name: example-5vlrn
  status:
    conditions:
    ...
    valueFromNopResource: fromNopResource
- apiVersion: example.org/v1alpha1
  kind: XSQLInstance
  metadata:
    finalizers:
    - composite.apiextensions.crossplane.io
    generateName: example-
    labels:
      crossplane.io/claim-name: example
      crossplane.io/claim-namespace: default
      crossplane.io/composite: example-5vlrn
      stage: develop
    name: example-5vlrn
  spec:
    claimRef:
      apiVersion: example.org/v1alpha1
      kind: SQLInstance
      name: example
      namespace: default
    compositionRef:
      name: nop.sqlinstances.example.org
    compositionRevisionRef:
      name: nop.sqlinstances.example.org-8e5a858
    compositionUpdatePolicy: Automatic
    parameters:
      storageGB: 10
    resourceRefs:
    - apiVersion: nop.crossplane.io/v1alpha1
      kind: NopResource
      name: example-5vlrn-k855k
  status:
    conditions:
    ...
    valueFromNopResource: fromNopResource
- apiVersion: nop.crossplane.io/v1alpha1
  kind: NopResource
  metadata:
    annotations:
      crossplane.io/composition-resource-name: nop
      crossplane.io/external-name: example-5vlrn-k855k
    finalizers:
    - finalizer.managedresource.crossplane.io
    generateName: example-5vlrn-
    generation: 3
    labels:
      crossplane.io/claim-name: example
      crossplane.io/claim-namespace: default
      crossplane.io/composite: example-5vlrn
    name: example-5vlrn-k855k
    ownerReferences:
    - apiVersion: example.org/v1alpha1
      blockOwnerDeletion: true
      controller: true
      kind: XSQLInstance
      name: example-5vlrn
  spec:
    deletionPolicy: Delete
    forProvider:
      conditionAfter:
      - conditionStatus: "False"
        conditionType: Ready
        time: 0s
      - conditionStatus: "True"
        conditionType: Ready
        time: 10s
      fields:
        fromNopResource: fromNopResource
    providerConfigRef:
      name: default
  status:
    atProvider: {}
    conditions:
    ...

Expect XSQLInstance to contain the following annotation and label, but it does not, even after Claim reports true for Synced and Ready.

---
apiVersion: example.org/v1alpha1
kind: XSQLInstance
metadata:
  annotations:
    valueFromEnv: "1"
  labels:
    valueFromEnv: "1"

Lastly, the same also appears to be true when using spec.environment.patches[].type: ToEnvironmentFieldPath. New key is never made available through in-memory environment.

What environment did it happen in?

Crossplane version: v1.14.5
function-patch-and-transform version: v0.2.1

[phisco]

This should work as expected:

---
apiVersion: apiextensions.crossplane.io/v1
kind: Composition
metadata:
  name: nop.sqlinstances.example.org
spec:
  compositeTypeRef:
    apiVersion: example.org/v1alpha1
    kind: XSQLInstance
  environment:
    environmentConfigs:
      - ref:
          name: example-environment-1
        type: Reference
  mode: Pipeline
  pipeline:
    - step: patches
      functionRef:
        name: function-patch-and-transform
      input:
        apiVersion: pt.fn.crossplane.io/v1beta1
        kind: Resources
        environment:
          patches:
            - type: ToCompositeFieldPath
              fromFieldPath: labels.environment
              toFieldPath: metadata.annotations[valueFromEnv]
            - type: ToCompositeFieldPath
              fromFieldPath: annotations.environment
              toFieldPath: metadata.labels[valueFromEnv]
        resources:
          - name: nop
            base:
              apiVersion: nop.crossplane.io/v1alpha1
              kind: NopResource
              spec:
                forProvider:
                  fields:
                    fromNopResource: fromNopResource
                  conditionAfter:
                    - conditionType: Ready
                      conditionStatus: "False"
                      time: 0s
                    - conditionType: Ready
                      conditionStatus: "True"
                      time: 10s
            patches:
              - type: ToCompositeFieldPath
                fromFieldPath: spec.forProvider.fields.fromNopResource
                toFieldPath: status.valueFromNopResource

[jbw976]

here's the diff between the original composition and what @phisco is suggesting:

14,20d13
<     patches:
<       - type: ToCompositeFieldPath
<         fromFieldPath: labels.environment
<         toFieldPath: metadata.annotations[valueFromEnv]
<       - type: ToCompositeFieldPath
<         fromFieldPath: annotations.environment
<         toFieldPath: metadata.labels[valueFromEnv]
28a22,29
>         environment:
>           patches:
>             - type: ToCompositeFieldPath
>               fromFieldPath: labels.environment
>               toFieldPath: metadata.annotations[valueFromEnv]
>             - type: ToCompositeFieldPath
>               fromFieldPath: annotations.environment
>               toFieldPath: metadata.labels[valueFromEnv]

Out of curiosity @lajchon, did you try out the https://github.com/crossplane-contrib/crossplane-migrator when doing this migration from native patch & transform to Functions? We are expecting to include that migration tooling into the official crossplane CLI soon.

[lajchon]

When I try that, my Composite resource reports the following fatal condition:

status:
  conditions:
    - message: >-
        cannot compose resources: pipeline step "patches" returned a fatal
        result: invalid Function input: environment.patches[0][type]: Invalid
        value: "ToCompositeFieldPath": invalid environment patch type
      reason: ReconcileError
      status: 'False'
      type: Synced

Also, when I look at the compositions.apiextensions.crossplane.io CRD spec, it does not appear to support defining spec.pipline[].input.environment as suggested.

@jbw976 I'll try crossplane-migrator now and report back my results.

[lajchon]

@jbw976 same result after using crossplane-migrator.

./crossplane-migrator new-pipeline-composition -i composition.yaml -o composition-pipeline.yaml

Resultant composition-pipeline.yaml:

apiVersion: apiextensions.crossplane.io/v1
kind: Composition
metadata:
  creationTimestamp: "2024-01-19T17:16:51Z"
  name: nop.sqlinstances.example.org
spec:
  compositeTypeRef:
    apiVersion: example.org/v1alpha1
    kind: XSQLInstance
  environment:
    environmentConfigs:
    - ref:
        name: example-environment-1
      type: Reference
  mode: Pipeline
  pipeline:
  - functionRef:
      name: function-patch-and-transform
    input:
      apiVersion: pt.fn.crossplane.io/v1beta1
      environment:
        patches:
        - fromFieldPath: complex.c.f
          toFieldPath: status.valueFromEnv
          type: ToCompositeFieldPath
        - fromFieldPath: complex.c.f
          toFieldPath: metadata.annotations[valueFromEnv]
          type: ToCompositeFieldPath
        - fromFieldPath: complex.c.f
          toFieldPath: metadata.labels[valueFromEnv]
          type: ToCompositeFieldPath
      kind: Resources
      patchSets: []
      resources:
      - base:
          apiVersion: nop.crossplane.io/v1alpha1
          kind: NopResource
          spec:
            forProvider:
              conditionAfter:
              - conditionStatus: "False"
                conditionType: Ready
                time: 0s
              - conditionStatus: "True"
                conditionType: Ready
                time: 10s
              fields:
                fromNopResource: fromNopResource
        name: nop
        patches:
        - fromFieldPath: spec.forProvider.fields.fromNopResource
          toFieldPath: status.valueFromNopResource
          type: ToCompositeFieldPath
    step: patch-and-transform
fromNopResource
                toFieldPath: status.valueFromNopResource
                type: ToCompositeFieldPath

apiVersion: example.org/v1alpha1
kind: XSQLInstance
metadata:
  finalizers:
    - composite.apiextensions.crossplane.io
  generateName: example-
  generation: 2
  labels:
    crossplane.io/claim-name: example
    crossplane.io/claim-namespace: default
    crossplane.io/composite: example-tbct6
    stage: develop
  name: example-tbct6
status:
  conditions:
    - lastTransitionTime: '2024-01-19T17:13:03Z'
      message: >-
        cannot compose resources: pipeline step "patch-and-transform" returned a
        fatal result: invalid Function input: environment.patches[0][type]:
        Invalid value: "ToCompositeFieldPath": invalid environment patch type
      reason: ReconcileError
      status: 'False'
      type: Synced
spec:
  claimRef:
    apiVersion: example.org/v1alpha1
    kind: SQLInstance
    name: example
    namespace: default
  compositionRef:
    name: nop.sqlinstances.example.org
  compositionRevisionRef:
    name: nop.sqlinstances.example.org-8ed5b9b
  compositionUpdatePolicy: Automatic
  parameters:
    storageGB: 10

[phisco]

@lajchon that was fixed here and will be in the next function-patch-and-transform release.

[negz]

If I'm following correctly there's nothing to do on core Crossplane, and the issue is fixed but unreleased in function-patch-and-transform.

I'd transfer this to function-patch-and-transform, but unfortunately you can't transfer issues to different orgs. Given that the issue is fixed and just pending release I'm going to mark this resolved.