operator: support TLS admin API on V1 Cluster CRs (K8S-939) - #1817
Draft
sago2k8 wants to merge 2 commits into
Draft
operator: support TLS admin API on V1 Cluster CRs (K8S-939)#1817sago2k8 wants to merge 2 commits into
sago2k8 wants to merge 2 commits into
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
redpandaAdminForV1Cluster refused to build a client whenever
Cluster.AdminAPITLS() returned a listener, with the message "non-TLS
admin API is not supported on V1 CRD". The message is backwards: it
fires when TLS *is* configured. The guard existed because the builder
handed NewNodePoolInternalAdminAPI an empty ClusterCertificates{} as its
TLS provider, which would nil-dereference in getCommonTLS the moment the
internal listener had TLS enabled. Nothing ever resolved the
certificates.
The fallout: every consumer that reaches a vectorized.io/v1alpha1
Cluster over the admin API is dead as soon as that cluster enables admin
TLS. That is the User, Role and ShadowLink CRs, Broker CRs on V1 node
pools, and the ghost-broker decommissioner. Serverless cells run the
admin API over mTLS, so all of them were failing there.
AdminAPITLS() also matches external listeners, so a cluster with a
plaintext internal admin listener and TLS only on the external one was
rejected as well, even though the operator only ever talks to the
internal listener.
Resolve the certificates with v1ClusterCerts, like the V1 Kafka and
Schema Registry builders do, but only when the internal admin listener
has TLS enabled. NewClusterCertificates walks every API's listeners and
reads their Issuers and node secrets, and a plaintext admin client must
not fail on the cert-manager state of the Kafka or Schema Registry
listeners. NewNodePoolInternalAdminAPI now returns an error instead of
dereferencing a nil provider when TLS is on.
Take the FQDN from HeadlessServiceFQDN instead of a hand-built string so
it honours dnsTrailingDotDisabled. Apply the Factory's userAuth like the
V2 admin builder does; the V1 builder silently dropped credentials, so
the acceptance authentication check passed for any password on the
vectorized variant. Close the Kafka client in UsersForCluster when the
admin client fails to build; the new transient failure modes make that
path hot.
Fixes K8S-939.
Drive redpandaAdminForV1Cluster through the stub manager against an httptest server behind a recording dialer, so the test proves the client speaks https with the pinned CA, presents the client certificate under mTLS, and sends Basic auth, instead of only asserting that a struct came back. Missing node or client secrets surface by name, an untrusted CA fails with x509, and a missing Kafka Issuer no longer blocks a plaintext admin client. The external admin listener in the plaintext-internal case could not exist before: port 9645 is outside the nodeport range the webhook enforces, and TLS on an external listener requires a subdomain. Use a shape that validates. Refs K8S-939.
sago2k8
force-pushed
the
sj/v1-cluster-tls-admin-api
branch
from
September 3, 2026 16:57
e366983 to
c5c6537
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Build a working admin API client for
vectorized.io/v1alpha1Clusters that have TLS on the internal admin listener. The v1 path inpkg/clientnow resolves the cluster's certificates throughv1ClusterCerts, the same helper the v1 Kafka and Schema Registry clients already use, but only when the internal admin listener actually has TLS enabled.Why
redpandaAdminForV1Clusterrefused to build a client wheneverCluster.AdminAPITLS()returned a listener and failed with "non-TLS admin API is not supported on V1 CRD". The check is backwards: it fires when TLS is on. The guard existed because the builder handedNewNodePoolInternalAdminAPIan empty certificate stub that would nil-dereference as soon as the internal listener had TLS. Nothing ever resolved the certificates.Every consumer that reaches a v1 Cluster over the admin API goes through this path: User, Role and ShadowLink CRs, Broker CRs on v1 node pools, and the ghost-broker decommissioner. Serverless cells run the admin API over mTLS, so on them:
RedpandaRoleCRs never sync. Cloud RBAC grants do not land as ACLs. This is what fires the "DP - Policy Materializer Roles Not Syncing" sev 2 alert.Two smaller defects in the same lines:
AdminAPITLS()also matches external listeners, so plaintext-internal + TLS-external was rejected too, and the hardcoded FQDN ignoreddnsTrailingDotDisabled.Implementation details
NewClusterCertificateswalks every API's listeners and reads their Issuers and node secrets, so a plaintext admin client must not fail on the cert-manager state of the Kafka or Schema Registry listeners.v1ClusterFQDNis split out so the plaintext path gets the FQDN without building the certificate graph.NewNodePoolInternalAdminAPIreturns an error when TLS is enabled and no provider was given, instead of dereferencing nil.userAuthis applied like the v2 admin builder does. The v1 builder silently dropped credentials, so the acceptance authentication check passed for any password on the vectorized variant.UsersForClustercloses the Kafka client when the admin client fails to build. The new transient failure modes (node cert not issued yet) make that path hot.a.(*rpadmin.AdminAPI)assertion is now checked.operator/pkg/client/v1_admin_tls_test.godrives the real builder through the stub manager against an httptest broker behind a recording dialer: https with the pinned CA on the dotted host, an untrusted CA failing with x509, mTLS presenting<cluster>-admin-api-client, missing node and client secrets surfacing by name, a missing Kafka Issuer not blocking a plaintext client, and the Basic auth header on the wire.v1ClusterCertshardcodescluster.localwhile the v1 controller mints SANs from--cluster-domain. Documented onv1ClusterFQDN, tracked separately.clusterissuersread, so a v1 Cluster withissuerRefkindClusterIssuergets Forbidden. Tracked separately.References