Skip to content

NO-ISSUE: Update linter version#10509

Open
pastequo wants to merge 1 commit into
openshift:masterfrom
pastequo:ci/upgrade-linter
Open

NO-ISSUE: Update linter version#10509
pastequo wants to merge 1 commit into
openshift:masterfrom
pastequo:ci/upgrade-linter

Conversation

@pastequo

@pastequo pastequo commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

List all the issues related to this PR

  • New Feature
  • Enhancement
  • Bug fix
  • Tests
  • Documentation
  • CI/CD

What environments does this code impact?

  • Automation (CI, tools, etc)
  • Cloud
  • Operator Managed Deployments
  • None

How was this code tested?

  • assisted-test-infra environment
  • dev-scripts environment
  • Reviewer's test appreciated
  • Waiting for CI to do a full test run
  • Manual (Elaborate on how it was tested)
  • No tests needed

Checklist

  • Title and description added to both, commit and PR.
  • Relevant issues have been associated (see CONTRIBUTING guide)
  • This change does not require a documentation update (docstring, docs, README, etc)
  • Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

  • Are the title and description (in both PR and commit) meaningful and clear?
  • Is there a bug required (and linked) for this change?
  • Should this PR be backported?

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of pointer-containing data when checking for non-empty values and when rendering structured secret output.
  • Chores

    • Updated the linting tool version used in build and CI images.

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

Copy link
Copy Markdown

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

Details

In response to this:

List all the issues related to this PR

  • New Feature
  • Enhancement
  • Bug fix
  • Tests
  • Documentation
  • CI/CD

What environments does this code impact?

  • Automation (CI, tools, etc)
  • Cloud
  • Operator Managed Deployments
  • None

How was this code tested?

  • assisted-test-infra environment
  • dev-scripts environment
  • Reviewer's test appreciated
  • Waiting for CI to do a full test run
  • Manual (Elaborate on how it was tested)
  • No tests needed

Checklist

  • Title and description added to both, commit and PR.
  • Relevant issues have been associated (see CONTRIBUTING guide)
  • This change does not require a documentation update (docstring, docs, README, etc)
  • Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

  • Are the title and description (in both PR and commit) meaningful and clear?
  • Is there a bug required (and linked) for this change?
  • Should this PR be backported?

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.

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Walkthrough

Two independent changes: golangci-lint is bumped from v2.11.4 to v2.12.2 in both Dockerfile.assisted-service-build and ci-images/Dockerfile.lint, and the deprecated reflect.Ptr constant is replaced with reflect.Pointer in IsSliceNonEmpty and dumpSecretStructInternal.

Changes

golangci-lint version bump

Layer / File(s) Summary
golangci-lint v2.11.4 → v2.12.2
Dockerfile.assisted-service-build, ci-images/Dockerfile.lint
Both Dockerfiles update the pinned golangci-lint version string passed to the upstream install.sh script.

reflect.Ptr → reflect.Pointer alias update

Layer / File(s) Summary
reflect.Pointer substitution in Go utilities
internal/common/common.go, pkg/secretdump/struct.go
IsSliceNonEmpty and dumpSecretStructInternal replace reflect.Ptr with the canonical reflect.Pointer alias for pointer-kind comparisons.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 13 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The template is present, but the required summary, issue link, motivation, and testing details are left empty or generic. Add a brief change summary, linked issue(s), motivation/context, any dependencies, and concrete test or CI notes.
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (13 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: updating the linter version.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed No test files or Ginkgo titles were changed; the PR only touches two Dockerfiles and two non-test Go helpers.
Test Structure And Quality ✅ Passed Touched files are Dockerfiles plus non-test helpers; no Ginkgo/BeforeEach/Eventually constructs were present, so this check is not applicable.
Microshift Test Compatibility ✅ Passed No new Ginkgo e2e tests were added; the PR only touches Dockerfiles and utility Go code, with no test declarations or MicroShift-sensitive APIs.
Single Node Openshift (Sno) Test Compatibility ✅ Passed PR only updates Dockerfiles and non-test Go helpers; no new Ginkgo e2e tests or SNO-sensitive assumptions were added.
Topology-Aware Scheduling Compatibility ✅ Passed The PR only updates linter/Dockerfile versions and two helper functions; no manifests, controllers, or scheduling primitives were added or changed.
Ote Binary Stdout Contract ✅ Passed Touched code is utility/Dockerfile only; no main/init/TestMain/suite setup or stdout/logging writes were added in the modified paths.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PR only updates Dockerfiles and two helper functions; no new Ginkgo e2e tests or IPv4/external-network assumptions were added.
No-Weak-Crypto ✅ Passed Touched files only adjust linter Dockerfiles and reflect.Kind checks; no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB or non-constant-time secret comparisons were added.
Container-Privileges ✅ Passed The PR only updates linter versions and Go reflection kinds; no privileged/root/hostPID/hostNetwork/hostIPC/allowPrivilegeEscalation settings were added or changed.
No-Sensitive-Data-In-Logs ✅ Passed No new or changed logging exposes sensitive data; touched files only update linter versions and pointer-kind checks, and the secret-dump helper still redacts secrets.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@openshift-ci openshift-ci Bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jun 24, 2026
@openshift-ci openshift-ci Bot requested review from giladravid16 and shay23bra June 24, 2026 08:16
@openshift-ci openshift-ci Bot added the kind/dependency-change Categorizes issue or PR as related to changing dependencies label Jun 24, 2026
@openshift-ci

openshift-ci Bot commented Jun 24, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pastequo

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 Jun 24, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ci-images/Dockerfile.lint`:
- Line 3: The golangci-lint installer in Dockerfile.lint is pulling install.sh
from the mutable main branch, which can change independently of this image
build. Update the curl source used by the RUN step to reference the same
immutable v2.12.2 tag as the installed version so the build is reproducible;
keep the existing install.sh invocation and pin the script URL consistently.

In `@Dockerfile.assisted-service-build`:
- Line 3: The golangci-lint installer in Dockerfile.assisted-service-build is
fetched from a moving branch reference, which can change unexpectedly; update
the curl source in the RUN step to use an immutable release/tagged ref matching
v2.12.2 or vendor the installer script locally. Keep the fix in the same
Dockerfile line that invokes install.sh so the build always uses a pinned,
reproducible installer version.
🪄 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: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 51413ba7-4502-4b2e-a0cc-f174c89d1df9

📥 Commits

Reviewing files that changed from the base of the PR and between 08dd374 and 8a7fce1.

📒 Files selected for processing (4)
  • Dockerfile.assisted-service-build
  • ci-images/Dockerfile.lint
  • internal/common/common.go
  • pkg/secretdump/struct.go

Comment thread ci-images/Dockerfile.lint
FROM base

RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/main/install.sh | sh -s -- -b /usr/bin v2.11.4
RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/main/install.sh | sh -s -- -b /usr/bin v2.12.2

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

git ls-files ci-images/Dockerfile.lint
echo '---'
cat -n ci-images/Dockerfile.lint | sed -n '1,40p'

Repository: openshift/assisted-service

Length of output: 389


🏁 Script executed:

#!/bin/bash
set -euo pipefail

git ls-files ci-images/Dockerfile.lint
echo '---'
cat -n ci-images/Dockerfile.lint | sed -n '1,40p'

Repository: openshift/assisted-service

Length of output: 389


Pin the installer script to an immutable tag.

Line 3 pulls install.sh from main, so the build can pick up changes outside this Dockerfile. Use the v2.12.2 tag in the script URL too.

Proposed fix
-RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/main/install.sh | sh -s -- -b /usr/bin v2.12.2
+RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/v2.12.2/install.sh | sh -s -- -b /usr/bin v2.12.2
📝 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
RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/main/install.sh | sh -s -- -b /usr/bin v2.12.2
RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/v2.12.2/install.sh | sh -s -- -b /usr/bin v2.12.2
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci-images/Dockerfile.lint` at line 3, The golangci-lint installer in
Dockerfile.lint is pulling install.sh from the mutable main branch, which can
change independently of this image build. Update the curl source used by the RUN
step to reference the same immutable v2.12.2 tag as the installed version so the
build is reproducible; keep the existing install.sh invocation and pin the
script URL consistently.

FROM registry.access.redhat.com/ubi9/go-toolset:1.26 AS golang

RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/main/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.11.4
RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/main/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.12.2

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

git ls-files | rg '(^|/)Dockerfile\.assisted-service-build$'
printf '\n--- file with line numbers ---\n'
cat -n Dockerfile.assisted-service-build | sed -n '1,40p'

Repository: openshift/assisted-service

Length of output: 2109


Pin the installer script URL to an immutable ref.

Line 3 pulls install.sh from main, so the build can pick up upstream changes unexpectedly. Use the release tag matching v2.12.2 or vendor the script instead.

Proposed fix
-RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/main/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.12.2
+RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/v2.12.2/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.12.2
📝 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
RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/main/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.12.2
RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/v2.12.2/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.12.2
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Dockerfile.assisted-service-build` at line 3, The golangci-lint installer in
Dockerfile.assisted-service-build is fetched from a moving branch reference,
which can change unexpectedly; update the curl source in the RUN step to use an
immutable release/tagged ref matching v2.12.2 or vendor the installer script
locally. Keep the fix in the same Dockerfile line that invokes install.sh so the
build always uses a pinned, reproducible installer version.

@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 44.33%. Comparing base (08dd374) to head (8a7fce1).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
internal/common/common.go 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10509      +/-   ##
==========================================
- Coverage   44.33%   44.33%   -0.01%     
==========================================
  Files         423      423              
  Lines       73512    73512              
==========================================
- Hits        32595    32594       -1     
- Misses      37985    37986       +1     
  Partials     2932     2932              
Files with missing lines Coverage Δ
pkg/secretdump/struct.go 100.00% <100.00%> (ø)
internal/common/common.go 37.64% <0.00%> (ø)

... and 3 files with indirect coverage changes

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

@pastequo

Copy link
Copy Markdown
Contributor Author

/retest

@pastequo

Copy link
Copy Markdown
Contributor Author

/test e2e-agent-compact-ipv4-iso-no-registry

@openshift-ci

openshift-ci Bot commented Jun 24, 2026

Copy link
Copy Markdown

@pastequo: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-agent-compact-ipv4-iso-no-registry 8a7fce1 link false /test e2e-agent-compact-ipv4-iso-no-registry

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. kind/dependency-change Categorizes issue or PR as related to changing dependencies size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants