Skip to content

Add system:cluster:<logical-cluster> group to effective users to enhance cross-workspace authz#3530

Merged
kcp-ci-bot merged 6 commits into
kcp-dev:mainfrom
ntnn:kcp3513
Oct 7, 2025
Merged

Add system:cluster:<logical-cluster> group to effective users to enhance cross-workspace authz#3530
kcp-ci-bot merged 6 commits into
kcp-dev:mainfrom
ntnn:kcp3513

Conversation

@ntnn

@ntnn ntnn commented Aug 12, 2025

Copy link
Copy Markdown
Member

Summary

Needs kcp-dev/kubernetes#176

This adds tests ensuring that global users (with scopes and with warrants), service accounts and per-workspace users can bind APIExports of another workspace when said workspace allows binding from the source workspace through the system:cluster:<logical-cluster> group, where <logical-cluster> is the name of the logical cluster backing the source workspace.
This is representative of other cross-workspace requests and should work for any cross-workspace authorization check.

Caveat:

Global users without scopes or warrants are not considered at the moment (though could be covered by simply adding the system:cluster:.. group to the request when creating an APIBinding - but that would only work for APIBinding/-Exports). They will still be bound by the usual authorization methods.

What Type of PR Is This?

/kind feature

Related Issue(s)

Fixes #3513

Release Notes

Users from other workspaces can be authorized by granting permission to the `system:cluster:<clusterid>` group
Authorization webhooks now get a payload with the target cluster in the `authorization.kcp.io/cluster-name` extra. The `authorization.kubernetes.io/cluster-name` extra is deprecated and will be removed in a future release

@kcp-ci-bot kcp-ci-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. 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 size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 12, 2025
Comment thread pkg/admission/apibinding/binding_permissions.go Outdated
Comment thread pkg/admission/apibinding/binding_permissions.go Outdated
@kcp-ci-bot kcp-ci-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 12, 2025
@kcp-ci-bot kcp-ci-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 27, 2025
@kcp-ci-bot kcp-ci-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 28, 2025
@kcp-ci-bot kcp-ci-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 8, 2025
@kcp-ci-bot 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 10, 2025
@ntnn ntnn requested a review from Copilot September 15, 2025 08:42

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 support for cross-workspace authorization by adding the system:cluster:<cluster> group to bind requests, allowing users from one workspace to access resources in another workspace when explicitly authorized. This is part of implementing cross-workspace request authorization through group membership.

  • Adds test coverage for various user types (OIDC, service accounts, scoped users, warranted users) binding APIExports across workspaces
  • Introduces a utility function for creating service accounts with tokens in test fixtures
  • Updates documentation to clarify how scoped users and warrants work with the new cluster group system

Reviewed Changes

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

Show a summary per file
File Description
test/e2e/fixtures/authfixtures/serviceAccount.go New utility function for creating service accounts with tokens for testing
test/e2e/authorizer/scopes_test.go Updates existing authorization tests with warrant user case and formatting improvements
test/e2e/apibinding/cross_workspace_auth_test.go Comprehensive test suite for cross-workspace APIBinding authorization scenarios
pkg/server/server.go Debug logging import addition (should be removed)
go.mod Removed Kubernetes dependency replacements and updated Go version
docs/content/concepts/authorization/authorizers.md Updated documentation to explain cluster group behavior for scoped users and warrants
Comments suppressed due to low confidence (1)

go.mod:10

  • Go version 1.24.0 does not exist. The latest stable Go version as of my knowledge cutoff is 1.23.x. This appears to be an invalid version number.
go 1.24.0

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread pkg/server/server.go Outdated
Comment thread pkg/server/server.go Outdated
Comment thread test/e2e/apibinding/cross_workspace_auth_test.go Outdated
Comment thread test/e2e/apibinding/cross_workspace_auth_test.go Outdated
Comment thread test/e2e/apibinding/cross_workspace_auth_test.go Outdated
Comment thread test/e2e/apibinding/cross_workspace_auth_test.go Outdated

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 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread test/e2e/apibinding/cross_workspace_auth_test.go Outdated
Comment thread test/e2e/apibinding/cross_workspace_auth_test.go Outdated
Comment thread docs/content/concepts/authorization/authorizers.md Outdated
@mjudeikis

Copy link
Copy Markdown
Contributor

/retest

@ntnn

ntnn commented Oct 6, 2025

Copy link
Copy Markdown
Member Author

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

ntnn and others added 5 commits October 6, 2025 16:09
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com>
Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com>
Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com>
Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com>
Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
@kcp-ci-bot kcp-ci-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 6, 2025
@ntnn

ntnn commented Oct 6, 2025

Copy link
Copy Markdown
Member Author

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

1 similar comment
@ntnn

ntnn commented Oct 6, 2025

Copy link
Copy Markdown
Member Author

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

@ntnn

ntnn commented Oct 6, 2025

Copy link
Copy Markdown
Member Author

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

1 similar comment
@ntnn

ntnn commented Oct 6, 2025

Copy link
Copy Markdown
Member Author

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

@ntnn

ntnn commented Oct 6, 2025

Copy link
Copy Markdown
Member Author

/test pull-kcp-test-e2e-multiple-runs
<.<

@ntnn ntnn requested a review from mjudeikis October 6, 2025 20:22
Co-authored-by: Nelo-T. Wallus <10514301+ntnn@users.noreply.github.com>

@mjudeikis mjudeikis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

/lgtm
/approve

@kcp-ci-bot kcp-ci-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 7, 2025
@kcp-ci-bot

Copy link
Copy Markdown
Contributor

[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:

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

Copy link
Copy Markdown
Contributor

LGTM label has been added.

DetailsGit tree hash: d06ef4ddc040a14bc1e9cdee7edaf38bec6ed74e

@kcp-ci-bot kcp-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 7, 2025
@ntnn

ntnn commented Oct 7, 2025

Copy link
Copy Markdown
Member Author

/test pull-kcp-test-e2e-shared
/test pull-kcp-test-e2e-sharded

@ntnn

ntnn commented Oct 7, 2025

Copy link
Copy Markdown
Member Author

/test pull-kcp-test-e2e-sharded

@kcp-ci-bot kcp-ci-bot merged commit 7888cc7 into kcp-dev:main Oct 7, 2025
14 checks passed
@ntnn ntnn deleted the kcp3513 branch October 7, 2025 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. 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/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feature: add logical cluster pseudo groups to cross-workspace authorization checks

4 participants