Prevent superfluous external-name annotation on XRs

[ulucinar]

Signed-off-by: Alper Rifat Ulucinar ulucinar@users.noreply.github.com

Description of your changes

Fixes #2350

I have:

• [X ] Read and followed Crossplane's contribution process.
• Run make reviewable test 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

Using getting-started guide's aws (with default VPC) configuration, I provisioned an XR via its claim. The superfluous external-name annotation is not observed. Please see the comment below for the semantics of this change.

[ulucinar]

The semantics of this change are as follows. The context is the interaction between a claim and the corresponding XR. external-name below refers to the external-name annotation:

• For already existing XRs (statically provisioned), the source of truth for the external-name is the XR. Even if an external-name is provided for the claim, claim's external-name is reconciled to (statically provisioned) XR's claim, and it's removed if the (statically provisioned) XR does not contain an external-name. This is motivated by

  crossplane/internal/controller/apiextensions/claim/configurator.go

  Line 47 in bab887d

  // It's possible we're being asked to configure a statically provisioned

• We no longer add a superfluous external-name if the claim does not have one (and XR is dynamically-provisioned (via the claim))
• For dynamically-provisioned XRs (via their claims) if the claim contains an external-name annotation, it's also used for the XR. And as explained above, external-name on the claim is reconciled from the XR's external-name. This is motivated by:
  

  crossplane/internal/controller/apiextensions/claim/api.go

  Line 66 in bab887d

  // Propagate the actual external name back from the composite to the claim.

[ulucinar]

We now have a behavior change in case there is a mismatch between a claim's external-name and the bound XR's external name: Controller now returns an error and refuses to reconcile the XR with the following error message:

mismatch of external names between claim and composite resource

[negz]

This looks good to me, apart from a few small nits.

WRT whether we return an error or overwrite the claim's external name when they conflict, I don't feel strongly either way. You make a good point at #2353 (comment) @ulucinar and I'd also be happy to approve the variant you suggest if you feel that is the better path.

[negz]

This looks great @ulucinar! Thank you. Let me know when you're ready for me to merge this.

[ulucinar]

Hi @negz,
After the backport changes are merged into their respective release branches, do we follow the same release procedure as wesaas for instance? I think I have read something in the lines of accumulating and releasing things in batches for crossplane, is this correct? Do I need to do anything else after this is merged?

[negz]

@ulucinar I think we will want to include these in patch release "soon", but whether we release patch releases just for this bug or wait for a few more to build up is pretty open ended. We should definitely get the backports done, but we might want to hold off a little to see if we can get a few more bugs fixed before we cut patch releases.

[github-actions]

Successfully created backport PR #2419 for release-1.1.

[github-actions]

Successfully created backport PR #2420 for release-1.2.

[github-actions]

Successfully created backport PR #2421 for release-1.3.