Skip to content

Remove dedicated front-proxy client CA#233

Open
xrstf wants to merge 7 commits into
kcp-dev:mainfrom
xrstf:remove-fp-client-ca
Open

Remove dedicated front-proxy client CA#233
xrstf wants to merge 7 commits into
kcp-dev:mainfrom
xrstf:remove-fp-client-ca

Conversation

@xrstf
Copy link
Copy Markdown
Contributor

@xrstf xrstf commented May 8, 2026

Summary

When I originally designed the PKI for the kcp-operator, I thought having separate CAs for front-proxy and the shards made sense. However it turns out this is causing more havoc by making kubeconfigs just so useless: You can read an APIExportEndpointSlice through the front-proxy, but then cannot access any of the endpoints because the shards/virtual-workspaces use a different client CA.

Due to this, the kcp-operator already deploys a CA bundle for the client CA in the front-proxy, a bundle consisting of the root client CA and the front-proxy's client CA.

This PR goes one step further and simply removes the old front-proxy client CA entirely, but introducing a new clientCASecretRef field for FrontProxy objects, so admins can configure their
own client CA (or any other) here and have it still be included in the front-proxy. Since we have already always configured both client CAs, this is not losing any pre-existing flexibility, we're just reducing the minimum number of CAs that a front-proxy runs from 2 to 1.


I also refactored the reconcilerFunc a bit, making it stateless. I then also cleaned up the controller a bit and renamed the "merged CA" to "backendCABundle" (in the code only, no Kube object name changes), which IMHO describes its purpose much better. From the documentation saying "is used to validate API Server certs", I couldn't immediately tell which API Servers :D

The adminKubeconfigCertificateReconciler has been removed entirely, since its not mounted anywhere and never actually used, especially now that we only have one client CA.

What Type of PR Is This?

/kind cleanup
/kind api-change

Release Notes

TBD

@kcp-ci-bot kcp-ci-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API dco-signoff: yes Indicates the PR's author has signed the DCO. labels May 8, 2026
@kcp-ci-bot
Copy link
Copy Markdown
Contributor

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

@kcp-ci-bot kcp-ci-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 8, 2026
@xrstf xrstf marked this pull request as draft May 8, 2026 16:20
@kcp-ci-bot kcp-ci-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. 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 May 8, 2026
@xrstf
Copy link
Copy Markdown
Contributor Author

xrstf commented May 8, 2026

/test all

@xrstf xrstf changed the title Use root client CA in front-proxy, remove dedicated front-proxy client CA Remove dedicated front-proxy client CA May 8, 2026
@xrstf xrstf marked this pull request as ready for review May 8, 2026 18:23
@kcp-ci-bot kcp-ci-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 8, 2026
@ntnn
Copy link
Copy Markdown
Member

ntnn commented May 8, 2026

but introducing a new clientCASecretRef field for FrontProxy objects,

Shouldn't that be on the root shard and the front proxy and shards include that as well if its set? Right now the front-proxy does authn/z and the shards trust that, but users that connect to a shard or virtual workspace directly still get declined if they have the custom ca signed client cert.

@ntnn ntnn moved this to Reviewing in tbd May 8, 2026
@ntnn ntnn added this to tbd May 8, 2026
@xrstf
Copy link
Copy Markdown
Contributor Author

xrstf commented May 8, 2026

My thought was that you might want to have different Client CA Bundles for each FrontProxy. So making every FP automatically inherit everything from the RootShard only would kind of break that.

However I can see how having a custom CA in the FP only, which is then not part of the shards or VWs, is right where we started.

Tbh I originally added the ClientCA bundle to the FrontProxy API only as a means to keep backwards-compatibility, in order to get this merged I then sold it as a feature 😁

How sold are you on this PR also introducing ClientCABundles to (Root)Shards/VWs?

@ntnn
Copy link
Copy Markdown
Member

ntnn commented May 11, 2026

How sold are you on this PR also introducing ClientCABundles to (Root)Shards/VWs?

I think it'd be a good idea overall. We could drop the fp client CA, use the root client CA everywhere and downstream can add their custom ca using clientCABundles?
I wouldn't be opposed to fp/shards having clientCABundles as well as a property.

We already have a number of properties that could default to the root shard value but don't, or?
Then not default fp/shard to the root shard value would just be following what we do already.

xrstf added 7 commits May 19, 2026 16:57
On-behalf-of: @SAP christoph.mewes@sap.com
On-behalf-of: @SAP christoph.mewes@sap.com
On-behalf-of: @SAP christoph.mewes@sap.com
On-behalf-of: @SAP christoph.mewes@sap.com
On-behalf-of: @SAP christoph.mewes@sap.com
On-behalf-of: @SAP christoph.mewes@sap.com
@xrstf xrstf force-pushed the remove-fp-client-ca branch from 6cd3393 to 09a019f Compare May 19, 2026 14:57
@kcp-ci-bot kcp-ci-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 19, 2026
@xrstf
Copy link
Copy Markdown
Contributor Author

xrstf commented May 19, 2026

Okay, I've extended the API to include a ClientCABundle in the CommonShardSpec, and in VWs, and added inheritance, some tests and updated the docs. Granting access has never been easier!(tm).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants