[redhat-chai-bot]

Summary

• Use checked, NUL-safe regular-file selection and bounded formatter invocations to avoid argument-list overflow.
• Propagate selection and formatter execution failures and fail when formatting differences are reported.
• Exclude vendor and _output directory subtrees while retaining first-party and generated-code checks.
• Add standard-library regression tests and document the verification policy in HACKING.md.

Validation

• Passed: make test, make build, focused regression and race tests, argument-limit stress coverage, shell syntax, profile comparison, generated-CRD verification, code review, and diff whitespace checks.
• Stress coverage processed 10,700 temporary paths exceeding the argument-size limit without overflow.
• The actual make verify consumer now returns nonzero for the existing generated-file formatting finding rather than hiding it.

Outstanding checks

• Draft because make verify still fails on unchanged tests-extension/test/qe/bindata.go formatting. That file remains checked and was not modified or exempted.
• CI-only dependency regeneration was not run.
• This PR changes only hack/verify-gofmt.sh, hack/verify_gofmt_test.go, and HACKING.md. It does not include the separate memory-request change.

---

AI-generated. Review for accuracy.

@pedjak requested from Slack

Summary by CodeRabbit

• Documentation
  • Added instructions for checking Go formatting, choosing a formatter, and handling reported differences, including in generated files. Existing unit and end-to-end test guidance remains available.
• Chores
  • Formatting verification now checks first-party Go files, skips vendor and build-output directories, and reports formatting or execution failures.
• Tests
  • Added coverage for formatting checks, failure handling, file selection, and temporary-file cleanup.

[openshift-merge-bot]

Pipeline controller notification

This PR uses the pipeline controller for second-stage tests. Selection and triggering follow the repository configuration.

Use /test ? to list jobs and /pipeline help for commands.

[openshift-ci]

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]

Walkthrough

The pull request updates Go formatting verification to use a configurable formatter, exclude vendor and _output, pass file paths in batches, and report failures. It adds tests for selection, formatting results, failure handling, argument limits, and Make integration. It also documents the verification workflow.

Changes

Go Formatting Verification

Layer / File(s)	Summary
Update and document formatting verification hack/verify-gofmt.sh, HACKING.md	The script uses GOFMT or defaults to gofmt, excludes vendor and _output, and passes null-delimited paths in batches. The documentation describes how to run verification and handle findings.
Test formatting verification hack/verify_gofmt_test.go	Tests cover clean, unformatted, and invalid Go files; formatter and file-selection failures; empty selections; unusual filenames; excluded paths; argument limits; and Make integration.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Suggested reviewers: ironcladlou

Merge Risk: Minimal · up to d7da6

This change has no established new merge-blocking defect. The existing generated-file formatting finding still prevents verification from passing and needs separate cleanup.

