feat: Add Public API Server Address Support by rossigee · Pull Request #986 · clastix/kamaji

rossigee · 2025-10-09T00:45:06Z

Add Public API Server Address Support

Overview

Adds an optional publicAPIServerAddress field to ServiceSpec, allowing a DNS hostname to be advertised externally instead of a LoadBalancer IP. This lets the advertised endpoint match a certificate SAN and avoids x509 errors for external clients.

Related: #1110 (split management/tenant addressing — this PR covers the DNS-hostname surface for external clients, complementing networkProfile.advertiseAddress), #1019 (LoadBalancer ingress hostname / AWS CNAME use case).

Key Changes

API

Added PublicAPIServerAddress to ServiceSpec (optional, no default).
Added PublicControlPlaneAddress(), returning the public address when set and falling back to AssignedControlPlaneAddress() otherwise.
Regenerated CRDs and deepcopy with the Makefile's pinned controller-gen.

Where the public address is (and is not) used

Following review feedback, the public address is used only on externally-facing surfaces:

internal/resources/kubeadm_config.go — sets the control plane endpoint and appends the public address to the certificate SANs.
DeclaredControlPlaneAddress() deliberately never returns the public address: kubeadm requires LocalAPIEndpoint.AdvertiseAddress to be a real IP.

The scheduler and controller-manager kubeconfigs continue to use the loopback address, as requested in review. The earlier replaceServerURLWithPublicAddress post-processing was reverted in 6e23040 and no longer exists. The only remaining change in internal/resources/kubeconfig.go is a no-op refactor collapsing two identical switch arms into one.

Note: my earlier comments on this PR (2026-07-24/25) stated that replaceServerURLWithPublicAddress had been restored for the controller-manager and scheduler kubeconfigs. That was inaccurate — those kubeconfigs use loopback, per your review. Apologies for the confusion.

Tests & docs

Unit tests for PublicControlPlaneAddress() (default/custom port, set/unset, error cases).
E2E test e2e/tcp_public_address_test.go, with per-test TCP cleanup.
Guide at docs/content/guides/public-api-server-address.md plus generated API reference.

Items needing maintainer input

Two changes on this branch go beyond the feature itself. Both are called out explicitly rather than buried in the diff, and I am happy to drop either.

1. DeclaredControlPlaneAddress() was rewritten — this is a behaviour change.

On master, for a LoadBalancer service it returns the LoadBalancer ingress IP, rejects ingress hostnames (with a documented rationale), and returns NonExposedLoadBalancerError / MissingValidIPError while no IP is assigned — errors that ShouldReconcileErrorBeIgnored uses to requeue.

On this branch it returns svc.Spec.ClusterIP, falling back to <name>.<namespace>.svc. Consequences:

A LoadBalancer TCP would advertise its ClusterIP rather than the LB IP.
Neither error is produced any more, so the requeue-while-waiting-for-LB-IP path is no longer reachable from this caller.
The explicit rejection of LoadBalancer ingress hostnames is gone.

I do not think this belongs in this PR. Unless you want it kept, I will revert DeclaredControlPlaneAddress() to the master implementation and rebase.

2. TriggerChannelTimeout raised from 30s to 3 minutes (controllers/utils/trigger_channel.go).

This predates #1258 and survived the rebase onto it. The rationale: the send must outlast controller-runtime's CacheSyncTimeout (default 2 minutes), which every registered controller blocks on at startup — otherwise a status-only notification can be dropped with nothing to re-trigger it. With #1258's buffered channel this only bites once the buffer is full, so it is now a narrower concern. If you would rather keep the 30s you just merged, say so and I will drop it.

Usage

apiVersion: kamaji.clastix.io/v1alpha1
kind: TenantControlPlane
spec:
  controlPlane:
    service:
      serviceType: LoadBalancer
      publicAPIServerAddress: "k8s-api.example.com"

CI

All checks green on the current head, including the full Kubernetes e2e suite.

