Skip to content

Conversation

@andrej1991
Copy link

No description provided.

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

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

Details

In 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.

@coderabbitai
Copy link

coderabbitai bot commented Jan 27, 2026

Walkthrough

Adds a GolangCI-Lint configuration, upgrades Go toolchain and builder image to 1.25, bumps golangci-lint installer version, adjusts Makefile test selection and controller-tools version, and updates a CRD controller-gen annotation. No API or exported symbols changed.

Changes

Cohort / File(s) Summary
GolangCI-Lint config
.golangci.yml
New GolangCI-Lint v2 configuration: run parameters (version, concurrency, timeout, tests), enabled linters (staticcheck, unused, govet, gocyclo, gosec, unconvert), per-linter settings, formatters (gofmt, goimports), generated/third_party/examples path exclusions, and issue formatting.
Go tool & builder image
go.mod, Dockerfile
go.mod: go directive updated 1.24.01.25.0, toolchain go1.24.5go1.25.5. Dockerfile: builder base image tag updated to registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.25-openshift-4.21.
GolangCI installer script
hack/golangci-lint.sh
Bumped VERSION for golangci-lint from 1.55.22.8.0; install and execution logic unchanged.
Makefile
Makefile
Added TEST variable and updated test target to run go test $(TEST) ... limiting tests to packages with test files; updated CONTROLLER_TOOLS_VERSION v0.16.2v0.17.0; minor formatting tweak.
CRD metadata
config/crd/bases/..._imageclusterinstalls.yaml
controller-gen.kubebuilder.io/version annotation updated from v0.16.2v0.17.0 in CRD metadata only.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

✨ Finishing touches
  • 📝 Generate docstrings

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

@openshift-ci openshift-ci bot requested review from javipolo and mresvanis January 27, 2026 10:18
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: 1

🤖 Fix all issues with AI agents
In @.golangci.yml:
- Around line 29-35: The gosec exclusions under the gosec: excludes block (G107,
G115, G401, G402, G501) are too broad; narrow them to only vendor code or
document the global exception: update the gosec configuration so each rule
exclusion is scoped to vendor paths (e.g., apply exclusions only for files under
vendor/) or add a top-of-file justification comment explaining why a global
exclusion is required and which vendor packages triggered it, referencing the
rules G402, G401, G501, and G107 and the gosec: excludes block so reviewers can
verify the scope.

Comment on lines +29 to +35
gosec:
excludes:
- G107
- G115
- G401
- G402
- G501
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== Checking for InsecureSkipVerify usage (G402) ==="
rg -n "InsecureSkipVerify" --type go -C 2 || echo "No matches found"

echo -e "\n=== Checking for MD5 usage (G401/G501) ==="
rg -n 'crypto/md5|md5\.New|md5\.Sum' --type go -C 2 || echo "No matches found"

echo -e "\n=== Checking for URL taint/HTTP request patterns (G107) ==="
rg -n 'http\.Get|http\.Post|url\.Parse' --type go -C 1 | head -30 || echo "No matches found"

echo -e "\n=== Full .golangci.yml content ==="
cat -n .golangci.yml

Repository: openshift/image-based-install-operator

Length of output: 36258


Gosec exclusions are overly broad; consider narrowing to vendor paths or documenting their necessity.

The exclusions disable important security checks globally:

  • G402 (TLS InsecureSkipVerify): Used extensively in vendor code (controller-runtime, k8s.io, vmware govmomi) for test harnesses and internal webhooks—all with legitimate justifications but flagged by global exclusion
  • G401/G501 (MD5 crypto): Appears only in vendor code for non-security uses (SSH fingerprinting, UUID v3 generation, AWS S3 checksums, database authentication)
  • G107 (URL taint): No actual SSRF risks in project code; appears to be vendor-only

Since the problematic patterns are confined to vendor dependencies, either:

  1. Scope these exclusions to vendor/ paths in the gosec settings, or
  2. If global exclusion is required, document why at the top of this file

This prevents the configuration from inadvertently silencing these checks in your own code.

🤖 Prompt for AI Agents
In @.golangci.yml around lines 29 - 35, The gosec exclusions under the gosec:
excludes block (G107, G115, G401, G402, G501) are too broad; narrow them to only
vendor code or document the global exception: update the gosec configuration so
each rule exclusion is scoped to vendor paths (e.g., apply exclusions only for
files under vendor/) or add a top-of-file justification comment explaining why a
global exclusion is required and which vendor packages triggered it, referencing
the rules G402, G401, G501, and G107 and the gosec: excludes block so reviewers
can verify the scope.

@andrej1991 andrej1991 force-pushed the bump-go-main branch 6 times, most recently from d8af43c to d878a2f Compare January 27, 2026 14:43
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 27, 2026
@openshift-ci
Copy link

openshift-ci bot commented Jan 27, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrej1991, carbonin

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 Jan 27, 2026
@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 392da90 and 2 for PR HEAD 5690e78 in total

@andrej1991
Copy link
Author

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 27, 2026
@gamli75
Copy link
Contributor

gamli75 commented Jan 28, 2026

/retest-required

@openshift-ci
Copy link

openshift-ci bot commented Jan 28, 2026

@andrej1991: The following tests 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-ibio 5690e78 link true /test e2e-ibio
ci/prow/ibio-reinstall 5690e78 link false /test ibio-reinstall

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. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants