OCPCLOUD-3359,OCPCLOUD-3345: Add support for TLS envsubst substitution - #519
Merged
openshift-merge-bot[bot] merged 12 commits intoApr 23, 2026
Merged
Conversation
Contributor
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
the
needs-rebase
|
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:
WalkthroughReplace the prior CommonOptions flow with InitOperatorConfig across CLIs; add TLS profile resolution and a SecurityProfileWatcher; return structured logger/operatorConfig and an initManager closure; propagate TLS-derived manifest substitutions into revision rendering; add TLS-focused unit and e2e tests and related manifest/RBAC changes. Changes
Sequence Diagram(s)sequenceDiagram
actor CLI as CLI
participant Init as InitOperatorConfig
participant API as configv1.APIServer
participant TLSWatcher as SecurityProfileWatcher
participant InitMgr as initManager
participant Manager as ctrl.Manager
participant Revision as RevisionController
CLI->>Init: InitOperatorConfig(ctx,cfg,scheme,managerName,..)
Init->>API: GET Apiserver (resolveTLSProfile)
API-->>Init: return TLSSecurityProfile & adherence
Init->>TLSWatcher: create SecurityProfileWatcher (with cancel callback)
Init-->>CLI: return logger, operatorConfig, mgrOpts, initManager
CLI->>InitMgr: initManager(ctx,cancel,mgrOpts)
InitMgr-->>Manager: create ctrl.Manager (with health/readiness)
CLI->>Revision: SetupWithManager(mgr, tlsProfileSpec)
Revision->>Revision: derive manifestSubstitutions (TLS_MIN_VERSION, TLS_CIPHER_SUITES)
Revision-->>Manager: register controller
TLSWatcher->>API: watch for profile changes
API-->>TLSWatcher: profile changed
TLSWatcher->>InitMgr: invoke cancel() to trigger manager restart
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 6 | ❌ 4❌ Failed checks (3 warnings, 1 inconclusive)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
mdbooth
force-pushed
the
manifest-substitutions
branch
from
April 10, 2026 08:22
73a41c8 to
d2ebb8e
Compare
openshift-ci
Bot
removed
the
needs-rebase
mdbooth
force-pushed
the
manifest-substitutions
branch
from
April 10, 2026 16:38
d2ebb8e to
fb116ac
Compare
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
pkg/controllers/revision/revision_controller_test.go (1)
524-529: Prefer order-independent assertions for manifest substitutions.These assertions depend on slice position. Making this key-based will reduce brittleness if serialization order changes.
Proposed refactor
rev := updatedClusterAPI.Status.Revisions[0] Expect(rev.ManifestSubstitutions).To(HaveLen(2)) -Expect(rev.ManifestSubstitutions[0].Key).To(Equal("TLS_CIPHER_SUITES")) -Expect(*rev.ManifestSubstitutions[0].Value).To(Equal("TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256")) -Expect(rev.ManifestSubstitutions[1].Key).To(Equal("TLS_MIN_VERSION")) -Expect(*rev.ManifestSubstitutions[1].Value).To(Equal("VersionTLS12")) +subs := map[string]string{} +for _, s := range rev.ManifestSubstitutions { + if s.Value != nil { + subs[s.Key] = *s.Value + } +} +Expect(subs).To(HaveKeyWithValue("TLS_CIPHER_SUITES", "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256")) +Expect(subs).To(HaveKeyWithValue("TLS_MIN_VERSION", "VersionTLS12"))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controllers/revision/revision_controller_test.go` around lines 524 - 529, The test is brittle because it asserts ManifestSubstitutions by index; update the checks to be order-independent by locating substitutions by Key instead of slice position: iterate rev.ManifestSubstitutions (or convert it to a map keyed by Key) and assert that entries for "TLS_CIPHER_SUITES" and "TLS_MIN_VERSION" exist and their Value pointers dereference to "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256" and "VersionTLS12" respectively; reference the rev variable and its ManifestSubstitutions field when making these key-based assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/crd-compatibility-checker/main.go`:
- Around line 74-84: Register the OpenShift configv1 types into the controller
runtime scheme before calling InitOperatorConfig: call the configv1 scheme
registration (e.g., configv1.AddToScheme) on the same scheme variable used in
this file so that InitOperatorConfig can resolve configv1.APIServer without
unregistered-kind errors; ensure this registration happens prior to invoking
commoncmdoptions.InitOperatorConfig(ctx, log, cfg, scheme, managerName,
defaultManagerNamespace).
In `@pkg/commoncmdoptions/commonoptions.go`:
- Around line 107-115: The code currently calls pflag.Parse() on the global
pflag.CommandLine (which uses ExitOnError). Replace use of the global
CommandLine with a new flag set created via pflag.NewFlagSet("operator",
pflag.ContinueOnError), register flags with that FlagSet (use
capiflags.AddManagerOptions, textLoggerConfig.AddFlags, and
options.BindLeaderElectionFlags against the new FlagSet instead of
pflag.CommandLine), call fs.Parse(os.Args[1:]) and return or propagate any parse
error from InitOperatorConfig so callers can handle failures instead of os.Exit
being invoked. Ensure all references to pflag.Parse() and pflag.CommandLine in
this init path are switched to the new FlagSet (fs) and that parsing errors are
returned to the caller.
In `@pkg/controllers/revision/helpers_test.go`:
- Around line 53-56: The test currently creates a zero-valued tlsProfile from
tlsProfiles and always forwards it into the controller setup; instead, make
tlsProfile a pointer (e.g., tlsProfilePtr *configv1.TLSProfileSpec), set
tlsProfilePtr = &tlsProfiles[0] only when len(tlsProfiles) > 0, and pass
tlsProfilePtr into the controller setup call rather than the zero-value
tlsProfile so no implicit defaults are forwarded; update any function signatures
or call sites in this test helper that expect a non-pointer accordingly.
In `@pkg/controllers/revision/revision_controller_test.go`:
- Around line 506-508: The deferred cleanup unconditionally calls mgr.stop()
which may nil-deref if manager creation failed; modify the test to guard the
cleanup by either registering the DeferCleanup (or defer) only after assigning
mgr or by checking if mgr != nil before calling mgr.stop() in the cleanup
function (referencing the mgr variable and its stop method used in the existing
DeferCleanup block).
In `@pkg/revisiongenerator/revision.go`:
- Around line 258-263: In ToAPIRevision(), avoid returning r.substitutions by
reference: create and assign a copy of r.substitutions to the
ManifestSubstitutions field on the returned
operatorv1alpha1.ClusterAPIInstallerRevision so callers cannot mutate the cached
revision; locate ToAPIRevision(), r.substitutions and the return constructing
ClusterAPIInstallerRevision and perform a deep/appropriate copy (e.g., new
slice/map and copy elements) before setting ManifestSubstitutions.
- Around line 173-184: The loop over r.substitutions that calls h.Write on raw
s.Key and *s.Value can produce ambiguous byte streams; instead canonicalize each
substitution before hashing (e.g., write a clear frame per entry by prefixing
lengths or by marshaling a deterministic representation such as JSON for the
pair) so keys/values cannot concatenate into the same bytes (update the loop
that references r.substitutions and the calls to h.Write to write
framed/length-prefixed key and value entries, handling nil s.Value explicitly);
ensure this deterministic encoding is what feeds the ContentID hash.
---
Nitpick comments:
In `@pkg/controllers/revision/revision_controller_test.go`:
- Around line 524-529: The test is brittle because it asserts
ManifestSubstitutions by index; update the checks to be order-independent by
locating substitutions by Key instead of slice position: iterate
rev.ManifestSubstitutions (or convert it to a map keyed by Key) and assert that
entries for "TLS_CIPHER_SUITES" and "TLS_MIN_VERSION" exist and their Value
pointers dereference to "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256" and
"VersionTLS12" respectively; reference the rev variable and its
ManifestSubstitutions field when making these key-based assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 83f75de7-4016-42fd-be3c-b9df0a745ab5
⛔ Files ignored due to path filters (169)
e2e/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sumgo.workis excluded by!**/*.workmanifests-gen/go.sumis excluded by!**/*.sumvendor/github.com/openshift/api/config/v1/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_cluster_version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_dns.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_00_cluster-version-operator_01_clusterversions-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_00_cluster-version-operator_01_clusterversions-DevPreviewNoUpgrade.crd.yamlis 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-DevPreviewNoUpgrade.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_dnses-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_dnses-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_dnses-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_dnses-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_dnses-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.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*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_cluster_image_policy.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_image_policy.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/envtest-releases.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features.mdis excluded by!**/vendor/**,!vendor/**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/openapi/generated_openapi/zz_generated.openapi.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/operator/v1/types_csi_cluster_driver.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/types_ingress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/types_machineconfiguration.gois excluded by!**/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.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_70_network_01_networks-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_70_network_01_networks-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_70_network_01_networks-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_70_network_01_networks-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_70_network_01_networks-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigurations-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigurations-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*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/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*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_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.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/client-go/apiextensions/applyconfigurations/internal/internal.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/awsdnsspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/infrastructurestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/prefixedclaimmapping.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/update.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/usernameclaimmapping.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/additionalalertmanagerconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/alertmanagercustomconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/authorizationconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/basicauth.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/clusterimagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/clusterimagepolicyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/clusterimagepolicystatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/clustermonitoringspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/containerresource.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/dropequalactionconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/hashmodactionconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagepolicyfulciocawithrekorrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagepolicypkirootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagepolicypublickeyrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagepolicyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagepolicystatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagesigstoreverificationpolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/keepequalactionconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/label.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/labelmapactionconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/lowercaseactionconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/metadataconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/metadataconfigcustom.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/metricsserverconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/oauth2.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/oauth2endpointparam.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/openshiftstatemetricsconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/pkicertificatesubject.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/policyfulciosubject.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/policyidentity.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/policymatchexactrepository.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/policymatchremapidentity.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/policyrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/prometheusconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/prometheusoperatoradmissionwebhookconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/prometheusoperatorconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/prometheusremotewriteheader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/queueconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/relabelactionconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/relabelconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/remotewriteauthorization.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/remotewritespec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/replaceactionconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/retention.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/secretkeyselector.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/sigv4.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/telemeterclientconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/tlsconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/uppercaseactionconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/internal/internal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/clusterimagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/config_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/generated_expansion.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/imagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/informers/externalversions/config/v1alpha1/clusterimagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/informers/externalversions/config/v1alpha1/imagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/informers/externalversions/config/v1alpha1/interface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/informers/externalversions/generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/listers/config/v1alpha1/clusterimagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/listers/config/v1alpha1/expansion_generated.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/listers/config/v1alpha1/imagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machine/applyconfigurations/internal/internal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/internal/internal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/awscsidriverconfigspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/bgpmanagedconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/ingresscontrollertuningoptions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/nooverlayconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/ovnkubernetesconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1alpha1/clusterapiinstallercomponent.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1alpha1/clusterapiinstallerrevision.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1alpha1/clusterapiinstallerrevisionmanifestsubstitution.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1alpha1/clusterapistatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/controller-runtime-common/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/controller-runtime-common/pkg/tls/controller.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/controller-runtime-common/pkg/tls/tls.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/crypto/cert_config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/crypto/keygen.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/crypto/options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/crypto/tls_adherence.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/certrotation/client_cert_rotation_controller.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/certrotation/signer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/certrotation/target.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/v1helpers/helpers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/pki/profile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/pki/provider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/pki/resolve.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/pki/types.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (18)
cmd/capi-controllers/main.gocmd/capi-operator/main.gocmd/crd-compatibility-checker/main.gocmd/machine-api-migration/main.goe2e/go.modgo.modmanifests-gen/go.modpkg/commoncmdoptions/commonoptions.gopkg/commoncmdoptions/commonoptions_test.gopkg/commoncmdoptions/helpers_test.gopkg/commoncmdoptions/tls.gopkg/controllers/revision/helpers_test.gopkg/controllers/revision/revision_controller.gopkg/controllers/revision/revision_controller_test.gopkg/revisiongenerator/revision.gopkg/revisiongenerator/revision_test.gopkg/revisiongenerator/transform.gopkg/revisiongenerator/transform_test.go
Contributor
|
/test e2e-aws-capi-techpreview checking if it's an outage - ignore matt :) |
mdbooth
force-pushed
the
manifest-substitutions
branch
from
April 14, 2026 13:51
8f693f8 to
18ff2af
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
pkg/revisiongenerator/revision.go (2)
255-265:⚠️ Potential issue | 🟠 MajorDeep-copy
ManifestSubstitutionsbefore returning.
ToAPIRevision()still exposesr.substitutionsdirectly. Mutating the returned API object mutates the cached revision without invalidatingr.contentID, so later calls can report a stale identity.Suggested fix
contentID, err := r.ContentID() if err != nil { return operatorv1alpha1.ClusterAPIInstallerRevision{}, fmt.Errorf("error calculating contentID: %w", err) } + + apiSubstitutions := make([]operatorv1alpha1.ClusterAPIInstallerRevisionManifestSubstitution, len(r.substitutions)) + for i, s := range r.substitutions { + apiSubstitutions[i] = s + if s.Value != nil { + v := *s.Value + apiSubstitutions[i].Value = &v + } + } return operatorv1alpha1.ClusterAPIInstallerRevision{ Name: r.revisionName, Revision: r.revisionIndex, ContentID: contentID, - ManifestSubstitutions: r.substitutions, + ManifestSubstitutions: apiSubstitutions, Components: apiComponents, }, nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/revisiongenerator/revision.go` around lines 255 - 265, To avoid exposing the internal r.substitutions and preventing external mutation from invalidating r.contentID, deep-copy r.substitutions before assigning it to the returned ClusterAPIInstallerRevision.ManifestSubstitutions in ToAPIRevision (the code that builds the operatorv1alpha1.ClusterAPIInstallerRevision using r.revisionName, r.revisionIndex and contentID). Create a new copy (allocate a new slice/map and copy elements) and assign that copy to ManifestSubstitutions instead of r.substitutions so callers cannot mutate the cached revision state.
173-184:⚠️ Potential issue | 🔴 CriticalFrame substitution entries before hashing.
This still concatenates raw keys and values, so distinct substitution sets can alias the same
ContentID({"A":"BC"}vs{"AB":"C"}), which breaks revision identity.Suggested fix
- for _, s := range r.substitutions { - h.Write([]byte(s.Key)) - - if s.Value != nil { - h.Write([]byte(*s.Value)) - } - } + data, err := json.Marshal(r.substitutions) + if err != nil { + return "", fmt.Errorf("error marshalling substitutions: %w", err) + } + h.Write(data)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/revisiongenerator/revision.go` around lines 173 - 184, The loop that writes substitution entries into the hasher (r.substitutions -> h.Write using s.Key and s.Value) must frame each field so different key/value boundaries cannot concatenate into the same byte stream (e.g. {"A":"BC"} vs {"AB":"C"}). Fix by producing a deterministic encoding for each entry before hashing: sort r.substitutions by s.Key if ordering isn’t guaranteed, then write a length-prefixed or delimiter-framed representation for s.Key and for s.Value (and explicitly encode nil vs empty string) into h instead of raw concatenation so boundaries are unambiguous when computing the ContentID.
🧹 Nitpick comments (5)
e2e/e2e_common.go (1)
61-61: Userest.CopyConfig()to avoid sharing mutable pointer references.The assignment at line 95 directly aliases the pointer from
cfg, which is problematic sincee2e/tls_test.goreads this global during port-forward operations. Copying the config prevents accidental shared-state mutations across tests:- restConfig = cfg + restConfig = rest.CopyConfig(cfg)Also applies to: 95-95
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/e2e_common.go` at line 61, The global restConfig field is being assigned the original cfg pointer, causing shared-mutable state; instead use rest.CopyConfig to clone cfg before storing it (replace the direct assignment to restConfig with rest.CopyConfig(cfg) and handle the error if any), so locate the assignment that sets restConfig from cfg and change it to use rest.CopyConfig(cfg) to avoid pointer aliasing during tests like e2e/tls_test.go.cmd/machine-api-migration/main.go (1)
29-29: Inconsistent logging:klogstill used ingetFeatureGates.The refactor replaces
klogwith structuredlogr.Loggerlogging throughout, butgetFeatureGatesstill usesklog.Infofat line 240. Consider passing thelogr.LoggertogetFeatureGatesfor consistency.♻️ Suggested fix
Update the function signature and replace klog:
-func getFeatureGates(ctx context.Context, mgr ctrl.Manager) (featuregates.FeatureGateAccess, error) { +func getFeatureGates(ctx context.Context, log logr.Logger, mgr ctrl.Manager) (featuregates.FeatureGateAccess, error) {select { case <-featureGateAccessor.InitialFeatureGatesObserved(): featureGates, _ := featureGateAccessor.CurrentFeatureGates() - klog.Infof("FeatureGates initialized: %v", featureGates.KnownFeatures()) + log.Info("FeatureGates initialized", "knownFeatures", featureGates.KnownFeatures()) case <-time.After(1 * time.Minute):Then update the call site and remove the
"k8s.io/klog"import.Also applies to: 238-240
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/machine-api-migration/main.go` at line 29, getFeatureGates currently uses klog.Infof while the codebase was refactored to use structured logr.Logger; update getFeatureGates to accept a logr.Logger parameter (e.g., add logger logr.Logger to its signature), replace the klog.Infof call(s) inside getFeatureGates with logger.Info invocations, update all call sites that invoke getFeatureGates to pass the existing logr.Logger instance, and remove the now-unused "k8s.io/klog" import from the file.pkg/commoncmdoptions/helpers_test.go (1)
62-69: Consider usingstrings.HasPrefixfor clarity.The manual prefix check works but
strings.HasPrefix(e, k+"=")would be more idiomatic and readable.♻️ Suggested improvement
for _, e := range env { for k := range keepSet { - if len(e) > len(k) && e[:len(k)+1] == k+"=" { + if strings.HasPrefix(e, k+"=") { filtered = append(filtered, e) break } } }Note: This would require adding
"strings"to the imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/commoncmdoptions/helpers_test.go` around lines 62 - 69, Replace the manual prefix check inside the loop that builds filtered (the code iterating over env and keepSet) with strings.HasPrefix(e, k+"=") for clarity and idiomatic style; update the import list to include "strings" and change the conditional from len(e) > len(k) && e[:len(k)+1] == k+"=" to strings.HasPrefix(e, k+"="). Ensure behavior is unchanged (keepSet, env, filtered variables remain the same).pkg/commoncmdoptions/commonoptions_test.go (1)
284-301: Test uses deprecated TLS 1.1 to verify CLI override—intentional but worth documenting.The test case at line 286 uses
--tls-min-version=VersionTLS11which is a deprecated TLS version. This appears intentional to verify that CLI flags can override cluster defaults, but a brief comment explaining this choice would improve clarity.📝 Suggested documentation
{ - name: "TLS flags overridden by CLI", + name: "TLS flags overridden by CLI", + // Uses TLS 1.1 intentionally to verify CLI override takes precedence + // over cluster profile (which defaults to TLS 1.2). flags: []string{"--tls-min-version=VersionTLS11", "--tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/commoncmdoptions/commonoptions_test.go` around lines 284 - 301, Add a brief inline comment in the test case named "TLS flags overridden by CLI" (inside the verify closure in commonoptions_test.go) explaining that the flag "--tls-min-version=VersionTLS11" is intentionally using deprecated TLS 1.1 solely to verify that CLI flags override cluster defaults; place the comment near the flags slice or the assert that checks ManagerTLSMinVersion so future readers understand this is deliberate and not an oversight.pkg/commoncmdoptions/commonoptions.go (1)
134-140: Potential panic fromOrDiefunctions if cluster TLS profile is malformed.
TLSVersionOrDieandTLSVersionToNameOrDiewill panic if the cluster'sMinTLSVersioncontains an unexpected value. While cluster configuration should be valid, consider whether graceful error handling would be more appropriate here, especially since the function signature supports returning errors.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/commoncmdoptions/commonoptions.go` around lines 134 - 140, The current code calls libgocrypto.TLSVersionOrDie and TLSVersionToNameOrDie which can panic on malformed cluster TLS values; change to the non-panicking equivalents (the functions that return (value, error)) and handle errors when computing capiManagerOptions.TLSMinVersion and capiManagerOptions.TLSCipherSuites: check pflag.CommandLine.Changed("tls-min-version") and "tls-cipher-suites" as before, call the safe TLSVersion and TLSVersionToName functions on clusterTLSProfileSpec.MinTLSVersion and handle/return/log any error instead of letting it panic, and likewise use a non-panicking OpenSSLToIANACipherSuites (or validate clusterTLSProfileSpec.Ciphers) and handle errors before assigning capiManagerOptions.TLSCipherSuites so the process can fail gracefully.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/tls_test.go`:
- Around line 263-280: The reject-case can incorrectly pass during rollout
because any connection error from tryTLSConnect(...) is treated as a successful
rejection; modify the test around the Eventually block using findRunningPodName,
portForwardToPod and tryTLSConnect so that when shouldSucceed==false you first
verify the server is actually accepting TLS handshakes with a known-good version
(e.g., perform a successful handshake with a safe TLS version or retry until
handshake succeeds) and only then assert that a connection using the tested
tlsVersion fails, or alternatively narrow the failure check to only treat
protocol-version handshake failures from tryTLSConnect as acceptable rejections
rather than any connection/port-forward error.
- Around line 133-143: The cleanup currently only calls setTLSConfig inside
DeferCleanup (restoring apiServer.Spec.TLSAdherence / TLSSecurityProfile) but
does not wait for the cluster-wide rollback to finish, which can let later tests
start while endpoints are mid-restart; update the DeferCleanup to apply the
restore via setTLSConfig and then block until the APIServer rollout/restore is
observed (e.g., poll the APIServer resource status or kube-apiserver pods with
an Eventually-style wait) ensuring the restored TLSAdherence and
TLSSecurityProfile are reflected and kube-apiserver pods are Ready before
returning so subsequent specs run against the fully-rolled-back cluster.
- Line 115: The test suite is accidentally focused: replace the use of FDescribe
in the TLS Security Profile suite with a normal Describe so the full e2e test
suite runs; locate the FDescribe("TLS Security Profile", Ordered, func() {
declaration and change it to Describe("TLS Security Profile", Ordered, func() {
before merging.
---
Duplicate comments:
In `@pkg/revisiongenerator/revision.go`:
- Around line 255-265: To avoid exposing the internal r.substitutions and
preventing external mutation from invalidating r.contentID, deep-copy
r.substitutions before assigning it to the returned
ClusterAPIInstallerRevision.ManifestSubstitutions in ToAPIRevision (the code
that builds the operatorv1alpha1.ClusterAPIInstallerRevision using
r.revisionName, r.revisionIndex and contentID). Create a new copy (allocate a
new slice/map and copy elements) and assign that copy to ManifestSubstitutions
instead of r.substitutions so callers cannot mutate the cached revision state.
- Around line 173-184: The loop that writes substitution entries into the hasher
(r.substitutions -> h.Write using s.Key and s.Value) must frame each field so
different key/value boundaries cannot concatenate into the same byte stream
(e.g. {"A":"BC"} vs {"AB":"C"}). Fix by producing a deterministic encoding for
each entry before hashing: sort r.substitutions by s.Key if ordering isn’t
guaranteed, then write a length-prefixed or delimiter-framed representation for
s.Key and for s.Value (and explicitly encode nil vs empty string) into h instead
of raw concatenation so boundaries are unambiguous when computing the ContentID.
---
Nitpick comments:
In `@cmd/machine-api-migration/main.go`:
- Line 29: getFeatureGates currently uses klog.Infof while the codebase was
refactored to use structured logr.Logger; update getFeatureGates to accept a
logr.Logger parameter (e.g., add logger logr.Logger to its signature), replace
the klog.Infof call(s) inside getFeatureGates with logger.Info invocations,
update all call sites that invoke getFeatureGates to pass the existing
logr.Logger instance, and remove the now-unused "k8s.io/klog" import from the
file.
In `@e2e/e2e_common.go`:
- Line 61: The global restConfig field is being assigned the original cfg
pointer, causing shared-mutable state; instead use rest.CopyConfig to clone cfg
before storing it (replace the direct assignment to restConfig with
rest.CopyConfig(cfg) and handle the error if any), so locate the assignment that
sets restConfig from cfg and change it to use rest.CopyConfig(cfg) to avoid
pointer aliasing during tests like e2e/tls_test.go.
In `@pkg/commoncmdoptions/commonoptions_test.go`:
- Around line 284-301: Add a brief inline comment in the test case named "TLS
flags overridden by CLI" (inside the verify closure in commonoptions_test.go)
explaining that the flag "--tls-min-version=VersionTLS11" is intentionally using
deprecated TLS 1.1 solely to verify that CLI flags override cluster defaults;
place the comment near the flags slice or the assert that checks
ManagerTLSMinVersion so future readers understand this is deliberate and not an
oversight.
In `@pkg/commoncmdoptions/commonoptions.go`:
- Around line 134-140: The current code calls libgocrypto.TLSVersionOrDie and
TLSVersionToNameOrDie which can panic on malformed cluster TLS values; change to
the non-panicking equivalents (the functions that return (value, error)) and
handle errors when computing capiManagerOptions.TLSMinVersion and
capiManagerOptions.TLSCipherSuites: check
pflag.CommandLine.Changed("tls-min-version") and "tls-cipher-suites" as before,
call the safe TLSVersion and TLSVersionToName functions on
clusterTLSProfileSpec.MinTLSVersion and handle/return/log any error instead of
letting it panic, and likewise use a non-panicking OpenSSLToIANACipherSuites (or
validate clusterTLSProfileSpec.Ciphers) and handle errors before assigning
capiManagerOptions.TLSCipherSuites so the process can fail gracefully.
In `@pkg/commoncmdoptions/helpers_test.go`:
- Around line 62-69: Replace the manual prefix check inside the loop that builds
filtered (the code iterating over env and keepSet) with strings.HasPrefix(e,
k+"=") for clarity and idiomatic style; update the import list to include
"strings" and change the conditional from len(e) > len(k) && e[:len(k)+1] ==
k+"=" to strings.HasPrefix(e, k+"="). Ensure behavior is unchanged (keepSet,
env, filtered variables remain the same).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 6f587132-2560-41fe-828d-33575654ce0e
⛔ Files ignored due to path filters (93)
e2e/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sumvendor/github.com/gorilla/websocket/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/AUTHORSis excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/compression.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/conn.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/join.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/json.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/mask.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/mask_safe.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/prepared.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/proxy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/MAINTAINERSis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/NOTICEis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/connection.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/handlers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/priority.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/dictionary.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/read.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/write.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/stream.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/utils.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mxk/go-flowrate/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/mxk/go-flowrate/flowrate/flowrate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mxk/go-flowrate/flowrate/io.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mxk/go-flowrate/flowrate/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/.golangci.yamlis 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_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-OKD.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/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.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/etcd/install.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1/types_pacemakercluster.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/etcd/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/etcd/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/etcd/v1alpha1/types_pacemakercluster.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/features.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/features.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/openapi/generated_openapi/zz_generated.openapi.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_20_kube-apiserver_01_kubeapiservers-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_20_kube-apiserver_01_kubeapiservers-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_20_kube-apiserver_01_kubeapiservers-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_20_kube-apiserver_01_kubeapiservers-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_20_kube-apiserver_01_kubeapiservers.crd.yamlis 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_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.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/quota/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/quota/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/quota/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/golang.org/x/net/internal/socks/client.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/internal/socks/socks.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/proxy/dial.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/proxy/direct.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/proxy/per_host.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/proxy/proxy.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/proxy/socks5.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/httpstream/spdy/connection.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/httpstream/spdy/roundtripper.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/httpstream/spdy/upgrade.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/proxy/dial.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/proxy/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/proxy/transport.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/proxy/upgradeaware.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/third_party/forked/golang/netutil/addr.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/tools/portforward/OWNERSis excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/tools/portforward/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/tools/portforward/fallback_dialer.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/tools/portforward/portforward.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/tools/portforward/tunneling_connection.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/tools/portforward/tunneling_dialer.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/transport/spdy/spdy.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/transport/websocket/roundtripper.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (18)
cmd/capi-controllers/main.gocmd/capi-operator/main.gocmd/crd-compatibility-checker/main.gocmd/machine-api-migration/main.goe2e/e2e_common.goe2e/framework/framework.goe2e/go.mode2e/tls_test.gogo.modmanifests/0000_20_crd-compatibility-checker_08_deployment.yamlpkg/commoncmdoptions/commonoptions.gopkg/commoncmdoptions/commonoptions_test.gopkg/commoncmdoptions/helpers_test.gopkg/commoncmdoptions/tls.gopkg/controllers/installer/installer_controller_test.gopkg/controllers/revision/revision_controller_test.gopkg/revisiongenerator/revision.gopkg/revisiongenerator/revision_test.go
✅ Files skipped from review due to trivial changes (1)
- e2e/framework/framework.go
🚧 Files skipped from review as they are similar to previous changes (7)
- e2e/go.mod
- go.mod
- cmd/capi-operator/main.go
- pkg/controllers/revision/revision_controller_test.go
- pkg/commoncmdoptions/tls.go
- cmd/crd-compatibility-checker/main.go
- pkg/revisiongenerator/revision_test.go
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/tls_test.go`:
- Line 324: Fix the minor typo in the comment "// succeeds or fails at the
specified TLS version.." by removing the extra period so it reads "// succeeds
or fails at the specified TLS version." — update the comment in e2e/tls_test.go
(the line containing that comment) to use a single period.
- Around line 338-347: The closure passed to Eventually can call close(nil)
because portForwardToPod returns stopCh == nil on error; fix by only deferring
close(stopCh) after verifying err == nil (or checking stopCh != nil) inside the
closure used by Eventually so the deferred close is never called on a nil
channel (refer to the portForwardToPod call, the stopCh variable and the
deferred close(stopCh) in the Eventually closure).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 5e3d9bef-8771-4138-a078-2aeaf5ba2524
📒 Files selected for processing (11)
e2e/tls_test.gomanifests/0000_20_cluster-api-tls-config_role.yamlmanifests/0000_20_crd-compatibility-checker_04_rbac_bindings.yamlmanifests/0000_20_crd-compatibility-checker_05_metrics-service.yamlmanifests/0000_20_crd-compatibility-checker_08_deployment.yamlmanifests/0000_30_cluster-api-installer_01_metrics-service.yamlmanifests/0000_30_cluster-api-installer_03_clusterrolebinding.yamlmanifests/0000_30_cluster-api-installer_05_deployment.yamlmanifests/0000_30_cluster-api_04_rbac_bindings.yamlmanifests/0000_30_cluster-api_10_metrics-service.yamlmanifests/0000_30_cluster-api_11_deployment.yaml
✅ Files skipped from review due to trivial changes (4)
- manifests/0000_30_cluster-api-installer_01_metrics-service.yaml
- manifests/0000_20_cluster-api-tls-config_role.yaml
- manifests/0000_20_crd-compatibility-checker_05_metrics-service.yaml
- manifests/0000_30_cluster-api_10_metrics-service.yaml
damdo
changed the title
|
@mdbooth: This pull request references OCPCLOUD-3359 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 story to target the "5.0.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. |
openshift-ci-robot
added
the
jira/valid-reference
damdo
changed the title
|
@mdbooth: This pull request references OCPCLOUD-3359 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 story to target the "5.0.0" version, but no target version was set. This pull request references OCPCLOUD-3345 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 story to target the "5.0.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. |
|
@mdbooth: This pull request references OCPCLOUD-3359 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 story to target the "5.0.0" version, but no target version was set. This pull request references OCPCLOUD-3345 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 story to target the "5.0.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. |
mdbooth
force-pushed
the
manifest-substitutions
branch
from
April 15, 2026 14:40
9a4f831 to
4ede719
Compare
|
@mdbooth: This pull request references OCPCLOUD-3359 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 story to target the "5.0.0" version, but no target version was set. This pull request references OCPCLOUD-3345 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 story to target the "5.0.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: 2
♻️ Duplicate comments (3)
pkg/revisiongenerator/revision.go (2)
173-184:⚠️ Potential issue | 🔴 CriticalFrame each substitution before hashing.
This is still ambiguous: different substitution sets can serialize to the same byte stream here, so distinct revisions can end up with the same
ContentID.Suggested fix
- for _, s := range r.substitutions { - h.Write([]byte(s.Key)) - - if s.Value != nil { - h.Write([]byte(*s.Value)) - } - } + data, err := json.Marshal(r.substitutions) + if err != nil { + return "", fmt.Errorf("error marshalling substitutions: %w", err) + } + + h.Write(data)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/revisiongenerator/revision.go` around lines 173 - 184, The current loop over r.substitutions writes Key and Value bytes directly into the hasher (h.Write) which can produce ambiguous concatenations and equal ContentID for distinct substitution sets; update the loop in revision.go (the code that computes ContentID from r.substitutions) to frame each substitution unambiguously by prefixing length headers or explicit separators and by encoding nil values distinctly (e.g., write uint32 length for Key then Key bytes, then a marker for Value nil vs non-nil and if non-nil write uint32 length + Value bytes) before calling h.Write, so each substitution produces a unique, unambiguous byte sequence for the hash.
260-265:⚠️ Potential issue | 🟠 MajorReturn a copy of
ManifestSubstitutions.This still exposes
r.substitutionsby reference. A caller can mutate the returned API object and leave the cachedcontentIDstale.Suggested fix
contentID, err := r.ContentID() if err != nil { return operatorv1alpha1.ClusterAPIInstallerRevision{}, fmt.Errorf("error calculating contentID: %w", err) } + + apiSubs := make([]operatorv1alpha1.ClusterAPIInstallerRevisionManifestSubstitution, len(r.substitutions)) + for i, s := range r.substitutions { + apiSubs[i] = s + if s.Value != nil { + v := *s.Value + apiSubs[i].Value = &v + } + } return operatorv1alpha1.ClusterAPIInstallerRevision{ Name: r.revisionName, Revision: r.revisionIndex, ContentID: contentID, - ManifestSubstitutions: r.substitutions, + ManifestSubstitutions: apiSubs, Components: apiComponents, }, nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/revisiongenerator/revision.go` around lines 260 - 265, The returned ClusterAPIInstallerRevision currently assigns ManifestSubstitutions to r.substitutions by reference, allowing callers to mutate the cached substitutions and desynchronize contentID; to fix, create and assign a copied value (deep copy) of r.substitutions when building the operatorv1alpha1.ClusterAPIInstallerRevision in the function that returns it (the block that sets Name, Revision, ContentID, ManifestSubstitutions, Components). Ensure the copy duplicates the underlying collection elements (e.g., copy map entries or clone slice elements) rather than pointing to r.substitutions so the returned ManifestSubstitutions is independent of the internal cache.pkg/commoncmdoptions/commonoptions.go (1)
108-116:⚠️ Potential issue | 🟠 MajorDon't let
InitOperatorConfigterminate the process during flag parsing.This still parses the global
pflag.CommandLine, so parse failures bypassInitOperatorConfig's returned error path and canos.Exit(2)instead. That makes this API harder to call safely from binaries and tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/commoncmdoptions/commonoptions.go` around lines 108 - 116, InitOperatorConfig currently calls pflag.Parse() on the global pflag.CommandLine which will call os.Exit(2) on parse errors; change it to create and use a local pflag.FlagSet so parse failures return an error instead of terminating the process. Specifically, replace uses of pflag.CommandLine/pflag.Parse() in InitOperatorConfig by creating fs := pflag.NewFlagSet("operator", pflag.ContinueOnError), register flags with that fs (pass fs to capiflags.AddManagerOptions and options.BindLeaderElectionFlags and use textLoggerConfig.AddFlags on a compatible flag set), call fs.AddGoFlagSet(flag.CommandLine) and then call fs.Parse(os.Args[1:]) and return any parse error to the caller instead of letting pflag.Parse() abort; keep references to symbols InitOperatorConfig, pflag.CommandLine, pflag.Parse, capiflags.AddManagerOptions, textLoggerConfig.AddFlags, and options.BindLeaderElectionFlags to locate the changes.
🧹 Nitpick comments (2)
cmd/capi-operator/main.go (1)
150-157: Minor: Redundant error logging before return.The error is logged on line 155 and then wrapped and returned on line 156-157. The caller (
main) also logs errors before exiting. Consider removing the inline log to avoid duplicate error messages in logs.♻️ Suggested simplification
if err := (&revision.RevisionController{ Client: mgr.GetClient(), ProviderProfiles: providerProfiles, ReleaseVersion: util.GetReleaseVersion(), }).SetupWithManager(mgr, operatorConfig.ClusterTLSProfileSpec); err != nil { - log.Error(err, "unable to create revision controller", "controller", "RevisionController") return fmt.Errorf("unable to create revision controller: %w", err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/capi-operator/main.go` around lines 150 - 157, The RevisionController setup block logs the error with log.Error and then wraps and returns the same error, causing duplicate logging; remove the inline log.Error call inside the if-block and simply return the wrapped error from the SetupWithManager failure path (i.e., inside the conditional that calls (&revision.RevisionController{...}).SetupWithManager(mgr, operatorConfig.ClusterTLSProfileSpec)), leaving error handling to the caller (main) that already logs before exit.cmd/machine-api-migration/main.go (1)
29-29: Unused import:klogis imported but only used once.The
klogimport at line 29 appears to only be used at line 240 (klog.Infof), while the rest of the file uses the structuredlogr.Logger. Consider using thelogparameter consistently or removing theklogimport if full migration to structured logging is intended.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/machine-api-migration/main.go` at line 29, The klog import is unused except for a single klog.Infof call; replace that call with the structured logger passed into main (use the log parameter, e.g., log.Info with the same message and key/value pairs) and then remove the klog import, or if you prefer klog keep it but convert other logging to klog; update the call site reference (klog.Infof) to use the logr.Logger variable (log) so the file consistently uses structured logging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/framework/util.go`:
- Around line 117-127: Replace direct cl.Get calls passed into Eventually with
komega.Get so Eventually will poll; specifically change the two calls that
currently use Eventually(cl.Get(ctx, client.ObjectKey{Name: "version"},
clusterVersion)) and Eventually(cl.Get(ctx, client.ObjectKey{Name: "cluster"},
featureGate)) to use komega.Get(ctx, cl, client.ObjectKey{Name: "version"},
clusterVersion) and komega.Get(ctx, cl, client.ObjectKey{Name: "cluster"},
featureGate) respectively (mirror the pattern used in
IsMachineAPIMigrationEnabled) so clusterVersion and featureGate retrievals are
retried until success.
In `@pkg/commoncmdoptions/commonoptions_test.go`:
- Around line 83-99: TestMain currently ignores a non-nil error returned from
runTests and exits with the test exit code only; change it so that when runTests
returns a non-nil err you log/print the error (using fmt.Fprintf(os.Stderr,
...)) and ensure the final exit code is non-zero (e.g., set code = 1 or code =
max(code,1)) so failures in runTests (envtest setup/teardown, cleanup) fail the
test process; update the logic around runTests, execMode, and TestMain to
propagate that non-zero exit.
---
Duplicate comments:
In `@pkg/commoncmdoptions/commonoptions.go`:
- Around line 108-116: InitOperatorConfig currently calls pflag.Parse() on the
global pflag.CommandLine which will call os.Exit(2) on parse errors; change it
to create and use a local pflag.FlagSet so parse failures return an error
instead of terminating the process. Specifically, replace uses of
pflag.CommandLine/pflag.Parse() in InitOperatorConfig by creating fs :=
pflag.NewFlagSet("operator", pflag.ContinueOnError), register flags with that fs
(pass fs to capiflags.AddManagerOptions and options.BindLeaderElectionFlags and
use textLoggerConfig.AddFlags on a compatible flag set), call
fs.AddGoFlagSet(flag.CommandLine) and then call fs.Parse(os.Args[1:]) and return
any parse error to the caller instead of letting pflag.Parse() abort; keep
references to symbols InitOperatorConfig, pflag.CommandLine, pflag.Parse,
capiflags.AddManagerOptions, textLoggerConfig.AddFlags, and
options.BindLeaderElectionFlags to locate the changes.
In `@pkg/revisiongenerator/revision.go`:
- Around line 173-184: The current loop over r.substitutions writes Key and
Value bytes directly into the hasher (h.Write) which can produce ambiguous
concatenations and equal ContentID for distinct substitution sets; update the
loop in revision.go (the code that computes ContentID from r.substitutions) to
frame each substitution unambiguously by prefixing length headers or explicit
separators and by encoding nil values distinctly (e.g., write uint32 length for
Key then Key bytes, then a marker for Value nil vs non-nil and if non-nil write
uint32 length + Value bytes) before calling h.Write, so each substitution
produces a unique, unambiguous byte sequence for the hash.
- Around line 260-265: The returned ClusterAPIInstallerRevision currently
assigns ManifestSubstitutions to r.substitutions by reference, allowing callers
to mutate the cached substitutions and desynchronize contentID; to fix, create
and assign a copied value (deep copy) of r.substitutions when building the
operatorv1alpha1.ClusterAPIInstallerRevision in the function that returns it
(the block that sets Name, Revision, ContentID, ManifestSubstitutions,
Components). Ensure the copy duplicates the underlying collection elements
(e.g., copy map entries or clone slice elements) rather than pointing to
r.substitutions so the returned ManifestSubstitutions is independent of the
internal cache.
---
Nitpick comments:
In `@cmd/capi-operator/main.go`:
- Around line 150-157: The RevisionController setup block logs the error with
log.Error and then wraps and returns the same error, causing duplicate logging;
remove the inline log.Error call inside the if-block and simply return the
wrapped error from the SetupWithManager failure path (i.e., inside the
conditional that calls (&revision.RevisionController{...}).SetupWithManager(mgr,
operatorConfig.ClusterTLSProfileSpec)), leaving error handling to the caller
(main) that already logs before exit.
In `@cmd/machine-api-migration/main.go`:
- Line 29: The klog import is unused except for a single klog.Infof call;
replace that call with the structured logger passed into main (use the log
parameter, e.g., log.Info with the same message and key/value pairs) and then
remove the klog import, or if you prefer klog keep it but convert other logging
to klog; update the call site reference (klog.Infof) to use the logr.Logger
variable (log) so the file consistently uses structured logging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 20238e2f-aa5c-4cfc-ac5a-b98e7b425a74
⛔ Files ignored due to path filters (245)
e2e/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sumgo.workis excluded by!**/*.workmanifests-gen/go.sumis excluded by!**/*.sumvendor/github.com/gorilla/websocket/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/AUTHORSis excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/compression.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/conn.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/join.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/json.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/mask.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/mask_safe.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/prepared.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/proxy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/MAINTAINERSis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/NOTICEis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/connection.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/handlers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/priority.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/dictionary.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/read.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/write.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/stream.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/utils.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mxk/go-flowrate/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/mxk/go-flowrate/flowrate/flowrate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mxk/go-flowrate/flowrate/io.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mxk/go-flowrate/flowrate/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/.golangci.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/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_cluster_version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_dns.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_00_cluster-version-operator_01_clusterversions-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_00_cluster-version-operator_01_clusterversions-DevPreviewNoUpgrade.crd.yamlis 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-DevPreviewNoUpgrade.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_dnses-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_dnses-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_dnses-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_dnses-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_dnses-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.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*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_cluster_image_policy.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_image_policy.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/envtest-releases.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/install.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1/types_pacemakercluster.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/etcd/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/etcd/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/etcd/v1alpha1/types_pacemakercluster.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/features.mdis excluded by!**/vendor/**,!vendor/**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/openapi/generated_openapi/zz_generated.openapi.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/operator/v1/types_csi_cluster_driver.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/types_ingress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/types_machineconfiguration.gois excluded by!**/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.crd-manifests/0000_20_kube-apiserver_01_kubeapiservers-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_20_kube-apiserver_01_kubeapiservers-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_20_kube-apiserver_01_kubeapiservers-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_20_kube-apiserver_01_kubeapiservers-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_20_kube-apiserver_01_kubeapiservers.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_70_network_01_networks-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_70_network_01_networks-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_70_network_01_networks-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_70_network_01_networks-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_70_network_01_networks-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigurations-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigurations-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*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/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*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_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.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/quota/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/quota/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/quota/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/client-go/apiextensions/applyconfigurations/internal/internal.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/awsdnsspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/infrastructurestatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/prefixedclaimmapping.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/update.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/usernameclaimmapping.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/additionalalertmanagerconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/alertmanagercustomconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/authorizationconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/basicauth.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/clusterimagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/clusterimagepolicyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/clusterimagepolicystatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/clustermonitoringspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/containerresource.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/dropequalactionconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/hashmodactionconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagepolicyfulciocawithrekorrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagepolicypkirootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagepolicypublickeyrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagepolicyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagepolicystatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagesigstoreverificationpolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/keepequalactionconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/label.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/labelmapactionconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/lowercaseactionconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/metadataconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/metadataconfigcustom.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/metricsserverconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/oauth2.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/oauth2endpointparam.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/openshiftstatemetricsconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/pkicertificatesubject.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/policyfulciosubject.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/policyidentity.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/policymatchexactrepository.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/policymatchremapidentity.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/policyrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/prometheusconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/prometheusoperatoradmissionwebhookconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/prometheusoperatorconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/prometheusremotewriteheader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/queueconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/relabelactionconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/relabelconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/remotewriteauthorization.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/remotewritespec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/replaceactionconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/retention.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/secretkeyselector.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/sigv4.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/telemeterclientconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/tlsconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/uppercaseactionconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/internal/internal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/clusterimagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/config_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/generated_expansion.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/imagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/informers/externalversions/config/v1alpha1/clusterimagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/informers/externalversions/config/v1alpha1/imagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/informers/externalversions/config/v1alpha1/interface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/informers/externalversions/generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/listers/config/v1alpha1/clusterimagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/listers/config/v1alpha1/expansion_generated.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/listers/config/v1alpha1/imagepolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/machine/applyconfigurations/internal/internal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/internal/internal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/awscsidriverconfigspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/bgpmanagedconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/ingresscontrollertuningoptions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/nooverlayconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/ovnkubernetesconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1alpha1/clusterapiinstallercomponent.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1alpha1/clusterapiinstallerrevision.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1alpha1/clusterapiinstallerrevisionmanifestsubstitution.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1alpha1/clusterapistatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/controller-runtime-common/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/controller-runtime-common/pkg/tls/controller.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/controller-runtime-common/pkg/tls/tls.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/crypto/cert_config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/crypto/keygen.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/crypto/options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/crypto/tls_adherence.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/certrotation/client_cert_rotation_controller.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/certrotation/signer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/certrotation/target.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/v1helpers/helpers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/pki/profile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/pki/provider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/pki/resolve.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/pki/types.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/internal/socks/client.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/internal/socks/socks.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/proxy/dial.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/proxy/direct.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/proxy/per_host.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/proxy/proxy.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/proxy/socks5.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/httpstream/spdy/connection.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/httpstream/spdy/roundtripper.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/httpstream/spdy/upgrade.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/proxy/dial.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/proxy/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/proxy/transport.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/proxy/upgradeaware.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/third_party/forked/golang/netutil/addr.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/tools/portforward/OWNERSis excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/tools/portforward/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/tools/portforward/fallback_dialer.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/tools/portforward/portforward.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/tools/portforward/tunneling_connection.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/tools/portforward/tunneling_dialer.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/transport/spdy/spdy.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/transport/websocket/roundtripper.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (35)
cmd/capi-controllers/main.gocmd/capi-operator/main.gocmd/crd-compatibility-checker/main.gocmd/machine-api-migration/main.goe2e/e2e_common.goe2e/framework/framework.goe2e/framework/util.goe2e/go.mode2e/tls_test.gogo.modmanifests-gen/go.modmanifests/0000_20_cluster-api-tls-config_role.yamlmanifests/0000_20_crd-compatibility-checker_04_rbac_bindings.yamlmanifests/0000_20_crd-compatibility-checker_05_metrics-service.yamlmanifests/0000_20_crd-compatibility-checker_08_deployment.yamlmanifests/0000_30_cluster-api-installer_01_metrics-service.yamlmanifests/0000_30_cluster-api-installer_03_clusterrolebinding.yamlmanifests/0000_30_cluster-api-installer_05_deployment.yamlmanifests/0000_30_cluster-api_04_rbac_bindings.yamlmanifests/0000_30_cluster-api_10_metrics-service.yamlmanifests/0000_30_cluster-api_11_deployment.yamlpkg/commoncmdoptions/commonoptions.gopkg/commoncmdoptions/commonoptions_test.gopkg/commoncmdoptions/helpers_test.gopkg/commoncmdoptions/tls.gopkg/controllers/installer/helpers_test.gopkg/controllers/installer/installer_controller_test.gopkg/controllers/revision/helpers_test.gopkg/controllers/revision/revision_controller.gopkg/controllers/revision/revision_controller_test.gopkg/revisiongenerator/revision.gopkg/revisiongenerator/revision_test.gopkg/revisiongenerator/transform.gopkg/revisiongenerator/transform_test.gopkg/test/envtest.go
✅ Files skipped from review due to trivial changes (10)
- manifests-gen/go.mod
- manifests/0000_30_cluster-api-installer_01_metrics-service.yaml
- e2e/framework/framework.go
- manifests/0000_20_cluster-api-tls-config_role.yaml
- pkg/commoncmdoptions/helpers_test.go
- manifests/0000_30_cluster-api_04_rbac_bindings.yaml
- manifests/0000_30_cluster-api_10_metrics-service.yaml
- pkg/commoncmdoptions/tls.go
- cmd/crd-compatibility-checker/main.go
- e2e/tls_test.go
🚧 Files skipped from review as they are similar to previous changes (9)
- e2e/e2e_common.go
- pkg/controllers/revision/helpers_test.go
- pkg/revisiongenerator/transform.go
- manifests/0000_20_crd-compatibility-checker_05_metrics-service.yaml
- manifests/0000_20_crd-compatibility-checker_04_rbac_bindings.yaml
- manifests/0000_30_cluster-api-installer_03_clusterrolebinding.yaml
- e2e/go.mod
- pkg/controllers/revision/revision_controller_test.go
- pkg/revisiongenerator/revision_test.go
mdbooth
force-pushed
the
manifest-substitutions
branch
from
April 15, 2026 15:01
4ede719 to
58981ac
Compare
|
@mdbooth: This pull request references OCPCLOUD-3359 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 story to target the "5.0.0" version, but no target version was set. This pull request references OCPCLOUD-3345 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 story to target the "5.0.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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
manifests/0000_20_crd-compatibility-checker_08_deployment.yaml (1)
38-86:⚠️ Potential issue | 🟠 MajorAdd explicit container/pod securityContext hardening.
The deployment still relies on default security context behavior, which is what Trivy/Checkov are flagging. Please set explicit non-root / no-privilege-escalation / seccomp defaults on the pod and container.
🔐 Proposed hardening patch
spec: template: spec: + securityContext: + runAsNonRoot: true + seccompProfile: + type: RuntimeDefault containers: - name: compatibility-requirements-controllers + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: + - ALL image: registry.ci.openshift.org/openshift:cluster-capi-operator🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@manifests/0000_20_crd-compatibility-checker_08_deployment.yaml` around lines 38 - 86, Add explicit pod and container securityContext entries to harden the deployment: in the PodSpec add a podSecurityContext with runAsNonRoot: true, runAsUser (non-zero uid), runAsGroup and fsGroup values and set seccompProfile.type: RuntimeDefault; in the container spec for the compatibility-requirements-controllers container add securityContext with allowPrivilegeEscalation: false, privileged: false, runAsNonRoot: true (matching pod runAsUser), capabilities.drop: ["ALL"], and seccompProfile.type: RuntimeDefault to ensure no privilege escalation and a non-root runtime.cmd/crd-compatibility-checker/main.go (1)
43-49:⚠️ Potential issue | 🔴 CriticalMissing
configv1scheme registration.The
initSchemefunction does not registerconfigv1, butInitOperatorConfigcallsresolveTLSProfilewhich requires it to fetch theconfigv1.APIServerresource to resolve the cluster TLS profile. Without this registration, a runtime error will occur when attempting to fetch the APIServer.Proposed fix
import ( "context" "flag" "fmt" "os" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" apiextensionsv1alpha1 "github.com/openshift/api/apiextensions/v1alpha1" + configv1 "github.com/openshift/api/config/v1" operatorv1 "github.com/openshift/api/operator/v1"func initScheme(scheme *runtime.Scheme) { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(admissionregistrationv1.AddToScheme(scheme)) utilruntime.Must(apiextensionsv1.AddToScheme(scheme)) utilruntime.Must(apiextensionsv1alpha1.AddToScheme(scheme)) + utilruntime.Must(configv1.Install(scheme)) utilruntime.Must(operatorv1.AddToScheme(scheme)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/crd-compatibility-checker/main.go` around lines 43 - 49, The initScheme function is missing registration of the configv1 API, causing resolveTLSProfile (called from InitOperatorConfig) to fail when fetching configv1.APIServer; update initScheme to add utilruntime.Must(configv1.AddToScheme(scheme)) so the configv1 types are registered before any client operations that call resolveTLSProfile/InitOperatorConfig.
♻️ Duplicate comments (1)
pkg/controllers/revision/revision_controller_test.go (1)
505-510:⚠️ Potential issue | 🟡 MinorGuard cleanup when manager creation never happens.
mgr.stop()is called unconditionally in theDeferCleanup, butmgris assigned inside theItblock at line 518. If the test fails before that assignment,mgrremains nil and causes a panic during cleanup, hiding the original failure.Proposed fix
DeferCleanup(func(ctx context.Context) { + if mgr != nil { mgr.stop() + } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controllers/revision/revision_controller_test.go` around lines 505 - 510, The DeferCleanup currently calls mgr.stop() unconditionally which can panic if mgr was never created; update the BeforeEach/DeferCleanup pairing so the cleanup guards against a nil manager—e.g., in the DeferCleanup closure check that mgr != nil (or mgr != nil && mgr.IsRunning() if available) before calling mgr.stop(); locate the DeferCleanup inside the BeforeEach around createFixtures and adjust it to perform the nil-check on the mgr variable to avoid panics when the manager creation in the It block never happens.
🧹 Nitpick comments (1)
cmd/machine-api-migration/main.go (1)
237-240: Consider using consistent logging.The rest of the file uses
logr.Logger, but this function still usesklog.Infof. While functional, using consistent logging throughout would improve maintainability.Suggested change
Pass the logger to
getFeatureGatesand use it instead ofklog:-func getFeatureGates(ctx context.Context, mgr ctrl.Manager) (featuregates.FeatureGateAccess, error) { +func getFeatureGates(ctx context.Context, log logr.Logger, mgr ctrl.Manager) (featuregates.FeatureGateAccess, error) { // ... select { case <-featureGateAccessor.InitialFeatureGatesObserved(): featureGates, _ := featureGateAccessor.CurrentFeatureGates() - klog.Infof("FeatureGates initialized: %v", featureGates.KnownFeatures()) + log.Info("FeatureGates initialized", "features", featureGates.KnownFeatures()) // ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/machine-api-migration/main.go` around lines 237 - 240, The logging in getFeatureGates still uses klog.Infof while the rest of the file uses a logr.Logger; update getFeatureGates to accept a logger parameter (e.g., logger logr.Logger) and replace klog.Infof with logger.Info calls, locating the change around getFeatureGates where featureGateAccessor.InitialFeatureGatesObserved() and featureGateAccessor.CurrentFeatureGates() are used and ensure the call sites that invoke getFeatureGates pass the existing logger instance.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@cmd/crd-compatibility-checker/main.go`:
- Around line 43-49: The initScheme function is missing registration of the
configv1 API, causing resolveTLSProfile (called from InitOperatorConfig) to fail
when fetching configv1.APIServer; update initScheme to add
utilruntime.Must(configv1.AddToScheme(scheme)) so the configv1 types are
registered before any client operations that call
resolveTLSProfile/InitOperatorConfig.
In `@manifests/0000_20_crd-compatibility-checker_08_deployment.yaml`:
- Around line 38-86: Add explicit pod and container securityContext entries to
harden the deployment: in the PodSpec add a podSecurityContext with
runAsNonRoot: true, runAsUser (non-zero uid), runAsGroup and fsGroup values and
set seccompProfile.type: RuntimeDefault; in the container spec for the
compatibility-requirements-controllers container add securityContext with
allowPrivilegeEscalation: false, privileged: false, runAsNonRoot: true (matching
pod runAsUser), capabilities.drop: ["ALL"], and seccompProfile.type:
RuntimeDefault to ensure no privilege escalation and a non-root runtime.
---
Duplicate comments:
In `@pkg/controllers/revision/revision_controller_test.go`:
- Around line 505-510: The DeferCleanup currently calls mgr.stop()
unconditionally which can panic if mgr was never created; update the
BeforeEach/DeferCleanup pairing so the cleanup guards against a nil
manager—e.g., in the DeferCleanup closure check that mgr != nil (or mgr != nil
&& mgr.IsRunning() if available) before calling mgr.stop(); locate the
DeferCleanup inside the BeforeEach around createFixtures and adjust it to
perform the nil-check on the mgr variable to avoid panics when the manager
creation in the It block never happens.
---
Nitpick comments:
In `@cmd/machine-api-migration/main.go`:
- Around line 237-240: The logging in getFeatureGates still uses klog.Infof
while the rest of the file uses a logr.Logger; update getFeatureGates to accept
a logger parameter (e.g., logger logr.Logger) and replace klog.Infof with
logger.Info calls, locating the change around getFeatureGates where
featureGateAccessor.InitialFeatureGatesObserved() and
featureGateAccessor.CurrentFeatureGates() are used and ensure the call sites
that invoke getFeatureGates pass the existing logger instance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 6b7db100-2a1a-4c0d-a729-e684694cfe04
⛔ Files ignored due to path filters (64)
e2e/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.summanifests-gen/go.sumis excluded by!**/*.sumvendor/github.com/gorilla/websocket/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/AUTHORSis excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/compression.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/conn.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/join.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/json.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/mask.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/mask_safe.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/prepared.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/proxy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/MAINTAINERSis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/NOTICEis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/connection.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/handlers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/priority.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/dictionary.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/read.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/write.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/stream.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/utils.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mxk/go-flowrate/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/mxk/go-flowrate/flowrate/flowrate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mxk/go-flowrate/flowrate/io.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mxk/go-flowrate/flowrate/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/controller-runtime-common/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/controller-runtime-common/pkg/tls/controller.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/controller-runtime-common/pkg/tls/tls.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/internal/socks/client.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/internal/socks/socks.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/proxy/dial.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/proxy/direct.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/proxy/per_host.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/proxy/proxy.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/proxy/socks5.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/httpstream/spdy/connection.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/httpstream/spdy/roundtripper.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/httpstream/spdy/upgrade.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/proxy/dial.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/proxy/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/proxy/transport.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/proxy/upgradeaware.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/third_party/forked/golang/netutil/addr.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/tools/portforward/OWNERSis excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/tools/portforward/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/tools/portforward/fallback_dialer.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/tools/portforward/portforward.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/tools/portforward/tunneling_connection.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/tools/portforward/tunneling_dialer.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/transport/spdy/spdy.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/client-go/transport/websocket/roundtripper.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (20)
cmd/capi-controllers/main.gocmd/capi-operator/main.gocmd/crd-compatibility-checker/main.gocmd/machine-api-migration/main.goe2e/e2e_common.goe2e/framework/framework.goe2e/framework/util.goe2e/go.mode2e/tls_test.gogo.modmanifests-gen/go.modmanifests/0000_20_crd-compatibility-checker_08_deployment.yamlpkg/commoncmdoptions/commonoptions.gopkg/commoncmdoptions/commonoptions_test.gopkg/commoncmdoptions/helpers_test.gopkg/commoncmdoptions/tls.gopkg/controllers/revision/helpers_test.gopkg/controllers/revision/revision_controller.gopkg/controllers/revision/revision_controller_test.gopkg/test/envtest.go
✅ Files skipped from review due to trivial changes (5)
- pkg/test/envtest.go
- manifests-gen/go.mod
- e2e/framework/framework.go
- pkg/commoncmdoptions/helpers_test.go
- e2e/tls_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- e2e/e2e_common.go
- go.mod
- pkg/controllers/revision/helpers_test.go
- cmd/capi-operator/main.go
3 tasks
mdbooth
force-pushed
the
manifest-substitutions
branch
from
April 21, 2026 15:28
d6dafdb to
d918255
Compare
Contributor
|
Scheduling tests matching the |
Contributor
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
1 similar comment
Contributor
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
mdbooth
force-pushed
the
manifest-substitutions
branch
from
April 22, 2026 16:29
d918255 to
bca2927
Compare
Contributor
|
Scheduling tests matching the |
Member
|
/retest |
1 similar comment
Member
|
/retest |
Contributor
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
2 similar comments
Contributor
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
Contributor
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
Contributor
Author
|
/verified by @mdbooth |
openshift-ci-robot
added
the
verified
|
@mdbooth: This PR has been marked as verified by 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. |
Contributor
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
damdo
reviewed
Apr 23, 2026
Contributor
Contributor
Author
|
The 2 OpenStack failures against this PR look like infra flakes. I would prefer not to override if we can get them to pass. |
Contributor
|
@mdbooth: 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. |
openshift-merge-bot
Bot
merged commit Apr 23, 2026
f46195f
into
openshift:main
23 of 25 checks passed
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
4 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.
Depends on openshift/api#2786
Summary by CodeRabbit
New Features
Improvements
Tests
Bug Fixes