OCPSTRAT-2876: Allow inclusion/exclusion of manifests based on the OCP major version#1282
Open
JoelSpeed wants to merge 2 commits intoopenshift:mainfrom
Open
OCPSTRAT-2876: Allow inclusion/exclusion of manifests based on the OCP major version#1282JoelSpeed wants to merge 2 commits intoopenshift:mainfrom
JoelSpeed wants to merge 2 commits intoopenshift:mainfrom
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
Contributor
|
Skipping CI for Draft Pull Request. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRelease semver is parsed and the major version is threaded through payload render and CVO flows; manifest inclusion/filtering now considers Changes
Sequence DiagramsequenceDiagram
participant Release as Release Metadata
participant Parser as Version Parser
participant Payload as Payload/Render
participant CVO as CVO Sync Worker
participant Filter as Manifest Include/Filter
participant Cluster as Cluster API
Release->>Parser: loadReleaseVersion()
Parser->>Parser: Parse semantic version (e.g. 5.0.0)
Parser->>Payload: provide ParsedVersion.Major
Payload->>CVO: pass majorVersion (*uint64)
CVO->>Filter: GetImplicitlyEnabledCapabilities(manifests, ..., majorVersion)
CVO->>Filter: Include(manifests..., majorVersion)
Filter->>Filter: Evaluate `release.openshift.io/major-version` annotation against majorVersion
alt matches major
Filter->>Cluster: apply manifest
else excluded
Filter-->>CVO: skip manifest
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: JoelSpeed 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 |
JoelSpeed
force-pushed
the
major-version-inclusion
branch
2 times, most recently
from
39354c8 to
ef0e8c6
Compare
JoelSpeed
changed the title
openshift-ci-robot
added
the
jira/valid-reference
Contributor
|
@JoelSpeed: This pull request references OCPSTRAT-2876 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the feature to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
JoelSpeed
force-pushed
the
major-version-inclusion
branch
from
ef0e8c6 to
2d6aae6
Compare
Contributor
|
@JoelSpeed: This pull request references OCPSTRAT-2876 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the feature to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
JoelSpeed
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
Contributor
Author
|
@DavidHurta If you are available, I'd appreciate if you could help me with this feature as well |
Contributor
|
@JoelSpeed, I will be happy to assist. /cc |
DavidHurta
reviewed
Feb 12, 2026
Contributor
DavidHurta
left a comment
DavidHurta
left a comment
There was a problem hiding this comment.
The PR looks solid!
A few comments about a thing here or there, but nothing drastic.
The overall functionality is 👍
pkg/payload/payload.go
Outdated
| ImageRef *imagev1.ImageStream | ||
|
|
||
| Architecture string | ||
| MajorVersion *uint64 |
Contributor
There was a problem hiding this comment.
Here, the purpose of MajorVersion is starting to become a bit fuzzy in comparison to the Release.Version field. The meaning of MajorVersion in the inclusion filtering context is clear; however, here we use it in a general struct type representing the contents of a release image. Thus, their purpose becomes less clear.
We could do one of the following:
- Document
MajorVersionfield to make the difference betweenRelease.VersionandMajorVersionclearer. - Leave only one source of truth (that is,
Release.Version), and extract the major version when needed using a function. - Create a new field
ParsedVersion semver.Version; we could use it to get access to the parsed cached version of the update easily. This could even be utilized in the future when we need easy access to the parsed version of an update, which is more handy for fine grained processing. This would bring additional value to the new field, make it more general, and make its distinction clearer. The call sites requiring theMajorVersionwould need to be updated to justptr.To(payload.ParsedVersion.Major), so it would still be readable. I am more inclined towards this approach, but I am curious what you think.
Contributor
Author
There was a problem hiding this comment.
I like the parsed version approach, I'll see what that looks like locally
| return requiredFeatureSet, enabledFeatureGates, nil | ||
| } | ||
|
|
||
| func loadReleaseVersion(releaseDir string) (semver.Version, error) { |
Contributor
There was a problem hiding this comment.
Here we start to duplicate some logic. I am curious whether we could leverage the existing loadReleaseMetadata function here instead of manually loading the release version.
The loadReleaseMetadata function also contains some additional validation, such as:
if metadata.Kind != "cincinnati-metadata-v0" {
return release, fmt.Errorf("unrecognized Cincinnati metadata kind %q", metadata.Kind)
}
Contributor
There was a problem hiding this comment.
Something like:
func loadReleaseVersion(releaseDir string) (semver.Version, error) {
release, err := loadReleaseMetadata(releaseDir)
if err != nil {
return semver.Version{}, err
}
parsedVersion, err := semver.Parse(release.Version)
if err != nil {
return semver.Version{}, err
}
return parsedVersion, nil
}| } | ||
| } | ||
|
|
||
| func TestRenderDirWithMajorVersionFiltering(t *testing.T) { |
Contributor
There was a problem hiding this comment.
Comment not related to the selected line
Thank you for the complex testing of the rendering!
The overall testing coverage of the PR is solid. However, we do not have an integration test to determine whether the CVO will actually apply the manifest based on the major version. I am doubtful whether this feature will receive end-to-end tests. Thus, I think it would be beneficial to increase the test coverage to ensure that the CVO truly does respect the major version when loading and applying manifests, WDYT?
Something like: DavidHurta@ee1907b
JoelSpeed
force-pushed
the
major-version-inclusion
branch
from
2d6aae6 to
adefaa3
Compare
Contributor
|
@JoelSpeed: This pull request references OCPSTRAT-2876 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the feature to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
JoelSpeed
force-pushed
the
major-version-inclusion
branch
from
adefaa3 to
ede695d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Line 37: Remove the personal-fork replace directive that points "replace
github.com/openshift/library-go => github.com/JoelSpeed/library-go
v0.0.0-20260130121715-6e5ccfd0da42" from go.mod and instead reference the
official upstream module; update the module requirement for
github.com/openshift/library-go to the appropriate upstream version/tag (or
remove the replace entirely if the required upstream version is already
satisfied) so builds use the official org package rather than the JoelSpeed
fork.
In `@lib/manifest/manifest.go`:
- Line 67: The test suite lacks coverage for cross-major-version behavior in
GetImplicitlyEnabledCapabilities: add a new case within
TestGetImplicitlyEnabledCapabilities that constructs a currentManifest and an
updateManifest with different MajorVersion values (e.g., currentMajor=4,
updateMajor=5) and invokes the code path that uses
manifestInclusionConfiguration.MajorVersion for each manifest evaluation; ensure
the updateManifest is evaluated with the update payload's MajorVersion (set the
manifestInclusionConfiguration used for updateManifest accordingly) and assert
the returned implicitly enabled capabilities reflect the update-major evaluation
rather than the current-major one, referencing
TestGetImplicitlyEnabledCapabilities, GetImplicitlyEnabledCapabilities,
currentManifest, updateManifest, and manifestInclusionConfiguration when
locating where to add the case.
In `@pkg/cvo/sync_worker.go`:
- Around line 451-452: The implicit-capability calculation is using the current
payload major version (w.payload.ParsedVersion.Major) instead of the
target/update payload major version, causing incorrect results during
cross-major upgrades; update the call to
payload.GetImplicitlyEnabledCapabilities (the invocation that assigns
implicitlyEnabledCaps) to pass the target payload's major version
(payloadUpdate.ParsedVersion.Major) via ptr.To(...) rather than
w.payload.ParsedVersion.Major so the evaluation uses the update payload's
version.
In
`@pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_10_no-version.yaml`:
- Around line 4-11: The CRD's metadata.name is invalid — update metadata.name to
follow the required "<plural>.<group>" format by replacing the current value of
metadata.name with the concatenation of spec.names.plural and spec.group (e.g.,
set metadata.name to "noversions.test.io") so it matches spec.names.plural and
spec.group; verify the fields metadata.name, spec.names.plural, and spec.group
are consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 86b819d9-2e3c-4b03-9cde-3e35027f9e1a
⛔ Files ignored due to path filters (21)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/api/config/v1/types_apiserver.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_authentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_infrastructure.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/types_backup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/features.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/manifest/manifest.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (17)
go.modlib/manifest/manifest.gopkg/cvo/cvo_scenarios_test.gopkg/cvo/featuregate_integration_test.gopkg/cvo/sync_worker.gopkg/cvo/testdata/majorversiontest-v5/manifests/.gitkeeppkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_10_no-version.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_20_version-4-only.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_30_version-5-only.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_40_version-4-or-5.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_50_exclude-version-3.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/image-referencespkg/cvo/testdata/majorversiontest-v5/release-manifests/release-metadatapkg/payload/payload.gopkg/payload/payload_test.gopkg/payload/render.gopkg/payload/render_test.go
go.mod
Outdated
| k8s.io/utils v0.0.0-20250604170112-4c0f3b243397 | ||
| ) | ||
|
|
||
| replace github.com/openshift/library-go => github.com/JoelSpeed/library-go v0.0.0-20260130121715-6e5ccfd0da42 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify whether go.mod contains a personal-fork replacement for library-go.
rg -n '^replace\s+github.com/openshift/library-go\s*=>\s*github.com/[^/]+/library-go' go.mod
# Show related direct requirement lines for context.
rg -n 'github.com/openshift/library-go|^replace ' go.modRepository: openshift/cluster-version-operator
Length of output: 487
Remove the personal-fork replace directive and update to the upstream github.com/openshift/library-go version.
Using a non-org fork for production dependencies weakens provenance, reproducibility, and long-term maintainability. The current replace at line 37 redirects to github.com/JoelSpeed/library-go, which bypasses official org version control. Switch to the upstream version before merging.
Proposed change
-replace github.com/openshift/library-go => github.com/JoelSpeed/library-go v0.0.0-20260130121715-6e5ccfd0da42🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go.mod` at line 37, Remove the personal-fork replace directive that points
"replace github.com/openshift/library-go => github.com/JoelSpeed/library-go
v0.0.0-20260130121715-6e5ccfd0da42" from go.mod and instead reference the
official upstream module; update the module requirement for
github.com/openshift/library-go to the appropriate upstream version/tag (or
remove the replace entirely if the required upstream version is already
satisfied) so builds use the official org package rather than the JoelSpeed
fork.
Comment on lines
+4
to
+11
| name: noversion | ||
| annotations: | ||
| include.release.openshift.io/self-managed-high-availability: "true" | ||
| spec: | ||
| group: test.io | ||
| names: | ||
| kind: NoVersion | ||
| plural: noversions |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Kubernetes CustomResourceDefinition metadata.name format spec.names.plural spec.group
💡 Result:
For a Kubernetes CustomResourceDefinition (CRD), the required relationship is:
metadata.nameMUST be:
<spec.names.plural>.<spec.group>(v1-32.docs.kubernetes.io)
So, given:
spec.group: stable.example.comspec.names.plural: crontabs
Then:
metadata.name: crontabs.stable.example.com(v1-32.docs.kubernetes.io)
Additional relevant constraints:
spec.names.pluralmust be all lowercase. (kubernetes.io)
Citations:
- 1: https://v1-32.docs.kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/?utm_source=openai
- 2: https://v1-32.docs.kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/?utm_source=openai
- 3: https://kubernetes.io/zh-cn/docs/reference/kubernetes-api/extend-resources/custom-resource-definition-v1?utm_source=openai
Use a valid CRD name format (<plural>.<group>)
Line 4 sets metadata.name: noversion, but with spec.names.plural: noversions and spec.group: test.io, this must be noversions.test.io per the Kubernetes CRD API contract. Invalid CRD names violate the required naming constraint and will fail validation.
Suggested fix
metadata:
- name: noversion
+ name: noversions.test.io
annotations:
include.release.openshift.io/self-managed-high-availability: "true"📝 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.
Suggested change
| name: noversion | |
| annotations: | |
| include.release.openshift.io/self-managed-high-availability: "true" | |
| spec: | |
| group: test.io | |
| names: | |
| kind: NoVersion | |
| plural: noversions | |
| name: noversions.test.io | |
| annotations: | |
| include.release.openshift.io/self-managed-high-availability: "true" | |
| spec: | |
| group: test.io | |
| names: | |
| kind: NoVersion | |
| plural: noversions |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_10_no-version.yaml`
around lines 4 - 11, The CRD's metadata.name is invalid — update metadata.name
to follow the required "<plural>.<group>" format by replacing the current value
of metadata.name with the concatenation of spec.names.plural and spec.group
(e.g., set metadata.name to "noversions.test.io") so it matches
spec.names.plural and spec.group; verify the fields metadata.name,
spec.names.plural, and spec.group are consistent.
Contributor
|
@JoelSpeed: This pull request references OCPSTRAT-2876 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the feature to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
JoelSpeed
force-pushed
the
major-version-inclusion
branch
from
ede695d to
47b4565
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (3)
pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_10_no-version.yaml (1)
4-11:⚠️ Potential issue | 🟠 MajorCRD name mismatch remains unresolved (
metadata.namevs<plural>.<group>).This still should be
noversions.test.ioto satisfy CRD naming requirements.Suggested fix
metadata: - name: noversion + name: noversions.test.io🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_10_no-version.yaml` around lines 4 - 11, The CRD's metadata.name is invalidly set to "noversion"; update metadata.name to match the required <plural>.<group> pattern by setting it to "noversions.test.io" so it matches spec.names.plural ("noversions") and spec.group ("test.io")—look for the metadata.name field and the spec.names.plural / spec.group entries in this manifest (the resource currently named "noversion") and change metadata.name to "noversions.test.io".lib/manifest/manifest.go (1)
60-87:⚠️ Potential issue | 🟠 MajorSeparate major-version context for update vs current manifest evaluation.
Both branches currently use the same
MajorVersion. In cross-major upgrades,currentManifestshould be evaluated with the current payload major, whileupdateManifestshould use the target payload major.Possible fix direction
-func GetImplicitlyEnabledCapabilities( - updatePayloadManifests []manifest.Manifest, - currentPayloadManifests []manifest.Manifest, - manifestInclusionConfiguration InclusionConfiguration, - currentImplicitlyEnabled sets.Set[configv1.ClusterVersionCapability], -) sets.Set[configv1.ClusterVersionCapability] { +func GetImplicitlyEnabledCapabilities( + updatePayloadManifests []manifest.Manifest, + currentPayloadManifests []manifest.Manifest, + updateInclusionConfiguration InclusionConfiguration, + currentInclusionConfiguration InclusionConfiguration, + currentImplicitlyEnabled sets.Set[configv1.ClusterVersionCapability], +) sets.Set[configv1.ClusterVersionCapability] { // update manifest include check - manifestInclusionConfiguration.MajorVersion, + updateInclusionConfiguration.MajorVersion, // current manifest include check - manifestInclusionConfiguration.MajorVersion, + currentInclusionConfiguration.MajorVersion,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/manifest/manifest.go` around lines 60 - 87, The two IncludeAllowUnknownCapabilities calls must evaluate with different major-version contexts: keep using manifestInclusionConfiguration.MajorVersion (target major) when calling updateManifest.IncludeAllowUnknownCapabilities (and assigning updateManErr), but when iterating currentPayloadManifests call currentManifest.IncludeAllowUnknownCapabilities with the current payload major instead of reusing the same MajorVersion; obtain the current payload major from the current payload metadata (or add a currentMajor variable derived from currentManifest/currentPayload) and pass that as the MajorVersion argument so updateManifest uses the target major and currentManifest uses the current-major context.pkg/cvo/sync_worker.go (1)
451-452:⚠️ Potential issue | 🟠 MajorUse target payload major version for implicit-capability evaluation.
At Line [452], this still passes the current payload major (
w.payload.ParsedVersion.Major). For cross-major updates, implicit-capability resolution should use the target payload major (payloadUpdate.ParsedVersion.Major), or manifests can be evaluated against the wrong major-version filter.Suggested fix
- implicitlyEnabledCaps = capability.SortedList(payload.GetImplicitlyEnabledCapabilities(payloadUpdate.Manifests, w.payload.Manifests, - work.Capabilities, work.EnabledFeatureGates, ptr.To(w.payload.ParsedVersion.Major))) + implicitlyEnabledCaps = capability.SortedList(payload.GetImplicitlyEnabledCapabilities(payloadUpdate.Manifests, w.payload.Manifests, + work.Capabilities, work.EnabledFeatureGates, ptr.To(payloadUpdate.ParsedVersion.Major)))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cvo/sync_worker.go` around lines 451 - 452, The call computing implicitlyEnabledCaps uses the current payload major (w.payload.ParsedVersion.Major) but should use the target payload major from payloadUpdate for cross-major updates: update the call to payload.GetImplicitlyEnabledCapabilities so its last argument is ptr.To(payloadUpdate.ParsedVersion.Major) (keeping capability.SortedList and assignment to implicitlyEnabledCaps unchanged) to ensure manifest filtering uses the target payload major instead of w.payload.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@lib/manifest/manifest.go`:
- Around line 60-87: The two IncludeAllowUnknownCapabilities calls must evaluate
with different major-version contexts: keep using
manifestInclusionConfiguration.MajorVersion (target major) when calling
updateManifest.IncludeAllowUnknownCapabilities (and assigning updateManErr), but
when iterating currentPayloadManifests call
currentManifest.IncludeAllowUnknownCapabilities with the current payload major
instead of reusing the same MajorVersion; obtain the current payload major from
the current payload metadata (or add a currentMajor variable derived from
currentManifest/currentPayload) and pass that as the MajorVersion argument so
updateManifest uses the target major and currentManifest uses the current-major
context.
In `@pkg/cvo/sync_worker.go`:
- Around line 451-452: The call computing implicitlyEnabledCaps uses the current
payload major (w.payload.ParsedVersion.Major) but should use the target payload
major from payloadUpdate for cross-major updates: update the call to
payload.GetImplicitlyEnabledCapabilities so its last argument is
ptr.To(payloadUpdate.ParsedVersion.Major) (keeping capability.SortedList and
assignment to implicitlyEnabledCaps unchanged) to ensure manifest filtering uses
the target payload major instead of w.payload.
In
`@pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_10_no-version.yaml`:
- Around line 4-11: The CRD's metadata.name is invalidly set to "noversion";
update metadata.name to match the required <plural>.<group> pattern by setting
it to "noversions.test.io" so it matches spec.names.plural ("noversions") and
spec.group ("test.io")—look for the metadata.name field and the
spec.names.plural / spec.group entries in this manifest (the resource currently
named "noversion") and change metadata.name to "noversions.test.io".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 366b0564-c600-4e8f-9a02-0bbc9a478b95
⛔ Files ignored due to path filters (21)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/api/config/v1/types_apiserver.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_authentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_infrastructure.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/types_backup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/features.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/manifest/manifest.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (17)
go.modlib/manifest/manifest.gopkg/cvo/cvo_scenarios_test.gopkg/cvo/featuregate_integration_test.gopkg/cvo/sync_worker.gopkg/cvo/testdata/majorversiontest-v5/manifests/.gitkeeppkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_10_no-version.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_20_version-4-only.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_30_version-5-only.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_40_version-4-or-5.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_50_exclude-version-3.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/image-referencespkg/cvo/testdata/majorversiontest-v5/release-manifests/release-metadatapkg/payload/payload.gopkg/payload/payload_test.gopkg/payload/render.gopkg/payload/render_test.go
🚧 Files skipped from review as they are similar to previous changes (7)
- pkg/cvo/testdata/majorversiontest-v5/release-manifests/image-references
- pkg/cvo/testdata/majorversiontest-v5/release-manifests/release-metadata
- go.mod
- pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_30_version-5-only.yaml
- pkg/payload/payload_test.go
- pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_20_version-4-only.yaml
- pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_40_version-4-or-5.yaml
Contributor
|
@JoelSpeed: This pull request references OCPSTRAT-2876 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the feature to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_10_no-version.yaml (1)
4-11:⚠️ Potential issue | 🟠 MajorUse a valid CRD name format (
<plural>.<group>).Line 4 uses
metadata.name: noversion, but withspec.names.plural: noversionsandspec.group: test.io, it should benoversions.test.io. This can hide real validation failures in tests that model CRD application.Suggested fix
metadata: - name: noversion + name: noversions.test.io annotations: include.release.openshift.io/self-managed-high-availability: "true"Run this read-only check to find CRD name mismatches in the same testdata set:
#!/bin/bash set -euo pipefail python - <<'PY' import glob import re import sys def parse_fields(path): name = plural = group = None section = None for raw in open(path, encoding="utf-8"): line = raw.rstrip("\n") if re.match(r'^\s*metadata:\s*$', line): section = "metadata" continue if re.match(r'^\s*spec:\s*$', line): section = "spec" continue if re.match(r'^\s*names:\s*$', line): section = "names" continue m = re.match(r'^\s*name:\s*("?[^"]+"?|\S+)\s*$', line) if m and section == "metadata" and name is None: name = m.group(1).strip('"') continue m = re.match(r'^\s*group:\s*("?[^"]+"?|\S+)\s*$', line) if m and section == "spec": group = m.group(1).strip('"') continue m = re.match(r'^\s*plural:\s*("?[^"]+"?|\S+)\s*$', line) if m and section == "names": plural = m.group(1).strip('"') continue return name, plural, group failed = False for path in sorted(glob.glob("pkg/cvo/testdata/majorversiontest-v5/release-manifests/*.yaml")): text = open(path, encoding="utf-8").read() if "kind: CustomResourceDefinition" not in text: continue name, plural, group = parse_fields(path) if not (name and plural and group): continue expected = f"{plural}.{group}" if name != expected: failed = True print(f"{path}: metadata.name={name!r}, expected={expected!r}") if failed: sys.exit(1) print("All CRD metadata.name values match <plural>.<group>.") PY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_10_no-version.yaml` around lines 4 - 11, The CRD's metadata.name is incorrect — it should be the fully-qualified "<plural>.<group>" value; update metadata.name from "noversion" to "noversions.test.io" so that metadata.name equals spec.names.plural + "." + spec.group (ensure fields referenced: metadata.name, spec.names.plural, spec.group, kind: NoVersion).
🧹 Nitpick comments (1)
pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_20_version-4-only.yaml (1)
3-13: CRD naming convention:metadata.nameshould be{plural}.{group}Kubernetes CRDs require
metadata.nameto follow the pattern{spec.names.plural}.{spec.group}. For this CRD, it should beversion4onlies.test.iorather thanversion4only.Since this is test data for manifest filtering logic (not applied to a real cluster), this may be intentional for simplicity. However, aligning with the real CRD naming convention would make the test data more representative.
Suggested fix
apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: - name: version4only + name: version4onlies.test.io annotations: include.release.openshift.io/self-managed-high-availability: "true" release.openshift.io/major-version: "4"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_20_version-4-only.yaml` around lines 3 - 13, The CRD's metadata.name is incorrect: Kubernetes requires metadata.name to be "{spec.names.plural}.{spec.group}"; update the metadata.name value from "version4only" to "version4onlies.test.io" so it matches spec.names.plural ("version4onlies") and spec.group ("test.io") for the Version4Only CRD entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_10_no-version.yaml`:
- Around line 4-11: The CRD's metadata.name is incorrect — it should be the
fully-qualified "<plural>.<group>" value; update metadata.name from "noversion"
to "noversions.test.io" so that metadata.name equals spec.names.plural + "." +
spec.group (ensure fields referenced: metadata.name, spec.names.plural,
spec.group, kind: NoVersion).
---
Nitpick comments:
In
`@pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_20_version-4-only.yaml`:
- Around line 3-13: The CRD's metadata.name is incorrect: Kubernetes requires
metadata.name to be "{spec.names.plural}.{spec.group}"; update the metadata.name
value from "version4only" to "version4onlies.test.io" so it matches
spec.names.plural ("version4onlies") and spec.group ("test.io") for the
Version4Only CRD entry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a30e62f6-db56-4087-b9c8-63ce0a309981
⛔ Files ignored due to path filters (21)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/api/config/v1/types_apiserver.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_authentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_infrastructure.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/types_backup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/features.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/manifest/manifest.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (17)
go.modlib/manifest/manifest.gopkg/cvo/cvo_scenarios_test.gopkg/cvo/featuregate_integration_test.gopkg/cvo/sync_worker.gopkg/cvo/testdata/majorversiontest-v5/manifests/.gitkeeppkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_10_no-version.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_20_version-4-only.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_30_version-5-only.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_40_version-4-or-5.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_50_exclude-version-3.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/image-referencespkg/cvo/testdata/majorversiontest-v5/release-manifests/release-metadatapkg/payload/payload.gopkg/payload/payload_test.gopkg/payload/render.gopkg/payload/render_test.go
🚧 Files skipped from review as they are similar to previous changes (6)
- pkg/cvo/featuregate_integration_test.go
- go.mod
- pkg/cvo/sync_worker.go
- pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_30_version-5-only.yaml
- pkg/cvo/testdata/majorversiontest-v5/release-manifests/release-metadata
- pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_50_exclude-version-3.yaml
JoelSpeed
force-pushed
the
major-version-inclusion
branch
from
47b4565 to
594b873
Compare
Contributor
|
@JoelSpeed: This pull request references OCPSTRAT-2876 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the feature to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
♻️ Duplicate comments (3)
go.mod (1)
37-37:⚠️ Potential issue | 🟠 MajorAvoid merging with a personal fork replacement in
go.mod.At Line 37,
github.com/openshift/library-gois replaced with a personal fork. This should be removed (or limited to local/dev-only workflows) before merge to preserve provenance and reproducibility.Suggested change
-replace github.com/openshift/library-go => github.com/JoelSpeed/library-go v0.0.0-20260130121715-6e5ccfd0da42#!/bin/bash set -euo pipefail # Verify whether personal/library fork replacements remain in module file. rg -n '^replace\s+github.com/openshift/library-go' go.mod # Show required library-go entry for upstream pin visibility. rg -n 'github.com/openshift/library-go\s+v' go.mod🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 37, Remove the personal-fork replace directive for github.com/openshift/library-go in go.mod (the line "replace github.com/openshift/library-go => github.com/JoelSpeed/library-go ...") before merging; either delete that replace, scope it to local/dev-only via a separate go.work or developer-only patch, or replace it with the appropriate upstream module version pin so provenance is preserved and builds are reproducible.pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_10_no-version.yaml (1)
1-19:⚠️ Potential issue | 🟡 MinorUse valid CRD name format (
<plural>.<group>).The CRD
metadata.namemust follow the Kubernetes naming convention:<spec.names.plural>.<spec.group>. Currently set tonoversion, but should benoversions.test.ioto matchspec.names.plural: noversionsandspec.group: test.io.Suggested fix
metadata: - name: noversion + name: noversions.test.io annotations:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_10_no-version.yaml` around lines 1 - 19, metadata.name is not following the `<plural>.<group>` convention: update the CRD's metadata.name to match spec.names.plural and spec.group by setting it to "noversions.test.io" (ensure you modify the metadata.name field in the CRD entry where metadata.name is currently "noversion" so it equals spec.names.plural + "." + spec.group).pkg/cvo/sync_worker.go (1)
451-452:⚠️ Potential issue | 🟠 MajorUse target payload major version for implicit capability evaluation.
At line 452,
w.payload.ParsedVersion.Major(current payload) is passed, butpayloadUpdate.ParsedVersion.Major(target payload) should be used. During cross-major upgrades (e.g., 4→5), evaluating update manifests against the current version could compute incorrect implicitly-enabled capabilities.Suggested fix
if w.payload != nil { implicitlyEnabledCaps = capability.SortedList(payload.GetImplicitlyEnabledCapabilities(payloadUpdate.Manifests, w.payload.Manifests, - work.Capabilities, work.EnabledFeatureGates, ptr.To(w.payload.ParsedVersion.Major))) + work.Capabilities, work.EnabledFeatureGates, ptr.To(payloadUpdate.ParsedVersion.Major))) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cvo/sync_worker.go` around lines 451 - 452, The call computing implicitlyEnabledCaps uses the current payload major version (w.payload.ParsedVersion.Major) but should use the target update's major version; update the argument passed to payload.GetImplicitlyEnabledCapabilities (inside capability.SortedList(...) that assigns implicitlyEnabledCaps) to use payloadUpdate.ParsedVersion.Major instead of w.payload.ParsedVersion.Major so implicit capability evaluation is performed against the target payload version.
🧹 Nitpick comments (1)
pkg/payload/render_test.go (1)
277-306: Consider removingnamespacefrom CRD test manifests.CRDs are cluster-scoped resources and don't have a namespace field in their metadata. While this may not break the test since it's just checking file inclusion/exclusion, it creates technically invalid CRD manifests.
💡 Suggested fix
func createTestManifestYAML(apiVersion, kind, name string, annotations map[string]string) string { yaml := `apiVersion: ` + apiVersion + ` kind: ` + kind + ` metadata: - name: ` + name + ` - namespace: test-namespace` + name: ` + name🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/payload/render_test.go` around lines 277 - 306, The createTestManifestYAML helper currently always injects a namespace into metadata which produces invalid CRD manifests; update createTestManifestYAML so it omits the "namespace: test-namespace" line when the manifest represents a cluster-scoped CRD (e.g. when kind == "CustomResourceDefinition" and/or apiVersion indicates apiextensions.k8s.io/v1). Keep the metadata block and annotations for all kinds, but conditionally build the metadata string without the namespace for CRDs so the output is valid cluster-scoped YAML.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@go.mod`:
- Line 37: Remove the personal-fork replace directive for
github.com/openshift/library-go in go.mod (the line "replace
github.com/openshift/library-go => github.com/JoelSpeed/library-go ...") before
merging; either delete that replace, scope it to local/dev-only via a separate
go.work or developer-only patch, or replace it with the appropriate upstream
module version pin so provenance is preserved and builds are reproducible.
In `@pkg/cvo/sync_worker.go`:
- Around line 451-452: The call computing implicitlyEnabledCaps uses the current
payload major version (w.payload.ParsedVersion.Major) but should use the target
update's major version; update the argument passed to
payload.GetImplicitlyEnabledCapabilities (inside capability.SortedList(...) that
assigns implicitlyEnabledCaps) to use payloadUpdate.ParsedVersion.Major instead
of w.payload.ParsedVersion.Major so implicit capability evaluation is performed
against the target payload version.
In
`@pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_10_no-version.yaml`:
- Around line 1-19: metadata.name is not following the `<plural>.<group>`
convention: update the CRD's metadata.name to match spec.names.plural and
spec.group by setting it to "noversions.test.io" (ensure you modify the
metadata.name field in the CRD entry where metadata.name is currently
"noversion" so it equals spec.names.plural + "." + spec.group).
---
Nitpick comments:
In `@pkg/payload/render_test.go`:
- Around line 277-306: The createTestManifestYAML helper currently always
injects a namespace into metadata which produces invalid CRD manifests; update
createTestManifestYAML so it omits the "namespace: test-namespace" line when the
manifest represents a cluster-scoped CRD (e.g. when kind ==
"CustomResourceDefinition" and/or apiVersion indicates apiextensions.k8s.io/v1).
Keep the metadata block and annotations for all kinds, but conditionally build
the metadata string without the namespace for CRDs so the output is valid
cluster-scoped YAML.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2be614e1-9314-4a05-a3bf-973f15cfc63b
⛔ Files ignored due to path filters (21)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/api/config/v1/types_apiserver.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_authentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_infrastructure.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/types_backup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/features.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/manifest/manifest.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (17)
go.modlib/manifest/manifest.gopkg/cvo/cvo_scenarios_test.gopkg/cvo/featuregate_integration_test.gopkg/cvo/sync_worker.gopkg/cvo/testdata/majorversiontest-v5/manifests/.gitkeeppkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_10_no-version.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_20_version-4-only.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_30_version-5-only.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_40_version-4-or-5.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_50_exclude-version-3.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/image-referencespkg/cvo/testdata/majorversiontest-v5/release-manifests/release-metadatapkg/payload/payload.gopkg/payload/payload_test.gopkg/payload/render.gopkg/payload/render_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- pkg/payload/payload.go
- pkg/cvo/testdata/majorversiontest-v5/release-manifests/release-metadata
- pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_50_exclude-version-3.yaml
- pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_30_version-5-only.yaml
JoelSpeed
force-pushed
the
major-version-inclusion
branch
from
594b873 to
f327a2f
Compare
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
JoelSpeed
force-pushed
the
major-version-inclusion
branch
from
f327a2f to
879afb3
Compare
Contributor
|
@JoelSpeed: This pull request references OCPSTRAT-2876 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the feature to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
pkg/payload/render_test.go (1)
254-266: Assert against unexpected extra manifests as well.Current checks validate expected includes/excludes, but they still pass if an unexpected manifest is rendered.
Proposed assertion hardening
// Verify expected exclusions for _, expectedExcludedName := range tt.expectedExclusions { if includedManifests[expectedExcludedName] { t.Errorf("Expected manifest %s to be excluded, but it was found in output", expectedExcludedName) } } + + // Verify there are no unexpected inclusions + expectedSet := sets.New[string](tt.expectedInclusions...) + for manifestName := range includedManifests { + if !expectedSet.Has(manifestName) { + t.Errorf("Unexpected manifest %s was included", manifestName) + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/payload/render_test.go` around lines 254 - 266, Test currently only checks expected inclusions/exclusions but allows unexpected extra manifests; after the existing loops in the test (which reference includedManifests, tt.expectedInclusions, tt.expectedExclusions) add a check that no extra manifests were produced: either assert len(includedManifests) == len(tt.expectedInclusions) or iterate over keys in includedManifests and fail if any key is not present in tt.expectedInclusions; this ensures any unexpected rendered manifest causes the test to fail.pkg/payload/render.go (1)
290-293: Consider usingsemver.ParseTolerant()for v-prefixed version robustness.
semver.Parse()strictly rejects leadingvand will fail on versions likev1.2.3. For Cincinnati metadata versions, usingsemver.ParseTolerant()is the idiomatic approach—it strips thevprefix automatically and is already available in the library.Also update the error format to use
%winstead of%vfor consistency withpkg/payload/payload.go:Suggested improvement
- parsedVersion, err := semver.Parse(release.Version) + parsedVersion, err := semver.ParseTolerant(release.Version) if err != nil { - return semver.Version{}, fmt.Errorf("cincinnati metadata version %q is not a valid semantic version: %v", release.Version, err) + return semver.Version{}, fmt.Errorf("cincinnati metadata version %q is not a valid semantic version: %w", release.Version, err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/payload/render.go` around lines 290 - 293, Replace the strict semver.Parse call with semver.ParseTolerant to accept v-prefixed versions (i.e., change semver.Parse(release.Version) to semver.ParseTolerant(release.Version)), and update the error construction to wrap the original error with %w (e.g., fmt.Errorf(...: %w, err)) so parsedVersion, release.Version and the returned error use the tolerant parser and proper error wrapping.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/manifest/manifest.go`:
- Around line 92-96: The code dereferences optional Capabilities
(currentManifestInclusionConfiguration.Capabilities.EnabledCapabilities) without
a nil check; guard the pointer by checking
currentManifestInclusionConfiguration.Capabilities != nil before accessing
EnabledCapabilities and treat a nil Capabilities as an empty set/slice when
building the sets.New call, so the Difference call uses an empty set instead of
panicking; apply the same nil-guarding approach anywhere Capabilities is
dereferenced (refer to updateManifest.GetManifestCapabilities(),
currentManifest.GetManifestCapabilities(), and
currentManifestInclusionConfiguration.Capabilities.EnabledCapabilities) and
return the union logic unchanged but using an empty set when Capabilities is
nil.
In
`@pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_30_version-5-only.yaml`:
- Line 4: The CRD's metadata.name must follow the Kubernetes API-valid form
"<spec.names.plural>.<spec.group>" for apiextensions.k8s.io/v1; update
metadata.name from "version5only" to match spec.names.plural and spec.group
(i.e., "version5onlies.test.io") so it equals spec.names.plural + "." +
spec.group, and verify the same correction is applied where those fields are
referenced elsewhere in this manifest (the spec.names.plural and spec.group
entries).
In
`@pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_50_exclude-version-3.yaml`:
- Around line 4-12: Replace the invalid CRD metadata.name value with the
canonical "<spec.names.plural>.<spec.group>" form: set metadata.name to the
concatenation of spec.names.plural and spec.group (e.g., change metadata.name
from "excludeversion3" to "excludeversion3s.test.io") so it matches the CRD
spec; update the metadata.name field in the manifest alongside spec.names.plural
and spec.group to ensure they align.
---
Nitpick comments:
In `@pkg/payload/render_test.go`:
- Around line 254-266: Test currently only checks expected inclusions/exclusions
but allows unexpected extra manifests; after the existing loops in the test
(which reference includedManifests, tt.expectedInclusions,
tt.expectedExclusions) add a check that no extra manifests were produced: either
assert len(includedManifests) == len(tt.expectedInclusions) or iterate over keys
in includedManifests and fail if any key is not present in
tt.expectedInclusions; this ensures any unexpected rendered manifest causes the
test to fail.
In `@pkg/payload/render.go`:
- Around line 290-293: Replace the strict semver.Parse call with
semver.ParseTolerant to accept v-prefixed versions (i.e., change
semver.Parse(release.Version) to semver.ParseTolerant(release.Version)), and
update the error construction to wrap the original error with %w (e.g.,
fmt.Errorf(...: %w, err)) so parsedVersion, release.Version and the returned
error use the tolerant parser and proper error wrapping.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: adc636c4-f2ae-43e9-acd1-c77d555e6102
⛔ Files ignored due to path filters (283)
go.sumis excluded by!**/*.sumvendor/github.com/go-logr/logr/.golangci.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/funcr/funcr.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/AUTHORSis excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/CONTRIBUTORSis excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/clone.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/custom_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/decode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/deprecated.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/discard.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/duration.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/duration_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/encode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/encode_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/equal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/extensions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/extensions_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/lib.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/lib_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/message_set.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/pointer_reflect.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/pointer_reflect_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/pointer_unsafe.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/pointer_unsafe_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/properties.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/properties_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/skip_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/table_marshal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/table_marshal_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/table_merge.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/table_unmarshal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/table_unmarshal_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/text.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/text_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/text_parser.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/timestamp.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/timestamp_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/wrappers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/wrappers_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/sortkeys/sortkeys.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/format/format.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gomega_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/internal/assertion.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/internal/async_assertion.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/internal/duration_bundle.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/internal/gomega.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/internal/polling_signal_error.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/internal/vetoptdesc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/and.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/assignable_to_type_of_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_a_directory.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_a_regular_file.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_an_existing_file.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_closed_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_comparable_to_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_element_of_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_empty_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_equivalent_to_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_false_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_identical_to.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_key_of_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_nil_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_numerically_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_sent_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_temporally_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_true_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_zero_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/consist_of.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/contain_element_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/contain_elements_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/contain_substring_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/equal_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_cap_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_each_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_exact_elements.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_existing_field_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_field.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_http_body_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_http_header_with_value_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_http_status_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_key_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_key_with_value_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_len_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_occurred_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_prefix_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_suffix_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_value.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/internal/miter/type_support_iter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/internal/miter/type_support_noiter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/match_error_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/match_json_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/match_regexp_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/match_xml_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/match_yaml_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/not.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/or.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/panic_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/receive_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/satisfy_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/semi_structured_data_support.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/succeed_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/support/goraph/bipartitegraph/bipartitegraph.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/support/goraph/node/node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/type_support.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/with_transform.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/types/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_apiserver.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_authentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_infrastructure.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_ingress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_network.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_tlssecurityprofile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_insightsdatagathers-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_insightsdatagathers-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_insightsdatagathers.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_networks.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_openshift-controller-manager_01_builds.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_backup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_cluster_monitoring.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_crio_credential_provider_config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha2/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha2/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/features/features.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/legacyfeaturegates.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/image/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/image/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/types_network.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/operator/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/types_clusterapi.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.crd-manifests/0000_00_cluster-version-operator_01_clusterversionoperators-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.crd-manifests/0000_00_cluster-version-operator_01_clusterversionoperators-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.crd-manifests/0000_00_cluster-version-operator_01_clusterversionoperators.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.crd-manifests/0000_10_etcd_01_etcdbackups-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.crd-manifests/0000_10_etcd_01_etcdbackups-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.crd-manifests/0000_10_etcd_01_etcdbackups.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.crd-manifests/0000_30_cluster-api_01_clusterapis.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/security/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/security/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/acceptrisk.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/alibabacloudplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/alibabacloudresourcetag.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/apiserver.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/apiserverencryption.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/apiservernamedservingcert.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/apiserverservingcerts.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/apiserverspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/audit.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/auditcustomrule.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/authentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/authenticationspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/authenticationstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsdnsspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsingressspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awskmsconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsresourcetag.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsserviceendpoint.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/azureplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/azureresourcetag.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/baremetalplatformloadbalancer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/baremetalplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/baremetalplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/basicauthidentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/build.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/builddefaults.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/buildoverrides.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/buildspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/cloudcontrollermanagerstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/cloudloadbalancerconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/cloudloadbalancerips.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clustercondition.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterimagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterimagepolicyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterimagepolicystatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusternetworkentry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusteroperator.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusteroperatorstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusteroperatorstatuscondition.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterversion.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterversioncapabilitiesspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterversioncapabilitiesstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterversionspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterversionstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/componentoverride.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/componentroutespec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/componentroutestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/conditionalupdate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/conditionalupdaterisk.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/configmapfilereference.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/configmapnamereference.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/console.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/consoleauthentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/consolespec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/consolestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/custom.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/customfeaturegates.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/customtlsprofile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/deprecatedwebhooktokenauthenticator.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/dns.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/dnsplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/dnsspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/dnszone.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/equinixmetalplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/externalipconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/externalippolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/externalplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/externalplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/extramapping.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/featuregate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/featuregateattributes.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/featuregatedetails.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/featuregateselection.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/featuregatestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gatherconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gathererconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gatherers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gcpplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gcpresourcelabel.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gcpresourcetag.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/githubidentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gitlabidentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/googleidentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/htpasswdidentityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/hubsource.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/hubsourcestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ibmcloudplatformspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ibmcloudplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ibmcloudserviceendpoint.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/identityprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/identityproviderconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/image.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagecontentpolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagecontentpolicyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagedigestmirrors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagedigestmirrorset.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagedigestmirrorsetspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagelabel.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicyfulciocawithrekorrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicypkirootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicypublickeyrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicystatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagesigstoreverificationpolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagespec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagetagmirrors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagetagmirrorset.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagetagmirrorsetspec.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (17)
go.modlib/manifest/manifest.gopkg/cvo/cvo_scenarios_test.gopkg/cvo/featuregate_integration_test.gopkg/cvo/sync_worker.gopkg/cvo/testdata/majorversiontest-v5/manifests/.gitkeeppkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_10_no-version.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_20_version-4-only.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_30_version-5-only.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_40_version-4-or-5.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_50_exclude-version-3.yamlpkg/cvo/testdata/majorversiontest-v5/release-manifests/image-referencespkg/cvo/testdata/majorversiontest-v5/release-manifests/release-metadatapkg/payload/payload.gopkg/payload/payload_test.gopkg/payload/render.gopkg/payload/render_test.go
✅ Files skipped from review due to trivial changes (1)
- pkg/cvo/testdata/majorversiontest-v5/release-manifests/image-references
🚧 Files skipped from review as they are similar to previous changes (4)
- pkg/cvo/featuregate_integration_test.go
- pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_40_version-4-or-5.yaml
- pkg/cvo/testdata/majorversiontest-v5/release-manifests/release-metadata
- pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_20_version-4-only.yaml
Comment on lines
92
to
96
| newImplicitlyEnabled := sets.New[configv1.ClusterVersionCapability](updateManifest.GetManifestCapabilities()...). | ||
| Difference(sets.New[configv1.ClusterVersionCapability](currentManifest.GetManifestCapabilities()...)). | ||
| Difference(currentImplicitlyEnabled). | ||
| Difference(sets.New[configv1.ClusterVersionCapability](manifestInclusionConfiguration.Capabilities.EnabledCapabilities...)) | ||
| Difference(sets.New[configv1.ClusterVersionCapability](currentManifestInclusionConfiguration.Capabilities.EnabledCapabilities...)) | ||
| ret = ret.Union(newImplicitlyEnabled) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the manifest.go file
find . -name "manifest.go" | grep -E "lib/manifest"Repository: openshift/cluster-version-operator
Length of output: 104
🏁 Script executed:
# Read the relevant section of the file to understand the context
head -c 20000 lib/manifest/manifest.go | tail -c 10000Repository: openshift/cluster-version-operator
Length of output: 4937
🏁 Script executed:
# Get the specific lines in question (92-96) with surrounding context
sed -n '75,110p' lib/manifest/manifest.goRepository: openshift/cluster-version-operator
Length of output: 1495
🏁 Script executed:
# Find the struct definition for InclusionConfiguration to understand the Capabilities field
rg "type.*InclusionConfiguration" -A 10Repository: openshift/cluster-version-operator
Length of output: 837
🏁 Script executed:
# Search for the Capabilities field definition
rg "Capabilities.*\[\]" lib/manifest/ -B 2 -A 2Repository: openshift/cluster-version-operator
Length of output: 60
🏁 Script executed:
# Search for callers of GetImplicitlyEnabledCapabilities
rg "GetImplicitlyEnabledCapabilities" -B 2 -A 5Repository: openshift/cluster-version-operator
Length of output: 4565
🏁 Script executed:
# Check if there's a nil guard before line 96 that wasn't visible
sed -n '85,100p' lib/manifest/manifest.goRepository: openshift/cluster-version-operator
Length of output: 995
Guard optional Capabilities before dereference.
Capabilities is documented as optional but the code dereferences it unconditionally without a nil check, which can cause a panic. Since Capabilities *configv1.ClusterVersionCapabilitiesStatus is a pointer field with documentation stating "if non-nil", the function must handle the nil case.
Suggested fix
+ enabledCaps := sets.Set[configv1.ClusterVersionCapability]{}
+ if currentManifestInclusionConfiguration.Capabilities != nil {
+ enabledCaps = sets.New[configv1.ClusterVersionCapability](
+ currentManifestInclusionConfiguration.Capabilities.EnabledCapabilities...,
+ )
+ }
newImplicitlyEnabled := sets.New[configv1.ClusterVersionCapability](updateManifest.GetManifestCapabilities()...).
Difference(sets.New[configv1.ClusterVersionCapability](currentManifest.GetManifestCapabilities()...)).
Difference(currentImplicitlyEnabled).
- Difference(sets.New[configv1.ClusterVersionCapability](currentManifestInclusionConfiguration.Capabilities.EnabledCapabilities...))
+ Difference(enabledCaps)📝 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.
Suggested change
| newImplicitlyEnabled := sets.New[configv1.ClusterVersionCapability](updateManifest.GetManifestCapabilities()...). | |
| Difference(sets.New[configv1.ClusterVersionCapability](currentManifest.GetManifestCapabilities()...)). | |
| Difference(currentImplicitlyEnabled). | |
| Difference(sets.New[configv1.ClusterVersionCapability](manifestInclusionConfiguration.Capabilities.EnabledCapabilities...)) | |
| Difference(sets.New[configv1.ClusterVersionCapability](currentManifestInclusionConfiguration.Capabilities.EnabledCapabilities...)) | |
| ret = ret.Union(newImplicitlyEnabled) | |
| enabledCaps := sets.Set[configv1.ClusterVersionCapability]{} | |
| if currentManifestInclusionConfiguration.Capabilities != nil { | |
| enabledCaps = sets.New[configv1.ClusterVersionCapability]( | |
| currentManifestInclusionConfiguration.Capabilities.EnabledCapabilities..., | |
| ) | |
| } | |
| newImplicitlyEnabled := sets.New[configv1.ClusterVersionCapability](updateManifest.GetManifestCapabilities()...). | |
| Difference(sets.New[configv1.ClusterVersionCapability](currentManifest.GetManifestCapabilities()...)). | |
| Difference(currentImplicitlyEnabled). | |
| Difference(enabledCaps) | |
| ret = ret.Union(newImplicitlyEnabled) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/manifest/manifest.go` around lines 92 - 96, The code dereferences
optional Capabilities
(currentManifestInclusionConfiguration.Capabilities.EnabledCapabilities) without
a nil check; guard the pointer by checking
currentManifestInclusionConfiguration.Capabilities != nil before accessing
EnabledCapabilities and treat a nil Capabilities as an empty set/slice when
building the sets.New call, so the Difference call uses an empty set instead of
panicking; apply the same nil-guarding approach anywhere Capabilities is
dereferenced (refer to updateManifest.GetManifestCapabilities(),
currentManifest.GetManifestCapabilities(), and
currentManifestInclusionConfiguration.Capabilities.EnabledCapabilities) and
return the union logic unchanged but using an empty set when Capabilities is
nil.
| apiVersion: apiextensions.k8s.io/v1 | ||
| kind: CustomResourceDefinition | ||
| metadata: | ||
| name: version5only |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Step 1: Check the target file content
echo "=== File Content ==="
cat -n "pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_30_version-5-only.yaml"
echo ""
echo "=== Running verification script ==="
python - <<'PY'
import glob, sys, yaml
failed = False
for path in glob.glob("pkg/cvo/testdata/**/release-manifests/*.yaml", recursive=True):
with open(path, "r", encoding="utf-8") as f:
for doc in yaml.safe_load_all(f):
if not isinstance(doc, dict):
continue
if doc.get("kind") != "CustomResourceDefinition":
continue
api = doc.get("apiVersion", "")
if not api.startswith("apiextensions.k8s.io/"):
continue
spec = doc.get("spec") or {}
names = spec.get("names") or {}
group = spec.get("group")
plural = names.get("plural")
actual = (doc.get("metadata") or {}).get("name")
expected = f"{plural}.{group}" if plural and group else None
if expected and actual != expected:
failed = True
print(f"{path}: metadata.name={actual!r}, expected={expected!r}")
if failed:
sys.exit(1)
print("All CRD fixture names match <plural>.<group>.")
PY
echo "Script exit code: $?"Repository: openshift/cluster-version-operator
Length of output: 1473
CRD name does not match API-valid format for apiextensions.k8s.io/v1.
Line 4, metadata.name: version5only does not match the required CRD naming form <spec.names.plural>.<spec.group>, which is version5onlies.test.io (lines 12 and 9). This affects the validity of the test fixture when applied to a Kubernetes cluster.
Suggested fix
metadata:
- name: version5only
+ name: version5onlies.test.ioAlso applies to: 9-9, 12-12
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_30_version-5-only.yaml`
at line 4, The CRD's metadata.name must follow the Kubernetes API-valid form
"<spec.names.plural>.<spec.group>" for apiextensions.k8s.io/v1; update
metadata.name from "version5only" to match spec.names.plural and spec.group
(i.e., "version5onlies.test.io") so it equals spec.names.plural + "." +
spec.group, and verify the same correction is applied where those fields are
referenced elsewhere in this manifest (the spec.names.plural and spec.group
entries).
Comment on lines
+4
to
+12
| name: excludeversion3 | ||
| annotations: | ||
| include.release.openshift.io/self-managed-high-availability: "true" | ||
| release.openshift.io/major-version: "-3" | ||
| spec: | ||
| group: test.io | ||
| names: | ||
| kind: ExcludeVersion3 | ||
| plural: excludeversion3s |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the actual file content
cat -n pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_50_exclude-version-3.yamlRepository: openshift/cluster-version-operator
Length of output: 670
🏁 Script executed:
# Check for other CRD examples in the same test directory
find pkg/cvo/testdata/majorversiontest-v5/release-manifests -name "*.yaml" -type f | head -20 | xargs grep -l "apiVersion.*apiextensions"Repository: openshift/cluster-version-operator
Length of output: 491
🏁 Script executed:
# Examine a few other CRD manifests to check naming patterns
find pkg/cvo/testdata/majorversiontest-v5/release-manifests -name "*.yaml" -type f -exec grep -l "kind: CustomResourceDefinition" {} \; | head -3 | xargs -I {} sh -c 'echo "=== {} ===" && head -20 "{}"'Repository: openshift/cluster-version-operator
Length of output: 1715
🌐 Web query:
Kubernetes CRD metadata.name requirement apiextensions.k8s.io/v1
💡 Result:
For a CustomResourceDefinition (apiVersion: apiextensions.k8s.io/v1), the CRD object’s metadata.name is required and must:
- Match the spec fields and be in the form:
<spec.names.plural>.<spec.group>(for example,crontabs.stable.example.com). [1] - Be a valid DNS subdomain name derived from the resource name and API group. [2]
Sources: Kubernetes CRD task docs show the explicit <plural>.<group> requirement in the CRD example. [1] Kubernetes “Custom Resources” concept docs describe the DNS-subdomain constraint for the CRD object name. [2]
🏁 Script executed:
# Check if these test manifests are actually validated or used as test data
find pkg/cvo/testdata -name "*.yaml" | head -5 | xargs grep -l "kind: CustomResourceDefinition" | xargs head -30Repository: openshift/cluster-version-operator
Length of output: 1090
🏁 Script executed:
# Check the test files that use these manifests to understand their purpose
fd -e "test.go" pkg/cvo | head -5 | xargs grep -l "majorversiontest-v5" | head -2Repository: openshift/cluster-version-operator
Length of output: 411
🏁 Script executed:
# Search for test code that uses these majorversiontest-v5 manifests
rg "majorversiontest-v5" -t go pkg/cvoRepository: openshift/cluster-version-operator
Length of output: 186
🏁 Script executed:
# Check if there's any validation or loading of these CRD manifests in tests
rg "0000_50_exclude-version-3" pkg/cvoRepository: openshift/cluster-version-operator
Length of output: 60
🏁 Script executed:
# Look for any Kubernetes CRD validation being performed in the codebase
rg "metadata\.name" pkg/cvo -t yaml | grep -i customresourceRepository: openshift/cluster-version-operator
Length of output: 60
🏁 Script executed:
# Check the test code to understand what it's doing with these manifests
sed -n '1,100p' pkg/cvo/cvo_scenarios_test.go | grep -A 30 "setupCVOTest"Repository: openshift/cluster-version-operator
Length of output: 1570
🏁 Script executed:
# Look for any comments about intentional malformation or test purposes in the test data directory
find pkg/cvo/testdata/majorversiontest-v5 -name "README*" -o -name "*.md" | xargs cat 2>/dev/nullRepository: openshift/cluster-version-operator
Length of output: 60
🏁 Script executed:
# Check if this test data directory has any documentation or comments about intended behavior
ls -la pkg/cvo/testdata/majorversiontest-v5/Repository: openshift/cluster-version-operator
Length of output: 314
🏁 Script executed:
# Look at the test that uses this data to see if it's testing error handling or valid scenarios
rg -A 50 "setupCVOTest.*majorversiontest-v5" pkg/cvo/cvo_scenarios_test.go | head -80Repository: openshift/cluster-version-operator
Length of output: 1514
🏁 Script executed:
# Check if these CRDs would actually be applied or if they're just parsed as test fixtures
rg "applyManifests\|LoadManifests\|parseManifests" pkg/cvo -t go | head -10Repository: openshift/cluster-version-operator
Length of output: 60
🏁 Script executed:
# Verify the test expectations more closely - what does GetName() return?
rg "GetName\(\)" pkg/cvo/cvo_scenarios_test.go -A 2 -B 2 | head -30Repository: openshift/cluster-version-operator
Length of output: 853
🏁 Script executed:
# Check if there are any other test directories with similar patterns to understand this design
find pkg/cvo/testdata -type d | head -10Repository: openshift/cluster-version-operator
Length of output: 457
🏁 Script executed:
# Look for any documentation or README in the testdata directory
find pkg/cvo/testdata -type f \( -name "README*" -o -name "*.md" -o -name "*.txt" \) | xargs cat 2>/dev/nullRepository: openshift/cluster-version-operator
Length of output: 60
🏁 Script executed:
# Check how the manifests are loaded and processed - are they validated?
rg "LoadPayload\|loadManifest\|parseManifest" pkg/cvo -t go -A 3 | head -40Repository: openshift/cluster-version-operator
Length of output: 60
🏁 Script executed:
# Check if there's any code that would validate CRD metadata.name format
rg "metadata.name\|spec.names.plural\|spec.group" pkg/cvo -t go | head -20Repository: openshift/cluster-version-operator
Length of output: 60
🏁 Script executed:
# Look at what the test actually checks - does it validate the manifests or just the behavior?
sed -n '1,150p' pkg/cvo/cvo_scenarios_test.go | tail -60Repository: openshift/cluster-version-operator
Length of output: 2147
🏁 Script executed:
# Final confirmation: check the exact content one more time to match review snippet
sed -n '3,12p' pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_50_exclude-version-3.yamlRepository: openshift/cluster-version-operator
Length of output: 330
Use a valid CRD metadata.name (<plural>.<group>).
This manifest sets metadata.name to excludeversion3, but per the Kubernetes CustomResourceDefinition specification, it must match the format <spec.names.plural>.<spec.group>. The correct value is excludeversion3s.test.io.
Suggested fix
metadata:
- name: excludeversion3
+ name: excludeversion3s.test.io📝 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.
Suggested change
| name: excludeversion3 | |
| annotations: | |
| include.release.openshift.io/self-managed-high-availability: "true" | |
| release.openshift.io/major-version: "-3" | |
| spec: | |
| group: test.io | |
| names: | |
| kind: ExcludeVersion3 | |
| plural: excludeversion3s | |
| name: excludeversion3s.test.io | |
| annotations: | |
| include.release.openshift.io/self-managed-high-availability: "true" | |
| release.openshift.io/major-version: "-3" | |
| spec: | |
| group: test.io | |
| names: | |
| kind: ExcludeVersion3 | |
| plural: excludeversion3s |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@pkg/cvo/testdata/majorversiontest-v5/release-manifests/0000_50_exclude-version-3.yaml`
around lines 4 - 12, Replace the invalid CRD metadata.name value with the
canonical "<spec.names.plural>.<spec.group>" form: set metadata.name to the
concatenation of spec.names.plural and spec.group (e.g., change metadata.name
from "excludeversion3" to "excludeversion3s.test.io") so it matches the CRD
spec; update the metadata.name field in the manifest alongside spec.names.plural
and spec.group to ensure they align.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This introduces a new CVO manifest filter based on the OCP major version.
The intended usage would allow folks to say "this manifest should be deployed on OCP 5 and skipped on OCP 4"
The library-go component here is limiting this feature to FeatureGate and CustomResourceDefinition only for now.
This is not intended for use beyond the feature gating mechanisms we have today. We expect FeatureGate and CustomResourceDefinition yamls to vary by release, but all other inclusion/exclusion should be handled by existing mechanisms like the feature-gate inclusion mechanism.
Example usage:
Summary by CodeRabbit
New Features
Tests
Chores