Cannot patch with CombineFromEnvironment

[davejhahn]

I've migrated from a resource composition to a pipeline and have been able to get everything to work except the patching of the annotation, so it defaults to the generated name preventing the resource from being created because it is not valid for the provider

apiVersion: apiextensions.crossplane.io/v1
kind: Composition
metadata:
  name: storage-pipeline
spec:
  compositeTypeRef:
    apiVersion: xxx.xxx.xxx/v1alpha1
    kind: xStorage
  environment:
    environmentConfigs:
    - type: Reference
      ref:
        name: global
    - type: Reference
      ref:
        name: organization
    - type: Reference
      ref:
        name: subscription
    - type: Reference
      ref:
        name: product
    - type: Reference
      ref:
        name: environment
    patches:
      - type: FromCompositeFieldPath
        fromFieldPath: spec.identifier            
        toFieldPath: data.instance.identifier

  mode: Pipeline
  pipeline:
  - step: create-storage
    functionRef:
      name: function-patch-and-transform
    input:
      apiVersion: pt.fn.crossplane.io/v1beta1
      kind: Resources
      resources:
      - name: storage
        base:
          apiVersion: storage.azure.upbound.io/v1beta1
          kind: Account
              
        patches:

          - type: CombineFromEnvironment
            combine:
              strategy: string
              variables:
              - fromFieldPath: organization.identifier
              - fromFieldPath: subscription.identifier
              - fromFieldPath: environment.code
              - fromFieldPath: location.short
              - fromFieldPath: product.identifier
              - fromFieldPath: data.instance.identifier
              string: 
                fmt: "%s%sst%s%s%s%s"
            toFieldPath: metadata.annotations[crossplane.io/external-name]

          - type: FromEnvironmentFieldPath
            fromFieldPath: storage.accountReplicationType
            toFieldPath: spec.forProvider.accountReplicationType
          - type: FromCompositeFieldPath
            fromFieldPath: spec.accountReplicationType
            toFieldPath: spec.forProvider.accountReplicationType

          - type: FromEnvironmentFieldPath
            fromFieldPath: storage.accountTier
            toFieldPath: spec.forProvider.accountTier
          - type: FromCompositeFieldPath
            fromFieldPath: spec.accountTier
            toFieldPath: spec.forProvider.accountTier

          - type: FromEnvironmentFieldPath
            fromFieldPath: storage.accountKind
            toFieldPath: spec.forProvider.accountKind
          - type: FromCompositeFieldPath
            fromFieldPath: spec.accountKind
            toFieldPath: spec.forProvider.accountKind

          - type: FromEnvironmentFieldPath
            fromFieldPath: storage.allowNestedItemsToBePublic
            toFieldPath: spec.forProvider.allowNestedItemsToBePublic
          - type: FromCompositeFieldPath
            fromFieldPath: spec.allowNestedItemsToBePublic
            toFieldPath: spec.forProvider.allowNestedItemsToBePublic

          - type: FromEnvironmentFieldPath
            fromFieldPath: storage.publicNetworkAccessEnabled
            toFieldPath: spec.forProvider.publicNetworkAccessEnabled
          - type: FromCompositeFieldPath
            fromFieldPath: spec.publicNetworkAccessEnabled
            toFieldPath: spec.forProvider.publicNetworkAccessEnabled

          - type: FromEnvironmentFieldPath
            fromFieldPath: location.long
            toFieldPath: spec.forProvider.location

          - type: FromEnvironmentFieldPath
            fromFieldPath: product.resourceGroupName
            toFieldPath: spec.forProvider.resourceGroupName
        
          - type: ToCompositeFieldPath
            fromFieldPath: status.atProvider.id
            toFieldPath: status.id

Everything else appears to patch correctly. I also tried patching the same thing to "metadata.name" and it also does not patch.

When I run the same from a resource composition it patches as expected.

[davejhahn]

Doing some more debugging and it seems that the problem is not that it cannot patch metadata, but it can't patch using CombineFromEnvironment.

e.g. if I provide the external name in the XR (which is not what I want to do, but just for testing):

          - type: FromCompositeFieldPath
            fromFieldPath: spec.externalName
            toFieldPath: annotations[crossplane.io/external-name]

It sets the annotation as expected. But when I use CombineFromEnvironment it does not.

Also, just tried doing a patch with FromEnvironmentFieldPath, not a value that would work for the provider, but just to see if it sets it and it does:

      - type: FromEnvironmentFieldPath
        fromFieldPath: storage.accountReplicationType
        toFieldPath: metadata.annotations[crossplane.io/external-name]

So it seems that CombineFromEnvironment doesn't patch properly in function-patch-and-transform, unless I'm just doing something wrong.

And reiterating, CombineFromEnvironment, doing the same exact thing, in a composition that just has a resource instead of a pipeline, it works as expected.

[davejhahn]

Updated title to reflect myfindings.

[stevendborrelli]

Hey @davejhahn, have you been able to test a build with #55? I don't think the latest release includes this fix.

[davejhahn]

@stevendborrelli no I have not, but will try now.

[davejhahn]

@stevendborrelli just a clarification, is there an image already pushed to xpkg.upbound.io/crossplane-contrib/function-patch-and-transform with this version or do I need to build it?

[stevendborrelli]

You might be able to use the version from the pull request xpkg.upbound.io/crossplane-contrib/function-patch-and-transform:v0.0.0-20231130064014-75b5a45cf119

[davejhahn]

Ok, tried that version, doesn't seem to have resolved what I was seeing. I will try and get a package created from #55 above. I just haven't gotten to the point of pushing functions to our container registry and need to work through that.

[davejhahn]

@stevendborrelli I was able to build and create a package, push to our private CR and deploy the function, for the PR #55, but the problem still persists unfortunately.

[marquesj2-ppb]

Have you been able to achieve something like this @davejhahn ?

[MisterMX]

@davejhahn this issue should be fixed by #78 and #96. Can you confirm this?