Skip to content

fix: use the RootShard's cluster domain in the internal proxy server certificate - #304

Open
2mind wants to merge 1 commit into
kcp-dev:mainfrom
2mind:fix/proxy-cert-cluster-domain
Open

2mind wants to merge 1 commit into
kcp-dev:mainfrom
2mind:fix/proxy-cert-cluster-domain

Conversation

@2mind

@2mind 2mind commented Sep 28, 2026 •

Copy link
Copy Markdown

Summary

The internal proxy's server certificate was built with a hard-coded cluster domain:

dnsNames := []string{
	fpService,
	fmt.Sprintf("%s.%s", fpService, r.rootShard.Namespace),
	fmt.Sprintf("%s.%s.svc.cluster.local", fpService, r.rootShard.Namespace),
}

while everything that dials the proxy goes through resources.GetRootShardProxyBaseHost() → <service>.<namespace>.svc.<clusterDomain>. With spec.clusterDomain: cozy.local a certificate for the proxy looks like this, and the operator's own client cannot verify it:

["kcp-proxy","kcp-proxy.kcp-system","kcp-proxy.kcp-system.svc.cluster.local"]

The affected client is client.NewInternalKubeconfigClient() (via NewRootShardProxyClient), which the kubeconfig-rbac controller uses to create the RBAC for Kubeconfig.spec.authorization — so scoped permissions were silently not granted for every Kubeconfig whose target is a FrontProxy.

Change

  • resources.GetRootShardClusterDomain() — new helper for the "cluster domain or default" lookup that was open-coded in several places; GetRootShardBaseHost() and GetRootShardProxyBaseHost() now use it too.
  • frontproxy.serverDNSNames() — the DNS name list is factored out of serverCertificateReconciler() and its fully qualified Service name is derived from that cluster domain instead of the hard-coded cluster.local.
  • Unit test covering the root-shard proxy and front-proxy cases, with the default and a custom cluster domain.

The SAN list keeps the short forms, and for front-proxies the external hostnames are added exactly as before; only the fully qualified name changes.

Verification

  • go build ./..., go vet ./internal/..., go test ./internal/... ./pkg/... — all pass.
  • With spec.clusterDomain: cozy.local the generated Certificate now carries kcp-proxy.kcp-system.svc.cozy.local, which is exactly the entry we had to add by hand through spec.proxy.certificateTemplates in our installation (kcp-operator v0.9.0, chart 0.7.10), where this was the reason the kubeconfig-rbac bindings never appeared — Kubeconfig.status.authorization.provisionedCluster stayed empty.

What Type of PR Is This?

/kind bug

Related Issue(s)

Fixes #303

Release Notes

Use the RootShard's cluster domain in the internal proxy server certificate instead of a hard-coded cluster.local

The server certificate of the root shard's internal proxy was generated with a
hard-coded "svc.cluster.local" SAN, while the operator addresses that proxy
through GetRootShardProxyBaseURL(), i.e. "<name>-proxy.<namespace>.svc.<clusterDomain>".

With a non-default spec.clusterDomain the operator's own client
(NewInternalKubeconfigClient -> NewRootShardProxyClient, used by the
kubeconfig-rbac controller) therefore failed the TLS handshake, so the RBAC for
Kubeconfig.spec.authorization was never created and scoped permissions were
silently not granted.

Derive the SAN from the RootShard's cluster domain, defaulting to cluster.local,
and factor the domain lookup into resources.GetRootShardClusterDomain(), which
GetRootShardBaseHost() and GetRootShardProxyBaseHost() use as well.

Fixes kcp-dev#303

Signed-off-by: Nikita Aboltin <aboltin.nikita@rwb.ru>
@kcp-ci-bot kcp-ci-bot added dco-signoff: yes Indicates the PR's author has signed the DCO. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Sep 28, 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 mjudeikis 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 do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 28, 2026
@kcp-ci-bot

Copy link
Copy Markdown
Contributor

Hi @2mind. Thanks for your PR.

I'm waiting for a kcp-dev member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@kcp-ci-bot kcp-ci-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Sep 28, 2026
@2mind

2mind commented Sep 28, 2026

Copy link
Copy Markdown
Author

/kind bug

@kcp-ci-bot kcp-ci-bot added kind/bug Categorizes issue or PR as related to a bug. and removed do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Sep 28, 2026
@2mind

2mind commented Oct 26, 2026

Copy link
Copy Markdown
Author

Friendly ping for review (fixes #303). We currently work around the missing internal-proxy SAN by setting spec.proxy.certificateTemplates by hand; happy to drop that once this lands and is released.

This branch has not been deployed

No deployments
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/bug Categorizes issue or PR as related to a bug. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: internal proxy server certificate ignores RootShard.spec.clusterDomain

2 participants