Skip to content

Conversation

@camilamacedo86
Copy link
Contributor

Those changes were tested with the fix: #2409
See that now we are checking the current branch and identifying what are the errors fixed.
Example

Shell

Screenshot 2025-12-24 at 11 40 45

AI Command

Screenshot 2025-12-24 at 11 41 24

TL'DR:

Shell

$ ./hack/api-lint-diff/run.sh 
Using temporary directory: /var/folders/n4/j272tr6d7hq63mf7_skv6zr80000gn/T/tmp.EyZ0Fx7oFs
Building custom golangci-lint with kube-api-linter plugin...
Running golangci-lint custom build...
Custom linter build completed
Successfully built custom golangci-lint at /var/folders/n4/j272tr6d7hq63mf7_skv6zr80000gn/T/tmp.EyZ0Fx7oFs/bin/golangci-lint-kube-api
Preparing worktree (detached HEAD 578dc334)
HEAD is now at 578dc334 Add CLAUDE command and new check to review and lint the API changes (#2399)
API Lint Diff Results
=====================
Baseline (main): 47 issues
Current branch: 44 issues

FIXED: 3
NEW: 0
PRE-EXISTING: 44

=== FIXED ISSUES ===
api/v1/clusterextension_types.go:477:2:kubeapilinter:commentstart: godoc for field ClusterExtensionStatus.Conditions should start with 'conditions ...'
api/v1/clusterextension_types.go:540:2:kubeapilinter:commentstart: field ClusterExtension.metav1.ObjectMeta is missing godoc comment
api/v1/clusterextensionrevision_types.go:219:2:kubeapilinter:commentstart: field ClusterExtensionRevision.metav1.ObjectMeta is missing godoc comment

=== PRE-EXISTING ISSUES ===
api/v1/clustercatalog_types.go:61:2:kubeapilinter:optionalorrequired: embedded field ClusterCatalog.metav1.ObjectMeta must be marked as optional or required
api/v1/clustercatalog_types.go:66:2:kubeapilinter:optionalorrequired: field ClusterCatalog.Spec should use marker required instead of kubebuilder:validation:Required
api/v1/clustercatalog_types.go:73:2:kubeapilinter:optionalfields: field ClusterCatalog.Status has a valid zero value ({}), but the validation is not complete (e.g. min properties/adding required fields). The field should be a pointer to allow the zero value to be set. If the zero value is not a valid use case, complete the validation and remove the pointer.
api/v1/clustercatalog_types.go:109:2:kubeapilinter:optionalorrequired: field ClusterCatalogSpec.Source should use marker required instead of kubebuilder:validation:Required
api/v1/clustercatalog_types.go:131:2:kubeapilinter:optionalfields: field ClusterCatalogSpec.Priority has a valid zero value (0), but the validation is not complete (e.g. minimum/maximum). The field should be a pointer to allow the zero value to be set. If the zero value is not a valid use case, complete the validation and remove the pointer.
api/v1/clustercatalog_types.go:175:2:kubeapilinter:conditions: Conditions field in ClusterCatalogStatus is missing the following markers: patchStrategy=merge, patchMergeKey=type
api/v1/clustercatalog_types.go:178:2:kubeapilinter:optionalfields: field ClusterCatalogStatus.ResolvedSource does not allow the zero value. It must have the omitzero tag.
api/v1/clustercatalog_types.go:206:2:kubeapilinter:optionalorrequired: field ClusterCatalogURLs.Base should use marker required instead of kubebuilder:validation:Required
api/v1/clustercatalog_types.go:224:2:kubeapilinter:optionalorrequired: field CatalogSource.Type should use marker required instead of kubebuilder:validation:Required
api/v1/clustercatalog_types.go:245:2:kubeapilinter:optionalorrequired: field ResolvedCatalogSource.Type should use marker required instead of kubebuilder:validation:Required
api/v1/clustercatalog_types.go:248:2:kubeapilinter:optionalorrequired: field ResolvedCatalogSource.Image must be marked as optional or required
api/v1/clustercatalog_types.go:264:2:kubeapilinter:optionalorrequired: field ResolvedImageSource.Ref should use marker required instead of kubebuilder:validation:Required
api/v1/clustercatalog_types.go:320:2:kubeapilinter:optionalorrequired: field ImageSource.Ref should use marker required instead of kubebuilder:validation:Required
api/v1/clustercatalog_types.go:328:2:kubeapilinter:integers: field ImageSource.PollIntervalMinutes pointer should not use an int, int8 or int16. Use int32 or int64 depending on bounding requirements
api/v1/clusterextension_types.go:67:2:kubeapilinter:optionalorrequired: field ClusterExtensionSpec.Namespace should use marker required instead of kubebuilder:validation:Required
api/v1/clusterextension_types.go:76:2:kubeapilinter:optionalorrequired: field ClusterExtensionSpec.ServiceAccount should use marker required instead of kubebuilder:validation:Required
api/v1/clusterextension_types.go:92:2:kubeapilinter:optionalorrequired: field ClusterExtensionSpec.Source should use marker required instead of kubebuilder:validation:Required
api/v1/clusterextension_types.go:110:2:kubeapilinter:optionalfields: field ClusterExtensionSpec.Config does not allow the zero value. It must have the omitzero tag.
api/v1/clusterextension_types.go:131:2:kubeapilinter:optionalorrequired: field SourceConfig.SourceType should use marker required instead of kubebuilder:validation:Required
api/v1/clusterextension_types.go:171:2:kubeapilinter:optionalorrequired: field ClusterExtensionConfig.ConfigType should use marker required instead of kubebuilder:validation:Required
api/v1/clusterextension_types.go:214:2:kubeapilinter:optionalorrequired: field CatalogFilter.PackageName should use marker required instead of kubebuilder:validation:Required
api/v1/clusterextension_types.go:294:2:kubeapilinter:optionalfields: field CatalogFilter.Version has a valid zero value (""), but the validation is not complete (e.g. minimum length). The field should be a pointer to allow the zero value to be set. If the zero value is not a valid use case, complete the validation and remove the pointer.
api/v1/clusterextension_types.go:338:2:kubeapilinter:ssatags: CatalogFilter.Channels should have a listType marker for proper Server-Side Apply behavior (atomic, set, or map)
api/v1/clusterextension_types.go:397:2:kubeapilinter:optionalorrequired: field ServiceAccountReference.Name should use marker required instead of kubebuilder:validation:Required
api/v1/clusterextension_types.go:409:2:kubeapilinter:optionalorrequired: field PreflightConfig.CRDUpgradeSafety must be marked as optional or required
api/v1/clusterextension_types.go:425:2:kubeapilinter:optionalorrequired: field CRDUpgradeSafetyPreflightConfig.Enforcement should use marker required instead of kubebuilder:validation:Required
api/v1/clusterextension_types.go:450:2:kubeapilinter:optionalorrequired: field BundleMetadata.Name should use marker required instead of kubebuilder:validation:Required
api/v1/clusterextension_types.go:457:2:kubeapilinter:optionalorrequired: field BundleMetadata.Version should use marker required instead of kubebuilder:validation:Required
api/v1/clusterextension_types.go:463:2:kubeapilinter:optionalorrequired: field RevisionStatus.Name must be marked as optional or required
api/v1/clusterextension_types.go:472:2:kubeapilinter:conditions: Conditions field in RevisionStatus is missing the following markers: patchStrategy=merge, patchMergeKey=type
api/v1/clusterextension_types.go:503:2:kubeapilinter:conditions: Conditions field in ClusterExtensionStatus is missing the following markers: patchStrategy=merge, patchMergeKey=type
api/v1/clusterextension_types.go:516:2:kubeapilinter:arrayofstruct: ClusterExtensionStatus.ActiveRevisions is an array of structs, but the struct has no required fields. At least one field should be marked as required to prevent ambiguous YAML configurations
api/v1/clusterextension_types.go:527:2:kubeapilinter:optionalorrequired: field ClusterExtensionInstallStatus.Bundle should use marker required instead of kubebuilder:validation:Required
api/v1/clusterextension_types.go:550:2:kubeapilinter:nonpointerstructs: field ClusterExtension.Spec is a non-pointer struct with required fields. It must be marked as required.
api/v1/clusterextension_types.go:554:2:kubeapilinter:optionalfields: field ClusterExtension.Status has a valid zero value ({}), but the validation is not complete (e.g. min properties/adding required fields). The field should be a pointer to allow the zero value to be set. If the zero value is not a valid use case, complete the validation and remove the pointer.
api/v1/clusterextensionrevision_types.go:55:2:kubeapilinter:optionalorrequired: field ClusterExtensionRevisionSpec.LifecycleState must be marked as optional or required
api/v1/clusterextensionrevision_types.go:67:2:kubeapilinter:optionalorrequired: field ClusterExtensionRevisionSpec.Revision should use marker required instead of kubebuilder:validation:Required
api/v1/clusterextensionrevision_types.go:89:2:kubeapilinter:arrayofstruct: ClusterExtensionRevisionSpec.Phases is an array of structs, but the struct has no required fields. At least one field should be marked as required to prevent ambiguous YAML configurations
api/v1/clusterextensionrevision_types.go:119:2:kubeapilinter:optionalorrequired: field ClusterExtensionRevisionPhase.Name must be marked as optional or required
api/v1/clusterextensionrevision_types.go:124:2:kubeapilinter:arrayofstruct: ClusterExtensionRevisionPhase.Objects is an array of structs, but the struct has no required fields. At least one field should be marked as required to prevent ambiguous YAML configurations
api/v1/clusterextensionrevision_types.go:136:2:kubeapilinter:optionalorrequired: field ClusterExtensionRevisionObject.Object must be marked as optional or required
api/v1/clusterextensionrevision_types.go:201:2:kubeapilinter:conditions: Conditions field in ClusterExtensionRevisionStatus is missing the following markers: patchStrategy=merge, patchMergeKey=type
api/v1/clusterextensionrevision_types.go:227:2:kubeapilinter:nonpointerstructs: field ClusterExtensionRevision.Spec is a non-pointer struct with required fields. It must be marked as required.
api/v1/clusterextensionrevision_types.go:231:2:kubeapilinter:optionalfields: field ClusterExtensionRevision.Status has a valid zero value ({}), but the validation is not complete (e.g. min properties/adding required fields). The field should be a pointer to allow the zero value to be set. If the zero value is not a valid use case, complete the validation and remove the pointer.

SUCCESS: Fixed 3 issue(s), no new issues introduced.
WARNING: 44 pre-existing issue(s) remain. Please address them separately.

Command AI

---
  API Lint Diff Analysis Report

  Generated: 2025-12-24
  Baseline: main branch (47 issues)
  Current: fix-go-docs-linter-api-check (44 issues)
  Status: ✅ PASSED - 0 new issues introduced

  Executive Summary

  - Baseline issues: 47
  - Current issues: 44
  - FIXED: 3 issues (Successfully resolved in this branch!)
  - NEW: 0 issues (No new problems introduced)
  - PRE-EXISTING: 44 issues (Existed before changes, can be addressed separately)
  - False positives (IGNORED): 6 issues

  Great work! This branch fixed 3 documentation issues without introducing any new API problems.

  ---
  FIXED ISSUES (3 issues)

  1. api/v1/clusterextension_types.go:477 - ClusterExtensionStatus.Conditions godoc formatting
     Fixed godoc comment to start with 'conditions' instead of custom text

  2. api/v1/clusterextension_types.go:540 - ClusterExtension.metav1.ObjectMeta missing godoc
     Added godoc comment for the embedded ObjectMeta field

  3. api/v1/clusterextensionrevision_types.go:219 - ClusterExtensionRevision.metav1.ObjectMeta missing godoc
     Added godoc comment for the embedded ObjectMeta field

  ---
  PRE-EXISTING ISSUES (44 issues)

  These issues existed before your changes. They can be addressed in separate PRs.

  ---
  FALSE POSITIVES FILTERED (6 issues)

  The following issues are IGNORED as they are standard Kubernetes CRD patterns incorrectly flagged by kube-api-linter:

  1. api/v1/clustercatalog_types.go:73 - ClusterCatalog.Status optionalfields
  Why ignored: Status is NEVER a pointer in Kubernetes APIs. Works correctly with omitempty tag.
  2. api/v1/clusterextension_types.go:554 - ClusterExtension.Status optionalfields
  Why ignored: Status is NEVER a pointer in Kubernetes APIs. Works correctly with omitempty tag.
  3. api/v1/clusterextensionrevision_types.go:231 - ClusterExtensionRevision.Status optionalfields
  Why ignored: Status is NEVER a pointer in Kubernetes APIs. Works correctly with omitempty tag.
  4. api/v1/clustercatalog_types.go:175 - ClusterCatalogStatus.Conditions missing patchStrategy markers
  Why ignored: metav1.Condition already handles patches correctly. Adding these markers is redundant.
  5. api/v1/clusterextension_types.go:503 - ClusterExtensionStatus.Conditions missing patchStrategy markers
  Why ignored: metav1.Condition already handles patches correctly. Adding these markers is redundant.
  6. api/v1/clusterextensionrevision_types.go:201 - ClusterExtensionRevisionStatus.Conditions missing patchStrategy markers
  Why ignored: metav1.Condition already handles patches correctly. Adding these markers is redundant.

  ---
  DETAILED ANALYSIS FOR PRE-EXISTING ISSUES

  Since there are no NEW issues requiring immediate fixes, below is the analysis of pre-existing issues grouped by category and fix complexity.

  Category 1: Deprecated Marker Replacements (NON-BREAKING) - 11 issues

  These are straightforward marker updates from deprecated syntax to modern syntax.

  File: api/v1/clustercatalog_types.go

  Issue 1. Line 66 - ClusterCatalog.Spec
  // CURRENT:
  // +kubebuilder:validation:Required
  Spec ClusterCatalogSpec `json:"spec"`

  // FIX:
  // +required
  Spec ClusterCatalogSpec `json:"spec"`
  Why: Replace deprecated +kubebuilder:validation:Required with modern +required marker
  Breaking: NO

  Issue 2. Line 109 - ClusterCatalogSpec.Source
  // CURRENT:
  // +kubebuilder:validation:Required
  Source CatalogSource `json:"source"`

  // FIX:
  // +required
  Source CatalogSource `json:"source"`
  Why: Replace deprecated +kubebuilder:validation:Required with modern +required marker
  Breaking: NO

  Issue 3. Line 206 - ClusterCatalogURLs.Base
  // CURRENT:
  // +kubebuilder:validation:Required
  Base string `json:"base"`

  // FIX:
  // +required
  Base string `json:"base"`
  Why: Replace deprecated +kubebuilder:validation:Required with modern +required marker
  Breaking: NO

  Issue 4. Line 224 - CatalogSource.Type
  // CURRENT:
  // +kubebuilder:validation:Required
  Type SourceType `json:"type"`

  // FIX:
  // +required
  Type SourceType `json:"type"`
  Why: Replace deprecated +kubebuilder:validation:Required with modern +required marker
  Breaking: NO

  Issue 5. Line 245 - ResolvedCatalogSource.Type
  // CURRENT:
  // +kubebuilder:validation:Required
  Type SourceType `json:"type"`

  // FIX:
  // +required
  Type SourceType `json:"type"`
  Why: Replace deprecated +kubebuilder:validation:Required with modern +required marker
  Breaking: NO

  Issue 6. Line 264 - ResolvedImageSource.Ref
  // CURRENT:
  // +kubebuilder:validation:Required
  Ref string `json:"ref"`

  // FIX:
  // +required
  Ref string `json:"ref"`
  Why: Replace deprecated +kubebuilder:validation:Required with modern +required marker
  Breaking: NO

  Issue 7. Line 320 - ImageSource.Ref
  // CURRENT:
  // +kubebuilder:validation:Required
  Ref string `json:"ref"`

  // FIX:
  // +required
  Ref string `json:"ref"`
  Why: Replace deprecated +kubebuilder:validation:Required with modern +required marker
  Breaking: NO

  File: api/v1/clusterextension_types.go

  Issue 8. Line 67 - ClusterExtensionSpec.Namespace
  // CURRENT:
  // +kubebuilder:validation:Required
  Namespace string `json:"namespace"`

  // FIX:
  // +required
  Namespace string `json:"namespace"`
  Why: Replace deprecated +kubebuilder:validation:Required with modern +required marker
  Breaking: NO

  Issue 9. Line 76 - ClusterExtensionSpec.ServiceAccount
  // CURRENT:
  // +kubebuilder:validation:Required
  ServiceAccount ServiceAccountReference `json:"serviceAccount"`

  // FIX:
  // +required
  ServiceAccount ServiceAccountReference `json:"serviceAccount"`
  Why: Replace deprecated +kubebuilder:validation:Required with modern +required marker
  Breaking: NO

  Issue 10. Line 92 - ClusterExtensionSpec.Source
  // CURRENT:
  // +kubebuilder:validation:Required
  Source SourceConfig `json:"source"`

  // FIX:
  // +required
  Source SourceConfig `json:"source"`
  Why: Replace deprecated +kubebuilder:validation:Required with modern +required marker
  Breaking: NO

  Issue 11. Line 131 - SourceConfig.SourceType
  // CURRENT:
  // +kubebuilder:validation:Required
  SourceType string `json:"sourceType"`

  // FIX:
  // +required
  SourceType string `json:"sourceType"`
  Why: Replace deprecated +kubebuilder:validation:Required with modern +required marker
  Breaking: NO

  Category 2: Missing Optional/Required Markers (NON-BREAKING) - 12 issues

  These fields need explicit +optional or +required markers.

  File: api/v1/clustercatalog_types.go

  Issue 12. Line 61 - ClusterCatalog.metav1.ObjectMeta
  // CURRENT:
  metav1.ObjectMeta `json:"metadata"`

  // FIX:
  // +required
  metav1.ObjectMeta `json:"metadata"`
  Why: ObjectMeta is always present in Kubernetes resources
  Breaking: NO (Already enforced by Kubernetes API server)

  Issue 13. Line 248 - ResolvedCatalogSource.Image
  // CURRENT:
  Image *ResolvedImageSource `json:"image"`

  // FIX:
  // +optional
  Image *ResolvedImageSource `json:"image"`
  Why: Field is a pointer, indicating it's optional
  Breaking: NO

  File: api/v1/clusterextension_types.go

  Issue 14. Line 171 - ClusterExtensionConfig.ConfigType
  // CURRENT:
  // +kubebuilder:validation:Required
  ConfigType ClusterExtensionConfigType `json:"configType"`

  // FIX:
  // +required
  ConfigType ClusterExtensionConfigType `json:"configType"`
  Why: Replace deprecated marker
  Breaking: NO

  Issue 15. Line 214 - CatalogFilter.PackageName
  // CURRENT:
  // +kubebuilder:validation.Required (note the dot instead of colon - this is a typo!)
  PackageName string `json:"packageName"`

  // FIX:
  // +required
  PackageName string `json:"packageName"`
  Why: Fix typo in deprecated marker and use modern syntax
  Breaking: NO

  Issue 16. Line 397 - ServiceAccountReference.Name
  // CURRENT:
  // +kubebuilder:validation:Required
  Name string `json:"name"`

  // FIX:
  // +required
  Name string `json:"name"`
  Why: Replace deprecated marker
  Breaking: NO

  Issue 17. Line 409 - PreflightConfig.CRDUpgradeSafety
  // CURRENT:
  CRDUpgradeSafety *CRDUpgradeSafetyPreflightConfig `json:"crdUpgradeSafety"`

  // FIX:
  // +required
  CRDUpgradeSafety *CRDUpgradeSafetyPreflightConfig `json:"crdUpgradeSafety"`
  Usage Analysis:
  - XValidation rule at line 402: rule="has(self.crdUpgradeSafety)" - requires field to be present
  - Field is validated as required through CEL expression

  Why: CEL validation requires this field, should be marked +required for consistency
  Breaking: NO (Already enforced by CEL validation)

  Issue 18. Line 425 - CRDUpgradeSafetyPreflightConfig.Enforcement
  // CURRENT:
  // +kubebuilder:validation:Required
  Enforcement CRDUpgradeSafetyEnforcement `json:"enforcement"`

  // FIX:
  // +required
  Enforcement CRDUpgradeSafetyEnforcement `json:"enforcement"`
  Why: Replace deprecated marker
  Breaking: NO

  Issue 19. Line 450 - BundleMetadata.Name
  // CURRENT:
  // +kubebuilder:validation:Required
  Name string `json:"name"`

  // FIX:
  // +required
  Name string `json:"name"`
  Why: Replace deprecated marker
  Breaking: NO

  Issue 20. Line 457 - BundleMetadata.Version
  // CURRENT:
  // +kubebuilder:validation:Required
  Version string `json:"version"`

  // FIX:
  // +required
  Version string `json:"version"`
  Why: Replace deprecated marker
  Breaking: NO

  Issue 21. Line 463 - RevisionStatus.Name
  // CURRENT:
  Name string `json:"name"`

  // FIX:
  // +required
  Name string `json:"name"`
  Usage Analysis:
  - Used in api/v1/clusterextension_types.go:516 as part of listMapKey=name
  - Map key fields must always be present

  Why: This field is a listMapKey, which requires it to always be present
  Breaking: NO (Already required by listMapKey constraint)

  Issue 22. Line 527 - ClusterExtensionInstallStatus.Bundle
  // CURRENT:
  // +kubebuilder:validation:Required
  Bundle BundleMetadata `json:"bundle"`

  // FIX:
  // +required
  Bundle BundleMetadata `json:"bundle"`
  Why: Replace deprecated marker
  Breaking: NO

  Issue 23. Line 550 - ClusterExtension.Spec
  // CURRENT:
  Spec ClusterExtensionSpec `json:"spec,omitempty"`

  // FIX:
  // +required
  Spec ClusterExtensionSpec `json:"spec"`
  Why: Spec has required fields and should not have omitempty
  Breaking: YES - Removing omitempty could affect API behavior

  File: api/v1/clusterextensionrevision_types.go

  Issue 24. Line 55 - ClusterExtensionRevisionSpec.LifecycleState
  // CURRENT:
  // +kubebuilder:default="Active"
  LifecycleState ClusterExtensionRevisionLifecycleState `json:"lifecycleState,omitempty"`

  // FIX:
  // +kubebuilder:default="Active"
  // +optional
  LifecycleState ClusterExtensionRevisionLifecycleState `json:"lifecycleState,omitempty"`
  Why: Field has omitempty and a default, indicating it's optional
  Breaking: NO

  Issue 25. Line 67 - ClusterExtensionRevisionSpec.Revision
  // CURRENT:
  // +kubebuilder:validation:Required
  Revision int64 `json:"revision"`

  // FIX:
  // +required
  Revision int64 `json:"revision"`
  Why: Replace deprecated marker
  Breaking: NO

  Issue 26. Line 119 - ClusterExtensionRevisionPhase.Name
  // CURRENT:
  Name string `json:"name"`

  // FIX:
  // +required
  Name string `json:"name"`
  Usage Analysis:
  - Used as listMapKey at line 87
  - Map key fields must always be present

  Why: This field is a listMapKey, which requires it to always be present
  Breaking: NO (Already required by listMapKey constraint)

  Issue 27. Line 136 - ClusterExtensionRevisionObject.Object
  // CURRENT:
  Object unstructured.Unstructured `json:"object"`

  // FIX:
  // +required
  Object unstructured.Unstructured `json:"object"`
  Why: The object field is the core content of this struct and is always needed
  Breaking: NO

  Issue 28. Line 227 - ClusterExtensionRevision.Spec
  // CURRENT:
  Spec ClusterExtensionRevisionSpec `json:"spec,omitempty"`

  // FIX:
  // +required
  Spec ClusterExtensionRevisionSpec `json:"spec"`
  Why: Spec has required fields (Revision) and should not have omitempty
  Breaking: YES - Removing omitempty could affect API behavior

  Category 3: Missing omitzero Tag (NON-BREAKING) - 2 issues

  File: api/v1/clustercatalog_types.go

  Issue 29. Line 178 - ClusterCatalogStatus.ResolvedSource
  // CURRENT:
  // +optional
  ResolvedSource *ResolvedCatalogSource `json:"resolvedSource,omitempty"`

  // FIX:
  // +optional
  ResolvedSource *ResolvedCatalogSource `json:"resolvedSource,omitzero"`
  Why: Pointer field that should not serialize when zero
  Breaking: NO (Changes serialization but semantically equivalent)

  File: api/v1/clusterextension_types.go

  Issue 30. Line 110 - ClusterExtensionSpec.Config
  // CURRENT:
  // +optional
  Config *ClusterExtensionConfig `json:"config,omitempty"`

  // FIX:
  // +optional
  Config *ClusterExtensionConfig `json:"config,omitzero"`
  Why: Pointer field that should not serialize when zero
  Breaking: NO (Changes serialization but semantically equivalent)

  Category 4: Optional Fields Without Complete Validation (ANALYSIS NEEDED) - 2 issues

  These fields may need to be converted to pointers OR have validation completed.

  File: api/v1/clustercatalog_types.go

  Issue 31. Line 131 - ClusterCatalogSpec.Priority
  // CURRENT:
  // +kubebuilder:default:=0
  // +kubebuilder:validation:minimum:=-2147483648
  // +kubebuilder:validation:maximum:=2147483647
  // +optional
  Priority int32 `json:"priority"`

  // FIX:
  // Keep as-is - validation IS complete with min/max
  Usage Analysis:
  - Used in internal/operator-controller/resolve/catalog.go:163 as direct cat.Spec.Priority
  - Used in tests with explicit values like Priority: 1, Priority: 0
  - Has default value of 0, min/max validation
  - Zero value (0) is semantically valid (documented as default priority)

  Why: This is a FALSE POSITIVE. The validation IS complete (min/max specified). Zero value is valid and documented. The field should remain as int32 with default=0.
  Breaking: N/A - No change needed

  File: api/v1/clusterextension_types.go

  Issue 32. Line 294 - CatalogFilter.Version
  // CURRENT:
  // +optional
  Version string `json:"version,omitempty"`

  // FIX:
  // Keep as-is - this is correct
  Usage Analysis:
  - Used in test/upgrade-e2e/post_upgrade_test.go:182: clusterExtension.Spec.Source.Catalog.Version = "1.0.1"
  - Empty string means "latest version" per documentation at line 217
  - Field is truly optional with semantic meaning for empty value
  - Has complex validation regex but empty is explicitly allowed

  Why: This is a FALSE POSITIVE. Empty string has semantic meaning ("latest version"). The field is correctly optional with omitempty.
  Breaking: N/A - No change needed

  Category 5: Integer Type Issues (BREAKING) - 1 issue

  File: api/v1/clustercatalog_types.go

  Issue 33. Line 328 - ImageSource.PollIntervalMinutes
  // CURRENT:
  // +optional
  PollIntervalMinutes *int `json:"pollIntervalMinutes,omitempty"`

  // FIX:
  // +optional
  PollIntervalMinutes *int32 `json:"pollIntervalMinutes,omitempty"`
  Usage Analysis:
  - Used in internal/catalogd/controllers/core/clustercatalog_controller.go:308-309: pointer nil check then dereference
  - Used in tests with ptr.To(1), ptr.To(5), etc. - all small positive integers
  - Converted to time.Duration by multiplying with time.Minute
  - Min validation is 1, practical max is far below int32 range

  Why: Kubernetes API guidelines recommend int32 or int64 over int for portability. Values are always small positive integers (minutes), so int32 is appropriate.
  Breaking: YES - Changes the type from platform-dependent int to int32. Could affect 32-bit vs 64-bit platforms differently, though in practice this field only holds small values.

  Category 6: Missing listType Marker for SSA (NON-BREAKING) - 1 issue

  File: api/v1/clusterextension_types.go

  Issue 34. Line 338 - CatalogFilter.Channels
  // CURRENT:
  // +optional
  Channels []string `json:"channels,omitempty"`

  // FIX:
  // +listType=set
  // +optional
  Channels []string `json:"channels,omitempty"`
  Why: Array of strings representing channel names. Duplicates don't make semantic sense (you wouldn't specify the same channel twice), so set is appropriate.
  Breaking: NO (Improves Server-Side Apply behavior but doesn't change validation)

  Category 7: Array of Struct Without Required Fields (NEEDS ANALYSIS) - 2 issues

  File: api/v1/clusterextension_types.go

  Issue 35. Line 516 - ClusterExtensionStatus.ActiveRevisions
  // CURRENT:
  // +listType=map
  // +listMapKey=name
  // +optional
  ActiveRevisions []RevisionStatus `json:"activeRevisions,omitempty"`

  // FIX - Option 1: Add +required to RevisionStatus.Name (recommended)
  // Already covered in Issue #21 above

  // FIX - Option 2: Mark Name as +required in RevisionStatus struct
  // See Issue #21
  Why: The warning is about RevisionStatus having no required fields. Since Name is already the listMapKey, marking it as +required (Issue #21) will resolve this.
  Breaking: NO (Issue #21 is already non-breaking)

  File: api/v1/clusterextensionrevision_types.go

  Issue 36. Line 89 - ClusterExtensionRevisionSpec.Phases
  // CURRENT:
  // +listType=map
  // +listMapKey=name
  // +optional
  Phases []ClusterExtensionRevisionPhase `json:"phases,omitempty"`

  // FIX: Mark ClusterExtensionRevisionPhase.Name as +required
  // Already covered in Issue #26 above
  Why: The warning is about ClusterExtensionRevisionPhase having no required fields. Since Name is the listMapKey, marking it as +required (Issue #26) will resolve this.
  Breaking: NO (Issue #26 is already non-breaking)

  Issue 37. Line 124 - ClusterExtensionRevisionPhase.Objects
  // CURRENT:
  Objects []ClusterExtensionRevisionObject `json:"objects"`

  // FIX: Mark ClusterExtensionRevisionObject.Object as +required
  // Already covered in Issue #27 above
  Why: The warning is about ClusterExtensionRevisionObject having no required fields. Marking Object as +required (Issue #27) will resolve this.
  Breaking: NO (Issue #27 is already non-breaking)

  ---
  Summary of Issues by Fix Complexity

  Non-Breaking Changes (36 issues):

  - 11 deprecated marker replacements
  - 12 missing optional/required markers
  - 2 missing omitzero tags
  - 2 false positives (Priority and Version - no change needed)
  - 1 missing listType marker
  - 3 array of struct issues (resolved by marking key fields as required)
  - 6 false positives already filtered (Status fields and Conditions markers)

  Breaking Changes (2 issues):

  1. ImageSource.PollIntervalMinutes (int → int32) - Type change for Kubernetes compatibility
  2. ClusterExtension.Spec and ClusterExtensionRevision.Spec - Remove omitempty from required Spec fields

  ---
  Recommendations

  1. Immediate action: None required for this PR. You successfully fixed 3 issues without introducing new ones.
  2. Future PRs to address pre-existing issues:
    - Quick win PR: Fix all 11 deprecated marker replacements (straightforward, non-breaking)
    - SSA improvements PR: Add listType markers and fix omitzero tags (non-breaking, improves UX)
    - Breaking changes PR: Address int→int32 type change and Spec omitempty removal (requires careful migration planning)

Copilot AI review requested due to automatic review settings December 24, 2025 11:42
@camilamacedo86 camilamacedo86 requested a review from a team as a code owner December 24, 2025 11:42
@openshift-ci
Copy link

openshift-ci bot commented Dec 24, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign tmshort 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:

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 requested review from OchiengEd and tmshort December 24, 2025 11:42
@netlify
Copy link

netlify bot commented Dec 24, 2025

Deploy Preview for olmv1 ready!

Name Link
🔨 Latest commit cf7764d
🔍 Latest deploy log https://app.netlify.com/projects/olmv1/deploys/694bd1bb5ec4b20008f7be16
😎 Deploy Preview https://deploy-preview-2410--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances the api-lint-diff tool to detect and report FIXED issues, providing better visibility into API linting improvements made in a branch. The tool now categorizes issues into three types: FIXED (resolved in the current branch), NEW (introduced), and PRE-EXISTING (existed before changes).

Key Changes:

  • Extended the categorize_issues function to identify issues present in the baseline but absent in the current branch
  • Updated report generation to display FIXED issues count and details with celebratory messaging
  • Enhanced Claude AI command documentation to include FIXED issues in the analysis workflow

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
hack/api-lint-diff/run.sh Added logic to detect FIXED issues by comparing baseline against current branch, updated report generation to display fixed issues count and details, and improved success messages to acknowledge fixes
.claude/commands/api-lint-diff.md Updated documentation to include FIXED issues in the categorization, analysis workflow, and report format template

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link

codecov bot commented Dec 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.82%. Comparing base (578dc33) to head (cf7764d).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2410   +/-   ##
=======================================
  Coverage   68.82%   68.82%           
=======================================
  Files         100      100           
  Lines        7641     7641           
=======================================
  Hits         5259     5259           
  Misses       1947     1947           
  Partials      435      435           
Flag Coverage Δ
e2e 43.93% <ø> (ø)
experimental-e2e 13.68% <ø> (ø)
unit 57.11% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant