Preserve rendered annotations & labels on composed resources

[ulucinar]

Description of your changes

As detailed in #2416, we would like to keep any rendered annotations or labels on composed resources when patches on metada.annotations or metadata.labels are applied. Notably, if there is a patch (from composition to composed) is defined on metadata.annotations in the composition:

...
  patchSets:
  - name: metadata
    patches:
    - fromFieldPath: metadata.annotations
      type: FromCompositeFieldPath

and the composite has non-zero metadata.annotations, we lose the rendered crossplane.io/composition-resource-name: rdsinstance annotation on the composed.

Fixes #2416

I have:

• Read and followed Crossplane's contribution process.
• 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

With a composition specifying a patch on metadata.annotations (exemplified above) selected, a dynamically provisioned XR with the following annotations:

...
metadata:
  annotations:
    a: b
  labels:
    crossplane.io/claim-name: my-db
    crossplane.io/claim-namespace: default
    crossplane.io/composite: my-db-p7vk9
...

now renders the following composed resource:

...
metadata:
  annotations:
    a: b
    crossplane.io/composition-resource-name: rdsinstance
  labels:
    crossplane.io/claim-name: my-db
    crossplane.io/claim-namespace: default
    crossplane.io/composite: my-db-p7vk9
...

Without the proposed changes, it renders a composed like the following:

...
metadata:
  annotations:
    a: b
  labels:
    crossplane.io/claim-name: my-db
    crossplane.io/claim-namespace: default
    crossplane.io/composite: my-db-p7vk9
...

[hasheddan]

@ulucinar would this change mean that it is now impossible to patch over the metadata.name and metadata.namespace values? If so, I think this may be a breaking change that we are not willing to make. The existing behavior is desirable in cases where you want to provision a resource like a ProviderConfig with an identifying name that is used to group all the resources in a composition: https://github.com/upbound/platform-ref-aws/blob/63e724fc104ca1147c61787acc8a2891aced0393/cluster/eks/composition.yaml#L180

[ulucinar]

Hi @hasheddan,
Yes it would prevent such use cases, thanks for pointing to the example use case.

[ulucinar]

With the second commit, we now allow such patches.

[hasheddan]

This seems reasonable to me -- we are essentially mandating that our opinionated labels always be present. I would like for @negz to sign off on this though as it is somewhat subjective and I believe he originally added this functionality.

[negz]

This Fix(NNNN) practice is new to me. Do you think it's something we should adopt? Typically I've mostly trusted git to create the audit trail we need through commit messages, git blame, etc such that code can be traced back to a PR. We do specifically link to a PR or via its full URL in some cases when we feel that the reader would benefit from additional context.

[ulucinar]

The idea is to use a concise format similar to GitHub's autolinked references to refer to a related issue (in case of a Fix, to refer to an issue reporting a bug) in order to add some more context on the source listing itself. I think it draws attention to the specific site and warns future maintainers about a potential pitfall on the site. But we can of course embed full URLs or just leave comments (+ git blame) to achieve the same goal.

[github-actions]

Backport failed for release-1.1, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin release-1.1
git worktree add -d .worktree/backport-2423-to-release-1.1 origin/release-1.1
cd .worktree/backport-2423-to-release-1.1
git checkout -b backport-2423-to-release-1.1
ancref=$(git merge-base 1842c4bb2e1258980bca4eda43a55761fe8e19f4 e83ee5777cbc3beab66ed7422654b30a43a66592)
git cherry-pick -x $ancref..e83ee5777cbc3beab66ed7422654b30a43a66592

[github-actions]

Backport failed for release-1.2, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin release-1.2
git worktree add -d .worktree/backport-2423-to-release-1.2 origin/release-1.2
cd .worktree/backport-2423-to-release-1.2
git checkout -b backport-2423-to-release-1.2
ancref=$(git merge-base 1842c4bb2e1258980bca4eda43a55761fe8e19f4 e83ee5777cbc3beab66ed7422654b30a43a66592)
git cherry-pick -x $ancref..e83ee5777cbc3beab66ed7422654b30a43a66592

[github-actions]

Successfully created backport PR #2428 for release-1.3.

[hasheddan]

@ulucinar it looks like we will need manual backports to release-1.1 and release-1.2, do you want to get those in motion?