Let front-proxy verify against non-fp client CA by ntnn · Pull Request #178 · kcp-dev/kcp-operator

ntnn · 2026-03-11T23:54:00Z

Summary

This PR adds another reconciler to create a merged front-proxy client CA and client CA secret.
This is then mounted for both the internal and external front proxies.
The internal doesn't need it, I just saw no reason to make a distinction there.

As noted in #164 the CA structure is a problem.

We are currently having a very hard time resolving a number of issues that almost resembles a gordian knot, partially because we cannot change architecture that must be changed because permissions break down, we cannot change permissions because what would enable changing permissions only exists on paper and any workaround is either very hacky or a lot of extra work with little to no gain and/or brittle at best.

This change simply allows the front proxy to also validate from the shard client CA. That means kubeconfigs created against the root shard can also talk to the front proxy.

It is a minimal bandaid for the problem until the CA layout can be changed later down the line.

I did not consider making shards accept the front proxy CA - the shards have no inherent knowledge of the front proxy and it should stay that way.

What Type of PR Is This?

/kind bug

Related Issue(s)

Fixes #

Release Notes

Front proxy also accepts client certificates signed by the shard CA

Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>

…undle Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>

Copilot

Pull request overview

This PR creates a merged client CA secret that concatenates the FrontProxyClientCA and the shard ClientCA, and mounts it for both internal and external front proxies. This allows the front proxy to accept client certificates signed by either CA, enabling kubeconfigs created against the root shard to also authenticate through the front proxy.

Changes:

New mergedClientCASecretReconciler that fetches both CA secrets, concatenates their tls.crt data, and stores the result in a new merged secret
Unified deployment volume mount and --client-ca-file argument for both internal (rootshard proxy) and external front proxy modes
Updated tests with the required CA secret fixtures

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
internal/resources/frontproxy/ca_bundle.go	New merged client CA secret reconciler and name helper
internal/resources/frontproxy/deployment.go	Unified client CA mount path and CLI arg for both proxy modes
internal/resources/frontproxy/reconciler.go	Register the new merged client CA reconciler
internal/resources/resources.go	New `GetMergedClientCAName` helper
internal/resources/frontproxy/deployment_test.go	Updated expected volume mount name
internal/controller/frontproxy/controller_test.go	Added CA secret fixtures for fake client
internal/controller/rootshard/controller_test.go	Added CA secret fixtures for fake client

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-11T23:56:18Z

internal/resources/frontproxy/ca_bundle.go

+		return fmt.Sprintf("%s-merged-client-ca", r.frontProxy.Name)
+	}
+	return fmt.Sprintf("%s-proxy-merged-client-ca", r.rootShard.Name)


The mergedClientCASecretName() method duplicates the naming logic from resources.GetMergedClientCAName() instead of calling it. For the r.frontProxy != nil branch they happen to produce the same result, but for the rootShard proxy branch (r.frontProxy == nil), mergedClientCASecretName() returns "{rootShard.Name}-proxy-merged-client-ca" which is inconsistent with GetMergedClientCAName(). This means GetMergedClientCAName cannot be used for the rootShard proxy case, and the two functions could silently diverge.

Consider having mergedClientCASecretName() call resources.GetMergedClientCAName() with the appropriate owner name, or remove GetMergedClientCAName and keep the logic in one place.

Suggested change

return fmt.Sprintf("%s-merged-client-ca", r.frontProxy.Name)

}

return fmt.Sprintf("%s-proxy-merged-client-ca", r.rootShard.Name)

return resources.GetMergedClientCAName(r.frontProxy.Name)

}

// For the rootShard proxy case, the owner name includes the "-proxy" suffix

// so that the resulting secret name remains "<rootShard.Name>-proxy-merged-client-ca".

return resources.GetMergedClientCAName(fmt.Sprintf("%s-proxy", r.rootShard.Name))

True but I think fine for now. We'll delete most of this anyhow when fixing the CA layout.

mjudeikis-bot · 2026-03-12T07:53:13Z

🐷 Certificate Trust Analysis Summary

What This PR Does

Creates a merged secret concatenating FrontProxyClientCA + ClientCA, mounted as --client-ca-file for both internal and external front-proxies. This allows the front-proxy to accept client certs signed by either CA.

Does It Solve the Problem?

Yes, for the immediate pain point. Kubeconfigs signed by the shard's ClientCA will now be accepted by the front-proxy.

Kubeconfig Resource Interaction

The Kubeconfig CRD already handles the common case correctly:

target.frontProxyRef → cert signed by FrontProxyClientCA, URL = front-proxy external URL ✅ (works today)
target.rootShardRef → cert signed by ClientCA, URL = root shard base URL
target.shardRef → cert signed by ClientCA, URL = shard base URL

PR #178 enables the case where someone takes a shard-targeting kubeconfig and wants to reuse it against the front-proxy (auth now works, but URL still needs manual swap).

