-
Notifications
You must be signed in to change notification settings - Fork 409
Add verbs bundle validation #3429
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
Conversation
[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 |
1344f1a
to
ede3efd
Compare
Signed-off-by: Mangirdas Judeikis <[email protected]> On-behalf-of: @SAP [email protected]
ede3efd
to
3c441fc
Compare
// validateVerbsBundle validates that the verbs are a valid bundle according | ||
// to the following rules: | ||
// get | possible standalone | ||
// list -> get, watch | if list requested, always requires get and watch too |
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.
Not to bikeshed this list into all of eternity, but I wonder if watch
should be a requirement for list
now? A script/tool that just fetches all objects doesn't necessarily create a watch
on the objects.
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.
But idea is that you can get objects data. So its still "read" type of action
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.
+1 to what @embik said, I can see valid cases where you want to allow list
, but not watch
, there are in a way distinct verbs.
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.
In my head its more about "what operations you can do to the object, and less about the kube world". Does it give you same access as other verbs in the bundle, when yes.
And again, we can always relax it later, where making it stricted it would be breaking change.
I think we should add documentation for this in this PR. I'm not 100% sure where to put it in the docs, but this extra validation should at least be mentioned somewhere. |
Summary
Add verbs bundles validation for permissionsClaims
What Type of PR Is This?
/kind feature
Related Issue(s)
Fixes #
Release Notes