Repository navigation
[release-0.33] fix(apiexport): treat "*" verb in permission claims as wildcard - #4541
Conversation
Motivation: In the APIExport virtual workspace binding authorizer, the allowed verbs of a permission claim were computed as a plain set intersection of the APIBinding and APIExport verbs. The literal "*" was not treated as a wildcard, so an export claim with verbs ["*"] combined with a binding claim listing explicit verbs produced an empty set, and every namespaced request for the claimed resource was denied with "failed to find suitable reason to allow access in APIBinding". Wildcard-cluster requests bypass this loop and were unaffected. Approach: If the export claim contains "*", the binding's verbs are allowed; if the binding claim contains "*", the export's verbs are allowed; otherwise the intersection is used as before. A wildcard on one side therefore never grants more than the other side permits. This matches how the permission claim label reconciler already treats "*". Validation: added two cases to TestBoundAPIAuthorizer (wildcard on the APIExport only, wildcard on the APIBinding only). Ran `go test ./pkg/virtual/apiexport/authorizer/`: both new cases fail without the change and the package passes with it. Also ran go vet and gofmt on the package. I did not run e2e tests or a live kcp instance. Report: kcp-dev#4503 Assisted-by: claude-sonnet-5-5 (via Claude Code) (cherry picked from commit eee9dfa) Conflicts resolved against release-0.33: the wildcard verb handling is applied to the older claim-matching block. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Signed-off-by: mjudeikis-bot <noreply@users.noreply.github.com>
kcp-ci-bot
added
release-note
kcp-ci-bot
added
do-not-merge/needs-kind
|
Hi @mjudeikis-bot. 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. |
mjudeikis-bot
left a comment
mjudeikis-bot
left a comment
There was a problem hiding this comment.
No issues found.
The wildcard handling in boundAPIAuthorizer.Authorize (binding.go:152-165) is correct. If the export claim has *, the allowed verbs are the binding's verbs. If the binding claim has *, they are the export's verbs. Otherwise the intersection is used. A wildcard on both sides gives {*}, which the final HasAny(verb, "*") accepts. If the export has no matching claim, the export verb set is empty and the request is denied, as before.
Checked
- Read beyond the diff: all of
Authorizeinpkg/virtual/apiexport/authorizer/binding.go, including the bound-resource, apibindings read-only and deny fall-throughs, and thewildcardVerbconstant. I also greppedpkgfor other verb comparisons.permissionclaimlabel_reconcile.go:462already treats*on either side as a wildcard, andreplication/authorizer/content.gois unrelated. - Correctness: the three-way switch is consistent with the final
HasAnycheck. An empty export verb set still denies. Result: no defect. - Concurrency: the code is stateless, with only local sets. Not applicable.
- kcp specifics: the logical-cluster scoping, the API domain key parsing and the binding lookup are unchanged. No new client or lister calls. Result: no change.
- API and storage: no types, CRDs or codegen touched. Not applicable.
- Security: the change only widens the match when one side explicitly says
*. It never exceeds the other side's verbs, so there is no escalation beyond what the export and binding both permit. The result is the same as the intersection semantics once*is expanded. - Tests: the two new cases (wildcard on the export only, wildcard on the binding only) exercise the new branches. With binding.go reverted to the parent commit,
TestBoundAPIAuthorizerfails. With the change it passes, so the tests do detect the fix. - Commands run on the PR head:
go build ./pkg/virtual/apiexport/...passed.go vet ./pkg/virtual/apiexport/authorizer/was clean.go test ./pkg/virtual/apiexport/authorizer/passed. I did not run the linter separately. - e2e: not run. The PR notes it was not run either. The unit test covers the changed branch, and the e2e suite needs a full kcp environment that is not available here.
- Backport scope: only binding.go and its test differ. The PR body says the conflict was resolved against an older claim-matching block, and I confirmed the resulting block compiles and behaves as above.
|
LGTM label has been added. DetailsGit tree hash: 81984fb31076150a42534835c965174d5ed4a51a |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ntnn The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kcp-ci-bot
added
the
approved
|
/ok-to-test |
kcp-ci-bot
added
kind/bug
|
/retest |
|
/retest |
Cherry-pick of #4504 (eee9dfa) onto release-0.33. Fixes #4503.
The automatic cherry-pick failed with a conflict in
pkg/virtual/apiexport/authorizer/binding.go: the release branch has an older claim-matching block, so the wildcard-verb handling is applied there instead. The test cases from #4504 apply unchanged.Validation:
go build ./pkg/virtual/apiexport/...andgo test ./pkg/virtual/apiexport/authorizer/pass on this branch. e2e not run.🤖 Generated with Claude Code