pkg/admission: block WS creation if user cannot bind to default exports by neolit123 · Pull Request #4311 · kcp-dev/kcp

neolit123 · 2026-08-04T16:21:17Z

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.

neolit123 · 2026-08-04T16:22:46Z

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

Copilot

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.

Copilot

Pull request overview

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

neolit123 · 2026-08-04T17:38:54Z


=== 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 · 2026-08-04T18:02:44Z

=== 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 · 2026-08-07T09:01:01Z

/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 · 2026-08-07T09:24:30Z

CI errors seem related

EDIT: NVM, it passed.

neolit123 · 2026-08-07T11:56:05Z

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

xrstf · 2026-08-27T14:21:44Z

+		notPermitted := errors.New("no permission to bind one or more of the default API bindings")
+		if showExportPathInErrors {
+			notPermitted = fmt.Errorf("no permission to bind to export %s",
+				logicalcluster.NewPath(ref.Path).Join(ref.Export).String())
+		}
+


I don't know how to argue it, but I have never seen a boolean parameter to toggle the error formatting in a function, and so I was wondering if this should simply be a custom error type that contains, as one field, the workspace Path. Then the caller can decide how they want to format it.

yep, seems better with a custom error type.
updated.

neolit123 · 2026-09-09T12:06:11Z

+// DefaultAPIBindingAccessError indicates the user lacks the 'bind' verb on one of a
+// WorkspaceType's default APIExports.
+type DefaultAPIBindingAccessError struct {
+	// ExportPath is the fully-qualified export path.
+	ExportPath string
+}
+
+func (e *DefaultAPIBindingAccessError) Error() string {
+	if e.ExportPath != "" {
+		return fmt.Sprintf("no permission to bind to export %s", e.ExportPath)
+	}
+	return "no permission to bind one or more of the default API bindings"
+}


i added this new error type
DefaultAPIBindingAccessError

neolit123 · 2026-09-09T12:07:14Z

