diff --git a/tools/clientcmd/client_config.go b/tools/clientcmd/client_config.go index e41fa46c1..ed35891e5 100644 --- a/tools/clientcmd/client_config.go +++ b/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 " - 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 " - 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 } } diff --git a/tools/clientcmd/client_config_test.go b/tools/clientcmd/client_config_test.go index 593d4b016..baa406f4e 100644 --- a/tools/clientcmd/client_config_test.go +++ b/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) }