Skip to content

operator: support TLS admin API on V1 Cluster CRs (K8S-939) #1817

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
sago2k8 wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

operator: support TLS admin API on V1 Cluster CRs (K8S-939) #1817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0581b8e
operator: support TLS admin API on V1 Cluster CRs (K8S-939)
sago2k8 Sep 1, 2026
c5c6537
operator: test the V1 admin client against a TLS broker (K8S-939)
sago2k8 Sep 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .changes/unreleased/operator-Fixed-20260901-164900.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
project: operator
kind: Fixed
body: Admin API clients for vectorized.io/v1alpha1 Clusters no longer fail with "non-TLS admin API is not supported on V1 CRD" when TLS is enabled on an admin listener. This unblocks every V2 CR and controller that reaches a V1 Cluster over the admin API (User, Role, and ShadowLink CRs, Broker CRs on V1 node pools, and the ghost-broker decommissioner). When the internal admin listener has TLS enabled the client now resolves the cluster's certificates the same way the V1 Kafka and Schema Registry clients do, and a plaintext internal listener keeps working regardless of TLS on the external admin listener or on other APIs' listeners

@david-yu david-yu Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this could shrink to a few lines naming the symptom and the affected mode — long single-paragraph entries tend to get rewritten in review.

time: 2026-09-01T16:49:00.000000000+02:00
3 changes: 3 additions & 0 deletions operator/pkg/admin/admin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ func NewNodePoolInternalAdminAPI(

var tlsConfig *tls.Config
if adminInternal.TLS.Enabled {
if adminTLSProvider == nil {
return nil, fmt.Errorf("internal admin API of cluster %s/%s has TLS enabled but no TLS provider was given", redpandaCluster.Namespace, redpandaCluster.Name)

@david-yu david-yu Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: cockroachdb errors.Newf for new code (stack traces); fmt.Errorf matches the rest of this file though, so fine either way.

}
var err error
tlsConfig, err = adminTLSProvider.GetTLSConfig(ctx, k8sClient)
if err != nil {
Expand Down
44 changes: 32 additions & 12 deletions operator/pkg/client/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ package client

import (
"context"
"fmt"

"github.com/cockroachdb/errors"
"github.com/redpanda-data/common-go/rpadmin"
"github.com/twmb/franz-go/pkg/kgo"
"github.com/twmb/franz-go/pkg/sr"
Expand All @@ -23,7 +23,7 @@ import (
vectorizedv1alpha1 "github.com/redpanda-data/redpanda-operator/operator/api/vectorized/v1alpha1"
"github.com/redpanda-data/redpanda-operator/operator/pkg/admin"
"github.com/redpanda-data/redpanda-operator/operator/pkg/client/shadow"
"github.com/redpanda-data/redpanda-operator/operator/pkg/resources/certmanager"
resourcetypes "github.com/redpanda-data/redpanda-operator/operator/pkg/resources/types"
)

// redpandaAdminForCluster returns a simple rpadmin.AdminAPI able to communicate with the given cluster specified via a Redpanda cluster.
Expand Down Expand Up @@ -57,26 +57,46 @@ func (c *Factory) redpandaAdminForCluster(ctx context.Context, cluster *redpanda
return client, nil
}

// redpandaAdminForV1Cluster returns a simple rpadmin.AdminAPI able to communicate with the given V1 cluster through its internal admin listener.
func (c *Factory) redpandaAdminForV1Cluster(ctx context.Context, cluster *vectorizedv1alpha1.Cluster, clusterName string) (*rpadmin.AdminAPI, error) {
client, err := c.GetClient(ctx, clusterName)
k8sClient, err := c.GetClient(ctx, clusterName)
if err != nil {
return nil, err
}

if cluster.AdminAPITLS() != nil {
return nil, fmt.Errorf("non-TLS admin API is not supported on V1 CRD")
// NewNodePoolInternalAdminAPI consults the TLS provider only when the
// internal admin listener has TLS enabled, so resolve the certificates only
// then. NewClusterCertificates walks the listeners of every API and reads
// their Issuers and node secrets, and a plaintext admin listener must not
// fail on the cert-manager state of the Kafka, Schema Registry, or Proxy
// listeners.
fqdn := v1ClusterFQDN(ctx, k8sClient, cluster)
var certs resourcetypes.AdminTLSConfigProvider
if internal := cluster.AdminAPIInternal(); internal != nil && internal.TLS.Enabled {
if _, certs, err = v1ClusterCerts(ctx, k8sClient, cluster); err != nil {

@david-yu david-yu Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Observation, fine as-is: with TLS on, NewClusterCertificates walks every API, so broken cert state on an unrelated listener (e.g. a missing Kafka Issuer) now fails the admin client too. Consistent with the v1 Kafka/SR builders; a follow-up could narrow resolution to the admin graph.

return nil, err
}
}
// Assume no TLS. Practically, we don't need to support it in Operator V1.
t := &certmanager.ClusterCertificates{}

a, err := admin.NewNodePoolInternalAdminAPI(ctx, client, cluster, fmt.Sprintf("%s.%s.svc.cluster.local", cluster.Name, cluster.Namespace), t, c.dialer, c.adminClientTimeout)
a, err := admin.NewNodePoolInternalAdminAPI(ctx, k8sClient, cluster, fqdn, certs, c.dialer, c.adminClientTimeout)
if err != nil {
return nil, err
}
// It's weird that the V1 admin factory returns an interface instead of the
// rpadmin struct. We'll cast it back; it's ugly but since this is a legacy
// code path we don't care too much.
return a.(*rpadmin.AdminAPI), nil
// The V1 admin factory returns an interface; the rest of the Factory works
// with the rpadmin struct.
adminClient, ok := a.(*rpadmin.AdminAPI)
if !ok {
return nil, errors.Newf("unexpected admin API client type %T for cluster %s/%s", a, cluster.Namespace, cluster.Name)
}

if c.userAuth != nil {
adminClient.SetAuth(&rpadmin.BasicAuth{
Username: c.userAuth.Username,
Password: c.userAuth.Password,
})
}

return adminClient, nil
}

// schemaRegistryForCluster returns a simple sr.Client able to communicate with the given cluster specified via a Redpanda cluster.
Expand Down
10 changes: 9 additions & 1 deletion operator/pkg/client/factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,10 +494,18 @@ func (c *Factory) UsersForCluster(ctx context.Context, obj redpandav1alpha2.Clus

adminClient, err := c.RedpandaAdminClientForCluster(ctx, obj, clusterName)
if err != nil {
kafkaClient.Close()
return nil, err
}

usersClient, err := users.NewClient(ctx, client, kadm.NewClient(kafkaClient), adminClient)
if err != nil {
kafkaClient.Close()
adminClient.Close()
return nil, err
}

return users.NewClient(ctx, client, kadm.NewClient(kafkaClient), adminClient)
return usersClient, nil
}

func (c *Factory) Users(ctx context.Context, obj redpandav1alpha2.ClusterReferencingObject, opts ...kgo.Opt) (*users.Client, error) {
Expand Down
16 changes: 14 additions & 2 deletions operator/pkg/client/v1.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,22 @@ func newNodePoolInternalSchemaRegistryAPI(
return sr.NewClient(append(copts, opts...)...)
}

func v1ClusterCerts(ctx context.Context, k8sClient client.Client, cluster *vectorizedv1alpha1.Cluster) (string, *certmanager.ClusterCertificates, error) {
// v1ClusterFQDN returns the headless service FQDN of a V1 cluster; the
// internal clients dial its brokers as <pod>.<fqdn>. The cluster domain is
// hardcoded here while the V1 controller reads it from --cluster-domain, so

@david-yu david-yu Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add the tracking ticket here (TODO + K8S-xxx) — "tracked separately" in the PR body gets lost once this merges.

// clusters on a non-default domain are not reachable through the Factory.
func v1ClusterFQDN(ctx context.Context, k8sClient client.Client, cluster *vectorizedv1alpha1.Cluster) string {
headlessSvc := resources.NewHeadlessService(k8sClient, cluster, controller.UnifiedScheme, nil, log.FromContext(ctx))
return headlessSvc.HeadlessServiceFQDN("cluster.local")
}

// v1ClusterCerts returns the headless service FQDN of a V1 cluster together
// with the TLS provider for its listeners. Building the provider resolves the
// certificate groups of every API, which reads the Issuers and node secrets
// the listeners reference.
func v1ClusterCerts(ctx context.Context, k8sClient client.Client, cluster *vectorizedv1alpha1.Cluster) (string, *certmanager.ClusterCertificates, error) {

@david-yu david-yu Sep 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

certmanager already exposes GetTLSConfigValues() *ir.TLSConfig, and the ShadowLink path later in this file already routes v1 TLS through ir.TLSConfig. Routing the admin path through IR the same way would let v1 and v2 share one client-construction core instead of parallel builders.

clusterSvc := resources.NewClusterService(k8sClient, cluster, controller.UnifiedScheme, nil, log.FromContext(ctx))
fqdn := headlessSvc.HeadlessServiceFQDN("cluster.local")
fqdn := v1ClusterFQDN(ctx, k8sClient, cluster)
clusterFQDN := clusterSvc.ServiceFQDN("cluster.local")
certs, err := certmanager.NewClusterCertificates(ctx, cluster, certmanager.KeyStoreKey(cluster), k8sClient, fqdn, clusterFQDN, controller.UnifiedScheme, log.FromContext(ctx))
if err != nil {
Expand Down
Loading
Loading