Skip to content

pkg/admission: block WS creation if user cannot bind to default exports - #4311

Open
neolit123 wants to merge 1 commit into
kcp-dev:mainfrom
neolit123:4299-block-users-without-bind-verb-on-default-bindings-for-wst
Open

pkg/admission: block WS creation if user cannot bind to default exports#4311
neolit123 wants to merge 1 commit into
kcp-dev:mainfrom
neolit123:4299-block-users-without-bind-verb-on-default-bindings-for-wst

Conversation

@neolit123

@neolit123 neolit123 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Block the creation of Workspace based on a custom WorkspaceType, if the user cannot bind to the APIExports requested on the WorkspaceType's defaultAPIBindings.

Extract some common logic from pkg/admission/workspacetype/admission.go in a function apibindingadmission.CheckDefaultAPIBindingsAccess().

i have tested the creation block with a script locally and it works.

What Type of PR Is This?

/kind bug

Related Issue(s)

Fixes #4299
(second attempt)

Release Notes

If the user cannot bind to the default API Bindings present on a `WorkspaceType`, prevent the user from being able to create a `Workspace` object of that type.

@kcp-ci-bot kcp-ci-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has signed the DCO. kind/bug Categorizes issue or PR as related to a bug. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API labels Aug 4, 2026
@kcp-ci-bot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign xrstf for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kcp-ci-bot kcp-ci-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 4, 2026
@neolit123

Copy link
Copy Markdown
Contributor Author

hmm, unclear why the api/change label was added.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds an admission-time permission gate to prevent creating a Workspace from a WorkspaceType when the requesting user cannot bind to the APIExports referenced by the type’s spec.defaultAPIBindings, and factors the shared authorization logic into a reusable helper in pkg/admission/apibinding.

Changes:

  • Add a default-APIBindings access check to the WorkspaceTypeExists admission plugin to block workspace creation early when bind is not permitted.
  • Extract common “default APIBindings bind permission” logic into apibindingadmission.CheckDefaultAPIBindingsAccess() and reuse it from the WorkspaceType admission path.
  • Add unit tests covering allow/deny scenarios, including inherited bindings through extended types.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
pkg/admission/workspacetypeexists/admission.go Adds defaultAPIBindings access enforcement during workspace create; wires LogicalCluster lookup helpers/indexers.
pkg/admission/workspacetypeexists/admission_test.go Adds tests to ensure workspace creation is rejected when bind on default APIExports is missing.
pkg/admission/workspacetype/admission.go Refactors existing WorkspaceType defaultAPIBindings permission logic to use the shared helper.
pkg/admission/apibinding/binding_permissions.go Introduces CheckDefaultAPIBindingsAccess helper for centralized default binding authorization checks.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/admission/workspacetypeexists/admission.go
Comment thread pkg/admission/apibinding/binding_permissions.go Outdated
Block the creation of Workspace based on a custom WorkspaceType,
if the user cannot bind to the APIExports requested on the WorkspaceType's
defaultAPIBindings.

Extract some common logic from pkg/admission/workspacetype/admission.go
in a function apibindingadmission.CheckDefaultAPIBindingsAccess().

Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
@neolit123
neolit123 force-pushed the 4299-block-users-without-bind-verb-on-default-bindings-for-wst branch from 6594ca5 to 6d38bea Compare August 4, 2026 17:01
@neolit123
neolit123 requested a lite review from Copilot August 4, 2026 17:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

@neolit123

Copy link
Copy Markdown
Contributor Author

=== Failed
=== FAIL: test/e2e/workspace TestInactiveLogicalClusterBlocksRequests (59.69s)
    inactive_test.go:83: Saving test artifacts for test "TestInactiveLogicalClusterBlocksRequests" under "/logs/artifacts/TestInactiveLogicalClusterBlocksRequests/302809948".
    inactive_test.go:83: Starting kcp servers...
    fixture.go:316: running: kcp start --root-directory /tmp/TestInactiveLogicalClusterBlocksRequests2709822804/002/kcp/main --secure-port=45357 --embedded-etcd-client-port=46537 --embedded-etcd-peer-port=40707 --embedded-etcd-wal-size-bytes=5000 --kubeconfig-path=/tmp/TestInactiveLogicalClusterBlocksRequests2709822804/002/kcp/main/admin.kubeconfig --audit-log-path /logs/artifacts/TestInactiveLogicalClusterBlocksRequests/302809948/test-inactive-logical-cluster-blocks-requests/kcp/main/kcp.audit --v=4 --feature-gates=DRAExtendedResource=false,SizeBasedListCostEstimate=false,UnknownVersionInteroperabilityProxy

https://public-prow.kcp.k8c.io/view/s3/prow-public-data/pr-logs/pull/kcp-dev_kcp/4311/pull-kcp-test-e2e-shared/2084686180107948032

/retest

@neolit123

Copy link
Copy Markdown
Contributor Author
=== Failed
=== FAIL: test/e2e/quota TestKubeQuotaCRDChurn (83.82s)
    quota_test.go:750: Saving test artifacts for test "TestKubeQuotaCRDChurn" under "/logs/artifacts/TestKubeQuotaCRDChurn/2761795242".
I0804 17:42:12.593926   75210 crypto.go:609] "generating new CA for cert" certName="kcp-client-ca" certFile="/tmp/TestKubeQuotaCRDChurn3248012119

/retest

@neolit123

Copy link
Copy Markdown
Contributor Author

/retest

=== Failed
=== FAIL: test/e2e/workspacetype TestWorkspaceTypes/create_a_workspace_without_an_explicit_type,_get_default_type (1.05s)
    controller_test.go:339: Created root:organization workspace root:e2e-workspace-gc2p8 as /clusters/2o3vshf2jweoqzbm on shard "root"
    controller_test.go:345: Create a workspace without explicit type
    controller_test.go:345: 
        	Error Trace:	/home/prow/go/src/github.com/kcp-dev/kcp/test/e2e/workspacetype/controller_test.go:69
        	            				/home/prow/go/src/github.com/kcp-dev/kcp/test/e2e/workspacetype/controller_test.go:345
        	Error:      	Received unexpected error:
        	            	the server could not find the requested resource (post workspaces.tenancy.kcp.io)
        	Test:       	TestWorkspaceTypes/create_a_workspace_without_an_explicit_type,_get_default_type
        	Messages:   	failed to create workspace
=== FAIL: test/e2e/workspacetype TestWorkspaceTypes (24.90s)
    controller_test.go:330: Saving test artifacts for test "TestWorkspaceTypes" under "/logs/artifacts/TestWorkspaceTypes/4143773557".
I0804 18:37:15.205684   78372 crypto.go:609] "generating new CA for cert" certName="kcp-client-ca" certFile="/tmp/TestWorkspaceTypes2387278109/003/client-ca.crt" keyFile="/tmp/TestWorkspaceTypes2387278109/003/client-ca.key"

@neolit123

neolit123 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

CI errors seem related

EDIT: NVM, it passed.

@neolit123

Copy link
Copy Markdown
Contributor Author

/test pull-kcp-test-e2e-multiple-runs
/test pull-kcp-test-e2e-shared

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dco-signoff: yes Indicates the PR's author has signed the DCO. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/bug Categorizes issue or PR as related to a bug. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: slow typed workspace creation if user lacks bind permission on the defaultAPIBindings

3 participants