-
Notifications
You must be signed in to change notification settings - Fork 23
Fix: Allow PascalCase in Resource of ExternalFramework #1237
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
|
/assign @kannon92 |
Kind of GVK in kubernetes is PascalCase. e.g. Kind is Deployment not deployment, PipelineRun not pipelinerun. Forcing resource to lowercase causes failure for externalFramework.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: khrm The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@khrm: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
|
Wouldn't this break @gbenhaim and the konflux integration? |
|
eh nvm. you are allowing either lowercase or uppercase. |
| // resource is the Resource type of the external framework. | ||
| // Resource types are lowercase and plural (e.g. pods, deployments). | ||
| // Must be a valid DNS 1123 label consisting of a lower-case alphanumeric string | ||
| // Resource types are lowercase and plural (e.g. pods, deployments) or Kind (e.g. Job, Pod). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resources and kinds are different things, they are not interchangeable
A Resource is part of the rest path of the resource. Using Group, Version and Resource I can concretely identify the correct rest path to which this object is referring.
What you are suggesting is to use Kind, Kind doesn't mean anything in terms of the actual rest path, and, Kind does not necessarily need to be unique within a GroupVersion.
To go from GroupVersionKind to the rest path, I must go through API discovery, fetch a rest mapping, and then hope that only one result is returned. This rest mapping will then give me the Resource, so that I can construct the appropriate URL to send my request to.
The API design here uses GVR (Which is accurate) and not GVK (which is ambiguous) |
|
/hold |
|
@JoelSpeed @kannon92 In upstream, it's GVK not GVR. Using GVR, causes failure for MultiKueue. |
|
Let me recheck this. |
|
@khrm what is the status of this? |
Kind of GVK in kubernetes is PascalCase. e.g. Kind is Deployment not deployment, PipelineRun not pipelinerun.