-
Notifications
You must be signed in to change notification settings - Fork 12
Support arbitrary related resources #119
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test all |
8fd6894 to
80d0940
Compare
|
/test all |
|
/test all |
On-behalf-of: @SAP [email protected]
|
/test all |
|
/retest |
|
@xrstf: The following tests failed, say
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Summary
This PR implements one of the OG "yeah this will be extended in the future" tickets, allowing for related resources to be of any group, version or resource.
For this, the
PublishedResourceCRD has been extended to not takekind, but instead takegroup,versionandresource. The oldkindfield is still supported, but deprecated, and can only handle ConfigMaps and Secrets, as before. If you want to have anything else, configure GVR instead.(Why
resourceall of a sudden when everything else inside a PR is Kind-based? Because to figure out if a resource needs to be permission claimed, we would first need to figure out if that resource belongs to the same APIExport, and APIExports only contain resources and so we cannot resolve it before adding it, but to add it we need to resolve it and so Kinds are just not suitable here. I don't like it either, but ... yeah.)Integration of custom GVRs into the syncer itself was pretty harmless, the more elaborate part was the APIExport controller which now has to figure out if a related resource points to a foreign resource (of another APIExport), so it can configure the permission claims correctly. Doing that while taking into account that PublishedResources are transmitted asynchonously (in the
apiresourceschemacontroller) took a bit more code than expected.Other than that, the change is relatively straightforward.
What Type of PR Is This?
/kind feature
Related Issue(s)
Fixes #109
Release Notes