[client-go] [cli-runtime] [133916]: handle properly config override logic when override provides ClientKey, ClientCertificate: use values from overrides when one of the field (file or data) is present in inverrides

Signed-off-by: Nikita B <n2h9z4@gmail.com>

Kubernetes-commit: 6b908c192cc828abef39c35dcc4921281f950958

tools/clientcmd/client_config.go

@@ -537,13 +537,15 @@ func (config *DirectClientConfig) getAuthInfo() (clientcmdapi.AuthInfo, error) {
 		// Handle ClientKey/ClientKeyData conflict: if override sets ClientKey, also use override's ClientKeyData
 		// otherwise if original config has ClientKeyData set,
 		// validation returns error "client-key-data and client-key are both specified <user-name>"
-		if len(config.overrides.AuthInfo.ClientKey) > 0 {
+		if len(config.overrides.AuthInfo.ClientKey) > 0 || len(config.overrides.AuthInfo.ClientKeyData) > 0 {
+			mergedAuthInfo.ClientKey = config.overrides.AuthInfo.ClientKey
 			mergedAuthInfo.ClientKeyData = config.overrides.AuthInfo.ClientKeyData
 		}
 		// Handle ClientCertificate/ClientCertificateData conflict, if override sets ClientCertificate, also use override's ClientCertificateData
 		// otherwise if original config has ClientCertificateData set,
 		// validation returns error "client-cert-data and client-cert are both specified <user-name>"
-		if len(config.overrides.AuthInfo.ClientCertificate) > 0 {
+		if len(config.overrides.AuthInfo.ClientCertificate) > 0 || len(config.overrides.AuthInfo.ClientCertificateData) > 0 {
+			mergedAuthInfo.ClientCertificate = config.overrides.AuthInfo.ClientCertificate
 			mergedAuthInfo.ClientCertificateData = config.overrides.AuthInfo.ClientCertificateData
 		}
 	}

tools/clientcmd/client_config_test.go

@@ -1224,74 +1224,164 @@ func TestMergeRawConfigDoOverride(t *testing.T) {
 }
 
 func TestClientCertOverrideData(t *testing.T) {
-	// Test that when overrides contain cert/key file paths, the corresponding
-	// data fields are properly handled to avoid validation conflicts
-	// in particular code in DirectClientConfig::getAuthInfo
+	// Test that when overrides contain cert/key file paths or data fields,
+	// the corresponding fields are properly handled to avoid validation conflicts
+	// in particular code in DirectClientConfig::getAuthInfo.
+	// This covers both scenarios: overrides with file paths (which clear data fields)
+	// and overrides with data fields (which clear file paths).
 
-	certFile, err := os.CreateTemp("", "test-client-*.crt")
-	if err != nil {
-		t.Fatalf("Failed to create temp cert file: %v", err)
-	}
-	defer utiltesting.CloseAndRemove(t, certFile)
+	testCases := []struct {
+		name        string
+		description string
+		setupTest   func(t *testing.T) (*clientcmdapi.Config, *ConfigOverrides, func())
+		validate    func(t *testing.T, authInfo *clientcmdapi.AuthInfo)
+	}{
+		{
+			name:        "override-with-file-paths",
+			description: "Test override with cert/key file paths",
+			setupTest: func(t *testing.T) (*clientcmdapi.Config, *ConfigOverrides, func()) {
+				certFile, err := os.CreateTemp("", "test-client-*.crt")
+				if err != nil {
+					t.Fatalf("Failed to create temp cert file: %v", err)
+				}
 
-	keyFile, err := os.CreateTemp("", "test-client-*.key")
-	if err != nil {
-		t.Fatalf("Failed to create temp key file: %v", err)
-	}
-	defer utiltesting.CloseAndRemove(t, keyFile)
+				keyFile, err := os.CreateTemp("", "test-client-*.key")
+				if err != nil {
+					t.Fatalf("Failed to create temp key file: %v", err)
+				}
 
-	if err := os.WriteFile(certFile.Name(), []byte("dummy-cert-content"), 0600); err != nil {
-		t.Fatalf("Failed to write cert file: %v", err)
-	}
-	if err := os.WriteFile(keyFile.Name(), []byte("dummy-key-content"), 0600); err != nil {
-		t.Fatalf("Failed to write key file: %v", err)
-	}
+				if err := os.WriteFile(certFile.Name(), []byte("dummy-cert-content"), 0600); err != nil {
+					t.Fatalf("Failed to write cert file: %v", err)
+				}
+				if err := os.WriteFile(keyFile.Name(), []byte("dummy-key-content"), 0600); err != nil {
+					t.Fatalf("Failed to write key file: %v", err)
+				}
 
-	baseConfig := clientcmdapi.Config{
-		Clusters: map[string]*clientcmdapi.Cluster{
-			"test-cluster": {
-				Server:                   "https://example.com:6443",
-				CertificateAuthorityData: []byte("fake-ca-data"),
+				baseConfig := clientcmdapi.Config{
+					Clusters: map[string]*clientcmdapi.Cluster{
+						"test-cluster": {
+							Server:                   "https://example.com:6443",
+							CertificateAuthorityData: []byte("fake-ca-data"),
+						},
+					},
+					AuthInfos: map[string]*clientcmdapi.AuthInfo{
+						"test-user": {
+							ClientCertificateData: []byte("base-cert-data"),
+							ClientKeyData:         []byte("base-key-data"),
+						},
+					},
+					Contexts: map[string]*clientcmdapi.Context{
+						"test-context": {
+							Cluster:  "test-cluster",
+							AuthInfo: "test-user",
+						},
+					},
+					CurrentContext: "test-context",
+				}
+
+				overrides := &ConfigOverrides{
+					AuthInfo: clientcmdapi.AuthInfo{
+						ClientCertificate:     certFile.Name(),
+						ClientCertificateData: nil,
+						ClientKey:             keyFile.Name(),
+						ClientKeyData:         nil,
+					},
+				}
+
+				cleanup := func() {
+					utiltesting.CloseAndRemove(t, certFile)
+					utiltesting.CloseAndRemove(t, keyFile)
+				}
+
+				return &baseConfig, overrides, cleanup
+			},
+			validate: func(t *testing.T, authInfo *clientcmdapi.AuthInfo) {
+				if authInfo.ClientCertificate == "" {
+					t.Errorf("Expected ClientCertificate file path to be set")
+				}
+				if authInfo.ClientKey == "" {
+					t.Errorf("Expected ClientKey file path to be set")
+				}
+				if authInfo.ClientCertificateData != nil {
+					t.Errorf("Expected ClientCertificateData to be nil when file path is used")
+				}
+				if authInfo.ClientKeyData != nil {
+					t.Errorf("Expected ClientKeyData to be nil when file path is used")
+				}
 			},
 		},
-		AuthInfos: map[string]*clientcmdapi.AuthInfo{
-			"test-user": {
-				ClientCertificateData: []byte("base-cert-data"),
-				ClientKeyData:         []byte("base-key-data"),
+		{
+			name:        "override-with-data-fields",
+			description: "Test override with cert/key data fields",
+			setupTest: func(t *testing.T) (*clientcmdapi.Config, *ConfigOverrides, func()) {
+				baseConfig := clientcmdapi.Config{
+					Clusters: map[string]*clientcmdapi.Cluster{
+						"test-cluster": {
+							Server:                   "https://example.com:6443",
+							CertificateAuthorityData: []byte("fake-ca-data"),
+						},
+					},
+					AuthInfos: map[string]*clientcmdapi.AuthInfo{
+						"test-user": {
+							ClientCertificate: "/path/to/base-cert.pem",
+							ClientKey:         "/path/to/base-key.pem",
+						},
+					},
+					Contexts: map[string]*clientcmdapi.Context{
+						"test-context": {
+							Cluster:  "test-cluster",
+							AuthInfo: "test-user",
+						},
+					},
+					CurrentContext: "test-context",
+				}
+
+				overrides := &ConfigOverrides{
+					AuthInfo: clientcmdapi.AuthInfo{
+						ClientCertificate:     "",
+						ClientCertificateData: []byte("override-cert-data"),
+						ClientKey:             "",
+						ClientKeyData:         []byte("override-key-data"),
+					},
+				}
+
+				return &baseConfig, overrides, func() {}
+			},
+			validate: func(t *testing.T, authInfo *clientcmdapi.AuthInfo) {
+				if authInfo.ClientCertificate != "" {
+					t.Errorf("Expected ClientCertificate file path to be empty when data is used")
+				}
+				if authInfo.ClientKey != "" {
+					t.Errorf("Expected ClientKey file path to be empty when data is used")
+				}
+				if string(authInfo.ClientCertificateData) != "override-cert-data" {
+					t.Errorf("Expected ClientCertificateData to be 'override-cert-data', got %s", string(authInfo.ClientCertificateData))
+				}
+				if string(authInfo.ClientKeyData) != "override-key-data" {
+					t.Errorf("Expected ClientKeyData to be 'override-key-data', got %s", string(authInfo.ClientKeyData))
+				}
 			},
 		},
-		Contexts: map[string]*clientcmdapi.Context{
-			"test-context": {
-				Cluster:  "test-cluster",
-				AuthInfo: "test-user",
-			},
-		},
-		CurrentContext: "test-context",
 	}
 
-	overrides := &ConfigOverrides{
-		AuthInfo: clientcmdapi.AuthInfo{
-			ClientCertificate:     certFile.Name(),
-			ClientCertificateData: nil,
-			ClientKey:             keyFile.Name(),
-			ClientKeyData:         nil,
-		},
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			baseConfig, overrides, cleanup := tc.setupTest(t)
+			defer cleanup()
+
+			clientConfig := NewNonInteractiveClientConfig(*baseConfig, "test-context", overrides, nil)
+
+			mergedConfig, err := clientConfig.MergedRawConfig()
+			if err != nil {
+				t.Fatalf("MergedRawConfig() failed: %v", err)
+			}
+
+			authInfo := mergedConfig.AuthInfos["test-user"]
+			if authInfo == nil {
+				t.Fatalf("Expected AuthInfo 'test-user' not found")
+			}
+
+			tc.validate(t, authInfo)
+		})
 	}
-
-	clientConfig := NewNonInteractiveClientConfig(baseConfig, "test-context", overrides, nil)
-
-	mergedConfig, err := clientConfig.MergedRawConfig()
-	if err != nil {
-		t.Fatalf("MergedRawConfig() failed: %v", err)
-	}
-
-	authInfo := mergedConfig.AuthInfos["test-user"]
-	if authInfo == nil {
-		t.Fatalf("Expected AuthInfo 'test-user' not found")
-	}
-
-	matchStringArg(certFile.Name(), authInfo.ClientCertificate, t)
-	matchStringArg(keyFile.Name(), authInfo.ClientKey, t)
-	matchByteArg(nil, authInfo.ClientCertificateData, t)
-	matchByteArg(nil, authInfo.ClientKeyData, t)
 }