Skip to content

NO-JIRA: Upgrade Golangci-lint to version 2#1237

Merged
openshift-merge-bot[bot] merged 9 commits intoopenshift:masterfrom
ncaak:maintenance/golangci/upgrade-to-version-2-10
Mar 2, 2026
Merged

NO-JIRA: Upgrade Golangci-lint to version 2#1237
openshift-merge-bot[bot] merged 9 commits intoopenshift:masterfrom
ncaak:maintenance/golangci/upgrade-to-version-2-10

Conversation

@ncaak
Copy link
Contributor

@ncaak ncaak commented Feb 24, 2026

summary

This PR attempts to resolve the linting issue caused by unexpected side effects of an older Golangci-lint version with modern Go versions. The current version was tested locally with Golang +1.25 after the migration.

The Golangci-lint configuration has been migrated to version 2, and some new linting issues have been resolved.

Categories

  • Bugfix
  • Data Enhancement
  • Feature
  • Backporting
  • Others (CI, Infrastructure, Documentation)

Summary by CodeRabbit

  • Chores
    • Replaced and simplified linting configuration and updated the linter installer for a consistent, versioned setup.
  • Refactor
    • Modernized internal data access and control-flow patterns and adjusted readiness/timing calculations.
  • Tests
    • Updated multiple tests to use revised time handling and updated fake client APIs for more robust test behavior.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Feb 24, 2026
@openshift-ci-robot
Copy link
Contributor

@ncaak: This pull request explicitly references no jira issue.

Details

In response to this:

summary

This PR attempts to resolve the linting issue caused by unexpected side effects of an older Golangci-lint version with modern Go versions. The current version was tested locally with Golang +1.25 after the migration.

The Golangci-lint configuration has been migrated to version 2, and some new linting issues have been resolved.

Categories

  • Bugfix
  • Data Enhancement
  • Feature
  • Backporting
  • Others (CI, Infrastructure, Documentation)

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 openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 24, 2026
@openshift-ci
Copy link

openshift-ci bot commented Feb 24, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@coderabbitai
Copy link

coderabbitai bot commented Feb 24, 2026

📝 Walkthrough

Walkthrough

Consolidates and reconfigures golangci-lint and its installer; updates test and production code to use direct metav1.Time accessors, top-level struct fields, and direct fake client AddReactor calls; minor lint directive and interval-source adjustments across several packages.

Changes

Cohort / File(s) Summary
Linter config & installer
\.golangci\.yml, .openshiftci/install-golangci-lint.sh, Makefile
Replace granular golangci-lint config with versioned, explicit-enable config, consolidated settings, exclusions and formatter rules; bump installer to v2.10.1; increase lint timeout in Makefile.
CLI tooling — linter directives & control flow
cmd/changelog/main.go, cmd/gendoc/main.go
Refactor if/else to switch in changelog; adjust nolint directives (add/change nolint:noctx) on exec.Command calls.
Kubernetes fake client test usage
pkg/anonymization/anonymizer_test.go, pkg/config/configobserver/secretconfigobserver_test.go
Replace reactor registrations via embedded Fake fields with direct AddReactor calls on the fake Clientset / FakeCoreV1 types.
metav1.Time and timing source changes
pkg/controller/status/controller.go, pkg/controller/operator.go, pkg/controller/periodic/periodic_test.go
Use metav1.Time accessors (Add, UTC) instead of .Time field; switch interval source from s.Controller.Interval to s.Interval for readiness timing.
Cluster gatherers — struct field/path updates
pkg/gatherers/clusterconfig/cluster_config_v1_config_map.go, pkg/gatherers/clusterconfig/events_filtering.go, pkg/gatherers/clusterconfig/gather_pod_disruption_budgets_test.go, pkg/gatherers/clusterconfig/gather_sap_pods.go
Move platform-scoped fields to top-level (installConfig.AWS.* etc.), use top-level Name/Namespace, and access OwnerReferences and event timestamps via public fields/methods.
Misc lint suppressions & small edits
pkg/gatherers/workloads/gather_workloads_runtime_infos.go, pkg/ocm/clustertransfer/types.go
Add nolint:gosec on HTTP call and struct Secret field to suppress gosec warnings.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

"I hopped through configs, tidy and neat,
Linted the lines with very quick feet.
I nudged some Time and fixed where refs lie,
Bumped golangci, gave warnings a sigh.
A carrot of tests — all green, oh so sweet!" 🐇

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 21.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the primary change: upgrading golangci-lint to version 2. It is concise, clear, and directly related to the main objective of the PR.
Description check ✅ Passed The PR description addresses the main purpose (resolving linting issues with upgraded golangci-lint) and correctly selects the category (Others: CI, Infrastructure, Documentation). However, it lacks complete coverage of the template's optional but recommended sections like Sample Archive, Documentation, Unit Tests, Privacy, Changelog, Breaking Changes, and References.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ncaak
Copy link
Contributor Author

ncaak commented Feb 24, 2026

/test lint

@openshift-ci
Copy link

openshift-ci bot commented Feb 24, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ncaak

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 24, 2026
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Nitpick comments (3)
pkg/controller/periodic/periodic_test.go (1)

873-934: Prefer a single base timestamp to avoid drift in age-based tests.

Using one metav1.Now() captured up-front keeps all object timestamps consistent and avoids any edge flakiness near prune thresholds.

♻️ Suggested refactor
 func TestPeriodicPrune(t *testing.T) {
+	baseNow := metav1.Now()
 	tests := []struct {
 		name                string
 		jobs                []runtime.Object
@@
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "to-be-removed-job-1",
 						Namespace: insightsNamespace,
 						CreationTimestamp: metav1.Time{
-							Time: metav1.Now().Add(-25 * time.Hour),
+							Time: baseNow.Add(-25 * time.Hour),
 						},
 					},
 				},
@@
 						Name:      "to-be-removed-job-2",
 						Namespace: insightsNamespace,
 						CreationTimestamp: metav1.Time{
-							Time: metav1.Now().Add(-1441 * time.Minute),
+							Time: baseNow.Add(-1441 * time.Minute),
 						},
 					},
 				},
@@
 						Name:      "to-keep-job-1",
 						Namespace: insightsNamespace,
 						CreationTimestamp: metav1.Time{
-							Time: metav1.Now().Add(-23 * time.Hour),
+							Time: baseNow.Add(-23 * time.Hour),
 						},
 					},
 				},
@@
 						Name:      "to-keep-job-2",
 						Namespace: insightsNamespace,
 						CreationTimestamp: metav1.Time{
-							Time: metav1.Now().Add(-2 * time.Hour),
+							Time: baseNow.Add(-2 * time.Hour),
 						},
 					},
 				},
 			},
@@
 					ObjectMeta: metav1.ObjectMeta{
 						Name: "to-be-removed-dg-1",
 						CreationTimestamp: metav1.Time{
-							Time: metav1.Now().Add(-25 * time.Hour),
+							Time: baseNow.Add(-25 * time.Hour),
 						},
 					},
 				},
@@
 					ObjectMeta: metav1.ObjectMeta{
 						Name: "to-be-removed-dg-2",
 						CreationTimestamp: metav1.Time{
-							Time: metav1.Now().Add(-1441 * time.Minute),
+							Time: baseNow.Add(-1441 * time.Minute),
 						},
 					},
 				},
