fix: default empty CertManager managementState to Managed for GitOps

[vishalgaikwad1979]

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.

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • CertManager management state now defaults to "Managed" when not explicitly configured, ensuring consistent operator behavior across all deployments.

• Tests

  • Added comprehensive tests validating management state default behavior and edge cases.

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[coderabbitai]

Walkthrough

The PR adds a default: Managed value to the spec.managementState field in the CertManager CRD schema via a new kustomize strategic-merge patch (wired into kustomization.yaml), regenerates the affected CRD manifests, and adds a Go helper operatorSpecForReconcile in OperatorClient that deep-copies the spec and coerces an unknown/empty ManagementState to Managed at runtime. Three unit tests validate the defaulting and preservation behavior.

Changes

Default managementState to Managed

Layer / File(s)	Summary
CRD schema default patch and generated manifests config/crd/patches/managementstate_default_in_certmanagers.yaml, config/crd/kustomization.yaml, config/crd/bases/operator.openshift.io_certmanagers.yaml, bundle/manifests/operator.openshift.io_certmanagers.yaml	New kustomize strategic-merge patch sets spec.managementState default to Managed for the certmanagers.operator.openshift.io CRD v1alpha1; patch is referenced in kustomization.yaml; both generated CRD YAMLs are updated with the resulting default: Managed entry.
Runtime managementState defaulting in OperatorClient pkg/operator/operatorclient/operatorclient.go, pkg/operator/operatorclient/operatorclient_test.go	operatorSpecForReconcile is introduced to deep-copy the spec and set ManagementState to operatorv1.Managed when management.IsOperatorUnknownState is true; both GetOperatorState and GetOperatorStateWithQuorum are updated to return this reconciled copy; three unit tests cover unknown-state defaulting, explicit-Unmanaged preservation, and the quorum path.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 13 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Test Structure And Quality	⚠️ Warning	Tests lack assertion messages and timeouts on blocking operations; all assertions are missing failure messages (e.g., require.NoError(t, err) instead of require.NoError(t, err, "message")), and tes...	Add meaningful failure messages to all assertions and use context.WithTimeout() instead of context.WithCancel() in tests 1&2 to protect WaitForCacheSync() calls from indefinite blocking.

✅ Passed checks (13 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: defaulting the CertManager managementState to Managed for GitOps compatibility, which is the core objective of the pull request.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	All test names in the new test file are stable and deterministic. The three test functions (TestGetOperatorStateDefaultsUnknownManagementStateToManaged, TestGetOperatorStatePreservesExplicitManagem...
Microshift Test Compatibility	✅ Passed	No new Ginkgo e2e tests found in PR. Only standard Go unit tests (TestXxx with testing.T) were added, which fall outside the scope of this MicroShift compatibility check.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	No Ginkgo e2e tests were added in this PR. The new test file (operatorclient_test.go) contains only standard Go unit tests using testing.T, not Ginkgo tests with It()/Describe() patterns.
Topology-Aware Scheduling Compatibility	✅ Passed	PR only adds CRD schema default and operator client logic for managementState field; introduces no scheduling constraints, affinity rules, topology spread constraints, node selectors, or PDBs that...
Ote Binary Stdout Contract	✅ Passed	No stdout contract violations detected. The PR adds data manipulation code only (spec defaulting logic) with no fmt.Print, klog, or log calls in main/init/suite functions.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No Ginkgo e2e tests added; only standard Go unit tests (TestXxx) in operatorclient_test.go which use fake clients and have no IPv4 or external connectivity assumptions.
No-Weak-Crypto	✅ Passed	This PR contains no cryptographic code, weak crypto algorithms (MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB), custom crypto implementations, or insecure secret comparisons. Changes are purely for Cert...
Container-Privileges	✅ Passed	No privileged container configurations (privileged: true, hostPID, hostNetwork, hostIPC, SYS_ADMIN, allowPrivilegeEscalation: true, or runAsUser: 0) were added in any modified files.
No-Sensitive-Data-In-Logs	✅ Passed	No logging statements exposing sensitive data found. Code uses only generic error messages via fmt.Errorf, and YAML files contain only schema definitions and configuration metadata.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: vishalgaikwad1979
Once this PR has been reviewed and has the lgtm label, please assign swghosh for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

🧹 Nitpick comments (1)

pkg/operator/operatorclient/operatorclient_test.go (1)

23-24: ⚡ Quick win

Bound cache sync waits in the first two tests.

Use context.WithTimeout(..., wait.ForeverTestTimeout) (like Line 83) so informer sync failures can’t hang CI indefinitely.

Suggested change

-	ctx, cancel := context.WithCancel(context.Background())
+	ctx, cancel := context.WithTimeout(context.Background(), wait.ForeverTestTimeout)
 	t.Cleanup(cancel)
...
-	ctx, cancel := context.WithCancel(context.Background())
+	ctx, cancel := context.WithTimeout(context.Background(), wait.ForeverTestTimeout)
 	t.Cleanup(cancel)

Also applies to: 37-37, 53-54, 67-67

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/operator/operatorclient/operatorclient_test.go` around lines 23 - 24, The
test cases are using context.WithCancel which can hang indefinitely if informer
sync fails. Replace all instances of context.WithCancel(context.Background())
with context.WithTimeout(context.Background(), wait.ForeverTestTimeout) to add a
timeout that prevents CI hangs. This pattern is already demonstrated elsewhere
in the file (line 83) and should be applied consistently to all affected test
setup locations where ctx and cancel are initialized.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@pkg/operator/operatorclient/operatorclient_test.go`:
- Around line 23-24: The test cases are using context.WithCancel which can hang
indefinitely if informer sync fails. Replace all instances of
context.WithCancel(context.Background()) with
context.WithTimeout(context.Background(), wait.ForeverTestTimeout) to add a
timeout that prevents CI hangs. This pattern is already demonstrated elsewhere
in the file (line 83) and should be applied consistently to all affected test
setup locations where ctx and cancel are initialized.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 73ba1298-aed3-422e-aa84-7241aa8cacda

📥 Commits

Reviewing files that changed from the base of the PR and between 514b381 and e17bf34.

📒 Files selected for processing (6)

• bundle/manifests/operator.openshift.io_certmanagers.yaml
• config/crd/bases/operator.openshift.io_certmanagers.yaml
• config/crd/kustomization.yaml
• config/crd/patches/managementstate_default_in_certmanagers.yaml
• pkg/operator/operatorclient/operatorclient.go
• pkg/operator/operatorclient/operatorclient_test.go

[openshift-ci]

@vishalgaikwad1979: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/verify	e17bf34	link	true	/test verify

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.