From 42a8434aff36201ecc27b6fab4b3928b28e31d56 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Fri, 17 Jul 2026 08:13:09 -0400 Subject: [PATCH 01/13] Convert OpenSSL cipher names to IANA and filter TLS 1.3 ciphers This commit enhances the TLS profile handling to support different cipher name formats and TLS version requirements: 1. TLS 1.3 cipher suite handling: - Clear cipher list when MinTLSVersion is TLS 1.3 - TLS 1.3 cipher suites are not configurable in Go (all are enabled) - Some components fail if ciphers are provided with TLS 1.3 2. Cipher name conversion: - Convert OpenSSL cipher names to IANA names - OCP pre-defined profiles use OpenSSL names - Components accepting cipher suite args expect IANA names - Support both formats in custom user profiles Also updated tests to verify the cipher name conversion logic handles both IANA and OpenSSL formatted cipher names correctly. Signed-off-by: Tom Pantelis --- pkg/network/bootstrap_test.go | 18 +++++++++++------- pkg/network/tls.go | 26 ++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/pkg/network/bootstrap_test.go b/pkg/network/bootstrap_test.go index 869d78b1cc..52da3f0534 100644 --- a/pkg/network/bootstrap_test.go +++ b/pkg/network/bootstrap_test.go @@ -64,8 +64,11 @@ func TestBootstrap(t *testing.T) { Type: configv1.TLSProfileCustomType, Custom: &configv1.CustomTLSProfile{ TLSProfileSpec: configv1.TLSProfileSpec{ - MinTLSVersion: configv1.VersionTLS13, - Ciphers: []string{"TLS_AES_128_GCM_SHA256"}, + MinTLSVersion: configv1.VersionTLS11, + Ciphers: []string{ + "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", // IANA name + "ECDHE-ECDSA-CHACHA20-POLY1305", // OpenSSL name + }, }, }, }, @@ -83,12 +86,12 @@ func TestBootstrap(t *testing.T) { t.Fatal("Bootstrap result is nil") } - if result.TLSProfile.Spec.MinTLSVersion != configv1.VersionTLS13 { + if result.TLSProfile.Spec.MinTLSVersion != configv1.VersionTLS11 { t.Errorf("Expected MinTLSVersion %v, got %v", - configv1.VersionTLS13, result.TLSProfile.Spec.MinTLSVersion) + configv1.VersionTLS11, result.TLSProfile.Spec.MinTLSVersion) } - expectedCiphers := []string{"TLS_AES_128_GCM_SHA256"} + expectedCiphers := []string{"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256"} if !reflect.DeepEqual(result.TLSProfile.Spec.Ciphers, expectedCiphers) { t.Errorf("Expected ciphers %v, got %v", expectedCiphers, result.TLSProfile.Spec.Ciphers) @@ -162,8 +165,9 @@ func TestBootstrap(t *testing.T) { configv1.VersionTLS13, result.TLSProfile.Spec.MinTLSVersion) } - if len(result.TLSProfile.Spec.Ciphers) == 0 { - t.Error("Expected ciphers to not be empty") + // TLS 1.3 cipher suites should be filtered out since they're not configurable in Go + if len(result.TLSProfile.Spec.Ciphers) != 0 { + t.Errorf("Expected ciphers to be empty for TLS 1.3, got %v", result.TLSProfile.Spec.Ciphers) } if result.TLSProfile.Adherence != configv1.TLSAdherencePolicyStrictAllComponents { diff --git a/pkg/network/tls.go b/pkg/network/tls.go index 1ba5ac1736..00a48bf4b5 100644 --- a/pkg/network/tls.go +++ b/pkg/network/tls.go @@ -61,6 +61,32 @@ func toTLSProfile(apiServerSpec *configv1.APIServerSpec) (bootstrap.TLSProfile, return bootstrap.TLSProfile{}, fmt.Errorf("failed to get TLS profile spec: %w", err) } + // TLS 1.3 cipher suites are not configurable in Go - all TLS 1.3 ciphers are always enabled. + // Clear the cipher list for TLS 1.3 as some components are strict and fail if ciphers are provided with min version 1.3. + if profileSpec.MinTLSVersion == configv1.VersionTLS13 { + profileSpec.Ciphers = nil + } + + // OCP uses OpenSSL names in its pre-defined TLS profile specs (although it's possible a user-defined custom profile + // spec could have IANA names). The components that accept cipher suites as an arg expect IANA names so convert them. + var convertedCiphers []string + for _, cipher := range profileSpec.Ciphers { + // First try as IANA name directly. + if _, err := crypto.CipherSuite(cipher); err == nil { + convertedCiphers = append(convertedCiphers, cipher) + continue + } + + // Try converting from OpenSSL name to IANA name. + ianaCiphers := crypto.OpenSSLToIANACipherSuites([]string{cipher}) + if len(ianaCiphers) > 0 { + convertedCiphers = append(convertedCiphers, ianaCiphers...) + } else { + convertedCiphers = append(convertedCiphers, cipher) + } + } + profileSpec.Ciphers = convertedCiphers + return bootstrap.TLSProfile{ Spec: profileSpec, Adherence: apiServerSpec.TLSAdherence, From c617131b92f33d65a9c0938cc111a6a7821ccea4 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Fri, 17 Jul 2026 08:23:07 -0400 Subject: [PATCH 02/13] Add test utility functions for component rendering tests This commit adds helper functions to testutil_test.go that are used by component rendering tests to verify rendered Kubernetes objects: - mustFindRenderedObj: Finds and converts unstructured objects using generics - mustFindContainer: Finds a container by name in a container list - findExecCommand: Extracts exec command strings from container command args These utilities facilitate testing of rendered manifests and container configurations across multiple components. Signed-off-by: Tom Pantelis --- pkg/network/testutil_test.go | 49 ++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/pkg/network/testutil_test.go b/pkg/network/testutil_test.go index 3a71c075ca..413f7c872e 100644 --- a/pkg/network/testutil_test.go +++ b/pkg/network/testutil_test.go @@ -3,14 +3,20 @@ package network import ( "context" "fmt" + "slices" + "strings" + "testing" + . "github.com/onsi/gomega" "github.com/onsi/gomega/types" configv1 "github.com/openshift/api/config/v1" "github.com/openshift/cluster-network-operator/pkg/bootstrap" "github.com/openshift/cluster-network-operator/pkg/client" "github.com/openshift/cluster-network-operator/pkg/hypershift" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" uns "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" ) // Fun matcher for testing the presence of Kubernetes objects @@ -108,3 +114,46 @@ func createProxy(client client.Client) error { } return client.Default().CRClient().Create(context.TODO(), proxy) } + +// mustFindRenderedObj finds and converts an unstructured object from a list of rendered objects. +// It uses Go generics to return the properly typed object. +func mustFindRenderedObj[T any](t *testing.T, objs []*uns.Unstructured, kind, name string) T { + t.Helper() + g := NewWithT(t) + + index := slices.IndexFunc(objs, func(obj *uns.Unstructured) bool { + return obj.GetKind() == kind && obj.GetName() == name + }) + g.Expect(index).NotTo(Equal(-1), "Could not find object with kind %q and name %q", kind, name) + + var result T + err := runtime.DefaultUnstructuredConverter.FromUnstructured(objs[index].Object, &result) + g.Expect(err).NotTo(HaveOccurred()) + + return result +} + +// mustFindContainer finds a container by name in a list of containers. +func mustFindContainer(t *testing.T, in []corev1.Container, name string) *corev1.Container { + t.Helper() + g := NewWithT(t) + + c, ok := findContainer(in, name) + g.Expect(ok).To(BeTrue(), "Could not find container with name %q", name) + + return &c +} + +// findExecCommand finds and returns the exec command for the given binary in container command args. +// It expects cmdArgs to have length 3 (bash, -c, script) and returns the portion from "exec /usr/bin/" onwards. +func findExecCommand(t *testing.T, cmdArgs []string, binaryName string) string { + t.Helper() + g := NewWithT(t) + + g.Expect(cmdArgs).To(HaveLen(3)) + execPattern := "exec /usr/bin/" + binaryName + startIdx := strings.Index(cmdArgs[2], execPattern) + g.Expect(startIdx).NotTo(Equal(-1), "Could not find '%s' in command args: %q", execPattern, cmdArgs[2]) + + return cmdArgs[2][startIdx:] +} From a211f70cb9463903a6e416dfe915ad57ab1c5c4e Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Wed, 10 Jun 2026 18:45:08 -0400 Subject: [PATCH 03/13] Use TLS profile to render CLI args in the node identity webhook This commit adds rendering of TLS CLI args based on the TLS profile to the webhook container command of the network-node-identity Deployment for both managed and self-hosted environments. Changes: - Add TLS profile configuration to renderNetworkNodeIdentity - Update yaml manifests to include TLS CLI arguments when configured Also added unit tests for the renderNetworkNodeIdentity function to verify correct rendering of the DaemonSet (self-hosted) and the Deployment (HyperShift) configurations, TLS argument handling based on adherence policies, and proper behavior when network node identity is disabled. Signed-off-by: Tom Pantelis --- .../node-identity/managed/node-identity.yaml | 6 + .../self-hosted/node-identity.yaml | 9 +- pkg/network/node_identity.go | 2 + pkg/network/node_identity_test.go | 242 ++++++++++++++++++ 4 files changed, 258 insertions(+), 1 deletion(-) create mode 100644 pkg/network/node_identity_test.go diff --git a/bindata/network/node-identity/managed/node-identity.yaml b/bindata/network/node-identity/managed/node-identity.yaml index 94440ec532..086e4b78ee 100644 --- a/bindata/network/node-identity/managed/node-identity.yaml +++ b/bindata/network/node-identity/managed/node-identity.yaml @@ -145,6 +145,12 @@ spec: --disable-approver \ --extra-allowed-user="system:serviceaccount:openshift-ovn-kubernetes:ovn-kubernetes-control-plane" \ --pod-admission-conditions="/var/run/ovnkube-identity-config/additional-pod-admission-cond.json" \ + {{- if .UseTLSProfile }} + --tls-min-version={{.TLSMinVersion}} \ + {{- if .TLSCipherSuites }} + --tls-cipher-suites={{.TLSCipherSuites}} \ + {{- end }} + {{- end }} --loglevel="${LOGLEVEL}" env: - name: LOGLEVEL diff --git a/bindata/network/node-identity/self-hosted/node-identity.yaml b/bindata/network/node-identity/self-hosted/node-identity.yaml index 9d028e9592..0512c58c16 100644 --- a/bindata/network/node-identity/self-hosted/node-identity.yaml +++ b/bindata/network/node-identity/self-hosted/node-identity.yaml @@ -57,7 +57,8 @@ spec: echo "I$(date "+%m%d %H:%M:%S.%N") - network-node-identity - start webhook" # extra-allowed-user: service account `ovn-kubernetes-control-plane` # sets pod annotations in multi-homing layer3 network controller (cluster-manager) - exec /usr/bin/ovnkube-identity --k8s-apiserver={{.K8S_APISERVER}} \ + exec /usr/bin/ovnkube-identity \ + --k8s-apiserver={{.K8S_APISERVER}} \ --webhook-cert-dir="/etc/webhook-cert" \ --webhook-host={{.NetworkNodeIdentityIP}} \ --webhook-port={{.NetworkNodeIdentityPort}} \ @@ -67,6 +68,12 @@ spec: --extra-allowed-user="system:serviceaccount:openshift-ovn-kubernetes:ovn-kubernetes-control-plane" \ --wait-for-kubernetes-api={{.NetworkNodeIdentityTerminationDurationSeconds}}s \ --pod-admission-conditions="/var/run/ovnkube-identity-config/additional-pod-admission-cond.json" \ + {{- if .UseTLSProfile }} + --tls-min-version={{.TLSMinVersion}} \ + {{- if .TLSCipherSuites }} + --tls-cipher-suites={{.TLSCipherSuites}} \ + {{- end }} + {{- end }} --loglevel="${LOGLEVEL}" env: - name: LOGLEVEL diff --git a/pkg/network/node_identity.go b/pkg/network/node_identity.go index cec3c7b5e2..3d443b6aa4 100644 --- a/pkg/network/node_identity.go +++ b/pkg/network/node_identity.go @@ -61,6 +61,8 @@ func renderNetworkNodeIdentity(conf *operv1.NetworkSpec, bootstrapResult *bootst } data.Data["NetworkNodeIdentityPort"] = NetworkNodeIdentityWebhookPort + addTLSInfoToRenderData(data.Data, bootstrapResult, true) + manifestDirs := make([]string, 0, 2) manifestDirs = append(manifestDirs, filepath.Join(manifestDir, "network/node-identity/common")) diff --git a/pkg/network/node_identity_test.go b/pkg/network/node_identity_test.go new file mode 100644 index 0000000000..4fbb033284 --- /dev/null +++ b/pkg/network/node_identity_test.go @@ -0,0 +1,242 @@ +package network + +import ( + "fmt" + "net" + "strings" + "testing" + + . "github.com/onsi/gomega" + configv1 "github.com/openshift/api/config/v1" + operv1 "github.com/openshift/api/operator/v1" + "github.com/openshift/cluster-network-operator/pkg/bootstrap" + cnoclient "github.com/openshift/cluster-network-operator/pkg/client" + cnofake "github.com/openshift/cluster-network-operator/pkg/client/fake" + "github.com/openshift/cluster-network-operator/pkg/hypershift" + appsv1 "k8s.io/api/apps/v1" + uns "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/utils/ptr" +) + +const ( + tlsMinVersionArg = "--tls-min-version" + tlsCipherSuitesArg = "--tls-cipher-suites" +) + +func TestRenderNetworkNodeIdentity(t *testing.T) { + const ( + ovnImage = "test-ovn-image" + releaseVersion = "5.0.0" + ovnCtrlPlaneImage = "test-ovn-ctrl-plane-image" + tokenMinterImage = "test-token-minter-image" + tokenAudience = "test-token-audience" + hostedClusterNS = "hosted-cluster-ns" + ) + + setupTest := func(t *testing.T) (*operv1.NetworkSpec, *bootstrap.BootstrapResult, cnoclient.Client) { + networkConfig := &operv1.NetworkSpec{ + ServiceNetwork: []string{"172.30.0.0/16"}, + ClusterNetwork: []operv1.ClusterNetworkEntry{ + { + CIDR: "10.128.0.0/15", + HostPrefix: 23, + }, + }, + DefaultNetwork: operv1.DefaultNetworkDefinition{ + Type: operv1.NetworkTypeOVNKubernetes, + OVNKubernetesConfig: &operv1.OVNKubernetesConfig{}, + }, + } + + bootstrapResult := fakeBootstrapResult() + bootstrapResult.Infra.NetworkNodeIdentityEnabled = true + bootstrapResult.TLSProfile = bootstrap.TLSProfile{ + Spec: configv1.TLSProfileSpec{ + MinTLSVersion: configv1.VersionTLS12, + Ciphers: []string{"TLS_AES_128_GCM_SHA256", "TLS_AES_256_GCM_SHA384"}, + }, + Adherence: configv1.TLSAdherencePolicyLegacyAdheringComponentsOnly, + } + + // Set required environment variables + t.Setenv("RELEASE_VERSION", releaseVersion) + t.Setenv("OVN_IMAGE", ovnImage) + + client := cnofake.NewFakeClient() + + return networkConfig, bootstrapResult, client + } + + assertRenderSuccess := func(t *testing.T, networkConfig *operv1.NetworkSpec, bootstrapResult *bootstrap.BootstrapResult, + client cnoclient.Client) []*uns.Unstructured { + g := NewWithT(t) + objs, err := renderNetworkNodeIdentity(networkConfig, bootstrapResult, manifestDir, client) + g.Expect(err).NotTo(HaveOccurred()) + + return objs + } + + t.Run("should successfully render the network-node-identity DaemonSet", func(t *testing.T) { + g := NewWithT(t) + networkConfig, bootstrapResult, client := setupTest(t) + daemonSet := mustFindRenderedObj[*appsv1.DaemonSet](t, assertRenderSuccess(t, networkConfig, bootstrapResult, client), + "DaemonSet", "network-node-identity") + container := mustFindContainer(t, daemonSet.Spec.Template.Spec.Containers, "webhook") + execStr := findOvnkubeIdentityExec(t, container.Command) + apiserverArg := "--k8s-apiserver=https://" + net.JoinHostPort(bootstrapResult.Infra.APIServers[bootstrap.APIServerDefault].Host, + bootstrapResult.Infra.APIServers[bootstrap.APIServerDefault].Port) + + g.Expect(execStr).To(ContainSubstring(apiserverArg)) + g.Expect(execStr).To(ContainSubstring("--webhook-host=127.0.0.1")) + g.Expect(execStr).To(ContainSubstring("--webhook-port=" + NetworkNodeIdentityWebhookPort)) + g.Expect(container.Image).To(Equal(ovnImage)) + g.Expect(daemonSet.Annotations["release.openshift.io/version"]).To(Equal(releaseVersion)) + + container = mustFindContainer(t, daemonSet.Spec.Template.Spec.Containers, "approver") + execStr = findOvnkubeIdentityExec(t, container.Command) + g.Expect(execStr).To(ContainSubstring(apiserverArg)) + }) + + testTLSArgRendering(t, "webhook ovnkube-identity", "", "", func(t *testing.T, tlsProfile bootstrap.TLSProfile) string { + networkConfig, bootstrapResult, client := setupTest(t) + bootstrapResult.TLSProfile = tlsProfile + daemonSet := mustFindRenderedObj[*appsv1.DaemonSet](t, assertRenderSuccess(t, networkConfig, bootstrapResult, client), + "DaemonSet", "network-node-identity") + return findOvnkubeIdentityExec(t, mustFindContainer(t, daemonSet.Spec.Template.Spec.Containers, "webhook").Command) + }) + + t.Run("HyperShift enabled", func(t *testing.T) { + networkConfig, bootstrapResult, client := setupTest(t) + + // Set HyperShift environment variables + t.Setenv("HYPERSHIFT", "true") + t.Setenv("HOSTED_CLUSTER_NAME", "test-cluster") + t.Setenv("HOSTED_CLUSTER_NAMESPACE", hostedClusterNS) + t.Setenv("OVN_CONTROL_PLANE_IMAGE", ovnCtrlPlaneImage) + t.Setenv("CLI_IMAGE", "quay.io/openshift/cli:latest") + t.Setenv("TOKEN_MINTER_IMAGE", tokenMinterImage) + t.Setenv("TOKEN_AUDIENCE", tokenAudience) + + bootstrapResult.Infra.HostedControlPlane = &hypershift.HostedControlPlane{ + ControllerAvailabilityPolicy: hypershift.HighlyAvailable, + NodeSelector: map[string]string{"node-selector-key": "node-selector-value"}, + Labels: map[string]string{"hypershift.openshift.io/cluster": "test"}, + PriorityClass: "hypershift-control-plane", + } + + // Add local API server for HyperShift + bootstrapResult.Infra.APIServers[bootstrap.APIServerDefaultLocal] = bootstrap.APIServer{ + Host: "kube-apiserver", + Port: "6443", + } + + t.Run("should successfully render the network-node-identity Deployment", func(t *testing.T) { + g := NewWithT(t) + deployment := mustFindRenderedObj[*appsv1.Deployment](t, assertRenderSuccess(t, networkConfig, bootstrapResult, client), + "Deployment", "network-node-identity") + container := mustFindContainer(t, deployment.Spec.Template.Spec.Containers, "webhook") + execStr := findOvnkubeIdentityExec(t, container.Command) + + g.Expect(execStr).To(ContainSubstring("--webhook-port=" + NetworkNodeIdentityWebhookPort)) + g.Expect(container.Image).To(Equal(ovnCtrlPlaneImage)) + g.Expect(deployment.Annotations["release.openshift.io/version"]).To(Equal(releaseVersion)) + g.Expect(deployment.Labels["hypershift.openshift.io/hosted-control-plane"]).To(Equal(hostedClusterNS)) + g.Expect(ptr.Deref(deployment.Spec.Replicas, 0)).To(Equal(int32(3))) + g.Expect(deployment.Spec.Strategy.Type).To(Equal(appsv1.RollingUpdateDeploymentStrategyType)) + g.Expect(deployment.Spec.Template.Spec.Affinity.PodAntiAffinity).NotTo(BeNil()) + g.Expect(deployment.Spec.Template.Spec.PriorityClassName).To(Equal(bootstrapResult.Infra.HostedControlPlane.PriorityClass)) + g.Expect(deployment.Spec.Template.Spec.NodeSelector).To(Equal(bootstrapResult.Infra.HostedControlPlane.NodeSelector)) + + mustFindContainer(t, deployment.Spec.Template.Spec.Containers, "approver") + + container = mustFindContainer(t, deployment.Spec.Template.Spec.Containers, "token-minter") + g.Expect(container.Image).To(Equal(tokenMinterImage)) + expectedArg := "--token-audience=" + tokenAudience + g.Expect(container.Args).To(ContainElement(expectedArg)) + }) + + testTLSArgRendering(t, "webhook ovnkube-identity", "", "", func(t *testing.T, tlsProfile bootstrap.TLSProfile) string { + bootstrapResult.TLSProfile = tlsProfile + deployment := mustFindRenderedObj[*appsv1.Deployment](t, assertRenderSuccess(t, networkConfig, bootstrapResult, client), + "Deployment", "network-node-identity") + return findOvnkubeIdentityExec(t, mustFindContainer(t, deployment.Spec.Template.Spec.Containers, "webhook").Command) + }) + }) + + t.Run("NetworkNodeIdentity is disabled", func(t *testing.T) { + g := NewWithT(t) + networkConfig, bootstrapResult, client := setupTest(t) + bootstrapResult.Infra.NetworkNodeIdentityEnabled = false + + objs := assertRenderSuccess(t, networkConfig, bootstrapResult, client) + g.Expect(objs).To(BeEmpty()) + }) +} + +func testTLSArgRendering(t *testing.T, name string, defaultMinVersion string, defaultCiphers string, getCommandStr func(*testing.T, bootstrap.TLSProfile) string) { + t.Run("when TLS profile adherence is LegacyAdheringComponentsOnly", func(t *testing.T) { + t.Run(fmt.Sprintf("should render the %s command with default TLS CLI args", name), func(t *testing.T) { + g := NewWithT(t) + tlsProfile := bootstrap.TLSProfile{ + Spec: configv1.TLSProfileSpec{ + MinTLSVersion: configv1.VersionTLS12, + Ciphers: []string{"TLS_AES_128_GCM_SHA256", "TLS_AES_256_GCM_SHA384"}, + }, + Adherence: configv1.TLSAdherencePolicyLegacyAdheringComponentsOnly, + } + + commandStr := getCommandStr(t, tlsProfile) + if defaultMinVersion != "" { + g.Expect(commandStr).To(ContainSubstring(tlsMinVersionArg + "=" + defaultMinVersion)) + } else { + g.Expect(commandStr).NotTo(ContainSubstring(tlsMinVersionArg)) + } + if defaultCiphers != "" { + g.Expect(commandStr).To(ContainSubstring(tlsCipherSuitesArg + "=" + defaultCiphers)) + } else { + g.Expect(commandStr).NotTo(ContainSubstring(tlsCipherSuitesArg)) + } + }) + }) + + t.Run("when TLS profile adherence is StrictAllComponents", func(t *testing.T) { + t.Run(fmt.Sprintf("should render the %s command with the TLS CLI args", name), func(t *testing.T) { + g := NewWithT(t) + tlsProfile := bootstrap.TLSProfile{ + Spec: configv1.TLSProfileSpec{ + MinTLSVersion: configv1.VersionTLS12, + Ciphers: []string{"TLS_AES_128_GCM_SHA256", "TLS_AES_256_GCM_SHA384"}, + }, + Adherence: configv1.TLSAdherencePolicyStrictAllComponents, + } + + commandStr := getCommandStr(t, tlsProfile) + expectedMinVersion := tlsMinVersionArg + "=" + string(tlsProfile.Spec.MinTLSVersion) + expectedCiphers := tlsCipherSuitesArg + "=" + strings.Join(tlsProfile.Spec.Ciphers, ",") + g.Expect(commandStr).To(ContainSubstring(expectedMinVersion)) + g.Expect(commandStr).To(ContainSubstring(expectedCiphers)) + }) + + t.Run("with empty cipher list", func(t *testing.T) { + t.Run(fmt.Sprintf("should not render the --tls-cipher-suites arg for the %s command", name), func(t *testing.T) { + g := NewWithT(t) + tlsProfile := bootstrap.TLSProfile{ + Spec: configv1.TLSProfileSpec{ + MinTLSVersion: configv1.VersionTLS13, + Ciphers: nil, + }, + Adherence: configv1.TLSAdherencePolicyStrictAllComponents, + } + + commandStr := getCommandStr(t, tlsProfile) + expectedMinVersion := tlsMinVersionArg + "=" + string(tlsProfile.Spec.MinTLSVersion) + g.Expect(commandStr).To(ContainSubstring(expectedMinVersion)) + g.Expect(commandStr).NotTo(ContainSubstring(tlsCipherSuitesArg)) + }) + }) + }) +} + +func findOvnkubeIdentityExec(t *testing.T, cmdArgs []string) string { + return findExecCommand(t, cmdArgs, "ovnkube-identity") +} From c4a3db15b27c99dc1428a2e379ee597bd6e06b2d Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Mon, 22 Jun 2026 10:18:32 -0400 Subject: [PATCH 04/13] Use TLS profile to render CLI args in the multus-admission-controller This commit adds rendering of TLS CLI args based on the TLS profile to the multus-admission-controller Deployment. Changes: - Add --tls-min-version and --tls-cipher-suites flags to the webhook container for both HyperShift and non-HyperShift deployments - Add --tls-min-version and --tls-cipher-suites flags to kube-rbac-proxy with fallback to hardcoded ciphers when TLS profile is not honored - Call addTLSInfoToRenderData in renderMultusAdmissonControllerConfig to populate TLS template data - Add comprehensive unit tests for TLS argument rendering in both HyperShift and non-HyperShift modes Signed-off-by: Tom Pantelis --- .../admission-controller.yaml | 13 ++++ pkg/network/multus_admission_controller.go | 2 + .../multus_admission_controller_test.go | 75 +++++++++++++++++-- 3 files changed, 82 insertions(+), 8 deletions(-) diff --git a/bindata/network/multus-admission-controller/admission-controller.yaml b/bindata/network/multus-admission-controller/admission-controller.yaml index 7d50dd8edc..01a947a0c9 100644 --- a/bindata/network/multus-admission-controller/admission-controller.yaml +++ b/bindata/network/multus-admission-controller/admission-controller.yaml @@ -166,6 +166,12 @@ spec: -metrics-listen-address=:9091 \ {{- else }} -metrics-listen-address=127.0.0.1:9091 \ +{{- end }} +{{- if .UseTLSProfile }} + --tls-min-version={{.TLSMinVersion}} \ +{{- if .TLSCipherSuites }} + --tls-cipher-suites={{.TLSCipherSuites}} \ +{{- end }} {{- end }} -alsologtostderr=true \ -ignore-namespaces=openshift-etcd,openshift-console,openshift-ingress-canary,{{.IgnoredNamespace}} @@ -198,7 +204,14 @@ spec: args: - --logtostderr - --secure-listen-address=:8443 +{{- if .UseTLSProfile }} + - --tls-min-version={{.TLSMinVersion}} +{{- if .TLSCipherSuites }} + - --tls-cipher-suites={{.TLSCipherSuites}} +{{- end }} +{{- else }} - --tls-cipher-suites=TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305 +{{- end }} - --upstream=http://127.0.0.1:9091/ - --tls-private-key-file=/etc/webhook/tls.key - --tls-cert-file=/etc/webhook/tls.crt diff --git a/pkg/network/multus_admission_controller.go b/pkg/network/multus_admission_controller.go index 05ff58b60b..19875c5573 100644 --- a/pkg/network/multus_admission_controller.go +++ b/pkg/network/multus_admission_controller.go @@ -145,6 +145,8 @@ func renderMultusAdmissonControllerConfig(manifestDir string, externalControlPla data.Data["ReleaseImage"] = hsc.ReleaseImage } + addTLSInfoToRenderData(data.Data, bootstrapResult, true) + manifests, err := render.RenderDir(filepath.Join(manifestDir, "network/multus-admission-controller"), &data) if err != nil { return nil, errors.Wrap(err, "failed to render multus admission controller manifests") diff --git a/pkg/network/multus_admission_controller_test.go b/pkg/network/multus_admission_controller_test.go index ad49e878bd..356dff7e83 100644 --- a/pkg/network/multus_admission_controller_test.go +++ b/pkg/network/multus_admission_controller_test.go @@ -1,16 +1,19 @@ package network import ( - "github.com/openshift/cluster-network-operator/pkg/hypershift" + "strings" "testing" . "github.com/onsi/gomega" operv1 "github.com/openshift/api/operator/v1" - "github.com/openshift/cluster-network-operator/pkg/names" - + "github.com/openshift/cluster-network-operator/pkg/bootstrap" cnofake "github.com/openshift/cluster-network-operator/pkg/client/fake" + "github.com/openshift/cluster-network-operator/pkg/hypershift" + "github.com/openshift/cluster-network-operator/pkg/names" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) var MultusAdmissionControllerConfig = operv1.Network{ @@ -59,17 +62,17 @@ func TestRenderMultusAdmissionController(t *testing.T) { }, }, }) - bootstrap := fakeBootstrapResult() + bootstrapResult := fakeBootstrapResult() // disable MultusAdmissionController - objs, err := renderMultusAdmissionController(config, manifestDir, false, bootstrap, fakeClient, getDefaultFeatureGates()) + objs, err := renderMultusAdmissionController(config, manifestDir, false, bootstrapResult, fakeClient, getDefaultFeatureGates()) g.Expect(err).NotTo(HaveOccurred()) g.Expect(objs).NotTo(ContainElement(HaveKubernetesID("Deployment", "openshift-multus", "multus-admission-controller"))) // enable MultusAdmissionController enabled := false config.DisableMultiNetwork = &enabled - objs, err = renderMultusAdmissionController(config, manifestDir, false, bootstrap, fakeClient, getDefaultFeatureGates()) + objs, err = renderMultusAdmissionController(config, manifestDir, false, bootstrapResult, fakeClient, getDefaultFeatureGates()) g.Expect(err).NotTo(HaveOccurred()) g.Expect(objs).To(ContainElement(HaveKubernetesID("Deployment", "openshift-multus", "multus-admission-controller"))) @@ -81,6 +84,33 @@ func TestRenderMultusAdmissionController(t *testing.T) { g.Expect(objs).To(ContainElement(HaveKubernetesID("ValidatingWebhookConfiguration", "", names.MULTUS_VALIDATING_WEBHOOK))) g.Expect(objs).To(ContainElement(HaveKubernetesID("Deployment", "openshift-multus", "multus-admission-controller"))) g.Expect(objs).To(ContainElement(HaveKubernetesID("NetworkPolicy", "openshift-multus", "multus-admission-controller"))) + + weboookCmd := findMultusWebhookExec(t, objs) + g.Expect(weboookCmd).To(ContainSubstring("-metrics-listen-address=127.0.0.1:9091")) + g.Expect(weboookCmd).NotTo(ContainSubstring("-encrypt-metrics")) + + // Test TLS rendering for webhook container + testTLSArgRendering(t, "multus-admission-controller webhook", "", "", func(t *testing.T, tlsProfile bootstrap.TLSProfile) string { + testBootstrap := *bootstrapResult + testBootstrap.TLSProfile = tlsProfile + objs, err := renderMultusAdmissionController(config, manifestDir, false, &testBootstrap, fakeClient, getDefaultFeatureGates()) + g.Expect(err).NotTo(HaveOccurred()) + return findMultusWebhookExec(t, objs) + }) + + // Test TLS rendering for kube-rbac-proxy container + // kube-rbac-proxy has hardcoded default ciphers when TLS profile is not honored + testTLSArgRendering(t, "multus-admission-controller kube-rbac-proxy", "", + "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305", + func(t *testing.T, tlsProfile bootstrap.TLSProfile) string { + testBootstrap := *bootstrapResult + testBootstrap.TLSProfile = tlsProfile + objs, err := renderMultusAdmissionController(config, manifestDir, false, &testBootstrap, fakeClient, getDefaultFeatureGates()) + g.Expect(err).NotTo(HaveOccurred()) + deployment := mustFindRenderedObj[*appsv1.Deployment](t, objs, "Deployment", "multus-admission-controller") + container := mustFindContainer(t, deployment.Spec.Template.Spec.Containers, "kube-rbac-proxy") + return strings.Join(container.Args, " ") + }) } // TestRenderMultusAdmissionController has some simple rendering tests @@ -132,7 +162,7 @@ func TestRenderMultusAdmissonControllerConfigForHyperShift(t *testing.T) { }, }, }) - bootstrap := fakeBootstrapResultWithHyperShift() + bootstrapResult := fakeBootstrapResultWithHyperShift() hsc := hypershift.NewHyperShiftConfig() hsc.Enabled = true @@ -144,7 +174,7 @@ func TestRenderMultusAdmissonControllerConfigForHyperShift(t *testing.T) { hsc.ReleaseImage = "MyImage" hsc.ControlPlaneImage = "MyCPOImage" - objs, err := renderMultusAdmissonControllerConfig(manifestDir, false, bootstrap, fakeClient, hsc, "", getDefaultFeatureGates()) + objs, err := renderMultusAdmissonControllerConfig(manifestDir, false, bootstrapResult, fakeClient, hsc, "", getDefaultFeatureGates()) g.Expect(err).NotTo(HaveOccurred()) // Check rendered object @@ -159,6 +189,23 @@ func TestRenderMultusAdmissonControllerConfigForHyperShift(t *testing.T) { g.Expect(annotations["network.operator.openshift.io/cluster-name"]).To(Equal("management")) } } + + weboookCmd := findMultusWebhookExec(t, objs) + g.Expect(weboookCmd).To(ContainSubstring("-metrics-listen-address=:9091")) + g.Expect(weboookCmd).To(ContainSubstring("-encrypt-metrics=true")) + + deployment := mustFindMultusAdmissionDeployment(t, objs) + _, ok := findContainer(deployment.Spec.Template.Spec.Containers, "kube-rbac-proxy") + g.Expect(ok).To(BeFalse(), "Found unexpected container \"kube-rbac-proxy\"") + + // Test TLS rendering for webhook container in HyperShift mode + testTLSArgRendering(t, "multus-admission-controller webhook (HyperShift)", "", "", func(t *testing.T, tlsProfile bootstrap.TLSProfile) string { + testBootstrap := *bootstrapResult + testBootstrap.TLSProfile = tlsProfile + objs, err := renderMultusAdmissonControllerConfig(manifestDir, false, &testBootstrap, fakeClient, hsc, "", getDefaultFeatureGates()) + g.Expect(err).NotTo(HaveOccurred()) + return findMultusWebhookExec(t, objs) + }) } // TestRenderMultusAdmissionControllerGetNamespace tests getOpenshiftNamespaces() @@ -196,3 +243,15 @@ func TestRenderMultusAdmissionControllerGetNamespace(t *testing.T) { g.Expect(err).NotTo(HaveOccurred()) g.Expect(namespaces).To(Equal("test1-ignored,test3-ignored")) } + +func mustFindMultusAdmissionDeployment(t *testing.T, objs []*unstructured.Unstructured) *appsv1.Deployment { + return mustFindRenderedObj[*appsv1.Deployment](t, objs, "Deployment", "multus-admission-controller") +} + +func findMultusWebhookExec(t *testing.T, objs []*unstructured.Unstructured) string { + t.Helper() + + deployment := mustFindMultusAdmissionDeployment(t, objs) + cmdArgs := mustFindContainer(t, deployment.Spec.Template.Spec.Containers, "multus-admission-controller").Command + return findExecCommand(t, cmdArgs, "webhook") +} From 6dd6533b9d92669fb6d39f4db7176642e30140c2 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Wed, 24 Jun 2026 08:09:58 -0400 Subject: [PATCH 05/13] Use shared start-rbac-proxy function in ovnkube-control-plane Replace inline kube-rbac-proxy script code with the shared start-rbac-proxy() function (renamed from start-rbac-proxy-node to reflect its generic usage) from the ovnkube-script-lib ConfigMap. This reduces code duplication and ensures consistent behavior across all OVN Kubernetes components. Signed-off-by: Tom Pantelis --- .../ovn-kubernetes/common/008-script-lib.yaml | 7 ++-- .../ovn-kubernetes/managed/ovnkube-node.yaml | 4 +- .../self-hosted/ovnkube-control-plane.yaml | 37 ++++--------------- .../self-hosted/ovnkube-node.yaml | 4 +- 4 files changed, 14 insertions(+), 38 deletions(-) diff --git a/bindata/network/ovn-kubernetes/common/008-script-lib.yaml b/bindata/network/ovn-kubernetes/common/008-script-lib.yaml index a611f88677..5269897d8c 100644 --- a/bindata/network/ovn-kubernetes/common/008-script-lib.yaml +++ b/bindata/network/ovn-kubernetes/common/008-script-lib.yaml @@ -209,7 +209,7 @@ data: # # Requires the following volume mounts: # /etc/pki/tls/metrics-cert - start-rbac-proxy-node() + start-rbac-proxy() { local detail=$1 local listen_port=$2 @@ -223,9 +223,8 @@ data: fi # As the secret mount is optional we must wait for the files to be present. - # The service is created in monitor-node.yaml but start-rbac-proxy-node is - # called from ovnkube-node.yaml. If the certificate isn't created there is - # probably an issue so we want to crashloop. + # If the certificate isn't created there is probably an issue so we want to + # crashloop. echo "$(date -Iseconds) INFO: waiting for ${detail} certs to be mounted" wait-for-certs "${detail}" "${privkey}" "${clientcert}" diff --git a/bindata/network/ovn-kubernetes/managed/ovnkube-node.yaml b/bindata/network/ovn-kubernetes/managed/ovnkube-node.yaml index ef4d1df21a..b928017522 100644 --- a/bindata/network/ovn-kubernetes/managed/ovnkube-node.yaml +++ b/bindata/network/ovn-kubernetes/managed/ovnkube-node.yaml @@ -183,7 +183,7 @@ spec: #!/bin/bash set -euo pipefail . /ovnkube-lib/ovnkube-lib.sh || exit 1 - start-rbac-proxy-node ovn-node-metrics 9103 29103 /etc/pki/tls/metrics-cert/tls.key /etc/pki/tls/metrics-cert/tls.crt + start-rbac-proxy ovn-node-metrics 9103 29103 /etc/pki/tls/metrics-cert/tls.key /etc/pki/tls/metrics-cert/tls.crt ports: - containerPort: 9103 name: https @@ -208,7 +208,7 @@ spec: #!/bin/bash set -euo pipefail . /ovnkube-lib/ovnkube-lib.sh || exit 1 - start-rbac-proxy-node ovn-metrics 9105 29105 /etc/pki/tls/metrics-cert/tls.key /etc/pki/tls/metrics-cert/tls.crt + start-rbac-proxy ovn-metrics 9105 29105 /etc/pki/tls/metrics-cert/tls.key /etc/pki/tls/metrics-cert/tls.crt ports: - containerPort: 9105 name: https diff --git a/bindata/network/ovn-kubernetes/self-hosted/ovnkube-control-plane.yaml b/bindata/network/ovn-kubernetes/self-hosted/ovnkube-control-plane.yaml index 2f69ef19ef..7769b610c7 100644 --- a/bindata/network/ovn-kubernetes/self-hosted/ovnkube-control-plane.yaml +++ b/bindata/network/ovn-kubernetes/self-hosted/ovnkube-control-plane.yaml @@ -47,36 +47,8 @@ spec: - | #!/bin/bash set -euo pipefail - TLS_PK=/etc/pki/tls/metrics-cert/tls.key - TLS_CERT=/etc/pki/tls/metrics-cert/tls.crt - # As the secret mount is optional we must wait for the files to be present. - # The service is created in monitor-control-plane.yaml. - TS=$(date +%s) - WARN_TS=$(( ${TS} + $(( 20 * 60)) )) - HAS_LOGGED_INFO=0 - - log_missing_certs(){ - CUR_TS=$(date +%s) - if [[ "${CUR_TS}" -gt "WARN_TS" ]]; then - echo $(date -Iseconds) WARN: ovn-control-plane-metrics-cert not mounted after 20 minutes. - elif [[ "${HAS_LOGGED_INFO}" -eq 0 ]] ; then - echo $(date -Iseconds) INFO: ovn-control-plane-metrics-cert not mounted. Waiting 20 minutes. - HAS_LOGGED_INFO=1 - fi - } - while [[ ! -f "${TLS_PK}" || ! -f "${TLS_CERT}" ]] ; do - log_missing_certs - sleep 5 - done - - echo $(date -Iseconds) INFO: ovn-control-plane-metrics-certs mounted, starting kube-rbac-proxy - exec /usr/bin/kube-rbac-proxy \ - --logtostderr \ - --secure-listen-address=:9108 \ - --tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 \ - --upstream=http://127.0.0.1:29108/ \ - --tls-private-key-file=${TLS_PK} \ - --tls-cert-file=${TLS_CERT} + . /ovnkube-lib/ovnkube-lib.sh || exit 1 + start-rbac-proxy ovn-control-plane-metrics 9108 29108 /etc/pki/tls/metrics-cert/tls.key /etc/pki/tls/metrics-cert/tls.crt ports: - containerPort: 9108 name: https @@ -89,6 +61,8 @@ spec: - name: ovn-control-plane-metrics-cert mountPath: /etc/pki/tls/metrics-cert readOnly: True + - name: ovnkube-script-lib + mountPath: /ovnkube-lib # ovnkube-control-plane: central component that allocates IPAM for each node in the cluster - name: ovnkube-cluster-manager image: "{{.OvnControlPlaneImage}}" @@ -223,6 +197,9 @@ spec: secret: secretName: ovn-control-plane-metrics-cert optional: true + - name: ovnkube-script-lib + configMap: + name: ovnkube-script-lib tolerations: - key: "node-role.kubernetes.io/master" operator: "Exists" diff --git a/bindata/network/ovn-kubernetes/self-hosted/ovnkube-node.yaml b/bindata/network/ovn-kubernetes/self-hosted/ovnkube-node.yaml index 9043e96693..5423622aef 100644 --- a/bindata/network/ovn-kubernetes/self-hosted/ovnkube-node.yaml +++ b/bindata/network/ovn-kubernetes/self-hosted/ovnkube-node.yaml @@ -219,7 +219,7 @@ spec: #!/bin/bash set -euo pipefail . /ovnkube-lib/ovnkube-lib.sh || exit 1 - start-rbac-proxy-node ovn-node-metrics 9103 29103 /etc/pki/tls/metrics-cert/tls.key /etc/pki/tls/metrics-cert/tls.crt + start-rbac-proxy ovn-node-metrics 9103 29103 /etc/pki/tls/metrics-cert/tls.key /etc/pki/tls/metrics-cert/tls.crt ports: - containerPort: 9103 name: https @@ -244,7 +244,7 @@ spec: #!/bin/bash set -euo pipefail . /ovnkube-lib/ovnkube-lib.sh || exit 1 - start-rbac-proxy-node ovn-metrics 9105 29105 /etc/pki/tls/metrics-cert/tls.key /etc/pki/tls/metrics-cert/tls.crt + start-rbac-proxy ovn-metrics 9105 29105 /etc/pki/tls/metrics-cert/tls.key /etc/pki/tls/metrics-cert/tls.crt ports: - containerPort: 9105 name: https From 46a985ccbefca46bf9bacbe6c34aa7db5293005d Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Wed, 24 Jun 2026 09:17:53 -0400 Subject: [PATCH 06/13] Use TLS profile to render CLI args for kube-rbac-proxy in OVNK Add TLS profile support to the start-rbac-proxy() function in the ovnkube-script-lib ConfigMap. This function is used by kube-rbac-proxy containers in ovnkube-node and ovnkube-control-plane pods. Changes: - Add TLS template code to start-rbac-proxy() function in 008-script-lib.yaml - Add addTLSInfoToRenderData() call in renderOVNKubernetes() function - Add TLS test to TestRenderOVNKubernetes using testTLSArgRendering helper Signed-off-by: Tom Pantelis --- .../ovn-kubernetes/common/008-script-lib.yaml | 7 +++++++ pkg/network/ovn_kubernetes.go | 3 +++ pkg/network/ovn_kubernetes_test.go | 19 +++++++++++++++++++ 3 files changed, 29 insertions(+) diff --git a/bindata/network/ovn-kubernetes/common/008-script-lib.yaml b/bindata/network/ovn-kubernetes/common/008-script-lib.yaml index 5269897d8c..e84eeb21d0 100644 --- a/bindata/network/ovn-kubernetes/common/008-script-lib.yaml +++ b/bindata/network/ovn-kubernetes/common/008-script-lib.yaml @@ -232,7 +232,14 @@ data: exec /usr/bin/kube-rbac-proxy \ --logtostderr \ --secure-listen-address=:${listen_port} \ +{{- if .UseTLSProfile }} + --tls-min-version={{.TLSMinVersion}} \ + {{- if .TLSCipherSuites }} + --tls-cipher-suites={{.TLSCipherSuites}} \ + {{- end }} +{{- else }} --tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 \ +{{- end }} --upstream=http://127.0.0.1:${upstream_port}/ \ --tls-private-key-file=${privkey} \ --tls-cert-file=${clientcert} diff --git a/pkg/network/ovn_kubernetes.go b/pkg/network/ovn_kubernetes.go index a96f6d1640..bf531cd725 100644 --- a/pkg/network/ovn_kubernetes.go +++ b/pkg/network/ovn_kubernetes.go @@ -111,6 +111,9 @@ func renderOVNKubernetes(conf *operv1.NetworkSpec, bootstrapResult *bootstrap.Bo // render the manifests on disk data := render.MakeRenderData() + + addTLSInfoToRenderData(data.Data, bootstrapResult, true) + data.Data["ReleaseVersion"] = os.Getenv("RELEASE_VERSION") data.Data["OvnImage"] = os.Getenv("OVN_IMAGE") data.Data["OvnControlPlaneImage"] = os.Getenv("OVN_IMAGE") diff --git a/pkg/network/ovn_kubernetes_test.go b/pkg/network/ovn_kubernetes_test.go index e768fbbc4d..ef343aa6bf 100644 --- a/pkg/network/ovn_kubernetes_test.go +++ b/pkg/network/ovn_kubernetes_test.go @@ -194,6 +194,25 @@ func TestRenderOVNKubernetes(t *testing.T) { g.Expect(clusterRole.Rules).To(ContainElements(expectedRules)) } } + + // Test TLS rendering for the kube-rbac-proxy in the start-rbac-proxy-node function + testTLSArgRendering(t, "ovnkube-script-lib kube-rbac-proxy", "", + "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", + func(t *testing.T, tlsProfile bootstrap.TLSProfile) string { + testBootstrap := *bootstrapResult + testBootstrap.TLSProfile = tlsProfile + objs, _, err = renderOVNKubernetes(config, &testBootstrap, manifestDirOvn, fakeClient, featureGatesCNO) + g.Expect(err).NotTo(HaveOccurred()) + + cm := mustFindRenderedObj(t, objs, "ConfigMap", "ovnkube-script-lib", &v1.ConfigMap{}) + data, ok := cm.Data["ovnkube-lib.sh"] + g.Expect(ok).To(BeTrue(), "ovnkube-lib.sh not found in ConfigMap") + + execStart := strings.Index(data, "exec /usr/bin/kube-rbac-proxy") + g.Expect(execStart).NotTo(Equal(-1), "exec /usr/bin/kube-rbac-proxy not found in script") + + return data[execStart:] + }) } func encodeClusterRole(obj *uns.Unstructured) (*rbacv1.ClusterRole, error) { From 3e6eaf371c3e828007568704785c1260bc1d8b33 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Fri, 26 Jun 2026 09:05:07 -0400 Subject: [PATCH 07/13] Use TLS profile to render CLI args for kube-proxy's kube-rbac-proxy - Update bindata/kube-proxy/kube-proxy.yaml to use TLS profile template variables (UseTLSProfile, TLSMinVersion, TLSCipherSuites) in the kube-rbac-proxy container's exec command - Add call to addTLSInfoToRenderData in renderStandaloneKubeProxy to populate TLS profile data in the render context - Add unit test in TestRenderKubeProxy to verify TLS CLI arguments are correctly rendered based on TLS profile adherence policy Signed-off-by: Tom Pantelis --- bindata/kube-proxy/kube-proxy.yaml | 7 +++++++ pkg/network/kube_proxy.go | 3 +++ pkg/network/kube_proxy_test.go | 15 +++++++++++++++ 3 files changed, 25 insertions(+) diff --git a/bindata/kube-proxy/kube-proxy.yaml b/bindata/kube-proxy/kube-proxy.yaml index 8ff1abbdc3..f9c32d6c4a 100644 --- a/bindata/kube-proxy/kube-proxy.yaml +++ b/bindata/kube-proxy/kube-proxy.yaml @@ -144,7 +144,14 @@ spec: exec /usr/bin/kube-rbac-proxy \ --logtostderr \ --secure-listen-address=:{{.MetricsPort}} \ +{{- if .UseTLSProfile }} + --tls-min-version={{.TLSMinVersion}} \ + {{- if .TLSCipherSuites }} + --tls-cipher-suites={{.TLSCipherSuites}} \ + {{- end }} +{{- else }} --tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 \ +{{- end }} --upstream=http://127.0.0.1:29102/ \ --tls-private-key-file=${TLS_PK} \ --tls-cert-file=${TLS_CERT} diff --git a/pkg/network/kube_proxy.go b/pkg/network/kube_proxy.go index 817d56281f..281d811d89 100644 --- a/pkg/network/kube_proxy.go +++ b/pkg/network/kube_proxy.go @@ -182,6 +182,9 @@ func renderStandaloneKubeProxy(conf *operv1.NetworkSpec, bootstrapResult *bootst } data := render.MakeRenderData() + + addTLSInfoToRenderData(data.Data, bootstrapResult, true) + data.Data["ReleaseVersion"] = os.Getenv("RELEASE_VERSION") data.Data["KubeProxyImage"] = os.Getenv("KUBE_PROXY_IMAGE") data.Data["KubeRBACProxyImage"] = os.Getenv("KUBE_RBAC_PROXY_IMAGE") diff --git a/pkg/network/kube_proxy_test.go b/pkg/network/kube_proxy_test.go index 2b078f06dd..d08dbdcada 100644 --- a/pkg/network/kube_proxy_test.go +++ b/pkg/network/kube_proxy_test.go @@ -5,6 +5,7 @@ import ( operv1 "github.com/openshift/api/operator/v1" "github.com/openshift/cluster-network-operator/pkg/bootstrap" + appsv1 "k8s.io/api/apps/v1" uns "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" . "github.com/onsi/gomega" @@ -491,4 +492,18 @@ winkernel: } } g.Expect(found).To(BeTrue()) + + // Test TLS rendering for kube-rbac-proxy container + testTLSArgRendering(t, "kube-proxy kube-rbac-proxy", "", + "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", + func(t *testing.T, tlsProfile bootstrap.TLSProfile) string { + testBootstrap := FakeKubeProxyBootstrapResult + testBootstrap.TLSProfile = tlsProfile + objs, err := renderStandaloneKubeProxy(c, &testBootstrap, manifestDir) + g.Expect(err).NotTo(HaveOccurred()) + + daemonSet := mustFindRenderedObj[*appsv1.DaemonSet](t, objs, "DaemonSet", "openshift-kube-proxy") + cmdArgs := mustFindContainer(t, daemonSet.Spec.Template.Spec.Containers, "kube-rbac-proxy").Command + return findExecCommand(t, cmdArgs, "kube-rbac-proxy") + }) } From 6d2e5430007366cbc00c3fe6bf88fe90cc1734de Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Fri, 26 Jun 2026 09:22:07 -0400 Subject: [PATCH 08/13] Use TLS profile to render CLI args for network-metrics kube-rbac-proxy - Update bindata/network/network-metrics/001-daemonset.yaml to use TLS profile template variables (UseTLSProfile, TLSMinVersion, TLSCipherSuites) in the kube-rbac-proxy container's args - Update renderNetworkMetricsDaemon to accept bootstrapResult parameter and add call to addTLSInfoToRenderData to populate TLS profile data - Update renderMultus to pass bootstrapResult to renderNetworkMetricsDaemon - Add unit test in TestRenderNetworkMetricsDaemon to verify TLS CLI arguments are correctly rendered based on TLS profile adherence policy Signed-off-by: Tom Pantelis --- .../network/network-metrics/001-daemonset.yaml | 7 +++++++ pkg/network/multus.go | 7 +++++-- pkg/network/network_metrics_test.go | 16 ++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/bindata/network/network-metrics/001-daemonset.yaml b/bindata/network/network-metrics/001-daemonset.yaml index 71a5f08759..d18af4c4ba 100644 --- a/bindata/network/network-metrics/001-daemonset.yaml +++ b/bindata/network/network-metrics/001-daemonset.yaml @@ -65,7 +65,14 @@ spec: args: - --logtostderr - --secure-listen-address=:8443 +{{- if .UseTLSProfile }} + - --tls-min-version={{.TLSMinVersion}} + {{- if .TLSCipherSuites }} + - --tls-cipher-suites={{.TLSCipherSuites}} + {{- end }} +{{- else }} - --tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 +{{- end }} - --upstream=http://127.0.0.1:9091/ - --tls-private-key-file=/etc/metrics/tls.key - --tls-cert-file=/etc/metrics/tls.crt diff --git a/pkg/network/multus.go b/pkg/network/multus.go index 41d8178350..750809c5b3 100644 --- a/pkg/network/multus.go +++ b/pkg/network/multus.go @@ -45,7 +45,7 @@ func renderMultus(conf *operv1.NetworkSpec, bootstrapResult *bootstrap.Bootstrap } out = append(out, objs...) - objs, err = renderNetworkMetricsDaemon(manifestDir) + objs, err = renderNetworkMetricsDaemon(manifestDir, bootstrapResult) if err != nil { return nil, err } @@ -107,12 +107,15 @@ func renderMultusConfig(manifestDir, defaultNetworkType string, useDHCP bool, us } // renderNetworkMetricsDaemon returns the manifests of the Network Metrics Daemon -func renderNetworkMetricsDaemon(manifestDir string) ([]*uns.Unstructured, error) { +func renderNetworkMetricsDaemon(manifestDir string, bootstrapResult *bootstrap.BootstrapResult) ([]*uns.Unstructured, error) { objs := []*uns.Unstructured{} // render the manifests on disk data := render.MakeRenderData() + + addTLSInfoToRenderData(data.Data, bootstrapResult, true) + data.Data["ReleaseVersion"] = os.Getenv("RELEASE_VERSION") data.Data["NetworkMetricsImage"] = os.Getenv("NETWORK_METRICS_DAEMON_IMAGE") data.Data["KubeRBACProxyImage"] = os.Getenv("KUBE_RBAC_PROXY_IMAGE") diff --git a/pkg/network/network_metrics_test.go b/pkg/network/network_metrics_test.go index 843fefd701..a488d043f8 100644 --- a/pkg/network/network_metrics_test.go +++ b/pkg/network/network_metrics_test.go @@ -1,10 +1,13 @@ package network import ( + "strings" "testing" . "github.com/onsi/gomega" operv1 "github.com/openshift/api/operator/v1" + "github.com/openshift/cluster-network-operator/pkg/bootstrap" + appsv1 "k8s.io/api/apps/v1" ) var NetworkMetricsDaemonConfig = operv1.Network{ @@ -54,4 +57,17 @@ func TestRenderNetworkMetricsDaemon(t *testing.T) { g.Expect(objs).To(ContainElement(HaveKubernetesID("ServiceMonitor", "openshift-multus", "monitor-network"))) g.Expect(objs).To(ContainElement(HaveKubernetesID("Role", "openshift-multus", "prometheus-k8s"))) g.Expect(objs).To(ContainElement(HaveKubernetesID("RoleBinding", "openshift-multus", "prometheus-k8s"))) + + // Test TLS rendering for kube-rbac-proxy container + testTLSArgRendering(t, "network-metrics kube-rbac-proxy", "", + "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", + func(t *testing.T, tlsProfile bootstrap.TLSProfile) string { + testBootstrap := fakeBootstrapResult() + testBootstrap.TLSProfile = tlsProfile + objs, err := renderMultus(config, testBootstrap, manifestDir) + g.Expect(err).NotTo(HaveOccurred()) + + daemonSet := mustFindRenderedObj[*appsv1.DaemonSet](t, objs, "DaemonSet", "network-metrics-daemon") + return strings.Join(mustFindContainer(t, daemonSet.Spec.Template.Spec.Containers, "kube-rbac-proxy").Args, " ") + }) } From 78a0e1a2ec7a91dd0d09c8702d8e09254272383d Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Fri, 26 Jun 2026 11:54:17 -0400 Subject: [PATCH 09/13] Use TLS profile to render CLI args for FRR components This commit adds rendering of TLS CLI args based on the TLS profile to the three FRR components that serve HTTPS endpoints: 1. controller container (port 9140) - Kubernetes controller for FRR 2. frr-metrics container (port 9141) - Prometheus metrics exporter 3. frr-k8s-statuscleaner container (port 9123) - Webhook for status cleanup Changes: - Add conditional template code to render --tls-min-version and --tls-cipher-suites CLI arguments when UseTLSProfile is true - Update renderAdditionalRoutingCapabilities to accept bootstrapResult parameter and call addTLSInfoToRenderData - Add comprehensive unit tests in Test_renderFRRRoutingCapabilities using the testTLSArgRendering helper function Signed-off-by: Tom Pantelis --- bindata/network/frr-k8s/frr-k8s.yaml | 12 +++++ .../network/frr-k8s/node-status-cleaner.yaml | 6 +++ pkg/network/render.go | 7 ++- pkg/network/render_test.go | 50 ++++++++++++++++++- 4 files changed, 72 insertions(+), 3 deletions(-) diff --git a/bindata/network/frr-k8s/frr-k8s.yaml b/bindata/network/frr-k8s/frr-k8s.yaml index 102c5007d9..7b0d4e7642 100644 --- a/bindata/network/frr-k8s/frr-k8s.yaml +++ b/bindata/network/frr-k8s/frr-k8s.yaml @@ -99,6 +99,12 @@ spec: - --namespace=$(NAMESPACE) - --metrics-bind-address=0.0.0.0:9140 - --metrics-cert-dir=/etc/metrics +{{- if .UseTLSProfile }} + - --tls-min-version={{.TLSMinVersion}} + {{- if .TLSCipherSuites }} + - --tls-cipher-suites={{.TLSCipherSuites}} + {{- end }} +{{- end }} - $(LOG_LEVEL) env: - name: FRR_CONFIG_FILE @@ -230,6 +236,12 @@ spec: - --metrics-bind-address=0.0.0.0 - --tls-cert-file=/etc/metrics/tls.crt - --tls-private-key-file=/etc/metrics/tls.key +{{- if .UseTLSProfile }} + - --tls-min-version={{.TLSMinVersion}} + {{- if .TLSCipherSuites }} + - --tls-cipher-suites={{.TLSCipherSuites}} + {{- end }} +{{- end }} ports: - containerPort: 9141 name: frrmetricshttps diff --git a/bindata/network/frr-k8s/node-status-cleaner.yaml b/bindata/network/frr-k8s/node-status-cleaner.yaml index 7a558d73fb..7c28bceffe 100644 --- a/bindata/network/frr-k8s/node-status-cleaner.yaml +++ b/bindata/network/frr-k8s/node-status-cleaner.yaml @@ -28,6 +28,12 @@ spec: - --namespace=$(NAMESPACE) - --webhook-port=9123 - --frrk8s-selector=component=frr-k8s +{{- if .UseTLSProfile }} + - --tls-min-version={{.TLSMinVersion}} + {{- if .TLSCipherSuites }} + - --tls-cipher-suites={{.TLSCipherSuites}} + {{- end }} +{{- end }} - $(LOG_LEVEL) env: - name: NAMESPACE diff --git a/pkg/network/render.go b/pkg/network/render.go index ffa2579e0f..b714b7e00f 100644 --- a/pkg/network/render.go +++ b/pkg/network/render.go @@ -129,7 +129,7 @@ func Render(operConf *operv1.NetworkSpec, clusterConf *configv1.NetworkSpec, man } objs = append(objs, o...) - o, err = renderAdditionalRoutingCapabilities(operConf, manifestDir) + o, err = renderAdditionalRoutingCapabilities(operConf, bootstrapResult, manifestDir) if err != nil { return nil, progressing, err } @@ -849,7 +849,7 @@ func registerNetworkingConsolePlugin(bootstrapResult *bootstrap.BootstrapResult, }) } -func renderAdditionalRoutingCapabilities(conf *operv1.NetworkSpec, manifestDir string) ([]*uns.Unstructured, error) { +func renderAdditionalRoutingCapabilities(conf *operv1.NetworkSpec, bootstrapResult *bootstrap.BootstrapResult, manifestDir string) ([]*uns.Unstructured, error) { if conf == nil || conf.AdditionalRoutingCapabilities == nil { return nil, nil } @@ -858,6 +858,9 @@ func renderAdditionalRoutingCapabilities(conf *operv1.NetworkSpec, manifestDir s switch provider { case operv1.RoutingCapabilitiesProviderFRR: data := render.MakeRenderData() + + addTLSInfoToRenderData(data.Data, bootstrapResult, true) + data.Data["FRRK8sImage"] = os.Getenv("FRR_K8S_IMAGE") data.Data["ReleaseVersion"] = os.Getenv("RELEASE_VERSION") data.Data["NoOverlayManagedEnabled"] = conf.DefaultNetwork.OVNKubernetesConfig != nil && diff --git a/pkg/network/render_test.go b/pkg/network/render_test.go index c9ce1f099e..b8d469b9dd 100644 --- a/pkg/network/render_test.go +++ b/pkg/network/render_test.go @@ -3,6 +3,7 @@ package network import ( "fmt" "reflect" + "strings" "testing" . "github.com/onsi/gomega" @@ -11,6 +12,8 @@ import ( openshifttls "github.com/openshift/controller-runtime-common/pkg/tls" "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" "github.com/stretchr/testify/assert" + appsv1 "k8s.io/api/apps/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/kubernetes/scheme" configv1 "github.com/openshift/api/config/v1" @@ -599,7 +602,7 @@ func Test_renderAdditionalRoutingCapabilities(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := renderAdditionalRoutingCapabilities(tt.args.operConf, manifestDir) + got, err := renderAdditionalRoutingCapabilities(tt.args.operConf, fakeBootstrapResult(), manifestDir) if !reflect.DeepEqual(tt.expectedErr, err) { t.Errorf("renderAdditionalRoutingCapabilities() err = %v, want %v", err, tt.expectedErr) } @@ -607,3 +610,48 @@ func Test_renderAdditionalRoutingCapabilities(t *testing.T) { }) } } + +func Test_renderFRRRoutingCapabilities(t *testing.T) { + g := NewGomegaWithT(t) + + getRenderedObjs := func(t *testing.T, tlsProfile bootstrap.TLSProfile) []*unstructured.Unstructured { + testBootstrap := fakeBootstrapResult() + testBootstrap.TLSProfile = tlsProfile + objs, err := renderAdditionalRoutingCapabilities(&operv1.NetworkSpec{ + AdditionalRoutingCapabilities: &operv1.AdditionalRoutingCapabilities{ + Providers: []operv1.RoutingCapabilitiesProvider{ + operv1.RoutingCapabilitiesProviderFRR, + }, + }, + }, testBootstrap, manifestDir) + g.Expect(err).NotTo(HaveOccurred()) + + return objs + } + + getDaemonsetContainerArgs := func(t *testing.T, tlsProfile bootstrap.TLSProfile, containerName string) string { + daemonSet := mustFindRenderedObj[*appsv1.DaemonSet](t, getRenderedObjs(t, tlsProfile), "DaemonSet", "frr-k8s") + container := mustFindContainer(t, daemonSet.Spec.Template.Spec.Containers, containerName) + return strings.Join(container.Args, " ") + } + + // Test TLS rendering for frr-metrics container + testTLSArgRendering(t, "frr-metrics", "", "", + func(t *testing.T, tlsProfile bootstrap.TLSProfile) string { + return getDaemonsetContainerArgs(t, tlsProfile, "frr-metrics") + }) + + // Test TLS rendering for controller container + testTLSArgRendering(t, "frr-k8s controller", "", "", + func(t *testing.T, tlsProfile bootstrap.TLSProfile) string { + return getDaemonsetContainerArgs(t, tlsProfile, "controller") + }) + + // Test TLS rendering for statuscleaner container + testTLSArgRendering(t, "frr-k8s-statuscleaner", "", "", + func(t *testing.T, tlsProfile bootstrap.TLSProfile) string { + deployment := mustFindRenderedObj[*appsv1.Deployment](t, getRenderedObjs(t, tlsProfile), "Deployment", "frr-k8s-statuscleaner") + g.Expect(deployment.Spec.Template.Spec.Containers).To(HaveLen(1)) + return strings.Join(deployment.Spec.Template.Spec.Containers[0].Args, " ") + }) +} From 6ad012f8e3f7e02d94b4f05032bf3217ef173532 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Wed, 15 Jul 2026 11:17:11 -0400 Subject: [PATCH 10/13] Use TLS profile to render CLI args for HyperShift ovnkube-control-plane In HyperShift deployments, the ovnkube-control-plane serves metrics on port 9108 using TLS. This commit adds --tls-min-version and --tls-cipher-suites CLI arguments to the ovnkube command to ensure it respects the cluster's TLS security profile. The managed/ovnkube-control-plane.yaml template now conditionally renders these TLS arguments based on the UseTLSProfile flag, similar to how kube-rbac-proxy is configured in non-HyperShift deployments. Added unit test coverage to TestRenderOVNKubernetes to verify the rendering. Signed-off-by: Tom Pantelis --- .../managed/ovnkube-control-plane.yaml | 6 ++++ pkg/network/ovn_kubernetes_test.go | 29 ++++++++++++++++++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/bindata/network/ovn-kubernetes/managed/ovnkube-control-plane.yaml b/bindata/network/ovn-kubernetes/managed/ovnkube-control-plane.yaml index 2bf64ccc55..7b2e729c70 100644 --- a/bindata/network/ovn-kubernetes/managed/ovnkube-control-plane.yaml +++ b/bindata/network/ovn-kubernetes/managed/ovnkube-control-plane.yaml @@ -219,6 +219,12 @@ spec: --metrics-enable-config-duration \ --node-server-privkey ${TLS_PK} \ --node-server-cert ${TLS_CERT} \ +{{- if .UseTLSProfile }} + --tls-min-version={{.TLSMinVersion}} \ + {{- if .TLSCipherSuites }} + --tls-cipher-suites={{.TLSCipherSuites}} \ + {{- end }} +{{- end }} ${ovn_v4_join_subnet_opt} \ ${ovn_v6_join_subnet_opt} \ ${ovn_v4_transit_switch_subnet_opt} \ diff --git a/pkg/network/ovn_kubernetes_test.go b/pkg/network/ovn_kubernetes_test.go index ef343aa6bf..2d30f8df3d 100644 --- a/pkg/network/ovn_kubernetes_test.go +++ b/pkg/network/ovn_kubernetes_test.go @@ -204,7 +204,7 @@ func TestRenderOVNKubernetes(t *testing.T) { objs, _, err = renderOVNKubernetes(config, &testBootstrap, manifestDirOvn, fakeClient, featureGatesCNO) g.Expect(err).NotTo(HaveOccurred()) - cm := mustFindRenderedObj(t, objs, "ConfigMap", "ovnkube-script-lib", &v1.ConfigMap{}) + cm := mustFindRenderedObj[*v1.ConfigMap](t, objs, "ConfigMap", "ovnkube-script-lib") data, ok := cm.Data["ovnkube-lib.sh"] g.Expect(ok).To(BeTrue(), "ovnkube-lib.sh not found in ConfigMap") @@ -213,6 +213,33 @@ func TestRenderOVNKubernetes(t *testing.T) { return data[execStart:] }) + + // Test TLS rendering for ovnkube in HyperShift managed ovnkube-control-plane + testTLSArgRendering(t, "HyperShift ovnkube-control-plane", "", "", + func(t *testing.T, tlsProfile bootstrap.TLSProfile) string { + testBootstrap := *bootstrapResult + testBootstrap.TLSProfile = tlsProfile + testBootstrap.Infra = bootstrap.InfraStatus{} + testBootstrap.Infra.HostedControlPlane = &hypershift.HostedControlPlane{} + ovnConfig := *testBootstrap.OVN.OVNKubernetesConfig + ovnConfig.HyperShiftConfig = &bootstrap.OVNHyperShiftBootstrapResult{ + Enabled: true, + } + testBootstrap.OVN.OVNKubernetesConfig = &ovnConfig + objs, _, err = renderOVNKubernetes(config, &testBootstrap, manifestDirOvn, fakeClient, featureGatesCNO) + g.Expect(err).NotTo(HaveOccurred()) + + deployment := mustFindRenderedObj[*appsv1.Deployment](t, objs, "Deployment", "ovnkube-control-plane") + container := mustFindContainer(t, deployment.Spec.Template.Spec.Containers, "ovnkube-control-plane") + g.Expect(container.Command).NotTo(BeEmpty(), "ovnkube-control-plane container command should not be empty") + + // The command is a bash script, extract the exec line + commandScript := strings.Join(container.Command, " ") + execStart := strings.Index(commandScript, "exec /usr/bin/ovnkube") + g.Expect(execStart).NotTo(Equal(-1), "exec /usr/bin/ovnkube not found in container command") + + return commandScript[execStart:] + }) } func encodeClusterRole(obj *uns.Unstructured) (*rbacv1.ClusterRole, error) { From 1117c8e634259c22449e99c12e26aee26d243b9b Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Fri, 17 Jul 2026 11:37:20 -0400 Subject: [PATCH 11/13] Use TLS profile to render CLI args for network-check-source Render TLS CLI arguments (--tls-min-version and --tls-cipher-suites) for the network-check-source deployment based on the cluster's TLS security profile. This follows the same pattern as other components in the CNO. Updates renderNetworkDiagnostics to accept bootstrapResult and adds TLS data to the render context. Includes unit tests to verify the TLS args are correctly rendered in the deployment manifest. Signed-off-by: Tom Pantelis --- .../network-check-source.yaml | 6 +++++ pkg/network/render.go | 7 ++++-- pkg/network/render_test.go | 25 ++++++++++++++++++- 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/bindata/network-diagnostics/network-check-source.yaml b/bindata/network-diagnostics/network-check-source.yaml index 914d3bab9f..d72a94e953 100644 --- a/bindata/network-diagnostics/network-check-source.yaml +++ b/bindata/network-diagnostics/network-check-source.yaml @@ -50,6 +50,12 @@ spec: - 0.0.0.0:17698 - --namespace - $(POD_NAMESPACE) +{{- if .UseTLSProfile }} + - --tls-min-version={{.TLSMinVersion}} +{{- if .TLSCipherSuites }} + - --tls-cipher-suites={{.TLSCipherSuites}} +{{- end }} +{{- end }} env: - name: POD_NAME valueFrom: diff --git a/pkg/network/render.go b/pkg/network/render.go index b714b7e00f..7defecde88 100644 --- a/pkg/network/render.go +++ b/pkg/network/render.go @@ -97,7 +97,7 @@ func Render(operConf *operv1.NetworkSpec, clusterConf *configv1.NetworkSpec, man objs = append(objs, o...) // render network diagnostics - o, err = renderNetworkDiagnostics(operConf, clusterConf, manifestDir) + o, err = renderNetworkDiagnostics(operConf, clusterConf, bootstrapResult, manifestDir) if err != nil { return nil, progressing, err } @@ -722,7 +722,7 @@ func renderMultiNetworkpolicy(conf *operv1.NetworkSpec, manifestDir string) ([]* } // renderNetworkDiagnostics renders the connectivity checks -func renderNetworkDiagnostics(operConf *operv1.NetworkSpec, clusterConf *configv1.NetworkSpec, manifestDir string) ([]*uns.Unstructured, error) { +func renderNetworkDiagnostics(operConf *operv1.NetworkSpec, clusterConf *configv1.NetworkSpec, bootstrapResult *bootstrap.BootstrapResult, manifestDir string) ([]*uns.Unstructured, error) { // network diagnostics feature is disabled when clusterConf.NetworkDiagnostics.Mode is set to "Disabled" // or when clusterConf.NetworkDiagnostics is empty and the legacy operConf.DisableNetworkDiagnostics is true if clusterConf.NetworkDiagnostics.Mode == configv1.NetworkDiagnosticsDisabled || @@ -748,6 +748,9 @@ func renderNetworkDiagnostics(operConf *operv1.NetworkSpec, clusterConf *configv if clusterConf.NetworkDiagnostics.TargetPlacement.Tolerations != nil { data.Data["NetworkCheckTargetTolerations"] = clusterConf.NetworkDiagnostics.TargetPlacement.Tolerations } + + addTLSInfoToRenderData(data.Data, bootstrapResult, true) + manifests, err := render.RenderDir(filepath.Join(manifestDir, "network-diagnostics"), &data) if err != nil { return nil, errors.Wrap(err, "failed to render network-diagnostics manifests") diff --git a/pkg/network/render_test.go b/pkg/network/render_test.go index b8d469b9dd..3aa3431b54 100644 --- a/pkg/network/render_test.go +++ b/pkg/network/render_test.go @@ -558,13 +558,36 @@ func Test_renderNetworkDiagnostics(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := renderNetworkDiagnostics(tt.args.operConf, tt.args.clusterConf, manifestDir) + got, err := renderNetworkDiagnostics(tt.args.operConf, tt.args.clusterConf, fakeBootstrapResult(), manifestDir) if !reflect.DeepEqual(tt.expectedErr, err) { t.Errorf("Test_renderNetworkDiagnostics() err = %v, want %v", err, tt.expectedErr) } assert.Equalf(t, tt.want, len(got), "renderNetworkDiagnostics(%v, %v, %v)", tt.args.operConf, tt.args.clusterConf, manifestDir) }) } + + // Test TLS args rendering for network-check-source + t.Run("TLS args rendering", func(t *testing.T) { + testTLSArgRendering(t, "network-check-source", "", "", func(t *testing.T, tlsProfile bootstrap.TLSProfile) string { + operConf := &operv1.NetworkSpec{} + clusterConf := &configv1.NetworkSpec{ + NetworkDiagnostics: configv1.NetworkDiagnostics{ + Mode: configv1.NetworkDiagnosticsAll, + }, + } + bootstrapResult := fakeBootstrapResult() + bootstrapResult.TLSProfile = tlsProfile + + objs, err := renderNetworkDiagnostics(operConf, clusterConf, bootstrapResult, manifestDir) + if err != nil { + t.Fatalf("renderNetworkDiagnostics failed: %v", err) + } + + deployment := mustFindRenderedObj[*appsv1.Deployment](t, objs, "Deployment", "network-check-source") + container := mustFindContainer(t, deployment.Spec.Template.Spec.Containers, "check-endpoints") + return strings.Join(container.Args, " ") + }) + }) } func Test_renderAdditionalRoutingCapabilities(t *testing.T) { From 7451017391225f5914370aff94d2799027865feb Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Fri, 17 Jul 2026 11:40:10 -0400 Subject: [PATCH 12/13] Migrate check-endpoints to controller-runtime with TLS CLI args Migrate the check-endpoints command from library-go's controllercmd framework to controller-runtime to support TLS configuration for the metrics server. This change: - Adds --tls-min-version and --tls-cipher-suites CLI flags - Uses cliflag parsers (TLSVersion, TLSCipherSuites) for validation - Applies CLI TLS settings first, then crypto.SecureTLSConfig() for defaults - Migrates metrics registration from legacyregistry to controller-runtime registry - Adds serviceability profiling support (BehaviorOnPanic, Profile, StartProfiler) The controller-runtime migration enables direct TLS customization via metricsserver.Options, which was not possible with library-go's controllercmd framework. Signed-off-by: Tom Pantelis --- .../network-check-source.yaml | 2 - pkg/cmd/checkendpoints/cmd.go | 248 ++++++++++++------ pkg/cmd/checkendpoints/controller/metrics.go | 24 +- 3 files changed, 181 insertions(+), 93 deletions(-) diff --git a/bindata/network-diagnostics/network-check-source.yaml b/bindata/network-diagnostics/network-check-source.yaml index d72a94e953..7a31c4e327 100644 --- a/bindata/network-diagnostics/network-check-source.yaml +++ b/bindata/network-diagnostics/network-check-source.yaml @@ -48,8 +48,6 @@ spec: args: - --listen - 0.0.0.0:17698 - - --namespace - - $(POD_NAMESPACE) {{- if .UseTLSProfile }} - --tls-min-version={{.TLSMinVersion}} {{- if .TLSCipherSuites }} diff --git a/pkg/cmd/checkendpoints/cmd.go b/pkg/cmd/checkendpoints/cmd.go index 8978f206c3..4c06be85b5 100644 --- a/pkg/cmd/checkendpoints/cmd.go +++ b/pkg/cmd/checkendpoints/cmd.go @@ -2,6 +2,8 @@ package checkendpoints import ( "context" + "crypto/tls" + "fmt" "os" "time" @@ -9,9 +11,10 @@ import ( operatorcontrolplaneinformers "github.com/openshift/client-go/operatorcontrolplane/informers/externalversions" "github.com/openshift/cluster-network-operator/pkg/cmd/checkendpoints/controller" "github.com/openshift/cluster-network-operator/pkg/version" - "github.com/openshift/library-go/pkg/controller/controllercmd" + "github.com/openshift/library-go/pkg/crypto" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/resource/retry" + "github.com/openshift/library-go/pkg/serviceability" "github.com/spf13/cobra" corev1 "k8s.io/api/core/v1" apiextensionsclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" @@ -19,92 +22,179 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" + cliflag "k8s.io/component-base/cli/flag" + "k8s.io/component-base/logs" + "k8s.io/klog/v2" "k8s.io/utils/clock" + ctrl "sigs.k8s.io/controller-runtime" + metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" ) func NewCheckEndpointsCommand() *cobra.Command { - config := controllercmd.NewControllerCommandConfig("check-endpoints", version.Get(), func(ctx context.Context, cctx *controllercmd.ControllerContext) error { - podName := os.Getenv("POD_NAME") - namespace := os.Getenv("POD_NAMESPACE") - kubeClient := kubernetes.NewForConfigOrDie(cctx.ProtoKubeConfig) - apiextensionsClient := apiextensionsclient.NewForConfigOrDie(cctx.KubeConfig) - operatorcontrolplaneClient := operatorcontrolplaneclient.NewForConfigOrDie(cctx.KubeConfig) - kubeInformers := informers.NewSharedInformerFactoryWithOptions(kubeClient, 10*time.Minute, informers.WithNamespace(namespace)) - operatorcontrolplaneInformers := operatorcontrolplaneinformers.NewSharedInformerFactoryWithOptions(operatorcontrolplaneClient, 10*time.Minute, operatorcontrolplaneinformers.WithNamespace(namespace)) - apiextensionsInformers := apiextensionsinformers.NewSharedInformerFactory(apiextensionsClient, 10*time.Minute) - - // create a recorder that sets the pod node as the involved object in events - var involvedObjectRef *corev1.ObjectReference - err := retry.RetryOnConnectionErrors(ctx, func(context.Context) (bool, error) { - pod, err := kubeClient.CoreV1().Pods(namespace).Get(ctx, podName, metav1.GetOptions{}) - if err != nil { - return false, err - } - node, err := kubeClient.CoreV1().Nodes().Get(ctx, pod.Spec.NodeName, metav1.GetOptions{}) - if err != nil { - return false, err - } - involvedObjectRef = &corev1.ObjectReference{ - Kind: "Node", - Namespace: namespace, - Name: node.Name, - UID: node.UID, - APIVersion: node.APIVersion, + var ( + listenAddr string + tlsMinVersion string + tlsCipherSuites []string + ) + + cmd := &cobra.Command{ + Use: "check-endpoints", + Short: "Checks that a tcp connection can be opened to one or more endpoints.", + Run: func(cmd *cobra.Command, args []string) { + logs.InitLogs() + defer logs.FlushLogs() + + ctx := ctrl.SetupSignalHandler() + podName := os.Getenv("POD_NAME") + namespace := os.Getenv("POD_NAMESPACE") + + if err := run(ctx, listenAddr, tlsMinVersion, tlsCipherSuites, podName, namespace); err != nil { + klog.Fatal(err) } - return true, nil - }) + }, + } + + cmd.Flags().StringVar(&listenAddr, "listen", ":17698", "The ip:port to serve metrics on.") + cmd.Flags().StringVar(&tlsMinVersion, "tls-min-version", "", "Minimum TLS version (e.g., VersionTLS12, VersionTLS13)") + cmd.Flags().StringSliceVar(&tlsCipherSuites, "tls-cipher-suites", nil, "Comma-separated list of TLS cipher suites") + + return cmd +} + +func run(ctx context.Context, listenAddr, tlsMinVersion string, tlsCipherSuites []string, podName, namespace string) error { + restConfig := ctrl.GetConfigOrDie() + + tlsOpts, err := buildTLSOptions(tlsMinVersion, tlsCipherSuites) + if err != nil { + return fmt.Errorf("failed to build TLS options: %w", err) + } + + mgr, err := ctrl.NewManager(restConfig, ctrl.Options{ + Metrics: metricsserver.Options{ + BindAddress: listenAddr, + SecureServing: true, + TLSOpts: tlsOpts, + }, + }) + if err != nil { + return fmt.Errorf("unable to create manager: %w", err) + } + + defer serviceability.BehaviorOnPanic(os.Getenv("OPENSHIFT_ON_PANIC"), version.Get())() + defer serviceability.Profile(os.Getenv("OPENSHIFT_PROFILE")).Stop() + + serviceability.StartProfiler() + + go func() { + klog.Info("Starting the metrics server") + if err := mgr.Start(ctx); err != nil { + klog.Fatalf("Problem running metrics server: %v", err) + } + }() + + klog.Info("Running the controllers") + + return runControllers(ctx, restConfig, podName, namespace) +} + +// runControllers runs the factory.Controller-based logic +func runControllers(ctx context.Context, restConfig *rest.Config, podName, namespace string) error { + kubeClient := kubernetes.NewForConfigOrDie(restConfig) + apiextensionsClient := apiextensionsclient.NewForConfigOrDie(restConfig) + operatorcontrolplaneClient := operatorcontrolplaneclient.NewForConfigOrDie(restConfig) + + kubeInformers := informers.NewSharedInformerFactoryWithOptions(kubeClient, 10*time.Minute, informers.WithNamespace(namespace)) + operatorcontrolplaneInformers := operatorcontrolplaneinformers.NewSharedInformerFactoryWithOptions(operatorcontrolplaneClient, + 10*time.Minute, operatorcontrolplaneinformers.WithNamespace(namespace)) + apiextensionsInformers := apiextensionsinformers.NewSharedInformerFactory(apiextensionsClient, 10*time.Minute) + + // create a recorder that sets the pod node as the involved object in events + var involvedObjectRef *corev1.ObjectReference + err := retry.RetryOnConnectionErrors(ctx, func(context.Context) (bool, error) { + pod, err := kubeClient.CoreV1().Pods(namespace).Get(ctx, podName, metav1.GetOptions{}) if err != nil { - return err + return false, err } - recorder := events.NewRecorder(kubeClient.CoreV1().Events(namespace), "check-endpoint", involvedObjectRef, clock.RealClock{}) - - check := controller.NewPodNetworkConnectivityCheckController( - podName, - namespace, - operatorcontrolplaneClient.ControlplaneV1alpha1(), - operatorcontrolplaneInformers.Controlplane().V1alpha1().PodNetworkConnectivityChecks(), - kubeInformers.Core().V1().Secrets(), - recorder, - ) - - timeToStart := newTimeToStartController( - apiextensionsInformers.Apiextensions().V1().CustomResourceDefinitions(), - recorder, - ) - - stopController := newStopController( - apiextensionsInformers.Apiextensions().V1().CustomResourceDefinitions(), - recorder, - ) - - controller.RegisterMetrics() - - // block until the PodNetworkConnectivityCheck CRD exists - apiextensionsInformers.Start(ctx.Done()) - ttsContext, ttsCancel := context.WithCancel(ctx) - go timeToStart.Run(ttsContext, 1) - select { - case err := <-timeToStart.Ready(): - ttsCancel() - if err != nil { - return err - } - case <-ctx.Done(): - ttsCancel() - return nil + + node, err := kubeClient.CoreV1().Nodes().Get(ctx, pod.Spec.NodeName, metav1.GetOptions{}) + if err != nil { + return false, err } - // continue startup - operatorcontrolplaneInformers.Start(ctx.Done()) - kubeInformers.Start(ctx.Done()) - go check.Run(ctx, 1) - go stopController.Run(ctx, 1) - <-ctx.Done() + involvedObjectRef = &corev1.ObjectReference{ + Kind: "Node", + Namespace: namespace, + Name: node.Name, + UID: node.UID, + APIVersion: node.APIVersion, + } + + return true, nil + }) + if err != nil { + return err + } + + recorder := events.NewRecorder(kubeClient.CoreV1().Events(namespace), "check-endpoint", involvedObjectRef, clock.RealClock{}) + + check := controller.NewPodNetworkConnectivityCheckController( + podName, + namespace, + operatorcontrolplaneClient.ControlplaneV1alpha1(), + operatorcontrolplaneInformers.Controlplane().V1alpha1().PodNetworkConnectivityChecks(), + kubeInformers.Core().V1().Secrets(), + recorder, + ) + + timeToStart := newTimeToStartController(apiextensionsInformers.Apiextensions().V1().CustomResourceDefinitions(), recorder) + + stopController := newStopController(apiextensionsInformers.Apiextensions().V1().CustomResourceDefinitions(), recorder) + + controller.RegisterMetrics() + + // block until the PodNetworkConnectivityCheck CRD exists + apiextensionsInformers.Start(ctx.Done()) + ttsContext, ttsCancel := context.WithCancel(ctx) + go timeToStart.Run(ttsContext, 1) + select { + case err := <-timeToStart.Ready(): + ttsCancel() + if err != nil { + return err + } + case <-ctx.Done(): + ttsCancel() return nil - }, clock.RealClock{}) - config.DisableLeaderElection = true - cmd := config.NewCommandWithContext(context.Background()) - cmd.Use = "check-endpoints" - cmd.Short = "Checks that a tcp connection can be opened to one or more endpoints." - return cmd + } + + operatorcontrolplaneInformers.Start(ctx.Done()) + kubeInformers.Start(ctx.Done()) + go check.Run(ctx, 1) + go stopController.Run(ctx, 1) + <-ctx.Done() + + return nil +} + +// buildTLSOptions creates TLS config functions from CLI flags or falls back to library-go defaults +func buildTLSOptions(tlsMinVersion string, tlsCipherSuites []string) ([]func(*tls.Config), error) { + tlsMinVersionID, err := cliflag.TLSVersion(tlsMinVersion) + if err != nil { + return nil, fmt.Errorf("error parsing TLS min version %q: %w", tlsMinVersion, err) + } + + tlsCipherSuiteIDs, err := cliflag.TLSCipherSuites(tlsCipherSuites) + if err != nil { + return nil, fmt.Errorf("error parsing TLS cipher suites %v: %w", tlsCipherSuites, err) + } + + return []func(*tls.Config){func(cfg *tls.Config) { + // Set from CLI args first + cfg.MinVersion = tlsMinVersionID + cfg.CipherSuites = tlsCipherSuiteIDs + + // Apply library-go strong defaults for any unset fields + crypto.SecureTLSConfig(cfg) + }}, nil } diff --git a/pkg/cmd/checkendpoints/controller/metrics.go b/pkg/cmd/checkendpoints/controller/metrics.go index 2ef0bef64f..d79222428e 100644 --- a/pkg/cmd/checkendpoints/controller/metrics.go +++ b/pkg/cmd/checkendpoints/controller/metrics.go @@ -4,38 +4,38 @@ import ( "sync" "github.com/openshift/cluster-network-operator/pkg/cmd/checkendpoints/trace" - "k8s.io/component-base/metrics" - "k8s.io/component-base/metrics/legacyregistry" + "github.com/prometheus/client_golang/prometheus" + ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics" ) var ( registerMetrics sync.Once - endpointCheckCounter *metrics.CounterVec - tcpConnectLatencyGauge *metrics.GaugeVec - dnsResolveLatencyGauge *metrics.GaugeVec + endpointCheckCounter *prometheus.CounterVec + tcpConnectLatencyGauge *prometheus.GaugeVec + dnsResolveLatencyGauge *prometheus.GaugeVec ) -// RegisterMetrics in the global registry +// RegisterMetrics in the controller-runtime global registry func RegisterMetrics() { registerMetrics.Do(func() { - endpointCheckCounter = metrics.NewCounterVec(&metrics.CounterOpts{ + endpointCheckCounter = prometheus.NewCounterVec(prometheus.CounterOpts{ Name: "pod_network_connectivity_check_count", Help: "Report status of pod network connectivity checks over time.", }, []string{"component", "checkName", "targetEndpoint", "tcpConnect", "dnsResolve"}) - tcpConnectLatencyGauge = metrics.NewGaugeVec(&metrics.GaugeOpts{ + tcpConnectLatencyGauge = prometheus.NewGaugeVec(prometheus.GaugeOpts{ Name: "pod_network_connectivity_check_tcp_connect_latency_gauge", Help: "Report latency of TCP connect to target endpoint over time.", }, []string{"component", "checkName", "targetEndpoint"}) - dnsResolveLatencyGauge = metrics.NewGaugeVec(&metrics.GaugeOpts{ + dnsResolveLatencyGauge = prometheus.NewGaugeVec(prometheus.GaugeOpts{ Name: "pod_network_connectivity_check_dns_resolve_latency_gauge", Help: "Report latency of DNS resolve of target endpoint over time.", }, []string{"component", "checkName", "targetEndpoint"}) - legacyregistry.MustRegister(endpointCheckCounter) - legacyregistry.MustRegister(tcpConnectLatencyGauge) - legacyregistry.MustRegister(dnsResolveLatencyGauge) + ctrlmetrics.Registry.MustRegister(endpointCheckCounter) + ctrlmetrics.Registry.MustRegister(tcpConnectLatencyGauge) + ctrlmetrics.Registry.MustRegister(dnsResolveLatencyGauge) }) } From 308503fe1727b78b79570390824bcff408a2e44a Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Wed, 22 Jul 2026 12:27:38 -0400 Subject: [PATCH 13/13] Use TLS profile to render networking-console-plugin NGINX directives Renders NGINX ssl_protocols and ssl_ciphers directives based on the TLS profile configuration. Enhanced addTLSInfoToRenderData() to support both Go CLI args (with IANA cipher names) and NGINX directives (with OpenSSL cipher names). Added NginxTLSProtocolsKey and NginxTLSCiphersKey constants for template data. Updated the NGINX configuration template to conditionally render TLS directives when UseTLSProfile is true, respecting the adherence policy. Added unit tests for NGINX configuration rendering. Signed-off-by: Tom Pantelis --- .../002-config-map.yaml | 7 ++ pkg/network/bootstrap_test.go | 2 +- pkg/network/render.go | 2 + pkg/network/render_test.go | 66 ++++++++++++++++++ pkg/network/tls.go | 69 +++++++++++++------ pkg/network/tls_test.go | 38 ++++++++++ 6 files changed, 162 insertions(+), 22 deletions(-) diff --git a/bindata/networking-console-plugin/002-config-map.yaml b/bindata/networking-console-plugin/002-config-map.yaml index 5bd95abf2d..72f52a186f 100644 --- a/bindata/networking-console-plugin/002-config-map.yaml +++ b/bindata/networking-console-plugin/002-config-map.yaml @@ -12,6 +12,13 @@ data: listen [::]:9443 ssl; ssl_certificate /var/cert/tls.crt; ssl_certificate_key /var/cert/tls.key; +{{- if .UseTLSProfile }} + ssl_protocols {{.NginxTLSProtocols}}; +{{- if .NginxTLSCiphers }} + ssl_ciphers {{.NginxTLSCiphers}}; +{{- end }} + ssl_prefer_server_ciphers on; +{{- end }} root /opt/app-root/src; # Prevent caching for plugin-manifest.json diff --git a/pkg/network/bootstrap_test.go b/pkg/network/bootstrap_test.go index 52da3f0534..b40f620cb5 100644 --- a/pkg/network/bootstrap_test.go +++ b/pkg/network/bootstrap_test.go @@ -91,7 +91,7 @@ func TestBootstrap(t *testing.T) { configv1.VersionTLS11, result.TLSProfile.Spec.MinTLSVersion) } - expectedCiphers := []string{"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256"} + expectedCiphers := []string{"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", "ECDHE-ECDSA-CHACHA20-POLY1305"} if !reflect.DeepEqual(result.TLSProfile.Spec.Ciphers, expectedCiphers) { t.Errorf("Expected ciphers %v, got %v", expectedCiphers, result.TLSProfile.Spec.Ciphers) diff --git a/pkg/network/render.go b/pkg/network/render.go index 7defecde88..626ffd5a21 100644 --- a/pkg/network/render.go +++ b/pkg/network/render.go @@ -822,6 +822,8 @@ func renderNetworkingConsolePlugin(manifestDir string, bootstrapResult *bootstra } data.Data["NetworkingConsolePluginImage"] = consolePluginImage + addTLSInfoToRenderData(data.Data, bootstrapResult, true) + manifests, err := render.RenderDir(filepath.Join(manifestDir, "networking-console-plugin"), &data) if err != nil { return nil, errors.Wrap(err, "failed to render networking-console-plugin manifests") diff --git a/pkg/network/render_test.go b/pkg/network/render_test.go index 3aa3431b54..8cabe366aa 100644 --- a/pkg/network/render_test.go +++ b/pkg/network/render_test.go @@ -13,6 +13,7 @@ import ( "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/kubernetes/scheme" @@ -678,3 +679,68 @@ func Test_renderFRRRoutingCapabilities(t *testing.T) { return strings.Join(deployment.Spec.Template.Spec.Containers[0].Args, " ") }) } + +func Test_renderNetworkingConsolePlugin(t *testing.T) { + renderAndFindNginxConfig := func(t *testing.T, tlsProfile bootstrap.TLSProfile) string { + g := NewWithT(t) + t.Setenv("NETWORKING_CONSOLE_PLUGIN_IMAGE", "quay.io/openshift/networking-console-plugin:latest") + bootstrapResult := fakeBootstrapResult() + bootstrapResult.Infra.ConsolePluginCRDExists = true + bootstrapResult.TLSProfile = tlsProfile + + objs, err := renderNetworkingConsolePlugin(manifestDir, bootstrapResult) + g.Expect(err).NotTo(HaveOccurred()) + + cm := mustFindRenderedObj[*corev1.ConfigMap](t, objs, "ConfigMap", "networking-console-plugin") + return cm.Data["nginx.conf"] + } + + t.Run("when TLS profile adherence is StrictAllComponents", func(t *testing.T) { + t.Run("should render ssl_protocols and ssl_ciphers directives", func(t *testing.T) { + g := NewWithT(t) + nginxConf := renderAndFindNginxConfig(t, bootstrap.TLSProfile{ + Spec: configv1.TLSProfileSpec{ + MinTLSVersion: configv1.VersionTLS12, + Ciphers: []string{"ECDHE-RSA-AES128-GCM-SHA256", "ECDHE-ECDSA-CHACHA20-POLY1305"}, + }, + Adherence: configv1.TLSAdherencePolicyStrictAllComponents, + }) + + g.Expect(nginxConf).To(MatchRegexp(`ssl_protocols\s+TLSv1\.2 TLSv1\.3;`)) + g.Expect(nginxConf).To(MatchRegexp(`ssl_ciphers\s+ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-CHACHA20-POLY1305;`)) + g.Expect(nginxConf).To(MatchRegexp(`ssl_prefer_server_ciphers\s+on;`)) + }) + + t.Run("with TLS 1.3 and empty ciphers", func(t *testing.T) { + g := NewWithT(t) + nginxConf := renderAndFindNginxConfig(t, bootstrap.TLSProfile{ + Spec: configv1.TLSProfileSpec{ + MinTLSVersion: configv1.VersionTLS13, + Ciphers: nil, + }, + Adherence: configv1.TLSAdherencePolicyStrictAllComponents, + }) + + g.Expect(nginxConf).To(MatchRegexp(`ssl_protocols\s+TLSv1\.3;`)) + g.Expect(nginxConf).NotTo(ContainSubstring("ssl_ciphers")) + g.Expect(nginxConf).To(MatchRegexp(`ssl_prefer_server_ciphers\s+on;`)) + }) + }) + + t.Run("when adherence is LegacyAdheringComponentsOnly", func(t *testing.T) { + t.Run("should not render ssl_protocols and ssl_ciphers directives", func(t *testing.T) { + g := NewWithT(t) + nginxConf := renderAndFindNginxConfig(t, bootstrap.TLSProfile{ + Spec: configv1.TLSProfileSpec{ + MinTLSVersion: configv1.VersionTLS12, + Ciphers: []string{"ECDHE-RSA-AES128-GCM-SHA256"}, + }, + Adherence: configv1.TLSAdherencePolicyLegacyAdheringComponentsOnly, + }) + + g.Expect(nginxConf).NotTo(ContainSubstring("ssl_protocols")) + g.Expect(nginxConf).NotTo(ContainSubstring("ssl_ciphers")) + g.Expect(nginxConf).NotTo(ContainSubstring("ssl_prefer_server_ciphers")) + }) + }) +} diff --git a/pkg/network/tls.go b/pkg/network/tls.go index 00a48bf4b5..520339ccd9 100644 --- a/pkg/network/tls.go +++ b/pkg/network/tls.go @@ -21,17 +21,47 @@ const ( TLSMinVersionKey = "TLSMinVersion" // TLSCipherSuitesKey is the template data key for the comma-separated cipher suites TLSCipherSuitesKey = "TLSCipherSuites" + // NginxTLSProtocolsKey is the template data key for NGINX ssl_protocols directive + NginxTLSProtocolsKey = "NginxTLSProtocols" + // NginxTLSCiphersKey is the template data key for NGINX ssl_ciphers directive + NginxTLSCiphersKey = "NginxTLSCiphers" ) // addTLSInfoToRenderData adds TLS-related template data to the render data. +// It converts OpenSSL cipher names (from TLSProfile.Spec.Ciphers) to IANA format for Go components, +// and also adds NGINX-specific parameters using the original OpenSSL names. func addTLSInfoToRenderData(data map[string]interface{}, bootstrapResult *bootstrap.BootstrapResult, respectAdherence bool) { if respectAdherence && !crypto.ShouldHonorClusterTLSProfile(bootstrapResult.TLSProfile.Adherence) { data[UseTLSProfileKey] = false return } + // Convert OpenSSL cipher names to IANA names for Go components (kube-rbac-proxy, controller-runtime, etc.) + var ianaCiphers []string + for _, cipher := range bootstrapResult.TLSProfile.Spec.Ciphers { + // First try as IANA name directly (in case of custom profiles with IANA names) + if _, err := crypto.CipherSuite(cipher); err == nil { + ianaCiphers = append(ianaCiphers, cipher) + continue + } + + // Try converting from OpenSSL name to IANA name + converted := crypto.OpenSSLToIANACipherSuites([]string{cipher}) + if len(converted) > 0 { + ianaCiphers = append(ianaCiphers, converted...) + } else { + ianaCiphers = append(ianaCiphers, cipher) + } + } + + // Add Go-style TLS parameters (IANA cipher names, comma-separated) data[TLSMinVersionKey] = bootstrapResult.TLSProfile.Spec.MinTLSVersion - data[TLSCipherSuitesKey] = strings.Join(bootstrapResult.TLSProfile.Spec.Ciphers, ",") + data[TLSCipherSuitesKey] = strings.Join(ianaCiphers, ",") + + // Add NGINX-style TLS parameters (OpenSSL cipher names, colon-separated) + data[NginxTLSProtocolsKey] = convertTLSVersionToNginx(bootstrapResult.TLSProfile.Spec.MinTLSVersion) + data[NginxTLSCiphersKey] = strings.Join(bootstrapResult.TLSProfile.Spec.Ciphers, ":") + data[UseTLSProfileKey] = true } @@ -67,28 +97,25 @@ func toTLSProfile(apiServerSpec *configv1.APIServerSpec) (bootstrap.TLSProfile, profileSpec.Ciphers = nil } - // OCP uses OpenSSL names in its pre-defined TLS profile specs (although it's possible a user-defined custom profile - // spec could have IANA names). The components that accept cipher suites as an arg expect IANA names so convert them. - var convertedCiphers []string - for _, cipher := range profileSpec.Ciphers { - // First try as IANA name directly. - if _, err := crypto.CipherSuite(cipher); err == nil { - convertedCiphers = append(convertedCiphers, cipher) - continue - } - - // Try converting from OpenSSL name to IANA name. - ianaCiphers := crypto.OpenSSLToIANACipherSuites([]string{cipher}) - if len(ianaCiphers) > 0 { - convertedCiphers = append(convertedCiphers, ianaCiphers...) - } else { - convertedCiphers = append(convertedCiphers, cipher) - } - } - profileSpec.Ciphers = convertedCiphers - return bootstrap.TLSProfile{ Spec: profileSpec, Adherence: apiServerSpec.TLSAdherence, }, nil } + +// convertTLSVersionToNginx converts OpenShift TLS version to NGINX ssl_protocols format +func convertTLSVersionToNginx(version configv1.TLSProtocolVersion) string { + switch version { + case configv1.VersionTLS10: + return "TLSv1 TLSv1.1 TLSv1.2 TLSv1.3" + case configv1.VersionTLS11: + return "TLSv1.1 TLSv1.2 TLSv1.3" + case configv1.VersionTLS12: + return "TLSv1.2 TLSv1.3" + case configv1.VersionTLS13: + return "TLSv1.3" + default: + // Default to TLS 1.2 and 1.3 for unknown versions + return "TLSv1.2 TLSv1.3" + } +} diff --git a/pkg/network/tls_test.go b/pkg/network/tls_test.go index bab55cacc3..9a75ca3daa 100644 --- a/pkg/network/tls_test.go +++ b/pkg/network/tls_test.go @@ -43,6 +43,16 @@ func TestAddTLSInfoToRenderData(t *testing.T) { if v, ok := data[TLSCipherSuitesKey]; !ok || v != expectedCiphers { t.Errorf("Expected %s to be %v, got %v", TLSCipherSuitesKey, expectedCiphers, v) } + + // Check NGINX parameters + expectedNginxProtocols := "TLSv1.2 TLSv1.3" + if v, ok := data[NginxTLSProtocolsKey]; !ok || v != expectedNginxProtocols { + t.Errorf("Expected %s to be %q, got %v", NginxTLSProtocolsKey, expectedNginxProtocols, v) + } + expectedNginxCiphers := "TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384" + if v, ok := data[NginxTLSCiphersKey]; !ok || v != expectedNginxCiphers { + t.Errorf("Expected %s to be %q, got %v", NginxTLSCiphersKey, expectedNginxCiphers, v) + } }) } }) @@ -82,6 +92,13 @@ func TestAddTLSInfoToRenderData(t *testing.T) { if _, ok := data[TLSCipherSuitesKey]; ok { t.Errorf("Expected %s to not be present, but it was: %v", TLSCipherSuitesKey, data[TLSCipherSuitesKey]) } + // NGINX parameters should also not be set + if _, ok := data[NginxTLSProtocolsKey]; ok { + t.Errorf("Expected %s to not be present, but it was: %v", NginxTLSProtocolsKey, data[NginxTLSProtocolsKey]) + } + if _, ok := data[NginxTLSCiphersKey]; ok { + t.Errorf("Expected %s to not be present, but it was: %v", NginxTLSCiphersKey, data[NginxTLSCiphersKey]) + } }) t.Run("and not respecting adherence", func(t *testing.T) { @@ -109,6 +126,17 @@ func TestAddTLSInfoToRenderData(t *testing.T) { if v, ok := data[TLSCipherSuitesKey]; !ok || v != expectedCiphers { t.Errorf("Expected %s to be %v, got %v", TLSCipherSuitesKey, expectedCiphers, v) } + + // Check NGINX parameters for TLS 1.3 + expectedNginxProtocols := "TLSv1.3" + if v, ok := data[NginxTLSProtocolsKey]; !ok || v != expectedNginxProtocols { + t.Errorf("Expected %s to be %q, got %v", NginxTLSProtocolsKey, expectedNginxProtocols, v) + } + // TLS 1.3 ciphers should be empty (not configurable) + expectedNginxCiphers := "TLS_AES_128_GCM_SHA256" + if v, ok := data[NginxTLSCiphersKey]; !ok || v != expectedNginxCiphers { + t.Errorf("Expected %s to be %q, got %v", NginxTLSCiphersKey, expectedNginxCiphers, v) + } }) }) } @@ -136,5 +164,15 @@ func TestAddTLSInfoToRenderData(t *testing.T) { if v, ok := data[TLSCipherSuitesKey]; !ok || v != "" { t.Errorf("Expected %s to be empty string, got %v", TLSCipherSuitesKey, v) } + + // Check NGINX parameters with nil ciphers + expectedNginxProtocols := "TLSv1.2 TLSv1.3" + if v, ok := data[NginxTLSProtocolsKey]; !ok || v != expectedNginxProtocols { + t.Errorf("Expected %s to be %q, got %v", NginxTLSProtocolsKey, expectedNginxProtocols, v) + } + expectedNginxCiphers := "" + if v, ok := data[NginxTLSCiphersKey]; !ok || v != expectedNginxCiphers { + t.Errorf("Expected %s to be %q, got %v", NginxTLSCiphersKey, expectedNginxCiphers, v) + } }) }