+			if err := apibindingadmission.CheckDefaultAPIBindingsAccess(ctx, a.GetUserInfo(), logicalcluster.From(alias), alias.Spec.DefaultAPIBindings, o.getLogicalCluster, newAuthorizer); err != nil {
+				// Do not leak the export path when creating a workspace.
+				if notPermitted, ok := errors.AsType[*apibindingadmission.DefaultAPIBindingAccessError](err); ok {
+					notPermitted.ExportPath = ""


here we don't want to leak the WS path to the user as they might not be the owner of the WST

mjudeikis · 2026-09-14T07:51:43Z

While this might ok syntax wise, can we have e2e test (or do we have one already?) for this. As its kinda hard to know if this does what it tells it does?

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(). Include unit and e2e tests. Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>

neolit123 · 2026-09-14T10:30:17Z

=== Failed
=== FAIL: test/e2e/workspace TestInactiveLogicalClusterBlocksRequests (49.40s)
inactive_test.go:83: Saving test artifacts for test "TestInactiveLogicalClusterBlocksRequests" under "/logs/artifacts/TestInactiveLogicalClusterBlocksRequests/3987638019".
inactive_test.go:83: Starting kcp servers...
fixture.go:316: running: kcp start --root-directory /tmp/TestInactiveLogicalClusterBlocksRequests2251264606/002/kcp/main --secure-port=43353 --embedded-etcd-client-port=34793 --embedded-etcd-peer-port=37159 --embedded-etcd-wal-size-bytes=5000 --kubeconfig-path=/tmp/TestInactiveLogicalClusterBlocksRequests2251264606/002/kcp/main/admin.kubeconfig --audit-log-path /logs/artifacts/TestInactiveLogicalClusterBlocksRequests/3987638019/test-inactive-logical-cluster-blocks-requests/kcp/main/kcp.audit --v=4 --feature-gates=DRAExtendedResource=false,SizeBasedListCostEstimate=false,UnknownVersionInteroperabilityProxy=false,WatchCacheInitializationPostStartHook=false --bind-address=127.0.0.1 --audit-policy-file /tmp/TestInactiveLogicalClusterBlocksRequests2251264606/001/audit-policy.yaml

/retest

neolit123 · 2026-09-14T10:49:22Z

same flake
/retest

neolit123 · 2026-09-14T11:45:40Z

=== Failed
=== FAIL: test/e2e/virtual/apiexport TestAPIExportAuthDelegation/LocalSubjectAccessReview_is_served_on_the_APIExport_virtual_workspace,_scoped_to_the_consumer (0.01s)
authdelegation_test.go:164:
Error Trace: /home/prow/go/src/github.com/kcp-dev/kcp/test/e2e/virtual/apiexport/authdelegation_test.go:164
Error: Received unexpected error:
the server could not find the requested resource (post localsubjectaccessreviews.authorization.k8s.io)
Test: TestAPIExportAuthDelegation/LocalSubjectAccessReview_is_served_on_the_APIExport_virtual_workspace,_scoped_to_the_consumer
Messages: LocalSubjectAccessReview should be served on the APIExport virtual workspace
=== FAIL: test/e2e/virtual/apiexport TestAPIExportAuthDelegation/SubjectAccessReview_is_served_on_the_APIExport_virtual_workspace,_scoped_to_the_consumer (0.01s)
authdelegation_test.go:140:
Error Trace: /home/prow/go/src/github.com/kcp-dev/kcp/test/e2e/virtual/apiexport/authdelegation_test.go:140
Error: Received unexpected error:
the server could not find the requested resource (post subjectaccessreviews.authorization.k8s.io)
Test: TestAPIExportAuthDelegation/SubjectAccessReview_is_served_on_the_APIExport_virtual_workspace,_scoped_to_the_consumer
Messages: SubjectAccessReview should be served on the APIExport virtual workspace
=== FAIL: test/e2e/virtual/apiexport TestAPIExportAuthDelegation (18.50s)

/retest

mjudeikis

/lgtm
/approve

kcp-ci-bot · 2026-09-15T10:26:47Z

LGTM label has been added.

Details

Git tree hash: e57b559aa7c3ce63f52393f94aeb14f207f421cf

kcp-ci-bot · 2026-09-15T10:26:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mjudeikis

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [mjudeikis]
~~pkg/admission/OWNERS~~ [mjudeikis]

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

neolit123 · 2026-09-15T11:32:12Z

/retest

neolit123 requested a lite review from Copilot August 4, 2026 16:35

Copilot started reviewing on behalf of neolit123 August 4, 2026 16:40 View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

Comment thread pkg/admission/workspacetypeexists/admission.go

Comment thread pkg/admission/apibinding/binding_permissions.go Outdated

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 requested a lite review from Copilot August 4, 2026 17:02

Copilot started reviewing on behalf of neolit123 August 4, 2026 17:07 View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

xrstf reviewed Aug 27, 2026

View reviewed changes

neolit123 force-pushed the 4299-block-users-without-bind-verb-on-default-bindings-for-wst branch from 6d38bea to 449161e Compare September 9, 2026 12:04

neolit123 commented Sep 9, 2026

View reviewed changes

neolit123 force-pushed the 4299-block-users-without-bind-verb-on-default-bindings-for-wst branch from 449161e to 828b75a Compare September 14, 2026 10:10

kcp-ci-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 14, 2026

mjudeikis approved these changes Sep 15, 2026

View reviewed changes

kcp-ci-bot assigned mjudeikis Sep 15, 2026

kcp-ci-bot added the lgtm Indicates that a PR is ready to be merged. label Sep 15, 2026

kcp-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 15, 2026

kcp-ci-bot merged commit f0df3cf into kcp-dev:main Sep 15, 2026
17 checks passed

Conversation

neolit123 commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What Type of PR Is This?

Related Issue(s)

Release Notes

Uh oh!

neolit123 commented Aug 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

neolit123 commented Aug 4, 2026

Uh oh!

neolit123 commented Aug 4, 2026

Uh oh!

neolit123 commented Aug 7, 2026

Uh oh!

neolit123 commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

neolit123 commented Aug 7, 2026

Uh oh!

xrstf Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

neolit123 Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

neolit123 Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

neolit123 Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

mjudeikis commented Sep 14, 2026

Uh oh!

neolit123 commented Sep 14, 2026

Uh oh!

neolit123 commented Sep 14, 2026

Uh oh!

neolit123 commented Sep 14, 2026

Uh oh!

mjudeikis left a comment

Choose a reason for hiding this comment

Uh oh!

kcp-ci-bot commented Sep 15, 2026

Uh oh!

kcp-ci-bot commented Sep 15, 2026

Uh oh!

neolit123 commented Sep 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

neolit123 commented Aug 4, 2026 •

edited

Loading

neolit123 commented Aug 7, 2026 •

edited

Loading