From 880f04f29214a62f4dbe1b30a495da0eb232d8b7 Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Mon, 3 Aug 2026 16:46:35 +0530 Subject: [PATCH 01/10] CM-1225: apply cluster TLS profile to trust-manager webhook Honor apiserver tlsAdherence and inject --tls-min-version / --tls-cipher-suites onto the trust-manager Deployment when required. --- pkg/controller/trustmanager/controller.go | 9 + pkg/controller/trustmanager/deployment_tls.go | 75 ++++++ .../trustmanager/deployment_tls_test.go | 244 ++++++++++++++++++ pkg/controller/trustmanager/deployments.go | 3 + pkg/tlsprofile/tlsprofile.go | 26 ++ pkg/tlsprofile/tlsprofile_test.go | 44 ++++ test/e2e/tls_profile_test.go | 10 + test/e2e/utils_test.go | 8 +- 8 files changed, 418 insertions(+), 1 deletion(-) create mode 100644 pkg/controller/trustmanager/deployment_tls.go create mode 100644 pkg/controller/trustmanager/deployment_tls_test.go diff --git a/pkg/controller/trustmanager/controller.go b/pkg/controller/trustmanager/controller.go index 9445542f9..a998f8054 100644 --- a/pkg/controller/trustmanager/controller.go +++ b/pkg/controller/trustmanager/controller.go @@ -25,6 +25,8 @@ import ( certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + configv1 "github.com/openshift/api/config/v1" + v1alpha1 "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" "github.com/openshift/cert-manager-operator/pkg/controller/common" ) @@ -59,6 +61,7 @@ type Reconciler struct { // +kubebuilder:rbac:groups=trust.cert-manager.io,resources=bundles,verbs=get;list;watch // +kubebuilder:rbac:groups=trust.cert-manager.io,resources=bundles/finalizers,verbs=update // +kubebuilder:rbac:groups=trust.cert-manager.io,resources=bundles/status,verbs=patch +// +kubebuilder:rbac:groups=config.openshift.io,resources=apiservers,verbs=get;list;watch // New returns a new Reconciler instance. func New(mgr ctrl.Manager) (*Reconciler, error) { @@ -130,6 +133,11 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { object.GetName() == common.TrustedCABundleConfigMapName }) + // Reconcile when the cluster APIServer TLS profile or adherence changes. + clusterAPIServerPredicate := predicate.NewPredicateFuncs(func(object client.Object) bool { + return object.GetName() == apiServerClusterName + }) + return ctrl.NewControllerManagedBy(mgr). For(&v1alpha1.TrustManager{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). Named(ControllerName). @@ -145,6 +153,7 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { Watches(&certmanagerv1.Certificate{}, handler.EnqueueRequestsFromMapFunc(mapFunc), withIgnoreStatusUpdatePredicates). Watches(&certmanagerv1.Issuer{}, handler.EnqueueRequestsFromMapFunc(mapFunc), withIgnoreStatusUpdatePredicates). Watches(&admissionregistrationv1.ValidatingWebhookConfiguration{}, handler.EnqueueRequestsFromMapFunc(mapFunc), controllerManagedResourcePredicates). + Watches(&configv1.APIServer{}, handler.EnqueueRequestsFromMapFunc(mapFunc), builder.WithPredicates(clusterAPIServerPredicate)). Complete(r) } diff --git a/pkg/controller/trustmanager/deployment_tls.go b/pkg/controller/trustmanager/deployment_tls.go new file mode 100644 index 000000000..8a797f0de --- /dev/null +++ b/pkg/controller/trustmanager/deployment_tls.go @@ -0,0 +1,75 @@ +package trustmanager + +import ( + "fmt" + + appsv1 "k8s.io/api/apps/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "k8s.io/klog/v2" + + configv1 "github.com/openshift/api/config/v1" + libgocrypto "github.com/openshift/library-go/pkg/crypto" + + "github.com/openshift/cert-manager-operator/pkg/controller/common" + "github.com/openshift/cert-manager-operator/pkg/tlsprofile" +) + +const apiServerClusterName = "cluster" + +// applyClusterTLSProfile merges cluster TLS security profile flags onto the +// trust-manager webhook container when apiserver tlsAdherence requires it. +// When the APIServer resource is missing (non-OpenShift) or adherence does not +// require enforcement, this is a no-op. +func (r *Reconciler) applyClusterTLSProfile(deployment *appsv1.Deployment) error { + if r.CtrlClient == nil { + return nil + } + + apiServer := &configv1.APIServer{} + if err := r.Get(r.ctx, types.NamespacedName{Name: apiServerClusterName}, apiServer); err != nil { + if apierrors.IsNotFound(err) { + klog.V(4).Info("skipping cluster TLS profile for trust-manager: apiserver.config.openshift.io/cluster not found") + return nil + } + return fmt.Errorf("failed to get apiserver.config.openshift.io/cluster: %w", err) + } + + adherence := apiServer.Spec.TLSAdherence + if !libgocrypto.ShouldHonorClusterTLSProfile(adherence) { + klog.V(4).Infof("skipping cluster TLS profile for trust-manager: apiserver tlsAdherence=%q", adherence) + return nil + } + if adherence != configv1.TLSAdherencePolicyStrictAllComponents { + klog.Warningf("apiserver.config.openshift.io/cluster has unknown tlsAdherence %q; treating as StrictAllComponents for trust-manager", adherence) + } + + effective, err := tlsprofile.EffectiveSpec(apiServer.Spec.TLSSecurityProfile) + if err != nil { + return err + } + + return applyTrustManagerWebhookTLSArgs(deployment, effective) +} + +// applyTrustManagerWebhookTLSArgs merges profile-derived webhook TLS flags onto +// the trust-manager container. Exported for unit tests via package-level use. +func applyTrustManagerWebhookTLSArgs(deployment *appsv1.Deployment, spec *configv1.TLSProfileSpec) error { + extra := tlsprofile.TrustManagerWebhookTLSArgs(spec) + if len(extra) == 0 { + return nil + } + + for i := range deployment.Spec.Template.Spec.Containers { + if deployment.Spec.Template.Spec.Containers[i].Name != trustManagerContainerName { + continue + } + sourceArgs := deployment.Spec.Template.Spec.Containers[i].Args + if spec != nil && spec.MinTLSVersion == configv1.VersionTLS13 { + sourceArgs = common.StripArgsByKeys(sourceArgs, common.ArgKeysSet(tlsprofile.TrustManagerCipherSuiteArgKeys)) + } + deployment.Spec.Template.Spec.Containers[i].Args = common.MergeContainerArgs(sourceArgs, extra) + return nil + } + return fmt.Errorf("deployment %s/%s missing container %q", deployment.Namespace, deployment.Name, trustManagerContainerName) +} diff --git a/pkg/controller/trustmanager/deployment_tls_test.go b/pkg/controller/trustmanager/deployment_tls_test.go new file mode 100644 index 000000000..a998f4695 --- /dev/null +++ b/pkg/controller/trustmanager/deployment_tls_test.go @@ -0,0 +1,244 @@ +package trustmanager + +import ( + "context" + "strings" + "testing" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + + "sigs.k8s.io/controller-runtime/pkg/client" + + configv1 "github.com/openshift/api/config/v1" + + "github.com/openshift/cert-manager-operator/pkg/controller/common/fakes" + "github.com/openshift/cert-manager-operator/pkg/tlsprofile" +) + +func TestApplyTrustManagerWebhookTLSArgs(t *testing.T) { + tests := []struct { + name string + spec *configv1.TLSProfileSpec + wantKeys []string + wantAbsent []string + }{ + { + name: "intermediate sets min version and ciphers", + spec: &configv1.TLSProfileSpec{ + Ciphers: []string{"ECDHE-RSA-AES128-GCM-SHA256"}, + MinTLSVersion: configv1.VersionTLS12, + }, + wantKeys: []string{"--tls-min-version", "--tls-cipher-suites"}, + }, + { + name: "modern tls13 omits cipher suites", + spec: &configv1.TLSProfileSpec{ + Ciphers: []string{"TLS_AES_128_GCM_SHA256"}, + MinTLSVersion: configv1.VersionTLS13, + }, + wantKeys: []string{"--tls-min-version"}, + wantAbsent: []string{"--tls-cipher-suites"}, + }, + { + name: "nil spec is no-op", + spec: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dep := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: trustManagerDeploymentName, Namespace: operandNamespace}, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: trustManagerContainerName, + Args: []string{"--webhook-port=6443", "--tls-cipher-suites=STALE"}, + }}, + }, + }, + }, + } + if err := applyTrustManagerWebhookTLSArgs(dep, tt.spec); err != nil { + t.Fatalf("unexpected error: %v", err) + } + argMap := map[string]string{} + for _, a := range dep.Spec.Template.Spec.Containers[0].Args { + parts := strings.SplitN(a, "=", 2) + if len(parts) == 2 { + argMap[parts[0]] = parts[1] + } else { + argMap[parts[0]] = "" + } + } + for _, key := range tt.wantKeys { + if _, ok := argMap[key]; !ok { + t.Fatalf("expected arg key %q, got %#v", key, argMap) + } + } + for _, key := range tt.wantAbsent { + if _, ok := argMap[key]; ok { + t.Fatalf("did not expect arg key %q, got %#v", key, argMap) + } + } + if tt.spec != nil && tt.spec.MinTLSVersion == configv1.VersionTLS13 { + if argMap["--tls-min-version"] != "VersionTLS13" { + t.Fatalf("got min version %q", argMap["--tls-min-version"]) + } + } + }) + } +} + +func TestApplyClusterTLSProfile_adherence(t *testing.T) { + tests := []struct { + name string + apiServer *configv1.APIServer + wantTLSArgs bool + wantMinVer string + wantCipherKey bool + }{ + { + name: "strict modern injects tls13 min version without ciphers", + apiServer: &configv1.APIServer{ + ObjectMeta: metav1.ObjectMeta{Name: apiServerClusterName}, + Spec: configv1.APIServerSpec{ + TLSAdherence: configv1.TLSAdherencePolicyStrictAllComponents, + TLSSecurityProfile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileModernType, + }, + }, + }, + wantTLSArgs: true, + wantMinVer: "VersionTLS13", + wantCipherKey: false, + }, + { + name: "legacy adherence skips injection", + apiServer: &configv1.APIServer{ + ObjectMeta: metav1.ObjectMeta{Name: apiServerClusterName}, + Spec: configv1.APIServerSpec{ + TLSAdherence: configv1.TLSAdherencePolicyLegacyAdheringComponentsOnly, + TLSSecurityProfile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileModernType, + }, + }, + }, + wantTLSArgs: false, + }, + { + name: "missing apiserver skips injection", + apiServer: nil, + wantTLSArgs: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv(trustManagerImageNameEnvVarName, testImage) + r := testReconciler(t) + r.CtrlClient = fakeCtrlClientWithAPIServer(tt.apiServer) + + tm := testTrustManager().Build() + dep, err := r.getDeploymentObject(tm, getResourceLabels(tm), getResourceAnnotations(tm), "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + argMap := containerArgMap(dep) + _, hasMin := argMap["--tls-min-version"] + _, hasCipher := argMap["--tls-cipher-suites"] + if tt.wantTLSArgs != hasMin { + t.Fatalf("wantTLSArgs=%v hasMin=%v args=%#v", tt.wantTLSArgs, hasMin, argMap) + } + if tt.wantTLSArgs && argMap["--tls-min-version"] != tt.wantMinVer { + t.Fatalf("min version got %q want %q", argMap["--tls-min-version"], tt.wantMinVer) + } + if hasCipher != tt.wantCipherKey { + t.Fatalf("wantCipherKey=%v hasCipher=%v", tt.wantCipherKey, hasCipher) + } + }) + } +} + +func TestApplyClusterTLSProfile_intermediateCiphers(t *testing.T) { + t.Setenv(trustManagerImageNameEnvVarName, testImage) + apiServer := &configv1.APIServer{ + ObjectMeta: metav1.ObjectMeta{Name: apiServerClusterName}, + Spec: configv1.APIServerSpec{ + TLSAdherence: configv1.TLSAdherencePolicyStrictAllComponents, + TLSSecurityProfile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileIntermediateType, + }, + }, + } + r := testReconciler(t) + r.CtrlClient = fakeCtrlClientWithAPIServer(apiServer) + + tm := testTrustManager().Build() + dep, err := r.getDeploymentObject(tm, getResourceLabels(tm), getResourceAnnotations(tm), "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + expected, err := tlsprofile.EffectiveSpec(apiServer.Spec.TLSSecurityProfile) + if err != nil { + t.Fatal(err) + } + want := tlsprofile.TrustManagerWebhookTLSArgs(expected) + gotArgs := dep.Spec.Template.Spec.Containers[0].Args + for _, w := range want { + found := false + for _, g := range gotArgs { + if g == w { + found = true + break + } + } + if !found { + t.Fatalf("expected arg %q in %#v", w, gotArgs) + } + } +} + +func fakeCtrlClientWithAPIServer(apiServer *configv1.APIServer) *fakes.FakeCtrlClient { + mock := &fakes.FakeCtrlClient{} + mock.GetCalls(func(_ context.Context, key client.ObjectKey, obj client.Object) error { + if key.Name != apiServerClusterName { + return apierrors.NewNotFound(schema.GroupResource{Group: configv1.GroupName, Resource: "apiservers"}, key.Name) + } + if apiServer == nil { + return apierrors.NewNotFound(schema.GroupResource{Group: configv1.GroupName, Resource: "apiservers"}, key.Name) + } + dst, ok := obj.(*configv1.APIServer) + if !ok { + return apierrors.NewBadRequest("unexpected object type") + } + apiServer.DeepCopyInto(dst) + return nil + }) + return mock +} + +func containerArgMap(dep *appsv1.Deployment) map[string]string { + argMap := map[string]string{} + for _, c := range dep.Spec.Template.Spec.Containers { + if c.Name != trustManagerContainerName { + continue + } + for _, a := range c.Args { + parts := strings.SplitN(a, "=", 2) + if len(parts) == 2 { + argMap[parts[0]] = parts[1] + } else { + argMap[parts[0]] = "" + } + } + } + return argMap +} diff --git a/pkg/controller/trustmanager/deployments.go b/pkg/controller/trustmanager/deployments.go index da96689d4..a6e6d372c 100644 --- a/pkg/controller/trustmanager/deployments.go +++ b/pkg/controller/trustmanager/deployments.go @@ -58,6 +58,9 @@ func (r *Reconciler) getDeploymentObject(trustManager *v1alpha1.TrustManager, re updateResourceAnnotations(deployment, resourceAnnotations) updatePodTemplateLabels(deployment, resourceLabels) updateDeploymentArgs(deployment, trustManager) + if err := r.applyClusterTLSProfile(deployment); err != nil { + return nil, err + } updateServiceAccountName(deployment) updateTLSSecretVolume(deployment) diff --git a/pkg/tlsprofile/tlsprofile.go b/pkg/tlsprofile/tlsprofile.go index 5be7aca56..715efd152 100644 --- a/pkg/tlsprofile/tlsprofile.go +++ b/pkg/tlsprofile/tlsprofile.go @@ -49,6 +49,12 @@ var CertManagerCipherSuiteArgKeys = []string{ "--metrics-tls-cipher-suites", } +// TrustManagerCipherSuiteArgKeys are trust-manager webhook flags that must not be +// set when the effective minimum TLS version is 1.3. +var TrustManagerCipherSuiteArgKeys = []string{ + "--tls-cipher-suites", +} + // CertManagerWebhookTLSArgs returns cert-manager-webhook flags for the main HTTPS // listener and the metrics TLS listener when TLS is enabled for metrics. func CertManagerWebhookTLSArgs(spec *configv1.TLSProfileSpec) []string { @@ -90,6 +96,26 @@ func CertManagerOperandMetricsTLSArgs(spec *configv1.TLSProfileSpec) []string { } } +// TrustManagerWebhookTLSArgs returns trust-manager webhook TLS flags for the +// cluster TLS security profile. Metrics remain plain HTTP upstream and are out +// of scope. +func TrustManagerWebhookTLSArgs(spec *configv1.TLSProfileSpec) []string { + if spec == nil { + return []string{} + } + minVersion := string(spec.MinTLSVersion) + if spec.MinTLSVersion == configv1.VersionTLS13 { + return []string{ + "--tls-min-version=" + minVersion, + } + } + ciphers := joinIANACiphers(spec.Ciphers) + return []string{ + "--tls-min-version=" + minVersion, + "--tls-cipher-suites=" + ciphers, + } +} + func joinIANACiphers(openSSLNames []string) string { iana := libgocrypto.OpenSSLToIANACipherSuites(openSSLNames) return strings.Join(iana, ",") diff --git a/pkg/tlsprofile/tlsprofile_test.go b/pkg/tlsprofile/tlsprofile_test.go index 5778609c2..65e982ac2 100644 --- a/pkg/tlsprofile/tlsprofile_test.go +++ b/pkg/tlsprofile/tlsprofile_test.go @@ -158,3 +158,47 @@ func TestCertManagerOperandMetricsTLSArgs_tls13OmitsCipherFlags(t *testing.T) { t.Fatalf("unexpected args: %#v", args) } } + +func TestTrustManagerWebhookTLSArgs_nilSpecReturnsEmpty(t *testing.T) { + args := TrustManagerWebhookTLSArgs(nil) + if len(args) != 0 { + t.Fatalf("expected empty args, got %#v", args) + } +} + +func TestTrustManagerWebhookTLSArgs_joinsCiphers(t *testing.T) { + spec := &configv1.TLSProfileSpec{ + Ciphers: []string{"ECDHE-RSA-AES128-GCM-SHA256", "TLS_AES_128_GCM_SHA256"}, + MinTLSVersion: configv1.VersionTLS12, + } + args := TrustManagerWebhookTLSArgs(spec) + argMap := map[string]string{} + for _, a := range args { + parts := strings.SplitN(a, "=", 2) + if len(parts) != 2 { + t.Fatalf("bad arg %q", a) + } + argMap[parts[0]] = parts[1] + } + if argMap["--tls-min-version"] != "VersionTLS12" { + t.Fatalf("unexpected min version: %q", argMap["--tls-min-version"]) + } + if !strings.Contains(argMap["--tls-cipher-suites"], "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256") { + t.Fatalf("unexpected tls ciphers: %q", argMap["--tls-cipher-suites"]) + } +} + +func TestTrustManagerWebhookTLSArgs_tls13OmitsCipherFlags(t *testing.T) { + spec := &configv1.TLSProfileSpec{ + Ciphers: []string{ + "TLS_AES_128_GCM_SHA256", + "TLS_AES_256_GCM_SHA384", + "TLS_CHACHA20_POLY1305_SHA256", + }, + MinTLSVersion: configv1.VersionTLS13, + } + args := TrustManagerWebhookTLSArgs(spec) + if len(args) != 1 || args[0] != "--tls-min-version=VersionTLS13" { + t.Fatalf("unexpected args: %#v", args) + } +} diff --git a/test/e2e/tls_profile_test.go b/test/e2e/tls_profile_test.go index 46c612cc6..e95b89bb1 100644 --- a/test/e2e/tls_profile_test.go +++ b/test/e2e/tls_profile_test.go @@ -11,6 +11,7 @@ import ( "github.com/openshift/cert-manager-operator/pkg/tlsprofile" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -67,5 +68,14 @@ var _ = Describe("Cluster TLS security profile", Label("Platform:Generic", "Feat err := verifyOperandTLSArgsMatchClusterProfile(name, expectedSpec) Expect(err).NotTo(HaveOccurred(), "deployment %s", name) } + + By("verifying trust-manager webhook TLS flags when the deployment is present") + _, tmErr := k8sClientSet.AppsV1().Deployments(operandNamespace).Get(ctx, "trust-manager", metav1.GetOptions{}) + if apierrors.IsNotFound(tmErr) { + Skip("trust-manager deployment not present; skipping trust-manager TLS profile verification") + } + Expect(tmErr).NotTo(HaveOccurred(), "failed to get trust-manager deployment") + err = verifyOperandTLSArgsMatchClusterProfile("trust-manager", expectedSpec) + Expect(err).NotTo(HaveOccurred(), "deployment trust-manager") }) }) diff --git a/test/e2e/utils_test.go b/test/e2e/utils_test.go index 6115bec6b..1bff8322c 100644 --- a/test/e2e/utils_test.go +++ b/test/e2e/utils_test.go @@ -1824,6 +1824,8 @@ func expectedOperandTLSArgs(deploymentName string, spec *configapiv1.TLSProfileS return tlsprofile.CertManagerWebhookTLSArgs(spec) case certmanagerControllerDeployment, certmanagerCAinjectorDeployment: return tlsprofile.CertManagerOperandMetricsTLSArgs(spec) + case "trust-manager": + return tlsprofile.TrustManagerWebhookTLSArgs(spec) default: return nil } @@ -1857,8 +1859,12 @@ func verifyOperandTLSArgsMatchClusterProfile(deploymentName string, spec *config return false, fmt.Errorf("deployment %q has no containers", deploymentName) } + cipherKeys := tlsprofile.CertManagerCipherSuiteArgKeys + if deploymentName == "trust-manager" { + cipherKeys = tlsprofile.TrustManagerCipherSuiteArgKeys + } for _, arg := range deployment.Spec.Template.Spec.Containers[0].Args { - for _, key := range tlsprofile.CertManagerCipherSuiteArgKeys { + for _, key := range cipherKeys { if strings.HasPrefix(arg, key+"=") { return false, nil } From 6187bde81a7ab4e0bb9d368ad81f8c5f13f5b064 Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Mon, 3 Aug 2026 16:48:28 +0530 Subject: [PATCH 02/10] CM-954: honor cluster TLS profile on operator metrics :8443 Apply apiserver tlsSecurityProfile to library-go HTTPServingInfo at startup when tlsAdherence requires it, and wire serving-cert mounts for the metrics Service. --- ...er-manager-metrics-service_v1_service.yaml | 2 + ...anager-operator.clusterserviceversion.yaml | 7 + config/manager/manager.yaml | 7 + config/rbac/auth_proxy_service.yaml | 2 + pkg/cmd/operator/cmd.go | 139 +++++++++++++++++- pkg/tlsprofile/serving.go | 70 +++++++++ pkg/tlsprofile/serving_test.go | 49 ++++++ pkg/tlsprofile/tlsprofile.go | 35 +++++ 8 files changed, 307 insertions(+), 4 deletions(-) create mode 100644 pkg/tlsprofile/serving.go create mode 100644 pkg/tlsprofile/serving_test.go diff --git a/bundle/manifests/cert-manager-operator-controller-manager-metrics-service_v1_service.yaml b/bundle/manifests/cert-manager-operator-controller-manager-metrics-service_v1_service.yaml index e76b8e454..ce9b34508 100644 --- a/bundle/manifests/cert-manager-operator-controller-manager-metrics-service_v1_service.yaml +++ b/bundle/manifests/cert-manager-operator-controller-manager-metrics-service_v1_service.yaml @@ -1,6 +1,8 @@ apiVersion: v1 kind: Service metadata: + annotations: + service.beta.openshift.io/serving-cert-secret-name: cert-manager-operator-serving-cert creationTimestamp: null labels: app.kubernetes.io/created-by: cert-manager-operator diff --git a/bundle/manifests/cert-manager-operator.clusterserviceversion.yaml b/bundle/manifests/cert-manager-operator.clusterserviceversion.yaml index aeb2b2fff..8bc124a24 100644 --- a/bundle/manifests/cert-manager-operator.clusterserviceversion.yaml +++ b/bundle/manifests/cert-manager-operator.clusterserviceversion.yaml @@ -848,6 +848,9 @@ spec: volumeMounts: - mountPath: /tmp name: tmp + - mountPath: /var/run/secrets/serving-cert + name: serving-cert + readOnly: true securityContext: runAsNonRoot: true seccompProfile: @@ -857,6 +860,10 @@ spec: volumes: - emptyDir: {} name: tmp + - name: serving-cert + secret: + optional: true + secretName: cert-manager-operator-serving-cert permissions: - rules: - apiGroups: diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index af071a2b7..411d243cd 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -123,8 +123,15 @@ spec: volumeMounts: - name: tmp mountPath: /tmp + - name: serving-cert + mountPath: /var/run/secrets/serving-cert + readOnly: true serviceAccountName: controller-manager terminationGracePeriodSeconds: 10 volumes: - name: tmp emptyDir: {} + - name: serving-cert + secret: + secretName: cert-manager-operator-serving-cert + optional: true diff --git a/config/rbac/auth_proxy_service.yaml b/config/rbac/auth_proxy_service.yaml index 3afdfb7d9..5057d176b 100644 --- a/config/rbac/auth_proxy_service.yaml +++ b/config/rbac/auth_proxy_service.yaml @@ -1,6 +1,8 @@ apiVersion: v1 kind: Service metadata: + annotations: + service.beta.openshift.io/serving-cert-secret-name: cert-manager-operator-serving-cert labels: control-plane: controller-manager app.kubernetes.io/name: service diff --git a/pkg/cmd/operator/cmd.go b/pkg/cmd/operator/cmd.go index 3321b121c..ba5723902 100644 --- a/pkg/cmd/operator/cmd.go +++ b/pkg/cmd/operator/cmd.go @@ -2,23 +2,86 @@ package operator import ( "context" + "math/rand" + "os" + "time" + + "github.com/spf13/cobra" + "k8s.io/apiserver/pkg/server" + "k8s.io/component-base/logs" + "k8s.io/klog/v2" + "k8s.io/utils/clock" "github.com/openshift/cert-manager-operator/pkg/operator" + "github.com/openshift/cert-manager-operator/pkg/tlsprofile" "github.com/openshift/cert-manager-operator/pkg/version" "github.com/openshift/library-go/pkg/controller/controllercmd" - "github.com/spf13/cobra" - "k8s.io/utils/clock" + "github.com/openshift/library-go/pkg/controller/fileobserver" + "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/serviceability" ) func NewOperator() *cobra.Command { - cmd := controllercmd.NewControllerCommandConfig( + cc := controllercmd.NewControllerCommandConfig( "cert-manager-operator", version.Get(), operator.RunOperator, clock.RealClock{}, - ).NewCommandWithContext(context.TODO()) + ) + + cmd := cc.NewCommandWithContext(context.TODO()) cmd.Use = "start" cmd.Short = "Start the cert-manager Operator" + + // Replace the default Run so we can apply the cluster TLS profile to the + // metrics serving config before the HTTPS listener is created. + cmd.Run = func(cmd *cobra.Command, args []string) { + rand.Seed(time.Now().UTC().UnixNano()) + logs.InitLogs() + defer logs.FlushLogs() + defer serviceability.BehaviorOnPanic(os.Getenv("OPENSHIFT_ON_PANIC"), version.Get())() + defer serviceability.Profile(os.Getenv("OPENSHIFT_PROFILE")).Stop() + serviceability.StartProfiler() + + shutdownCtx, cancel := context.WithCancel(context.Background()) + shutdownHandler := server.SetupSignalHandler() + go func() { + defer cancel() + <-shutdownHandler + klog.Infof("Received SIGTERM or SIGINT signal, shutting down controller.") + }() + + ctx, terminate := context.WithCancel(shutdownCtx) + defer terminate() + + terminateOnFiles, _ := cmd.Flags().GetStringArray("terminate-on-files") + if len(terminateOnFiles) > 0 { + obs, err := fileobserver.NewObserver(10 * time.Second) + if err != nil { + klog.Fatal(err) + } + files := map[string][]byte{} + for _, fn := range terminateOnFiles { + fileBytes, err := os.ReadFile(fn) + if err != nil { + klog.Warningf("Unable to read initial content of %q: %v", fn, err) + continue + } + files[fn] = fileBytes + } + obs.AddReactor(func(filename string, action fileobserver.ActionType) error { + klog.Infof("exiting because %q changed", filename) + terminate() + return nil + }, files, terminateOnFiles...) + go obs.Run(shutdownHandler) + } + + if err := startControllerWithClusterTLS(ctx, cc, cmd); err != nil { + klog.Fatal(err) + } + } + cmd.Flags().StringVar(&operator.TrustedCAConfigMapName, "trusted-ca-configmap", "", "The name of the config map containing TLS CA(s) which should be trusted by the controller's containers. PEM encoded file under \"ca-bundle.crt\" key is expected.") cmd.Flags().StringVar(&operator.CloudCredentialSecret, "cloud-credentials-secret", "", "The name of the secret containing cloud credentials for authenticating using cert-manager ambient credentials mode.") @@ -34,3 +97,71 @@ These features provide early access to upcoming product features, enabling customers to test functionality and provide feedback during the development process.`) return cmd } + +func startControllerWithClusterTLS(ctx context.Context, c *controllercmd.ControllerCommandConfig, cmd *cobra.Command) error { + unstructuredConfig, config, configContent, err := c.Config() + if err != nil { + return err + } + + startingFileContent, observedFiles, err := c.AddDefaultRotationToConfig(config, configContent) + if err != nil { + return err + } + + if listen, _ := cmd.Flags().GetString("listen"); len(listen) != 0 { + config.ServingInfo.BindAddress = listen + } + + kubeConfigFile, _ := cmd.Flags().GetString("kubeconfig") + namespace, _ := cmd.Flags().GetString("namespace") + + if !c.DisableServing { + restConfig, err := tlsprofile.RESTConfigFromKubeConfig(kubeConfigFile) + if err != nil { + klog.V(2).Infof("unable to build rest config for cluster TLS profile lookup: %v", err) + } else if err := tlsprofile.ApplyClusterProfileToHTTPServingInfo(ctx, restConfig, &config.ServingInfo); err != nil { + return err + } + } + + exitOnChangeReactorCh := make(chan struct{}) + controllerCtx, cancel := context.WithCancel(ctx) + go func() { + select { + case <-exitOnChangeReactorCh: + cancel() + case <-ctx.Done(): + cancel() + } + }() + + config.LeaderElection.Disable = c.DisableLeaderElection + config.LeaderElection.LeaseDuration = c.LeaseDuration + config.LeaderElection.RenewDeadline = c.RenewDeadline + config.LeaderElection.RetryPeriod = c.RetryPeriod + + builder := controllercmd.NewController("cert-manager-operator", operator.RunOperator, clock.RealClock{}). + WithKubeConfigFile(kubeConfigFile, nil). + WithComponentNamespace(namespace). + WithLeaderElection(config.LeaderElection, namespace, "cert-manager-operator-lock"). + WithVersion(version.Get()). + WithEventRecorderOptions(events.RecommendedClusterSingletonCorrelatorOptions()). + WithRestartOnChange(exitOnChangeReactorCh, startingFileContent, observedFiles...) + + if !c.DisableServing { + builder = builder.WithServer(config.ServingInfo, config.Authentication, config.Authorization) + if c.EnableHTTP2 { + builder = builder.WithHTTP2() + } + if c.SkipInClusterAuthenticationLookup { + builder = builder.WithSkipInClusterAuthenticationLookup() + } + } + + if c.TopologyDetector != nil { + builder = builder.WithTopologyDetector(c.TopologyDetector) + } + + return builder.Run(controllerCtx, unstructuredConfig) +} diff --git a/pkg/tlsprofile/serving.go b/pkg/tlsprofile/serving.go new file mode 100644 index 000000000..1cfe6f3cd --- /dev/null +++ b/pkg/tlsprofile/serving.go @@ -0,0 +1,70 @@ +package tlsprofile + +import ( + "context" + "fmt" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" + "k8s.io/klog/v2" + + configv1 "github.com/openshift/api/config/v1" + configv1client "github.com/openshift/client-go/config/clientset/versioned" + libgocrypto "github.com/openshift/library-go/pkg/crypto" +) + +const apiServerClusterName = "cluster" + +// ApplyClusterProfileToHTTPServingInfo reads apiserver.config.openshift.io/cluster +// and, when tlsAdherence requires enforcement, applies the effective TLS profile +// to serving. Missing APIServer (non-OpenShift) or non-enforcing adherence leaves +// serving unchanged. +func ApplyClusterProfileToHTTPServingInfo(ctx context.Context, restConfig *rest.Config, serving *configv1.HTTPServingInfo) error { + if restConfig == nil { + return fmt.Errorf("rest config is nil") + } + if serving == nil { + return fmt.Errorf("HTTPServingInfo is nil") + } + + configClient, err := configv1client.NewForConfig(restConfig) + if err != nil { + return fmt.Errorf("failed to create config client: %w", err) + } + + apiServer, err := configClient.ConfigV1().APIServers().Get(ctx, apiServerClusterName, metav1.GetOptions{}) + if err != nil { + // Non-OpenShift or RBAC/API unavailable: keep library-go defaults. + klog.V(2).Infof("skipping cluster TLS profile for operator serving: failed to get apiserver/cluster: %v", err) + return nil + } + + adherence := apiServer.Spec.TLSAdherence + if !libgocrypto.ShouldHonorClusterTLSProfile(adherence) { + klog.V(2).Infof("skipping cluster TLS profile for operator serving: apiserver tlsAdherence=%q", adherence) + return nil + } + if adherence != configv1.TLSAdherencePolicyStrictAllComponents { + klog.Warningf("apiserver.config.openshift.io/cluster has unknown tlsAdherence %q; treating as StrictAllComponents for operator serving", adherence) + } + + effective, err := EffectiveSpec(apiServer.Spec.TLSSecurityProfile) + if err != nil { + return err + } + if err := ApplyToHTTPServingInfo(serving, effective); err != nil { + return err + } + klog.V(2).Infof("applied cluster TLS profile to operator serving: minTLSVersion=%s ciphers=%d", serving.MinTLSVersion, len(serving.CipherSuites)) + return nil +} + +// RESTConfigFromKubeConfig returns an in-cluster rest.Config when kubeConfigFile +// is empty, otherwise loads the given kubeconfig path. +func RESTConfigFromKubeConfig(kubeConfigFile string) (*rest.Config, error) { + if len(kubeConfigFile) == 0 { + return rest.InClusterConfig() + } + return clientcmd.BuildConfigFromFlags("", kubeConfigFile) +} diff --git a/pkg/tlsprofile/serving_test.go b/pkg/tlsprofile/serving_test.go new file mode 100644 index 000000000..1a7f19bbf --- /dev/null +++ b/pkg/tlsprofile/serving_test.go @@ -0,0 +1,49 @@ +package tlsprofile + +import ( + "testing" + + configv1 "github.com/openshift/api/config/v1" +) + +func TestApplyToHTTPServingInfo(t *testing.T) { + t.Run("intermediate", func(t *testing.T) { + spec, err := EffectiveSpec(&configv1.TLSSecurityProfile{Type: configv1.TLSProfileIntermediateType}) + if err != nil { + t.Fatal(err) + } + serving := &configv1.HTTPServingInfo{} + if err := ApplyToHTTPServingInfo(serving, spec); err != nil { + t.Fatal(err) + } + if serving.MinTLSVersion != string(configv1.VersionTLS12) { + t.Fatalf("min version: %q", serving.MinTLSVersion) + } + if len(serving.CipherSuites) == 0 { + t.Fatal("expected ciphers") + } + }) + + t.Run("modern tls13 keeps non-empty ciphers to block defaults", func(t *testing.T) { + spec, err := EffectiveSpec(&configv1.TLSSecurityProfile{Type: configv1.TLSProfileModernType}) + if err != nil { + t.Fatal(err) + } + serving := &configv1.HTTPServingInfo{} + if err := ApplyToHTTPServingInfo(serving, spec); err != nil { + t.Fatal(err) + } + if serving.MinTLSVersion != string(configv1.VersionTLS13) { + t.Fatalf("min version: %q", serving.MinTLSVersion) + } + if len(serving.CipherSuites) == 0 { + t.Fatal("expected non-empty cipher list so library-go defaults are not reapplied") + } + }) + + t.Run("nil serving", func(t *testing.T) { + if err := ApplyToHTTPServingInfo(nil, &configv1.TLSProfileSpec{MinTLSVersion: configv1.VersionTLS12}); err == nil { + t.Fatal("expected error") + } + }) +} diff --git a/pkg/tlsprofile/tlsprofile.go b/pkg/tlsprofile/tlsprofile.go index 715efd152..3569709cc 100644 --- a/pkg/tlsprofile/tlsprofile.go +++ b/pkg/tlsprofile/tlsprofile.go @@ -120,3 +120,38 @@ func joinIANACiphers(openSSLNames []string) string { iana := libgocrypto.OpenSSLToIANACipherSuites(openSSLNames) return strings.Join(iana, ",") } + +// ApplyToHTTPServingInfo sets MinTLSVersion and CipherSuites on serving info from +// a resolved cluster TLS profile. For TLS 1.3, cipher suites are cleared so +// library-go defaults are not re-applied over an intentional empty list when +// callers set MinTLSVersion first; callers should set both fields together and +// rely on WithServer's SetRecommended* only filling empty values. +func ApplyToHTTPServingInfo(serving *configv1.HTTPServingInfo, spec *configv1.TLSProfileSpec) error { + if serving == nil { + return fmt.Errorf("HTTPServingInfo is nil") + } + if spec == nil { + return fmt.Errorf("TLS profile spec is nil") + } + serving.MinTLSVersion = string(spec.MinTLSVersion) + if spec.MinTLSVersion == configv1.VersionTLS13 { + // TLS 1.3 ignores CipherSuites in Go; leave empty so defaults are not + // forced to Intermediate TLS 1.2 suites after MinTLSVersion is set. + // Set a single TLS 1.3 suite name placeholder? No - empty means + // SetRecommended will fill Intermediate ciphers. To prevent that, + // set Modern profile's TLS 1.3 cipher names explicitly when available. + serving.CipherSuites = append([]string(nil), libgocrypto.OpenSSLToIANACipherSuites(spec.Ciphers)...) + if len(serving.CipherSuites) == 0 { + // Keep a non-empty list so SetRecommendedHTTPServingInfoDefaults does + // not overwrite with Intermediate defaults; TLS 1.3 ignores these. + serving.CipherSuites = []string{"TLS_AES_128_GCM_SHA256"} + } + return nil + } + iana := libgocrypto.OpenSSLToIANACipherSuites(spec.Ciphers) + if len(spec.Ciphers) > 0 && len(iana) == 0 { + return fmt.Errorf("no cipher suites after OpenSSL→IANA mapping") + } + serving.CipherSuites = iana + return nil +} From a2391db13a851f5998abb6d0e1f0c6abb13c11c0 Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Mon, 3 Aug 2026 16:51:13 +0530 Subject: [PATCH 03/10] CM-954: enable HTTPS metrics TLS on cert-manager operands Turn on dynamic metrics serving certificates for controller, webhook, and cainjector so --metrics-tls-* profile flags apply to real TLS on :9402, with RBAC for the shared metrics CA secret. --- .../cert-manager-cainjector-deployment.yaml | 1 + .../controller/cert-manager-deployment.yaml | 1 + ...rt-manager-metrics-dynamic-serving-rb.yaml | 25 +++++ ...-manager-metrics-dynamic-serving-role.yaml | 29 ++++++ .../cert-manager-webhook-deployment.yaml | 1 + .../cert_manager_controller_deployment.go | 2 + .../certmanager/deployment_metrics_tls.go | 59 +++++++++++ .../deployment_metrics_tls_test.go | 90 +++++++++++++++++ .../generic_deployment_controller.go | 3 + pkg/operator/assets/bindata.go | 97 +++++++++++++++++++ 10 files changed, 308 insertions(+) create mode 100644 bindata/cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-rb.yaml create mode 100644 bindata/cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-role.yaml create mode 100644 pkg/controller/certmanager/deployment_metrics_tls.go create mode 100644 pkg/controller/certmanager/deployment_metrics_tls_test.go diff --git a/bindata/cert-manager-deployment/cainjector/cert-manager-cainjector-deployment.yaml b/bindata/cert-manager-deployment/cainjector/cert-manager-cainjector-deployment.yaml index eadb8fb89..ccf375bb5 100644 --- a/bindata/cert-manager-deployment/cainjector/cert-manager-cainjector-deployment.yaml +++ b/bindata/cert-manager-deployment/cainjector/cert-manager-cainjector-deployment.yaml @@ -21,6 +21,7 @@ spec: annotations: prometheus.io/path: /metrics prometheus.io/port: "9402" + prometheus.io/scheme: https prometheus.io/scrape: "true" labels: app: cainjector diff --git a/bindata/cert-manager-deployment/controller/cert-manager-deployment.yaml b/bindata/cert-manager-deployment/controller/cert-manager-deployment.yaml index a45746a57..2ef959647 100644 --- a/bindata/cert-manager-deployment/controller/cert-manager-deployment.yaml +++ b/bindata/cert-manager-deployment/controller/cert-manager-deployment.yaml @@ -21,6 +21,7 @@ spec: annotations: prometheus.io/path: /metrics prometheus.io/port: "9402" + prometheus.io/scheme: https prometheus.io/scrape: "true" labels: app: cert-manager diff --git a/bindata/cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-rb.yaml b/bindata/cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-rb.yaml new file mode 100644 index 000000000..698759898 --- /dev/null +++ b/bindata/cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-rb.yaml @@ -0,0 +1,25 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + labels: + app: cert-manager + app.kubernetes.io/component: controller + app.kubernetes.io/instance: cert-manager + app.kubernetes.io/name: cert-manager + app.kubernetes.io/version: v1.20.3 + name: cert-manager-metrics-dynamic-serving + namespace: cert-manager +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: cert-manager-metrics-dynamic-serving +subjects: + - kind: ServiceAccount + name: cert-manager + namespace: cert-manager + - kind: ServiceAccount + name: cert-manager-webhook + namespace: cert-manager + - kind: ServiceAccount + name: cert-manager-cainjector + namespace: cert-manager diff --git a/bindata/cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-role.yaml b/bindata/cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-role.yaml new file mode 100644 index 000000000..e16013379 --- /dev/null +++ b/bindata/cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-role.yaml @@ -0,0 +1,29 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + labels: + app: cert-manager + app.kubernetes.io/component: controller + app.kubernetes.io/instance: cert-manager + app.kubernetes.io/name: cert-manager + app.kubernetes.io/version: v1.20.3 + name: cert-manager-metrics-dynamic-serving + namespace: cert-manager +rules: + - apiGroups: + - "" + resourceNames: + - cert-manager-metrics-ca + resources: + - secrets + verbs: + - get + - list + - watch + - update + - apiGroups: + - "" + resources: + - secrets + verbs: + - create diff --git a/bindata/cert-manager-deployment/webhook/cert-manager-webhook-deployment.yaml b/bindata/cert-manager-deployment/webhook/cert-manager-webhook-deployment.yaml index 127e5ec08..82711bbec 100644 --- a/bindata/cert-manager-deployment/webhook/cert-manager-webhook-deployment.yaml +++ b/bindata/cert-manager-deployment/webhook/cert-manager-webhook-deployment.yaml @@ -21,6 +21,7 @@ spec: annotations: prometheus.io/path: /metrics prometheus.io/port: "9402" + prometheus.io/scheme: https prometheus.io/scrape: "true" labels: app: webhook diff --git a/pkg/controller/certmanager/cert_manager_controller_deployment.go b/pkg/controller/certmanager/cert_manager_controller_deployment.go index 4042f4490..0e77a423b 100644 --- a/pkg/controller/certmanager/cert_manager_controller_deployment.go +++ b/pkg/controller/certmanager/cert_manager_controller_deployment.go @@ -47,6 +47,8 @@ var ( "cert-manager-deployment/controller/cert-manager-tokenrequest-rb.yaml", "cert-manager-deployment/controller/cert-manager-tokenrequest-role.yaml", "cert-manager-deployment/controller/cert-manager-view-cr.yaml", + "cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-role.yaml", + "cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-rb.yaml", "cert-manager-deployment/cert-manager/cert-manager-controller-approve-cert-manager-io-cr.yaml", "cert-manager-deployment/cert-manager/cert-manager-controller-approve-cert-manager-io-crb.yaml", "cert-manager-deployment/cert-manager/cert-manager-controller-certificatesigningrequests-cr.yaml", diff --git a/pkg/controller/certmanager/deployment_metrics_tls.go b/pkg/controller/certmanager/deployment_metrics_tls.go new file mode 100644 index 000000000..8c9f2a87c --- /dev/null +++ b/pkg/controller/certmanager/deployment_metrics_tls.go @@ -0,0 +1,59 @@ +package certmanager + +import ( + "fmt" + + appsv1 "k8s.io/api/apps/v1" + + operatorv1 "github.com/openshift/api/operator/v1" + + "github.com/openshift/cert-manager-operator/pkg/controller/common" +) + +const ( + metricsDynamicServingCASecretName = "cert-manager-metrics-ca" +) + +// withOperandMetricsTLS enables HTTPS on the cert-manager operand metrics +// listeners (port 9402) using cert-manager's dynamic metrics serving CA. +// Cipher/min-version flags continue to come from WithClusterTLSProfileFromAPIServer. +func withOperandMetricsTLS(_ *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { + if len(deployment.Spec.Template.Spec.Containers) == 0 { + return fmt.Errorf("deployment %s/%s has no containers", deployment.Namespace, deployment.Name) + } + + extra, ok := operandMetricsTLSArgs(deployment.Name) + if !ok { + return nil + } + + container := &deployment.Spec.Template.Spec.Containers[0] + container.Args = common.MergeContainerArgs(container.Args, extra) + + if deployment.Spec.Template.Annotations == nil { + deployment.Spec.Template.Annotations = map[string]string{} + } + deployment.Spec.Template.Annotations["prometheus.io/scheme"] = "https" + + return nil +} + +func operandMetricsTLSArgs(deploymentName string) ([]string, bool) { + var dnsNames string + switch deploymentName { + case certmanagerControllerDeployment: + dnsNames = "cert-manager,cert-manager.$(POD_NAMESPACE),cert-manager.$(POD_NAMESPACE).svc" + case certmanagerWebhookDeployment: + dnsNames = "cert-manager-webhook,cert-manager-webhook.$(POD_NAMESPACE),cert-manager-webhook.$(POD_NAMESPACE).svc" + case certmanagerCAinjectorDeployment: + dnsNames = "cert-manager-cainjector,cert-manager-cainjector.$(POD_NAMESPACE),cert-manager-cainjector.$(POD_NAMESPACE).svc" + default: + return nil, false + } + + return []string{ + "--metrics-dynamic-serving-ca-secret-namespace=$(POD_NAMESPACE)", + "--metrics-dynamic-serving-ca-secret-name=" + metricsDynamicServingCASecretName, + "--metrics-dynamic-serving-dns-names=" + dnsNames, + }, true +} diff --git a/pkg/controller/certmanager/deployment_metrics_tls_test.go b/pkg/controller/certmanager/deployment_metrics_tls_test.go new file mode 100644 index 000000000..ac8bbacad --- /dev/null +++ b/pkg/controller/certmanager/deployment_metrics_tls_test.go @@ -0,0 +1,90 @@ +package certmanager + +import ( + "strings" + "testing" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestWithOperandMetricsTLS(t *testing.T) { + tests := []struct { + name string + deploymentName string + wantArgs []string + wantScheme string + }{ + { + name: "controller", + deploymentName: certmanagerControllerDeployment, + wantArgs: []string{ + "--metrics-dynamic-serving-ca-secret-namespace=$(POD_NAMESPACE)", + "--metrics-dynamic-serving-ca-secret-name=cert-manager-metrics-ca", + "--metrics-dynamic-serving-dns-names=cert-manager,cert-manager.$(POD_NAMESPACE),cert-manager.$(POD_NAMESPACE).svc", + }, + wantScheme: "https", + }, + { + name: "webhook", + deploymentName: certmanagerWebhookDeployment, + wantArgs: []string{ + "--metrics-dynamic-serving-ca-secret-name=cert-manager-metrics-ca", + "--metrics-dynamic-serving-dns-names=cert-manager-webhook,cert-manager-webhook.$(POD_NAMESPACE),cert-manager-webhook.$(POD_NAMESPACE).svc", + }, + wantScheme: "https", + }, + { + name: "cainjector", + deploymentName: certmanagerCAinjectorDeployment, + wantArgs: []string{ + "--metrics-dynamic-serving-ca-secret-name=cert-manager-metrics-ca", + "--metrics-dynamic-serving-dns-names=cert-manager-cainjector,cert-manager-cainjector.$(POD_NAMESPACE),cert-manager-cainjector.$(POD_NAMESPACE).svc", + }, + wantScheme: "https", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dep := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: tt.deploymentName, Namespace: "cert-manager"}, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "prometheus.io/port": "9402", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: tt.deploymentName, + Args: []string{"--v=2"}, + }}, + }, + }, + }, + } + if err := withOperandMetricsTLS(nil, dep); err != nil { + t.Fatalf("unexpected error: %v", err) + } + argMap := map[string]string{} + for _, a := range dep.Spec.Template.Spec.Containers[0].Args { + parts := strings.SplitN(a, "=", 2) + if len(parts) == 2 { + argMap[parts[0]] = parts[1] + } + } + for _, want := range tt.wantArgs { + parts := strings.SplitN(want, "=", 2) + if argMap[parts[0]] != parts[1] { + t.Fatalf("arg %s: got %q want %q (all=%#v)", parts[0], argMap[parts[0]], parts[1], argMap) + } + } + if dep.Spec.Template.Annotations["prometheus.io/scheme"] != tt.wantScheme { + t.Fatalf("scheme annotation: %q", dep.Spec.Template.Annotations["prometheus.io/scheme"]) + } + }) + } +} diff --git a/pkg/controller/certmanager/generic_deployment_controller.go b/pkg/controller/certmanager/generic_deployment_controller.go index f24fefeff..1ba72b949 100644 --- a/pkg/controller/certmanager/generic_deployment_controller.go +++ b/pkg/controller/certmanager/generic_deployment_controller.go @@ -72,6 +72,9 @@ func newGenericDeploymentController( informers = append(informers, infraInformerFactory.Config().V1().APIServers().Informer()) } + // Enable HTTPS metrics before unsupported overrides so break-glass can still win. + hooks = append(hooks, withOperandMetricsTLS) + // unsupportedConfigOverrides must run after cluster TLS so break-glass operand args win. hooks = append(hooks, withUnsupportedArgsOverrideHook) diff --git a/pkg/operator/assets/bindata.go b/pkg/operator/assets/bindata.go index 56e814627..ad90cf737 100644 --- a/pkg/operator/assets/bindata.go +++ b/pkg/operator/assets/bindata.go @@ -29,6 +29,8 @@ // bindata/cert-manager-deployment/controller/cert-manager-edit-cr.yaml // bindata/cert-manager-deployment/controller/cert-manager-leaderelection-rb.yaml // bindata/cert-manager-deployment/controller/cert-manager-leaderelection-role.yaml +// bindata/cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-rb.yaml +// bindata/cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-role.yaml // bindata/cert-manager-deployment/controller/cert-manager-sa.yaml // bindata/cert-manager-deployment/controller/cert-manager-svc.yaml // bindata/cert-manager-deployment/controller/cert-manager-tokenrequest-rb.yaml @@ -275,6 +277,7 @@ spec: annotations: prometheus.io/path: /metrics prometheus.io/port: "9402" + prometheus.io/scheme: https prometheus.io/scrape: "true" labels: app: cainjector @@ -1446,6 +1449,7 @@ spec: annotations: prometheus.io/path: /metrics prometheus.io/port: "9402" + prometheus.io/scheme: https prometheus.io/scrape: "true" labels: app: cert-manager @@ -1665,6 +1669,94 @@ func certManagerDeploymentControllerCertManagerLeaderelectionRoleYaml() (*asset, return a, nil } +var _certManagerDeploymentControllerCertManagerMetricsDynamicServingRbYaml = []byte(`apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + labels: + app: cert-manager + app.kubernetes.io/component: controller + app.kubernetes.io/instance: cert-manager + app.kubernetes.io/name: cert-manager + app.kubernetes.io/version: v1.20.3 + name: cert-manager-metrics-dynamic-serving + namespace: cert-manager +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: cert-manager-metrics-dynamic-serving +subjects: + - kind: ServiceAccount + name: cert-manager + namespace: cert-manager + - kind: ServiceAccount + name: cert-manager-webhook + namespace: cert-manager + - kind: ServiceAccount + name: cert-manager-cainjector + namespace: cert-manager +`) + +func certManagerDeploymentControllerCertManagerMetricsDynamicServingRbYamlBytes() ([]byte, error) { + return _certManagerDeploymentControllerCertManagerMetricsDynamicServingRbYaml, nil +} + +func certManagerDeploymentControllerCertManagerMetricsDynamicServingRbYaml() (*asset, error) { + bytes, err := certManagerDeploymentControllerCertManagerMetricsDynamicServingRbYamlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-rb.yaml", size: 0, mode: os.FileMode(0), modTime: time.Unix(0, 0)} + a := &asset{bytes: bytes, info: info} + return a, nil +} + +var _certManagerDeploymentControllerCertManagerMetricsDynamicServingRoleYaml = []byte(`apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + labels: + app: cert-manager + app.kubernetes.io/component: controller + app.kubernetes.io/instance: cert-manager + app.kubernetes.io/name: cert-manager + app.kubernetes.io/version: v1.20.3 + name: cert-manager-metrics-dynamic-serving + namespace: cert-manager +rules: + - apiGroups: + - "" + resourceNames: + - cert-manager-metrics-ca + resources: + - secrets + verbs: + - get + - list + - watch + - update + - apiGroups: + - "" + resources: + - secrets + verbs: + - create +`) + +func certManagerDeploymentControllerCertManagerMetricsDynamicServingRoleYamlBytes() ([]byte, error) { + return _certManagerDeploymentControllerCertManagerMetricsDynamicServingRoleYaml, nil +} + +func certManagerDeploymentControllerCertManagerMetricsDynamicServingRoleYaml() (*asset, error) { + bytes, err := certManagerDeploymentControllerCertManagerMetricsDynamicServingRoleYamlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-role.yaml", size: 0, mode: os.FileMode(0), modTime: time.Unix(0, 0)} + a := &asset{bytes: bytes, info: info} + return a, nil +} + var _certManagerDeploymentControllerCertManagerSaYaml = []byte(`apiVersion: v1 automountServiceAccountToken: true kind: ServiceAccount @@ -1880,6 +1972,7 @@ spec: annotations: prometheus.io/path: /metrics prometheus.io/port: "9402" + prometheus.io/scheme: https prometheus.io/scrape: "true" labels: app: webhook @@ -4094,6 +4187,8 @@ var _bindata = map[string]func() (*asset, error){ "cert-manager-deployment/controller/cert-manager-edit-cr.yaml": certManagerDeploymentControllerCertManagerEditCrYaml, "cert-manager-deployment/controller/cert-manager-leaderelection-rb.yaml": certManagerDeploymentControllerCertManagerLeaderelectionRbYaml, "cert-manager-deployment/controller/cert-manager-leaderelection-role.yaml": certManagerDeploymentControllerCertManagerLeaderelectionRoleYaml, + "cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-rb.yaml": certManagerDeploymentControllerCertManagerMetricsDynamicServingRbYaml, + "cert-manager-deployment/controller/cert-manager-metrics-dynamic-serving-role.yaml": certManagerDeploymentControllerCertManagerMetricsDynamicServingRoleYaml, "cert-manager-deployment/controller/cert-manager-sa.yaml": certManagerDeploymentControllerCertManagerSaYaml, "cert-manager-deployment/controller/cert-manager-svc.yaml": certManagerDeploymentControllerCertManagerSvcYaml, "cert-manager-deployment/controller/cert-manager-tokenrequest-rb.yaml": certManagerDeploymentControllerCertManagerTokenrequestRbYaml, @@ -4225,6 +4320,8 @@ var _bintree = &bintree{nil, map[string]*bintree{ "cert-manager-edit-cr.yaml": {certManagerDeploymentControllerCertManagerEditCrYaml, map[string]*bintree{}}, "cert-manager-leaderelection-rb.yaml": {certManagerDeploymentControllerCertManagerLeaderelectionRbYaml, map[string]*bintree{}}, "cert-manager-leaderelection-role.yaml": {certManagerDeploymentControllerCertManagerLeaderelectionRoleYaml, map[string]*bintree{}}, + "cert-manager-metrics-dynamic-serving-rb.yaml": {certManagerDeploymentControllerCertManagerMetricsDynamicServingRbYaml, map[string]*bintree{}}, + "cert-manager-metrics-dynamic-serving-role.yaml": {certManagerDeploymentControllerCertManagerMetricsDynamicServingRoleYaml, map[string]*bintree{}}, "cert-manager-sa.yaml": {certManagerDeploymentControllerCertManagerSaYaml, map[string]*bintree{}}, "cert-manager-svc.yaml": {certManagerDeploymentControllerCertManagerSvcYaml, map[string]*bintree{}}, "cert-manager-tokenrequest-rb.yaml": {certManagerDeploymentControllerCertManagerTokenrequestRbYaml, map[string]*bintree{}}, From 11b8fa30a689df9f521c64dd660668ee88ab6dcf Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Mon, 3 Aug 2026 16:51:49 +0530 Subject: [PATCH 04/10] CM-954: claim TLS profiles feature in CSV annotations Set features.operators.openshift.io/tls-profiles to true now that operand, operator metrics, and trust-manager webhook TLS are wired. --- .../bases/cert-manager-operator.clusterserviceversion.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/manifests/bases/cert-manager-operator.clusterserviceversion.yaml b/config/manifests/bases/cert-manager-operator.clusterserviceversion.yaml index 9f3841f14..efe298419 100644 --- a/config/manifests/bases/cert-manager-operator.clusterserviceversion.yaml +++ b/config/manifests/bases/cert-manager-operator.clusterserviceversion.yaml @@ -14,7 +14,7 @@ metadata: features.operators.openshift.io/disconnected: "true" features.operators.openshift.io/fips-compliant: "true" features.operators.openshift.io/proxy-aware: "true" - features.operators.openshift.io/tls-profiles: "false" + features.operators.openshift.io/tls-profiles: "true" features.operators.openshift.io/token-auth-aws: "true" features.operators.openshift.io/token-auth-azure: "true" features.operators.openshift.io/token-auth-gcp: "true" From 11d5f6983ec816b849c4afe66bffff3bb3349b9c Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Mon, 3 Aug 2026 16:52:21 +0530 Subject: [PATCH 05/10] CM-954: sync bundle CSV tls-profiles annotation Keep the bundled ClusterServiceVersion aligned with the manifests base tls-profiles feature claim. --- .../manifests/cert-manager-operator.clusterserviceversion.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/manifests/cert-manager-operator.clusterserviceversion.yaml b/bundle/manifests/cert-manager-operator.clusterserviceversion.yaml index 8bc124a24..54b469184 100644 --- a/bundle/manifests/cert-manager-operator.clusterserviceversion.yaml +++ b/bundle/manifests/cert-manager-operator.clusterserviceversion.yaml @@ -284,7 +284,7 @@ metadata: features.operators.openshift.io/disconnected: "true" features.operators.openshift.io/fips-compliant: "true" features.operators.openshift.io/proxy-aware: "true" - features.operators.openshift.io/tls-profiles: "false" + features.operators.openshift.io/tls-profiles: "true" features.operators.openshift.io/token-auth-aws: "true" features.operators.openshift.io/token-auth-azure: "true" features.operators.openshift.io/token-auth-gcp: "true" From b795b39ab47b944af21cb4059d52f54be87490e2 Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Tue, 4 Aug 2026 15:29:30 +0530 Subject: [PATCH 06/10] CM-954: mark k8s.io/apiserver as a direct dependency Operator cmd imports k8s.io/apiserver/pkg/server for ServingInfo TLS wiring; tidy expects it as a direct require so verify-deps stays clean. --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index b3dc04280..1744b345f 100644 --- a/go.mod +++ b/go.mod @@ -16,6 +16,7 @@ require ( k8s.io/api v0.35.2 k8s.io/apiextensions-apiserver v0.35.2 k8s.io/apimachinery v0.35.2 + k8s.io/apiserver v0.35.2 k8s.io/client-go v0.35.2 k8s.io/component-base v0.35.2 k8s.io/klog/v2 v2.140.0 @@ -123,7 +124,6 @@ require ( gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - k8s.io/apiserver v0.35.2 // indirect k8s.io/component-helpers v0.35.2 // indirect k8s.io/controller-manager v0.35.2 // indirect k8s.io/kms v0.35.2 // indirect From e7a47ce1a0c30b8351a6c24439fc5d125b14a2cf Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Tue, 4 Aug 2026 15:44:40 +0530 Subject: [PATCH 07/10] Addressing coderabbit's PR review comments --- pkg/cmd/operator/cmd.go | 32 ++++-- pkg/controller/trustmanager/controller.go | 3 +- pkg/controller/trustmanager/deployment_tls.go | 34 ++----- .../trustmanager/deployment_tls_test.go | 8 +- pkg/tlsprofile/cluster.go | 97 +++++++++++++++++++ pkg/tlsprofile/cluster_test.go | 41 ++++++++ pkg/tlsprofile/serving.go | 34 ++----- 7 files changed, 185 insertions(+), 64 deletions(-) create mode 100644 pkg/tlsprofile/cluster.go create mode 100644 pkg/tlsprofile/cluster_test.go diff --git a/pkg/cmd/operator/cmd.go b/pkg/cmd/operator/cmd.go index ba5723902..422668486 100644 --- a/pkg/cmd/operator/cmd.go +++ b/pkg/cmd/operator/cmd.go @@ -54,7 +54,10 @@ func NewOperator() *cobra.Command { ctx, terminate := context.WithCancel(shutdownCtx) defer terminate() - terminateOnFiles, _ := cmd.Flags().GetStringArray("terminate-on-files") + terminateOnFiles, err := cmd.Flags().GetStringArray("terminate-on-files") + if err != nil { + klog.Fatal(err) + } if len(terminateOnFiles) > 0 { obs, err := fileobserver.NewObserver(10 * time.Second) if err != nil { @@ -109,19 +112,34 @@ func startControllerWithClusterTLS(ctx context.Context, c *controllercmd.Control return err } - if listen, _ := cmd.Flags().GetString("listen"); len(listen) != 0 { + listen, err := cmd.Flags().GetString("listen") + if err != nil { + return err + } + if len(listen) != 0 { config.ServingInfo.BindAddress = listen } - kubeConfigFile, _ := cmd.Flags().GetString("kubeconfig") - namespace, _ := cmd.Flags().GetString("namespace") + kubeConfigFile, err := cmd.Flags().GetString("kubeconfig") + if err != nil { + return err + } + namespace, err := cmd.Flags().GetString("namespace") + if err != nil { + return err + } if !c.DisableServing { restConfig, err := tlsprofile.RESTConfigFromKubeConfig(kubeConfigFile) if err != nil { - klog.V(2).Infof("unable to build rest config for cluster TLS profile lookup: %v", err) - } else if err := tlsprofile.ApplyClusterProfileToHTTPServingInfo(ctx, restConfig, &config.ServingInfo); err != nil { - return err + klog.Warningf("unable to build rest config for cluster TLS profile lookup; using Controllercmd default TLS settings: %v", err) + } else { + lookupCtx, cancelLookup := context.WithTimeout(ctx, 30*time.Second) + err := tlsprofile.ApplyClusterProfileToHTTPServingInfo(lookupCtx, restConfig, &config.ServingInfo) + cancelLookup() + if err != nil { + return err + } } } diff --git a/pkg/controller/trustmanager/controller.go b/pkg/controller/trustmanager/controller.go index a998f8054..78541680d 100644 --- a/pkg/controller/trustmanager/controller.go +++ b/pkg/controller/trustmanager/controller.go @@ -29,6 +29,7 @@ import ( v1alpha1 "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" "github.com/openshift/cert-manager-operator/pkg/controller/common" + "github.com/openshift/cert-manager-operator/pkg/tlsprofile" ) // RequestEnqueueLabelValue is the label value used for filtering reconcile @@ -135,7 +136,7 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { // Reconcile when the cluster APIServer TLS profile or adherence changes. clusterAPIServerPredicate := predicate.NewPredicateFuncs(func(object client.Object) bool { - return object.GetName() == apiServerClusterName + return object.GetName() == tlsprofile.APIServerClusterName }) return ctrl.NewControllerManagedBy(mgr). diff --git a/pkg/controller/trustmanager/deployment_tls.go b/pkg/controller/trustmanager/deployment_tls.go index 8a797f0de..59657e675 100644 --- a/pkg/controller/trustmanager/deployment_tls.go +++ b/pkg/controller/trustmanager/deployment_tls.go @@ -4,19 +4,13 @@ import ( "fmt" appsv1 "k8s.io/api/apps/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/types" - "k8s.io/klog/v2" configv1 "github.com/openshift/api/config/v1" - libgocrypto "github.com/openshift/library-go/pkg/crypto" "github.com/openshift/cert-manager-operator/pkg/controller/common" "github.com/openshift/cert-manager-operator/pkg/tlsprofile" ) -const apiServerClusterName = "cluster" - // applyClusterTLSProfile merges cluster TLS security profile flags onto the // trust-manager webhook container when apiserver tlsAdherence requires it. // When the APIServer resource is missing (non-OpenShift) or adherence does not @@ -26,28 +20,18 @@ func (r *Reconciler) applyClusterTLSProfile(deployment *appsv1.Deployment) error return nil } - apiServer := &configv1.APIServer{} - if err := r.Get(r.ctx, types.NamespacedName{Name: apiServerClusterName}, apiServer); err != nil { - if apierrors.IsNotFound(err) { - klog.V(4).Info("skipping cluster TLS profile for trust-manager: apiserver.config.openshift.io/cluster not found") - return nil - } - return fmt.Errorf("failed to get apiserver.config.openshift.io/cluster: %w", err) - } - - adherence := apiServer.Spec.TLSAdherence - if !libgocrypto.ShouldHonorClusterTLSProfile(adherence) { - klog.V(4).Infof("skipping cluster TLS profile for trust-manager: apiserver tlsAdherence=%q", adherence) - return nil - } - if adherence != configv1.TLSAdherencePolicyStrictAllComponents { - klog.Warningf("apiserver.config.openshift.io/cluster has unknown tlsAdherence %q; treating as StrictAllComponents for trust-manager", adherence) - } - - effective, err := tlsprofile.EffectiveSpec(apiServer.Spec.TLSSecurityProfile) + effective, err := tlsprofile.ResolveHonoredTLSProfile( + r.ctx, + tlsprofile.NewClientReaderAPIServerFetch(r.CtrlClient), + "trust-manager", + tlsprofile.FetchErrorPropagateExceptNotFound, + ) if err != nil { return err } + if effective == nil { + return nil + } return applyTrustManagerWebhookTLSArgs(deployment, effective) } diff --git a/pkg/controller/trustmanager/deployment_tls_test.go b/pkg/controller/trustmanager/deployment_tls_test.go index a998f4695..cc46cb479 100644 --- a/pkg/controller/trustmanager/deployment_tls_test.go +++ b/pkg/controller/trustmanager/deployment_tls_test.go @@ -106,7 +106,7 @@ func TestApplyClusterTLSProfile_adherence(t *testing.T) { { name: "strict modern injects tls13 min version without ciphers", apiServer: &configv1.APIServer{ - ObjectMeta: metav1.ObjectMeta{Name: apiServerClusterName}, + ObjectMeta: metav1.ObjectMeta{Name: tlsprofile.APIServerClusterName}, Spec: configv1.APIServerSpec{ TLSAdherence: configv1.TLSAdherencePolicyStrictAllComponents, TLSSecurityProfile: &configv1.TLSSecurityProfile{ @@ -121,7 +121,7 @@ func TestApplyClusterTLSProfile_adherence(t *testing.T) { { name: "legacy adherence skips injection", apiServer: &configv1.APIServer{ - ObjectMeta: metav1.ObjectMeta{Name: apiServerClusterName}, + ObjectMeta: metav1.ObjectMeta{Name: tlsprofile.APIServerClusterName}, Spec: configv1.APIServerSpec{ TLSAdherence: configv1.TLSAdherencePolicyLegacyAdheringComponentsOnly, TLSSecurityProfile: &configv1.TLSSecurityProfile{ @@ -169,7 +169,7 @@ func TestApplyClusterTLSProfile_adherence(t *testing.T) { func TestApplyClusterTLSProfile_intermediateCiphers(t *testing.T) { t.Setenv(trustManagerImageNameEnvVarName, testImage) apiServer := &configv1.APIServer{ - ObjectMeta: metav1.ObjectMeta{Name: apiServerClusterName}, + ObjectMeta: metav1.ObjectMeta{Name: tlsprofile.APIServerClusterName}, Spec: configv1.APIServerSpec{ TLSAdherence: configv1.TLSAdherencePolicyStrictAllComponents, TLSSecurityProfile: &configv1.TLSSecurityProfile{ @@ -209,7 +209,7 @@ func TestApplyClusterTLSProfile_intermediateCiphers(t *testing.T) { func fakeCtrlClientWithAPIServer(apiServer *configv1.APIServer) *fakes.FakeCtrlClient { mock := &fakes.FakeCtrlClient{} mock.GetCalls(func(_ context.Context, key client.ObjectKey, obj client.Object) error { - if key.Name != apiServerClusterName { + if key.Name != tlsprofile.APIServerClusterName { return apierrors.NewNotFound(schema.GroupResource{Group: configv1.GroupName, Resource: "apiservers"}, key.Name) } if apiServer == nil { diff --git a/pkg/tlsprofile/cluster.go b/pkg/tlsprofile/cluster.go new file mode 100644 index 000000000..4f1053727 --- /dev/null +++ b/pkg/tlsprofile/cluster.go @@ -0,0 +1,97 @@ +package tlsprofile + +import ( + "context" + "fmt" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/rest" + "k8s.io/klog/v2" + "sigs.k8s.io/controller-runtime/pkg/client" + + configv1 "github.com/openshift/api/config/v1" + configv1client "github.com/openshift/client-go/config/clientset/versioned" + libgocrypto "github.com/openshift/library-go/pkg/crypto" +) + +// APIServerClusterName is the singleton apiserver.config.openshift.io object. +const APIServerClusterName = "cluster" + +// FetchAPIServerFunc retrieves apiserver.config.openshift.io/cluster. +type FetchAPIServerFunc func(ctx context.Context) (*configv1.APIServer, error) + +// FetchErrorMode controls how ResolveHonoredTLSProfile treats fetch failures. +type FetchErrorMode int + +const ( + // FetchErrorPropagateExceptNotFound returns NotFound as a soft skip and + // propagates all other fetch errors. + FetchErrorPropagateExceptNotFound FetchErrorMode = iota +) + +// ObjectGetter is the subset of controller-runtime client used to fetch APIServer. +// It matches both client.Reader and this repo's CtrlClient Get signature. +type ObjectGetter interface { + Get(ctx context.Context, key client.ObjectKey, obj client.Object) error +} + +// NewRESTConfigAPIServerFetch returns a client-go fetcher for the cluster APIServer. +func NewRESTConfigAPIServerFetch(restConfig *rest.Config) (FetchAPIServerFunc, error) { + if restConfig == nil { + return nil, fmt.Errorf("rest config is nil") + } + configClient, err := configv1client.NewForConfig(restConfig) + if err != nil { + return nil, fmt.Errorf("failed to create config client: %w", err) + } + return func(ctx context.Context) (*configv1.APIServer, error) { + return configClient.ConfigV1().APIServers().Get(ctx, APIServerClusterName, metav1.GetOptions{}) + }, nil +} + +// NewClientReaderAPIServerFetch returns a controller-runtime fetcher for the cluster APIServer. +func NewClientReaderAPIServerFetch(r ObjectGetter) FetchAPIServerFunc { + return func(ctx context.Context) (*configv1.APIServer, error) { + apiServer := &configv1.APIServer{} + if err := r.Get(ctx, types.NamespacedName{Name: APIServerClusterName}, apiServer); err != nil { + return nil, err + } + return apiServer, nil + } +} + +// ResolveHonoredTLSProfile fetches the cluster APIServer via fetch and, when +// tlsAdherence requires enforcement, returns EffectiveSpec. A nil profile with +// a nil error means the caller should leave existing TLS settings unchanged. +func ResolveHonoredTLSProfile(ctx context.Context, fetch FetchAPIServerFunc, component string, mode FetchErrorMode) (*configv1.TLSProfileSpec, error) { + if fetch == nil { + return nil, fmt.Errorf("APIServer fetch function is nil") + } + + apiServer, err := fetch(ctx) + if err != nil { + switch mode { + case FetchErrorPropagateExceptNotFound: + if apierrors.IsNotFound(err) { + klog.V(4).Infof("skipping cluster TLS profile for %s: apiserver.config.openshift.io/cluster not found", component) + return nil, nil + } + return nil, fmt.Errorf("failed to get apiserver.config.openshift.io/cluster: %w", err) + default: + return nil, fmt.Errorf("failed to get apiserver.config.openshift.io/cluster: %w", err) + } + } + + adherence := apiServer.Spec.TLSAdherence + if !libgocrypto.ShouldHonorClusterTLSProfile(adherence) { + klog.V(4).Infof("skipping cluster TLS profile for %s: apiserver tlsAdherence=%q", component, adherence) + return nil, nil + } + if adherence != configv1.TLSAdherencePolicyStrictAllComponents { + klog.Warningf("apiserver.config.openshift.io/cluster has unknown tlsAdherence %q; treating as StrictAllComponents for %s", adherence, component) + } + + return EffectiveSpec(apiServer.Spec.TLSSecurityProfile) +} diff --git a/pkg/tlsprofile/cluster_test.go b/pkg/tlsprofile/cluster_test.go new file mode 100644 index 000000000..6ce0abcdd --- /dev/null +++ b/pkg/tlsprofile/cluster_test.go @@ -0,0 +1,41 @@ +package tlsprofile + +import ( + "context" + "errors" + "testing" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" + + configv1 "github.com/openshift/api/config/v1" +) + +func TestResolveHonoredTLSProfile_fetchErrors(t *testing.T) { + notFound := apierrors.NewNotFound(schema.GroupResource{Group: configv1.GroupName, Resource: "apiservers"}, APIServerClusterName) + forbidden := apierrors.NewForbidden(schema.GroupResource{Group: configv1.GroupName, Resource: "apiservers"}, APIServerClusterName, errors.New("denied")) + + t.Run("NotFound is soft skip", func(t *testing.T) { + spec, err := ResolveHonoredTLSProfile(context.Background(), func(context.Context) (*configv1.APIServer, error) { + return nil, notFound + }, "test", FetchErrorPropagateExceptNotFound) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if spec != nil { + t.Fatalf("expected nil spec on NotFound, got %#v", spec) + } + }) + + t.Run("Forbidden propagates", func(t *testing.T) { + spec, err := ResolveHonoredTLSProfile(context.Background(), func(context.Context) (*configv1.APIServer, error) { + return nil, forbidden + }, "test", FetchErrorPropagateExceptNotFound) + if err == nil { + t.Fatal("expected error") + } + if spec != nil { + t.Fatalf("expected nil spec on Forbidden, got %#v", spec) + } + }) +} diff --git a/pkg/tlsprofile/serving.go b/pkg/tlsprofile/serving.go index 1cfe6f3cd..c034d0699 100644 --- a/pkg/tlsprofile/serving.go +++ b/pkg/tlsprofile/serving.go @@ -4,55 +4,35 @@ import ( "context" "fmt" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" "k8s.io/klog/v2" configv1 "github.com/openshift/api/config/v1" - configv1client "github.com/openshift/client-go/config/clientset/versioned" - libgocrypto "github.com/openshift/library-go/pkg/crypto" ) -const apiServerClusterName = "cluster" - // ApplyClusterProfileToHTTPServingInfo reads apiserver.config.openshift.io/cluster // and, when tlsAdherence requires enforcement, applies the effective TLS profile -// to serving. Missing APIServer (non-OpenShift) or non-enforcing adherence leaves -// serving unchanged. +// to serving. A missing APIServer (NotFound) or non-enforcing adherence leaves +// serving unchanged; other APIServer lookup failures are returned. func ApplyClusterProfileToHTTPServingInfo(ctx context.Context, restConfig *rest.Config, serving *configv1.HTTPServingInfo) error { - if restConfig == nil { - return fmt.Errorf("rest config is nil") - } if serving == nil { return fmt.Errorf("HTTPServingInfo is nil") } - configClient, err := configv1client.NewForConfig(restConfig) + fetch, err := NewRESTConfigAPIServerFetch(restConfig) if err != nil { - return fmt.Errorf("failed to create config client: %w", err) + return err } - apiServer, err := configClient.ConfigV1().APIServers().Get(ctx, apiServerClusterName, metav1.GetOptions{}) + effective, err := ResolveHonoredTLSProfile(ctx, fetch, "operator serving", FetchErrorPropagateExceptNotFound) if err != nil { - // Non-OpenShift or RBAC/API unavailable: keep library-go defaults. - klog.V(2).Infof("skipping cluster TLS profile for operator serving: failed to get apiserver/cluster: %v", err) - return nil + return err } - - adherence := apiServer.Spec.TLSAdherence - if !libgocrypto.ShouldHonorClusterTLSProfile(adherence) { - klog.V(2).Infof("skipping cluster TLS profile for operator serving: apiserver tlsAdherence=%q", adherence) + if effective == nil { return nil } - if adherence != configv1.TLSAdherencePolicyStrictAllComponents { - klog.Warningf("apiserver.config.openshift.io/cluster has unknown tlsAdherence %q; treating as StrictAllComponents for operator serving", adherence) - } - effective, err := EffectiveSpec(apiServer.Spec.TLSSecurityProfile) - if err != nil { - return err - } if err := ApplyToHTTPServingInfo(serving, effective); err != nil { return err } From 01c94a92ef4b4c4a01377a769385a6e77d627656 Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Tue, 4 Aug 2026 17:51:50 +0530 Subject: [PATCH 08/10] Add missing TLS unit coverage and fix ObjectGetter docs Expand ResolveHonoredTLSProfile and trust-manager TLS tests for adherence, error propagation, and serving guards; clarify ObjectGetter matches CtrlClient. --- .../trustmanager/deployment_tls_test.go | 96 +++++++++++++++ pkg/tlsprofile/cluster.go | 5 +- pkg/tlsprofile/cluster_test.go | 115 ++++++++++++++++++ pkg/tlsprofile/serving_test.go | 22 ++++ 4 files changed, 236 insertions(+), 2 deletions(-) diff --git a/pkg/controller/trustmanager/deployment_tls_test.go b/pkg/controller/trustmanager/deployment_tls_test.go index cc46cb479..eb14c77a8 100644 --- a/pkg/controller/trustmanager/deployment_tls_test.go +++ b/pkg/controller/trustmanager/deployment_tls_test.go @@ -2,6 +2,7 @@ package trustmanager import ( "context" + "errors" "strings" "testing" @@ -118,6 +119,33 @@ func TestApplyClusterTLSProfile_adherence(t *testing.T) { wantMinVer: "VersionTLS13", wantCipherKey: false, }, + { + name: "unknown adherence treated as strict", + apiServer: &configv1.APIServer{ + ObjectMeta: metav1.ObjectMeta{Name: tlsprofile.APIServerClusterName}, + Spec: configv1.APIServerSpec{ + TLSAdherence: configv1.TLSAdherencePolicy("FutureStrictMode"), + TLSSecurityProfile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileModernType, + }, + }, + }, + wantTLSArgs: true, + wantMinVer: "VersionTLS13", + wantCipherKey: false, + }, + { + name: "empty adherence skips injection", + apiServer: &configv1.APIServer{ + ObjectMeta: metav1.ObjectMeta{Name: tlsprofile.APIServerClusterName}, + Spec: configv1.APIServerSpec{ + TLSSecurityProfile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileModernType, + }, + }, + }, + wantTLSArgs: false, + }, { name: "legacy adherence skips injection", apiServer: &configv1.APIServer{ @@ -166,6 +194,74 @@ func TestApplyClusterTLSProfile_adherence(t *testing.T) { } } +func TestApplyClusterTLSProfile_forbiddenPropagates(t *testing.T) { + t.Setenv(trustManagerImageNameEnvVarName, testImage) + r := testReconciler(t) + mock := &fakes.FakeCtrlClient{} + mock.GetCalls(func(_ context.Context, key client.ObjectKey, _ client.Object) error { + return apierrors.NewForbidden(schema.GroupResource{Group: configv1.GroupName, Resource: "apiservers"}, key.Name, errors.New("denied")) + }) + r.CtrlClient = mock + + dep := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: trustManagerDeploymentName, Namespace: operandNamespace}, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Name: trustManagerContainerName}}, + }, + }, + }, + } + if err := r.applyClusterTLSProfile(dep); err == nil { + t.Fatal("expected Forbidden to propagate") + } +} + +func TestApplyClusterTLSProfile_nilClientIsNoop(t *testing.T) { + r := testReconciler(t) + r.CtrlClient = nil + dep := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: trustManagerDeploymentName, Namespace: operandNamespace}, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: trustManagerContainerName, + Args: []string{"--webhook-port=6443"}, + }}, + }, + }, + }, + } + if err := r.applyClusterTLSProfile(dep); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(dep.Spec.Template.Spec.Containers[0].Args) != 1 { + t.Fatalf("expected args unchanged, got %#v", dep.Spec.Template.Spec.Containers[0].Args) + } +} + +func TestApplyTrustManagerWebhookTLSArgs_missingContainer(t *testing.T) { + dep := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: trustManagerDeploymentName, Namespace: operandNamespace}, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Name: "not-trust-manager"}}, + }, + }, + }, + } + err := applyTrustManagerWebhookTLSArgs(dep, &configv1.TLSProfileSpec{ + MinTLSVersion: configv1.VersionTLS12, + Ciphers: []string{"ECDHE-RSA-AES128-GCM-SHA256"}, + }) + if err == nil { + t.Fatal("expected error for missing container") + } +} + func TestApplyClusterTLSProfile_intermediateCiphers(t *testing.T) { t.Setenv(trustManagerImageNameEnvVarName, testImage) apiServer := &configv1.APIServer{ diff --git a/pkg/tlsprofile/cluster.go b/pkg/tlsprofile/cluster.go index 4f1053727..ccbebebd8 100644 --- a/pkg/tlsprofile/cluster.go +++ b/pkg/tlsprofile/cluster.go @@ -31,8 +31,9 @@ const ( FetchErrorPropagateExceptNotFound FetchErrorMode = iota ) -// ObjectGetter is the subset of controller-runtime client used to fetch APIServer. -// It matches both client.Reader and this repo's CtrlClient Get signature. +// ObjectGetter is the Get subset used to fetch APIServer. It matches +// common.CtrlClient's Get signature (no GetOption variadic), which is what +// production callers pass via NewClientReaderAPIServerFetch. type ObjectGetter interface { Get(ctx context.Context, key client.ObjectKey, obj client.Object) error } diff --git a/pkg/tlsprofile/cluster_test.go b/pkg/tlsprofile/cluster_test.go index 6ce0abcdd..7a8707105 100644 --- a/pkg/tlsprofile/cluster_test.go +++ b/pkg/tlsprofile/cluster_test.go @@ -3,6 +3,8 @@ package tlsprofile import ( "context" "errors" + "reflect" + "strings" "testing" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -38,4 +40,117 @@ func TestResolveHonoredTLSProfile_fetchErrors(t *testing.T) { t.Fatalf("expected nil spec on Forbidden, got %#v", spec) } }) + + t.Run("nil fetch function errors", func(t *testing.T) { + _, err := ResolveHonoredTLSProfile(context.Background(), nil, "test", FetchErrorPropagateExceptNotFound) + if err == nil { + t.Fatal("expected error for nil fetch") + } + }) +} + +func TestResolveHonoredTLSProfile_adherence(t *testing.T) { + wantModern, err := EffectiveSpec(&configv1.TLSSecurityProfile{Type: configv1.TLSProfileModernType}) + if err != nil { + t.Fatal(err) + } + wantIntermediate, err := EffectiveSpec(nil) + if err != nil { + t.Fatal(err) + } + + cases := []struct { + name string + apiServer *configv1.APIServer + wantSpec *configv1.TLSProfileSpec + wantErr string + }{ + { + name: "empty adherence skips", + apiServer: &configv1.APIServer{ + Spec: configv1.APIServerSpec{ + TLSSecurityProfile: &configv1.TLSSecurityProfile{Type: configv1.TLSProfileModernType}, + }, + }, + }, + { + name: "legacy adherence skips", + apiServer: &configv1.APIServer{ + Spec: configv1.APIServerSpec{ + TLSAdherence: configv1.TLSAdherencePolicyLegacyAdheringComponentsOnly, + TLSSecurityProfile: &configv1.TLSSecurityProfile{Type: configv1.TLSProfileModernType}, + }, + }, + }, + { + name: "strict modern returns effective spec", + apiServer: &configv1.APIServer{ + Spec: configv1.APIServerSpec{ + TLSAdherence: configv1.TLSAdherencePolicyStrictAllComponents, + TLSSecurityProfile: &configv1.TLSSecurityProfile{Type: configv1.TLSProfileModernType}, + }, + }, + wantSpec: wantModern, + }, + { + name: "unknown adherence treated as strict with nil profile falls back to Intermediate", + apiServer: &configv1.APIServer{ + Spec: configv1.APIServerSpec{ + TLSAdherence: configv1.TLSAdherencePolicy("FutureStrictMode"), + }, + }, + wantSpec: wantIntermediate, + }, + { + name: "strict with invalid custom profile propagates error", + apiServer: &configv1.APIServer{ + Spec: configv1.APIServerSpec{ + TLSAdherence: configv1.TLSAdherencePolicyStrictAllComponents, + TLSSecurityProfile: &configv1.TLSSecurityProfile{ + Type: configv1.TLSProfileCustomType, + }, + }, + }, + wantErr: "custom TLS profile is missing custom settings", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := ResolveHonoredTLSProfile(context.Background(), func(context.Context) (*configv1.APIServer, error) { + return tc.apiServer, nil + }, "test", FetchErrorPropagateExceptNotFound) + if tc.wantErr != "" { + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("error = %v, want containing %q", err, tc.wantErr) + } + if got != nil { + t.Fatalf("expected nil spec on error, got %#v", got) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if tc.wantSpec == nil { + if got != nil { + t.Fatalf("expected nil spec, got %#v", got) + } + return + } + if got.MinTLSVersion != tc.wantSpec.MinTLSVersion { + t.Fatalf("MinTLSVersion = %q, want %q", got.MinTLSVersion, tc.wantSpec.MinTLSVersion) + } + if !reflect.DeepEqual(got.Ciphers, tc.wantSpec.Ciphers) { + t.Fatalf("Ciphers = %#v, want %#v", got.Ciphers, tc.wantSpec.Ciphers) + } + }) + } +} + +func TestNewRESTConfigAPIServerFetch_nilConfig(t *testing.T) { + _, err := NewRESTConfigAPIServerFetch(nil) + if err == nil { + t.Fatal("expected error for nil rest config") + } } diff --git a/pkg/tlsprofile/serving_test.go b/pkg/tlsprofile/serving_test.go index 1a7f19bbf..22dcb7595 100644 --- a/pkg/tlsprofile/serving_test.go +++ b/pkg/tlsprofile/serving_test.go @@ -1,11 +1,33 @@ package tlsprofile import ( + "context" "testing" + "k8s.io/client-go/rest" + configv1 "github.com/openshift/api/config/v1" ) +func TestApplyClusterProfileToHTTPServingInfo_guards(t *testing.T) { + t.Run("nil serving", func(t *testing.T) { + err := ApplyClusterProfileToHTTPServingInfo(context.Background(), &rest.Config{}, nil) + if err == nil { + t.Fatal("expected error") + } + }) + t.Run("nil rest config", func(t *testing.T) { + serving := &configv1.HTTPServingInfo{} + err := ApplyClusterProfileToHTTPServingInfo(context.Background(), nil, serving) + if err == nil { + t.Fatal("expected error") + } + if serving.MinTLSVersion != "" { + t.Fatalf("serving should remain unchanged on error, got min=%q", serving.MinTLSVersion) + } + }) +} + func TestApplyToHTTPServingInfo(t *testing.T) { t.Run("intermediate", func(t *testing.T) { spec, err := EffectiveSpec(&configv1.TLSSecurityProfile{Type: configv1.TLSProfileIntermediateType}) From 7a63e04adbc12fb8ee1e34434c1a04ea2ab2ca1d Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Wed, 12 Aug 2026 14:52:27 +0530 Subject: [PATCH 09/10] CM-1225: expand TLS profile e2e and serving unit coverage Add operand Day-2/cipher-strip and Legacy HTTPS-metrics e2e, harden APIServer profile patches for Modern/Intermediate union fields, and extend serving/adherence unit tests in place of a live :8443 dial. --- pkg/tlsprofile/cluster_test.go | 10 + pkg/tlsprofile/serving_test.go | 122 +++++++--- test/e2e/tls_profile_test.go | 401 ++++++++++++++++++++++++++++++++- test/e2e/utils_test.go | 115 +++++++++- 4 files changed, 602 insertions(+), 46 deletions(-) diff --git a/pkg/tlsprofile/cluster_test.go b/pkg/tlsprofile/cluster_test.go index 7a8707105..a8162ade9 100644 --- a/pkg/tlsprofile/cluster_test.go +++ b/pkg/tlsprofile/cluster_test.go @@ -82,6 +82,16 @@ func TestResolveHonoredTLSProfile_adherence(t *testing.T) { }, }, }, + { + // TLSAdherencePolicyNoOpinion is ""; explicit const documents operator-start soft-skip. + name: "noOpinion adherence skips", + apiServer: &configv1.APIServer{ + Spec: configv1.APIServerSpec{ + TLSAdherence: configv1.TLSAdherencePolicyNoOpinion, + TLSSecurityProfile: &configv1.TLSSecurityProfile{Type: configv1.TLSProfileModernType}, + }, + }, + }, { name: "strict modern returns effective spec", apiServer: &configv1.APIServer{ diff --git a/pkg/tlsprofile/serving_test.go b/pkg/tlsprofile/serving_test.go index 22dcb7595..9495783cc 100644 --- a/pkg/tlsprofile/serving_test.go +++ b/pkg/tlsprofile/serving_test.go @@ -2,6 +2,7 @@ package tlsprofile import ( "context" + "strings" "testing" "k8s.io/client-go/rest" @@ -29,43 +30,90 @@ func TestApplyClusterProfileToHTTPServingInfo_guards(t *testing.T) { } func TestApplyToHTTPServingInfo(t *testing.T) { - t.Run("intermediate", func(t *testing.T) { - spec, err := EffectiveSpec(&configv1.TLSSecurityProfile{Type: configv1.TLSProfileIntermediateType}) - if err != nil { - t.Fatal(err) - } - serving := &configv1.HTTPServingInfo{} - if err := ApplyToHTTPServingInfo(serving, spec); err != nil { - t.Fatal(err) - } - if serving.MinTLSVersion != string(configv1.VersionTLS12) { - t.Fatalf("min version: %q", serving.MinTLSVersion) - } - if len(serving.CipherSuites) == 0 { - t.Fatal("expected ciphers") - } - }) + intermediateSpec, err := EffectiveSpec(&configv1.TLSSecurityProfile{Type: configv1.TLSProfileIntermediateType}) + if err != nil { + t.Fatal(err) + } + modernSpec, err := EffectiveSpec(&configv1.TLSSecurityProfile{Type: configv1.TLSProfileModernType}) + if err != nil { + t.Fatal(err) + } - t.Run("modern tls13 keeps non-empty ciphers to block defaults", func(t *testing.T) { - spec, err := EffectiveSpec(&configv1.TLSSecurityProfile{Type: configv1.TLSProfileModernType}) - if err != nil { - t.Fatal(err) - } - serving := &configv1.HTTPServingInfo{} - if err := ApplyToHTTPServingInfo(serving, spec); err != nil { - t.Fatal(err) - } - if serving.MinTLSVersion != string(configv1.VersionTLS13) { - t.Fatalf("min version: %q", serving.MinTLSVersion) - } - if len(serving.CipherSuites) == 0 { - t.Fatal("expected non-empty cipher list so library-go defaults are not reapplied") - } - }) + servingWithMin := func(min string) *configv1.HTTPServingInfo { + s := &configv1.HTTPServingInfo{} + s.MinTLSVersion = min + return s + } - t.Run("nil serving", func(t *testing.T) { - if err := ApplyToHTTPServingInfo(nil, &configv1.TLSProfileSpec{MinTLSVersion: configv1.VersionTLS12}); err == nil { - t.Fatal("expected error") - } - }) + cases := []struct { + name string + serving *configv1.HTTPServingInfo + spec *configv1.TLSProfileSpec + wantErr string + wantMin string + wantCipher bool + preserveMinOnErr string + }{ + { + name: "intermediate", + serving: &configv1.HTTPServingInfo{}, + spec: intermediateSpec, + wantMin: string(configv1.VersionTLS12), + wantCipher: true, + }, + { + name: "modern tls13 keeps non-empty ciphers to block defaults", + serving: &configv1.HTTPServingInfo{}, + spec: modernSpec, + wantMin: string(configv1.VersionTLS13), + wantCipher: true, + }, + { + name: "nil serving", + serving: nil, + spec: &configv1.TLSProfileSpec{MinTLSVersion: configv1.VersionTLS12}, + wantErr: "HTTPServingInfo is nil", + }, + { + name: "nil spec", + serving: servingWithMin("unchanged"), + spec: nil, + wantErr: "TLS profile spec is nil", + preserveMinOnErr: "unchanged", + }, + { + name: "unmappable ciphers", + serving: servingWithMin("unchanged"), + spec: &configv1.TLSProfileSpec{ + MinTLSVersion: configv1.VersionTLS12, + Ciphers: []string{"NOT-A-REAL-OPENSSL-CIPHER"}, + }, + wantErr: "no cipher suites after OpenSSL", + // MinTLSVersion is assigned before cipher mapping; only nil-spec leaves serving untouched. + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := ApplyToHTTPServingInfo(tc.serving, tc.spec) + if tc.wantErr != "" { + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("error = %v, want containing %q", err, tc.wantErr) + } + if tc.preserveMinOnErr != "" && tc.serving != nil && tc.serving.MinTLSVersion != tc.preserveMinOnErr { + t.Fatalf("MinTLSVersion = %q, want preserved %q", tc.serving.MinTLSVersion, tc.preserveMinOnErr) + } + return + } + if err != nil { + t.Fatal(err) + } + if tc.serving.MinTLSVersion != tc.wantMin { + t.Fatalf("min version: %q, want %q", tc.serving.MinTLSVersion, tc.wantMin) + } + if tc.wantCipher && len(tc.serving.CipherSuites) == 0 { + t.Fatal("expected non-empty cipher list") + } + }) + } } diff --git a/test/e2e/tls_profile_test.go b/test/e2e/tls_profile_test.go index e95b89bb1..dcc176ec5 100644 --- a/test/e2e/tls_profile_test.go +++ b/test/e2e/tls_profile_test.go @@ -6,17 +6,28 @@ package e2e import ( "context" "fmt" + "strings" + "time" configapiv1 "github.com/openshift/api/config/v1" "github.com/openshift/cert-manager-operator/pkg/tlsprofile" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/kubernetes" + "k8s.io/utils/ptr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) +var csvSchema = schema.GroupVersionResource{ + Group: "operators.coreos.com", + Version: "v1alpha1", + Resource: "clusterserviceversions", +} + var _ = Describe("Cluster TLS security profile", Label("Platform:Generic", "Feature:TLSProfile", "TechPreview"), Ordered, func() { var ctx context.Context @@ -30,7 +41,8 @@ var _ = Describe("Cluster TLS security profile", Label("Platform:Generic", "Feat Expect(err).NotTo(HaveOccurred(), "Operator is expected to be available") }) - It("should configure operand container TLS args from apiserver cluster profile", func() { + // Journey 6 (partial): cert-manager operands always present — no TrustManager required. + It("should configure cert-manager operand container TLS args from apiserver cluster profile", func() { original, err := getClusterAPIServerTLSConfig(ctx) if apierrors.IsNotFound(err) { Skip("apiserver.config.openshift.io/cluster is not available on this cluster") @@ -68,14 +80,387 @@ var _ = Describe("Cluster TLS security profile", Label("Platform:Generic", "Feat err := verifyOperandTLSArgsMatchClusterProfile(name, expectedSpec) Expect(err).NotTo(HaveOccurred(), "deployment %s", name) } + }) + + It("should enable HTTPS metrics dynamic serving on cert-manager operands", func() { + By("verifying metrics-dynamic-serving args and prometheus.io/scheme=https annotation") + for _, name := range []string{ + certmanagerControllerDeployment, + certmanagerWebhookDeployment, + certmanagerCAinjectorDeployment, + } { + err := verifyOperandMetricsHTTPS(name) + Expect(err).NotTo(HaveOccurred(), "deployment %s", name) + } + }) + + It("should claim tls-profiles feature on the operator CSV when installed via OLM", func() { + installed, err := certManagerOperatorSubscriptionInstalled(ctx, loader) + Expect(err).NotTo(HaveOccurred()) + if !installed { + Skip("no OLM Subscription; CSV annotation check not applicable") + } + + By("listing ClusterServiceVersions in the operator namespace") + csvClient := loader.DynamicClient.Resource(csvSchema).Namespace(operatorNamespace) + csvs, err := csvClient.List(ctx, metav1.ListOptions{}) + Expect(err).NotTo(HaveOccurred()) + Expect(csvs.Items).NotTo(BeEmpty(), "expected at least one CSV in %s", operatorNamespace) + + found := false + for _, csv := range csvs.Items { + name := csv.GetName() + if !strings.Contains(name, "cert-manager-operator") { + continue + } + annotations := csv.GetAnnotations() + Expect(annotations).To(HaveKeyWithValue("features.operators.openshift.io/tls-profiles", "true"), + "CSV %s missing tls-profiles feature annotation", name) + found = true + break + } + Expect(found).To(BeTrue(), "no cert-manager-operator CSV found in %s", operatorNamespace) + }) + + // E2E-012 — Legacy: always-on HTTPS metrics remain; Strict profile TLS flags absent. + It("should keep HTTPS metrics under LegacyAdheringComponentsOnly while omitting profile TLS flags", func() { + original := requireAPIServerTLSConfig(ctx) + DeferCleanup(restoreAPIServerTLSConfigCleanup(ctx, original)) + + modernProfile := &configapiv1.TLSSecurityProfile{ + Type: configapiv1.TLSProfileModernType, + } + By("patching apiserver to LegacyAdheringComponentsOnly with Modern profile") + err := updateClusterAPIServerTLSConfig(ctx, modernProfile, configapiv1.TLSAdherencePolicyLegacyAdheringComponentsOnly) + if isTLSAdherenceUnsupported(err) { + Skip(fmt.Sprintf("apiserver tlsAdherence is not available on this cluster: %v", err)) + } + Expect(err).NotTo(HaveOccurred()) + + operandDeployments := []string{ + certmanagerControllerDeployment, + certmanagerWebhookDeployment, + certmanagerCAinjectorDeployment, + } + + By("verifying metrics-dynamic-serving HTTPS args remain present under Legacy") + for _, name := range operandDeployments { + Eventually(func() error { + return verifyOperandMetricsHTTPS(name) + }, lowTimeout, fastPollInterval).Should(Succeed(), "deployment %s", name) + } + + modernSpec, err := tlsprofile.EffectiveSpec(modernProfile) + Expect(err).NotTo(HaveOccurred()) + + By("waiting for Modern Strict profile TLS flags to be absent, then consistently re-checking") + for _, name := range operandDeployments { + unexpected := expectedOperandTLSArgs(name, modernSpec) + Expect(unexpected).NotTo(BeEmpty(), "deployment %s", name) - By("verifying trust-manager webhook TLS flags when the deployment is present") - _, tmErr := k8sClientSet.AppsV1().Deployments(operandNamespace).Get(ctx, "trust-manager", metav1.GetOptions{}) - if apierrors.IsNotFound(tmErr) { - Skip("trust-manager deployment not present; skipping trust-manager TLS profile verification") + err := waitForOperandTLSArgsAbsent(name, unexpected) + Expect(err).NotTo(HaveOccurred(), "deployment %s still has profile TLS flags under Legacy", name) + + Consistently(func() error { + return verifyOperandTLSArgsNotPresent(name, unexpected) + }, 15*time.Second, fastPollInterval).Should(Succeed(), "deployment %s", name) } - Expect(tmErr).NotTo(HaveOccurred(), "failed to get trust-manager deployment") - err = verifyOperandTLSArgsMatchClusterProfile("trust-manager", expectedSpec) - Expect(err).NotTo(HaveOccurred(), "deployment trust-manager") + }) + + // E2E-010 — Day-2 Modern → Intermediate on cert-manager operands. + It("should update cert-manager operand TLS args when apiserver profile changes Modern to Intermediate", func() { + original := requireAPIServerTLSConfig(ctx) + DeferCleanup(restoreAPIServerTLSConfigCleanup(ctx, original)) + + modernSpec := patchStrictTLSProfile(ctx, &configapiv1.TLSSecurityProfile{ + Type: configapiv1.TLSProfileModernType, + }) + + operandDeployments := []string{ + certmanagerControllerDeployment, + certmanagerWebhookDeployment, + certmanagerCAinjectorDeployment, + } + + By("verifying cert-manager operands match Modern EffectiveSpec") + for _, name := range operandDeployments { + err := verifyOperandTLSArgsMatchClusterProfile(name, modernSpec) + Expect(err).NotTo(HaveOccurred(), "modern profile args on %s", name) + } + + By("patching apiserver cluster profile to Intermediate") + intermediateSpec := patchStrictTLSProfile(ctx, &configapiv1.TLSSecurityProfile{ + Type: configapiv1.TLSProfileIntermediateType, + }) + + By("verifying cert-manager operands converge to Intermediate EffectiveSpec") + for _, name := range operandDeployments { + err := verifyOperandTLSArgsMatchClusterProfile(name, intermediateSpec) + Expect(err).NotTo(HaveOccurred(), "intermediate profile args on %s", name) + } + }) + + Context("trust-manager webhook TLS", Ordered, func() { + var ( + tmCtx = context.Background() + clientset *kubernetes.Clientset + originalUnsupportedAddonFeatures string + originalOperatorLogLevel string + ) + + BeforeAll(trustManagerBeforeAll(tmCtx, &clientset, &originalUnsupportedAddonFeatures, &originalOperatorLogLevel)) + AfterAll(trustManagerAfterAll(tmCtx, &originalUnsupportedAddonFeatures, &originalOperatorLogLevel)) + AfterEach(trustManagerAfterEach(tmCtx)) + + // Journey 1 — E2E-001 + It("should configure trust-manager webhook TLS args from StrictAllComponents Modern profile", func() { + original := requireAPIServerTLSConfig(tmCtx) + DeferCleanup(restoreAPIServerTLSConfigCleanup(tmCtx, original)) + + createTrustManager(tmCtx, newTrustManagerCR()) + expectTrustManagerDeploymentPresent(tmCtx) + + expectedSpec := patchStrictTLSProfile(tmCtx, &configapiv1.TLSSecurityProfile{ + Type: configapiv1.TLSProfileModernType, + }) + + By("verifying trust-manager webhook TLS flags match Modern EffectiveSpec") + err := verifyOperandTLSArgsMatchClusterProfile(trustManagerDeploymentName, expectedSpec) + Expect(err).NotTo(HaveOccurred(), "deployment %s", trustManagerDeploymentName) + + By("verifying trust-manager deployment is Available after TLS reconcile") + err = waitForDeploymentRollout(tmCtx, operandNamespace, trustManagerDeploymentName, lowTimeout) + Expect(err).NotTo(HaveOccurred()) + }) + + // Journey 2 — E2E-002 + It("should update trust-manager TLS args when apiserver profile changes Modern to Intermediate", func() { + original := requireAPIServerTLSConfig(tmCtx) + DeferCleanup(restoreAPIServerTLSConfigCleanup(tmCtx, original)) + + createTrustManager(tmCtx, newTrustManagerCR()) + expectTrustManagerDeploymentPresent(tmCtx) + + modernSpec := patchStrictTLSProfile(tmCtx, &configapiv1.TLSSecurityProfile{ + Type: configapiv1.TLSProfileModernType, + }) + err := verifyOperandTLSArgsMatchClusterProfile(trustManagerDeploymentName, modernSpec) + Expect(err).NotTo(HaveOccurred(), "modern profile args") + + By("patching apiserver cluster profile to Intermediate") + intermediateSpec := patchStrictTLSProfile(tmCtx, &configapiv1.TLSSecurityProfile{ + Type: configapiv1.TLSProfileIntermediateType, + }) + + By("verifying trust-manager args converge to Intermediate EffectiveSpec") + err = verifyOperandTLSArgsMatchClusterProfile(trustManagerDeploymentName, intermediateSpec) + Expect(err).NotTo(HaveOccurred(), "intermediate profile args") + }) + + // E2E-011 — Intermediate → Modern strips cipher flags on cert-manager operands + trust-manager. + It("should strip TLS cipher flags when apiserver profile changes Intermediate to Modern", func() { + original := requireAPIServerTLSConfig(tmCtx) + DeferCleanup(restoreAPIServerTLSConfigCleanup(tmCtx, original)) + + createTrustManager(tmCtx, newTrustManagerCR()) + expectTrustManagerDeploymentPresent(tmCtx) + + deployments := []string{ + certmanagerControllerDeployment, + certmanagerWebhookDeployment, + certmanagerCAinjectorDeployment, + trustManagerDeploymentName, + } + + By("applying Strict Intermediate and confirming cipher-bearing TLS args") + intermediateSpec := patchStrictTLSProfile(tmCtx, &configapiv1.TLSSecurityProfile{ + Type: configapiv1.TLSProfileIntermediateType, + }) + for _, name := range deployments { + err := verifyOperandTLSArgsMatchClusterProfile(name, intermediateSpec) + Expect(err).NotTo(HaveOccurred(), "intermediate profile args on %s", name) + } + + By("patching apiserver cluster profile to Modern (TLS1.3)") + modernSpec := patchStrictTLSProfile(tmCtx, &configapiv1.TLSSecurityProfile{ + Type: configapiv1.TLSProfileModernType, + }) + + By("verifying deployments converge to Modern min-version args with cipher flags stripped") + for _, name := range deployments { + err := verifyOperandTLSArgsMatchClusterProfile(name, modernSpec) + Expect(err).NotTo(HaveOccurred(), "modern profile args / cipher strip on %s", name) + } + }) + + // Journey 3 — E2E-003 + It("should apply Custom TLS profile flags to trust-manager", func() { + original := requireAPIServerTLSConfig(tmCtx) + DeferCleanup(restoreAPIServerTLSConfigCleanup(tmCtx, original)) + + createTrustManager(tmCtx, newTrustManagerCR()) + expectTrustManagerDeploymentPresent(tmCtx) + + customProfile := &configapiv1.TLSSecurityProfile{ + Type: configapiv1.TLSProfileCustomType, + Custom: &configapiv1.CustomTLSProfile{ + TLSProfileSpec: configapiv1.TLSProfileSpec{ + MinTLSVersion: configapiv1.VersionTLS12, + Ciphers: []string{ + "ECDHE-ECDSA-AES128-GCM-SHA256", + "ECDHE-RSA-AES128-GCM-SHA256", + }, + }, + }, + } + expectedSpec := patchStrictTLSProfile(tmCtx, customProfile) + + By("verifying trust-manager args match Custom EffectiveSpec") + err := verifyOperandTLSArgsMatchClusterProfile(trustManagerDeploymentName, expectedSpec) + Expect(err).NotTo(HaveOccurred(), "custom profile args") + }) + + // Journey 4 — E2E-004 + It("should keep trust-manager Certificate Ready after Modern TLS profile is applied", func() { + original := requireAPIServerTLSConfig(tmCtx) + DeferCleanup(restoreAPIServerTLSConfigCleanup(tmCtx, original)) + + createTrustManager(tmCtx, newTrustManagerCR()) + expectTrustManagerDeploymentPresent(tmCtx) + + By("waiting for trust-manager Certificate to become ready before TLS patch") + err := waitForCertificateReadiness(tmCtx, trustManagerCertificateName, trustManagerNamespace) + Expect(err).NotTo(HaveOccurred()) + + expectedSpec := patchStrictTLSProfile(tmCtx, &configapiv1.TLSSecurityProfile{ + Type: configapiv1.TLSProfileModernType, + }) + err = verifyOperandTLSArgsMatchClusterProfile(trustManagerDeploymentName, expectedSpec) + Expect(err).NotTo(HaveOccurred()) + + By("re-checking Certificate readiness and TLS secret after profile application") + err = waitForCertificateReadiness(tmCtx, trustManagerCertificateName, trustManagerNamespace) + Expect(err).NotTo(HaveOccurred()) + + Eventually(func(g Gomega) { + secret, err := k8sClientSet.CoreV1().Secrets(trustManagerNamespace).Get(tmCtx, trustManagerTLSSecretName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(secret.Data).To(HaveKey("tls.crt")) + g.Expect(secret.Data).To(HaveKey("tls.key")) + }, lowTimeout, fastPollInterval).Should(Succeed()) + }) + + // Journey 5 — NEG-001 + It("should not apply Modern TLS flags to trust-manager under LegacyAdheringComponentsOnly", func() { + original := requireAPIServerTLSConfig(tmCtx) + DeferCleanup(restoreAPIServerTLSConfigCleanup(tmCtx, original)) + + modernProfile := &configapiv1.TLSSecurityProfile{ + Type: configapiv1.TLSProfileModernType, + } + By("patching apiserver to LegacyAdheringComponentsOnly with Modern profile before TrustManager exists") + err := updateClusterAPIServerTLSConfig(tmCtx, modernProfile, configapiv1.TLSAdherencePolicyLegacyAdheringComponentsOnly) + if isTLSAdherenceUnsupported(err) { + Skip(fmt.Sprintf("apiserver tlsAdherence is not available on this cluster: %v", err)) + } + Expect(err).NotTo(HaveOccurred()) + + createTrustManager(tmCtx, newTrustManagerCR()) + expectTrustManagerDeploymentPresent(tmCtx) + + modernSpec, err := tlsprofile.EffectiveSpec(modernProfile) + Expect(err).NotTo(HaveOccurred()) + unexpected := tlsprofile.TrustManagerWebhookTLSArgs(modernSpec) + Expect(unexpected).NotTo(BeEmpty()) + + By("consistently verifying Modern Strict TLS args are absent on trust-manager") + Consistently(func() error { + return verifyOperandTLSArgsNotPresent(trustManagerDeploymentName, unexpected) + }, 15*time.Second, fastPollInterval).Should(Succeed()) + }) + + // Journey 5 — NEG-002 + It("should retain trust-manager TLS args after operator pod restart under Strict Modern", func() { + original := requireAPIServerTLSConfig(tmCtx) + DeferCleanup(restoreAPIServerTLSConfigCleanup(tmCtx, original)) + + createTrustManager(tmCtx, newTrustManagerCR()) + expectTrustManagerDeploymentPresent(tmCtx) + + expectedSpec := patchStrictTLSProfile(tmCtx, &configapiv1.TLSSecurityProfile{ + Type: configapiv1.TLSProfileModernType, + }) + err := verifyOperandTLSArgsMatchClusterProfile(trustManagerDeploymentName, expectedSpec) + Expect(err).NotTo(HaveOccurred()) + + By("deleting operator controller-manager pods") + err = deleteOperatorControllerPods(tmCtx) + Expect(err).NotTo(HaveOccurred()) + + By("waiting for operator to become healthy again") + Eventually(func() error { + return VerifyHealthyOperatorConditions(certmanageroperatorclient.OperatorV1alpha1()) + }, lowTimeout, fastPollInterval).Should(Succeed()) + + By("verifying trust-manager TLS args still match Modern EffectiveSpec") + err = verifyOperandTLSArgsMatchClusterProfile(trustManagerDeploymentName, expectedSpec) + Expect(err).NotTo(HaveOccurred()) + }) }) }) + +func requireAPIServerTLSConfig(ctx context.Context) *apiserverTLSConfig { + GinkgoHelper() + original, err := getClusterAPIServerTLSConfig(ctx) + if apierrors.IsNotFound(err) { + Skip("apiserver.config.openshift.io/cluster is not available on this cluster") + } + Expect(err).NotTo(HaveOccurred(), "failed to read apiserver TLS configuration") + return original +} + +func restoreAPIServerTLSConfigCleanup(ctx context.Context, original *apiserverTLSConfig) func() { + return func() { + By("[cleanup] restoring original apiserver TLS configuration") + Eventually(func() error { + return restoreClusterAPIServerTLSConfig(ctx, original) + }, lowTimeout, fastPollInterval).Should(Succeed()) + } +} + +func patchStrictTLSProfile(ctx context.Context, profile *configapiv1.TLSSecurityProfile) *configapiv1.TLSProfileSpec { + GinkgoHelper() + By(fmt.Sprintf("patching apiserver cluster to StrictAllComponents with %s TLS profile", profile.Type)) + err := updateClusterAPIServerTLSConfig(ctx, profile, configapiv1.TLSAdherencePolicyStrictAllComponents) + if isTLSAdherenceUnsupported(err) { + Skip(fmt.Sprintf("apiserver tlsAdherence is not available on this cluster: %v", err)) + } + Expect(err).NotTo(HaveOccurred(), "failed to patch apiserver TLS configuration") + + expectedSpec, err := tlsprofile.EffectiveSpec(profile) + Expect(err).NotTo(HaveOccurred(), "failed to resolve expected TLS profile spec") + return expectedSpec +} + +func expectTrustManagerDeploymentPresent(ctx context.Context) { + GinkgoHelper() + By("waiting for trust-manager deployment to exist") + Eventually(func() error { + _, err := k8sClientSet.AppsV1().Deployments(operandNamespace).Get(ctx, trustManagerDeploymentName, metav1.GetOptions{}) + return err + }, lowTimeout, fastPollInterval).Should(Succeed()) +} + +func deleteOperatorControllerPods(ctx context.Context) error { + dep, err := k8sClientSet.AppsV1().Deployments(operatorNamespace).Get(ctx, operatorDeploymentName, metav1.GetOptions{}) + if err != nil { + return err + } + selector, err := metav1.LabelSelectorAsSelector(dep.Spec.Selector) + if err != nil { + return err + } + return k8sClientSet.CoreV1().Pods(operatorNamespace).DeleteCollection(ctx, metav1.DeleteOptions{ + GracePeriodSeconds: ptr.To[int64](0), + }, metav1.ListOptions{LabelSelector: selector.String()}) +} diff --git a/test/e2e/utils_test.go b/test/e2e/utils_test.go index 1bff8322c..9dc92a21d 100644 --- a/test/e2e/utils_test.go +++ b/test/e2e/utils_test.go @@ -1772,6 +1772,31 @@ func getClusterAPIServerTLSConfig(ctx context.Context) (*apiserverTLSConfig, err return cfg, nil } +// normalizeTLSSecurityProfile ensures the union member matching Type is non-nil. +// The APIServer validation requires e.g. spec.tlsSecurityProfile.modern when type=Modern; +// omitempty on empty structs otherwise drops the field and the update is rejected. +func normalizeTLSSecurityProfile(profile *configapiv1.TLSSecurityProfile) *configapiv1.TLSSecurityProfile { + if profile == nil { + return nil + } + out := profile.DeepCopy() + switch out.Type { + case configapiv1.TLSProfileOldType: + if out.Old == nil { + out.Old = &configapiv1.OldTLSProfile{} + } + case configapiv1.TLSProfileIntermediateType: + if out.Intermediate == nil { + out.Intermediate = &configapiv1.IntermediateTLSProfile{} + } + case configapiv1.TLSProfileModernType: + if out.Modern == nil { + out.Modern = &configapiv1.ModernTLSProfile{} + } + } + return out +} + // updateClusterAPIServerTLSConfig patches apiserver.config.openshift.io/cluster TLS settings. func updateClusterAPIServerTLSConfig(ctx context.Context, profile *configapiv1.TLSSecurityProfile, adherence configapiv1.TLSAdherencePolicy) error { return retry.RetryOnConflict(retry.DefaultRetry, func() error { @@ -1782,7 +1807,7 @@ func updateClusterAPIServerTLSConfig(ctx context.Context, profile *configapiv1.T updated := apiServer.DeepCopy() if profile != nil { - updated.Spec.TLSSecurityProfile = profile.DeepCopy() + updated.Spec.TLSSecurityProfile = normalizeTLSSecurityProfile(profile) } else { updated.Spec.TLSSecurityProfile = nil } @@ -1874,6 +1899,94 @@ func verifyOperandTLSArgsMatchClusterProfile(deploymentName string, spec *config }) } +// verifyOperandTLSArgsNotPresent succeeds when none of the given args are present on the +// deployment's first container. Used for Legacy adherence negative checks. +func verifyOperandTLSArgsNotPresent(deploymentName string, unexpected []string) error { + if len(unexpected) == 0 { + return fmt.Errorf("unexpected arg list is empty") + } + + deployment, err := k8sClientSet.AppsV1().Deployments(operandNamespace).Get(context.TODO(), deploymentName, metav1.GetOptions{}) + if err != nil { + return err + } + if len(deployment.Spec.Template.Spec.Containers) == 0 { + return fmt.Errorf("deployment %q has no containers", deploymentName) + } + + args := sets.New(deployment.Spec.Template.Spec.Containers[0].Args...) + var present []string + for _, u := range unexpected { + if args.Has(u) { + present = append(present, u) + } + } + if len(present) > 0 { + return fmt.Errorf("deployment %q unexpectedly has TLS args %v", deploymentName, present) + } + return nil +} + +// waitForOperandTLSArgsAbsent polls until unexpected TLS args are gone from the deployment +// (e.g. after switching to Legacy adherence). Uses the same retry budget as other TLS helpers. +func waitForOperandTLSArgsAbsent(deploymentName string, unexpected []string) error { + return wait.PollUntilContextTimeout(context.TODO(), fastPollInterval, lowTimeout, true, func(context.Context) (bool, error) { + err := verifyOperandTLSArgsNotPresent(deploymentName, unexpected) + if err == nil { + return true, nil + } + if apierrors.IsNotFound(err) { + return false, nil + } + // Args still present or transient read issues — keep polling until timeout. + return false, nil + }) +} + +// verifyOperandMetricsHTTPS waits until cert-manager operand deployments expose +// dynamic metrics serving flags and prometheus.io/scheme=https on the pod template. +func verifyOperandMetricsHTTPS(deploymentName string) error { + return wait.PollUntilContextTimeout(context.TODO(), fastPollInterval, lowTimeout, true, func(context.Context) (bool, error) { + deployment, err := k8sClientSet.AppsV1().Deployments(operandNamespace).Get(context.TODO(), deploymentName, metav1.GetOptions{}) + if err != nil { + if apierrors.IsNotFound(err) { + return false, nil + } + return false, err + } + if len(deployment.Spec.Template.Spec.Containers) == 0 { + return false, fmt.Errorf("deployment %q has no containers", deploymentName) + } + + args := sets.New(deployment.Spec.Template.Spec.Containers[0].Args...) + required := []string{ + "--metrics-dynamic-serving-ca-secret-namespace=$(POD_NAMESPACE)", + "--metrics-dynamic-serving-ca-secret-name=cert-manager-metrics-ca", + } + for _, req := range required { + if !args.Has(req) { + return false, nil + } + } + + hasDNSNames := false + for _, arg := range deployment.Spec.Template.Spec.Containers[0].Args { + if strings.HasPrefix(arg, "--metrics-dynamic-serving-dns-names=") { + hasDNSNames = true + break + } + } + if !hasDNSNames { + return false, nil + } + + if deployment.Spec.Template.Annotations["prometheus.io/scheme"] != "https" { + return false, nil + } + return true, nil + }) +} + // isSTSCluster checks if the AWS/GCP/Azure cluster is using Security Token Service or Workload Identity // by checking if the serviceAccountIssuer is configured in the cluster's Authentication config func isSTSCluster(ctx context.Context, opClient operatorv1.OperatorV1Interface, configClient configv1.ConfigV1Interface) (bool, error) { From 5dbd644f8749da286e18cfc2de35c4e0e9082dcf Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Wed, 12 Aug 2026 17:51:53 +0530 Subject: [PATCH 10/10] CM-1225: fix ConditionMatcher Any-mode short-circuit false negative Matches() returned false on the first type-matching condition with an unexpected status even when Any=true, so an unrelated condition (e.g. *-static-resources-Degraded=False) could mask a later matching one (e.g. *-deploymentDegraded=True), causing verifyOperatorStatusCondition to time out instead of succeeding. This caused the flaky "Overrides test ... cainjector override args" e2e failure where the operator had already correctly degraded the deployment. Gate the mismatch short-circuit behind !Any and add a regression test reproducing the exact condition ordering seen in CI. --- test/e2e/condition_matcher_test.go | 18 +++++++++++++++--- test/e2e/condition_matcher_unit_test.go | 18 ++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/test/e2e/condition_matcher_test.go b/test/e2e/condition_matcher_test.go index 8f2865554..e6fa92de3 100644 --- a/test/e2e/condition_matcher_test.go +++ b/test/e2e/condition_matcher_test.go @@ -43,20 +43,32 @@ func (m *ConditionMatcher) MatchesStatus(cond *opv1.OperatorCondition) bool { func (m *ConditionMatcher) Matches(conditions []opv1.OperatorCondition) bool { matchCount := 0 for _, cond := range conditions { - if m.MatchesType(&cond) && m.MatchesStatus(&cond) { + if !m.MatchesType(&cond) { + continue + } + if m.MatchesStatus(&cond) { if m.Any { return true } - matchCount += 1 + continue } - if m.MatchesType(&cond) && !m.MatchesStatus(&cond) { + // Status mismatch on a matching type: in "match all" mode this + // disqualifies the whole matcher immediately. In "match any" mode, + // a mismatch here must not short-circuit other conditions of the + // same type (e.g. "*-deploymentDegraded" vs "*-static-resources-Degraded") + // that may still satisfy the expected status later in the slice. + if !m.Any { return false } } + if m.Any { + return false + } + return matchCount > 0 } diff --git a/test/e2e/condition_matcher_unit_test.go b/test/e2e/condition_matcher_unit_test.go index d88ea3b72..c70f06df5 100644 --- a/test/e2e/condition_matcher_unit_test.go +++ b/test/e2e/condition_matcher_unit_test.go @@ -61,6 +61,24 @@ func TestVerifyOperatorStatusCondition(t *testing.T) { expectError: false, errorContains: "context deadline exceeded", }, + { + // Reproduces the CI flake from PR #466: cainjector's "-static-resources-Degraded" + // (unrelated, correctly False) is visited before "-deploymentDegraded" (True, the + // one we actually care about). With Any=true, the mismatch on the first condition + // must not short-circuit the search for a later matching condition. + name: "Any matches later condition even when an earlier same-type condition has the wrong status", + expectedConditions: map[string]opv1.ConditionStatus{ + "Degraded": opv1.ConditionTrue, + }, + initialObjects: []runtime.Object{ + newCertManagerObjectWithConditions( + opv1.OperatorCondition{Type: controllerPrefix + "-static-resources-Degraded", Status: opv1.ConditionFalse}, + opv1.OperatorCondition{Type: controllerPrefix + "-deploymentDegraded", Status: opv1.ConditionTrue}, + ), + }, + matchAny: true, + expectError: false, + }, { name: "Both degraded is false", expectedConditions: map[string]opv1.ConditionStatus{