-
Notifications
You must be signed in to change notification settings - Fork 366
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
🐛 Decouple internal and external logical-cluster-admin access #2882
🐛 Decouple internal and external logical-cluster-admin access #2882
Conversation
/hold This is WIP until I fix the e2e tests - currently there is an assumption that |
/cc @sttts |
d167291
to
98c5407
Compare
/retitle 🐛 Decouple internal and external logical-cluster-admin access |
/hold cancel |
98c5407
to
e14c629
Compare
/hold Still figuring out authz for the new external-logical-cluster-admin group |
e14c629
to
a32b89e
Compare
a32b89e
to
ddf4f7f
Compare
/hold cancel @sttts @s-urbaniak this is ready for another review pass when you get a moment, thanks! |
3cc8c8c
to
5c0128c
Compare
5c0128c
to
be119dc
Compare
/test e2e-sharded Looks like #2865 |
be119dc
to
a5bfb36
Compare
/lgtm |
Requires kcp-dev/kcp#2882 Fixes: kcp-dev#32
@sttts this is ready for another review pass when you get time please - it's blocker for rebasing deployments based on the helm templates |
a5bfb36
to
e129761
Compare
The current logical-cluster-admin config is used for both internal shard-direct access, and also for access via the front-proxy. This won't work when the CA certs for the shard and front-proxy are different, so we add a new flag to specify an external config that enables access via the front-proxy - the existing config should be configured to enable access direct to the shards. This is derived from 339a071 which was reverted in 3854f1b Co-Authored-By: Dr. Stefan Schimanski <stefan.schimanski@gmail.com>
The group evaluation is already done prior to the switch, returning early if either group is found, so this test for non-membership is redundant.
e129761
to
36aa88c
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sttts 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 |
Requires kcp-dev/kcp#2882 Fixes: kcp-dev#32
Requires kcp-dev/kcp#2882 Fixes: kcp-dev#32
Summary
Currently the logical cluster admin kubeconfig is used to access both the external front-proxy endpoint, and also the shard directly via the baseURL - this won't work in the case where different CA certs are used for shard and front-proxy (such as in the helm-charts)
This only works in CI because we use the same serving-ca.crt for both shard and front-proxy, (in a future PR I will propose a change which better reflects the likely scenario in production deployments).
Fixes #2872