Graduate CompatibilityRequirement API to v1#2905
Conversation
Add apiextensions.openshift.io/v1 with compatibility level 1 (stable). The API surface is unchanged from v1alpha1 — types, fields, validation markers, and CEL rules are structurally identical. The CRD now serves both v1 (storage) and v1alpha1 (backward compat, storage=false). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
📝 WalkthroughWalkthroughAdds the Suggested reviewers
🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Comment |
|
Hello @stefanonardo! Some important instructions when contributing to openshift/api: |
openshift-ci
Bot
added
the
size/XXL
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apiextensions/v1/Makefile (1)
2-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
$(MAKE)for the recursive target.Calling plain
makehere skips GNU make’s recursive-build handling, so parent flags like-j/-nand the jobserver are not propagated reliably.Suggested change
test: - make -C ../../tests test GINKGO_EXTRA_ARGS=--focus="apiextensions.openshift.io/v1" + $(MAKE) -C ../../tests test GINKGO_EXTRA_ARGS=--focus="apiextensions.openshift.io/v1"🤖 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 `@apiextensions/v1/Makefile` around lines 2 - 3, The recursive test target in the Makefile should use $(MAKE) instead of plain make so GNU make can propagate parent flags and jobserver settings correctly. Update the test recipe in the test target to invoke the nested build through $(MAKE), keeping the existing -C ../../tests test and GINKGO_EXTRA_ARGS=--focus="apiextensions.openshift.io/v1" behavior intact.apiextensions/v1/tests/compatibilityrequirements.apiextensions.openshift.io/CRDCompatibilityRequirementOperator.yaml (1)
1332-1431: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGive this case a distinct name.
This scenario is useful coverage for “same UID, same generation is allowed,” but it reuses the exact title from Lines 604-700. A unique name will make failures/reporting unambiguous.
🤖 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 `@apiextensions/v1/tests/compatibilityrequirements.apiextensions.openshift.io/CRDCompatibilityRequirementOperator.yaml` around lines 1332 - 1431, Rename this test case to a distinct title in CRDCompatibilityRequirementOperator so it is unique from the earlier “Should be able to set valid observedCRD uid” scenario. Update the test name in the YAML case covering the same UID and generation path so failures and reporting clearly identify this specific “same UID, same generation is allowed” behavior.apiextensions/v1/types_compatibilityrequirement.go (1)
185-195: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the
MinItemsconstraint in field comments.Several list fields declare
+kubebuilder:validation:MinItems=1but the comment only documents the upper bound. The coding guidelines require min/max item counts to be documented in the field comment. This affectsexcludedFields(here, "may contain at most 64 fields"), as well asadditionalVersions(line 133),excludedFields[].versions(line 166), andconditions(line 329) — none state the minimum-of-one requirement.As per coding guidelines: "
+kubebuilder:validation:MinItemsand+kubebuilder:validation:MaxItems- document item count ranges".🤖 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 `@apiextensions/v1/types_compatibilityrequirement.go` around lines 185 - 195, Update the field comments for the list properties that use +kubebuilder:validation:MinItems=1 so they explicitly document the minimum item count as well as the maximum. In the affected types, adjust the comments on ExcludedFields, AdditionalVersions, ExcludedFields[].Versions, and Conditions to state that the list must contain at least one item and, where applicable, at most the documented limit. Keep the existing validation annotations unchanged and make the comment wording match the constraint range for each field.Source: Coding guidelines
🤖 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 `@apiextensions/v1/types_compatibilityrequirement.go`:
- Around line 128-130: The doc comments for additionalVersions and
excludedFields[].versions contain a duplicated “must must” typo and a repeated
trailing sentence joined into the same line; clean up the wording so each
validation rule appears once and reads correctly. Update the comment text in
types_compatibilityrequirement.go near the additionalVersions and
excludedFields[].versions definitions, then regenerate the derived CRD/manifests
with make update so the propagated descriptions are fixed too.
---
Nitpick comments:
In `@apiextensions/v1/Makefile`:
- Around line 2-3: The recursive test target in the Makefile should use $(MAKE)
instead of plain make so GNU make can propagate parent flags and jobserver
settings correctly. Update the test recipe in the test target to invoke the
nested build through $(MAKE), keeping the existing -C ../../tests test and
GINKGO_EXTRA_ARGS=--focus="apiextensions.openshift.io/v1" behavior intact.
In
`@apiextensions/v1/tests/compatibilityrequirements.apiextensions.openshift.io/CRDCompatibilityRequirementOperator.yaml`:
- Around line 1332-1431: Rename this test case to a distinct title in
CRDCompatibilityRequirementOperator so it is unique from the earlier “Should be
able to set valid observedCRD uid” scenario. Update the test name in the YAML
case covering the same UID and generation path so failures and reporting clearly
identify this specific “same UID, same generation is allowed” behavior.
In `@apiextensions/v1/types_compatibilityrequirement.go`:
- Around line 185-195: Update the field comments for the list properties that
use +kubebuilder:validation:MinItems=1 so they explicitly document the minimum
item count as well as the maximum. In the affected types, adjust the comments on
ExcludedFields, AdditionalVersions, ExcludedFields[].Versions, and Conditions to
state that the list must contain at least one item and, where applicable, at
most the documented limit. Keep the existing validation annotations unchanged
and make the comment wording match the constraint range for each field.
🪄 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: b0c229df-7cce-4de6-9c34-7a398cd0fe93
⛔ Files ignored due to path filters (9)
apiextensions/v1/zz_generated.crd-manifests/0000_20_crd-compatibility-checker_01_compatibilityrequirements.crd.yamlis excluded by!**/zz_generated.crd-manifests/*apiextensions/v1/zz_generated.crd-manifests/doc.gois excluded by!**/zz_generated.crd-manifests/*apiextensions/v1/zz_generated.deepcopy.gois excluded by!**/zz_generated*apiextensions/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/zz_generated*apiextensions/v1/zz_generated.featuregated-crd-manifests/compatibilityrequirements.apiextensions.openshift.io/CRDCompatibilityRequirementOperator.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**apiextensions/v1/zz_generated.model_name.gois excluded by!**/zz_generated*apiextensions/v1/zz_generated.swagger_doc_generated.gois excluded by!**/zz_generated*openapi/generated_openapi/zz_generated.openapi.gois excluded by!openapi/**,!**/zz_generated*openapi/openapi.jsonis excluded by!openapi/**
📒 Files selected for processing (9)
apiextensions/install.goapiextensions/v1/Makefileapiextensions/v1/doc.goapiextensions/v1/manual-override-crd-manifests/compatibilityrequirements.apiextensions.openshift.io/CRDCompatibilityRequirementOperator.yamlapiextensions/v1/register.goapiextensions/v1/tests/compatibilityrequirements.apiextensions.openshift.io/CRDCompatibilityRequirementOperator.yamlapiextensions/v1/types_compatibilityrequirement.gohack/update-payload-crds.shpayload-manifests/crds/0000_20_crd-compatibility-checker_01_compatibilityrequirements.crd.yaml
| // Each item must be at most 63 characters in length, and must must consist | ||
| // of only lowercase alphanumeric characters and hyphens, and must start | ||
| // with an alphabetic character and end with an alphanumeric character.// with an alphabetic character and end with an alphanumeric character. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix duplicated sentence and must must typo in additionalVersions doc.
Line 130 contains the same sentence twice joined by .//, and line 128 has a doubled "must must". This comment is the source for the generated CRD descriptions, so the artifact propagates into the manual-override and payload manifests (e.g. payload lines 164 and 688). Fix here and regenerate with make update.
📝 Proposed fix
- // Each item must be at most 63 characters in length, and must must consist
- // of only lowercase alphanumeric characters and hyphens, and must start
- // with an alphabetic character and end with an alphanumeric character.// with an alphabetic character and end with an alphanumeric character.
+ // Each item must be at most 63 characters in length, and must consist
+ // of only lowercase alphanumeric characters and hyphens, and must start
+ // with an alphabetic character and end with an alphanumeric character.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Each item must be at most 63 characters in length, and must must consist | |
| // of only lowercase alphanumeric characters and hyphens, and must start | |
| // with an alphabetic character and end with an alphanumeric character.// with an alphabetic character and end with an alphanumeric character. | |
| // Each item must be at most 63 characters in length, and must consist | |
| // of only lowercase alphanumeric characters and hyphens, and must start | |
| // with an alphabetic character and end with an alphanumeric character. |
🤖 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 `@apiextensions/v1/types_compatibilityrequirement.go` around lines 128 - 130,
The doc comments for additionalVersions and excludedFields[].versions contain a
duplicated “must must” typo and a repeated trailing sentence joined into the
same line; clean up the wording so each validation rule appears once and reads
correctly. Update the comment text in types_compatibilityrequirement.go near the
additionalVersions and excludedFields[].versions definitions, then regenerate
the derived CRD/manifests with make update so the propagated descriptions are
fixed too.
|
@stefanonardo: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
1 participant
Summary
apiextensions.openshift.io/v1with compatibility level 1 (stable)