Pre-merge checks: 14 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 14 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly describes the main changes: improved formatting-verification argument handling and failure propagation.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names		PASS: The pull request adds standard-library Go tests, not Ginkgo tests. All Test... names and t.Run names are fixed literals or fixed table entries such as clean, formatter, and predefined fi…
Test Structure And Quality		The pull request adds standard-library Go tests in hack/verify_gofmt_test.go using Test..., t.Run, and t.Cleanup. It adds no Ginkgo It blocks, cluster operations, Eventually calls, or `Con…
Microshift Test Compatibility		The pull request adds standard-library Go tests in hack/verify_gofmt_test.go using testing.T and t.Run. It adds no Ginkgo e2e tests, MicroShift-sensitive OpenShift API references, namespaces, or…
Single Node Openshift (Sno) Test Compatibility		PASS: The pull request changes only HACKING.md, hack/verify-gofmt.sh, and hack/verify_gofmt_test.go. The added tests use Go's standard testing package and shell fixtures. They add no Ginkgo e2e tests …
Topology-Aware Scheduling Compatibility		The pull request changes only HACKING.md, hack/verify-gofmt.sh, and hack/verify_gofmt_test.go. These files document and test Go formatting verification. The diff adds no deployment manifests, operator…
Ote Binary Stdout Contract		The pull request changes only HACKING.md, hack/verify-gofmt.sh, and the new package hack test. It does not modify tests-extension or an OTE binary entry point. The only added stdout write, fmt.Println…
Ipv6 And Disconnected Network Test Compatibility		PASS. The pull request changes HACKING.md, hack/verify-gofmt.sh, and the standard-library test file hack/verify_gofmt_test.go. The added tests use package hack and testing.T; they add no Ginkgo It, De…
No-Weak-Crypto		The pull request changes only Go formatting verification code, tests, and documentation. The reviewed patch contains no MD5, SHA1, DES, 3DES, RC4, Blowfish, ECB, custom cryptography, or secret/token c…
Container-Privileges		The pull request changes only HACKING.md, hack/verify-gofmt.sh, and hack/verify_gofmt_test.go. The diff adds no container or Kubernetes manifests and contains none of privileged, hostPID, hostNetwork,…
No-Sensitive-Data-In-Logs		The pull request adds only generic verification errors and formatter-reported Go file paths. It does not log passwords, tokens, API keys, PII, session IDs, hostnames, or customer data.

Full details: Stable And Deterministic Test Names

Explanation

PASS: The pull request adds standard-library Go tests, not Ginkgo tests. All Test... names and t.Run names are fixed literals or fixed table entries such as clean, formatter, and predefined fixture paths. No test title contains timestamps, random identifiers, node or namespace names, IP addresses, or legacy OpenShift metadata tokens.

Full details: Test Structure And Quality

Explanation

The pull request adds standard-library Go tests in hack/verify_gofmt_test.go using Test..., t.Run, and t.Cleanup. It adds no Ginkgo It blocks, cluster operations, Eventually calls, or Consistently calls. The Ginkgo-specific quality check is therefore not applicable.

Full details: Microshift Test Compatibility

Explanation

The pull request adds standard-library Go tests in hack/verify_gofmt_test.go using testing.T and t.Run. It adds no Ginkgo e2e tests, MicroShift-sensitive OpenShift API references, namespaces, or unsupported feature assumptions. HACKING.md only adds documentation.

Full details: Single Node Openshift (Sno) Test Compatibility

Explanation

PASS: The pull request changes only HACKING.md, hack/verify-gofmt.sh, and hack/verify_gofmt_test.go. The added tests use Go's standard testing package and shell fixtures. They add no Ginkgo e2e tests and make no multi-node or HA assumptions.

Full details: Topology-Aware Scheduling Compatibility

Explanation

The pull request changes only HACKING.md, hack/verify-gofmt.sh, and hack/verify_gofmt_test.go. These files document and test Go formatting verification. The diff adds no deployment manifests, operator code, controllers, replicas, affinities, topology spread constraints, node selectors, tolerations, or PDBs. The topology-aware scheduling check is therefore not applicable.

Full details: Ote Binary Stdout Contract

Explanation

The pull request changes only HACKING.md, hack/verify-gofmt.sh, and the new package hack test. It does not modify tests-extension or an OTE binary entry point. The only added stdout write, fmt.Println at hack/verify_gofmt_test.go:117, is test-fixture code that simulates formatter output, not main, init, TestMain, or suite setup.

Full details: Ipv6 And Disconnected Network Test Compatibility

Explanation

PASS. The pull request changes HACKING.md, hack/verify-gofmt.sh, and the standard-library test file hack/verify_gofmt_test.go. The added tests use package hack and testing.T; they add no Ginkgo It, Describe, Context, or When e2e tests. No IPv4 or external-connectivity assessment is required.

Full details: No-Weak-Crypto

Explanation

The pull request changes only Go formatting verification code, tests, and documentation. The reviewed patch contains no MD5, SHA1, DES, 3DES, RC4, Blowfish, ECB, custom cryptography, or secret/token comparisons.

Full details: Container-Privileges

Explanation

The pull request changes only HACKING.md, hack/verify-gofmt.sh, and hack/verify_gofmt_test.go. The diff adds no container or Kubernetes manifests and contains none of privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, allowPrivilegeEscalation, or securityContext settings. It does not introduce a root-running container.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 12 functions across 2 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign alebedev87 for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[coderabbitai]

Additional comments: 2

hack/verify_gofmt_test.go-296-296: 1

296-296: Minor · Maintainability & Code Quality · Quick win

Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Use exec.CommandContext for the getconf call.

golangci-lint (noctx) rejects exec.Command here. The other process calls in this file use a context. This call has no timeout or cancellation. Use t.Context() (the module declares Go 1.26.0).

Proposed fix

-	output, err := exec.Command("getconf", "ARG_MAX").Output()
+	output, err := exec.CommandContext(t.Context(), "getconf", "ARG_MAX").Output()

Source: Linters/SAST tools

hack/verify_gofmt_test.go: 1

48-48: Trivial · Maintainability & Code Quality · Low value

Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Replace the deprecated runtime.GOROOT().

staticcheck reports SA1019 here. The compiled-in GOROOT can differ from the active toolchain's root. The test could then use the wrong gofmt. Resolve the root with go env GOROOT through exec.CommandContext(t.Context(), "go", "env", "GOROOT"). Do this in a small helper, and cache the result with sync.OnceValue so parallel fixtures do not each spawn go.

Source: Linters/SAST tools

---

Review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Central YAML; Path: .coderabbit.yaml
• Review profile: CHILL
• Plan: Enterprise
• Run ID: ba6e617f-cd94-45a5-a7c9-1ecb51c73c0c

Commits

Reviewing files that changed from the base of the PR and between 9d18bdd and d7da66f.

Files selected for processing: 3

• HACKING.md
• hack/verify-gofmt.sh
• hack/verify_gofmt_test.go

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 4 remain after this review.