This branch also carries the fix for the flaky tcp_migration_test.go webhook-wait, widening the Eventually from 1 to 5 minutes. CI run 30307341733 showed the manager going silent for 2m16s with nothing logged — CPU starvation on the 2-vCPU runner, where many control planes, datastores and apiservers compete for two cores, not a Kamaji bug. The same spec currently fails on #1252 and #1251, which lack this fix. Happy to split it into its own PR if that unblocks them sooner.

Breaking changes

The publicAPIServerAddress field itself is optional and defaults to existing behaviour. The DeclaredControlPlaneAddress() rewrite described above is a behaviour change for LoadBalancer services; my proposal is to revert it.

netlify · 2025-10-09T00:45:11Z

✅ Deploy Preview for kamaji-documentation canceled.

Name	Link
🔨 Latest commit	`2bea3f5`
🔍 Latest deploy log	https://app.netlify.com/projects/kamaji-documentation/deploys/68f33d9743b68e00089341f1

prometherion

I'm a bit lost here: it seems the proposed public API Server address is not pushed into the kubeadm functions, such as:

kamaji/internal/resources/kubeadm_config.go

Line 106 in 99fb71e

    
           TenantControlPlaneEndpoint:      r.getControlPlaneEndpoint(tenantControlPlane.Spec.ControlPlane.Ingress, address, port),

and the following one for kubeconfig

kamaji/internal/resources/kubeconfig.go

Line 234 in 99fb71e

    
           config.InitConfiguration.ControlPlaneEndpoint = fmt.Sprintf("%s.%s.svc:%d", tenantControlPlane.Name, tenantControlPlane.Namespace, tenantControlPlane.Spec.NetworkProfile.Port)

prometherion · 2025-11-28T14:19:45Z

@rossigee I'm sorry the rebase will be painful, let me know if I can take care of that!

netlify · 2026-02-22T22:14:21Z

✅ Deploy Preview for kamaji-documentation canceled.

Name	Link
🔨 Latest commit	`376dd2a`
🔍 Latest deploy log	https://app.netlify.com/projects/kamaji-documentation/deploys/6aa945dcde73420008edb42d

rossigee · 2026-07-24T23:22:11Z

Hi @prometherion - I've addressed your review comments from the previous iteration:

kubeadm_config.go: The is now used for:
- Setting the control plane endpoint when specified
- Adding the public address to certificate SANs
kubeconfig.go: The function has been restored for controller-manager and scheduler kubeconfigs to use the public API server address.

The branch has been rebased onto latest master, and lint/tests pass. Ready for re-review!

rossigee · 2026-07-24T23:22:21Z

Hi @prometherion - the review comments have been addressed:

kubeadm_config.go now uses PublicAPIServerAddress for endpoint and cert SANs
kubeconfig.go uses replaceServerURLWithPublicAddress for controller-manager/scheduler

Rebased on latest master, lint/tests pass. Ready for re-review!

rossigee · 2026-07-24T23:24:34Z

Updated - now uses PublicAPIServerAddress in both kubeadm config and kubeconfigs per original review request. Ready for re-review!

rossigee · 2026-07-25T00:28:07Z

Hi @prometherion - I've addressed your review comments:

kubeadm_config.go - PublicAPIServerAddress is now used for:
- Setting the control plane endpoint (line 96-115)
- Adding the public address to certificate SANs (line 119-121)
kubeconfig.go - The function is now used for controller-manager and scheduler kubeconfigs (line 225-232)

The branch has been rebased and cleaned up. Ready for re-review!

rossigee · 2026-07-25T03:37:08Z

The fix has been updated to use cluster-local SVC addresses (e.g., tcp-name.namespace.svc:6443) for controller-manager and scheduler kubeconfigs, instead of the public API Server address. This avoids unnecessary network hops through the load balancer while keeping traffic within the cluster.

The test now expects the server URL to have suffix .svc:6443 which is what this fix provides.

rossigee · 2026-07-25T03:42:00Z

I've addressed the review comments:

Controller-manager and scheduler addresses: The fix now uses cluster-local SVC addresses (e.g., ) instead of the public API Server address. This ensures traffic stays within the cluster and avoids unnecessary LB hops.
controller-gen: The CRDs have been generated using the Makefile.