Risks to Watch

Risk	Severity	Notes
Security boundary blur	Medium	Front-proxy now trusts all `ClientCA`-signed certs. Acceptable since FP just proxies to shards anyway.
CA rotation / staleness	Low	If either CA is rotated, the merged secret must be re-reconciled. Confirm CA secret changes trigger re-reconciliation (no explicit watch on source secrets).
Bootstrap ordering	Low	If either CA secret doesn't exist yet during initial setup, the reconciler will fail and retry, potentially delaying front-proxy readiness.
No PEM validation	Low	Certs are `bytes.Join`'d with a newline — no PEM parsing/validation. Malformed cert data would silently propagate.

Alternative Considered: Both CAs in Both Directions

Also discussed: mounting both CAs into shards too (so shards trust FrontProxyClientCA). Comparison:

Scenario	PR #178 (both CAs in FP only)	Both CAs in shards AND FP
FP Kubeconfig → front-proxy	✅ already works	✅ works
FP Kubeconfig → shard directly	❌ shard rejects FP CA cert	✅ would work
Shard Kubeconfig → shard	✅ already works	✅ works
Shard Kubeconfig → front-proxy	✅ auth fixed, URL manual	✅ same

Adding FrontProxyClientCA to shards would make a FrontProxy-targeting Kubeconfig portable to direct shard access too. However, the PR explicitly preserves the architectural boundary that shards have no knowledge of the front-proxy.

Recommendation

PR #178 is a sound minimal bandaid. The "both directions" approach is cleaner for UX but crosses the architectural boundary — which may be acceptable since issue #164 proposes a single shared client CA long-term anyway. Both are bandaids until that redesign.

Analysis by 🐷 via Slack thread discussion with @mjudeikis and @ntnn

mjudeikis · 2026-03-12T08:00:55Z

/lgtm
/approve

kcp-ci-bot · 2026-03-12T08:01:02Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mjudeikis

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [mjudeikis]

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 · 2026-03-12T08:01:02Z

LGTM label has been added.

Details

Git tree hash: bff0f1e6a58ce705ce7e5321ff5c1881364074a4

ntnn added 2 commits March 12, 2026 00:46

Add reconciler for a merged front proxy client CA + client CA bundle

d5f4926

Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>

Deploy both external and internal front proxy with merged client CA b…

32ae84f

…undle Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>

ntnn requested review from Copilot, mjudeikis and xrstf March 11, 2026 23:54

ntnn self-assigned this Mar 11, 2026

Copilot started reviewing on behalf of ntnn March 11, 2026 23:54 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

ntnn mentioned this pull request Mar 12, 2026

Add kcp-operator kubeconfig platform-mesh/helm-charts#1344

Merged

kcp-ci-bot assigned mjudeikis Mar 12, 2026

kcp-ci-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 12, 2026

kcp-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 12, 2026

kcp-ci-bot merged commit 5d0a511 into kcp-dev:main Mar 12, 2026
16 checks passed

ntnn deleted the front-proxy-clien-ca branch March 12, 2026 09:04

ntnn added this to tbd Mar 13, 2026

github-project-automation bot moved this to Done in tbd Mar 13, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Let front-proxy verify against non-fp client CA#178

Let front-proxy verify against non-fp client CA#178
kcp-ci-bot merged 2 commits intokcp-dev:mainfrom
ntnn:front-proxy-clien-ca

ntnn commented Mar 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

ntnn Mar 12, 2026

Uh oh!

mjudeikis-bot commented Mar 12, 2026

Uh oh!

mjudeikis commented Mar 12, 2026

Uh oh!

kcp-ci-bot commented Mar 12, 2026

Uh oh!

kcp-ci-bot commented Mar 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

-		return fmt.Sprintf("%s-merged-client-ca", r.frontProxy.Name)
-	}
-	return fmt.Sprintf("%s-proxy-merged-client-ca", r.rootShard.Name)
+		return resources.GetMergedClientCAName(r.frontProxy.Name)
+	}
+	// For the rootShard proxy case, the owner name includes the "-proxy" suffix
+	// so that the resulting secret name remains "<rootShard.Name>-proxy-merged-client-ca".
+	return resources.GetMergedClientCAName(fmt.Sprintf("%s-proxy", r.rootShard.Name))

Conversation

ntnn commented Mar 11, 2026

Summary

What Type of PR Is This?

Related Issue(s)

Release Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

ntnn Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

mjudeikis-bot commented Mar 12, 2026

🐷 Certificate Trust Analysis Summary

What This PR Does

Does It Solve the Problem?

Kubeconfig Resource Interaction

Risks to Watch

Alternative Considered: Both CAs in Both Directions

Recommendation

Uh oh!

mjudeikis commented Mar 12, 2026

Uh oh!

kcp-ci-bot commented Mar 12, 2026

Uh oh!

kcp-ci-bot commented Mar 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants