feat: improve SSAR and SSRR performance and usability#1069
Open
lukasboettcher wants to merge 1 commit into
Open
feat: improve SSAR and SSRR performance and usability#1069lukasboettcher wants to merge 1 commit into
lukasboettcher wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves performance and fidelity of Kubernetes SelfSubjectAccessReview (SSAR) / SelfSubjectRulesReview (SSRR) handling by forwarding self-review requests to the upstream API server using the caller’s bearer token when available, then appending capsule-proxy-granted permissions (instead of impersonating each group).
Changes:
- Add bearer-token detection and, for SSAR/SSRR paths, proxy upstream using the caller’s bearer token to avoid per-group impersonation overhead.
- Adjust authorization mutation logic to (a) avoid panics on nil attributes, (b) use lowercase RBAC verbs where appropriate, and (c) merge SSRR rules instead of overwriting upstream results.
- Add unit tests covering SSRR merge behavior and SSAR namespace list behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/webserver/webserver.go | Forward SSAR/SSRR upstream directly when bearer token is present; make decoding/mutation flow more defensive. |
| internal/authorization/middleware.go | Refine SSAR/SSRR mutation: safer attribute handling, merge behavior for SSRR, and clearer “granted” status updates. |
| internal/authorization/middleware_test.go | Add tests validating SSRR merge semantics and SSAR grant behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
d2b0e54 to
c9fa5a1
Compare
c9fa5a1 to
f84c093
Compare
f84c093 to
efcdd8c
Compare
Signed-off-by: Lukas Boettcher <1340215+lukasboettcher@users.noreply.github.com>
efcdd8c to
18752f9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR lets
authorizationMiddlewarerequests pass through to the upstream apiserver if bearer token auth is used. The SSAR response gets modified to include cluster-wide permissions for namespaced resources.Additionally this PR lets non tenant owners get namespace list permissions from GlobalProxySettings.
This has a few advantages:
kubectl get pods -AReasoning: we use AKS with capsule/-proxy. In AKS clusters it is common for users to have more than 300 groups assigned in Entra ID for large tenants. If we let capsule-proxy impersonate each group of the user's token, the upstream apiserver needs to perform the permission review for each impersonated group individually.
This is extremely slow and causes timeouts in lots of k8s clients.
Could be related to #989
Example for a user with ~300 groups:
before
after
config: