Add path annotation to apibindings

[mjudeikis]

Summary

This adds path annotation to apibindings. APIbindings are accessible via VirtualWorkspaces. This means providers will be able to resolve the canonical path and cluster more easily.

- apiVersion: apis.kcp.io/v1alpha2
  kind: APIBinding
  metadata:
    annotations:
      kcp.io/cluster: hezfu00lhrvcdnls
      kcp.io/path: root:test

Caveat: there is a race condition with logicalcluster. But it resolves quite quickly due to the binding handshake, which is followed by the update. Not ideal, but the alternatives (the ones I could think of) were even more hacky.

What Type of PR Is This?

/kind feature

Related Issue(s)

Fixes #3673

Release Notes

Add kcp.io/path annotation to APIBindings

[kcp-ci-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from mjudeikis. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• pkg/admission/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[mjudeikis]

moving to draft. some strange errors pop up. Investigating.

[mjudeikis]

/test all

[mjudeikis]

/retest

[gman0]

Another approach would be to:

1. Skip mutating/validating objects whose logical cluster resolves to NotFound:
   • The reasoning would be that it's probably not a job of this plugin to deny requests with missing LogicalCluster -- something else should catch and validate this if we really need such a check.
2. Modify the APIBinding admission plugin to add an empty kcp.io/path annotation on the object if it's missing during create/update.
   • The pathannotation admission plugin would then catch this, and update the path accordingly once the APIBinding's logical cluster is ready.

Maybe this divides the responsibilities a bit better? But it may be that I'm missing some knowledge about why it is done like that in the first place :P

[gman0]

Also, existing APIBindings are not updated right? Is that a problem?

[mjudeikis]

Another approach would be to:

1. Skip mutating/validating objects whose logical cluster resolves to NotFound:

   • The reasoning would be that it's probably not a job of this plugin to deny requests with missing LogicalCluster -- something else should catch and validate this if we really need such a check.

2. Modify the APIBinding admission plugin to add an empty kcp.io/path annotation on the object if it's missing during create/update.

   • The pathannotation admission plugin would then catch this, and update the path accordingly once the APIBinding's logical cluster is ready.

Maybe this divides the responsibilities a bit better? But it may be that I'm missing some knowledge about why it is done like that in the first place :P

So my code is attempting to do option 2:
But I didn't want to do an empty annotation as some of the other code relies on this, and empty is invalid. So skipping kinda looked like a better alternative.

And it still gets updated/added once a logical cluster is added and the next resync/ status update.

[gman0]

I agree 👍

/lgtm

[kcp-ci-bot]

LGTM label has been added.

Git tree hash: ce0a2111a22aca51e986c418f7d76cd7090bf820