@@
 					ObjectMeta: metav1.ObjectMeta{
 						Name: "to-keep-dg-1",
 						CreationTimestamp: metav1.Time{
-							Time: metav1.Now().Add(-1339 * time.Minute),
+							Time: baseNow.Add(-1339 * time.Minute),
 						},
 					},
 				},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/controller/periodic/periodic_test.go` around lines 873 - 934, The test
uses multiple metav1.Now() calls causing timestamp drift; fix by capturing a
single base timestamp (e.g., baseTime := metav1.Now()) at the start of the
"Basic pruning test" case and replace all metav1.Now().Add(...) uses in the
batchv1.Job and insightsv1.DataGather CreationTimestamp fields with
baseTime.Add(...) so all objects share the same reference time and avoid edge
flakiness around prune thresholds.
.golangci.yml (2)

37-38: gocognit is configured but never enabled.

gocognit has a settings entry (min-complexity: 15) but does not appear in the enable list under linters. The settings block is dead. Either add gocognit to the enabled list or drop the settings.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.golangci.yml around lines 37 - 38, The gocognit settings block (gocognit /
min-complexity: 15) is defined but not applied because gocognit is missing from
the linters enable list; either add "gocognit" to the "linters.enable" array so
the min-complexity setting is used, or remove the gocognit settings block
entirely if you don't want the linter enabled. Locate the "linters" / "enable"
list and add the exact symbol "gocognit" to it, or delete the
gocognit/min-complexity entries to eliminate the dead configuration.

70-73: nolintlint is configured but not enabled — allow-unused: false has no effect.

The nolintlint settings (including allow-unused: false) are orphaned: the linter is absent from the enable list. Given that the codebase still carries several old-style // nolint: directives that are now silently broken (see comments on cmd/gendoc/main.go and cmd/changelog/main.go), enabling nolintlint would actually help catch future stale or malformed directives. Consider either enabling it or removing the dead settings block.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.golangci.yml around lines 70 - 73, The nolintlint configuration block
(nolintlint: require-explanation/require-specific/allow-unused) is present but
not enabled, so allow-unused: false has no effect; either add "nolintlint" to
the linters.enable list in .golangci.yml so the nolintlint rules (including
allow-unused: false) are enforced, or remove the entire nolintlint: block to
avoid dead config—update the top-level linters.enable array or delete the
nolintlint keys accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.golangci.yml:
- Around line 109-111: The goimports.local-prefixes entry is set to
golangci-lint's own module (github.com/golangci/golangci-lint) instead of this
repository, so local imports in insights-operator won't be recognized; update
the goimports.local-prefixes value in .golangci.yml to this project's module
path (the module string from your go.mod, e.g.,
github.com/your-org/insights-operator) or remove the entry if you want defaults,
ensuring goimports can correctly group the third import section.
- Around line 59-65: The govet.printf.funcs entries in .golangci.yml reference
golangci-lint's internal symbols
((github.com/golangci/golangci-lint/pkg/logutils.Log).Infof, .Warnf, .Errorf,
.Fatalf) which never occur in this repo; remove these entries or replace them
with actual printf-style wrappers used here (e.g., your project's logger
functions) so the govet printf check targets real symbols — update the
settings.printf.funcs block to either be deleted or to list the project's true
logger methods instead.

In @.openshiftci/install-golangci-lint.sh:
- Line 13: The command unquotes the GOPATH expansion which can cause
word-splitting; update the install invocation to quote the substitution for the
-b argument so the shell passes the full path when GOPATH contains spaces
(change the use of $(go env GOPATH)/bin in the curl | sh -s -- -b ... invocation
to a quoted form like "$(go env GOPATH)/bin").

In `@cmd/changelog/main.go`:
- Around line 357-358: Replace the old standalone " // nolint: gosec"
suppression with the proper nolint token format and placement so golangci-lint
recognizes it: change the two occurrences around the vulnerable exec.Command
calls to use "//nolint:gosec" (no space after // and no spaces around :) and
attach it to the exec.Command line (the lines building out, err :=
exec.Command(...)) that use fmt.Sprintf with after/until/hash so gosec G204 is
suppressed for those exec.Command invocations.
- Line 336: The call in releaseBranchesContain that runs exec.Command("git",
"branch", "--contains", hash) is flagged by gosec G204; add the same suppression
used elsewhere by appending a gosec nosec comment to that call (e.g., add "//
`#nosec` G204" or the project's preferred gosec suppression token) so the linter
ignores this intentional use of a variable git argument; update the exec.Command
line in releaseBranchesContain accordingly and ensure the comment matches the
style used in timeFrameReverseGitLog/sinceHashReverseGitLog.

