From 2fd0e8b751c5f21f21a9bf79b7030cb290b757ec Mon Sep 17 00:00:00 2001 From: Roke Jung Date: Mon, 27 Apr 2026 21:34:04 -0400 Subject: [PATCH] fix: tls-server-name field of external-managed-kubeconfig is missing (#1502) Signed-off-by: Roke Jung --- pkg/operator/helpers/sa_syncer.go | 9 ++ pkg/operator/helpers/sa_syncer_test.go | 145 +++++++++++++++++++++++++ 2 files changed, 154 insertions(+) diff --git a/pkg/operator/helpers/sa_syncer.go b/pkg/operator/helpers/sa_syncer.go index a71a2d6b2..86e5a8bd3 100644 --- a/pkg/operator/helpers/sa_syncer.go +++ b/pkg/operator/helpers/sa_syncer.go @@ -177,6 +177,11 @@ func clusterInfoNotChanged(ctx context.Context, secret *corev1.Secret, templateK "before", cluster.InsecureSkipTLSVerify, "after", templateCluster.InsecureSkipTLSVerify) return false } + if cluster.TLSServerName != templateCluster.TLSServerName { + logger.Info("Cluster tls-server-name changed", + "before", cluster.TLSServerName, "after", templateCluster.TLSServerName) + return false + } return true } @@ -245,11 +250,13 @@ func applyKubeconfigSecret(ctx context.Context, templateKubeconfig *rest.Config, } func assembleClusterConfig(templateKubeconfig *rest.Config) (*clientcmdapi.Cluster, error) { + tlsServerName := templateKubeconfig.ServerName var c *clientcmdapi.Cluster if len(templateKubeconfig.CAData) != 0 { //nolint:gocritic c = &clientcmdapi.Cluster{ Server: templateKubeconfig.Host, CertificateAuthorityData: templateKubeconfig.CAData, + TLSServerName: tlsServerName, } } else if len(templateKubeconfig.CAFile) != 0 { caData, err := os.ReadFile(templateKubeconfig.CAFile) @@ -259,11 +266,13 @@ func assembleClusterConfig(templateKubeconfig *rest.Config) (*clientcmdapi.Clust c = &clientcmdapi.Cluster{ Server: templateKubeconfig.Host, CertificateAuthorityData: caData, + TLSServerName: tlsServerName, } } else { c = &clientcmdapi.Cluster{ Server: templateKubeconfig.Host, InsecureSkipTLSVerify: true, + TLSServerName: tlsServerName, } } return c, nil diff --git a/pkg/operator/helpers/sa_syncer_test.go b/pkg/operator/helpers/sa_syncer_test.go index 9c6504a2b..b7c094e8a 100644 --- a/pkg/operator/helpers/sa_syncer_test.go +++ b/pkg/operator/helpers/sa_syncer_test.go @@ -3,6 +3,8 @@ package helpers import ( "context" "fmt" + "os" + "path/filepath" "reflect" "testing" @@ -14,10 +16,153 @@ import ( testclient "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/rest" clienttesting "k8s.io/client-go/testing" + "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" "open-cluster-management.io/sdk-go/pkg/basecontroller/events" ) +func TestAssembleClusterConfig_TLSServerName(t *testing.T) { + tlsName := "kubernetes.default.svc" + t.Run("CAData", func(t *testing.T) { + cfg := &rest.Config{ + Host: "https://10.0.0.1:6443", + TLSClientConfig: rest.TLSClientConfig{ + CAData: []byte("ca-bytes"), + ServerName: tlsName, + }, + } + c, err := assembleClusterConfig(cfg) + if err != nil { + t.Fatal(err) + } + if c.TLSServerName != tlsName { + t.Fatalf("TLSServerName = %q, want %q", c.TLSServerName, tlsName) + } + }) + t.Run("CAFile", func(t *testing.T) { + dir := t.TempDir() + caPath := filepath.Join(dir, "ca.crt") + if err := os.WriteFile(caPath, []byte("ca-file-bytes"), 0o600); err != nil { + t.Fatal(err) + } + cfg := &rest.Config{ + Host: "https://10.0.0.1:6443", + TLSClientConfig: rest.TLSClientConfig{ + CAFile: caPath, + ServerName: tlsName, + }, + } + c, err := assembleClusterConfig(cfg) + if err != nil { + t.Fatal(err) + } + if c.TLSServerName != tlsName { + t.Fatalf("TLSServerName = %q, want %q", c.TLSServerName, tlsName) + } + }) + t.Run("insecure", func(t *testing.T) { + // No CAData/CAFile selects the InsecureSkipTLSVerify branch in assembleClusterConfig. + cfg := &rest.Config{ + Host: "https://10.0.0.1:6443", + TLSClientConfig: rest.TLSClientConfig{ + ServerName: tlsName, + }, + } + c, err := assembleClusterConfig(cfg) + if err != nil { + t.Fatal(err) + } + if !c.InsecureSkipTLSVerify { + t.Fatalf("InsecureSkipTLSVerify = false, want true") + } + if c.TLSServerName != tlsName { + t.Fatalf("TLSServerName = %q, want %q", c.TLSServerName, tlsName) + } + }) +} + +func TestSyncKubeConfigSecret_tlsServerNameDrift(t *testing.T) { + tkc := &rest.Config{ + Host: "https://api.example:6443", + TLSClientConfig: rest.TLSClientConfig{ + CAData: []byte("caData"), + ServerName: "kubernetes.default.svc", + }, + } + kubeconfigMissingTLSName, err := clientcmd.Write(clientcmdapi.Config{ + Kind: "Config", + APIVersion: "v1", + Clusters: map[string]*clientcmdapi.Cluster{ + "cluster": { + Server: tkc.Host, + CertificateAuthorityData: tkc.TLSClientConfig.CAData, + }, + }, + Contexts: map[string]*clientcmdapi.Context{ + "context": {Cluster: "cluster", AuthInfo: "user"}, + }, + AuthInfos: map[string]*clientcmdapi.AuthInfo{ + "user": {Token: "ignored"}, + }, + CurrentContext: "context", + }) + if err != nil { + t.Fatal(err) + } + + secretName := "test-secret" + secretNamespace := "test-ns" + client := testclient.NewSimpleClientset(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: secretName, Namespace: secretNamespace}, + Data: map[string][]byte{ + "token": []byte("aaa"), + "kubeconfig": kubeconfigMissingTLSName, + }, + }) + tokenGetter := func() ([]byte, []byte, map[string][]byte, error) { + return []byte("aaa"), nil, nil, nil + } + err = SyncKubeConfigSecret( + context.TODO(), secretName, secretNamespace, + "/tmp/kubeconfig", tkc, client.CoreV1(), tokenGetter, + events.NewContextualLoggingEventRecorder(t.Name()), nil) + if err != nil { + t.Fatal(err) + } + actions := client.Actions() + var lastSecret *corev1.Secret + for i := len(actions) - 1; i >= 0; i-- { + switch a := actions[i].(type) { + case clienttesting.CreateAction: + if s, ok := a.GetObject().(*corev1.Secret); ok { + lastSecret = s + } + case clienttesting.UpdateAction: + if s, ok := a.GetObject().(*corev1.Secret); ok { + lastSecret = s + } + } + if lastSecret != nil { + break + } + } + if lastSecret == nil { + t.Fatalf("expected a create/update secret action, got %#v", actions) + } + loaded, err := clientcmd.Load(lastSecret.Data["kubeconfig"]) + if err != nil { + t.Fatal(err) + } + cluster := loaded.Clusters["cluster"] + if cluster == nil { + t.Fatal("cluster context missing") + } + if cluster.TLSServerName != tkc.ServerName { + t.Fatalf("synced TLSServerName = %q, want %q", cluster.TLSServerName, tkc.ServerName) + } +} + func TestTokenGetter(t *testing.T) { saName := "test-sa" saNamespace := "test-ns"