The e2e test has been updated to expect suffix in the server URL for controller-manager and scheduler kubeconfigs.

rossigee · 2026-08-02T13:28:31Z

@prometherion rebased onto current master and rewrote the description to match what the branch actually does. Two things you should know:

A correction. My comments of 2026-07-24/25 said replaceServerURLWithPublicAddress had been restored for the controller-manager and scheduler kubeconfigs. That was wrong. Those kubeconfigs use the loopback address, exactly as you asked — the post-processing was reverted in 6e23040 and there are no public-address references left in internal/resources/kubeconfig.go. Sorry for the noise; the code was right and my comments were not.

Two things on this branch that need your call, both now spelled out in the description:

DeclaredControlPlaneAddress() has been rewritten and it is a behaviour change: a LoadBalancer TCP would advertise its ClusterIP instead of the LB ingress IP, NonExposedLoadBalancerError / MissingValidIPError are no longer produced (so the requeue-while-waiting path is unreachable from that caller), and the explicit rejection of LB ingress hostnames is gone. I do not think it belongs here — say the word and I will revert it to the master implementation.
TriggerChannelTimeout is raised from the 30s you merged in fix: use buffered channels and increase TriggerChannel timeout #1258 to 3 minutes, so the send outlasts controller-runtime's 2-minute CacheSyncTimeout. With fix: use buffered channels and increase TriggerChannel timeout #1258's buffered channel this only matters once the buffer fills, so I am happy to drop it if you prefer your value.

Also: this branch carries the fix for the flaky tcp_migration_test.go webhook-wait (1 minute → 5). CI run 30307341733 showed the manager silent for 2m16s with nothing logged — CPU starvation on the 2-vCPU runner, not a Kamaji bug. e2e is green here, and the same spec fails on #1252 and #1251 purely for want of this one line. Happy to split it into a standalone PR if that is easier to land first.

Add PreGeneratedCertificatesSpec to allow users to provide their own certificates instead of auto-generating them. Key features: - CertificateReference and KeyReference types for referencing secrets - PreGeneratedCertificatesSpec with CA, APIServer, KubeletClient, FrontProxyCA, FrontProxyClient, and ServiceAccount fields - Webhook validation for certificate/key references - Unit and e2e tests for the feature Signed-off-by: Ross Golder <ross@golder.org>

The webhook validates that pre-generated certificates exist in secrets, but the certificate generation code wasn't reading from them. This adds the usePreGeneratedCACertificate method to actually use the user-provided certificates instead of generating new ones.

…ources - ca_certificate.go: usePreGeneratedCACertificate - api_server_certificate.go: usePreGeneratedAPIServerCertificate - api_server_kubelet_client_certificate.go: usePreGeneratedKubeletClientCertificate - front_proxy_ca_certificate.go: usePreGeneratedFrontProxyCACertificate - front-proxy-client-certificate.go: usePreGeneratedFrontProxyClientCertificate - sa_certificate.go: usePreGeneratedSACertificate

The pre-generated certificates validation webhook was not being invoked because the handler was not registered in the webhook chain.

…rtificates The SecretNamespace field has been removed from CertificateReference and KeyReference types since cross-namespace secret references require complex RBAC setup that violates the principle of least privilege. Secrets referenced in pre-generated certificates must now be in the same namespace as the TenantControlPlane. Updated: - api/v1alpha1/tenantcontrolplane_types.go (removed SecretNamespace fields) - internal/resources/*.go (removed secretNamespace handling) - internal/webhook/handlers/tcp_pregenerated_certs.go (removed validation) - internal/webhook/handlers/tcp_pregenerated_certs_test.go (updated test)

…pport removal

…mespace fields

Add PreGeneratedCertificatesSpec to allow users to provide their own certificates instead of auto-generating them. Key features: - CertificateReference and KeyReference types for referencing secrets - PreGeneratedCertificatesSpec with CA, APIServer, KubeletClient, FrontProxyCA, FrontProxyClient, and ServiceAccount fields - Webhook validation for certificate/key references - Unit and e2e tests for the feature Signed-off-by: Ross Golder <ross@golder.org>

The webhook validates that pre-generated certificates exist in secrets, but the certificate generation code wasn't reading from them. This adds the usePreGeneratedCACertificate method to actually use the user-provided certificates instead of generating new ones.

…ources - ca_certificate.go: usePreGeneratedCACertificate - api_server_certificate.go: usePreGeneratedAPIServerCertificate - api_server_kubelet_client_certificate.go: usePreGeneratedKubeletClientCertificate - front_proxy_ca_certificate.go: usePreGeneratedFrontProxyCACertificate - front-proxy-client-certificate.go: usePreGeneratedFrontProxyClientCertificate - sa_certificate.go: usePreGeneratedSACertificate

…rtificates The SecretNamespace field has been removed from CertificateReference and KeyReference types since cross-namespace secret references require complex RBAC setup that violates the principle of least privilege. Secrets referenced in pre-generated certificates must now be in the same namespace as the TenantControlPlane. Updated: - api/v1alpha1/tenantcontrolplane_types.go (removed SecretNamespace fields) - internal/resources/*.go (removed secretNamespace handling) - internal/webhook/handlers/tcp_pregenerated_certs.go (removed validation) - internal/webhook/handlers/tcp_pregenerated_certs_test.go (updated test)

…pport removal

…mespace fields

Add RBACBootstrapSpec with AdminGroups and AdminUsers defaults. Bootstrap field added to TenantControlPlaneSpec. This enables RBAC bootstrap functionality with sensible defaults: - AdminUsers defaults to ["kubernetes-admin"] - AdminGroups defaults to ["system:masters"] Signed-off-by: Ross Golder <ross@golder.org>

This adds the RBAC bootstrap controller implementation that creates ClusterRoleBindings for admin users and groups specified in the TenantControlPlane's BootstrapSpec. Changes: - Add BootstrapStatus and RBACBootstrapStatus types to track status - Add RBACBootstrap resource that implements the Resource interface - Create ClusterRoleBinding with cluster-admin role for specified users/groups - Add unit tests for RBAC bootstrap functionality - Integrate RBAC bootstrap resource into controller resources The controller creates ClusterRoleBindings in the tenant cluster to grant cluster-admin privileges to specified users and groups at bootstrap time.

…atement, blank line before return

The ClusterRoleBinding was never created. Define() set a placeholder name and mutate() reassigned it, but controllerutil.CreateOrUpdate captures the object key before invoking the MutateFn and rejects a change to it, so every reconcile failed with "MutateFn cannot mutate object name and/or object namespace". CleanUp() resolved the same placeholder and so never found the object it was meant to delete. The name is now computed in Define() from the Tenant Control Plane name alone. Deriving it from the spec contents meant that editing adminUsers or adminGroups renamed the object and orphaned the previously created binding. RBACBootstrapSpec.Enabled becomes a *bool. With a plain bool, omitempty dropped an explicit false during serialization and the API server re-applied the true default, leaving no way to disable the feature. The repeated nil checks are replaced by TenantControlPlane.IsRBACBootstrapEnabled(). The existing unit tests all called mutate() directly, so no test exercised the code path that runs in production. Added specs covering creation, idempotence, naming stability across spec edits, unset and explicit-false Enabled, and cleanup, plus an e2e spec asserting the binding lands in the Tenant cluster and is removed when the feature is disabled. Documents the feature at docs/content/guides/rbac-bootstrap.md.

…rap test The spec used a ClusterIP service with the kind node address and the default port, so the generated kubeconfig pointed at https://<kind-node>:6443 - the management cluster's own API server. The tenant CA correctly refused to verify that certificate and the spec failed with an x509 unknown-authority error before it ever reached an RBAC assertion. Uses a NodePort on a random high port, as the datastore migration spec does, so the tenant API server is actually reachable from the test process. Also scopes the "exactly one binding" assertion to the component label: Kamaji creates other ClusterRoleBindings in the tenant cluster, from the kubeadm phases and from the addons, that carry the same control plane name label.

…udget TriggerChannel is a one-shot, best-effort notification with no retry for status-only TenantControlPlane changes. Its 10s send deadline was shorter than controller-runtime's own default 2-minute CacheSyncTimeout, which every registered controller (including the receiving one) blocks on during startup. Under a slow initial cache sync, the notification could be silently dropped forever, since nothing else re-triggers it. This surfaced as e2e/tcp_migration_test.go timing out waiting for the kamaji-freeze ValidatingWebhookConfiguration after the Kubernetes version bump to v1.35.7 (commit 8a19328), which broadened the tenant apiserver's discoverable API surface and made cache-sync take measurably longer.

When using preGeneratedCertificates, the kubeconfig checksum calculation included config.Checksum() which depends on cert SANs added from the control plane Service IPs (ClusterIPs and LoadBalancer ingress IPs). As the Service gets provisioned with IPs, the cert SANs change, causing config.Checksum() to change, which triggers kubeconfig regeneration even though the actual certificates are fixed. This creates an infinite reconciliation loop where: 1. Kubeconfig is regenerated → component checksums change 2. Deployment pod-template-hash changes → new ReplicaSet created 3. Next reconcile triggered → go to step 1 For pregenerated certificates, the actual certs are immutable, so changes to cert SANs should not trigger kubeconfig regeneration. Fix by excluding config.Checksum() from the kubeconfig checksum calculation when preGeneratedCertificates are specified. Fixes TenantControlPlanes stuck in "Provisioning" status with pregenerated certificates, enabling them to transition to "Ready" once service provisioning completes.

IsKubeconfigCAValid used strict bytes.Equal between the certificate embedded in the kubeconfig and the CA stored in the CA secret. When the CA secret holds the certificate in a different encoding (e.g. an openssl x509 -text dump), the comparison always failed, forcing the kubeconfig to be regenerated on every reconciliation and causing a continuous regeneration loop. Compare the parsed x509 certificates instead, which is encoding agnostic and matches the proof check performed in IsKubeconfigValid.

… SANs

- Add missing Resources field to KonnectivityAgentSpec struct for resource allocation control on konnectivity agents - Fix unreachable code in DeclaredControlPlaneAddress() by removing dead return statement after switch default case - Remove unused getLoadBalancerAddress() function and related error import - Fix import ordering in trigger_channel.go (gci/gofmt linter issues) - Add blank line before break statement (nlreturn linter requirement) All tests passing, golangci-lint: 0 issues

Regenerate CRD definitions to reflect changes from source code: - Remove dead code and unused functions - Add Resources field to KonnectivityAgentSpec - Update derived CRD schemas accordingly

rossigee mentioned this pull request Oct 9, 2025

feat: add pregenerated certificates support #928

Closed

prometherion requested changes Oct 10, 2025

View reviewed changes

Comment thread charts/kamaji/crds/kamaji.clastix.io_datastores.yaml Outdated

rossigee requested a review from prometherion October 18, 2025 07:11

prometherion requested changes Oct 18, 2025

View reviewed changes

Comment thread Makefile Outdated

Comment thread api/v1alpha1/tenantcontrolplane_public_address.go

Comment thread internal/resources/kubeconfig.go Outdated

Comment thread internal/resources/kubeconfig.go Outdated

rossigee force-pushed the feature/public-address branch from 2bea3f5 to eaffca1 Compare February 22, 2026 22:14

rossigee force-pushed the feature/public-address branch from 922610a to 566d3c9 Compare February 22, 2026 22:22

prometherion mentioned this pull request Mar 22, 2026

Add advertiseAddress to NetworkProfileSpec for split management/tenant addressing #1110

Closed

rossigee force-pushed the feature/public-address branch from d3e5503 to 44e305e Compare April 26, 2026 21:52

rossigee force-pushed the feature/public-address branch from 44e305e to 3574521 Compare July 24, 2026 23:21

rossigee requested a review from prometherion July 24, 2026 23:25

rossigee changed the title ~~feature: Add Public API Server Address Support~~ feat: Add Public API Server Address Support Jul 25, 2026

rossigee force-pushed the feature/public-address branch 2 times, most recently from b5291db to 143a262 Compare July 25, 2026 00:27

rossigee force-pushed the feature/public-address branch 3 times, most recently from 834d2fe to 55bfc57 Compare August 2, 2026 10:10

This was referenced Aug 2, 2026

feat: add pre-generated certificates support #1252

Open

feat: add default admin groups to RBAC bootstrap #1251

Open

rossigee mentioned this pull request Aug 3, 2026

fix(e2e): widen the migration webhook wait above the observed CI stall #1260

Merged

rossigee force-pushed the feature/public-address branch from 1d16cdf to 4813b0c Compare August 13, 2026 04:00

rossigee added 28 commits September 15, 2026 20:00

fix: register TenantControlPlanePreGeneratedCerts webhook handler

6e75210

The pre-generated certificates validation webhook was not being invoked because the handler was not registered in the webhook chain.

fix: remove stale SecretNamespace references after cross-namespace su…

cf5fa80

…pport removal

fix: format test file and regenerate API docs removing stale secretNa…

9e350dd

…mespace fields

fix: remove stale SecretNamespace references after cross-namespace su…

014f202

…pport removal

fix: format test file and regenerate API docs removing stale secretNa…

5a10430

…mespace fields

feat: add default admin groups to RBAC bootstrap

69d9635

chore: regenerate CRDs and API docs for RBAC bootstrap status fields

5611659

fix: resolve lint issues - copyright year, import grouping, switch st…

0bc97d2

…atement, blank line before return

feat: add PublicAPIServerAddress support for kubeadm and kubeconfigs

b24032d

fix: add missing errors import to tenantcontrolplane_funcs.go

dbae362

fix: include assigned control plane address in API server certificate…

30493ea

… SANs

chore: regenerate files

9fedf56

rossigee force-pushed the feature/public-address branch from 07aa0ef to 73b4972 Compare September 15, 2026 13:15

chore: regenerate CRDs after source changes

376dd2a

Regenerate CRD definitions to reflect changes from source code: - Remove dead code and unused functions - Add Resources field to KonnectivityAgentSpec - Update derived CRD schemas accordingly

Navigation Menu

feat: Add Public API Server Address Support - #986

feat: Add Public API Server Address Support#986
rossigee wants to merge 41 commits into
clastix:masterfrom
rossigee:feature/public-address

rossigee commented Oct 9, 2025 •

edited

Loading

Uh oh!

netlify Bot commented Oct 9, 2025 •

edited

Loading

Uh oh!

Uh oh!

prometherion left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

prometherion commented Nov 28, 2025

Uh oh!

netlify Bot commented Feb 22, 2026 •

edited

Loading

Uh oh!

rossigee commented Jul 24, 2026

Uh oh!

rossigee commented Jul 24, 2026

Uh oh!

rossigee commented Jul 24, 2026

Uh oh!

rossigee commented Jul 25, 2026

Uh oh!

rossigee commented Jul 25, 2026

Uh oh!

rossigee commented Jul 25, 2026

Uh oh!

rossigee commented Aug 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

rossigee commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Add Public API Server Address Support

Overview

Key Changes

API

Where the public address is (and is not) used

Tests & docs

Items needing maintainer input

Usage

CI

Breaking changes

Uh oh!

netlify Bot commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for kamaji-documentation canceled.

Uh oh!

Uh oh!

prometherion left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

prometherion commented Nov 28, 2025

Uh oh!

netlify Bot commented Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for kamaji-documentation canceled.

Uh oh!

rossigee commented Jul 24, 2026

Uh oh!

rossigee commented Jul 24, 2026

Uh oh!

rossigee commented Jul 24, 2026

Uh oh!

rossigee commented Jul 25, 2026

Uh oh!

rossigee commented Jul 25, 2026

Uh oh!

rossigee commented Jul 25, 2026

Uh oh!

rossigee commented Aug 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rossigee commented Oct 9, 2025 •

edited

Loading

netlify Bot commented Oct 9, 2025 •

edited

Loading

netlify Bot commented Feb 22, 2026 •

edited

Loading