In `@cmd/gendoc/main.go`:
- Around line 371-372: The standalone old-style " // nolint: gosec" should be
removed and the gosec suppression merged into the existing inline nolint on the
exec.Command line: update the directive attached to the exec.Command call (the
statement that assigns to cmd and uses cleanRoot and f) to use the v2 format
without spaces (e.g., //nolint:gosec,noctx) so golangci-lint v2 recognizes the
suppression for gosec G204 while preserving the existing noctx suppression.

---

Nitpick comments:
In @.golangci.yml:
- Around line 37-38: The gocognit settings block (gocognit / min-complexity: 15)
is defined but not applied because gocognit is missing from the linters enable
list; either add "gocognit" to the "linters.enable" array so the min-complexity
setting is used, or remove the gocognit settings block entirely if you don't
want the linter enabled. Locate the "linters" / "enable" list and add the exact
symbol "gocognit" to it, or delete the gocognit/min-complexity entries to
eliminate the dead configuration.
- Around line 70-73: The nolintlint configuration block (nolintlint:
require-explanation/require-specific/allow-unused) is present but not enabled,
so allow-unused: false has no effect; either add "nolintlint" to the
linters.enable list in .golangci.yml so the nolintlint rules (including
allow-unused: false) are enforced, or remove the entire nolintlint: block to
avoid dead config—update the top-level linters.enable array or delete the
nolintlint keys accordingly.

In `@pkg/controller/periodic/periodic_test.go`:
- Around line 873-934: The test uses multiple metav1.Now() calls causing
timestamp drift; fix by capturing a single base timestamp (e.g., baseTime :=
metav1.Now()) at the start of the "Basic pruning test" case and replace all
metav1.Now().Add(...) uses in the batchv1.Job and insightsv1.DataGather
CreationTimestamp fields with baseTime.Add(...) so all objects share the same
reference time and avoid edge flakiness around prune thresholds.

ℹ️ Review info

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between dfa9ec1 and ba5264b.

📒 Files selected for processing (13)
  • .golangci.yml
  • .openshiftci/install-golangci-lint.sh
  • cmd/changelog/main.go
  • cmd/gendoc/main.go
  • pkg/anonymization/anonymizer_test.go
  • pkg/config/configobserver/secretconfigobserver_test.go
  • pkg/controller/operator.go
  • pkg/controller/periodic/periodic_test.go
  • pkg/controller/status/controller.go
  • pkg/gatherers/clusterconfig/cluster_config_v1_config_map.go
  • pkg/gatherers/clusterconfig/events_filtering.go
  • pkg/gatherers/clusterconfig/gather_pod_disruption_budgets_test.go
  • pkg/gatherers/clusterconfig/gather_sap_pods.go

func releaseBranchesContain(hash string) []ReleaseVersion {
var releaseBranches []ReleaseVersion
out, err := exec.Command("git", "branch", "--contains", hash).CombinedOutput()
out, err := exec.Command("git", "branch", "--contains", hash).CombinedOutput() //nolint:noctx
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

releaseBranchesContain: missing gosec suppression for G204.

exec.Command("git", "branch", "--contains", hash) passes hash as a variable argument, which gosec G204 flags. Unlike timeFrameReverseGitLog and sinceHashReverseGitLog, this call has no gosec suppression at all (neither the old nor new format).

🛠️ Proposed fix
-	out, err := exec.Command("git", "branch", "--contains", hash).CombinedOutput() //nolint:noctx
+	out, err := exec.Command("git", "branch", "--contains", hash).CombinedOutput() //nolint:noctx,gosec
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
out, err := exec.Command("git", "branch", "--contains", hash).CombinedOutput() //nolint:noctx
out, err := exec.Command("git", "branch", "--contains", hash).CombinedOutput() //nolint:noctx,gosec
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmd/changelog/main.go` at line 336, The call in releaseBranchesContain that
runs exec.Command("git", "branch", "--contains", hash) is flagged by gosec G204;
add the same suppression used elsewhere by appending a gosec nosec comment to
that call (e.g., add "// `#nosec` G204" or the project's preferred gosec
suppression token) so the linter ignores this intentional use of a variable git
argument; update the exec.Command line in releaseBranchesContain accordingly and
ensure the comment matches the style used in
timeFrameReverseGitLog/sinceHashReverseGitLog.

Comment on lines 357 to +358
// nolint: gosec
out, err := exec.Command(
out, err := exec.Command( //nolint:noctx
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Old-format // nolint: gosec directives are silently ignored in golangci-lint v2.

Same root cause as cmd/gendoc/main.go line 371: the directive syntax requires no spaces around : or after //. The standalone // nolint: gosec lines 357 and 373 are not recognized as suppression directives, so gosec G204 will fire on both exec.Command calls (which pass fmt.Sprintf results derived from after/until/hash variables).

🛠️ Proposed fix (apply to both occurrences)
-	// nolint: gosec
-	out, err := exec.Command( //nolint:noctx
+	out, err := exec.Command( //nolint:noctx,gosec

Also applies to: 373-374

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmd/changelog/main.go` around lines 357 - 358, Replace the old standalone "
// nolint: gosec" suppression with the proper nolint token format and placement
so golangci-lint recognizes it: change the two occurrences around the vulnerable
exec.Command calls to use "//nolint:gosec" (no space after // and no spaces
around :) and attach it to the exec.Command line (the lines building out, err :=
exec.Command(...)) that use fmt.Sprintf with after/until/hash so gosec G204 is
suppressed for those exec.Command invocations.

Comment on lines 371 to +372
// nolint: gosec
cmd := exec.Command("go", "run", fmt.Sprintf("%s%s", cleanRoot, f))
cmd := exec.Command("go", "run", fmt.Sprintf("%s%s", cleanRoot, f)) //nolint:noctx
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Old-format // nolint: gosec (with space) is not recognized as a directive in golangci-lint v2.

The syntax of nolint directives in golangci-lint v2 is //[a-zA-Z]+:[a-zA-Z].* — spaces around : and after // must be removed. The standalone // nolint: gosec on line 371 is treated as a plain comment, so gosec G204 will fire on the exec.Command call that passes cleanRoot and f as variable arguments. The allow-leading-space option was dropped in v2, so there is no config knob to re-enable the old behavior.

Merge the gosec suppression into the existing inline directive:

🛠️ Proposed fix
-	// nolint: gosec
-	cmd := exec.Command("go", "run", fmt.Sprintf("%s%s", cleanRoot, f)) //nolint:noctx
+	cmd := exec.Command("go", "run", fmt.Sprintf("%s%s", cleanRoot, f)) //nolint:noctx,gosec
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// nolint: gosec
cmd := exec.Command("go", "run", fmt.Sprintf("%s%s", cleanRoot, f))
cmd := exec.Command("go", "run", fmt.Sprintf("%s%s", cleanRoot, f)) //nolint:noctx
cmd := exec.Command("go", "run", fmt.Sprintf("%s%s", cleanRoot, f)) //nolint:noctx,gosec
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmd/gendoc/main.go` around lines 371 - 372, The standalone old-style " //
nolint: gosec" should be removed and the gosec suppression merged into the
existing inline nolint on the exec.Command line: update the directive attached
to the exec.Command call (the statement that assigns to cmd and uses cleanRoot
and f) to use the v2 format without spaces (e.g., //nolint:gosec,noctx) so
golangci-lint v2 recognizes the suppression for gosec G204 while preserving the
existing noctx suppression.

@ncaak
Copy link
Contributor Author

ncaak commented Feb 24, 2026

/test lint

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
.golangci.yml (2)

37-38: gocognit settings are dead — linter is not enabled.

gocognit has min-complexity: 15 configured here but is absent from the enable list above (only gocyclo is enabled). Either add gocognit to the enable list or remove these settings.

♻️ Proposed fix
-    gocognit:
-      min-complexity: 15

Or, to also enable it alongside gocyclo:

     - gocyclo
+    - gocognit
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.golangci.yml around lines 37 - 38, The gocognit configuration block is
inactive because the linter is not in the enabled list; either add "gocognit" to
the enable list alongside "gocyclo" or remove the gocognit settings entirely.
Locate the enable section (where gocyclo is listed) and add "gocognit" to that
list, or delete the gocognit: min-complexity: 15 block so there are no orphaned
settings.

62-65: nolintlint is configured but not enabled — allow-unused: false is silently a no-op.

The nolintlint linter is not in the enable list, so these settings never take effect. In particular, allow-unused: false (which would flag stale //nolint directives) provides no enforcement. Without nolintlint active, unused nolint comments will accumulate undetected.

Either enable the linter (the settings are already present) or remove the dead configuration block.

♻️ Option A — enable the linter (recommended)
     - whitespace
+    - nolintlint
♻️ Option B — remove dead settings
-    nolintlint:
-      require-explanation: false
-      require-specific: false
-      allow-unused: false
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.golangci.yml around lines 62 - 65, The .golangci.yml contains a dead
configuration block for nolintlint (keys: nolintlint and allow-unused) but the
linter isn't enabled; either add "nolintlint" to the enable list in the config
so the nolintlint settings (including allow-unused: false) take effect, or
remove the entire nolintlint block to avoid confusion; locate the "nolintlint"
section and either add its name into the top-level enable list or delete the
block to resolve the no-op configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In @.golangci.yml:
- Around line 37-38: The gocognit configuration block is inactive because the
linter is not in the enabled list; either add "gocognit" to the enable list
alongside "gocyclo" or remove the gocognit settings entirely. Locate the enable
section (where gocyclo is listed) and add "gocognit" to that list, or delete the
gocognit: min-complexity: 15 block so there are no orphaned settings.
- Around line 62-65: The .golangci.yml contains a dead configuration block for
nolintlint (keys: nolintlint and allow-unused) but the linter isn't enabled;
either add "nolintlint" to the enable list in the config so the nolintlint
settings (including allow-unused: false) take effect, or remove the entire
nolintlint block to avoid confusion; locate the "nolintlint" section and either
add its name into the top-level enable list or delete the block to resolve the
no-op configuration.

ℹ️ Review info

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between ba5264b and fe0e437.

📒 Files selected for processing (2)
  • .golangci.yml
  • .openshiftci/install-golangci-lint.sh

@ncaak
Copy link
Contributor Author

ncaak commented Feb 25, 2026

/test lint

@ncaak
Copy link
Contributor Author

ncaak commented Feb 25, 2026

/test lint

@ncaak
Copy link
Contributor Author

ncaak commented Feb 26, 2026

/test lint

@ncaak ncaak marked this pull request as ready for review February 26, 2026 12:06
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 26, 2026
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
pkg/ocm/clustertransfer/types.go (1)

33-33: Consider specifying the gosec rule and adding a justification.

The nolint:gosec directive correctly suppresses a false positive (G101 flags field names containing "secret"). For better maintainability, consider specifying the rule and adding a brief reason:

-	Secret         string    `json:"secret,omitempty"` // nolint:gosec
+	Secret         string    `json:"secret,omitempty"` //nolint:gosec // G101 - field name, not a hardcoded credential

This helps future maintainers understand why the suppression exists.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/ocm/clustertransfer/types.go` at line 33, Update the nolint on the Secret
field in pkg/ocm/clustertransfer/types.go to explicitly name the gosec rule and
add a brief justification: locate the Secret struct field (Secret string
`json:"secret,omitempty"`) and replace the generic `// nolint:gosec` with a
targeted suppression that references G101 and a short reason (e.g., false
positive because this field stores a reference/ID rather than sensitive
material), so maintainers know why the rule is disabled.
pkg/gatherers/workloads/gather_workloads_runtime_infos.go (1)

182-182: Consider specifying the gosec rule and adding a justification.

The nolint directive suppresses all gosec warnings. Best practice is to specify the exact rule (likely G107 for variable URL) and document why suppression is safe. This helps future maintainers understand the intent and ensures only the intended warning is suppressed.

✨ Suggested improvement
-	resp, err := httpCli.Do(request) // nolint:gosec
+	resp, err := httpCli.Do(request) //nolint:gosec // G107: URL constructed from trusted pod IP in cluster
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/gatherers/workloads/gather_workloads_runtime_infos.go` at line 182,
Replace the blanket suppression on the http call at the resp, err :=
httpCli.Do(request) site by targeting the specific gosec rule (G107) and adding
an inline justification; e.g., change the comment to suppress only gosec G107
(use // nolint:gosec) and append a short reason like "// reason: G107 suppressed
because request URL is a constant/validated internal value" referencing httpCli
and request so future readers know why the variable-URL warning is safe.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pkg/gatherers/workloads/gather_workloads_runtime_infos.go`:
- Line 182: Replace the blanket suppression on the http call at the resp, err :=
httpCli.Do(request) site by targeting the specific gosec rule (G107) and adding
an inline justification; e.g., change the comment to suppress only gosec G107
(use // nolint:gosec) and append a short reason like "// reason: G107 suppressed
because request URL is a constant/validated internal value" referencing httpCli
and request so future readers know why the variable-URL warning is safe.

In `@pkg/ocm/clustertransfer/types.go`:
- Line 33: Update the nolint on the Secret field in
pkg/ocm/clustertransfer/types.go to explicitly name the gosec rule and add a
brief justification: locate the Secret struct field (Secret string
`json:"secret,omitempty"`) and replace the generic `// nolint:gosec` with a
targeted suppression that references G101 and a short reason (e.g., false
positive because this field stores a reference/ID rather than sensitive
material), so maintainers know why the rule is disabled.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between fe0e437 and 468de59.

📒 Files selected for processing (4)
  • .golangci.yml
  • Makefile
  • pkg/gatherers/workloads/gather_workloads_runtime_infos.go
  • pkg/ocm/clustertransfer/types.go

@ncaak
Copy link
Contributor Author

ncaak commented Feb 26, 2026

/retest-required

@ncaak
Copy link
Contributor Author

ncaak commented Feb 27, 2026

/retest

@BaiyangZhou
Copy link

/retest

1 similar comment
@BaiyangZhou
Copy link

/retest

@BaiyangZhou
Copy link

/override pull-ci-openshift-insights-operator-master-e2e-gcp-ovn-techpreview

@BaiyangZhou
Copy link

/override pull-ci-openshift-insights-operator-master-insights-operator-conditions-tests

@BaiyangZhou
Copy link

/override pull-ci-openshift-insights-operator-master-insights-operator-e2e-tests

@BaiyangZhou
Copy link

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Mar 2, 2026
@openshift-ci
Copy link

openshift-ci bot commented Mar 2, 2026

@BaiyangZhou: Overrode contexts on behalf of BaiyangZhou: ci/prow/e2e-gcp-ovn-techpreview

Details

In response to this:

/override pull-ci-openshift-insights-operator-master-e2e-gcp-ovn-techpreview

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 kubernetes-sigs/prow repository.

@openshift-ci
Copy link

openshift-ci bot commented Mar 2, 2026

@BaiyangZhou: Overrode contexts on behalf of BaiyangZhou: ci/prow/insights-operator-conditions-tests

Details

In response to this:

/override pull-ci-openshift-insights-operator-master-insights-operator-conditions-tests

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 kubernetes-sigs/prow repository.

@openshift-ci
Copy link

openshift-ci bot commented Mar 2, 2026

@BaiyangZhou: Overrode contexts on behalf of BaiyangZhou: ci/prow/insights-operator-e2e-tests

Details

In response to this:

/override pull-ci-openshift-insights-operator-master-insights-operator-e2e-tests

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 kubernetes-sigs/prow repository.

@BaiyangZhou
Copy link

/verified later by @BaiyangZhou

@openshift-ci-robot
Copy link
Contributor

@BaiyangZhou: Only users can be targets for the /verified later command.

Details

In response to this:

/verified later by @BaiyangZhou

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.

@BaiyangZhou
Copy link

/verified bypass

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Mar 2, 2026
@openshift-ci-robot
Copy link
Contributor

@BaiyangZhou: The verified label has been added.

Details

In response to this:

/verified bypass

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.

@opokornyy
Copy link
Contributor

/override pull-ci-openshift-insights-operator-master-insights-operator-conditions-tests
/override pull-ci-openshift-insights-operator-master-insights-operator-e2e-tests
/override pull-ci-openshift-insights-operator-master-e2e-gcp-ovn-techpreview
/override pull-ci-openshift-insights-operator-master-insights-operator-conditions-gcp-ovn-techpreview
/override lint

@openshift-ci
Copy link

openshift-ci bot commented Mar 2, 2026

@opokornyy: /override requires failed status contexts, check run or a prowjob name to operate on.
The following unknown contexts/checkruns were given:

  • lint

Only the following failed contexts/checkruns were expected:

  • CodeRabbit
  • ci/prow/e2e
  • ci/prow/e2e-agnostic-upgrade
  • ci/prow/e2e-gcp-ovn-techpreview
  • ci/prow/images
  • ci/prow/insights-operator-conditions-gcp-ovn-techpreview
  • ci/prow/insights-operator-conditions-tests
  • ci/prow/insights-operator-e2e-tests
  • ci/prow/lint
  • ci/prow/okd-scos-images
  • ci/prow/unit
  • ci/prow/verify-deps
  • pull-ci-openshift-insights-operator-master-e2e
  • pull-ci-openshift-insights-operator-master-e2e-agnostic-upgrade
  • pull-ci-openshift-insights-operator-master-e2e-gcp-ovn-techpreview
  • pull-ci-openshift-insights-operator-master-images
  • pull-ci-openshift-insights-operator-master-insights-operator-conditions-gcp-ovn-techpreview
  • pull-ci-openshift-insights-operator-master-insights-operator-conditions-tests
  • pull-ci-openshift-insights-operator-master-insights-operator-e2e-tests
  • pull-ci-openshift-insights-operator-master-lint
  • pull-ci-openshift-insights-operator-master-okd-scos-images
  • pull-ci-openshift-insights-operator-master-unit
  • pull-ci-openshift-insights-operator-master-verify-deps
  • tide

If you are trying to override a checkrun that has a space in it, you must put a double quote on the context.

Details

In response to this:

/override pull-ci-openshift-insights-operator-master-insights-operator-conditions-tests
/override pull-ci-openshift-insights-operator-master-insights-operator-e2e-tests
/override pull-ci-openshift-insights-operator-master-e2e-gcp-ovn-techpreview
/override pull-ci-openshift-insights-operator-master-insights-operator-conditions-gcp-ovn-techpreview
/override lint

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 kubernetes-sigs/prow repository.

@openshift-ci
Copy link

openshift-ci bot commented Mar 2, 2026

@ncaak: Overrode contexts on behalf of ncaak: ci/prow/e2e-gcp-ovn-techpreview

Details

In response to this:

/override ci/prow/e2e-gcp-ovn-techpreview

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 kubernetes-sigs/prow repository.

@opokornyy
Copy link
Contributor

/override pull-ci-openshift-insights-operator-master-lint

@openshift-ci
Copy link

openshift-ci bot commented Mar 2, 2026

@opokornyy: Overrode contexts on behalf of opokornyy: ci/prow/lint

Details

In response to this:

/override pull-ci-openshift-insights-operator-master-lint

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 kubernetes-sigs/prow repository.

@opokornyy
Copy link
Contributor

/override pull-ci-openshift-insights-operator-master-insights-operator-conditions-tests

@openshift-ci
Copy link

openshift-ci bot commented Mar 2, 2026

@opokornyy: Overrode contexts on behalf of opokornyy: ci/prow/insights-operator-conditions-tests

Details

In response to this:

/override pull-ci-openshift-insights-operator-master-insights-operator-conditions-tests

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 kubernetes-sigs/prow repository.

@opokornyy
Copy link
Contributor

/override pull-ci-openshift-insights-operator-master-e2e-gcp-ovn-techpreview

@openshift-ci
Copy link

openshift-ci bot commented Mar 2, 2026

@opokornyy: Overrode contexts on behalf of opokornyy: ci/prow/e2e-gcp-ovn-techpreview

Details

In response to this:

/override pull-ci-openshift-insights-operator-master-e2e-gcp-ovn-techpreview

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 kubernetes-sigs/prow repository.

@opokornyy
Copy link
Contributor

/override pull-ci-openshift-insights-operator-master-insights-operator-e2e-tests

@openshift-ci
Copy link

openshift-ci bot commented Mar 2, 2026

@opokornyy: Overrode contexts on behalf of opokornyy: ci/prow/insights-operator-e2e-tests

Details

In response to this:

/override pull-ci-openshift-insights-operator-master-insights-operator-e2e-tests

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 kubernetes-sigs/prow repository.

@opokornyy
Copy link
Contributor

/override pull-ci-openshift-insights-operator-master-insights-operator-conditions-gcp-ovn-techpreview

@openshift-ci
Copy link

openshift-ci bot commented Mar 2, 2026

@opokornyy: Overrode contexts on behalf of opokornyy: ci/prow/insights-operator-conditions-gcp-ovn-techpreview

Details

In response to this:

/override pull-ci-openshift-insights-operator-master-insights-operator-conditions-gcp-ovn-techpreview

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 kubernetes-sigs/prow repository.

@openshift-merge-bot openshift-merge-bot bot merged commit 87a1112 into openshift:master Mar 2, 2026
13 checks passed
@openshift-ci
Copy link

openshift-ci bot commented Mar 2, 2026

@ncaak: all tests passed!

Full PR test history. Your PR dashboard.

Details

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. qe-approved Signifies that QE has signed off on this PR verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants