From 8a64905f6b15d4383758b3c5ec21971991abebb2 Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Thu, 10 Sep 2026 14:56:12 +0530 Subject: [PATCH 1/7] CM-1367: Add TrustManager filterNonCACerts API. Expose Enabled/Disabled on spec and status and pass --filter-non-ca-certs=true when Enabled. Requires a trust-manager operand that supports the flag (v0.21.0+). --- .../trustmanager.testsuite.yaml | 82 +++++++++++++++++++ api/operator/v1alpha1/trustmanager_types.go | 25 ++++++ .../operator.openshift.io_trustmanagers.yaml | 19 +++++ .../operator.openshift.io_trustmanagers.yaml | 19 +++++ pkg/controller/trustmanager/deployments.go | 4 + .../trustmanager/deployments_test.go | 19 ++++- .../trustmanager/install_trustmanager.go | 5 ++ .../trustmanager/install_trustmanager_test.go | 4 + pkg/controller/trustmanager/test_utils.go | 5 ++ .../operator/v1alpha1/trustmanagerconfig.go | 14 ++++ .../operator/v1alpha1/trustmanagerstatus.go | 10 +++ test/e2e/trustmanager_bundle_test.go | 82 ++++++++++++++++++- test/e2e/trustmanager_helpers_test.go | 7 +- test/e2e/trustmanager_test.go | 37 +++++++++ 14 files changed, 327 insertions(+), 5 deletions(-) diff --git a/api/operator/v1alpha1/tests/trustmanagers.operator.openshift.io/trustmanager.testsuite.yaml b/api/operator/v1alpha1/tests/trustmanagers.operator.openshift.io/trustmanager.testsuite.yaml index 6a22b6de9..edb326e31 100644 --- a/api/operator/v1alpha1/tests/trustmanagers.operator.openshift.io/trustmanager.testsuite.yaml +++ b/api/operator/v1alpha1/tests/trustmanagers.operator.openshift.io/trustmanager.testsuite.yaml @@ -22,6 +22,7 @@ tests: logFormat: text trustNamespace: cert-manager filterExpiredCertificates: Disabled + filterNonCACerts: Disabled - name: Should not allow creating TrustManager with invalid name resourceName: invalid-name @@ -52,6 +53,7 @@ tests: logFormat: text trustNamespace: cert-manager filterExpiredCertificates: Disabled + filterNonCACerts: Disabled - name: Should not allow logLevel below minimum resourceName: cluster @@ -93,6 +95,7 @@ tests: logFormat: json trustNamespace: cert-manager filterExpiredCertificates: Disabled + filterNonCACerts: Disabled - name: Should not allow invalid logFormat resourceName: cluster @@ -124,6 +127,7 @@ tests: logFormat: text trustNamespace: custom-namespace filterExpiredCertificates: Disabled + filterNonCACerts: Disabled # ========================================== # FilterExpiredCertificates Tests @@ -136,6 +140,7 @@ tests: spec: trustManagerConfig: filterExpiredCertificates: Enabled + filterNonCACerts: Disabled expected: | apiVersion: operator.openshift.io/v1alpha1 kind: TrustManager @@ -145,6 +150,7 @@ tests: logFormat: text trustNamespace: cert-manager filterExpiredCertificates: Enabled + filterNonCACerts: Disabled - name: Should not allow invalid filterExpiredCertificates value resourceName: cluster @@ -154,8 +160,41 @@ tests: spec: trustManagerConfig: filterExpiredCertificates: Invalid + filterNonCACerts: Disabled expectedError: "filterExpiredCertificates" + # ========================================== + # FilterNonCACerts Tests + # ========================================== + - name: Should create with filterNonCACerts Enabled + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: TrustManager + spec: + trustManagerConfig: + filterNonCACerts: Enabled + expected: | + apiVersion: operator.openshift.io/v1alpha1 + kind: TrustManager + spec: + trustManagerConfig: + logLevel: 1 + logFormat: text + trustNamespace: cert-manager + filterExpiredCertificates: Disabled + filterNonCACerts: Enabled + + - name: Should not allow invalid filterNonCACerts value + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: TrustManager + spec: + trustManagerConfig: + filterNonCACerts: Invalid + expectedError: "filterNonCACerts" + # ========================================== # DefaultCAPackage Tests # ========================================== @@ -177,6 +216,7 @@ tests: logFormat: text trustNamespace: cert-manager filterExpiredCertificates: Disabled + filterNonCACerts: Disabled defaultCAPackage: policy: Enabled @@ -215,6 +255,7 @@ tests: logFormat: text trustNamespace: cert-manager filterExpiredCertificates: Disabled + filterNonCACerts: Disabled secretTargets: policy: Custom authorizedSecrets: @@ -282,6 +323,7 @@ tests: logFormat: text trustNamespace: cert-manager filterExpiredCertificates: Disabled + filterNonCACerts: Disabled resources: requests: cpu: 100m @@ -315,6 +357,7 @@ tests: logFormat: text trustNamespace: cert-manager filterExpiredCertificates: Disabled + filterNonCACerts: Disabled affinity: nodeAffinity: requiredDuringSchedulingIgnoredDuringExecution: @@ -345,6 +388,7 @@ tests: logFormat: text trustNamespace: cert-manager filterExpiredCertificates: Disabled + filterNonCACerts: Disabled tolerations: - key: node-role.kubernetes.io/master operator: Exists @@ -368,6 +412,7 @@ tests: logFormat: text trustNamespace: cert-manager filterExpiredCertificates: Disabled + filterNonCACerts: Disabled nodeSelector: kubernetes.io/os: linux @@ -393,6 +438,7 @@ tests: logFormat: text trustNamespace: cert-manager filterExpiredCertificates: Disabled + filterNonCACerts: Disabled controllerConfig: labels: app.kubernetes.io/managed-by: cert-manager-operator @@ -416,6 +462,7 @@ tests: logFormat: text trustNamespace: cert-manager filterExpiredCertificates: Disabled + filterNonCACerts: Disabled controllerConfig: annotations: custom-annotation: custom-value @@ -463,6 +510,7 @@ tests: logFormat: text trustNamespace: cert-manager filterExpiredCertificates: Disabled + filterNonCACerts: Disabled - name: Should allow updating logFormat resourceName: cluster @@ -487,6 +535,7 @@ tests: logFormat: json trustNamespace: cert-manager filterExpiredCertificates: Disabled + filterNonCACerts: Disabled - name: Should allow updating filterExpiredCertificates resourceName: cluster @@ -496,12 +545,14 @@ tests: spec: trustManagerConfig: filterExpiredCertificates: Disabled + filterNonCACerts: Disabled updated: | apiVersion: operator.openshift.io/v1alpha1 kind: TrustManager spec: trustManagerConfig: filterExpiredCertificates: Enabled + filterNonCACerts: Disabled expected: | apiVersion: operator.openshift.io/v1alpha1 kind: TrustManager @@ -511,6 +562,32 @@ tests: logFormat: text trustNamespace: cert-manager filterExpiredCertificates: Enabled + filterNonCACerts: Disabled + + - name: Should allow updating filterNonCACerts + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: TrustManager + spec: + trustManagerConfig: + filterNonCACerts: Disabled + updated: | + apiVersion: operator.openshift.io/v1alpha1 + kind: TrustManager + spec: + trustManagerConfig: + filterNonCACerts: Enabled + expected: | + apiVersion: operator.openshift.io/v1alpha1 + kind: TrustManager + spec: + trustManagerConfig: + logLevel: 1 + logFormat: text + trustNamespace: cert-manager + filterExpiredCertificates: Disabled + filterNonCACerts: Enabled - name: Should allow updating defaultCAPackage policy resourceName: cluster @@ -537,6 +614,7 @@ tests: logFormat: text trustNamespace: cert-manager filterExpiredCertificates: Disabled + filterNonCACerts: Disabled defaultCAPackage: policy: Enabled @@ -567,6 +645,7 @@ tests: logFormat: text trustNamespace: cert-manager filterExpiredCertificates: Disabled + filterNonCACerts: Disabled secretTargets: policy: Custom authorizedSecrets: @@ -602,6 +681,7 @@ tests: logFormat: text trustNamespace: cert-manager filterExpiredCertificates: Disabled + filterNonCACerts: Disabled secretTargets: policy: Custom authorizedSecrets: @@ -635,6 +715,7 @@ tests: logFormat: text trustNamespace: cert-manager filterExpiredCertificates: Disabled + filterNonCACerts: Disabled secretTargets: policy: Disabled @@ -710,6 +791,7 @@ tests: logFormat: text trustNamespace: cert-manager filterExpiredCertificates: Disabled + filterNonCACerts: Disabled secretTargets: policy: Custom authorizedSecrets: diff --git a/api/operator/v1alpha1/trustmanager_types.go b/api/operator/v1alpha1/trustmanager_types.go index a18caf370..ebf9332ee 100644 --- a/api/operator/v1alpha1/trustmanager_types.go +++ b/api/operator/v1alpha1/trustmanager_types.go @@ -118,6 +118,17 @@ type TrustManagerConfig struct { // +optional FilterExpiredCertificates FilterExpiredCertificatesPolicy `json:"filterExpiredCertificates,omitempty"` + // filterNonCACerts controls whether trust-manager filters out + // non-CA certificates from trust bundles before distributing them. + // When set to "Enabled", only certificates with the X.509 basicConstraints + // CA bit set are included in bundles. + // When set to "Disabled", non-CA certificates are included (default behavior). + // +kubebuilder:default:="Disabled" + // +kubebuilder:validation:Enum:=Enabled;Disabled + // +kubebuilder:validation:Optional + // +optional + FilterNonCACerts FilterNonCACertsPolicy `json:"filterNonCACerts,omitempty"` + // defaultCAPackage configures the default CA package for trust-manager. // When enabled, the operator will use OpenShift's trusted CA bundle injection mechanism. // +kubebuilder:validation:Optional @@ -225,6 +236,16 @@ const ( FilterExpiredCertificatesPolicyDisabled FilterExpiredCertificatesPolicy = "Disabled" ) +// FilterNonCACertsPolicy defines the policy for filtering non-CA certificates. +type FilterNonCACertsPolicy string + +const ( + // FilterNonCACertsPolicyEnabled filters out non-CA certificates from bundles. + FilterNonCACertsPolicyEnabled FilterNonCACertsPolicy = "Enabled" + // FilterNonCACertsPolicyDisabled includes non-CA certificates in bundles. + FilterNonCACertsPolicyDisabled FilterNonCACertsPolicy = "Disabled" +) + // SecretTargetsPolicy defines the policy for writing trust bundles to Secrets. type SecretTargetsPolicy string @@ -268,4 +289,8 @@ type TrustManagerStatus struct { // filterExpiredCertificatesPolicy indicates the current policy for filtering expired certificates. // +kubebuilder:validation:Enum:=Enabled;Disabled FilterExpiredCertificatesPolicy FilterExpiredCertificatesPolicy `json:"filterExpiredCertificatesPolicy,omitempty"` + + // filterNonCACertsPolicy indicates the current policy for filtering non-CA certificates. + // +kubebuilder:validation:Enum:=Enabled;Disabled + FilterNonCACertsPolicy FilterNonCACertsPolicy `json:"filterNonCACertsPolicy,omitempty"` } diff --git a/bundle/manifests/operator.openshift.io_trustmanagers.yaml b/bundle/manifests/operator.openshift.io_trustmanagers.yaml index 2da748fda..3ec28a0e4 100644 --- a/bundle/manifests/operator.openshift.io_trustmanagers.yaml +++ b/bundle/manifests/operator.openshift.io_trustmanagers.yaml @@ -1038,6 +1038,18 @@ spec: - Enabled - Disabled type: string + filterNonCACerts: + default: Disabled + description: |- + filterNonCACerts controls whether trust-manager filters out + non-CA certificates from trust bundles before distributing them. + When set to "Enabled", only certificates with the X.509 basicConstraints + CA bit set are included in bundles. + When set to "Disabled", non-CA certificates are included (default behavior). + enum: + - Enabled + - Disabled + type: string logFormat: default: text description: |- @@ -1304,6 +1316,13 @@ spec: - Enabled - Disabled type: string + filterNonCACertsPolicy: + description: filterNonCACertsPolicy indicates the current policy for + filtering non-CA certificates. + enum: + - Enabled + - Disabled + type: string secretTargetsPolicy: description: secretTargetsPolicy indicates the current secret targets policy. diff --git a/config/crd/bases/operator.openshift.io_trustmanagers.yaml b/config/crd/bases/operator.openshift.io_trustmanagers.yaml index 0a334f47d..94c96d274 100644 --- a/config/crd/bases/operator.openshift.io_trustmanagers.yaml +++ b/config/crd/bases/operator.openshift.io_trustmanagers.yaml @@ -1038,6 +1038,18 @@ spec: - Enabled - Disabled type: string + filterNonCACerts: + default: Disabled + description: |- + filterNonCACerts controls whether trust-manager filters out + non-CA certificates from trust bundles before distributing them. + When set to "Enabled", only certificates with the X.509 basicConstraints + CA bit set are included in bundles. + When set to "Disabled", non-CA certificates are included (default behavior). + enum: + - Enabled + - Disabled + type: string logFormat: default: text description: |- @@ -1304,6 +1316,13 @@ spec: - Enabled - Disabled type: string + filterNonCACertsPolicy: + description: filterNonCACertsPolicy indicates the current policy for + filtering non-CA certificates. + enum: + - Enabled + - Disabled + type: string secretTargetsPolicy: description: secretTargetsPolicy indicates the current secret targets policy. diff --git a/pkg/controller/trustmanager/deployments.go b/pkg/controller/trustmanager/deployments.go index da96689d4..841326c80 100644 --- a/pkg/controller/trustmanager/deployments.go +++ b/pkg/controller/trustmanager/deployments.go @@ -147,6 +147,10 @@ func updateDeploymentArgs(deployment *appsv1.Deployment, trustManager *v1alpha1. args = append(args, "--filter-expired-certificates=true") } + if config.FilterNonCACerts == v1alpha1.FilterNonCACertsPolicyEnabled { + args = append(args, "--filter-non-ca-certs=true") + } + if defaultCAPackageEnabled(config.DefaultCAPackage) { args = append(args, fmt.Sprintf("--default-package-location=%s", defaultCAPackageLocation)) } diff --git a/pkg/controller/trustmanager/deployments_test.go b/pkg/controller/trustmanager/deployments_test.go index 8e86c456d..3ffe761cd 100644 --- a/pkg/controller/trustmanager/deployments_test.go +++ b/pkg/controller/trustmanager/deployments_test.go @@ -148,6 +148,7 @@ func TestDeploymentContainerArgs(t *testing.T) { notExpectedArgs: []string{ "--secret-targets-enabled=true", "--filter-expired-certificates=true", + "--filter-non-ca-certs=true", fmt.Sprintf("--default-package-location=%s", defaultCAPackageLocation), }, }, @@ -157,12 +158,14 @@ func TestDeploymentContainerArgs(t *testing.T) { WithLogLevel(5). WithLogFormat("json"). WithTrustNamespace("custom-ns"). - WithFilterExpiredCertificates(v1alpha1.FilterExpiredCertificatesPolicyEnabled), + WithFilterExpiredCertificates(v1alpha1.FilterExpiredCertificatesPolicyEnabled). + WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicyEnabled), expectedArgs: []string{ "--log-level=5", "--log-format=json", "--trust-namespace=custom-ns", "--filter-expired-certificates=true", + "--filter-non-ca-certs=true", }, notExpectedArgs: []string{ "--log-level=1", @@ -210,6 +213,20 @@ func TestDeploymentContainerArgs(t *testing.T) { fmt.Sprintf("--default-package-location=%s", defaultCAPackageLocation), }, }, + { + name: "includes filter-non-ca-certs when filterNonCACerts is Enabled", + tmBuilder: testTrustManager().WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicyEnabled), + expectedArgs: []string{ + "--filter-non-ca-certs=true", + }, + }, + { + name: "excludes filter-non-ca-certs when filterNonCACerts is Disabled", + tmBuilder: testTrustManager().WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicyDisabled), + notExpectedArgs: []string{ + "--filter-non-ca-certs=true", + }, + }, } for _, tt := range tests { diff --git a/pkg/controller/trustmanager/install_trustmanager.go b/pkg/controller/trustmanager/install_trustmanager.go index 932253355..32294928e 100644 --- a/pkg/controller/trustmanager/install_trustmanager.go +++ b/pkg/controller/trustmanager/install_trustmanager.go @@ -112,6 +112,11 @@ func (r *Reconciler) updateStatusObservedState(trustManager *v1alpha1.TrustManag changed = true } + if policy := trustManager.Spec.TrustManagerConfig.FilterNonCACerts; trustManager.Status.FilterNonCACertsPolicy != policy { + trustManager.Status.FilterNonCACertsPolicy = policy + changed = true + } + if !changed { return nil } diff --git a/pkg/controller/trustmanager/install_trustmanager_test.go b/pkg/controller/trustmanager/install_trustmanager_test.go index 437022391..b5382f9d3 100644 --- a/pkg/controller/trustmanager/install_trustmanager_test.go +++ b/pkg/controller/trustmanager/install_trustmanager_test.go @@ -21,6 +21,7 @@ func TestUpdateStatusObservedState(t *testing.T) { SecretTargetsPolicy: "", DefaultCAPackagePolicy: "", FilterExpiredCertificatesPolicy: "", + FilterNonCACertsPolicy: "", } tests := []struct { @@ -45,6 +46,7 @@ func TestUpdateStatusObservedState(t *testing.T) { WithSecretTargets(v1alpha1.SecretTargetsPolicyCustom, []string{"allowed-secret"}). WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled). WithFilterExpiredCertificates(v1alpha1.FilterExpiredCertificatesPolicyEnabled). + WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicyEnabled). Build() }, wantStatusUpdate: 1, @@ -54,6 +56,7 @@ func TestUpdateStatusObservedState(t *testing.T) { SecretTargetsPolicy: v1alpha1.SecretTargetsPolicyCustom, DefaultCAPackagePolicy: v1alpha1.DefaultCAPackagePolicyEnabled, FilterExpiredCertificatesPolicy: v1alpha1.FilterExpiredCertificatesPolicyEnabled, + FilterNonCACertsPolicy: v1alpha1.FilterNonCACertsPolicyEnabled, }, }, { @@ -65,6 +68,7 @@ func TestUpdateStatusObservedState(t *testing.T) { tm.Status.SecretTargetsPolicy = tm.Spec.TrustManagerConfig.SecretTargets.Policy tm.Status.DefaultCAPackagePolicy = tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy tm.Status.FilterExpiredCertificatesPolicy = tm.Spec.TrustManagerConfig.FilterExpiredCertificates + tm.Status.FilterNonCACertsPolicy = tm.Spec.TrustManagerConfig.FilterNonCACerts return tm }, wantStatusUpdate: 0, diff --git a/pkg/controller/trustmanager/test_utils.go b/pkg/controller/trustmanager/test_utils.go index e0b96cc9d..33e822070 100644 --- a/pkg/controller/trustmanager/test_utils.go +++ b/pkg/controller/trustmanager/test_utils.go @@ -89,6 +89,11 @@ func (b *trustManagerBuilder) WithFilterExpiredCertificates(policy v1alpha1.Filt return b } +func (b *trustManagerBuilder) WithFilterNonCACerts(policy v1alpha1.FilterNonCACertsPolicy) *trustManagerBuilder { + b.Spec.TrustManagerConfig.FilterNonCACerts = policy + return b +} + func (b *trustManagerBuilder) WithDefaultCAPackage(policy v1alpha1.DefaultCAPackagePolicy) *trustManagerBuilder { b.Spec.TrustManagerConfig.DefaultCAPackage.Policy = policy return b diff --git a/pkg/operator/applyconfigurations/operator/v1alpha1/trustmanagerconfig.go b/pkg/operator/applyconfigurations/operator/v1alpha1/trustmanagerconfig.go index 4ac95edd8..e7b546b31 100644 --- a/pkg/operator/applyconfigurations/operator/v1alpha1/trustmanagerconfig.go +++ b/pkg/operator/applyconfigurations/operator/v1alpha1/trustmanagerconfig.go @@ -31,6 +31,12 @@ type TrustManagerConfigApplyConfiguration struct { // When set to "Enabled", expired certificates are removed from bundles. // When set to "Disabled", expired certificates are included (default behavior). FilterExpiredCertificates *operatorv1alpha1.FilterExpiredCertificatesPolicy `json:"filterExpiredCertificates,omitempty"` + // filterNonCACerts controls whether trust-manager filters out + // non-CA certificates from trust bundles before distributing them. + // When set to "Enabled", only certificates with the X.509 basicConstraints + // CA bit set are included in bundles. + // When set to "Disabled", non-CA certificates are included (default behavior). + FilterNonCACerts *operatorv1alpha1.FilterNonCACertsPolicy `json:"filterNonCACerts,omitempty"` // defaultCAPackage configures the default CA package for trust-manager. // When enabled, the operator will use OpenShift's trusted CA bundle injection mechanism. DefaultCAPackage *DefaultCAPackageConfigApplyConfiguration `json:"defaultCAPackage,omitempty"` @@ -94,6 +100,14 @@ func (b *TrustManagerConfigApplyConfiguration) WithFilterExpiredCertificates(val return b } +// WithFilterNonCACerts sets the FilterNonCACerts field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the FilterNonCACerts field is set to the value of the last call. +func (b *TrustManagerConfigApplyConfiguration) WithFilterNonCACerts(value operatorv1alpha1.FilterNonCACertsPolicy) *TrustManagerConfigApplyConfiguration { + b.FilterNonCACerts = &value + return b +} + // WithDefaultCAPackage sets the DefaultCAPackage field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the DefaultCAPackage field is set to the value of the last call. diff --git a/pkg/operator/applyconfigurations/operator/v1alpha1/trustmanagerstatus.go b/pkg/operator/applyconfigurations/operator/v1alpha1/trustmanagerstatus.go index 05974408a..02e1b567d 100644 --- a/pkg/operator/applyconfigurations/operator/v1alpha1/trustmanagerstatus.go +++ b/pkg/operator/applyconfigurations/operator/v1alpha1/trustmanagerstatus.go @@ -24,6 +24,8 @@ type TrustManagerStatusApplyConfiguration struct { DefaultCAPackagePolicy *operatorv1alpha1.DefaultCAPackagePolicy `json:"defaultCAPackagePolicy,omitempty"` // filterExpiredCertificatesPolicy indicates the current policy for filtering expired certificates. FilterExpiredCertificatesPolicy *operatorv1alpha1.FilterExpiredCertificatesPolicy `json:"filterExpiredCertificatesPolicy,omitempty"` + // filterNonCACertsPolicy indicates the current policy for filtering non-CA certificates. + FilterNonCACertsPolicy *operatorv1alpha1.FilterNonCACertsPolicy `json:"filterNonCACertsPolicy,omitempty"` } // TrustManagerStatusApplyConfiguration constructs a declarative configuration of the TrustManagerStatus type for use with @@ -84,3 +86,11 @@ func (b *TrustManagerStatusApplyConfiguration) WithFilterExpiredCertificatesPoli b.FilterExpiredCertificatesPolicy = &value return b } + +// WithFilterNonCACertsPolicy sets the FilterNonCACertsPolicy field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the FilterNonCACertsPolicy field is set to the value of the last call. +func (b *TrustManagerStatusApplyConfiguration) WithFilterNonCACertsPolicy(value operatorv1alpha1.FilterNonCACertsPolicy) *TrustManagerStatusApplyConfiguration { + b.FilterNonCACertsPolicy = &value + return b +} diff --git a/test/e2e/trustmanager_bundle_test.go b/test/e2e/trustmanager_bundle_test.go index c6dea5840..86981a3f2 100644 --- a/test/e2e/trustmanager_bundle_test.go +++ b/test/e2e/trustmanager_bundle_test.go @@ -43,6 +43,10 @@ // Group 6 — FilterExpiredCertificates enabled: // - ConfigMap source with valid + expired certs → only valid cert in ConfigMap target // - Transition to Disabled → same Bundle re-syncs with both certs in target +// +// Group 7 — FilterNonCACerts enabled: +// - ConfigMap source with CA + leaf certs → only CA cert in ConfigMap target +// - Transition to Disabled → same Bundle re-syncs with both certs in target package e2e import ( @@ -58,8 +62,8 @@ import ( trustapi "github.com/cert-manager/trust-manager/pkg/apis/trust/v1alpha1" configopenshiftv1 "github.com/openshift/api/config/v1" "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" - "github.com/openshift/cert-manager-operator/test/library" testutils "github.com/openshift/cert-manager-operator/pkg/controller/istiocsr" + "github.com/openshift/cert-manager-operator/test/library" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -77,8 +81,8 @@ var _ = Describe("Bundle", Ordered, Label("Platform:Generic", "Feature:TrustMana ctx := context.TODO() var ( - testNS *corev1.Namespace - testCertPEM1, testCertPEM2, expiredCertPEM string + testNS *corev1.Namespace + testCertPEM1, testCertPEM2, expiredCertPEM, leafCertPEM string originalUnsupportedAddonFeatures string originalOperatorLogLevel string @@ -121,6 +125,7 @@ var _ = Describe("Bundle", Ordered, Label("Platform:Generic", "Feature:TrustMana cert.NotAfter = time.Now().Add(-24 * time.Hour) } expiredCertPEM = testutils.GenerateCertificate("e2e-expired-ca", []string{"cert-manager-operator-e2e"}, expiredCATweak) + leafCertPEM = testutils.GenerateCertificate("e2e-leaf", []string{"cert-manager-operator-e2e"}, func(*x509.Certificate) {}) By("creating test namespace for target verification") testNS = createNamespaceWithCleanup(ctx, "bundle-e2e-", map[string]string{bundleTestNamespaceLabel: "true"}) @@ -1073,4 +1078,75 @@ var _ = Describe("Bundle", Ordered, Label("Platform:Generic", "Feature:TrustMana verifyBundleSynced(ctx, filterBundleName) }) }) + + // ===== Group 7: FilterNonCACerts ===== + Context("with FilterNonCACerts enabled", Ordered, func() { + var ( + filterBundleName string + sourceCMName string + ) + + BeforeAll(func() { + createTrustManager(ctx, newTrustManagerCR(). + WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicyEnabled)) + + sourceCMName = "filter-nca-src-cm-" + randomStr(5) + filterBundleName = "bundle-filter-non-ca-" + randomStr(5) + combinedPEM := testCertPEM1 + leafCertPEM + + By("creating source ConfigMap with CA + leaf certs in trust namespace") + createSourceConfigMap(ctx, trustManagerNamespace, sourceCMName, bundleSourceKey, combinedPEM) + + bundle := newBundle(filterBundleName). + WithConfigMapSource(sourceCMName, bundleSourceKey). + WithConfigMapTarget(bundleTargetKey). + Build() + + createBundleWithCleanup(ctx, bundle) + }) + AfterAll(func() { deleteTrustManager(ctx) }) + + It("should exclude non-CA certificates from ConfigMap target when using ConfigMap source", func() { + By("verifying target contains the CA certificate") + err := waitForConfigMapTarget(ctx, bundleClient, filterBundleName, testNS.Name, bundleTargetKey, testCertPEM1, highTimeout) + Expect(err).ShouldNot(HaveOccurred()) + + By("verifying target does NOT contain the leaf certificate") + Eventually(func(g Gomega) { + cm, err := k8sClientSet.CoreV1().ConfigMaps(testNS.Name).Get(ctx, filterBundleName, metav1.GetOptions{}) + g.Expect(err).ShouldNot(HaveOccurred()) + data := cm.Data[bundleTargetKey] + g.Expect(strings.Contains(data, strings.TrimSpace(testCertPEM1))).Should(BeTrue(), "should contain CA cert") + g.Expect(strings.Contains(data, strings.TrimSpace(leafCertPEM))).Should(BeFalse(), "should not contain leaf cert") + }, highTimeout, fastPollInterval).Should(Succeed()) + + verifyBundleSynced(ctx, filterBundleName) + }) + + It("should re-sync same Bundle with leaf certs included after disabling filter", func() { + By("disabling filterNonCACerts on TrustManager CR") + Eventually(func() error { + tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + return err + } + tm.Spec.TrustManagerConfig.FilterNonCACerts = v1alpha1.FilterNonCACertsPolicyDisabled + _, err = trustManagerClient().Update(ctx, tm, metav1.UpdateOptions{}) + return err + }, lowTimeout, fastPollInterval).Should(Succeed()) + + waitForTrustManagerReady(ctx) + + By("verifying the same Bundle's target now includes the leaf certificate") + Eventually(func(g Gomega) { + cm, err := k8sClientSet.CoreV1().ConfigMaps(testNS.Name).Get(ctx, filterBundleName, metav1.GetOptions{}) + g.Expect(err).ShouldNot(HaveOccurred()) + data := cm.Data[bundleTargetKey] + g.Expect(strings.Contains(data, strings.TrimSpace(testCertPEM1))).Should(BeTrue(), "should contain CA cert") + g.Expect(strings.Contains(data, strings.TrimSpace(leafCertPEM))).Should(BeTrue(), "should contain leaf cert after disabling filter") + }, highTimeout, fastPollInterval).Should(Succeed()) + + verifyBundleSynced(ctx, filterBundleName) + }) + }) }) diff --git a/test/e2e/trustmanager_helpers_test.go b/test/e2e/trustmanager_helpers_test.go index 6a542594c..a84679f71 100644 --- a/test/e2e/trustmanager_helpers_test.go +++ b/test/e2e/trustmanager_helpers_test.go @@ -14,8 +14,8 @@ import ( . "github.com/onsi/gomega" "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" - "github.com/openshift/cert-manager-operator/test/library" operatorclientv1alpha1 "github.com/openshift/cert-manager-operator/pkg/operator/clientset/versioned/typed/operator/v1alpha1" + "github.com/openshift/cert-manager-operator/test/library" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -97,6 +97,11 @@ func (b *trustManagerCRBuilder) WithFilterExpiredCertificates(policy v1alpha1.Fi return b } +func (b *trustManagerCRBuilder) WithFilterNonCACerts(policy v1alpha1.FilterNonCACertsPolicy) *trustManagerCRBuilder { + b.tm.Spec.TrustManagerConfig.FilterNonCACerts = policy + return b +} + func (b *trustManagerCRBuilder) Build() *v1alpha1.TrustManager { return b.tm } diff --git a/test/e2e/trustmanager_test.go b/test/e2e/trustmanager_test.go index 0b7026e7b..eb51a28e7 100644 --- a/test/e2e/trustmanager_test.go +++ b/test/e2e/trustmanager_test.go @@ -616,6 +616,31 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru }, lowTimeout, fastPollInterval).Should(Succeed()) }) + It("should add filter-non-ca-certs arg when filterNonCACerts is Enabled", func() { + createTrustManager(ctx, newTrustManagerCR(). + WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicyEnabled)) + + By("verifying deployment args contain --filter-non-ca-certs=true") + Eventually(func(g Gomega) { + dep, err := clientset.AppsV1().Deployments(trustManagerNamespace).Get(ctx, trustManagerDeploymentName, metav1.GetOptions{}) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(dep.Spec.Template.Spec.Containers).ShouldNot(BeEmpty()) + g.Expect(dep.Spec.Template.Spec.Containers[0].Args).Should(ContainElement("--filter-non-ca-certs=true")) + }, lowTimeout, fastPollInterval).Should(Succeed()) + }) + + It("should not have filter-non-ca-certs arg when filterNonCACerts is Disabled", func() { + createTrustManager(ctx, newTrustManagerCR()) + + By("verifying deployment args do not contain --filter-non-ca-certs=true") + Eventually(func(g Gomega) { + dep, err := clientset.AppsV1().Deployments(trustManagerNamespace).Get(ctx, trustManagerDeploymentName, metav1.GetOptions{}) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(dep.Spec.Template.Spec.Containers).ShouldNot(BeEmpty()) + g.Expect(dep.Spec.Template.Spec.Containers[0].Args).ShouldNot(ContainElement("--filter-non-ca-certs=true")) + }, lowTimeout, fastPollInterval).Should(Succeed()) + }) + }) // ------------------------------------------------------------------------- @@ -1280,6 +1305,18 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru g.Expect(tm.Status.FilterExpiredCertificatesPolicy).Should(Equal(v1alpha1.FilterExpiredCertificatesPolicyEnabled)) }, lowTimeout, fastPollInterval).Should(Succeed()) }) + + It("should report filterNonCACerts policy in status", func() { + createTrustManager(ctx, newTrustManagerCR(). + WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicyEnabled)) + + By("verifying status reports Enabled policy") + Eventually(func(g Gomega) { + tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(tm.Status.FilterNonCACertsPolicy).Should(Equal(v1alpha1.FilterNonCACertsPolicyEnabled)) + }, lowTimeout, fastPollInterval).Should(Succeed()) + }) }) // ------------------------------------------------------------------------- From 4ea3f276b64fa24d09ad1550dc881b4d501e8b7b Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Thu, 10 Sep 2026 17:13:53 +0530 Subject: [PATCH 2/7] CM-1367: Stop echoing TrustManager spec into status. Keep observed image only. Reuse shared Mode Enabled/Disabled for the filter and default-CA policy types instead of per-field constants. --- api/operator/v1alpha1/trustmanager_types.go | 49 ++-------- .../operator.openshift.io_trustmanagers.yaml | 32 ------ .../operator.openshift.io_trustmanagers.yaml | 32 ------ .../trustmanager/configmaps_test.go | 18 ++-- pkg/controller/trustmanager/deployments.go | 4 +- .../trustmanager/deployments_test.go | 18 ++-- .../trustmanager/install_trustmanager.go | 25 ----- .../trustmanager/install_trustmanager_test.go | 40 +++----- pkg/controller/trustmanager/utils.go | 2 +- .../operator/v1alpha1/trustmanagerstatus.go | 51 ---------- test/e2e/multiple_operands_test.go | 8 +- test/e2e/trustmanager_bundle_test.go | 12 +-- test/e2e/trustmanager_test.go | 98 +------------------ 13 files changed, 52 insertions(+), 337 deletions(-) diff --git a/api/operator/v1alpha1/trustmanager_types.go b/api/operator/v1alpha1/trustmanager_types.go index ebf9332ee..d8ffd6b42 100644 --- a/api/operator/v1alpha1/trustmanager_types.go +++ b/api/operator/v1alpha1/trustmanager_types.go @@ -226,26 +226,14 @@ type TrustManagerControllerConfig struct { Annotations map[string]string `json:"annotations,omitempty"` } -// FilterExpiredCertificatesPolicy defines the policy for filtering expired certificates. +// FilterExpiredCertificatesPolicy controls whether expired certificates are filtered from bundles. +// Allowed values are Enabled and Disabled. type FilterExpiredCertificatesPolicy string -const ( - // FilterExpiredCertificatesPolicyEnabled filters out expired certificates from bundles. - FilterExpiredCertificatesPolicyEnabled FilterExpiredCertificatesPolicy = "Enabled" - // FilterExpiredCertificatesPolicyDisabled includes expired certificates in bundles. - FilterExpiredCertificatesPolicyDisabled FilterExpiredCertificatesPolicy = "Disabled" -) - -// FilterNonCACertsPolicy defines the policy for filtering non-CA certificates. +// FilterNonCACertsPolicy controls whether non-CA certificates are filtered from bundles. +// Allowed values are Enabled and Disabled. type FilterNonCACertsPolicy string -const ( - // FilterNonCACertsPolicyEnabled filters out non-CA certificates from bundles. - FilterNonCACertsPolicyEnabled FilterNonCACertsPolicy = "Enabled" - // FilterNonCACertsPolicyDisabled includes non-CA certificates in bundles. - FilterNonCACertsPolicyDisabled FilterNonCACertsPolicy = "Disabled" -) - // SecretTargetsPolicy defines the policy for writing trust bundles to Secrets. type SecretTargetsPolicy string @@ -257,16 +245,10 @@ const ( SecretTargetsPolicyCustom SecretTargetsPolicy = "Custom" ) -// DefaultCAPackagePolicy defines the policy for the default CA package feature. +// DefaultCAPackagePolicy controls whether the default CA package feature is enabled. +// Allowed values are Enabled and Disabled. type DefaultCAPackagePolicy string -const ( - // DefaultCAPackagePolicyEnabled enables the default CA package feature. - DefaultCAPackagePolicyEnabled DefaultCAPackagePolicy = "Enabled" - // DefaultCAPackagePolicyDisabled disables the default CA package feature. - DefaultCAPackagePolicyDisabled DefaultCAPackagePolicy = "Disabled" -) - // TrustManagerStatus defines the observed state of TrustManager. type TrustManagerStatus struct { // conditions holds information about the current state of the trust-manager deployment. @@ -274,23 +256,4 @@ type TrustManagerStatus struct { // trustManagerImage is the container image (name:tag) used for trust-manager. TrustManagerImage string `json:"trustManagerImage,omitempty"` - - // trustNamespace is the namespace where trust-manager looks for trust sources. - TrustNamespace string `json:"trustNamespace,omitempty"` - - // secretTargetsPolicy indicates the current secret targets policy. - // +kubebuilder:validation:Enum:=Disabled;Custom - SecretTargetsPolicy SecretTargetsPolicy `json:"secretTargetsPolicy,omitempty"` - - // defaultCAPackagePolicy indicates the current default CA package policy. - // +kubebuilder:validation:Enum:=Enabled;Disabled - DefaultCAPackagePolicy DefaultCAPackagePolicy `json:"defaultCAPackagePolicy,omitempty"` - - // filterExpiredCertificatesPolicy indicates the current policy for filtering expired certificates. - // +kubebuilder:validation:Enum:=Enabled;Disabled - FilterExpiredCertificatesPolicy FilterExpiredCertificatesPolicy `json:"filterExpiredCertificatesPolicy,omitempty"` - - // filterNonCACertsPolicy indicates the current policy for filtering non-CA certificates. - // +kubebuilder:validation:Enum:=Enabled;Disabled - FilterNonCACertsPolicy FilterNonCACertsPolicy `json:"filterNonCACertsPolicy,omitempty"` } diff --git a/bundle/manifests/operator.openshift.io_trustmanagers.yaml b/bundle/manifests/operator.openshift.io_trustmanagers.yaml index 3ec28a0e4..680f0c22c 100644 --- a/bundle/manifests/operator.openshift.io_trustmanagers.yaml +++ b/bundle/manifests/operator.openshift.io_trustmanagers.yaml @@ -1302,42 +1302,10 @@ spec: x-kubernetes-list-map-keys: - type x-kubernetes-list-type: map - defaultCAPackagePolicy: - description: defaultCAPackagePolicy indicates the current default - CA package policy. - enum: - - Enabled - - Disabled - type: string - filterExpiredCertificatesPolicy: - description: filterExpiredCertificatesPolicy indicates the current - policy for filtering expired certificates. - enum: - - Enabled - - Disabled - type: string - filterNonCACertsPolicy: - description: filterNonCACertsPolicy indicates the current policy for - filtering non-CA certificates. - enum: - - Enabled - - Disabled - type: string - secretTargetsPolicy: - description: secretTargetsPolicy indicates the current secret targets - policy. - enum: - - Disabled - - Custom - type: string trustManagerImage: description: trustManagerImage is the container image (name:tag) used for trust-manager. type: string - trustNamespace: - description: trustNamespace is the namespace where trust-manager looks - for trust sources. - type: string type: object required: - metadata diff --git a/config/crd/bases/operator.openshift.io_trustmanagers.yaml b/config/crd/bases/operator.openshift.io_trustmanagers.yaml index 94c96d274..b341d9004 100644 --- a/config/crd/bases/operator.openshift.io_trustmanagers.yaml +++ b/config/crd/bases/operator.openshift.io_trustmanagers.yaml @@ -1302,42 +1302,10 @@ spec: x-kubernetes-list-map-keys: - type x-kubernetes-list-type: map - defaultCAPackagePolicy: - description: defaultCAPackagePolicy indicates the current default - CA package policy. - enum: - - Enabled - - Disabled - type: string - filterExpiredCertificatesPolicy: - description: filterExpiredCertificatesPolicy indicates the current - policy for filtering expired certificates. - enum: - - Enabled - - Disabled - type: string - filterNonCACertsPolicy: - description: filterNonCACertsPolicy indicates the current policy for - filtering non-CA certificates. - enum: - - Enabled - - Disabled - type: string - secretTargetsPolicy: - description: secretTargetsPolicy indicates the current secret targets - policy. - enum: - - Disabled - - Custom - type: string trustManagerImage: description: trustManagerImage is the container image (name:tag) used for trust-manager. type: string - trustNamespace: - description: trustNamespace is the namespace where trust-manager looks - for trust sources. - type: string type: object required: - metadata diff --git a/pkg/controller/trustmanager/configmaps_test.go b/pkg/controller/trustmanager/configmaps_test.go index 4629949b0..1a119017c 100644 --- a/pkg/controller/trustmanager/configmaps_test.go +++ b/pkg/controller/trustmanager/configmaps_test.go @@ -159,7 +159,7 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { }{ { name: "skips when policy is Disabled", - tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyDisabled), + tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Disabled)), preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { }, wantExistsCount: 0, @@ -175,7 +175,7 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { }, { name: "returns error when injection ConfigMap is not found", - tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled), + tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)), preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { return errTestClient @@ -185,7 +185,7 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { }, { name: "returns error when CA bundle key is missing", - tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled), + tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)), preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { cm := obj.(*corev1.ConfigMap) @@ -198,7 +198,7 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { }, { name: "returns error when CA bundle is empty", - tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled), + tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)), preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { cm := obj.(*corev1.ConfigMap) @@ -211,7 +211,7 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { }, { name: "creates ConfigMap and returns hash when bundle is available", - tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled), + tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)), preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { cm := obj.(*corev1.ConfigMap) @@ -229,7 +229,7 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { }, { name: "skips patch when existing ConfigMap matches desired", - tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled), + tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)), preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { cm := obj.(*corev1.ConfigMap) @@ -251,7 +251,7 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { }, { name: "patches when existing ConfigMap data differs", - tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled), + tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)), preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { cm := obj.(*corev1.ConfigMap) @@ -271,7 +271,7 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { }, { name: "propagates Exists error", - tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled), + tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)), preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { cm := obj.(*corev1.ConfigMap) @@ -288,7 +288,7 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { }, { name: "propagates Patch error", - tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled), + tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)), preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { cm := obj.(*corev1.ConfigMap) diff --git a/pkg/controller/trustmanager/deployments.go b/pkg/controller/trustmanager/deployments.go index 841326c80..1156689ad 100644 --- a/pkg/controller/trustmanager/deployments.go +++ b/pkg/controller/trustmanager/deployments.go @@ -143,11 +143,11 @@ func updateDeploymentArgs(deployment *appsv1.Deployment, trustManager *v1alpha1. args = append(args, "--secret-targets-enabled=true") } - if config.FilterExpiredCertificates == v1alpha1.FilterExpiredCertificatesPolicyEnabled { + if config.FilterExpiredCertificates == v1alpha1.FilterExpiredCertificatesPolicy(v1alpha1.Enabled) { args = append(args, "--filter-expired-certificates=true") } - if config.FilterNonCACerts == v1alpha1.FilterNonCACertsPolicyEnabled { + if config.FilterNonCACerts == v1alpha1.FilterNonCACertsPolicy(v1alpha1.Enabled) { args = append(args, "--filter-non-ca-certs=true") } diff --git a/pkg/controller/trustmanager/deployments_test.go b/pkg/controller/trustmanager/deployments_test.go index 3ffe761cd..cb8028a2f 100644 --- a/pkg/controller/trustmanager/deployments_test.go +++ b/pkg/controller/trustmanager/deployments_test.go @@ -158,8 +158,8 @@ func TestDeploymentContainerArgs(t *testing.T) { WithLogLevel(5). WithLogFormat("json"). WithTrustNamespace("custom-ns"). - WithFilterExpiredCertificates(v1alpha1.FilterExpiredCertificatesPolicyEnabled). - WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicyEnabled), + WithFilterExpiredCertificates(v1alpha1.FilterExpiredCertificatesPolicy(v1alpha1.Enabled)). + WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicy(v1alpha1.Enabled)), expectedArgs: []string{ "--log-level=5", "--log-format=json", @@ -202,7 +202,7 @@ func TestDeploymentContainerArgs(t *testing.T) { }, { name: "includes default-package-location when defaultCAPackage is Enabled", - tmBuilder: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled), + tmBuilder: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)), expectedArgs: []string{ fmt.Sprintf("--default-package-location=%s", defaultCAPackageLocation), }, @@ -215,14 +215,14 @@ func TestDeploymentContainerArgs(t *testing.T) { }, { name: "includes filter-non-ca-certs when filterNonCACerts is Enabled", - tmBuilder: testTrustManager().WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicyEnabled), + tmBuilder: testTrustManager().WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicy(v1alpha1.Enabled)), expectedArgs: []string{ "--filter-non-ca-certs=true", }, }, { name: "excludes filter-non-ca-certs when filterNonCACerts is Disabled", - tmBuilder: testTrustManager().WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicyDisabled), + tmBuilder: testTrustManager().WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicy(v1alpha1.Disabled)), notExpectedArgs: []string{ "--filter-non-ca-certs=true", }, @@ -265,7 +265,7 @@ func TestDeploymentDefaultCAPackage(t *testing.T) { t.Run("adds arg, volume, mount, and hash annotation when enabled", func(t *testing.T) { r := testReconciler(t) - tm := testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled).Build() + tm := testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)).Build() dep, err := r.getDeploymentObject(tm, testResourceLabels(), testResourceAnnotations(), "abc123hash") if err != nil { t.Fatalf("unexpected error: %v", err) @@ -517,7 +517,7 @@ func TestDeploymentReconciliation(t *testing.T) { setImage: true, preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { - tm := testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled).Build() + tm := testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)).Build() dep, err := r.getDeploymentObject(tm, testResourceLabels(), testResourceAnnotations(), "abc123hash") if err != nil { t.Fatalf("unexpected error: %v", err) @@ -531,12 +531,12 @@ func TestDeploymentReconciliation(t *testing.T) { }, { name: "apply when existing has pod template annotation drift", - tmBuilder: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled), + tmBuilder: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)), caBundleHash: "abc123hash", setImage: true, preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { - tm := testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled).Build() + tm := testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)).Build() dep, err := r.getDeploymentObject(tm, testResourceLabels(), testResourceAnnotations(), "abc123hash") if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/pkg/controller/trustmanager/install_trustmanager.go b/pkg/controller/trustmanager/install_trustmanager.go index 32294928e..0c4415314 100644 --- a/pkg/controller/trustmanager/install_trustmanager.go +++ b/pkg/controller/trustmanager/install_trustmanager.go @@ -92,31 +92,6 @@ func (r *Reconciler) updateStatusObservedState(trustManager *v1alpha1.TrustManag changed = true } - if ns := getTrustNamespace(trustManager); trustManager.Status.TrustNamespace != ns { - trustManager.Status.TrustNamespace = ns - changed = true - } - - if policy := trustManager.Spec.TrustManagerConfig.SecretTargets.Policy; trustManager.Status.SecretTargetsPolicy != policy { - trustManager.Status.SecretTargetsPolicy = policy - changed = true - } - - if policy := trustManager.Spec.TrustManagerConfig.DefaultCAPackage.Policy; trustManager.Status.DefaultCAPackagePolicy != policy { - trustManager.Status.DefaultCAPackagePolicy = policy - changed = true - } - - if policy := trustManager.Spec.TrustManagerConfig.FilterExpiredCertificates; trustManager.Status.FilterExpiredCertificatesPolicy != policy { - trustManager.Status.FilterExpiredCertificatesPolicy = policy - changed = true - } - - if policy := trustManager.Spec.TrustManagerConfig.FilterNonCACerts; trustManager.Status.FilterNonCACertsPolicy != policy { - trustManager.Status.FilterNonCACertsPolicy = policy - changed = true - } - if !changed { return nil } diff --git a/pkg/controller/trustmanager/install_trustmanager_test.go b/pkg/controller/trustmanager/install_trustmanager_test.go index b5382f9d3..d419f9b65 100644 --- a/pkg/controller/trustmanager/install_trustmanager_test.go +++ b/pkg/controller/trustmanager/install_trustmanager_test.go @@ -14,14 +14,8 @@ import ( func TestUpdateStatusObservedState(t *testing.T) { t.Setenv(trustManagerImageNameEnvVarName, testImage) - // Observed status after sync from testTrustManager() defaults (empty status fields + default spec). - wantStatusSyncedFromDefaultSpec := v1alpha1.TrustManagerStatus{ - TrustManagerImage: testImage, - TrustNamespace: defaultTrustNamespace, - SecretTargetsPolicy: "", - DefaultCAPackagePolicy: "", - FilterExpiredCertificatesPolicy: "", - FilterNonCACertsPolicy: "", + wantImageStatus := v1alpha1.TrustManagerStatus{ + TrustManagerImage: testImage, } tests := []struct { @@ -31,48 +25,36 @@ func TestUpdateStatusObservedState(t *testing.T) { wantStatus v1alpha1.TrustManagerStatus }{ { - name: "updates all observed fields when status is empty", + name: "sets trust-manager image when status is empty", trustManager: func() *v1alpha1.TrustManager { return testTrustManager().Build() }, wantStatusUpdate: 1, - wantStatus: wantStatusSyncedFromDefaultSpec, + wantStatus: wantImageStatus, }, { - name: "updates all observed fields for custom spec", + name: "does not echo spec fields into status", trustManager: func() *v1alpha1.TrustManager { return testTrustManager(). WithTrustNamespace("custom-trust-ns"). WithSecretTargets(v1alpha1.SecretTargetsPolicyCustom, []string{"allowed-secret"}). - WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled). - WithFilterExpiredCertificates(v1alpha1.FilterExpiredCertificatesPolicyEnabled). - WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicyEnabled). + WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)). + WithFilterExpiredCertificates(v1alpha1.FilterExpiredCertificatesPolicy(v1alpha1.Enabled)). + WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicy(v1alpha1.Enabled)). Build() }, wantStatusUpdate: 1, - wantStatus: v1alpha1.TrustManagerStatus{ - TrustManagerImage: testImage, - TrustNamespace: "custom-trust-ns", - SecretTargetsPolicy: v1alpha1.SecretTargetsPolicyCustom, - DefaultCAPackagePolicy: v1alpha1.DefaultCAPackagePolicyEnabled, - FilterExpiredCertificatesPolicy: v1alpha1.FilterExpiredCertificatesPolicyEnabled, - FilterNonCACertsPolicy: v1alpha1.FilterNonCACertsPolicyEnabled, - }, + wantStatus: wantImageStatus, }, { - name: "no-op when observed state already matches spec and env", + name: "no-op when image already matches env", trustManager: func() *v1alpha1.TrustManager { tm := testTrustManager().Build() tm.Status.TrustManagerImage = testImage - tm.Status.TrustNamespace = defaultTrustNamespace - tm.Status.SecretTargetsPolicy = tm.Spec.TrustManagerConfig.SecretTargets.Policy - tm.Status.DefaultCAPackagePolicy = tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy - tm.Status.FilterExpiredCertificatesPolicy = tm.Spec.TrustManagerConfig.FilterExpiredCertificates - tm.Status.FilterNonCACertsPolicy = tm.Spec.TrustManagerConfig.FilterNonCACerts return tm }, wantStatusUpdate: 0, - wantStatus: wantStatusSyncedFromDefaultSpec, + wantStatus: wantImageStatus, }, } diff --git a/pkg/controller/trustmanager/utils.go b/pkg/controller/trustmanager/utils.go index 3e4507959..1e921c7fd 100644 --- a/pkg/controller/trustmanager/utils.go +++ b/pkg/controller/trustmanager/utils.go @@ -146,7 +146,7 @@ func secretTargetsEnabled(config v1alpha1.SecretTargetsConfig) bool { // defaultCAPackageEnabled returns true when the defaultCAPackage policy is Enabled. func defaultCAPackageEnabled(config v1alpha1.DefaultCAPackageConfig) bool { - return config.Policy == v1alpha1.DefaultCAPackagePolicyEnabled + return config.Policy == v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled) } // getTrustNamespace returns the trust namespace from the TrustManager config. diff --git a/pkg/operator/applyconfigurations/operator/v1alpha1/trustmanagerstatus.go b/pkg/operator/applyconfigurations/operator/v1alpha1/trustmanagerstatus.go index 02e1b567d..54e24f8ab 100644 --- a/pkg/operator/applyconfigurations/operator/v1alpha1/trustmanagerstatus.go +++ b/pkg/operator/applyconfigurations/operator/v1alpha1/trustmanagerstatus.go @@ -3,7 +3,6 @@ package v1alpha1 import ( - operatorv1alpha1 "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" v1 "k8s.io/client-go/applyconfigurations/meta/v1" ) @@ -16,16 +15,6 @@ type TrustManagerStatusApplyConfiguration struct { ConditionalStatusApplyConfiguration `json:",omitempty,inline"` // trustManagerImage is the container image (name:tag) used for trust-manager. TrustManagerImage *string `json:"trustManagerImage,omitempty"` - // trustNamespace is the namespace where trust-manager looks for trust sources. - TrustNamespace *string `json:"trustNamespace,omitempty"` - // secretTargetsPolicy indicates the current secret targets policy. - SecretTargetsPolicy *operatorv1alpha1.SecretTargetsPolicy `json:"secretTargetsPolicy,omitempty"` - // defaultCAPackagePolicy indicates the current default CA package policy. - DefaultCAPackagePolicy *operatorv1alpha1.DefaultCAPackagePolicy `json:"defaultCAPackagePolicy,omitempty"` - // filterExpiredCertificatesPolicy indicates the current policy for filtering expired certificates. - FilterExpiredCertificatesPolicy *operatorv1alpha1.FilterExpiredCertificatesPolicy `json:"filterExpiredCertificatesPolicy,omitempty"` - // filterNonCACertsPolicy indicates the current policy for filtering non-CA certificates. - FilterNonCACertsPolicy *operatorv1alpha1.FilterNonCACertsPolicy `json:"filterNonCACertsPolicy,omitempty"` } // TrustManagerStatusApplyConfiguration constructs a declarative configuration of the TrustManagerStatus type for use with @@ -54,43 +43,3 @@ func (b *TrustManagerStatusApplyConfiguration) WithTrustManagerImage(value strin b.TrustManagerImage = &value return b } - -// WithTrustNamespace sets the TrustNamespace field in the declarative configuration to the given value -// and returns the receiver, so that objects can be built by chaining "With" function invocations. -// If called multiple times, the TrustNamespace field is set to the value of the last call. -func (b *TrustManagerStatusApplyConfiguration) WithTrustNamespace(value string) *TrustManagerStatusApplyConfiguration { - b.TrustNamespace = &value - return b -} - -// WithSecretTargetsPolicy sets the SecretTargetsPolicy field in the declarative configuration to the given value -// and returns the receiver, so that objects can be built by chaining "With" function invocations. -// If called multiple times, the SecretTargetsPolicy field is set to the value of the last call. -func (b *TrustManagerStatusApplyConfiguration) WithSecretTargetsPolicy(value operatorv1alpha1.SecretTargetsPolicy) *TrustManagerStatusApplyConfiguration { - b.SecretTargetsPolicy = &value - return b -} - -// WithDefaultCAPackagePolicy sets the DefaultCAPackagePolicy field in the declarative configuration to the given value -// and returns the receiver, so that objects can be built by chaining "With" function invocations. -// If called multiple times, the DefaultCAPackagePolicy field is set to the value of the last call. -func (b *TrustManagerStatusApplyConfiguration) WithDefaultCAPackagePolicy(value operatorv1alpha1.DefaultCAPackagePolicy) *TrustManagerStatusApplyConfiguration { - b.DefaultCAPackagePolicy = &value - return b -} - -// WithFilterExpiredCertificatesPolicy sets the FilterExpiredCertificatesPolicy field in the declarative configuration to the given value -// and returns the receiver, so that objects can be built by chaining "With" function invocations. -// If called multiple times, the FilterExpiredCertificatesPolicy field is set to the value of the last call. -func (b *TrustManagerStatusApplyConfiguration) WithFilterExpiredCertificatesPolicy(value operatorv1alpha1.FilterExpiredCertificatesPolicy) *TrustManagerStatusApplyConfiguration { - b.FilterExpiredCertificatesPolicy = &value - return b -} - -// WithFilterNonCACertsPolicy sets the FilterNonCACertsPolicy field in the declarative configuration to the given value -// and returns the receiver, so that objects can be built by chaining "With" function invocations. -// If called multiple times, the FilterNonCACertsPolicy field is set to the value of the last call. -func (b *TrustManagerStatusApplyConfiguration) WithFilterNonCACertsPolicy(value operatorv1alpha1.FilterNonCACertsPolicy) *TrustManagerStatusApplyConfiguration { - b.FilterNonCACertsPolicy = &value - return b -} diff --git a/test/e2e/multiple_operands_test.go b/test/e2e/multiple_operands_test.go index a12b8aaa5..aeb62e74a 100644 --- a/test/e2e/multiple_operands_test.go +++ b/test/e2e/multiple_operands_test.go @@ -254,8 +254,8 @@ func multiOperandTrustManagerCR() *trustManagerCRBuilder { return newTrustManagerCR(). WithLabels(map[string]string{"env": "trustmanager-test"}). WithAnnotations(map[string]string{"trustmanager.operator.openshift.io/cluster": "trustmanager-test"}). - WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled). - WithFilterExpiredCertificates(v1alpha1.FilterExpiredCertificatesPolicyEnabled). + WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)). + WithFilterExpiredCertificates(v1alpha1.FilterExpiredCertificatesPolicy(v1alpha1.Enabled)). WithSecretTargets(v1alpha1.SecretTargetsPolicyCustom, []string{"ca-bundle-secret", multiOperandBundleName}). WithTrustNamespace(trustManagerNamespace) } @@ -554,9 +554,9 @@ func assertTrustManagerCRConfigPropagation(ctx context.Context, clientset *kuber tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) Expect(err).NotTo(HaveOccurred()) - Expect(tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy).To(Equal(v1alpha1.DefaultCAPackagePolicyEnabled)) + Expect(tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy).To(Equal(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled))) Expect(tm.Spec.TrustManagerConfig.SecretTargets.Policy).To(Equal(v1alpha1.SecretTargetsPolicyCustom)) - Expect(tm.Spec.TrustManagerConfig.FilterExpiredCertificates).To(Equal(v1alpha1.FilterExpiredCertificatesPolicyEnabled)) + Expect(tm.Spec.TrustManagerConfig.FilterExpiredCertificates).To(Equal(v1alpha1.FilterExpiredCertificatesPolicy(v1alpha1.Enabled))) } func runMultiOperandBundleSecretTargetTest(ctx context.Context, sourcePEM string) { diff --git a/test/e2e/trustmanager_bundle_test.go b/test/e2e/trustmanager_bundle_test.go index 86981a3f2..67655a8d1 100644 --- a/test/e2e/trustmanager_bundle_test.go +++ b/test/e2e/trustmanager_bundle_test.go @@ -560,7 +560,7 @@ var _ = Describe("Bundle", Ordered, Label("Platform:Generic", "Feature:TrustMana // ===== Group 3: DefaultCAPackage enabled ===== Context("with DefaultCAPackage enabled", Ordered, func() { BeforeAll(func() { - createTrustManager(ctx, newTrustManagerCR().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled)) + createTrustManager(ctx, newTrustManagerCR().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled))) By("waiting for default CA package ConfigMap to be created") err := pollTillConfigMapAvailable(ctx, k8sClientSet, trustManagerNamespace, defaultCAPackageConfigMapName) @@ -871,7 +871,7 @@ var _ = Describe("Bundle", Ordered, Label("Platform:Generic", "Feature:TrustMana BeforeAll(func() { createTrustManager(ctx, newTrustManagerCR(). WithSecretTargets(v1alpha1.SecretTargetsPolicyCustom, []string{bundleCombined}). - WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled)) + WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled))) By("waiting for default CA package ConfigMap to be created") err := pollTillConfigMapAvailable(ctx, k8sClientSet, trustManagerNamespace, defaultCAPackageConfigMapName) @@ -1017,7 +1017,7 @@ var _ = Describe("Bundle", Ordered, Label("Platform:Generic", "Feature:TrustMana BeforeAll(func() { createTrustManager(ctx, newTrustManagerCR(). - WithFilterExpiredCertificates(v1alpha1.FilterExpiredCertificatesPolicyEnabled)) + WithFilterExpiredCertificates(v1alpha1.FilterExpiredCertificatesPolicy(v1alpha1.Enabled))) sourceCMName = "filter-src-cm-" + randomStr(5) filterBundleName = "bundle-filter-expired-" + randomStr(5) @@ -1059,7 +1059,7 @@ var _ = Describe("Bundle", Ordered, Label("Platform:Generic", "Feature:TrustMana if err != nil { return err } - tm.Spec.TrustManagerConfig.FilterExpiredCertificates = v1alpha1.FilterExpiredCertificatesPolicyDisabled + tm.Spec.TrustManagerConfig.FilterExpiredCertificates = v1alpha1.FilterExpiredCertificatesPolicy(v1alpha1.Disabled) _, err = trustManagerClient().Update(ctx, tm, metav1.UpdateOptions{}) return err }, lowTimeout, fastPollInterval).Should(Succeed()) @@ -1088,7 +1088,7 @@ var _ = Describe("Bundle", Ordered, Label("Platform:Generic", "Feature:TrustMana BeforeAll(func() { createTrustManager(ctx, newTrustManagerCR(). - WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicyEnabled)) + WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicy(v1alpha1.Enabled))) sourceCMName = "filter-nca-src-cm-" + randomStr(5) filterBundleName = "bundle-filter-non-ca-" + randomStr(5) @@ -1130,7 +1130,7 @@ var _ = Describe("Bundle", Ordered, Label("Platform:Generic", "Feature:TrustMana if err != nil { return err } - tm.Spec.TrustManagerConfig.FilterNonCACerts = v1alpha1.FilterNonCACertsPolicyDisabled + tm.Spec.TrustManagerConfig.FilterNonCACerts = v1alpha1.FilterNonCACertsPolicy(v1alpha1.Disabled) _, err = trustManagerClient().Update(ctx, tm, metav1.UpdateOptions{}) return err }, lowTimeout, fastPollInterval).Should(Succeed()) diff --git a/test/e2e/trustmanager_test.go b/test/e2e/trustmanager_test.go index eb51a28e7..b8feba608 100644 --- a/test/e2e/trustmanager_test.go +++ b/test/e2e/trustmanager_test.go @@ -593,7 +593,7 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru It("should add filter-expired-certificates arg when filterExpiredCertificates is Enabled", func() { createTrustManager(ctx, newTrustManagerCR(). - WithFilterExpiredCertificates(v1alpha1.FilterExpiredCertificatesPolicyEnabled)) + WithFilterExpiredCertificates(v1alpha1.FilterExpiredCertificatesPolicy(v1alpha1.Enabled))) By("verifying deployment args contain --filter-expired-certificates=true") Eventually(func(g Gomega) { @@ -618,7 +618,7 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru It("should add filter-non-ca-certs arg when filterNonCACerts is Enabled", func() { createTrustManager(ctx, newTrustManagerCR(). - WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicyEnabled)) + WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicy(v1alpha1.Enabled))) By("verifying deployment args contain --filter-non-ca-certs=true") Eventually(func(g Gomega) { @@ -693,7 +693,7 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru if err != nil { return err } - tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy = v1alpha1.DefaultCAPackagePolicyEnabled + tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy = v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled) _, err = trustManagerClient().Update(ctx, tm, metav1.UpdateOptions{}) return err }, lowTimeout, fastPollInterval).Should(Succeed()) @@ -775,7 +775,7 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru if err != nil { return err } - tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy = v1alpha1.DefaultCAPackagePolicyDisabled + tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy = v1alpha1.DefaultCAPackagePolicy(v1alpha1.Disabled) _, err = trustManagerClient().Update(ctx, tm, metav1.UpdateOptions{}) return err }, lowTimeout, fastPollInterval).Should(Succeed()) @@ -1227,96 +1227,6 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru g.Expect(tm.Status.TrustManagerImage).ShouldNot(BeEmpty()) }, lowTimeout, fastPollInterval).Should(Succeed()) }) - - It("should report trust namespace in status", func() { - createTrustManager(ctx, newTrustManagerCR()) - - By("verifying TrustManager status has default trust namespace set") - Eventually(func(g Gomega) { - tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(tm.Status.TrustNamespace).Should(Equal("cert-manager")) - }, lowTimeout, fastPollInterval).Should(Succeed()) - }) - - It("should report custom trust namespace in status", func() { - By("creating custom trust namespace") - customTrustNS := createUniqueNamespace("custom-trust-ns-status") - createAndDestroyTestNamespace(ctx, clientset, customTrustNS) - - createTrustManager(ctx, newTrustManagerCR().WithTrustNamespace(customTrustNS)) - - By("verifying TrustManager status has custom trust namespace set") - Eventually(func(g Gomega) { - tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(tm.Status.TrustNamespace).Should(Equal(customTrustNS)) - }, lowTimeout, fastPollInterval).Should(Succeed()) - }) - - It("should report secretTargets policy in status", func() { - createTrustManager(ctx, newTrustManagerCR().WithSecretTargets(v1alpha1.SecretTargetsPolicyCustom, []string{"status-test-secret"})) - - By("verifying TrustManager status reflects Custom secretTargets policy") - Eventually(func(g Gomega) { - tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(tm.Status.SecretTargetsPolicy).Should(Equal(v1alpha1.SecretTargetsPolicyCustom)) - }, lowTimeout, fastPollInterval).Should(Succeed()) - }) - - It("should report default CA package policy in status", func() { - createTrustManager(ctx, newTrustManagerCR().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled)) - - By("verifying status reports Enabled policy") - Eventually(func(g Gomega) { - tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(tm.Status.DefaultCAPackagePolicy).Should(Equal(v1alpha1.DefaultCAPackagePolicyEnabled)) - }, lowTimeout, fastPollInterval).Should(Succeed()) - - By("updating TrustManager CR to disable default CA package") - Eventually(func() error { - tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) - if err != nil { - return err - } - tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy = v1alpha1.DefaultCAPackagePolicyDisabled - _, err = trustManagerClient().Update(ctx, tm, metav1.UpdateOptions{}) - return err - }, lowTimeout, fastPollInterval).Should(Succeed()) - - By("verifying status reports Disabled policy") - Eventually(func(g Gomega) { - tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(tm.Status.DefaultCAPackagePolicy).Should(Equal(v1alpha1.DefaultCAPackagePolicyDisabled)) - }, lowTimeout, fastPollInterval).Should(Succeed()) - }) - - It("should report filterExpiredCertificates policy in status", func() { - createTrustManager(ctx, newTrustManagerCR(). - WithFilterExpiredCertificates(v1alpha1.FilterExpiredCertificatesPolicyEnabled)) - - By("verifying status reports Enabled policy") - Eventually(func(g Gomega) { - tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(tm.Status.FilterExpiredCertificatesPolicy).Should(Equal(v1alpha1.FilterExpiredCertificatesPolicyEnabled)) - }, lowTimeout, fastPollInterval).Should(Succeed()) - }) - - It("should report filterNonCACerts policy in status", func() { - createTrustManager(ctx, newTrustManagerCR(). - WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicyEnabled)) - - By("verifying status reports Enabled policy") - Eventually(func(g Gomega) { - tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(tm.Status.FilterNonCACertsPolicy).Should(Equal(v1alpha1.FilterNonCACertsPolicyEnabled)) - }, lowTimeout, fastPollInterval).Should(Succeed()) - }) }) // ------------------------------------------------------------------------- From df12e17c26968225a3b8d33e60f19b018c8d06a8 Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Thu, 17 Sep 2026 15:50:29 +0530 Subject: [PATCH 3/7] CM-1367: Add webhook TLS duration and approver-policy, reuse Mode for Enabled/Disabled fields. Expose Helm-equivalent webhook certificate duration and optional CertificateRequestPolicy creation, and type Enabled/Disabled TrustManager fields as Mode so callers can use v1alpha1.Enabled without casts. --- api/operator/v1alpha1/trustmanager_types.go | 62 +++-- .../v1alpha1/zz_generated.deepcopy.go | 37 +++ .../operator.openshift.io_trustmanagers.yaml | 37 +++ config/rbac/role.yaml | 11 + pkg/controller/trustmanager/approverpolicy.go | 216 ++++++++++++++++++ .../trustmanager/approverpolicy_test.go | 162 +++++++++++++ pkg/controller/trustmanager/certificates.go | 11 +- .../trustmanager/certificates_test.go | 73 +++++- .../trustmanager/configmaps_test.go | 18 +- pkg/controller/trustmanager/constants.go | 11 + pkg/controller/trustmanager/controller.go | 1 + pkg/controller/trustmanager/deployments.go | 4 +- .../trustmanager/deployments_test.go | 18 +- .../trustmanager/install_trustmanager.go | 6 + .../trustmanager/install_trustmanager_test.go | 6 +- pkg/controller/trustmanager/test_utils.go | 17 +- pkg/controller/trustmanager/utils.go | 8 +- .../operator/v1alpha1/approverpolicyconfig.go | 34 +++ .../v1alpha1/defaultcapackageconfig.go | 4 +- .../operator/v1alpha1/trustmanagerconfig.go | 19 +- .../operator/v1alpha1/webhooktlsconfig.go | 44 ++++ pkg/operator/applyconfigurations/utils.go | 4 + test/e2e/multiple_operands_test.go | 16 +- test/e2e/trustmanager_bundle_test.go | 12 +- test/e2e/trustmanager_helpers_test.go | 6 +- test/e2e/trustmanager_test.go | 8 +- 26 files changed, 767 insertions(+), 78 deletions(-) create mode 100644 pkg/controller/trustmanager/approverpolicy.go create mode 100644 pkg/controller/trustmanager/approverpolicy_test.go create mode 100644 pkg/operator/applyconfigurations/operator/v1alpha1/approverpolicyconfig.go create mode 100644 pkg/operator/applyconfigurations/operator/v1alpha1/webhooktlsconfig.go diff --git a/api/operator/v1alpha1/trustmanager_types.go b/api/operator/v1alpha1/trustmanager_types.go index d8ffd6b42..edd12f706 100644 --- a/api/operator/v1alpha1/trustmanager_types.go +++ b/api/operator/v1alpha1/trustmanager_types.go @@ -116,7 +116,7 @@ type TrustManagerConfig struct { // +kubebuilder:validation:Enum:=Enabled;Disabled // +kubebuilder:validation:Optional // +optional - FilterExpiredCertificates FilterExpiredCertificatesPolicy `json:"filterExpiredCertificates,omitempty"` + FilterExpiredCertificates Mode `json:"filterExpiredCertificates,omitempty"` // filterNonCACerts controls whether trust-manager filters out // non-CA certificates from trust bundles before distributing them. @@ -127,7 +127,7 @@ type TrustManagerConfig struct { // +kubebuilder:validation:Enum:=Enabled;Disabled // +kubebuilder:validation:Optional // +optional - FilterNonCACerts FilterNonCACertsPolicy `json:"filterNonCACerts,omitempty"` + FilterNonCACerts Mode `json:"filterNonCACerts,omitempty"` // defaultCAPackage configures the default CA package for trust-manager. // When enabled, the operator will use OpenShift's trusted CA bundle injection mechanism. @@ -164,6 +164,12 @@ type TrustManagerConfig struct { // +kubebuilder:validation:Optional // +optional NodeSelector map[string]string `json:"nodeSelector,omitempty"` + + // webhookTLS configures the cert-manager Certificate used for the + // trust-manager validating webhook serving certificate. + // +kubebuilder:validation:Optional + // +optional + WebhookTLS WebhookTLSConfig `json:"webhookTLS,omitempty"` } // SecretTargetsConfig configures whether and how trust-manager can write @@ -193,6 +199,44 @@ type SecretTargetsConfig struct { AuthorizedSecrets []string `json:"authorizedSecrets,omitempty"` } +// WebhookTLSConfig configures the trust-manager webhook TLS certificate. +type WebhookTLSConfig struct { + // certificateDuration is the requested validity period of the webhook TLS certificate. + // When unset, cert-manager's default certificate duration is used. + // Example: "8760h" for one year. + // +kubebuilder:validation:Optional + // +optional + CertificateDuration *metav1.Duration `json:"certificateDuration,omitempty"` + + // approverPolicy configures a CertificateRequestPolicy so that + // cert-manager-approver-policy can auto-approve the webhook CertificateRequest. + // Resources are created only when policy is Enabled. If Enabled while the + // CertificateRequestPolicy CRD is not installed, reconciliation fails until + // approver-policy is installed or policy is set to Disabled. + // +kubebuilder:validation:Optional + // +optional + ApproverPolicy ApproverPolicyConfig `json:"approverPolicy,omitempty"` +} + +// ApproverPolicyConfig controls creation of a CertificateRequestPolicy for the +// trust-manager webhook certificate. +type ApproverPolicyConfig struct { + // policy controls whether a CertificateRequestPolicy and the RBAC that + // allows the cert-manager controller ServiceAccount to use it are created. + // "Enabled" creates CertificateRequestPolicy trust-manager-policy (to + // auto-approve the webhook certificate), ClusterRole trust-manager-policy-role, + // and ClusterRoleBinding trust-manager-policy-binding for the cert-manager + // ServiceAccount. Nothing is created unless this is set to Enabled. + // If Enabled while cert-manager-approver-policy is not installed, reconcile + // fails because the CertificateRequestPolicy CRD is missing. + // "Disabled" does not create these resources (default). + // +kubebuilder:default:="Disabled" + // +kubebuilder:validation:Enum:=Enabled;Disabled + // +kubebuilder:validation:Optional + // +optional + Policy Mode `json:"policy,omitempty"` +} + // DefaultCAPackageConfig configures the default CA package feature for trust-manager. type DefaultCAPackageConfig struct { // policy controls whether the default CA package feature is enabled. @@ -203,7 +247,7 @@ type DefaultCAPackageConfig struct { // +kubebuilder:validation:Enum:=Enabled;Disabled // +kubebuilder:validation:Optional // +optional - Policy DefaultCAPackagePolicy `json:"policy,omitempty"` + Policy Mode `json:"policy,omitempty"` } // TrustManagerControllerConfig configures the operator's behavior for @@ -226,14 +270,6 @@ type TrustManagerControllerConfig struct { Annotations map[string]string `json:"annotations,omitempty"` } -// FilterExpiredCertificatesPolicy controls whether expired certificates are filtered from bundles. -// Allowed values are Enabled and Disabled. -type FilterExpiredCertificatesPolicy string - -// FilterNonCACertsPolicy controls whether non-CA certificates are filtered from bundles. -// Allowed values are Enabled and Disabled. -type FilterNonCACertsPolicy string - // SecretTargetsPolicy defines the policy for writing trust bundles to Secrets. type SecretTargetsPolicy string @@ -245,10 +281,6 @@ const ( SecretTargetsPolicyCustom SecretTargetsPolicy = "Custom" ) -// DefaultCAPackagePolicy controls whether the default CA package feature is enabled. -// Allowed values are Enabled and Disabled. -type DefaultCAPackagePolicy string - // TrustManagerStatus defines the observed state of TrustManager. type TrustManagerStatus struct { // conditions holds information about the current state of the trust-manager deployment. diff --git a/api/operator/v1alpha1/zz_generated.deepcopy.go b/api/operator/v1alpha1/zz_generated.deepcopy.go index 883cddf1b..2ead4f696 100644 --- a/api/operator/v1alpha1/zz_generated.deepcopy.go +++ b/api/operator/v1alpha1/zz_generated.deepcopy.go @@ -27,6 +27,21 @@ import ( runtime "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ApproverPolicyConfig) DeepCopyInto(out *ApproverPolicyConfig) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ApproverPolicyConfig. +func (in *ApproverPolicyConfig) DeepCopy() *ApproverPolicyConfig { + if in == nil { + return nil + } + out := new(ApproverPolicyConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CertManager) DeepCopyInto(out *CertManager) { *out = *in @@ -637,6 +652,7 @@ func (in *TrustManagerConfig) DeepCopyInto(out *TrustManagerConfig) { (*out)[key] = val } } + in.WebhookTLS.DeepCopyInto(&out.WebhookTLS) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TrustManagerConfig. @@ -820,3 +836,24 @@ func (in *UnsupportedConfigOverridesForCertManagerWebhook) DeepCopy() *Unsupport in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *WebhookTLSConfig) DeepCopyInto(out *WebhookTLSConfig) { + *out = *in + if in.CertificateDuration != nil { + in, out := &in.CertificateDuration, &out.CertificateDuration + *out = new(metav1.Duration) + **out = **in + } + out.ApproverPolicy = in.ApproverPolicy +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WebhookTLSConfig. +func (in *WebhookTLSConfig) DeepCopy() *WebhookTLSConfig { + if in == nil { + return nil + } + out := new(WebhookTLSConfig) + in.DeepCopyInto(out) + return out +} diff --git a/config/crd/bases/operator.openshift.io_trustmanagers.yaml b/config/crd/bases/operator.openshift.io_trustmanagers.yaml index b341d9004..102e8a3bd 100644 --- a/config/crd/bases/operator.openshift.io_trustmanagers.yaml +++ b/config/crd/bases/operator.openshift.io_trustmanagers.yaml @@ -1234,6 +1234,43 @@ spec: x-kubernetes-validations: - message: trustNamespace is immutable once set rule: oldSelf == '' || self == oldSelf + webhookTLS: + description: |- + webhookTLS configures the cert-manager Certificate used for the + trust-manager validating webhook serving certificate. + properties: + approverPolicy: + description: |- + approverPolicy configures a CertificateRequestPolicy so that + cert-manager-approver-policy can auto-approve the webhook CertificateRequest. + Resources are created only when policy is Enabled. If Enabled while the + CertificateRequestPolicy CRD is not installed, reconciliation fails until + approver-policy is installed or policy is set to Disabled. + properties: + policy: + default: Disabled + description: |- + policy controls whether a CertificateRequestPolicy and the RBAC that + allows the cert-manager controller ServiceAccount to use it are created. + "Enabled" creates CertificateRequestPolicy trust-manager-policy (to + auto-approve the webhook certificate), ClusterRole trust-manager-policy-role, + and ClusterRoleBinding trust-manager-policy-binding for the cert-manager + ServiceAccount. Nothing is created unless this is set to Enabled. + If Enabled while cert-manager-approver-policy is not installed, reconcile + fails because the CertificateRequestPolicy CRD is missing. + "Disabled" does not create these resources (default). + enum: + - Enabled + - Disabled + type: string + type: object + certificateDuration: + description: |- + certificateDuration is the requested validity period of the webhook TLS certificate. + When unset, cert-manager's default certificate duration is used. + Example: "8760h" for one year. + type: string + type: object type: object required: - trustManagerConfig diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index f5d902e21..5937bdb45 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -271,6 +271,17 @@ rules: - patch - update - watch +- apiGroups: + - policy.cert-manager.io + resources: + - certificaterequestpolicies + verbs: + - create + - get + - list + - patch + - update + - watch - apiGroups: - rbac.authorization.k8s.io resources: diff --git a/pkg/controller/trustmanager/approverpolicy.go b/pkg/controller/trustmanager/approverpolicy.go new file mode 100644 index 000000000..ab9e06b64 --- /dev/null +++ b/pkg/controller/trustmanager/approverpolicy.go @@ -0,0 +1,216 @@ +package trustmanager + +import ( + "fmt" + "reflect" + + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" + "github.com/openshift/cert-manager-operator/pkg/controller/common" +) + +var certificateRequestPolicyGVK = schema.GroupVersionKind{ + Group: "policy.cert-manager.io", + Version: "v1alpha1", + Kind: "CertificateRequestPolicy", +} + +// createOrApplyApproverPolicyResources creates the webhook CertificateRequestPolicy +// and the RBAC that lets the cert-manager ServiceAccount use it, matching upstream +// Helm app.webhook.tls.approverPolicy. Nothing is created unless policy is Enabled. +// If Enabled and the CertificateRequestPolicy CRD is missing (approver-policy not +// installed), reconciliation fails with a CRD-missing error. +func (r *Reconciler) createOrApplyApproverPolicyResources(trustManager *v1alpha1.TrustManager, resourceLabels, resourceAnnotations map[string]string) error { + if !approverPolicyEnabled(trustManager.Spec.TrustManagerConfig.WebhookTLS.ApproverPolicy) { + return nil + } + + if err := r.createOrApplyCertificateRequestPolicy(trustManager, resourceLabels, resourceAnnotations); err != nil { + return err + } + if err := r.createOrApplyPolicyClusterRole(trustManager, resourceLabels, resourceAnnotations); err != nil { + return err + } + if err := r.createOrApplyPolicyClusterRoleBinding(trustManager, resourceLabels, resourceAnnotations); err != nil { + return err + } + return nil +} + +// createOrApplyCertificateRequestPolicy applies CertificateRequestPolicy +// trust-manager-policy so approver-policy can auto-approve the webhook cert. +// A missing CRP CRD is treated as a reconcile error (install approver-policy +// or set policy to Disabled). +func (r *Reconciler) createOrApplyCertificateRequestPolicy(trustManager *v1alpha1.TrustManager, resourceLabels, resourceAnnotations map[string]string) error { + desired := getCertificateRequestPolicyObject(resourceLabels, resourceAnnotations) + resourceName := desired.GetName() + r.log.V(4).Info("reconciling certificaterequestpolicy resource", "name", resourceName) + + existing := &unstructured.Unstructured{} + existing.SetGroupVersionKind(certificateRequestPolicyGVK) + exists, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), existing) + if err != nil { + if meta.IsNoMatchError(err) { + return common.FromClientError(err, "CertificateRequestPolicy CRD is not installed; install cert-manager-approver-policy or set spec.trustManagerConfig.webhookTLS.approverPolicy.policy to Disabled") + } + return common.FromClientError(err, "failed to check if certificaterequestpolicy %q exists", resourceName) + } + if exists && !certificateRequestPolicyModified(desired, existing) { + r.log.V(4).Info("certificaterequestpolicy resource exists and is in desired state", "name", resourceName) + return nil + } + + r.log.V(2).Info("certificaterequestpolicy resource has been modified, updating to desired state", "name", resourceName) + if err := r.Patch(r.ctx, desired, client.Apply, client.FieldOwner(fieldOwner), client.ForceOwnership); err != nil { + if meta.IsNoMatchError(err) { + return common.FromClientError(err, "CertificateRequestPolicy CRD is not installed; install cert-manager-approver-policy or set spec.trustManagerConfig.webhookTLS.approverPolicy.policy to Disabled") + } + return common.FromClientError(err, "failed to apply certificaterequestpolicy %q", resourceName) + } + + r.eventRecorder.Eventf(trustManager, corev1.EventTypeNormal, "Reconciled", "certificaterequestpolicy resource %s applied", resourceName) + return nil +} + +func getCertificateRequestPolicyObject(resourceLabels, resourceAnnotations map[string]string) *unstructured.Unstructured { + dnsName := fmt.Sprintf("%s.%s.svc", trustManagerServiceName, operandNamespace) + obj := &unstructured.Unstructured{Object: map[string]interface{}{}} + obj.SetGroupVersionKind(certificateRequestPolicyGVK) + obj.SetName(trustManagerCertificateRequestPolicyName) + common.UpdateResourceLabels(obj, resourceLabels) + updateResourceAnnotations(obj, resourceAnnotations) + obj.Object["spec"] = map[string]interface{}{ + "allowed": map[string]interface{}{ + "commonName": map[string]interface{}{ + "value": dnsName, + "required": true, + }, + "dnsNames": map[string]interface{}{ + "values": []interface{}{dnsName}, + "required": true, + }, + }, + "selector": map[string]interface{}{ + "issuerRef": map[string]interface{}{ + "name": trustManagerIssuerName, + "kind": "Issuer", + "group": "cert-manager.io", + }, + }, + } + return obj +} + +func certificateRequestPolicyModified(desired, existing *unstructured.Unstructured) bool { + return managedMetadataModified(desired, existing) || + !reflect.DeepEqual(desired.Object["spec"], existing.Object["spec"]) +} + +func (r *Reconciler) createOrApplyPolicyClusterRole(trustManager *v1alpha1.TrustManager, resourceLabels, resourceAnnotations map[string]string) error { + desired := getPolicyClusterRoleObject(resourceLabels, resourceAnnotations) + resourceName := desired.GetName() + r.log.V(4).Info("reconciling approver-policy clusterrole resource", "name", resourceName) + + existing := &rbacv1.ClusterRole{} + exists, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), existing) + if err != nil { + return common.FromClientError(err, "failed to check if clusterrole %q exists", resourceName) + } + if exists && !clusterRoleModified(desired, existing) { + r.log.V(4).Info("approver-policy clusterrole resource exists and is in desired state", "name", resourceName) + return nil + } + + r.log.V(2).Info("approver-policy clusterrole resource has been modified, updating to desired state", "name", resourceName) + if err := r.Patch(r.ctx, desired, client.Apply, client.FieldOwner(fieldOwner), client.ForceOwnership); err != nil { + return common.FromClientError(err, "failed to apply clusterrole %q", resourceName) + } + + r.eventRecorder.Eventf(trustManager, corev1.EventTypeNormal, "Reconciled", "clusterrole resource %s applied", resourceName) + return nil +} + +// getPolicyClusterRoleObject returns ClusterRole trust-manager-policy-role, +// granting the cert-manager ServiceAccount use of CertificateRequestPolicy trust-manager-policy. +func getPolicyClusterRoleObject(resourceLabels, resourceAnnotations map[string]string) *rbacv1.ClusterRole { + role := &rbacv1.ClusterRole{ + TypeMeta: metav1.TypeMeta{ + APIVersion: rbacv1.SchemeGroupVersion.String(), + Kind: "ClusterRole", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: trustManagerPolicyClusterRoleName, + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"policy.cert-manager.io"}, + Resources: []string{"certificaterequestpolicies"}, + Verbs: []string{"use"}, + ResourceNames: []string{trustManagerCertificateRequestPolicyName}, + }, + }, + } + common.UpdateResourceLabels(role, resourceLabels) + updateResourceAnnotations(role, resourceAnnotations) + return role +} + +func (r *Reconciler) createOrApplyPolicyClusterRoleBinding(trustManager *v1alpha1.TrustManager, resourceLabels, resourceAnnotations map[string]string) error { + desired := getPolicyClusterRoleBindingObject(resourceLabels, resourceAnnotations) + resourceName := desired.GetName() + r.log.V(4).Info("reconciling approver-policy clusterrolebinding resource", "name", resourceName) + + existing := &rbacv1.ClusterRoleBinding{} + exists, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), existing) + if err != nil { + return common.FromClientError(err, "failed to check if clusterrolebinding %q exists", resourceName) + } + if exists && !clusterRoleBindingModified(desired, existing) { + r.log.V(4).Info("approver-policy clusterrolebinding resource exists and is in desired state", "name", resourceName) + return nil + } + + r.log.V(2).Info("approver-policy clusterrolebinding resource has been modified, updating to desired state", "name", resourceName) + if err := r.Patch(r.ctx, desired, client.Apply, client.FieldOwner(fieldOwner), client.ForceOwnership); err != nil { + return common.FromClientError(err, "failed to apply clusterrolebinding %q", resourceName) + } + + r.eventRecorder.Eventf(trustManager, corev1.EventTypeNormal, "Reconciled", "clusterrolebinding resource %s applied", resourceName) + return nil +} + +// getPolicyClusterRoleBindingObject binds trust-manager-policy-role to the +// cert-manager ServiceAccount in the operand namespace. +func getPolicyClusterRoleBindingObject(resourceLabels, resourceAnnotations map[string]string) *rbacv1.ClusterRoleBinding { + binding := &rbacv1.ClusterRoleBinding{ + TypeMeta: metav1.TypeMeta{ + APIVersion: rbacv1.SchemeGroupVersion.String(), + Kind: "ClusterRoleBinding", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: trustManagerPolicyClusterRoleBindingName, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "ClusterRole", + Name: trustManagerPolicyClusterRoleName, + }, + Subjects: []rbacv1.Subject{ + { + Kind: roleBindingSubjectKind, + Name: certManagerControllerServiceAccountName, + Namespace: operandNamespace, + }, + }, + } + common.UpdateResourceLabels(binding, resourceLabels) + updateResourceAnnotations(binding, resourceAnnotations) + return binding +} diff --git a/pkg/controller/trustmanager/approverpolicy_test.go b/pkg/controller/trustmanager/approverpolicy_test.go new file mode 100644 index 000000000..69c399d8f --- /dev/null +++ b/pkg/controller/trustmanager/approverpolicy_test.go @@ -0,0 +1,162 @@ +package trustmanager + +import ( + "context" + "testing" + + rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" + "github.com/openshift/cert-manager-operator/pkg/controller/common/fakes" +) + +func TestCertificateRequestPolicyObject(t *testing.T) { + labels := testResourceLabels() + annotations := map[string]string{"user-annotation": "test-value"} + obj := getCertificateRequestPolicyObject(labels, annotations) + + if obj.GetName() != trustManagerCertificateRequestPolicyName { + t.Errorf("expected name %q, got %q", trustManagerCertificateRequestPolicyName, obj.GetName()) + } + if obj.GroupVersionKind() != certificateRequestPolicyGVK { + t.Errorf("expected gvk %s, got %s", certificateRequestPolicyGVK, obj.GroupVersionKind()) + } + if obj.GetLabels()["app"] != trustManagerCommonName { + t.Errorf("expected app label %q, got %q", trustManagerCommonName, obj.GetLabels()["app"]) + } + if obj.GetAnnotations()["user-annotation"] != "test-value" { + t.Errorf("expected user annotation to be preserved") + } + + expectedDNS := trustManagerServiceName + "." + operandNamespace + ".svc" + cn, found, err := unstructured.NestedString(obj.Object, "spec", "allowed", "commonName", "value") + if err != nil || !found || cn != expectedDNS { + t.Errorf("expected commonName %q, got %q found=%v err=%v", expectedDNS, cn, found, err) + } +} + +func TestPolicyClusterRoleBindingSubjects(t *testing.T) { + binding := getPolicyClusterRoleBindingObject(testResourceLabels(), testResourceAnnotations()) + if binding.Name != trustManagerPolicyClusterRoleBindingName { + t.Errorf("expected name %q, got %q", trustManagerPolicyClusterRoleBindingName, binding.Name) + } + if binding.RoleRef.Name != trustManagerPolicyClusterRoleName { + t.Errorf("expected roleRef %q, got %q", trustManagerPolicyClusterRoleName, binding.RoleRef.Name) + } + if len(binding.Subjects) != 1 { + t.Fatalf("expected 1 subject, got %d", len(binding.Subjects)) + } + if binding.Subjects[0].Name != certManagerControllerServiceAccountName || binding.Subjects[0].Namespace != operandNamespace { + t.Errorf("expected subject %s/%s, got %s/%s", operandNamespace, certManagerControllerServiceAccountName, binding.Subjects[0].Namespace, binding.Subjects[0].Name) + } +} + +func TestApproverPolicyReconciliation(t *testing.T) { + tests := []struct { + name string + tmBuilder *trustManagerBuilder + preReq func(*Reconciler, *fakes.FakeCtrlClient) + wantErr string + wantExistsCount int + wantPatchCount int + }{ + { + name: "skip when approver policy is disabled", + wantExistsCount: 0, + wantPatchCount: 0, + }, + { + name: "apply policy resources when enabled and missing", + tmBuilder: testTrustManager().WithApproverPolicy(v1alpha1.Enabled), + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + return false, nil + }) + }, + wantExistsCount: 3, + wantPatchCount: 3, + }, + { + name: "skip apply when existing policy resources match", + tmBuilder: testTrustManager().WithApproverPolicy(v1alpha1.Enabled), + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + switch dst := obj.(type) { + case *unstructured.Unstructured: + getCertificateRequestPolicyObject(testResourceLabels(), testResourceAnnotations()).DeepCopyInto(dst) + case *rbacv1.ClusterRole: + getPolicyClusterRoleObject(testResourceLabels(), testResourceAnnotations()).DeepCopyInto(dst) + case *rbacv1.ClusterRoleBinding: + getPolicyClusterRoleBindingObject(testResourceLabels(), testResourceAnnotations()).DeepCopyInto(dst) + } + return true, nil + }) + }, + wantExistsCount: 3, + wantPatchCount: 0, + }, + { + name: "apply when certificate request policy spec drifted", + tmBuilder: testTrustManager().WithApproverPolicy(v1alpha1.Enabled), + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + switch dst := obj.(type) { + case *unstructured.Unstructured: + existing := getCertificateRequestPolicyObject(testResourceLabels(), testResourceAnnotations()) + _ = unstructured.SetNestedStringMap(existing.Object, map[string]string{"name": "wrong"}, "spec", "selector", "issuerRef") + existing.DeepCopyInto(dst) + case *rbacv1.ClusterRole: + getPolicyClusterRoleObject(testResourceLabels(), testResourceAnnotations()).DeepCopyInto(dst) + case *rbacv1.ClusterRoleBinding: + getPolicyClusterRoleBindingObject(testResourceLabels(), testResourceAnnotations()).DeepCopyInto(dst) + } + return true, nil + }) + }, + wantExistsCount: 3, + wantPatchCount: 1, + }, + { + name: "reports missing CertificateRequestPolicy CRD", + tmBuilder: testTrustManager().WithApproverPolicy(v1alpha1.Enabled), + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + return false, &meta.NoKindMatchError{GroupKind: schema.GroupKind{Group: "policy.cert-manager.io", Kind: "CertificateRequestPolicy"}} + }) + }, + wantErr: "CertificateRequestPolicy CRD is not installed", + wantExistsCount: 1, + wantPatchCount: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := testReconciler(t) + mock := &fakes.FakeCtrlClient{} + if tt.preReq != nil { + tt.preReq(r, mock) + } + r.CtrlClient = mock + + tmBuilder := tt.tmBuilder + if tmBuilder == nil { + tmBuilder = testTrustManager() + } + tm := tmBuilder.Build() + err := r.createOrApplyApproverPolicyResources(tm, getResourceLabels(tm), getResourceAnnotations(tm)) + assertError(t, err, tt.wantErr) + + if got := mock.ExistsCallCount(); got != tt.wantExistsCount { + t.Errorf("expected %d Exists calls, got %d", tt.wantExistsCount, got) + } + if got := mock.PatchCallCount(); got != tt.wantPatchCount { + t.Errorf("expected %d Patch calls, got %d", tt.wantPatchCount, got) + } + }) + } +} diff --git a/pkg/controller/trustmanager/certificates.go b/pkg/controller/trustmanager/certificates.go index be60843e7..1d4f196bd 100644 --- a/pkg/controller/trustmanager/certificates.go +++ b/pkg/controller/trustmanager/certificates.go @@ -53,7 +53,7 @@ func getIssuerObject(resourceLabels, resourceAnnotations map[string]string) *cer // createOrApplyCertificate reconciles the Certificate used for trust-manager's webhook TLS. func (r *Reconciler) createOrApplyCertificate(trustManager *v1alpha1.TrustManager, resourceLabels, resourceAnnotations map[string]string) error { - desired := getCertificateObject(resourceLabels, resourceAnnotations) + desired := getCertificateObject(trustManager.Spec.TrustManagerConfig, resourceLabels, resourceAnnotations) resourceName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName()) r.log.V(4).Info("reconciling certificate resource", "name", resourceName) @@ -76,7 +76,7 @@ func (r *Reconciler) createOrApplyCertificate(trustManager *v1alpha1.TrustManage return nil } -func getCertificateObject(resourceLabels, resourceAnnotations map[string]string) *certmanagerv1.Certificate { +func getCertificateObject(config v1alpha1.TrustManagerConfig, resourceLabels, resourceAnnotations map[string]string) *certmanagerv1.Certificate { certificate := common.DecodeObjBytes[*certmanagerv1.Certificate](codecs, certmanagerv1.SchemeGroupVersion, assets.MustAsset(certificateAssetName)) common.UpdateName(certificate, trustManagerCertificateName) common.UpdateNamespace(certificate, operandNamespace) @@ -92,6 +92,9 @@ func getCertificateObject(resourceLabels, resourceAnnotations map[string]string) Kind: "Issuer", Group: "cert-manager.io", } + if config.WebhookTLS.CertificateDuration != nil { + certificate.Spec.Duration = config.WebhookTLS.CertificateDuration + } return certificate } @@ -105,6 +108,7 @@ func issuerModified(desired, existing *certmanagerv1.Issuer) bool { // certificateModified compares only the fields we manage via SSA. // We compare individual spec fields rather than the full Spec because // cert-manager's webhook may default fields we don't set (e.g. Duration). +// Duration is compared only when we explicitly set it on the desired object. func certificateModified(desired, existing *certmanagerv1.Certificate) bool { if managedMetadataModified(desired, existing) { return true @@ -116,5 +120,8 @@ func certificateModified(desired, existing *certmanagerv1.Certificate) bool { !reflect.DeepEqual(desired.Spec.IssuerRef, existing.Spec.IssuerRef) { return true } + if desired.Spec.Duration != nil && !ptr.Equal(desired.Spec.Duration, existing.Spec.Duration) { + return true + } return false } diff --git a/pkg/controller/trustmanager/certificates_test.go b/pkg/controller/trustmanager/certificates_test.go index 122773ed8..467315ff1 100644 --- a/pkg/controller/trustmanager/certificates_test.go +++ b/pkg/controller/trustmanager/certificates_test.go @@ -4,12 +4,15 @@ import ( "context" "fmt" "testing" + "time" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" certmanagermetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" + "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" "github.com/openshift/cert-manager-operator/pkg/controller/common/fakes" ) @@ -109,7 +112,7 @@ func TestCertificateObject(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { tm := tt.tm.Build() - cert := getCertificateObject(getResourceLabels(tm), getResourceAnnotations(tm)) + cert := getCertificateObject(tm.Spec.TrustManagerConfig, getResourceLabels(tm), getResourceAnnotations(tm)) if tt.wantName != "" && cert.Name != tt.wantName { t.Errorf("expected name %q, got %q", tt.wantName, cert.Name) @@ -133,7 +136,7 @@ func TestCertificateObject(t *testing.T) { func TestCertificateSpec(t *testing.T) { tm := testTrustManager().Build() - cert := getCertificateObject(getResourceLabels(tm), getResourceAnnotations(tm)) + cert := getCertificateObject(tm.Spec.TrustManagerConfig, getResourceLabels(tm), getResourceAnnotations(tm)) expectedDNSName := fmt.Sprintf("%s.%s.svc", trustManagerServiceName, operandNamespace) t.Run("sets correct common name", func(t *testing.T) { @@ -165,6 +168,20 @@ func TestCertificateSpec(t *testing.T) { t.Errorf("expected issuerRef.group %q, got %q", "cert-manager.io", cert.Spec.IssuerRef.Group) } }) + + t.Run("omits duration when webhookTLS.certificateDuration is unset", func(t *testing.T) { + if cert.Spec.Duration != nil { + t.Errorf("expected duration to be unset, got %v", cert.Spec.Duration) + } + }) +} + +func TestCertificateDuration(t *testing.T) { + tm := testTrustManager().WithWebhookCertificateDuration(8760 * time.Hour).Build() + cert := getCertificateObject(tm.Spec.TrustManagerConfig, getResourceLabels(tm), getResourceAnnotations(tm)) + if cert.Spec.Duration == nil || cert.Spec.Duration.Duration != 8760*time.Hour { + t.Errorf("expected duration 8760h, got %+v", cert.Spec.Duration) + } } func TestIssuerReconciliation(t *testing.T) { @@ -303,7 +320,7 @@ func TestCertificateReconciliation(t *testing.T) { name: "skip apply when existing matches desired", preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { - cert := getCertificateObject(testResourceLabels(), testResourceAnnotations()) + cert := getCertificateObject(v1alpha1.TrustManagerConfig{}, testResourceLabels(), testResourceAnnotations()) cert.DeepCopyInto(obj.(*certmanagerv1.Certificate)) return true, nil }) @@ -315,7 +332,7 @@ func TestCertificateReconciliation(t *testing.T) { name: "apply when existing has label drift", preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { - cert := getCertificateObject(testResourceLabels(), testResourceAnnotations()) + cert := getCertificateObject(v1alpha1.TrustManagerConfig{}, testResourceLabels(), testResourceAnnotations()) cert.Labels["app.kubernetes.io/instance"] = "modified-value" cert.DeepCopyInto(obj.(*certmanagerv1.Certificate)) return true, nil @@ -330,7 +347,7 @@ func TestCertificateReconciliation(t *testing.T) { preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { tm := testTrustManager().WithAnnotations(map[string]string{"user-annotation": "original"}).Build() - cert := getCertificateObject(getResourceLabels(tm), getResourceAnnotations(tm)) + cert := getCertificateObject(tm.Spec.TrustManagerConfig, getResourceLabels(tm), getResourceAnnotations(tm)) cert.Annotations["user-annotation"] = "tampered" cert.DeepCopyInto(obj.(*certmanagerv1.Certificate)) return true, nil @@ -343,7 +360,7 @@ func TestCertificateReconciliation(t *testing.T) { name: "apply when existing has secret name drift", preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { - cert := getCertificateObject(testResourceLabels(), testResourceAnnotations()) + cert := getCertificateObject(v1alpha1.TrustManagerConfig{}, testResourceLabels(), testResourceAnnotations()) cert.Spec.SecretName = "wrong-secret" cert.DeepCopyInto(obj.(*certmanagerv1.Certificate)) return true, nil @@ -356,7 +373,7 @@ func TestCertificateReconciliation(t *testing.T) { name: "apply when existing has issuer ref drift", preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { - cert := getCertificateObject(testResourceLabels(), testResourceAnnotations()) + cert := getCertificateObject(v1alpha1.TrustManagerConfig{}, testResourceLabels(), testResourceAnnotations()) cert.Spec.IssuerRef = certmanagermetav1.ObjectReference{ Name: "wrong-issuer", Kind: "Issuer", @@ -369,6 +386,48 @@ func TestCertificateReconciliation(t *testing.T) { wantExistsCount: 1, wantPatchCount: 1, }, + { + name: "apply when existing duration differs from configured duration", + tmBuilder: testTrustManager().WithWebhookCertificateDuration(8760 * time.Hour), + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + tm := testTrustManager().WithWebhookCertificateDuration(8760 * time.Hour).Build() + cert := getCertificateObject(tm.Spec.TrustManagerConfig, getResourceLabels(tm), getResourceAnnotations(tm)) + cert.Spec.Duration = &metav1.Duration{Duration: time.Hour} + cert.DeepCopyInto(obj.(*certmanagerv1.Certificate)) + return true, nil + }) + }, + wantExistsCount: 1, + wantPatchCount: 1, + }, + { + name: "skip apply when existing duration matches configured duration", + tmBuilder: testTrustManager().WithWebhookCertificateDuration(8760 * time.Hour), + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + tm := testTrustManager().WithWebhookCertificateDuration(8760 * time.Hour).Build() + cert := getCertificateObject(tm.Spec.TrustManagerConfig, getResourceLabels(tm), getResourceAnnotations(tm)) + cert.DeepCopyInto(obj.(*certmanagerv1.Certificate)) + return true, nil + }) + }, + wantExistsCount: 1, + wantPatchCount: 0, + }, + { + name: "skip apply when duration unset and existing has defaulted duration", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + cert := getCertificateObject(v1alpha1.TrustManagerConfig{}, testResourceLabels(), testResourceAnnotations()) + cert.Spec.Duration = &metav1.Duration{Duration: 90 * 24 * time.Hour} + cert.DeepCopyInto(obj.(*certmanagerv1.Certificate)) + return true, nil + }) + }, + wantExistsCount: 1, + wantPatchCount: 0, + }, { name: "exists error propagates", preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { diff --git a/pkg/controller/trustmanager/configmaps_test.go b/pkg/controller/trustmanager/configmaps_test.go index 1a119017c..c0cb4cf3a 100644 --- a/pkg/controller/trustmanager/configmaps_test.go +++ b/pkg/controller/trustmanager/configmaps_test.go @@ -159,7 +159,7 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { }{ { name: "skips when policy is Disabled", - tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Disabled)), + tm: testTrustManager().WithDefaultCAPackage(v1alpha1.Disabled), preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { }, wantExistsCount: 0, @@ -175,7 +175,7 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { }, { name: "returns error when injection ConfigMap is not found", - tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)), + tm: testTrustManager().WithDefaultCAPackage(v1alpha1.Enabled), preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { return errTestClient @@ -185,7 +185,7 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { }, { name: "returns error when CA bundle key is missing", - tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)), + tm: testTrustManager().WithDefaultCAPackage(v1alpha1.Enabled), preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { cm := obj.(*corev1.ConfigMap) @@ -198,7 +198,7 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { }, { name: "returns error when CA bundle is empty", - tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)), + tm: testTrustManager().WithDefaultCAPackage(v1alpha1.Enabled), preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { cm := obj.(*corev1.ConfigMap) @@ -211,7 +211,7 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { }, { name: "creates ConfigMap and returns hash when bundle is available", - tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)), + tm: testTrustManager().WithDefaultCAPackage(v1alpha1.Enabled), preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { cm := obj.(*corev1.ConfigMap) @@ -229,7 +229,7 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { }, { name: "skips patch when existing ConfigMap matches desired", - tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)), + tm: testTrustManager().WithDefaultCAPackage(v1alpha1.Enabled), preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { cm := obj.(*corev1.ConfigMap) @@ -251,7 +251,7 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { }, { name: "patches when existing ConfigMap data differs", - tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)), + tm: testTrustManager().WithDefaultCAPackage(v1alpha1.Enabled), preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { cm := obj.(*corev1.ConfigMap) @@ -271,7 +271,7 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { }, { name: "propagates Exists error", - tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)), + tm: testTrustManager().WithDefaultCAPackage(v1alpha1.Enabled), preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { cm := obj.(*corev1.ConfigMap) @@ -288,7 +288,7 @@ func TestDefaultCAPackageConfigMapReconciliation(t *testing.T) { }, { name: "propagates Patch error", - tm: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)), + tm: testTrustManager().WithDefaultCAPackage(v1alpha1.Enabled), preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { cm := obj.(*corev1.ConfigMap) diff --git a/pkg/controller/trustmanager/constants.go b/pkg/controller/trustmanager/constants.go index 11bfccc25..bf4015f5d 100644 --- a/pkg/controller/trustmanager/constants.go +++ b/pkg/controller/trustmanager/constants.go @@ -100,6 +100,17 @@ const ( trustManagerTLSSecretName = trustManagerCommonResourceName + "-tls" trustManagerWebhookConfigName = trustManagerCommonResourceName + + // trustManagerCertificateRequestPolicyName is created when webhookTLS.approverPolicy.policy is Enabled. + trustManagerCertificateRequestPolicyName = trustManagerCommonResourceName + "-policy" + // trustManagerPolicyClusterRoleName grants the cert-manager SA use access to the CRP. + trustManagerPolicyClusterRoleName = trustManagerCommonResourceName + "-policy-role" + // trustManagerPolicyClusterRoleBindingName binds trust-manager-policy-role to the cert-manager SA. + trustManagerPolicyClusterRoleBindingName = trustManagerCommonResourceName + "-policy-binding" + + // certManagerControllerServiceAccountName is the cert-manager controller SA + // bound to trust-manager-policy so approver-policy can auto-approve the webhook cert. + certManagerControllerServiceAccountName = "cert-manager" ) var ( diff --git a/pkg/controller/trustmanager/controller.go b/pkg/controller/trustmanager/controller.go index 9445542f9..954e01040 100644 --- a/pkg/controller/trustmanager/controller.go +++ b/pkg/controller/trustmanager/controller.go @@ -55,6 +55,7 @@ type Reconciler struct { // +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles;clusterrolebindings,verbs=get;list;watch;create;update;patch // +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=get;list;watch;create;update;patch // +kubebuilder:rbac:groups=cert-manager.io,resources=certificates;issuers,verbs=get;list;watch;create;update;patch +// +kubebuilder:rbac:groups=policy.cert-manager.io,resources=certificaterequestpolicies,verbs=get;list;watch;create;update;patch // +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=validatingwebhookconfigurations,verbs=get;list;watch;create;update;patch // +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 diff --git a/pkg/controller/trustmanager/deployments.go b/pkg/controller/trustmanager/deployments.go index 1156689ad..88061aed8 100644 --- a/pkg/controller/trustmanager/deployments.go +++ b/pkg/controller/trustmanager/deployments.go @@ -143,11 +143,11 @@ func updateDeploymentArgs(deployment *appsv1.Deployment, trustManager *v1alpha1. args = append(args, "--secret-targets-enabled=true") } - if config.FilterExpiredCertificates == v1alpha1.FilterExpiredCertificatesPolicy(v1alpha1.Enabled) { + if config.FilterExpiredCertificates == v1alpha1.Enabled { args = append(args, "--filter-expired-certificates=true") } - if config.FilterNonCACerts == v1alpha1.FilterNonCACertsPolicy(v1alpha1.Enabled) { + if config.FilterNonCACerts == v1alpha1.Enabled { args = append(args, "--filter-non-ca-certs=true") } diff --git a/pkg/controller/trustmanager/deployments_test.go b/pkg/controller/trustmanager/deployments_test.go index cb8028a2f..c36f697cb 100644 --- a/pkg/controller/trustmanager/deployments_test.go +++ b/pkg/controller/trustmanager/deployments_test.go @@ -158,8 +158,8 @@ func TestDeploymentContainerArgs(t *testing.T) { WithLogLevel(5). WithLogFormat("json"). WithTrustNamespace("custom-ns"). - WithFilterExpiredCertificates(v1alpha1.FilterExpiredCertificatesPolicy(v1alpha1.Enabled)). - WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicy(v1alpha1.Enabled)), + WithFilterExpiredCertificates(v1alpha1.Enabled). + WithFilterNonCACerts(v1alpha1.Enabled), expectedArgs: []string{ "--log-level=5", "--log-format=json", @@ -202,7 +202,7 @@ func TestDeploymentContainerArgs(t *testing.T) { }, { name: "includes default-package-location when defaultCAPackage is Enabled", - tmBuilder: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)), + tmBuilder: testTrustManager().WithDefaultCAPackage(v1alpha1.Enabled), expectedArgs: []string{ fmt.Sprintf("--default-package-location=%s", defaultCAPackageLocation), }, @@ -215,14 +215,14 @@ func TestDeploymentContainerArgs(t *testing.T) { }, { name: "includes filter-non-ca-certs when filterNonCACerts is Enabled", - tmBuilder: testTrustManager().WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicy(v1alpha1.Enabled)), + tmBuilder: testTrustManager().WithFilterNonCACerts(v1alpha1.Enabled), expectedArgs: []string{ "--filter-non-ca-certs=true", }, }, { name: "excludes filter-non-ca-certs when filterNonCACerts is Disabled", - tmBuilder: testTrustManager().WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicy(v1alpha1.Disabled)), + tmBuilder: testTrustManager().WithFilterNonCACerts(v1alpha1.Disabled), notExpectedArgs: []string{ "--filter-non-ca-certs=true", }, @@ -265,7 +265,7 @@ func TestDeploymentDefaultCAPackage(t *testing.T) { t.Run("adds arg, volume, mount, and hash annotation when enabled", func(t *testing.T) { r := testReconciler(t) - tm := testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)).Build() + tm := testTrustManager().WithDefaultCAPackage(v1alpha1.Enabled).Build() dep, err := r.getDeploymentObject(tm, testResourceLabels(), testResourceAnnotations(), "abc123hash") if err != nil { t.Fatalf("unexpected error: %v", err) @@ -517,7 +517,7 @@ func TestDeploymentReconciliation(t *testing.T) { setImage: true, preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { - tm := testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)).Build() + tm := testTrustManager().WithDefaultCAPackage(v1alpha1.Enabled).Build() dep, err := r.getDeploymentObject(tm, testResourceLabels(), testResourceAnnotations(), "abc123hash") if err != nil { t.Fatalf("unexpected error: %v", err) @@ -531,12 +531,12 @@ func TestDeploymentReconciliation(t *testing.T) { }, { name: "apply when existing has pod template annotation drift", - tmBuilder: testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)), + tmBuilder: testTrustManager().WithDefaultCAPackage(v1alpha1.Enabled), caBundleHash: "abc123hash", setImage: true, preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { - tm := testTrustManager().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)).Build() + tm := testTrustManager().WithDefaultCAPackage(v1alpha1.Enabled).Build() dep, err := r.getDeploymentObject(tm, testResourceLabels(), testResourceAnnotations(), "abc123hash") if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/pkg/controller/trustmanager/install_trustmanager.go b/pkg/controller/trustmanager/install_trustmanager.go index 0c4415314..7f383d0f3 100644 --- a/pkg/controller/trustmanager/install_trustmanager.go +++ b/pkg/controller/trustmanager/install_trustmanager.go @@ -52,6 +52,12 @@ func (r *Reconciler) reconcileTrustManagerDeployment(trustManager *v1alpha1.Trus return err } + // Optional: CertificateRequestPolicy + RBAC when webhookTLS.approverPolicy.policy is Enabled. + if err := r.createOrApplyApproverPolicyResources(trustManager, resourceLabels, resourceAnnotations); err != nil { + r.log.Error(err, "failed to reconcile approver-policy resources") + return err + } + if err := r.createOrApplyDeployment(trustManager, resourceLabels, resourceAnnotations, caBundleHash); err != nil { r.log.Error(err, "failed to reconcile deployment resource") return err diff --git a/pkg/controller/trustmanager/install_trustmanager_test.go b/pkg/controller/trustmanager/install_trustmanager_test.go index d419f9b65..b331ecbc9 100644 --- a/pkg/controller/trustmanager/install_trustmanager_test.go +++ b/pkg/controller/trustmanager/install_trustmanager_test.go @@ -38,9 +38,9 @@ func TestUpdateStatusObservedState(t *testing.T) { return testTrustManager(). WithTrustNamespace("custom-trust-ns"). WithSecretTargets(v1alpha1.SecretTargetsPolicyCustom, []string{"allowed-secret"}). - WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)). - WithFilterExpiredCertificates(v1alpha1.FilterExpiredCertificatesPolicy(v1alpha1.Enabled)). - WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicy(v1alpha1.Enabled)). + WithDefaultCAPackage(v1alpha1.Enabled). + WithFilterExpiredCertificates(v1alpha1.Enabled). + WithFilterNonCACerts(v1alpha1.Enabled). Build() }, wantStatusUpdate: 1, diff --git a/pkg/controller/trustmanager/test_utils.go b/pkg/controller/trustmanager/test_utils.go index 33e822070..031666ab5 100644 --- a/pkg/controller/trustmanager/test_utils.go +++ b/pkg/controller/trustmanager/test_utils.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" "testing" + "time" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -84,17 +85,17 @@ func (b *trustManagerBuilder) WithTrustNamespace(ns string) *trustManagerBuilder return b } -func (b *trustManagerBuilder) WithFilterExpiredCertificates(policy v1alpha1.FilterExpiredCertificatesPolicy) *trustManagerBuilder { +func (b *trustManagerBuilder) WithFilterExpiredCertificates(policy v1alpha1.Mode) *trustManagerBuilder { b.Spec.TrustManagerConfig.FilterExpiredCertificates = policy return b } -func (b *trustManagerBuilder) WithFilterNonCACerts(policy v1alpha1.FilterNonCACertsPolicy) *trustManagerBuilder { +func (b *trustManagerBuilder) WithFilterNonCACerts(policy v1alpha1.Mode) *trustManagerBuilder { b.Spec.TrustManagerConfig.FilterNonCACerts = policy return b } -func (b *trustManagerBuilder) WithDefaultCAPackage(policy v1alpha1.DefaultCAPackagePolicy) *trustManagerBuilder { +func (b *trustManagerBuilder) WithDefaultCAPackage(policy v1alpha1.Mode) *trustManagerBuilder { b.Spec.TrustManagerConfig.DefaultCAPackage.Policy = policy return b } @@ -107,6 +108,16 @@ func (b *trustManagerBuilder) WithSecretTargets(policy v1alpha1.SecretTargetsPol return b } +func (b *trustManagerBuilder) WithWebhookCertificateDuration(d time.Duration) *trustManagerBuilder { + b.Spec.TrustManagerConfig.WebhookTLS.CertificateDuration = &metav1.Duration{Duration: d} + return b +} + +func (b *trustManagerBuilder) WithApproverPolicy(policy v1alpha1.Mode) *trustManagerBuilder { + b.Spec.TrustManagerConfig.WebhookTLS.ApproverPolicy.Policy = policy + return b +} + func (b *trustManagerBuilder) Build() *v1alpha1.TrustManager { return b.TrustManager } diff --git a/pkg/controller/trustmanager/utils.go b/pkg/controller/trustmanager/utils.go index 1e921c7fd..fb0d8b525 100644 --- a/pkg/controller/trustmanager/utils.go +++ b/pkg/controller/trustmanager/utils.go @@ -146,7 +146,13 @@ func secretTargetsEnabled(config v1alpha1.SecretTargetsConfig) bool { // defaultCAPackageEnabled returns true when the defaultCAPackage policy is Enabled. func defaultCAPackageEnabled(config v1alpha1.DefaultCAPackageConfig) bool { - return config.Policy == v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled) + return config.Policy == v1alpha1.Enabled +} + +// approverPolicyEnabled returns true when spec.trustManagerConfig.webhookTLS.approverPolicy.policy +// is Enabled. When false, no CertificateRequestPolicy or related RBAC is created. +func approverPolicyEnabled(config v1alpha1.ApproverPolicyConfig) bool { + return config.Policy == v1alpha1.Enabled } // getTrustNamespace returns the trust namespace from the TrustManager config. diff --git a/pkg/operator/applyconfigurations/operator/v1alpha1/approverpolicyconfig.go b/pkg/operator/applyconfigurations/operator/v1alpha1/approverpolicyconfig.go new file mode 100644 index 000000000..d31b32696 --- /dev/null +++ b/pkg/operator/applyconfigurations/operator/v1alpha1/approverpolicyconfig.go @@ -0,0 +1,34 @@ +// Code generated by applyconfiguration-gen. DO NOT EDIT. + +package v1alpha1 + +import ( + operatorv1alpha1 "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" +) + +// ApproverPolicyConfigApplyConfiguration represents a declarative configuration of the ApproverPolicyConfig type for use +// with apply. +// +// ApproverPolicyConfig controls creation of a CertificateRequestPolicy for the +// trust-manager webhook certificate. +type ApproverPolicyConfigApplyConfiguration struct { + // policy controls whether a CertificateRequestPolicy and the RBAC that + // allows cert-manager to use it are created. + // "Enabled" creates CertificateRequestPolicy trust-manager-policy. + // "Disabled" does not create it (default). + Policy *operatorv1alpha1.Mode `json:"policy,omitempty"` +} + +// ApproverPolicyConfigApplyConfiguration constructs a declarative configuration of the ApproverPolicyConfig type for use with +// apply. +func ApproverPolicyConfig() *ApproverPolicyConfigApplyConfiguration { + return &ApproverPolicyConfigApplyConfiguration{} +} + +// WithPolicy sets the Policy field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the Policy field is set to the value of the last call. +func (b *ApproverPolicyConfigApplyConfiguration) WithPolicy(value operatorv1alpha1.Mode) *ApproverPolicyConfigApplyConfiguration { + b.Policy = &value + return b +} diff --git a/pkg/operator/applyconfigurations/operator/v1alpha1/defaultcapackageconfig.go b/pkg/operator/applyconfigurations/operator/v1alpha1/defaultcapackageconfig.go index ccfe11dc1..99b17a0f0 100644 --- a/pkg/operator/applyconfigurations/operator/v1alpha1/defaultcapackageconfig.go +++ b/pkg/operator/applyconfigurations/operator/v1alpha1/defaultcapackageconfig.go @@ -15,7 +15,7 @@ type DefaultCAPackageConfigApplyConfiguration struct { // When set to "Enabled", the operator will inject OpenShift's trusted CA bundle // into trust-manager, enabling the "useDefaultCAs: true" source in Bundle resources. // When set to "Disabled", no default CA package is configured and Bundles cannot use useDefaultCAs (default behavior). - Policy *operatorv1alpha1.DefaultCAPackagePolicy `json:"policy,omitempty"` + Policy *operatorv1alpha1.Mode `json:"policy,omitempty"` } // DefaultCAPackageConfigApplyConfiguration constructs a declarative configuration of the DefaultCAPackageConfig type for use with @@ -27,7 +27,7 @@ func DefaultCAPackageConfig() *DefaultCAPackageConfigApplyConfiguration { // WithPolicy sets the Policy field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the Policy field is set to the value of the last call. -func (b *DefaultCAPackageConfigApplyConfiguration) WithPolicy(value operatorv1alpha1.DefaultCAPackagePolicy) *DefaultCAPackageConfigApplyConfiguration { +func (b *DefaultCAPackageConfigApplyConfiguration) WithPolicy(value operatorv1alpha1.Mode) *DefaultCAPackageConfigApplyConfiguration { b.Policy = &value return b } diff --git a/pkg/operator/applyconfigurations/operator/v1alpha1/trustmanagerconfig.go b/pkg/operator/applyconfigurations/operator/v1alpha1/trustmanagerconfig.go index e7b546b31..bbdf677ac 100644 --- a/pkg/operator/applyconfigurations/operator/v1alpha1/trustmanagerconfig.go +++ b/pkg/operator/applyconfigurations/operator/v1alpha1/trustmanagerconfig.go @@ -30,13 +30,13 @@ type TrustManagerConfigApplyConfiguration struct { // expired certificates from trust bundles before distributing them. // When set to "Enabled", expired certificates are removed from bundles. // When set to "Disabled", expired certificates are included (default behavior). - FilterExpiredCertificates *operatorv1alpha1.FilterExpiredCertificatesPolicy `json:"filterExpiredCertificates,omitempty"` + FilterExpiredCertificates *operatorv1alpha1.Mode `json:"filterExpiredCertificates,omitempty"` // filterNonCACerts controls whether trust-manager filters out // non-CA certificates from trust bundles before distributing them. // When set to "Enabled", only certificates with the X.509 basicConstraints // CA bit set are included in bundles. // When set to "Disabled", non-CA certificates are included (default behavior). - FilterNonCACerts *operatorv1alpha1.FilterNonCACertsPolicy `json:"filterNonCACerts,omitempty"` + FilterNonCACerts *operatorv1alpha1.Mode `json:"filterNonCACerts,omitempty"` // defaultCAPackage configures the default CA package for trust-manager. // When enabled, the operator will use OpenShift's trusted CA bundle injection mechanism. DefaultCAPackage *DefaultCAPackageConfigApplyConfiguration `json:"defaultCAPackage,omitempty"` @@ -52,6 +52,9 @@ type TrustManagerConfigApplyConfiguration struct { // nodeSelector restricts which nodes the trust-manager pod can be scheduled on. // ref: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/ NodeSelector map[string]string `json:"nodeSelector,omitempty"` + // webhookTLS configures the cert-manager Certificate used for the + // trust-manager validating webhook serving certificate. + WebhookTLS *WebhookTLSConfigApplyConfiguration `json:"webhookTLS,omitempty"` } // TrustManagerConfigApplyConfiguration constructs a declarative configuration of the TrustManagerConfig type for use with @@ -95,7 +98,7 @@ func (b *TrustManagerConfigApplyConfiguration) WithSecretTargets(value *SecretTa // WithFilterExpiredCertificates sets the FilterExpiredCertificates field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the FilterExpiredCertificates field is set to the value of the last call. -func (b *TrustManagerConfigApplyConfiguration) WithFilterExpiredCertificates(value operatorv1alpha1.FilterExpiredCertificatesPolicy) *TrustManagerConfigApplyConfiguration { +func (b *TrustManagerConfigApplyConfiguration) WithFilterExpiredCertificates(value operatorv1alpha1.Mode) *TrustManagerConfigApplyConfiguration { b.FilterExpiredCertificates = &value return b } @@ -103,7 +106,7 @@ func (b *TrustManagerConfigApplyConfiguration) WithFilterExpiredCertificates(val // WithFilterNonCACerts sets the FilterNonCACerts field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the FilterNonCACerts field is set to the value of the last call. -func (b *TrustManagerConfigApplyConfiguration) WithFilterNonCACerts(value operatorv1alpha1.FilterNonCACertsPolicy) *TrustManagerConfigApplyConfiguration { +func (b *TrustManagerConfigApplyConfiguration) WithFilterNonCACerts(value operatorv1alpha1.Mode) *TrustManagerConfigApplyConfiguration { b.FilterNonCACerts = &value return b } @@ -155,3 +158,11 @@ func (b *TrustManagerConfigApplyConfiguration) WithNodeSelector(entries map[stri } return b } + +// WithWebhookTLS sets the WebhookTLS field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the WebhookTLS field is set to the value of the last call. +func (b *TrustManagerConfigApplyConfiguration) WithWebhookTLS(value *WebhookTLSConfigApplyConfiguration) *TrustManagerConfigApplyConfiguration { + b.WebhookTLS = value + return b +} diff --git a/pkg/operator/applyconfigurations/operator/v1alpha1/webhooktlsconfig.go b/pkg/operator/applyconfigurations/operator/v1alpha1/webhooktlsconfig.go new file mode 100644 index 000000000..40e954518 --- /dev/null +++ b/pkg/operator/applyconfigurations/operator/v1alpha1/webhooktlsconfig.go @@ -0,0 +1,44 @@ +// Code generated by applyconfiguration-gen. DO NOT EDIT. + +package v1alpha1 + +import ( + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// WebhookTLSConfigApplyConfiguration represents a declarative configuration of the WebhookTLSConfig type for use +// with apply. +// +// WebhookTLSConfig configures the trust-manager webhook TLS certificate. +type WebhookTLSConfigApplyConfiguration struct { + // certificateDuration is the requested validity period of the webhook TLS certificate. + // When unset, cert-manager's default certificate duration is used. + // Example: "8760h" for one year. + CertificateDuration *v1.Duration `json:"certificateDuration,omitempty"` + // approverPolicy configures a CertificateRequestPolicy so that + // cert-manager-approver-policy can auto-approve the webhook CertificateRequest. + // Enable this when approver-policy is installed in the cluster. + ApproverPolicy *ApproverPolicyConfigApplyConfiguration `json:"approverPolicy,omitempty"` +} + +// WebhookTLSConfigApplyConfiguration constructs a declarative configuration of the WebhookTLSConfig type for use with +// apply. +func WebhookTLSConfig() *WebhookTLSConfigApplyConfiguration { + return &WebhookTLSConfigApplyConfiguration{} +} + +// WithCertificateDuration sets the CertificateDuration field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the CertificateDuration field is set to the value of the last call. +func (b *WebhookTLSConfigApplyConfiguration) WithCertificateDuration(value v1.Duration) *WebhookTLSConfigApplyConfiguration { + b.CertificateDuration = &value + return b +} + +// WithApproverPolicy sets the ApproverPolicy field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the ApproverPolicy field is set to the value of the last call. +func (b *WebhookTLSConfigApplyConfiguration) WithApproverPolicy(value *ApproverPolicyConfigApplyConfiguration) *WebhookTLSConfigApplyConfiguration { + b.ApproverPolicy = value + return b +} diff --git a/pkg/operator/applyconfigurations/utils.go b/pkg/operator/applyconfigurations/utils.go index 37cba6978..35cddf01e 100644 --- a/pkg/operator/applyconfigurations/utils.go +++ b/pkg/operator/applyconfigurations/utils.go @@ -16,6 +16,8 @@ import ( func ForKind(kind schema.GroupVersionKind) interface{} { switch kind { // Group=operator.openshift.io, Version=v1alpha1 + case v1alpha1.SchemeGroupVersion.WithKind("ApproverPolicyConfig"): + return &operatorv1alpha1.ApproverPolicyConfigApplyConfiguration{} case v1alpha1.SchemeGroupVersion.WithKind("CertManager"): return &operatorv1alpha1.CertManagerApplyConfiguration{} case v1alpha1.SchemeGroupVersion.WithKind("CertManagerConfig"): @@ -66,6 +68,8 @@ func ForKind(kind schema.GroupVersionKind) interface{} { return &operatorv1alpha1.TrustManagerSpecApplyConfiguration{} case v1alpha1.SchemeGroupVersion.WithKind("TrustManagerStatus"): return &operatorv1alpha1.TrustManagerStatusApplyConfiguration{} + case v1alpha1.SchemeGroupVersion.WithKind("WebhookTLSConfig"): + return &operatorv1alpha1.WebhookTLSConfigApplyConfiguration{} } return nil diff --git a/test/e2e/multiple_operands_test.go b/test/e2e/multiple_operands_test.go index aeb62e74a..73ed0b4d2 100644 --- a/test/e2e/multiple_operands_test.go +++ b/test/e2e/multiple_operands_test.go @@ -25,15 +25,15 @@ import ( "golang.org/x/sync/errgroup" "sigs.k8s.io/yaml" - testutils "github.com/openshift/cert-manager-operator/pkg/controller/istiocsr" "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" + testutils "github.com/openshift/cert-manager-operator/pkg/controller/istiocsr" "github.com/openshift/cert-manager-operator/test/library" 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/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes" @@ -54,8 +54,8 @@ const ( selfSignedCertDNSName = "multi-operand.selfsigned.test.example" bogusIssuerCertDNSName = "multi-operand.bogus-issuer.invalid.example" - multiOperandBundleName = "multi-operand-ca-bundle-secret" - multiOperandBundleSourceCM = "multi-operand-bundle-source" + multiOperandBundleName = "multi-operand-ca-bundle-secret" + multiOperandBundleSourceCM = "multi-operand-bundle-source" multiOperandBundleSourceKey = "ca-bundle.crt" multiOperandBundleTargetKey = "ca-bundle.crt" ) @@ -254,8 +254,8 @@ func multiOperandTrustManagerCR() *trustManagerCRBuilder { return newTrustManagerCR(). WithLabels(map[string]string{"env": "trustmanager-test"}). WithAnnotations(map[string]string{"trustmanager.operator.openshift.io/cluster": "trustmanager-test"}). - WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled)). - WithFilterExpiredCertificates(v1alpha1.FilterExpiredCertificatesPolicy(v1alpha1.Enabled)). + WithDefaultCAPackage(v1alpha1.Enabled). + WithFilterExpiredCertificates(v1alpha1.Enabled). WithSecretTargets(v1alpha1.SecretTargetsPolicyCustom, []string{"ca-bundle-secret", multiOperandBundleName}). WithTrustNamespace(trustManagerNamespace) } @@ -554,9 +554,9 @@ func assertTrustManagerCRConfigPropagation(ctx context.Context, clientset *kuber tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) Expect(err).NotTo(HaveOccurred()) - Expect(tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy).To(Equal(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled))) + Expect(tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy).To(Equal(v1alpha1.Enabled)) Expect(tm.Spec.TrustManagerConfig.SecretTargets.Policy).To(Equal(v1alpha1.SecretTargetsPolicyCustom)) - Expect(tm.Spec.TrustManagerConfig.FilterExpiredCertificates).To(Equal(v1alpha1.FilterExpiredCertificatesPolicy(v1alpha1.Enabled))) + Expect(tm.Spec.TrustManagerConfig.FilterExpiredCertificates).To(Equal(v1alpha1.Enabled)) } func runMultiOperandBundleSecretTargetTest(ctx context.Context, sourcePEM string) { diff --git a/test/e2e/trustmanager_bundle_test.go b/test/e2e/trustmanager_bundle_test.go index 67655a8d1..7316cdeb5 100644 --- a/test/e2e/trustmanager_bundle_test.go +++ b/test/e2e/trustmanager_bundle_test.go @@ -560,7 +560,7 @@ var _ = Describe("Bundle", Ordered, Label("Platform:Generic", "Feature:TrustMana // ===== Group 3: DefaultCAPackage enabled ===== Context("with DefaultCAPackage enabled", Ordered, func() { BeforeAll(func() { - createTrustManager(ctx, newTrustManagerCR().WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled))) + createTrustManager(ctx, newTrustManagerCR().WithDefaultCAPackage(v1alpha1.Enabled)) By("waiting for default CA package ConfigMap to be created") err := pollTillConfigMapAvailable(ctx, k8sClientSet, trustManagerNamespace, defaultCAPackageConfigMapName) @@ -871,7 +871,7 @@ var _ = Describe("Bundle", Ordered, Label("Platform:Generic", "Feature:TrustMana BeforeAll(func() { createTrustManager(ctx, newTrustManagerCR(). WithSecretTargets(v1alpha1.SecretTargetsPolicyCustom, []string{bundleCombined}). - WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled))) + WithDefaultCAPackage(v1alpha1.Enabled)) By("waiting for default CA package ConfigMap to be created") err := pollTillConfigMapAvailable(ctx, k8sClientSet, trustManagerNamespace, defaultCAPackageConfigMapName) @@ -1017,7 +1017,7 @@ var _ = Describe("Bundle", Ordered, Label("Platform:Generic", "Feature:TrustMana BeforeAll(func() { createTrustManager(ctx, newTrustManagerCR(). - WithFilterExpiredCertificates(v1alpha1.FilterExpiredCertificatesPolicy(v1alpha1.Enabled))) + WithFilterExpiredCertificates(v1alpha1.Enabled)) sourceCMName = "filter-src-cm-" + randomStr(5) filterBundleName = "bundle-filter-expired-" + randomStr(5) @@ -1059,7 +1059,7 @@ var _ = Describe("Bundle", Ordered, Label("Platform:Generic", "Feature:TrustMana if err != nil { return err } - tm.Spec.TrustManagerConfig.FilterExpiredCertificates = v1alpha1.FilterExpiredCertificatesPolicy(v1alpha1.Disabled) + tm.Spec.TrustManagerConfig.FilterExpiredCertificates = v1alpha1.Disabled _, err = trustManagerClient().Update(ctx, tm, metav1.UpdateOptions{}) return err }, lowTimeout, fastPollInterval).Should(Succeed()) @@ -1088,7 +1088,7 @@ var _ = Describe("Bundle", Ordered, Label("Platform:Generic", "Feature:TrustMana BeforeAll(func() { createTrustManager(ctx, newTrustManagerCR(). - WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicy(v1alpha1.Enabled))) + WithFilterNonCACerts(v1alpha1.Enabled)) sourceCMName = "filter-nca-src-cm-" + randomStr(5) filterBundleName = "bundle-filter-non-ca-" + randomStr(5) @@ -1130,7 +1130,7 @@ var _ = Describe("Bundle", Ordered, Label("Platform:Generic", "Feature:TrustMana if err != nil { return err } - tm.Spec.TrustManagerConfig.FilterNonCACerts = v1alpha1.FilterNonCACertsPolicy(v1alpha1.Disabled) + tm.Spec.TrustManagerConfig.FilterNonCACerts = v1alpha1.Disabled _, err = trustManagerClient().Update(ctx, tm, metav1.UpdateOptions{}) return err }, lowTimeout, fastPollInterval).Should(Succeed()) diff --git a/test/e2e/trustmanager_helpers_test.go b/test/e2e/trustmanager_helpers_test.go index a84679f71..d19626e38 100644 --- a/test/e2e/trustmanager_helpers_test.go +++ b/test/e2e/trustmanager_helpers_test.go @@ -87,17 +87,17 @@ func (b *trustManagerCRBuilder) WithSecretTargets(policy v1alpha1.SecretTargetsP return b } -func (b *trustManagerCRBuilder) WithDefaultCAPackage(policy v1alpha1.DefaultCAPackagePolicy) *trustManagerCRBuilder { +func (b *trustManagerCRBuilder) WithDefaultCAPackage(policy v1alpha1.Mode) *trustManagerCRBuilder { b.tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy = policy return b } -func (b *trustManagerCRBuilder) WithFilterExpiredCertificates(policy v1alpha1.FilterExpiredCertificatesPolicy) *trustManagerCRBuilder { +func (b *trustManagerCRBuilder) WithFilterExpiredCertificates(policy v1alpha1.Mode) *trustManagerCRBuilder { b.tm.Spec.TrustManagerConfig.FilterExpiredCertificates = policy return b } -func (b *trustManagerCRBuilder) WithFilterNonCACerts(policy v1alpha1.FilterNonCACertsPolicy) *trustManagerCRBuilder { +func (b *trustManagerCRBuilder) WithFilterNonCACerts(policy v1alpha1.Mode) *trustManagerCRBuilder { b.tm.Spec.TrustManagerConfig.FilterNonCACerts = policy return b } diff --git a/test/e2e/trustmanager_test.go b/test/e2e/trustmanager_test.go index b8feba608..0a361362e 100644 --- a/test/e2e/trustmanager_test.go +++ b/test/e2e/trustmanager_test.go @@ -593,7 +593,7 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru It("should add filter-expired-certificates arg when filterExpiredCertificates is Enabled", func() { createTrustManager(ctx, newTrustManagerCR(). - WithFilterExpiredCertificates(v1alpha1.FilterExpiredCertificatesPolicy(v1alpha1.Enabled))) + WithFilterExpiredCertificates(v1alpha1.Enabled)) By("verifying deployment args contain --filter-expired-certificates=true") Eventually(func(g Gomega) { @@ -618,7 +618,7 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru It("should add filter-non-ca-certs arg when filterNonCACerts is Enabled", func() { createTrustManager(ctx, newTrustManagerCR(). - WithFilterNonCACerts(v1alpha1.FilterNonCACertsPolicy(v1alpha1.Enabled))) + WithFilterNonCACerts(v1alpha1.Enabled)) By("verifying deployment args contain --filter-non-ca-certs=true") Eventually(func(g Gomega) { @@ -693,7 +693,7 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru if err != nil { return err } - tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy = v1alpha1.DefaultCAPackagePolicy(v1alpha1.Enabled) + tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy = v1alpha1.Enabled _, err = trustManagerClient().Update(ctx, tm, metav1.UpdateOptions{}) return err }, lowTimeout, fastPollInterval).Should(Succeed()) @@ -775,7 +775,7 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru if err != nil { return err } - tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy = v1alpha1.DefaultCAPackagePolicy(v1alpha1.Disabled) + tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy = v1alpha1.Disabled _, err = trustManagerClient().Update(ctx, tm, metav1.UpdateOptions{}) return err }, lowTimeout, fastPollInterval).Should(Succeed()) From 39f02b70778328eb2027a953defcafccd67565c2 Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Fri, 18 Sep 2026 13:00:19 +0530 Subject: [PATCH 4/7] CM-1367: Refresh generated applyconfigurations and gofmt. Keep clientgen comments in sync with TrustManager API docs so verify-scripts passes, and apply gofmt on files the verify job rewrites. --- .../cert_manager_controller_set_test.go | 4 ++-- pkg/controller/certmanager/console_resources.go | 8 ++++++-- .../certmanager/console_resources_test.go | 16 ++++++++++++---- .../operator/v1alpha1/approverpolicyconfig.go | 11 ++++++++--- .../operator/v1alpha1/webhooktlsconfig.go | 4 +++- 5 files changed, 31 insertions(+), 12 deletions(-) diff --git a/pkg/controller/certmanager/cert_manager_controller_set_test.go b/pkg/controller/certmanager/cert_manager_controller_set_test.go index 87a68b42a..47acbd1f7 100644 --- a/pkg/controller/certmanager/cert_manager_controller_set_test.go +++ b/pkg/controller/certmanager/cert_manager_controller_set_test.go @@ -9,9 +9,9 @@ import ( type stubController struct{} -func (s *stubController) Run(ctx context.Context, workers int) {} +func (s *stubController) Run(ctx context.Context, workers int) {} func (s *stubController) Sync(ctx context.Context, syncContext factory.SyncContext) error { return nil } -func (s *stubController) Name() string { return "stub" } +func (s *stubController) Name() string { return "stub" } func TestToArrayConsoleControllerInclusion(t *testing.T) { stub := &stubController{} diff --git a/pkg/controller/certmanager/console_resources.go b/pkg/controller/certmanager/console_resources.go index a1bbf7331..1653d6ec9 100644 --- a/pkg/controller/certmanager/console_resources.go +++ b/pkg/controller/certmanager/console_resources.go @@ -102,7 +102,9 @@ func (c *consoleResourcesController) sync(ctx context.Context, _ factory.SyncCon yamlClient.Get, yamlClient.Create, yamlClient.Update, func(a, b *consolev1.ConsoleYAMLSample) bool { return equality.Semantic.DeepEqual(a.Spec, b.Spec) }, func(existing, desired *consolev1.ConsoleYAMLSample) *consolev1.ConsoleYAMLSample { - u := existing.DeepCopy(); u.Spec = desired.Spec; return u + u := existing.DeepCopy() + u.Spec = desired.Spec + return u }, ); err != nil { errs = append(errs, fmt.Errorf("failed to apply ConsoleYAMLSample/%s: %w", desired.Name, err)) @@ -115,7 +117,9 @@ func (c *consoleResourcesController) sync(ctx context.Context, _ factory.SyncCon qsClient.Get, qsClient.Create, qsClient.Update, func(a, b *consolev1.ConsoleQuickStart) bool { return equality.Semantic.DeepEqual(a.Spec, b.Spec) }, func(existing, desired *consolev1.ConsoleQuickStart) *consolev1.ConsoleQuickStart { - u := existing.DeepCopy(); u.Spec = desired.Spec; return u + u := existing.DeepCopy() + u.Spec = desired.Spec + return u }, ); err != nil { errs = append(errs, fmt.Errorf("failed to apply ConsoleQuickStart/%s: %w", desired.Name, err)) diff --git a/pkg/controller/certmanager/console_resources_test.go b/pkg/controller/certmanager/console_resources_test.go index e3763acd6..8c18a91c0 100644 --- a/pkg/controller/certmanager/console_resources_test.go +++ b/pkg/controller/certmanager/console_resources_test.go @@ -263,7 +263,9 @@ func TestApplyConsoleResourceCreatesWhenNotFound(t *testing.T) { client.Get, client.Create, client.Update, func(a, b *consolev1.ConsoleYAMLSample) bool { return a.Spec == b.Spec }, func(existing, desired *consolev1.ConsoleYAMLSample) *consolev1.ConsoleYAMLSample { - u := existing.DeepCopy(); u.Spec = desired.Spec; return u + u := existing.DeepCopy() + u.Spec = desired.Spec + return u }, ) if err != nil { @@ -307,7 +309,9 @@ func TestApplyConsoleResourceUpdatesWhenSpecDiffers(t *testing.T) { client.Get, client.Create, client.Update, func(a, b *consolev1.ConsoleQuickStart) bool { return a.Spec.DisplayName == b.Spec.DisplayName }, func(existing, desired *consolev1.ConsoleQuickStart) *consolev1.ConsoleQuickStart { - u := existing.DeepCopy(); u.Spec = desired.Spec; return u + u := existing.DeepCopy() + u.Spec = desired.Spec + return u }, ) if err != nil { @@ -347,7 +351,9 @@ func TestApplyConsoleResourceNoOpWhenSpecsEqual(t *testing.T) { client.Get, client.Create, client.Update, func(a, b *consolev1.ConsoleYAMLSample) bool { return a.Spec.Title == b.Spec.Title }, func(existing, desired *consolev1.ConsoleYAMLSample) *consolev1.ConsoleYAMLSample { - u := existing.DeepCopy(); u.Spec = desired.Spec; return u + u := existing.DeepCopy() + u.Spec = desired.Spec + return u }, ) if err != nil { @@ -436,7 +442,9 @@ func TestApplyConsoleResourceUpdateError(t *testing.T) { client.Get, client.Create, client.Update, func(a, b *consolev1.ConsoleYAMLSample) bool { return a.Spec.Title == b.Spec.Title }, func(existing, desired *consolev1.ConsoleYAMLSample) *consolev1.ConsoleYAMLSample { - u := existing.DeepCopy(); u.Spec = desired.Spec; return u + u := existing.DeepCopy() + u.Spec = desired.Spec + return u }, ) if err == nil { diff --git a/pkg/operator/applyconfigurations/operator/v1alpha1/approverpolicyconfig.go b/pkg/operator/applyconfigurations/operator/v1alpha1/approverpolicyconfig.go index d31b32696..310d9bbd2 100644 --- a/pkg/operator/applyconfigurations/operator/v1alpha1/approverpolicyconfig.go +++ b/pkg/operator/applyconfigurations/operator/v1alpha1/approverpolicyconfig.go @@ -13,9 +13,14 @@ import ( // trust-manager webhook certificate. type ApproverPolicyConfigApplyConfiguration struct { // policy controls whether a CertificateRequestPolicy and the RBAC that - // allows cert-manager to use it are created. - // "Enabled" creates CertificateRequestPolicy trust-manager-policy. - // "Disabled" does not create it (default). + // allows the cert-manager controller ServiceAccount to use it are created. + // "Enabled" creates CertificateRequestPolicy trust-manager-policy (to + // auto-approve the webhook certificate), ClusterRole trust-manager-policy-role, + // and ClusterRoleBinding trust-manager-policy-binding for the cert-manager + // ServiceAccount. Nothing is created unless this is set to Enabled. + // If Enabled while cert-manager-approver-policy is not installed, reconcile + // fails because the CertificateRequestPolicy CRD is missing. + // "Disabled" does not create these resources (default). Policy *operatorv1alpha1.Mode `json:"policy,omitempty"` } diff --git a/pkg/operator/applyconfigurations/operator/v1alpha1/webhooktlsconfig.go b/pkg/operator/applyconfigurations/operator/v1alpha1/webhooktlsconfig.go index 40e954518..b60ac9f55 100644 --- a/pkg/operator/applyconfigurations/operator/v1alpha1/webhooktlsconfig.go +++ b/pkg/operator/applyconfigurations/operator/v1alpha1/webhooktlsconfig.go @@ -17,7 +17,9 @@ type WebhookTLSConfigApplyConfiguration struct { CertificateDuration *v1.Duration `json:"certificateDuration,omitempty"` // approverPolicy configures a CertificateRequestPolicy so that // cert-manager-approver-policy can auto-approve the webhook CertificateRequest. - // Enable this when approver-policy is installed in the cluster. + // Resources are created only when policy is Enabled. If Enabled while the + // CertificateRequestPolicy CRD is not installed, reconciliation fails until + // approver-policy is installed or policy is set to Disabled. ApproverPolicy *ApproverPolicyConfigApplyConfiguration `json:"approverPolicy,omitempty"` } From c6ada8a9272a64ba18c179178ba56819dcc7ed83 Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Fri, 18 Sep 2026 16:39:46 +0530 Subject: [PATCH 5/7] CM-1367: Commit generated bundle CSV RBAC and TrustManager webhookTLS CRD. --- ...anager-operator.clusterserviceversion.yaml | 11 ++++++ .../operator.openshift.io_trustmanagers.yaml | 37 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/bundle/manifests/cert-manager-operator.clusterserviceversion.yaml b/bundle/manifests/cert-manager-operator.clusterserviceversion.yaml index aeb2b2fff..0d4986900 100644 --- a/bundle/manifests/cert-manager-operator.clusterserviceversion.yaml +++ b/bundle/manifests/cert-manager-operator.clusterserviceversion.yaml @@ -680,6 +680,17 @@ spec: - patch - update - watch + - apiGroups: + - policy.cert-manager.io + resources: + - certificaterequestpolicies + verbs: + - create + - get + - list + - patch + - update + - watch - apiGroups: - rbac.authorization.k8s.io resources: diff --git a/bundle/manifests/operator.openshift.io_trustmanagers.yaml b/bundle/manifests/operator.openshift.io_trustmanagers.yaml index 680f0c22c..83ca53fa2 100644 --- a/bundle/manifests/operator.openshift.io_trustmanagers.yaml +++ b/bundle/manifests/operator.openshift.io_trustmanagers.yaml @@ -1234,6 +1234,43 @@ spec: x-kubernetes-validations: - message: trustNamespace is immutable once set rule: oldSelf == '' || self == oldSelf + webhookTLS: + description: |- + webhookTLS configures the cert-manager Certificate used for the + trust-manager validating webhook serving certificate. + properties: + approverPolicy: + description: |- + approverPolicy configures a CertificateRequestPolicy so that + cert-manager-approver-policy can auto-approve the webhook CertificateRequest. + Resources are created only when policy is Enabled. If Enabled while the + CertificateRequestPolicy CRD is not installed, reconciliation fails until + approver-policy is installed or policy is set to Disabled. + properties: + policy: + default: Disabled + description: |- + policy controls whether a CertificateRequestPolicy and the RBAC that + allows the cert-manager controller ServiceAccount to use it are created. + "Enabled" creates CertificateRequestPolicy trust-manager-policy (to + auto-approve the webhook certificate), ClusterRole trust-manager-policy-role, + and ClusterRoleBinding trust-manager-policy-binding for the cert-manager + ServiceAccount. Nothing is created unless this is set to Enabled. + If Enabled while cert-manager-approver-policy is not installed, reconcile + fails because the CertificateRequestPolicy CRD is missing. + "Disabled" does not create these resources (default). + enum: + - Enabled + - Disabled + type: string + type: object + certificateDuration: + description: |- + certificateDuration is the requested validity period of the webhook TLS certificate. + When unset, cert-manager's default certificate duration is used. + Example: "8760h" for one year. + type: string + type: object type: object required: - trustManagerConfig From e9c3956e1831d912c86c5ab6aec30e7f69bc3621 Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Mon, 28 Sep 2026 21:03:46 +0530 Subject: [PATCH 6/7] CM-1367: Fix TrustManager webhook certificate duration and approver-policy usages. Reject invalid certificateDuration values at admission, drop a controller-owned duration when the field is unset, and allow the default key usages so approver-policy can approve the webhook certificate. --- .../trustmanager.testsuite.yaml | 36 +++++++++++++++++++ api/operator/v1alpha1/trustmanager_types.go | 2 ++ .../operator.openshift.io_trustmanagers.yaml | 1 + .../operator.openshift.io_trustmanagers.yaml | 1 + pkg/controller/trustmanager/approverpolicy.go | 3 ++ .../trustmanager/approverpolicy_test.go | 5 +++ pkg/controller/trustmanager/certificates.go | 28 ++++++++++++++- .../trustmanager/certificates_test.go | 19 ++++++++++ 8 files changed, 94 insertions(+), 1 deletion(-) diff --git a/api/operator/v1alpha1/tests/trustmanagers.operator.openshift.io/trustmanager.testsuite.yaml b/api/operator/v1alpha1/tests/trustmanagers.operator.openshift.io/trustmanager.testsuite.yaml index edb326e31..59c4cf31d 100644 --- a/api/operator/v1alpha1/tests/trustmanagers.operator.openshift.io/trustmanager.testsuite.yaml +++ b/api/operator/v1alpha1/tests/trustmanagers.operator.openshift.io/trustmanager.testsuite.yaml @@ -467,6 +467,42 @@ tests: annotations: custom-annotation: custom-value + # ========================================== + # WebhookTLS certificateDuration Tests + # ========================================== + - name: Should create with a valid webhookTLS certificateDuration + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: TrustManager + spec: + trustManagerConfig: + webhookTLS: + certificateDuration: 8760h + expected: | + apiVersion: operator.openshift.io/v1alpha1 + kind: TrustManager + spec: + trustManagerConfig: + logLevel: 1 + logFormat: text + trustNamespace: cert-manager + filterExpiredCertificates: Disabled + filterNonCACerts: Disabled + webhookTLS: + certificateDuration: 8760h + + - name: Should not allow a malformed webhookTLS certificateDuration + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: TrustManager + spec: + trustManagerConfig: + webhookTLS: + certificateDuration: not-a-duration + expectedError: "certificateDuration" + # ========================================== # Immutability Tests (onUpdate) # ========================================== diff --git a/api/operator/v1alpha1/trustmanager_types.go b/api/operator/v1alpha1/trustmanager_types.go index edd12f706..750cd1387 100644 --- a/api/operator/v1alpha1/trustmanager_types.go +++ b/api/operator/v1alpha1/trustmanager_types.go @@ -204,6 +204,8 @@ type WebhookTLSConfig struct { // certificateDuration is the requested validity period of the webhook TLS certificate. // When unset, cert-manager's default certificate duration is used. // Example: "8760h" for one year. + // +kubebuilder:validation:Type=string + // +kubebuilder:validation:Pattern=`^[-+]?(([0-9]+(\.[0-9]*)?|\.[0-9]+)(ns|us|µs|μs|ms|h|m|s))+$|^[-+]?0$` // +kubebuilder:validation:Optional // +optional CertificateDuration *metav1.Duration `json:"certificateDuration,omitempty"` diff --git a/bundle/manifests/operator.openshift.io_trustmanagers.yaml b/bundle/manifests/operator.openshift.io_trustmanagers.yaml index 83ca53fa2..f3f63bdde 100644 --- a/bundle/manifests/operator.openshift.io_trustmanagers.yaml +++ b/bundle/manifests/operator.openshift.io_trustmanagers.yaml @@ -1269,6 +1269,7 @@ spec: certificateDuration is the requested validity period of the webhook TLS certificate. When unset, cert-manager's default certificate duration is used. Example: "8760h" for one year. + pattern: ^[-+]?(([0-9]+(\.[0-9]*)?|\.[0-9]+)(ns|us|µs|μs|ms|h|m|s))+$|^[-+]?0$ type: string type: object type: object diff --git a/config/crd/bases/operator.openshift.io_trustmanagers.yaml b/config/crd/bases/operator.openshift.io_trustmanagers.yaml index 102e8a3bd..1a41f893c 100644 --- a/config/crd/bases/operator.openshift.io_trustmanagers.yaml +++ b/config/crd/bases/operator.openshift.io_trustmanagers.yaml @@ -1269,6 +1269,7 @@ spec: certificateDuration is the requested validity period of the webhook TLS certificate. When unset, cert-manager's default certificate duration is used. Example: "8760h" for one year. + pattern: ^[-+]?(([0-9]+(\.[0-9]*)?|\.[0-9]+)(ns|us|µs|μs|ms|h|m|s))+$|^[-+]?0$ type: string type: object type: object diff --git a/pkg/controller/trustmanager/approverpolicy.go b/pkg/controller/trustmanager/approverpolicy.go index ab9e06b64..7329fd4fb 100644 --- a/pkg/controller/trustmanager/approverpolicy.go +++ b/pkg/controller/trustmanager/approverpolicy.go @@ -96,6 +96,9 @@ func getCertificateRequestPolicyObject(resourceLabels, resourceAnnotations map[s "values": []interface{}{dnsName}, "required": true, }, + // cert-manager defaults an unset Certificate spec.usages to these two values. + // approver-policy treats an omitted allowed.usages list as permitting none. + "usages": []interface{}{"digital signature", "key encipherment"}, }, "selector": map[string]interface{}{ "issuerRef": map[string]interface{}{ diff --git a/pkg/controller/trustmanager/approverpolicy_test.go b/pkg/controller/trustmanager/approverpolicy_test.go index 69c399d8f..8f5f34d1d 100644 --- a/pkg/controller/trustmanager/approverpolicy_test.go +++ b/pkg/controller/trustmanager/approverpolicy_test.go @@ -37,6 +37,11 @@ func TestCertificateRequestPolicyObject(t *testing.T) { if err != nil || !found || cn != expectedDNS { t.Errorf("expected commonName %q, got %q found=%v err=%v", expectedDNS, cn, found, err) } + + usages, found, err := unstructured.NestedStringSlice(obj.Object, "spec", "allowed", "usages") + if err != nil || !found || len(usages) != 2 || usages[0] != "digital signature" || usages[1] != "key encipherment" { + t.Errorf("expected usages [digital signature, key encipherment], got %v found=%v err=%v", usages, found, err) + } } func TestPolicyClusterRoleBindingSubjects(t *testing.T) { diff --git a/pkg/controller/trustmanager/certificates.go b/pkg/controller/trustmanager/certificates.go index 1d4f196bd..8821cfda5 100644 --- a/pkg/controller/trustmanager/certificates.go +++ b/pkg/controller/trustmanager/certificates.go @@ -1,6 +1,7 @@ package trustmanager import ( + "bytes" "fmt" "reflect" "slices" @@ -8,6 +9,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/structured-merge-diff/v6/fieldpath" certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" certmanagermetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" @@ -108,7 +110,10 @@ func issuerModified(desired, existing *certmanagerv1.Issuer) bool { // certificateModified compares only the fields we manage via SSA. // We compare individual spec fields rather than the full Spec because // cert-manager's webhook may default fields we don't set (e.g. Duration). -// Duration is compared only when we explicitly set it on the desired object. +// An explicit duration is compared when set. When it is unset, the live +// duration is applied away only if trust-manager-controller still owns +// spec.duration, so server-side apply drops that field and cert-manager can +// restore its default. A webhook-defaulted duration is left alone. func certificateModified(desired, existing *certmanagerv1.Certificate) bool { if managedMetadataModified(desired, existing) { return true @@ -123,5 +128,26 @@ func certificateModified(desired, existing *certmanagerv1.Certificate) bool { if desired.Spec.Duration != nil && !ptr.Equal(desired.Spec.Duration, existing.Spec.Duration) { return true } + if desired.Spec.Duration == nil && existing.Spec.Duration != nil && controllerOwnsCertificateDuration(existing) { + return true + } + return false +} + +// controllerOwnsCertificateDuration reports whether trust-manager-controller's +// server-side apply managed fields still include spec.duration. +func controllerOwnsCertificateDuration(cert *certmanagerv1.Certificate) bool { + for _, entry := range cert.GetManagedFields() { + if entry.Manager != fieldOwner || entry.Subresource != "" || entry.FieldsV1 == nil || len(entry.FieldsV1.Raw) == 0 { + continue + } + var fields fieldpath.Set + if err := fields.FromJSON(bytes.NewReader(entry.FieldsV1.Raw)); err != nil { + continue + } + if fields.Has(fieldpath.MakePathOrDie("spec", "duration")) { + return true + } + } return false } diff --git a/pkg/controller/trustmanager/certificates_test.go b/pkg/controller/trustmanager/certificates_test.go index 467315ff1..ba057680f 100644 --- a/pkg/controller/trustmanager/certificates_test.go +++ b/pkg/controller/trustmanager/certificates_test.go @@ -428,6 +428,25 @@ func TestCertificateReconciliation(t *testing.T) { wantExistsCount: 1, wantPatchCount: 0, }, + { + name: "apply when explicit duration is removed and controller owns it", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + cert := getCertificateObject(v1alpha1.TrustManagerConfig{}, testResourceLabels(), testResourceAnnotations()) + cert.Spec.Duration = &metav1.Duration{Duration: 8760 * time.Hour} + cert.SetManagedFields([]metav1.ManagedFieldsEntry{{ + Manager: fieldOwner, + Operation: metav1.ManagedFieldsOperationApply, + FieldsType: "FieldsV1", + FieldsV1: &metav1.FieldsV1{Raw: []byte(`{"f:spec":{"f:duration":{}}}`)}, + }}) + cert.DeepCopyInto(obj.(*certmanagerv1.Certificate)) + return true, nil + }) + }, + wantExistsCount: 1, + wantPatchCount: 1, + }, { name: "exists error propagates", preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { From 884a6527f49afcc1c0086801600a08eb75380dae Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Mon, 28 Sep 2026 22:42:11 +0530 Subject: [PATCH 7/7] CM-1367: Add TrustManager webhook cert-manager configuration. Move the webhook certificate duration under webhookTLS.certManager and reconcile issuer, key, renewal, signature, and secret metadata from that struct. --- .../trustmanager.testsuite.yaml | 22 +++- api/operator/v1alpha1/trustmanager_types.go | 77 +++++++++++- .../v1alpha1/zz_generated.deepcopy.go | 37 +++++- .../operator.openshift.io_trustmanagers.yaml | 110 ++++++++++++++++- .../operator.openshift.io_trustmanagers.yaml | 110 ++++++++++++++++- pkg/controller/trustmanager/certificates.go | 104 +++++++++++++--- .../trustmanager/certificates_test.go | 37 ++++++ pkg/controller/trustmanager/test_utils.go | 2 +- .../v1alpha1/trustmanagercertconfig.go | 113 ++++++++++++++++++ .../operator/v1alpha1/webhooktlsconfig.go | 19 ++- pkg/operator/applyconfigurations/utils.go | 2 + 11 files changed, 580 insertions(+), 53 deletions(-) create mode 100644 pkg/operator/applyconfigurations/operator/v1alpha1/trustmanagercertconfig.go diff --git a/api/operator/v1alpha1/tests/trustmanagers.operator.openshift.io/trustmanager.testsuite.yaml b/api/operator/v1alpha1/tests/trustmanagers.operator.openshift.io/trustmanager.testsuite.yaml index 59c4cf31d..bbcdab74a 100644 --- a/api/operator/v1alpha1/tests/trustmanagers.operator.openshift.io/trustmanager.testsuite.yaml +++ b/api/operator/v1alpha1/tests/trustmanagers.operator.openshift.io/trustmanager.testsuite.yaml @@ -478,7 +478,8 @@ tests: spec: trustManagerConfig: webhookTLS: - certificateDuration: 8760h + certManager: + certificateDuration: 8760h expected: | apiVersion: operator.openshift.io/v1alpha1 kind: TrustManager @@ -490,7 +491,8 @@ tests: filterExpiredCertificates: Disabled filterNonCACerts: Disabled webhookTLS: - certificateDuration: 8760h + certManager: + certificateDuration: 8760h - name: Should not allow a malformed webhookTLS certificateDuration resourceName: cluster @@ -500,9 +502,23 @@ tests: spec: trustManagerConfig: webhookTLS: - certificateDuration: not-a-duration + certManager: + certificateDuration: not-a-duration expectedError: "certificateDuration" + - name: Should not allow an RSA private key with an ECDSA size + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: TrustManager + spec: + trustManagerConfig: + webhookTLS: + certManager: + privateKeyAlgorithm: RSA + privateKeySize: 256 + expectedError: "privateKeySize" + # ========================================== # Immutability Tests (onUpdate) # ========================================== diff --git a/api/operator/v1alpha1/trustmanager_types.go b/api/operator/v1alpha1/trustmanager_types.go index 750cd1387..b563d4e38 100644 --- a/api/operator/v1alpha1/trustmanager_types.go +++ b/api/operator/v1alpha1/trustmanager_types.go @@ -3,6 +3,8 @@ package v1alpha1 import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + certmanagermetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" ) func init() { @@ -201,14 +203,11 @@ type SecretTargetsConfig struct { // WebhookTLSConfig configures the trust-manager webhook TLS certificate. type WebhookTLSConfig struct { - // certificateDuration is the requested validity period of the webhook TLS certificate. - // When unset, cert-manager's default certificate duration is used. - // Example: "8760h" for one year. - // +kubebuilder:validation:Type=string - // +kubebuilder:validation:Pattern=`^[-+]?(([0-9]+(\.[0-9]*)?|\.[0-9]+)(ns|us|µs|μs|ms|h|m|s))+$|^[-+]?0$` + // certManager configures the cert-manager Certificate issued for the webhook. + // When unset, the operator uses its self-signed Issuer and cert-manager defaults. // +kubebuilder:validation:Optional // +optional - CertificateDuration *metav1.Duration `json:"certificateDuration,omitempty"` + CertManager TrustManagerCertConfig `json:"certManager,omitempty"` // approverPolicy configures a CertificateRequestPolicy so that // cert-manager-approver-policy can auto-approve the webhook CertificateRequest. @@ -220,6 +219,72 @@ type WebhookTLSConfig struct { ApproverPolicy ApproverPolicyConfig `json:"approverPolicy,omitempty"` } +// TrustManagerCertConfig configures the cert-manager Certificate used for the +// trust-manager webhook serving certificate. +// +kubebuilder:validation:XValidation:rule="!has(self.privateKeySize) || self.privateKeySize == 0 || !has(self.privateKeyAlgorithm) || self.privateKeyAlgorithm == \"Ed25519\" || (self.privateKeyAlgorithm == \"RSA\" && self.privateKeySize in [2048, 4096, 8192]) || (self.privateKeyAlgorithm == \"ECDSA\" && self.privateKeySize in [256, 384, 521])",message="privateKeySize must match privateKeyAlgorithm" +type TrustManagerCertConfig struct { + // certificateDuration is the requested validity period of the webhook TLS certificate. + // When unset, cert-manager's default certificate duration is used. + // Example: "8760h" for one year. + // +kubebuilder:validation:Type=string + // +kubebuilder:validation:Pattern=`^[-+]?(([0-9]+(\.[0-9]*)?|\.[0-9]+)(ns|us|µs|μs|ms|h|m|s))+$|^[-+]?0$` + // +kubebuilder:validation:Optional + // +optional + CertificateDuration *metav1.Duration `json:"certificateDuration,omitempty"` + + // certificateRenewBefore is how long before expiry cert-manager renews the webhook certificate. + // When unset, cert-manager renews at one third of the certificate lifetime. + // +kubebuilder:validation:Type=string + // +kubebuilder:validation:Pattern=`^[-+]?(([0-9]+(\.[0-9]*)?|\.[0-9]+)(ns|us|µs|μs|ms|h|m|s))+$|^[-+]?0$` + // +kubebuilder:validation:Optional + // +optional + CertificateRenewBefore *metav1.Duration `json:"certificateRenewBefore,omitempty"` + + // issuerRef overrides the Issuer used for the webhook certificate. + // When unset, the operator uses the self-signed Issuer it creates. + // kind must be Issuer or ClusterIssuer. group must be cert-manager.io. + // +kubebuilder:validation:XValidation:rule="self.kind.lowerAscii() == 'issuer' || self.kind.lowerAscii() == 'clusterissuer'",message="kind must be either 'Issuer' or 'ClusterIssuer'" + // +kubebuilder:validation:XValidation:rule="self.group.lowerAscii() == 'cert-manager.io'",message="group must be 'cert-manager.io'" + // +kubebuilder:validation:Optional + // +optional + IssuerRef *certmanagermetav1.ObjectReference `json:"issuerRef,omitempty"` + + // privateKeyAlgorithm is the private key algorithm for the webhook certificate. + // Allowed values are RSA, ECDSA, and Ed25519. + // +kubebuilder:validation:Enum=RSA;ECDSA;Ed25519 + // +kubebuilder:validation:Optional + // +optional + PrivateKeyAlgorithm string `json:"privateKeyAlgorithm,omitempty"` + + // privateKeyRotationPolicy controls whether a new private key is generated on re-issuance. + // Allowed values are Always and Never. + // +kubebuilder:validation:Enum=Always;Never + // +kubebuilder:validation:Optional + // +optional + PrivateKeyRotationPolicy string `json:"privateKeyRotationPolicy,omitempty"` + + // privateKeySize is the private key size for the webhook certificate. + // RSA allows 2048, 4096, and 8192. ECDSA allows 256, 384, and 521. Ed25519 ignores this field. + // +kubebuilder:validation:Enum=256;384;521;2048;4096;8192 + // +kubebuilder:validation:Optional + // +optional + PrivateKeySize int32 `json:"privateKeySize,omitempty"` + + // certificateSignatureAlgorithm is the signature algorithm for the webhook certificate. + // +kubebuilder:validation:Enum=SHA256WithRSA;SHA384WithRSA;SHA512WithRSA;ECDSAWithSHA256;ECDSAWithSHA384;ECDSAWithSHA512;PureEd25519 + // +kubebuilder:validation:Optional + // +optional + CertificateSignatureAlgorithm string `json:"certificateSignatureAlgorithm,omitempty"` + + // propagateMetadataToSecret copies the labels and annotations the operator + // sets on the webhook Certificate onto that Certificate's Secret. + // "Enabled" copies them. "Disabled" does not (default). + // +kubebuilder:validation:Enum=Enabled;Disabled + // +kubebuilder:validation:Optional + // +optional + PropagateMetadataToSecret Mode `json:"propagateMetadataToSecret,omitempty"` +} + // ApproverPolicyConfig controls creation of a CertificateRequestPolicy for the // trust-manager webhook certificate. type ApproverPolicyConfig struct { diff --git a/api/operator/v1alpha1/zz_generated.deepcopy.go b/api/operator/v1alpha1/zz_generated.deepcopy.go index 2ead4f696..97e5edf1a 100644 --- a/api/operator/v1alpha1/zz_generated.deepcopy.go +++ b/api/operator/v1alpha1/zz_generated.deepcopy.go @@ -21,6 +21,7 @@ limitations under the License. package v1alpha1 import ( + apismetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -627,6 +628,36 @@ func (in *TrustManager) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TrustManagerCertConfig) DeepCopyInto(out *TrustManagerCertConfig) { + *out = *in + if in.CertificateDuration != nil { + in, out := &in.CertificateDuration, &out.CertificateDuration + *out = new(metav1.Duration) + **out = **in + } + if in.CertificateRenewBefore != nil { + in, out := &in.CertificateRenewBefore, &out.CertificateRenewBefore + *out = new(metav1.Duration) + **out = **in + } + if in.IssuerRef != nil { + in, out := &in.IssuerRef, &out.IssuerRef + *out = new(apismetav1.ObjectReference) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TrustManagerCertConfig. +func (in *TrustManagerCertConfig) DeepCopy() *TrustManagerCertConfig { + if in == nil { + return nil + } + out := new(TrustManagerCertConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TrustManagerConfig) DeepCopyInto(out *TrustManagerConfig) { *out = *in @@ -840,11 +871,7 @@ func (in *UnsupportedConfigOverridesForCertManagerWebhook) DeepCopy() *Unsupport // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *WebhookTLSConfig) DeepCopyInto(out *WebhookTLSConfig) { *out = *in - if in.CertificateDuration != nil { - in, out := &in.CertificateDuration, &out.CertificateDuration - *out = new(metav1.Duration) - **out = **in - } + in.CertManager.DeepCopyInto(&out.CertManager) out.ApproverPolicy = in.ApproverPolicy } diff --git a/bundle/manifests/operator.openshift.io_trustmanagers.yaml b/bundle/manifests/operator.openshift.io_trustmanagers.yaml index f3f63bdde..b661d1634 100644 --- a/bundle/manifests/operator.openshift.io_trustmanagers.yaml +++ b/bundle/manifests/operator.openshift.io_trustmanagers.yaml @@ -1264,13 +1264,111 @@ spec: - Disabled type: string type: object - certificateDuration: + certManager: description: |- - certificateDuration is the requested validity period of the webhook TLS certificate. - When unset, cert-manager's default certificate duration is used. - Example: "8760h" for one year. - pattern: ^[-+]?(([0-9]+(\.[0-9]*)?|\.[0-9]+)(ns|us|µs|μs|ms|h|m|s))+$|^[-+]?0$ - type: string + certManager configures the cert-manager Certificate issued for the webhook. + When unset, the operator uses its self-signed Issuer and cert-manager defaults. + properties: + certificateDuration: + description: |- + certificateDuration is the requested validity period of the webhook TLS certificate. + When unset, cert-manager's default certificate duration is used. + Example: "8760h" for one year. + pattern: ^[-+]?(([0-9]+(\.[0-9]*)?|\.[0-9]+)(ns|us|µs|μs|ms|h|m|s))+$|^[-+]?0$ + type: string + certificateRenewBefore: + description: |- + certificateRenewBefore is how long before expiry cert-manager renews the webhook certificate. + When unset, cert-manager renews at one third of the certificate lifetime. + pattern: ^[-+]?(([0-9]+(\.[0-9]*)?|\.[0-9]+)(ns|us|µs|μs|ms|h|m|s))+$|^[-+]?0$ + type: string + certificateSignatureAlgorithm: + description: certificateSignatureAlgorithm is the signature + algorithm for the webhook certificate. + enum: + - SHA256WithRSA + - SHA384WithRSA + - SHA512WithRSA + - ECDSAWithSHA256 + - ECDSAWithSHA384 + - ECDSAWithSHA512 + - PureEd25519 + type: string + issuerRef: + description: |- + issuerRef overrides the Issuer used for the webhook certificate. + When unset, the operator uses the self-signed Issuer it creates. + kind must be Issuer or ClusterIssuer. group must be cert-manager.io. + properties: + group: + description: |- + Group of the issuer being referred to. + Defaults to 'cert-manager.io'. + type: string + kind: + description: |- + Kind of the issuer being referred to. + Defaults to 'Issuer'. + type: string + name: + description: Name of the issuer being referred to. + type: string + required: + - name + type: object + x-kubernetes-validations: + - message: kind must be either 'Issuer' or 'ClusterIssuer' + rule: self.kind.lowerAscii() == 'issuer' || self.kind.lowerAscii() + == 'clusterissuer' + - message: group must be 'cert-manager.io' + rule: self.group.lowerAscii() == 'cert-manager.io' + privateKeyAlgorithm: + description: |- + privateKeyAlgorithm is the private key algorithm for the webhook certificate. + Allowed values are RSA, ECDSA, and Ed25519. + enum: + - RSA + - ECDSA + - Ed25519 + type: string + privateKeyRotationPolicy: + description: |- + privateKeyRotationPolicy controls whether a new private key is generated on re-issuance. + Allowed values are Always and Never. + enum: + - Always + - Never + type: string + privateKeySize: + description: |- + privateKeySize is the private key size for the webhook certificate. + RSA allows 2048, 4096, and 8192. ECDSA allows 256, 384, and 521. Ed25519 ignores this field. + enum: + - 256 + - 384 + - 521 + - 2048 + - 4096 + - 8192 + format: int32 + type: integer + propagateMetadataToSecret: + description: |- + propagateMetadataToSecret copies the labels and annotations the operator + sets on the webhook Certificate onto that Certificate's Secret. + "Enabled" copies them. "Disabled" does not (default). + enum: + - Enabled + - Disabled + type: string + type: object + x-kubernetes-validations: + - message: privateKeySize must match privateKeyAlgorithm + rule: '!has(self.privateKeySize) || self.privateKeySize + == 0 || !has(self.privateKeyAlgorithm) || self.privateKeyAlgorithm + == "Ed25519" || (self.privateKeyAlgorithm == "RSA" && + self.privateKeySize in [2048, 4096, 8192]) || (self.privateKeyAlgorithm + == "ECDSA" && self.privateKeySize in [256, 384, 521])' type: object type: object required: diff --git a/config/crd/bases/operator.openshift.io_trustmanagers.yaml b/config/crd/bases/operator.openshift.io_trustmanagers.yaml index 1a41f893c..b02614988 100644 --- a/config/crd/bases/operator.openshift.io_trustmanagers.yaml +++ b/config/crd/bases/operator.openshift.io_trustmanagers.yaml @@ -1264,13 +1264,111 @@ spec: - Disabled type: string type: object - certificateDuration: + certManager: description: |- - certificateDuration is the requested validity period of the webhook TLS certificate. - When unset, cert-manager's default certificate duration is used. - Example: "8760h" for one year. - pattern: ^[-+]?(([0-9]+(\.[0-9]*)?|\.[0-9]+)(ns|us|µs|μs|ms|h|m|s))+$|^[-+]?0$ - type: string + certManager configures the cert-manager Certificate issued for the webhook. + When unset, the operator uses its self-signed Issuer and cert-manager defaults. + properties: + certificateDuration: + description: |- + certificateDuration is the requested validity period of the webhook TLS certificate. + When unset, cert-manager's default certificate duration is used. + Example: "8760h" for one year. + pattern: ^[-+]?(([0-9]+(\.[0-9]*)?|\.[0-9]+)(ns|us|µs|μs|ms|h|m|s))+$|^[-+]?0$ + type: string + certificateRenewBefore: + description: |- + certificateRenewBefore is how long before expiry cert-manager renews the webhook certificate. + When unset, cert-manager renews at one third of the certificate lifetime. + pattern: ^[-+]?(([0-9]+(\.[0-9]*)?|\.[0-9]+)(ns|us|µs|μs|ms|h|m|s))+$|^[-+]?0$ + type: string + certificateSignatureAlgorithm: + description: certificateSignatureAlgorithm is the signature + algorithm for the webhook certificate. + enum: + - SHA256WithRSA + - SHA384WithRSA + - SHA512WithRSA + - ECDSAWithSHA256 + - ECDSAWithSHA384 + - ECDSAWithSHA512 + - PureEd25519 + type: string + issuerRef: + description: |- + issuerRef overrides the Issuer used for the webhook certificate. + When unset, the operator uses the self-signed Issuer it creates. + kind must be Issuer or ClusterIssuer. group must be cert-manager.io. + properties: + group: + description: |- + Group of the issuer being referred to. + Defaults to 'cert-manager.io'. + type: string + kind: + description: |- + Kind of the issuer being referred to. + Defaults to 'Issuer'. + type: string + name: + description: Name of the issuer being referred to. + type: string + required: + - name + type: object + x-kubernetes-validations: + - message: kind must be either 'Issuer' or 'ClusterIssuer' + rule: self.kind.lowerAscii() == 'issuer' || self.kind.lowerAscii() + == 'clusterissuer' + - message: group must be 'cert-manager.io' + rule: self.group.lowerAscii() == 'cert-manager.io' + privateKeyAlgorithm: + description: |- + privateKeyAlgorithm is the private key algorithm for the webhook certificate. + Allowed values are RSA, ECDSA, and Ed25519. + enum: + - RSA + - ECDSA + - Ed25519 + type: string + privateKeyRotationPolicy: + description: |- + privateKeyRotationPolicy controls whether a new private key is generated on re-issuance. + Allowed values are Always and Never. + enum: + - Always + - Never + type: string + privateKeySize: + description: |- + privateKeySize is the private key size for the webhook certificate. + RSA allows 2048, 4096, and 8192. ECDSA allows 256, 384, and 521. Ed25519 ignores this field. + enum: + - 256 + - 384 + - 521 + - 2048 + - 4096 + - 8192 + format: int32 + type: integer + propagateMetadataToSecret: + description: |- + propagateMetadataToSecret copies the labels and annotations the operator + sets on the webhook Certificate onto that Certificate's Secret. + "Enabled" copies them. "Disabled" does not (default). + enum: + - Enabled + - Disabled + type: string + type: object + x-kubernetes-validations: + - message: privateKeySize must match privateKeyAlgorithm + rule: '!has(self.privateKeySize) || self.privateKeySize + == 0 || !has(self.privateKeyAlgorithm) || self.privateKeyAlgorithm + == "Ed25519" || (self.privateKeyAlgorithm == "RSA" && + self.privateKeySize in [2048, 4096, 8192]) || (self.privateKeyAlgorithm + == "ECDSA" && self.privateKeySize in [256, 384, 521])' type: object type: object required: diff --git a/pkg/controller/trustmanager/certificates.go b/pkg/controller/trustmanager/certificates.go index 8821cfda5..a9d90b3ef 100644 --- a/pkg/controller/trustmanager/certificates.go +++ b/pkg/controller/trustmanager/certificates.go @@ -3,6 +3,7 @@ package trustmanager import ( "bytes" "fmt" + "maps" "reflect" "slices" @@ -89,16 +90,57 @@ func getCertificateObject(config v1alpha1.TrustManagerConfig, resourceLabels, re certificate.Spec.CommonName = dnsName certificate.Spec.DNSNames = []string{dnsName} certificate.Spec.SecretName = trustManagerTLSSecretName + applyWebhookCertConfig(certificate, config.WebhookTLS.CertManager, resourceLabels, resourceAnnotations) + + return certificate +} + +// applyWebhookCertConfig copies webhookTLS.certManager onto the Certificate. +// Unset fields keep the operator default (self-signed Issuer) or are left for cert-manager to default. +func applyWebhookCertConfig(certificate *certmanagerv1.Certificate, certConfig v1alpha1.TrustManagerCertConfig, resourceLabels, resourceAnnotations map[string]string) { certificate.Spec.IssuerRef = certmanagermetav1.ObjectReference{ Name: trustManagerIssuerName, Kind: "Issuer", Group: "cert-manager.io", } - if config.WebhookTLS.CertificateDuration != nil { - certificate.Spec.Duration = config.WebhookTLS.CertificateDuration + if certConfig.IssuerRef != nil { + certificate.Spec.IssuerRef = *certConfig.IssuerRef + } + if certConfig.CertificateDuration != nil { + certificate.Spec.Duration = certConfig.CertificateDuration + } + if certConfig.CertificateRenewBefore != nil { + certificate.Spec.RenewBefore = certConfig.CertificateRenewBefore + } + if certConfig.CertificateSignatureAlgorithm != "" { + certificate.Spec.SignatureAlgorithm = certmanagerv1.SignatureAlgorithm(certConfig.CertificateSignatureAlgorithm) + } + if privateKey := privateKeyFromCertConfig(certConfig); privateKey != nil { + certificate.Spec.PrivateKey = privateKey } + if certConfig.PropagateMetadataToSecret == v1alpha1.Enabled { + certificate.Spec.SecretTemplate = &certmanagerv1.CertificateSecretTemplate{ + Labels: maps.Clone(resourceLabels), + Annotations: maps.Clone(resourceAnnotations), + } + } +} - return certificate +func privateKeyFromCertConfig(certConfig v1alpha1.TrustManagerCertConfig) *certmanagerv1.CertificatePrivateKey { + if certConfig.PrivateKeyAlgorithm == "" && certConfig.PrivateKeyRotationPolicy == "" && certConfig.PrivateKeySize == 0 { + return nil + } + privateKey := &certmanagerv1.CertificatePrivateKey{} + if certConfig.PrivateKeyAlgorithm != "" { + privateKey.Algorithm = certmanagerv1.PrivateKeyAlgorithm(certConfig.PrivateKeyAlgorithm) + } + if certConfig.PrivateKeyRotationPolicy != "" { + privateKey.RotationPolicy = certmanagerv1.PrivateKeyRotationPolicy(certConfig.PrivateKeyRotationPolicy) + } + if certConfig.PrivateKeySize != 0 { + privateKey.Size = int(certConfig.PrivateKeySize) + } + return privateKey } // issuerModified compares only the fields we manage via SSA. @@ -109,11 +151,12 @@ func issuerModified(desired, existing *certmanagerv1.Issuer) bool { // certificateModified compares only the fields we manage via SSA. // We compare individual spec fields rather than the full Spec because -// cert-manager's webhook may default fields we don't set (e.g. Duration). -// An explicit duration is compared when set. When it is unset, the live -// duration is applied away only if trust-manager-controller still owns -// spec.duration, so server-side apply drops that field and cert-manager can -// restore its default. A webhook-defaulted duration is left alone. +// cert-manager's webhook may default fields we don't set. +// A field webhookTLS.certManager sets is drift when it differs. A field it +// leaves unset is applied away only while trust-manager-controller still owns +// it, so server-side apply drops that field and cert-manager can restore its +// default. A webhook-defaulted value is left alone. IssuerRef is always set, +// either from certManager.issuerRef or the operator's self-signed Issuer. func certificateModified(desired, existing *certmanagerv1.Certificate) bool { if managedMetadataModified(desired, existing) { return true @@ -125,18 +168,43 @@ func certificateModified(desired, existing *certmanagerv1.Certificate) bool { !reflect.DeepEqual(desired.Spec.IssuerRef, existing.Spec.IssuerRef) { return true } - if desired.Spec.Duration != nil && !ptr.Equal(desired.Spec.Duration, existing.Spec.Duration) { + if certificateFieldDrift(desired.Spec.Duration != nil, ptr.Equal(desired.Spec.Duration, existing.Spec.Duration), existing, "spec", "duration") { + return true + } + if certificateFieldDrift(desired.Spec.RenewBefore != nil, ptr.Equal(desired.Spec.RenewBefore, existing.Spec.RenewBefore), existing, "spec", "renewBefore") { + return true + } + if certificateFieldDrift(desired.Spec.PrivateKey != nil, reflect.DeepEqual(desired.Spec.PrivateKey, existing.Spec.PrivateKey), existing, "spec", "privateKey") { + return true + } + if certificateFieldDrift(desired.Spec.SignatureAlgorithm != "", desired.Spec.SignatureAlgorithm == existing.Spec.SignatureAlgorithm, existing, "spec", "signatureAlgorithm") { return true } - if desired.Spec.Duration == nil && existing.Spec.Duration != nil && controllerOwnsCertificateDuration(existing) { + if certificateFieldDrift(desired.Spec.SecretTemplate != nil, reflect.DeepEqual(desired.Spec.SecretTemplate, existing.Spec.SecretTemplate), existing, "spec", "secretTemplate") { return true } return false } -// controllerOwnsCertificateDuration reports whether trust-manager-controller's -// server-side apply managed fields still include spec.duration. -func controllerOwnsCertificateDuration(cert *certmanagerv1.Certificate) bool { +// certificateFieldDrift reports whether a Certificate field should be applied. +// A field the desired object sets is drift when it differs. A field the desired +// object leaves unset is drift only while trust-manager-controller still owns it, +// so server-side apply can drop it and cert-manager can restore its default. +func certificateFieldDrift(desiredSet, equal bool, existing *certmanagerv1.Certificate, parts ...string) bool { + if desiredSet { + return !equal + } + return !equal && controllerOwnsField(existing, parts...) +} + +// controllerOwnsField reports whether trust-manager-controller's server-side apply +// managed fields include path or one of its children. +func controllerOwnsField(cert *certmanagerv1.Certificate, parts ...string) bool { + pathElems := make([]any, len(parts)) + for i, part := range parts { + pathElems[i] = part + } + path := fieldpath.MakePathOrDie(pathElems...) for _, entry := range cert.GetManagedFields() { if entry.Manager != fieldOwner || entry.Subresource != "" || entry.FieldsV1 == nil || len(entry.FieldsV1.Raw) == 0 { continue @@ -145,7 +213,15 @@ func controllerOwnsCertificateDuration(cert *certmanagerv1.Certificate) bool { if err := fields.FromJSON(bytes.NewReader(entry.FieldsV1.Raw)); err != nil { continue } - if fields.Has(fieldpath.MakePathOrDie("spec", "duration")) { + if fields.Has(path) { + return true + } + subset := &fields + for _, part := range parts { + name := part + subset = subset.WithPrefix(fieldpath.PathElement{FieldName: &name}) + } + if subset != nil && !subset.Empty() { return true } } diff --git a/pkg/controller/trustmanager/certificates_test.go b/pkg/controller/trustmanager/certificates_test.go index ba057680f..8ed1472be 100644 --- a/pkg/controller/trustmanager/certificates_test.go +++ b/pkg/controller/trustmanager/certificates_test.go @@ -184,6 +184,43 @@ func TestCertificateDuration(t *testing.T) { } } +func TestWebhookCertConfig(t *testing.T) { + tm := testTrustManager().Build() + labels := getResourceLabels(tm) + annotations := getResourceAnnotations(tm) + tm.Spec.TrustManagerConfig.WebhookTLS.CertManager = v1alpha1.TrustManagerCertConfig{ + CertificateDuration: &metav1.Duration{Duration: 8760 * time.Hour}, + CertificateRenewBefore: &metav1.Duration{Duration: 360 * time.Hour}, + IssuerRef: &certmanagermetav1.ObjectReference{Name: "custom-issuer", Kind: "ClusterIssuer", Group: "cert-manager.io"}, + PrivateKeyAlgorithm: "ECDSA", + PrivateKeyRotationPolicy: "Always", + PrivateKeySize: 256, + CertificateSignatureAlgorithm: "ECDSAWithSHA256", + PropagateMetadataToSecret: v1alpha1.Enabled, + } + + cert := getCertificateObject(tm.Spec.TrustManagerConfig, labels, annotations) + if cert.Spec.IssuerRef.Name != "custom-issuer" || cert.Spec.IssuerRef.Kind != "ClusterIssuer" { + t.Errorf("expected custom issuer, got %+v", cert.Spec.IssuerRef) + } + if cert.Spec.RenewBefore == nil || cert.Spec.RenewBefore.Duration != 360*time.Hour { + t.Errorf("expected renewBefore 360h, got %+v", cert.Spec.RenewBefore) + } + if cert.Spec.PrivateKey == nil || cert.Spec.PrivateKey.Algorithm != certmanagerv1.ECDSAKeyAlgorithm || cert.Spec.PrivateKey.Size != 256 || cert.Spec.PrivateKey.RotationPolicy != certmanagerv1.RotationPolicyAlways { + t.Errorf("expected ECDSA private key, got %+v", cert.Spec.PrivateKey) + } + if cert.Spec.SignatureAlgorithm != certmanagerv1.ECDSAWithSHA256 { + t.Errorf("expected signature algorithm ECDSAWithSHA256, got %q", cert.Spec.SignatureAlgorithm) + } + if cert.Spec.SecretTemplate == nil || cert.Spec.SecretTemplate.Labels["app.kubernetes.io/instance"] != labels["app.kubernetes.io/instance"] { + t.Errorf("expected secret template labels copied, got %+v", cert.Spec.SecretTemplate) + } + labels["app.kubernetes.io/instance"] = "changed-after-copy" + if cert.Spec.SecretTemplate.Labels["app.kubernetes.io/instance"] == "changed-after-copy" { + t.Errorf("secret template labels share the operator label map") + } +} + func TestIssuerReconciliation(t *testing.T) { tests := []struct { name string diff --git a/pkg/controller/trustmanager/test_utils.go b/pkg/controller/trustmanager/test_utils.go index 031666ab5..5f3786f4d 100644 --- a/pkg/controller/trustmanager/test_utils.go +++ b/pkg/controller/trustmanager/test_utils.go @@ -109,7 +109,7 @@ func (b *trustManagerBuilder) WithSecretTargets(policy v1alpha1.SecretTargetsPol } func (b *trustManagerBuilder) WithWebhookCertificateDuration(d time.Duration) *trustManagerBuilder { - b.Spec.TrustManagerConfig.WebhookTLS.CertificateDuration = &metav1.Duration{Duration: d} + b.Spec.TrustManagerConfig.WebhookTLS.CertManager.CertificateDuration = &metav1.Duration{Duration: d} return b } diff --git a/pkg/operator/applyconfigurations/operator/v1alpha1/trustmanagercertconfig.go b/pkg/operator/applyconfigurations/operator/v1alpha1/trustmanagercertconfig.go new file mode 100644 index 000000000..7e10ee658 --- /dev/null +++ b/pkg/operator/applyconfigurations/operator/v1alpha1/trustmanagercertconfig.go @@ -0,0 +1,113 @@ +// Code generated by applyconfiguration-gen. DO NOT EDIT. + +package v1alpha1 + +import ( + metav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" + operatorv1alpha1 "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// TrustManagerCertConfigApplyConfiguration represents a declarative configuration of the TrustManagerCertConfig type for use +// with apply. +// +// TrustManagerCertConfig configures the cert-manager Certificate used for the +// trust-manager webhook serving certificate. +type TrustManagerCertConfigApplyConfiguration struct { + // certificateDuration is the requested validity period of the webhook TLS certificate. + // When unset, cert-manager's default certificate duration is used. + // Example: "8760h" for one year. + CertificateDuration *v1.Duration `json:"certificateDuration,omitempty"` + // certificateRenewBefore is how long before expiry cert-manager renews the webhook certificate. + // When unset, cert-manager renews at one third of the certificate lifetime. + CertificateRenewBefore *v1.Duration `json:"certificateRenewBefore,omitempty"` + // issuerRef overrides the Issuer used for the webhook certificate. + // When unset, the operator uses the self-signed Issuer it creates. + // kind must be Issuer or ClusterIssuer. group must be cert-manager.io. + IssuerRef *metav1.IssuerReference `json:"issuerRef,omitempty"` + // privateKeyAlgorithm is the private key algorithm for the webhook certificate. + // Allowed values are RSA, ECDSA, and Ed25519. + PrivateKeyAlgorithm *string `json:"privateKeyAlgorithm,omitempty"` + // privateKeyRotationPolicy controls whether a new private key is generated on re-issuance. + // Allowed values are Always and Never. + PrivateKeyRotationPolicy *string `json:"privateKeyRotationPolicy,omitempty"` + // privateKeySize is the private key size for the webhook certificate. + // RSA allows 2048, 4096, and 8192. ECDSA allows 256, 384, and 521. Ed25519 ignores this field. + PrivateKeySize *int32 `json:"privateKeySize,omitempty"` + // certificateSignatureAlgorithm is the signature algorithm for the webhook certificate. + CertificateSignatureAlgorithm *string `json:"certificateSignatureAlgorithm,omitempty"` + // propagateMetadataToSecret copies the labels and annotations the operator + // sets on the webhook Certificate onto that Certificate's Secret. + // "Enabled" copies them. "Disabled" does not (default). + PropagateMetadataToSecret *operatorv1alpha1.Mode `json:"propagateMetadataToSecret,omitempty"` +} + +// TrustManagerCertConfigApplyConfiguration constructs a declarative configuration of the TrustManagerCertConfig type for use with +// apply. +func TrustManagerCertConfig() *TrustManagerCertConfigApplyConfiguration { + return &TrustManagerCertConfigApplyConfiguration{} +} + +// WithCertificateDuration sets the CertificateDuration field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the CertificateDuration field is set to the value of the last call. +func (b *TrustManagerCertConfigApplyConfiguration) WithCertificateDuration(value v1.Duration) *TrustManagerCertConfigApplyConfiguration { + b.CertificateDuration = &value + return b +} + +// WithCertificateRenewBefore sets the CertificateRenewBefore field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the CertificateRenewBefore field is set to the value of the last call. +func (b *TrustManagerCertConfigApplyConfiguration) WithCertificateRenewBefore(value v1.Duration) *TrustManagerCertConfigApplyConfiguration { + b.CertificateRenewBefore = &value + return b +} + +// WithIssuerRef sets the IssuerRef field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the IssuerRef field is set to the value of the last call. +func (b *TrustManagerCertConfigApplyConfiguration) WithIssuerRef(value metav1.IssuerReference) *TrustManagerCertConfigApplyConfiguration { + b.IssuerRef = &value + return b +} + +// WithPrivateKeyAlgorithm sets the PrivateKeyAlgorithm field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the PrivateKeyAlgorithm field is set to the value of the last call. +func (b *TrustManagerCertConfigApplyConfiguration) WithPrivateKeyAlgorithm(value string) *TrustManagerCertConfigApplyConfiguration { + b.PrivateKeyAlgorithm = &value + return b +} + +// WithPrivateKeyRotationPolicy sets the PrivateKeyRotationPolicy field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the PrivateKeyRotationPolicy field is set to the value of the last call. +func (b *TrustManagerCertConfigApplyConfiguration) WithPrivateKeyRotationPolicy(value string) *TrustManagerCertConfigApplyConfiguration { + b.PrivateKeyRotationPolicy = &value + return b +} + +// WithPrivateKeySize sets the PrivateKeySize field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the PrivateKeySize field is set to the value of the last call. +func (b *TrustManagerCertConfigApplyConfiguration) WithPrivateKeySize(value int32) *TrustManagerCertConfigApplyConfiguration { + b.PrivateKeySize = &value + return b +} + +// WithCertificateSignatureAlgorithm sets the CertificateSignatureAlgorithm field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the CertificateSignatureAlgorithm field is set to the value of the last call. +func (b *TrustManagerCertConfigApplyConfiguration) WithCertificateSignatureAlgorithm(value string) *TrustManagerCertConfigApplyConfiguration { + b.CertificateSignatureAlgorithm = &value + return b +} + +// WithPropagateMetadataToSecret sets the PropagateMetadataToSecret field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the PropagateMetadataToSecret field is set to the value of the last call. +func (b *TrustManagerCertConfigApplyConfiguration) WithPropagateMetadataToSecret(value operatorv1alpha1.Mode) *TrustManagerCertConfigApplyConfiguration { + b.PropagateMetadataToSecret = &value + return b +} diff --git a/pkg/operator/applyconfigurations/operator/v1alpha1/webhooktlsconfig.go b/pkg/operator/applyconfigurations/operator/v1alpha1/webhooktlsconfig.go index b60ac9f55..a192c626d 100644 --- a/pkg/operator/applyconfigurations/operator/v1alpha1/webhooktlsconfig.go +++ b/pkg/operator/applyconfigurations/operator/v1alpha1/webhooktlsconfig.go @@ -2,19 +2,14 @@ package v1alpha1 -import ( - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - // WebhookTLSConfigApplyConfiguration represents a declarative configuration of the WebhookTLSConfig type for use // with apply. // // WebhookTLSConfig configures the trust-manager webhook TLS certificate. type WebhookTLSConfigApplyConfiguration struct { - // certificateDuration is the requested validity period of the webhook TLS certificate. - // When unset, cert-manager's default certificate duration is used. - // Example: "8760h" for one year. - CertificateDuration *v1.Duration `json:"certificateDuration,omitempty"` + // certManager configures the cert-manager Certificate issued for the webhook. + // When unset, the operator uses its self-signed Issuer and cert-manager defaults. + CertManager *TrustManagerCertConfigApplyConfiguration `json:"certManager,omitempty"` // approverPolicy configures a CertificateRequestPolicy so that // cert-manager-approver-policy can auto-approve the webhook CertificateRequest. // Resources are created only when policy is Enabled. If Enabled while the @@ -29,11 +24,11 @@ func WebhookTLSConfig() *WebhookTLSConfigApplyConfiguration { return &WebhookTLSConfigApplyConfiguration{} } -// WithCertificateDuration sets the CertificateDuration field in the declarative configuration to the given value +// WithCertManager sets the CertManager field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. -// If called multiple times, the CertificateDuration field is set to the value of the last call. -func (b *WebhookTLSConfigApplyConfiguration) WithCertificateDuration(value v1.Duration) *WebhookTLSConfigApplyConfiguration { - b.CertificateDuration = &value +// If called multiple times, the CertManager field is set to the value of the last call. +func (b *WebhookTLSConfigApplyConfiguration) WithCertManager(value *TrustManagerCertConfigApplyConfiguration) *WebhookTLSConfigApplyConfiguration { + b.CertManager = value return b } diff --git a/pkg/operator/applyconfigurations/utils.go b/pkg/operator/applyconfigurations/utils.go index 35cddf01e..2f6b767d3 100644 --- a/pkg/operator/applyconfigurations/utils.go +++ b/pkg/operator/applyconfigurations/utils.go @@ -60,6 +60,8 @@ func ForKind(kind schema.GroupVersionKind) interface{} { return &operatorv1alpha1.ServerConfigApplyConfiguration{} case v1alpha1.SchemeGroupVersion.WithKind("TrustManager"): return &operatorv1alpha1.TrustManagerApplyConfiguration{} + case v1alpha1.SchemeGroupVersion.WithKind("TrustManagerCertConfig"): + return &operatorv1alpha1.TrustManagerCertConfigApplyConfiguration{} case v1alpha1.SchemeGroupVersion.WithKind("TrustManagerConfig"): return &operatorv1alpha1.TrustManagerConfigApplyConfiguration{} case v1alpha1.SchemeGroupVersion.WithKind("TrustManagerControllerConfig"):