-
Notifications
You must be signed in to change notification settings - Fork 6
Fix authentication in sharded setup. #75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
b783c29
to
5ea69d1
Compare
b20b2a3
to
3b6022f
Compare
There was a problem hiding this comment.
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 fixes authentication issues in sharded KCP setups by implementing proper ServiceAccount authentication configuration. The changes enable FrontProxy and shards to share ServiceAccount certificates and configure OIDC token verification across all components.
Key changes:
- Adds ServiceAccount authentication configuration to AuthSpec for FrontProxy, RootShard, and Shard resources
- Implements automatic mounting of all shard ServiceAccount certificates to FrontProxy when enabled
- Updates RootShard controller to track child shards in status for certificate distribution
Reviewed Changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 5 comments.
Show a summary per file
File | Description |
---|---|
sdk/apis/operator/v1alpha1/frontproxy_types.go | Adds ServiceAccountAuthentication struct and field to AuthSpec |
sdk/apis/operator/v1alpha1/shard_types.go | Adds Auth field to CommonShardSpec for shard authentication |
sdk/apis/operator/v1alpha1/rootshard_types.go | Adds ShardReference tracking to RootShardStatus |
internal/resources/utils/authentication.go | Implements ServiceAccount certificate mounting logic |
internal/resources/frontproxy/deployment.go | Updates to use new FrontProxy-specific auth configuration |
internal/controller/rootshard_controller.go | Adds shard tracking and watching logic |
config/samples/ | Sample configurations demonstrating new authentication features |
69575e3
to
31c976c
Compare
31c976c
to
2507e8b
Compare
80dfcb5
to
11ce278
Compare
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor language/typo nit. Don't forget to re-run codegen afterwards.
On-behalf-of: SAP <[email protected]> Signed-off-by: Mangirdas Judeikis <[email protected]>
11ce278
to
938a027
Compare
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
LGTM label has been added. Git tree hash: 2b1ee69a5f5d3ec0ed376168deb0f5e54de7272d
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: embik The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Summary
What Type of PR Is This?
Kubernetes only checks on the provided keys to SA validity. Not chain.
FrontProxy without flags denies SA's as "not enabled"
This boils down to "every shard & frontproxy must use same SA certs"
In addition
SubjectAccessReview
calls in delegated authentication mode are passed down to the shards. So if OIDC is used, shards need to be able to verify the tokens.FrontProxy with enabled SA authentication:
/kind bug
Related Issue(s)
Fixes #
Release Notes