Skip to content

Add system:cluster:... groups to effective users#176

Merged
mjudeikis merged 3 commits into
kcp-dev:kcp-1.33.3-1from
ntnn:cluster-groups
Sep 29, 2025
Merged

Add system:cluster:... groups to effective users#176
mjudeikis merged 3 commits into
kcp-dev:kcp-1.33.3-1from
ntnn:cluster-groups

Conversation

@ntnn

@ntnn ntnn commented Sep 10, 2025

Copy link
Copy Markdown
Member

What type of PR is this?

/kind feature

What this PR does / why we need it:

Adds the extra authentication.kcp.io/scopes to service accounts similar to per-workspace auth users.
Adds the group system:cluster:... to effective users based off of authentication.kcp.io/scopes.

Needed for github.com/kcp-dev/kcp/pull/3530

Problems

Users that do not originate from per-ws auth and do not have a warrant or scope cannot get a system:cluster:....

Originally I had added the group explicitly for APIBinding requests and allowed the group through here. However that pattern would require adding the system:cluster: group wherever cross-ws requests can happen.

A potential solution could be what this PR proposes with building the groups based off of the scopes as well as allowing system:clluster: group through, adding those groups explicitly where necessary (e.g. in APIBinding) request.

Since per-ws auth does not allow setting system: groups coming from the auth provider that could be reasonably secure. Although it feels very hacky.

Alternatively we'd have to update EffectiveUsers/Groups to get the information on the source and target cluster so the correct info can be calculated more accurately.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?


Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:


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

Copy link
Copy Markdown

[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 deads2k for approval. For more information see the Kubernetes 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

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 system:cluster:... groups to effective users based on authentication.kcp.io/scopes, extending the existing warrant and scope functionality to include cluster-scoped group memberships for authorization purposes.

  • Adds ClusterScopeKey constant and sets cluster scope for service accounts
  • Introduces EffectiveScopes function to compute intersections of user scopes
  • Updates effective user logic to add system:cluster: groups based on scopes

Reviewed Changes

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

File Description
staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount/util.go Adds ClusterScopeKey constant and sets cluster scope in service account UserInfo
pkg/registry/rbac/validation/kcp.go Implements EffectiveScopes function and updates effective user logic to add cluster groups
pkg/registry/rbac/validation/kcp_test.go Updates tests to use constants and adds comprehensive test coverage for EffectiveScopes

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

Comment thread pkg/registry/rbac/validation/kcp_test.go Outdated
Comment thread pkg/registry/rbac/validation/kcp.go
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

@mjudeikis mjudeikis 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.

not with easy hand. This feels complex :D
/lgtm
/apprve

@mjudeikis mjudeikis merged commit 84c443c into kcp-dev:kcp-1.33.3-1 Sep 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants