From 11c8dba350a07f842fe040aa527596d3d9e05cac Mon Sep 17 00:00:00 2001 From: Vincent de Phily Date: Thu, 7 May 2026 17:00:38 +0100 Subject: [PATCH 1/2] fix: Keep TLSSecurityProfile cipher order when injecting TLS The cipher order matters, as it is used during TLS handshake to determine the prefered cipher. Don't sort lexicographically. --- lib/resourcebuilder/core.go | 3 -- lib/resourcebuilder/core_test.go | 47 ++++++++++++++++---------------- 2 files changed, 23 insertions(+), 27 deletions(-) diff --git a/lib/resourcebuilder/core.go b/lib/resourcebuilder/core.go index ce326b684..fa296a6df 100644 --- a/lib/resourcebuilder/core.go +++ b/lib/resourcebuilder/core.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "sort" "sigs.k8s.io/kustomize/kyaml/yaml" @@ -145,8 +144,6 @@ func (b *builder) observeTLSConfiguration(ctx context.Context, cm *corev1.Config if cipherSuites, ciphersFound, err := unstructured.NestedStringSlice(observedConfig, "servingInfo", "cipherSuites"); err != nil { return nil, err } else if ciphersFound { - // Sort cipher suites for consistent ordering - sort.Strings(cipherSuites) config.cipherSuites = optional[[]string]{value: cipherSuites, found: true} } diff --git a/lib/resourcebuilder/core_test.go b/lib/resourcebuilder/core_test.go index 2d58b7e16..bd62354be 100644 --- a/lib/resourcebuilder/core_test.go +++ b/lib/resourcebuilder/core_test.go @@ -6,7 +6,6 @@ import ( "encoding/json" "fmt" "reflect" - "sort" "testing" "text/template" @@ -77,10 +76,10 @@ func makeGenericControllerConfigYAML(cipherSuites []string, minTLSVersion string return makeGenericConfigYAML(configv1.GroupVersion.String(), "GenericControllerConfig", cipherSuites, minTLSVersion) } -func getDefaultCipherSuitesSorted() []string { - cipherSuites := crypto.CipherSuitesToNamesOrDie(crypto.DefaultCiphers()) - sort.Strings(cipherSuites) - return cipherSuites +// Returns the same default cipher suite as apiserver.ObserveTLSSecurityProfile() +func getDefaultCipherSuites() []string { + profile := configv1.TLSProfiles[crypto.DefaultTLSProfileType] + return crypto.OpenSSLToIANACipherSuites(profile.Ciphers) } const ( @@ -99,35 +98,37 @@ const ( ) var ( - // testCipherSuites is a common cipher suite list used across multiple test cases + // Common cipher suite used across multiple test cases testCipherSuites = []string{ "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", } - - testCipherSuites2 = []string{ - "TLS_RSA_WITH_AES_128_CBC_SHA256", - "TLS_RSA_WITH_AES_128_GCM_SHA256", - "TLS_RSA_WITH_AES_256_GCM_SHA384", - } - + // Same as previous, but using OpenSSL ids testOpenSSLCipherSuites = []string{ "ECDHE-ECDSA-AES128-GCM-SHA256", "ECDHE-RSA-AES128-GCM-SHA256", } - + // Same as previous, but in a different order testOpenSSLCipherSuitesReversed = []string{ "ECDHE-RSA-AES128-GCM-SHA256", "ECDHE-ECDSA-AES128-GCM-SHA256", } + // Alternate cipher suite + testCipherSuites2 = []string{ + "TLS_RSA_WITH_AES_128_CBC_SHA256", + "TLS_RSA_WITH_AES_128_GCM_SHA256", + "TLS_RSA_WITH_AES_256_GCM_SHA384", + } + // Same as previous, but using OpenSSL ids testOpenSSLCipherSuites2 = []string{ + "AES128-SHA256", "AES128-GCM-SHA256", "AES256-GCM-SHA384", - "AES128-SHA256", } - defaultCipherSuites = getDefaultCipherSuitesSorted() + // Default cipher suite returned bu library-go + defaultCipherSuites = getDefaultCipherSuites() ) // makeConfigMap is a helper to create a ConfigMap with optional annotation and data @@ -273,8 +274,6 @@ func validateGenericConfigTLSInjected(modified *corev1.ConfigMap, fieldName stri return fmt.Errorf("servingInfo.cipherSuites not found") } - sort.Strings(expectedCiphers) - sort.Strings(cipherSuites) if diff := cmp.Diff(expectedCiphers, cipherSuites); diff != "" { return fmt.Errorf("list of ciphers mismatch (-want +got):\n%s", diff) } @@ -563,16 +562,16 @@ servingInfo: }, }, { - name: "ConfigMap with TLS already injected - same ciphers in different order with no modification", + // Cipher order matters for TLS preference, so different order should trigger update + name: "ConfigMap with TLS already injected - same ciphers in different order - TLS injected with correct order", configMap: makeConfigMap(true, map[string]string{ - genericOperatorConfigCMKey: makeGenericOperatorConfigYAML(testCipherSuites, tlsVersion13), + genericOperatorConfigCMKey: makeGenericOperatorConfigYAML(testOpenSSLCipherSuitesReversed, tlsVersion13), }), - // Reverse the cipher order to test that same ciphers in different order don't trigger update - apiServer: makeAPIServerConfig(withCustomTLSProfile(testOpenSSLCipherSuitesReversed, configv1.VersionTLS13)), + apiServer: makeAPIServerConfig(withCustomTLSProfile(testOpenSSLCipherSuites, configv1.VersionTLS13)), expectError: false, validateConfigMap: func(t *testing.T, original, modified *corev1.ConfigMap) { - if diff := cmp.Diff(original.Data[genericOperatorConfigCMKey], modified.Data[genericOperatorConfigCMKey]); diff != "" { - t.Errorf("ConfigMap YAML mismatch (-want +got):\n%s", diff) + if err := validateGenericOperatorConfigTLSInjected(modified, genericOperatorConfigCMKey, testCipherSuites, tlsVersion13); err != nil { + t.Fatalf("validation failed: %v", err) } }, }, From 4efff84d3d50c9bdc053ac07cdaa3f0a5aaf165c Mon Sep 17 00:00:00 2001 From: Vincent de Phily Date: Thu, 7 May 2026 17:02:41 +0100 Subject: [PATCH 2/2] qa: Log the actual NodeKind during tls injection --- lib/resourcebuilder/core.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/resourcebuilder/core.go b/lib/resourcebuilder/core.go index fa296a6df..42c1d51ed 100644 --- a/lib/resourcebuilder/core.go +++ b/lib/resourcebuilder/core.go @@ -82,15 +82,16 @@ func (b *builder) modifyConfigMap(ctx context.Context, cm *corev1.ConfigMap) err } // Check if this is a supported config kind + rnodeKind := rnode.GetKind() switch { - case rnode.GetKind() == "GenericOperatorConfig" && rnode.GetApiVersion() == operatorv1alpha1.GroupVersion.String(): - case rnode.GetKind() == "GenericControllerConfig" && rnode.GetApiVersion() == configv1.GroupVersion.String(): + case rnodeKind == "GenericOperatorConfig" && rnode.GetApiVersion() == operatorv1alpha1.GroupVersion.String(): + case rnodeKind == "GenericControllerConfig" && rnode.GetApiVersion() == configv1.GroupVersion.String(): default: klog.V(4).Infof("ConfigMap's %q entry is not a supported config type. Only GenericOperatorConfig (%v) and GenericControllerConfig (%v) are. Skipping this entry", key, operatorv1alpha1.GroupVersion.String(), configv1.GroupVersion.String()) continue } - klog.V(2).Infof("ConfigMap %s/%s processing GenericOperatorConfig in key %s", cm.Namespace, cm.Name, key) + klog.V(2).Infof("ConfigMap %s/%s processing %s in key %s", cm.Namespace, cm.Name, rnodeKind, key) // Inject TLS settings into the GenericOperatorConfig while preserving structure if err := updateRNodeWithTLSSettings(rnode, tlsConf); err != nil { @@ -105,7 +106,7 @@ func (b *builder) modifyConfigMap(ctx context.Context, cm *corev1.ConfigMap) err // Update the ConfigMap data entry with the modified YAML cm.Data[key] = modifiedYAML - klog.V(2).Infof("ConfigMap %s/%s updated GenericOperatorConfig with TLS profile in key %s", cm.Namespace, cm.Name, key) + klog.V(2).Infof("ConfigMap %s/%s updated TLS profile of %s in key %s", cm.Namespace, cm.Name, rnodeKind, key) } return nil }