ToCompositeFieldPath not updating status on XRD when used in a PatchSet

[cdloh]

What happened?

When attempting to patch back onto the status fields of XRD using ToCompositeFieldPath and a PatchSet the fields are not updated or set. If you create a Patch directly inline it works.

I thought it may have been due to the policy I have set but removing the Optional doesn't change any of the results.

In the below XRD and Composition I expect the status.atProvider.arn and status.uid fields to be updated on the XRD but they aren't. Even after some time that the resource has gone healthy.

I thought it might be https://github.com/crossplane/crossplane/issues/2300 but I am creating the fields on the XRD

How can we reproduce it?

Note that this Composition is largely just a wrapper composition around a normal s3 bucket resource that we use to set some defaults.

The resources themselves are created successfully and healthy.

Composition (using patchsets. Doesnt work) - https://gist.github.com/cdloh/38b56570b41f038df8054d8b6e5077fb Composition (using inline patches. Does work) - https://gist.github.com/cdloh/eddb86740d3597838a6b11cf27b4422c Composite Resource Definition - https://gist.github.com/cdloh/05f798f697d99b8a8789e84f24ee1059

Sample Resource

apiVersion: aws.devops.work.systems/v1alpha1
kind: S3Bucket
metadata:
  name: crossplane-test-bucket
spec:
  compositionRef:
    name: s3-bucket-aws
  deletionPolicy: Orphan
  forProvider:
    locationConstraint: us-east-1
    serverSideEncryptionConfiguration:
      rules:
      - applyServerSideEncryptionByDefault:
          sseAlgorithm: aws:kms
  providerConfigRef:
    name: default

The above compositions and XRDs work fine in 1.3.0 but I can't find anything in the patch notes that would break them. They are still following the documentation that crossplane has as far as I'm aware.

What environment did it happen in?

Crossplane version: v1.8.1 AWS Provider: v0.27.1 EKS: 1.22

[bobh66]

This can be caused by an error that happens during reconciliation. Did you try running the crossplane controller with debug enabled and review the logs?

[cdloh]

Nothing in the logs that suggest it is erroring.

1.6552048656408617e+09	DEBUG	crossplane	Reconciling	{"controller": "defined/compositeresourcedefinition.apiextensions.crossplane.io", "request": "/s3buckets.aws.devops.work.systems", "uid": "69c6602e-4f48-4fa6-90a0-297eebaba0e8", "version": "12471", "name": "s3buckets.aws.devops.work.systems", "controller": "composite/s3buckets.aws.devops.work.systems", "request": "/cloh-test-bucket"}
1.6552048656503649e+09	DEBUG	crossplane.events	Normal	{"object": {"kind":"S3Bucket","name":"cloh-test-bucket","uid":"6b6eb6a8-3be9-4ca1-8b01-46bbd7b5040d","apiVersion":"aws.devops.work.systems/v1alpha1","resourceVersion":"18647"}, "reason": "SelectComposition", "message": "Successfully selected composition"}
1.6552048657246745e+09	DEBUG	crossplane.events	Normal	{"object": {"kind":"S3Bucket","name":"cloh-test-bucket","uid":"6b6eb6a8-3be9-4ca1-8b01-46bbd7b5040d","apiVersion":"aws.devops.work.systems/v1alpha1","resourceVersion":"18647"}, "reason": "ComposeResources", "message": "Successfully composed resources"}

[cdloh]

The above compositions and XRDs work fine in 1.3.0 (just tested) but in 1.8.1 it never updates.

[cdloh]

Right I've gotten further with this as well.

This works as I would expect in v1.4.3

It breaks in v1.5.3 however I can't find anything in the release notes in 1.5.X that would break this functionality.

[jbw976]

Thanks for providing all of this debugging and repro information @cdloh, and for narrowing down the Crossplane version (1.5.x) where the potential regression may be.

One thing to try out here is that I notice the status values are being patched via a PatchSet: https://gist.github.com/cdloh/38b56570b41f038df8054d8b6e5077fb#file-s3-composition-yaml-L297-L308

Does the same behavior happen if you declare the patches directly/inline, instead of via a PatchSet?

Some other questions:

• Should status.atProvider.arn be marked as required?
• Is it intentional that status.atProvider.arn is being patched to both the status fields atProvider.arn and uid?

[cdloh]

Hey @jbw976

Does the same behavior happen if you declare the patches directly/inline, instead of via a PatchSet?

Just tested this. If I put them directly into the patches it all works as expected. Just copy and pasting the patches.

Should status.atProvider.arn be marked as required?

Honestly I'm not entirely sure. It doesn't matter for the status of the XRD imo but still learning all of this.

Is it intentional that status.atProvider.arn is being patched to both the status fields atProvider.arn and uid?

Yes. We just have a general practise internally of storing a UID of a resource into a UID status field. That way regardless of the CloudProvider that is managing the resource you can know that that field is going to contain a UID field.

[haarchri]

we facing the same issue

when we using

  - name: Status
    patches:
    - fromFieldPath: metadata.annotations["crossplane.io/external-name"]
      policy:
        fromFieldPath: Optional
      toFieldPath: status.uid
      type: ToCompositeFieldPath

it is not working in patchSet - directly it is working

[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 - I know this is still causing pain for folks 😂

[lsviben]

I did some investigation on this issue and I am happy to say that it seems like this is already fixed :1st_place_medal: . At least for v1.11+ . For older versions it still unfortunately broken.

I tried to replicate the behaviour with the provided gist data, and here are some results:

All of the tests were done with the AWS Community Provider v0.33.0 (that is the earliest I could find available)

Crossplane v1.9.2: Issue appears Crossplane v1.10.3: Issue appears Crossplane v1.11.2: No issue

I narrowed down the commit that fixed it to: https://github.com/crossplane/crossplane/commit/fdb1ffd91de15d8ba5b2a3060e02fd9c6c8e7c3c

which was a part of a bigger change (introducing Functions) : https://github.com/crossplane/crossplane/pull/3465

Ill try to figure out if I can extract the part that fixed it to something we could backport to the earlier releases.

[ytsarev]

@lsviben that is dope, thanks a ton for the investigation 👍

[cdloh]

Thanks for the work @lsviben !

We're in the process of doing the final bump to 1.11 so this will be great to have it fixed.

[jbw976]

Thanks for the investigation into this issue @lsviben, this issue has been a bit of a mystery for a bit!

For more visibility, we discussed more offline and came to the conclusion that we will not backport a fix to previous releases for the following reasons:

• This scenario is working OK now in v1.11+ and will also be working well in v1.12 which releases this month. At v1.12 release time, that will be 2/3 of the supported versions that have the fix.
• A potential backport of this functionality has a degree of risk associated with it as well as a cost of time/effort because this issue was fixed as part of a much larger feature implementation. A scoped fix to backport to previous releases would require some untangling of functionality and potentially introduce new regressions into previous versions.
• The affected folks that have been waiting for a fix are mostly OK with using v1.11+, so we don't see a large benefit to taking on this risk and cost.

Thanks again @lsviben for looking into this and sharing your findings. Folks are of course always welcome to continue discussion here on this issue, but I'll go ahead and close it for now with this decision ✅ 🚀