feature: CachedResources should validate that the replicated resource is cluster-wide

[gman0]

Feature Description

Virtual resources (https://github.com/kcp-dev/kcp/pull/3620) for Replication VW expose the replicated resource as-is: if the resource is namespaced, the objects will be made available in their respective namespaces.

The problem is that if that namespace doesn't exist in consumer's workspace, the replicated objects are still made available in that namespace regardless:

• In provider:
  • Create a CRD with a namespaced resource.
  • Create a CachedResource for that resource.
  • Create an object of that resource in namespace exists-only-in-provider.
  • (Create an APIResourceSchema from the CRD, to be used by the APIExport in the next step.)
  • Create an APIExport for that CachedResource.
• In consumer:
  • Create an APIBinding for the export created above.
  • In consumer's workspace, running kubectl -n exists-only-in-provider get myresources succeeds.

This breaks the semantics of how namespaced resources are expected to behave.

Proposed Solution

We've decided to allow only cluster-wide resources for now. There is a number of ways to implement this:

• (a) As a CachedResource admission plugin:
  • We check the source schema of the cached resource during creation or modification of the CachedResource object.
  • There is a corner case where the schema changes during the lifetime of its associated CachedResource object, and escapes the validation.
• (b) As a CachedResource condition from the replication controller:
  • The controller can detect that the schema of the object it's about to replicate is namespaced, and so it sets up the condition with CachedResourceValid=false.
• (c) As an APIExport admission plugin:
  • The schema that is ultimately used in consumers is the one specified in APIExport's resource schema. We could make the check there. This approach is problematic though: the schema resource definition in APIExport contains only the reference to an endpoint slice (.spec.resources[].storage.virtual.reference). We'd have to make an exception for CachedResources, or any other virtual-resource-kind-of-resource. I don't think this is viable.

I'd vote for (b) as it is sufficient, but we can additionally implement (a) later, if it improves the user experience.

Alternative Solutions

No response

Want to contribute?

• [ ] I would like to work on this issue.

Additional Context

No response

[mjudeikis]

Can we do A and B? Should not be hard to have both?

[gman0]

Yes I thought a combination of A and B at first too, but then figured that functionally-wise, B would be enough for now. I agree that B alone is not very user friendly though -- we shouldn't allow creating invalid CachedResources anyway. So let's do it!

[gman0]

/assign