Skip cacerts distribution for clusters without product claims

[sridhargaddam]

The operator installation code skips clusters that are
missing their product claim, but ensureCertificatesCreated
and ensureCacertsDistributed were still processing them.
This PR avoids creating certs and cacerts ManifestWorks
for clusters without the product claims.

[mkolesnik]

Just a note: this won't be needed if we proceed with #115

[sridhargaddam]

Just a note: this won't be needed if we proceed with #115

Since its still a draft and we have not yet finalized on the approach, let me update the current PR with your suggestion.

[mkolesnik]

Small comment otherwise LGTM

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mkolesnik, sridhargaddam

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [mkolesnik,sridhargaddam]

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

[sridhargaddam]

/test integration

[openshift-ci]

New changes are detected. LGTM label has been removed.

[sridhargaddam]

@jewertow I had to modify an existing integration test as we cannot have a namespace greater than 63 chars. PTAL, thanks.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 17fb86c3-5c0f-4f35-acec-027b6410c512

📥 Commits

Reviewing files that changed from the base of the PR and between a56683b and a86b26b.

📒 Files selected for processing (1)

• test/integration/controller_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• test/integration/controller_test.go

---

📝 Walkthrough

Walkthrough

doReconcile now skips clusters without a product.open-cluster-management.io claim before logging or running cert-manager work. When cert-manager issuer configuration is present, certificate creation and cacerts ManifestWork creation happen inline per eligible cluster after the operator ManifestWork is applied. The prior post-loop batch helpers for certificates and cacerts distribution were removed. Integration tests add coverage for the no-product-claim case and a helper for asserting that a Certificate is absent.

Sequence Diagram(s)

sequenceDiagram
  participant Controller
  participant Cluster
  participant cert-manager

  Controller->>Cluster: getProductClaim
  alt product claim present
    Controller->>Cluster: apply operator ManifestWork
    Controller->>cert-manager: ensureCertificateForCluster
    Controller->>Cluster: ensureCacertsManifestWork
  else product claim missing
    Controller-->>Cluster: skip cert-manager work
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

• jewertow

🚥 Pre-merge checks | ✅ 8 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.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	New cert-manager tests use Eventually/Consistently via helpers with no explicit timeouts, and the added expectations lack failure messages.	Add explicit timeouts to the new Eventually/Consistently calls (helpers and tests) and attach descriptive failure messages to key Expect/Require assertions.

✅ Passed checks (8 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the main change: skipping cacerts distribution for clusters without product claims.
Description check	✅ Passed	The description accurately describes the issue and the fix applied in this changeset.
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	Added Ginkgo titles are static; no dynamic names, dates, UUIDs, or generated suffixes appear in test titles.
No-Weak-Crypto	✅ Passed	Touched code only wires cert-manager resources/tests; no weak crypto, custom crypto, or secret/token comparisons found (only strings.Compare on cluster names).
Container-Privileges	✅ Passed	No changed manifest/container specs contain privileged flags; searched all differing files and found none.
No-Sensitive-Data-In-Logs	✅ Passed	Logs only include cluster/mesh/secret names and status messages; no secrets, tokens, PII, or other sensitive payloads are logged.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[sridhargaddam]

@mkolesnik I had to rebase this PR, can you reapprove? TIA

[mkolesnik]

Small suggestion to make test more robust

[coderabbitai]

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@test/integration/controller_test.go`:
- Around line 837-844: The `Consistently` assertion in this test relies on
default timing, so update the anonymous check around `k8sClient.List` to pass
explicit timeout and polling interval values. Keep the existing
certificate-listing logic intact, but make the wait parameters explicit in the
`Consistently` call to match the project’s Ginkgo timing guidelines and avoid
environment-dependent behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 74f360ac-4024-4ae9-82b3-bf97a529069c

📥 Commits

Reviewing files that changed from the base of the PR and between 17e64d7 and a56683b.

📒 Files selected for processing (1)

• test/integration/controller_test.go