crossplane-contrib / function-patch-and-transform

A patch & transform composition function
https://crossplane.io
Apache License 2.0
23 stars 24 forks source link

Cannot patch with CombineFromEnvironment #68

Open davejhahn opened 9 months ago

davejhahn commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

Updated title to reflect myfindings.

stevendborrelli commented 9 months ago

Hey @davejhahn, have you been able to test a build with https://github.com/crossplane-contrib/function-patch-and-transform/pull/55? I don't think the latest release includes this fix.

davejhahn commented 9 months ago

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

davejhahn commented 9 months ago

@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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

@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 commented 8 months ago

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

MisterMX commented 6 months ago

@davejhahn this issue should be fixed by https://github.com/crossplane-contrib/function-patch-and-transform/pull/78 and https://github.com/crossplane-contrib/function-patch-and-transform/pull/96. Can you confirm this?