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
Feature: Validates partial path for flow-schema's non-resource-url rules #84706
Feature: Validates partial path for flow-schema's non-resource-url rules #84706
Conversation
fec7c90
to
5928ca8
Compare
// 2. A non-resource-url path must not end up with slash | ||
// 3. Continuous/double slash is forbidden in the path | ||
// 4. White-space is forbidden in the path | ||
// 5. Wildcard "*" should be only character in a segment if present |
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.
what is the use-case for honoring embedded wildcards (as opposed to suffix-only wildcards)?
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.
that depends on whether wildcard can match slash or not. rbac matcher matches slash, should we align w/ rbac in the first place?
kubernetes/pkg/apis/rbac/v1/evaluation_helpers.go
Lines 107 to 109 in 1488460
if strings.HasSuffix(ruleURL, "*") && strings.HasPrefix(requestedURL, strings.TrimRight(ruleURL, "*")) { | |
return true | |
} |
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.
RBAC only does suffix glob matching. I haven't seen compelling use cases around internal glob matching.
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.
i updated to align w/ rbac. the wildcard in rbac matches slash, e.g. /healthz/*
will match /healthz/poststarthook/start-kube-apiserver-informers
. if wildcard doesnt match slash, we have to set /healthz/*/*
otherwise. i just added a note to emphasis that..
0f7be1e
to
829aea9
Compare
/assign @deads2k |
829aea9
to
da612a3
Compare
/test pull-kubernetes-integration |
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.
/LGTM
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, liggitt, yue9944882 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/sig api-machinery
/kind feature