From e17bf344f587ff9f5596c02721f115b7e081851f Mon Sep 17 00:00:00 2001 From: vgaikwad Date: Wed, 17 Jun 2026 18:16:48 +0530 Subject: [PATCH] fix: default empty CertManager managementState to Managed for GitOps GitOps-created CertManager CRs often omit managementState, which left static resources applied but deployment controllers idle. Treat unknown state as Managed during reconcile and default the CRD field to Managed. Co-authored-by: Cursor --- .../operator.openshift.io_certmanagers.yaml | 1 + .../operator.openshift.io_certmanagers.yaml | 1 + config/crd/kustomization.yaml | 1 + ...nagementstate_default_in_certmanagers.yaml | 14 +++ pkg/operator/operatorclient/operatorclient.go | 16 ++- .../operatorclient/operatorclient_test.go | 98 +++++++++++++++++++ 6 files changed, 129 insertions(+), 2 deletions(-) create mode 100644 config/crd/patches/managementstate_default_in_certmanagers.yaml create mode 100644 pkg/operator/operatorclient/operatorclient_test.go diff --git a/bundle/manifests/operator.openshift.io_certmanagers.yaml b/bundle/manifests/operator.openshift.io_certmanagers.yaml index 5343383c7..b863b0977 100644 --- a/bundle/manifests/operator.openshift.io_certmanagers.yaml +++ b/bundle/manifests/operator.openshift.io_certmanagers.yaml @@ -630,6 +630,7 @@ spec: - TraceAll type: string managementState: + default: Managed description: managementState indicates whether and how the operator should manage the component pattern: ^(Managed|Unmanaged|Force|Removed)$ diff --git a/config/crd/bases/operator.openshift.io_certmanagers.yaml b/config/crd/bases/operator.openshift.io_certmanagers.yaml index 015e301fd..04a74b6ee 100644 --- a/config/crd/bases/operator.openshift.io_certmanagers.yaml +++ b/config/crd/bases/operator.openshift.io_certmanagers.yaml @@ -630,6 +630,7 @@ spec: - TraceAll type: string managementState: + default: Managed description: managementState indicates whether and how the operator should manage the component pattern: ^(Managed|Unmanaged|Force|Removed)$ diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index b88e88475..a1a999b10 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -15,6 +15,7 @@ resources: #+kubebuilder:scaffold:crdkustomizeresource patchesStrategicMerge: +- patches/managementstate_default_in_certmanagers.yaml # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix. # patches here are for enabling the conversion webhook for each CRD #- patches/webhook_in_certmanagers.yaml diff --git a/config/crd/patches/managementstate_default_in_certmanagers.yaml b/config/crd/patches/managementstate_default_in_certmanagers.yaml new file mode 100644 index 000000000..0b12a0bec --- /dev/null +++ b/config/crd/patches/managementstate_default_in_certmanagers.yaml @@ -0,0 +1,14 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: certmanagers.operator.openshift.io +spec: + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + properties: + spec: + properties: + managementState: + default: Managed diff --git a/pkg/operator/operatorclient/operatorclient.go b/pkg/operator/operatorclient/operatorclient.go index 3a72aceb0..9e2a3bf65 100644 --- a/pkg/operator/operatorclient/operatorclient.go +++ b/pkg/operator/operatorclient/operatorclient.go @@ -16,6 +16,7 @@ import ( operatorv1 "github.com/openshift/api/operator/v1" applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1" "github.com/openshift/library-go/pkg/apiserver/jsonpatch" + "github.com/openshift/library-go/pkg/operator/management" "github.com/openshift/library-go/pkg/operator/v1helpers" "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" @@ -140,7 +141,7 @@ func (c OperatorClient) GetOperatorState() (*operatorv1.OperatorSpec, *operatorv return nil, nil, "", err } - return &instance.Spec.OperatorSpec, &instance.Status.OperatorStatus, instance.ResourceVersion, nil + return operatorSpecForReconcile(&instance.Spec.OperatorSpec), &instance.Status.OperatorStatus, instance.ResourceVersion, nil } func (c OperatorClient) GetOperatorStateWithQuorum(ctx context.Context) (*operatorv1.OperatorSpec, *operatorv1.OperatorStatus, string, error) { @@ -149,7 +150,18 @@ func (c OperatorClient) GetOperatorStateWithQuorum(ctx context.Context) (*operat return nil, nil, "", err } - return &instance.Spec.OperatorSpec, &instance.Status.OperatorStatus, instance.ResourceVersion, nil + return operatorSpecForReconcile(&instance.Spec.OperatorSpec), &instance.Status.OperatorStatus, instance.ResourceVersion, nil +} + +// operatorSpecForReconcile returns a copy of spec with unknown managementState treated as Managed. +// GitOps tools such as Argo CD often create CertManager without managementState; library-go static +// resource controllers already proceed in that case, but deployment controllers require Managed. +func operatorSpecForReconcile(spec *operatorv1.OperatorSpec) *operatorv1.OperatorSpec { + reconcileSpec := spec.DeepCopy() + if management.IsOperatorUnknownState(reconcileSpec.ManagementState) { + reconcileSpec.ManagementState = operatorv1.Managed + } + return reconcileSpec } func GetUnsupportedConfigOverrides(operatorSpec *operatorv1.OperatorSpec) (*v1alpha1.UnsupportedConfigOverrides, error) { diff --git a/pkg/operator/operatorclient/operatorclient_test.go b/pkg/operator/operatorclient/operatorclient_test.go new file mode 100644 index 000000000..49554d6d5 --- /dev/null +++ b/pkg/operator/operatorclient/operatorclient_test.go @@ -0,0 +1,98 @@ +package operatorclient + +import ( + "context" + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/tools/cache" + "k8s.io/utils/clock" + + operatorv1 "github.com/openshift/api/operator/v1" + "github.com/stretchr/testify/require" + + "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" + "github.com/openshift/cert-manager-operator/pkg/operator/clientset/versioned/fake" + certmanoperatorinformers "github.com/openshift/cert-manager-operator/pkg/operator/informers/externalversions" +) + +func TestGetOperatorStateDefaultsUnknownManagementStateToManaged(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + + fakeClient := fake.NewClientset(&v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + OperatorSpec: operatorv1.OperatorSpec{ + LogLevel: operatorv1.Normal, + }, + }, + }) + informers := certmanoperatorinformers.NewSharedInformerFactory(fakeClient, 0) + informer := informers.Operator().V1alpha1().CertManagers() + go informer.Informer().Run(ctx.Done()) + require.True(t, cache.WaitForCacheSync(ctx.Done(), informer.Informer().HasSynced)) + + client := OperatorClient{ + Informers: informers, + Client: fakeClient.OperatorV1alpha1(), + Clock: clock.RealClock{}, + } + + spec, _, _, err := client.GetOperatorState() + require.NoError(t, err) + require.Equal(t, operatorv1.Managed, spec.ManagementState) +} + +func TestGetOperatorStatePreservesExplicitManagementState(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + + fakeClient := fake.NewClientset(&v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + OperatorSpec: operatorv1.OperatorSpec{ + ManagementState: operatorv1.Unmanaged, + }, + }, + }) + informers := certmanoperatorinformers.NewSharedInformerFactory(fakeClient, 0) + informer := informers.Operator().V1alpha1().CertManagers() + go informer.Informer().Run(ctx.Done()) + require.True(t, cache.WaitForCacheSync(ctx.Done(), informer.Informer().HasSynced)) + + client := OperatorClient{ + Informers: informers, + Client: fakeClient.OperatorV1alpha1(), + Clock: clock.RealClock{}, + } + + spec, _, _, err := client.GetOperatorState() + require.NoError(t, err) + require.Equal(t, operatorv1.Unmanaged, spec.ManagementState) +} + +func TestGetOperatorStateWithQuorumDefaultsUnknownManagementStateToManaged(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), wait.ForeverTestTimeout) + t.Cleanup(cancel) + + fakeClient := fake.NewClientset(&v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{}, + }) + client := OperatorClient{ + Client: fakeClient.OperatorV1alpha1(), + Clock: clock.RealClock{}, + } + + spec, _, _, err := client.GetOperatorStateWithQuorum(ctx) + require.NoError(t, err) + require.Equal(t, operatorv1.Managed, spec.ManagementState) +}