no-jira: vendor: update o/api to the latest after several Feature promotions to default - #10746
no-jira: vendor: update o/api to the latest after several Feature promotions to default#10746sadasu wants to merge 4 commits into
Conversation
AWSClusterHostedInstall, AWSDualStackInstall and NoRegistryClusterInstall have all been promoted to default.
Removed checks for featuegates AWSClusterHostedDNSInstall and AWSDualStackInstall checks since they are already promoted to default.
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (27)
📒 Files selected for processing (9)
💤 Files with no reviewable changes (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe change removes no-registry feature-gate checks from internal release image assets, removes two AWS feature-gated validation entries, updates related documentation and tests, and updates the OpenShift API dependency. ChangesInternal release image generation
AWS validation
OpenShift API dependency
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/asset/manifests/operators.go (1)
240-246: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for internal release image manifests.
When
InternalReleaseImage.FileListis non-empty, verify both output files are emitted with the feature gate enabled and disabled. Verify that an emptyFileListemits neither file. Update the conflicting expectation inpkg/asset/tls/iriregistryauth_test.go.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/asset/manifests/operators.go` around lines 240 - 246, Add regression tests around the InternalReleaseImage handling to verify appendIRIcerts and appendIRIRegistryCredentials emit both files when FileList is non-empty, regardless of the feature gate state, and emit neither when FileList is empty. Update the conflicting expectation in the iriregistryauth tests to match this behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pkg/asset/manifests/operators.go`:
- Around line 240-246: Add regression tests around the InternalReleaseImage
handling to verify appendIRIcerts and appendIRIRegistryCredentials emit both
files when FileList is non-empty, regardless of the feature gate state, and emit
neither when FileList is empty. Update the conflicting expectation in the
iriregistryauth tests to match this behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 45d06c2e-d78b-4aa6-91e7-e6fc974e34d8
⛔ Files ignored due to path filters (27)
go.sumis excluded by!**/*.sumvendor/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_kmsencryption.gois 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_cluster_monitoring.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.model_name.gois 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/features/features.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/features/legacyfeaturegates.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/machineconfiguration/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!vendor/**,!**/vendor/**,!**/zz_generated*vendor/github.com/openshift/api/operator/v1/types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/operator/v1/types_kmsencryption.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.swagger_doc_generated.gois 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_ingress.gois 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.model_name.gois 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/route/v1/generated.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/route/v1/types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/route/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!vendor/**,!**/vendor/**,!**/zz_generated*vendor/modules.txtis excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (7)
go.modpkg/asset/manifests/operators.gopkg/asset/templates/content/bootkube/internal-release-image-registry-auth-secret.gopkg/asset/templates/content/bootkube/internal-release-image-server-tls-secret.gopkg/asset/tls/iricertkey.gopkg/asset/tls/iriregistryauth.gopkg/types/aws/validation/featuregates.go
💤 Files with no reviewable changes (5)
- pkg/asset/templates/content/bootkube/internal-release-image-server-tls-secret.go
- pkg/asset/tls/iricertkey.go
- pkg/asset/templates/content/bootkube/internal-release-image-registry-auth-secret.go
- pkg/types/aws/validation/featuregates.go
- pkg/asset/tls/iriregistryauth.go
|
/retitle no-jira: vendor: update o/api to the latest after several Feature promotions to default |
openshift-ci
Bot
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@sadasu: This pull request explicitly references no jira issue. 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. |
tthvo
left a comment
tthvo
left a comment
There was a problem hiding this comment.
Woohoo 🚀 Though, we have a few more unit tests to fix/remove: ci/prow/unit 👀
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/asset/tls/iriregistryauth_test.go (1)
77-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate comments after removing the feature-gate setup.
The comment at Line 42 still says
with feature gate.pkg/asset/ignition/bootstrap/common.goalso states at Lines 387-389 thatNoRegistryClusterInstallmust be enabled. Update both comments to describe generation based onInternalReleaseImagepresence.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/asset/tls/iriregistryauth_test.go` at line 77, Update the comments in the IRI manifest test and the NoRegistryClusterInstall guidance in common.go to remove feature-gate language and state that generation is determined by the presence of InternalReleaseImage. Keep the comments aligned with the current behavior after removing the feature-gate setup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pkg/asset/tls/iriregistryauth_test.go`:
- Line 77: Update the comments in the IRI manifest test and the
NoRegistryClusterInstall guidance in common.go to remove feature-gate language
and state that generation is determined by the presence of InternalReleaseImage.
Keep the comments aligned with the current behavior after removing the
feature-gate setup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 89e2a5df-17c9-495d-ab84-bceb303266ca
⛔ Files ignored due to path filters (27)
go.sumis excluded by!**/*.sumvendor/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_kmsencryption.gois 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_cluster_monitoring.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.model_name.gois 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/features/features.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/features/legacyfeaturegates.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/machineconfiguration/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!vendor/**,!**/vendor/**,!**/zz_generated*vendor/github.com/openshift/api/operator/v1/types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/operator/v1/types_kmsencryption.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.swagger_doc_generated.gois 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_ingress.gois 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.model_name.gois 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/route/v1/generated.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/route/v1/types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/route/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!vendor/**,!**/vendor/**,!**/zz_generated*vendor/modules.txtis excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (8)
go.modpkg/asset/manifests/operators.gopkg/asset/templates/content/bootkube/internal-release-image-registry-auth-secret.gopkg/asset/templates/content/bootkube/internal-release-image-server-tls-secret.gopkg/asset/tls/iricertkey.gopkg/asset/tls/iriregistryauth.gopkg/asset/tls/iriregistryauth_test.gopkg/types/aws/validation/featuregates.go
💤 Files with no reviewable changes (5)
- pkg/asset/tls/iriregistryauth.go
- pkg/asset/tls/iricertkey.go
- pkg/types/aws/validation/featuregates.go
- pkg/asset/templates/content/bootkube/internal-release-image-server-tls-secret.go
- pkg/asset/templates/content/bootkube/internal-release-image-registry-auth-secret.go
🚧 Files skipped from review as they are similar to previous changes (2)
- go.mod
- pkg/asset/manifests/operators.go
NoRegistryClusterInstall feature is now enabled for default installs.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/approve |
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: patrickdillon The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
/test e2e-aws-ovn-dualstack-ipv6-primary-techpreview |
|
/test e2e-agent-compact-ipv4-iso-no-registry |
|
@sadasu: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
4 participants
Bring in the latest from openshift/api after AWSClusterHostedInstall, AWSDualStackInstall and NoRegistryClusterInstall have all been promoted to default.
Update installer to not check for these featuregates when corresponding features have been enabled.
Summary by CodeRabbit