Repository navigation
Conversation
The restructured authorization chain splits the authorizers into bypass, grant and gate groups, but the only e2e coverage for the gates was the maximal permission policy webhook escalation test. Add an e2e test that runs kcp with an authorization webhook evaluated before the RBAC chain, and shows that with the webhook allowing everything: - the required-groups gate still denies users outside the annotated groups, while the same user impersonating the required group passes, - the system CRD gate still denies APIExport and APIBinding status updates, although creating those resources is not gated, - a webhook Deny short-circuits requests the gates would have allowed. The gates are exercised through the same logical-cluster requests the chain guards, reusing the webhook and server setup helpers from the authorization order tests. Refs: kcp-dev#4512 Signed-off-by: GUOZIJAN <GUOZIJAN@users.noreply.github.com>
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. 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. |
kcp-ci-bot
added
do-not-merge/release-note-label-needed
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
kcp-ci-bot
added
do-not-merge/needs-kind
|
Hi @GUOZIJAN. Thanks for your PR. I'm waiting for a kcp-dev member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
| // The webhook is evaluated before the RBAC chain, so with the webhook | ||
| // allowing everything, the gates are the only remaining thing that can | ||
| // deny a request. | ||
| srv, kcpClusterClient, kubeClusterClient := setupTest(t, "Webhook,AlwaysAllowGroups,AlwaysAllowPaths,RBAC", webhookCAFile, webhookPort) |
There was a problem hiding this comment.
I think after recent changes in authz changes this now need to change. See #4507
|
Once adjusted, please from draft. Moving to draft to keep track of ready to merge and not yet prs ;) |
kcp-ci-bot
added
the
do-not-merge/work-in-progress
|
Closing this — #4517 landed the same e2e coverage (required-groups gate, the system CRD gate on apiexports/apibindings status, and webhook Deny short-circuit) while this PR was in draft, and issue #4512 is now closed as completed. Thanks @souvik-biswas-dev for the thorough tests, and @mjudeikis for the quick turnaround on the review. |
Note
Drafted with an AI coding assistant (ZCode) and reviewed before submitting.
TL;DR
apiexports/statusandapibindings/statusupdates (creating those resources is not gated), and a webhookDenyshort-circuits requests the gates would have allowed.RunWebhook,setupTest) and the required-groups fixture (framework.WithRequiredGroups).What I verified
GOOS=linux go build); I could not run kcp e2e locally on Windows, so CI is the gate here.pkg/authorization/requiredgroups_authorizer.goandpkg/authorization/system_crd_authorizer.go.Resolves: #4512