Skip to content

🌱 add ClusterObjectSet CRD validation unit tests for probes and immutability - #3024

Merged
openshift-merge-bot[bot] merged 1 commit into
operator-framework:mainfrom
perdasilva:cos-crd-validation-tests
Oct 19, 2026
Merged

openshift-merge-bot[bot] merged 1 commit into
operator-framework:mainfrom
perdasilva:cos-crd-validation-tests

Conversation

@perdasilva

@perdasilva perdasilva commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Description

Fills gaps in the ClusterObjectSet CRD validation test coverage. These are additive, test-only changes that exercise CEL and OpenAPI schema rules against the envtest API server, complementing the existing table-driven validation tests in api/v1.

Immutability (TestClusterObjectSetImmutability, TestClusterObjectSetDigestImmutable)

  • spec.lifecycleState cannot be un-archived (Archived→Active rejected; Active→Archived allowed)
  • status.observedPhases[].digest is immutable once set (settable from empty, rejected on change)

This completes immutability coverage for every immutable field in the CRD (revision, phases, collisionProtection, lifecycleState, completedAt, observedPhases[].digest).

Progression probe union rules (TestClusterObjectSetValidity)

  • ObjectSelector: GroupKind requires groupKind / forbids label; Label requires label; Label requires exactly one of matchLabels/matchExpressions; invalid selector type rejected
  • Assertion: invalid type rejected; ConditionEqual requires conditionEqual; mismatched union member rejected; ConditionEqual.status enum enforced; FieldValue/FieldsEqual field-path syntax enforced; at-least-one-assertion required
  • Valid baselines (GroupKind+ConditionEqual, Label+FieldValue) to confirm well-formed probes are accepted

Notes

  • Follows the existing test style: assert errors.IsInvalid on rejection rather than matching specific messages.
  • The Label "neither matchLabels nor matchExpressions" case is intentionally omitted — an empty metav1.LabelSelector is omitzero, so the typed client omits it and the request is caught by the "label required" rule before reaching the exactly-one rule. The "both" case covers that rule's reject path.

Testing

make test-unit (or go test ./api/v1/... with envtest assets) — all cases pass.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Expanded automated coverage for cluster object set lifecycle transitions, configuration limits, required fields, uniqueness, and progression probe validation.
    • Added coverage confirming phase digests can be set or kept unchanged, but cannot be changed after being set.

@netlify

netlify Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for olmv1 ready!

Name Link
🔨 Latest commit 5546f1c
🔍 Latest deploy log https://app.netlify.com/projects/olmv1/deploys/6ad6308a1bd1b92a5c682390
😎 Deploy Preview https://deploy-preview-3024--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: a34823b0-4030-473a-9e9a-00d9bb478be9
📥 Commits

Reviewing files that changed from the base of the PR and between 9c9ec0e and 5546f1c.

📒 Files selected for processing: 2
  • api/v1/clusterobjectset_types_test.go
  • api/v1/validation_test.go

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

📝 Walkthrough

Adds API tests for ClusterObjectSet lifecycle transitions, phase digest immutability, and validation of phase names, object references, and progression probes.

Changes: ClusterObjectSet validation

Layer / File(s) Summary
Status immutability tests
api/v1/clusterobjectset_types_test.go, api/v1/validation_test.go
Tests cover allowed lifecycle transitions and reject changing a phase digest after it is set.
Spec and progression-probe validation tests
api/v1/clusterobjectset_types_test.go
Tests cover phase-name limits and uniqueness, required and length-limited reference fields, and progression-probe selector and assertion validation. Helper functions construct valid test inputs.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to 5546f

The added validation tests present no identified merge-blocking risk; merge after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 45.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies the ClusterObjectSet CRD validation tests and the main coverage areas: probes and immutability. The seedling icon matches the repository template.
Description check Passed The description provides the change summary, motivation, detailed test coverage, implementation notes, and testing information. It does not include the reviewer checklist from the template, but the su…
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.
Full details: Description check

Explanation

The description provides the change summary, motivation, detailed test coverage, implementation notes, and testing information. It does not include the reviewer checklist from the template, but the substantive description is complete.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@perdasilva

Copy link
Copy Markdown
Contributor Author

/hold until group PR merges

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 19, 2026
…ability

Fill gaps in the ClusterObjectSet CRD validation coverage:

- Immutability: lifecycleState cannot be un-archived.
- Progression probe union rules: ObjectSelector and Assertion discriminator
  requirements, ConditionEqual status enum, field-path syntax, and the
  at-least-one-assertion requirement, plus valid baselines.
- Phase and ObjectSourceRef bounds: phase-name length and uniqueness,
  ref name/key/namespace bounds, and the progressionProbes item cap.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Per G. da Silva <pegoncal@redhat.com>
@perdasilva
perdasilva force-pushed the cos-crd-validation-tests branch from 9c9ec0e to 5546f1c Compare October 19, 2026 15:00
@perdasilva

Copy link
Copy Markdown
Contributor Author

/unblock

@perdasilva

Copy link
Copy Markdown
Contributor Author

/unhold

@openshift-ci openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 19, 2026

@fao89 fao89 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Oct 19, 2026

@fgiudici fgiudici left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@tmshort

tmshort commented Oct 19, 2026

Copy link
Copy Markdown
Member

/approve

@openshift-ci

openshift-ci Bot commented Oct 19, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fao89, tmshort

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:

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 19, 2026
@openshift-merge-bot
openshift-merge-bot Bot merged commit 7cbc3e5 into operator-framework:main Oct 19, 2026
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants