🐛 Fix lint scripts to support remote config download fallback#201
Merged
openshift-merge-bot[bot] merged 2 commits intoopen-cluster-management-io:mainfrom Feb 25, 2026
Conversation
When lint scripts are used in other repos without golangci-v2.yml alongside them, the config copy fails. Add a remote download fallback from the sdk-go GitHub repository when the local config file is not found. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: xuezhaojun <zxue@redhat.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: xuezhaojun The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughAdds REMOTE_CONFIG_URL and updates CI lint scripts to prefer a local golangci-v2.yml but fall back to downloading it from a remote GitHub URL; download failures now emit a warning and return success instead of hard-failing. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
ci/lint/install-golangci-lint.sh (1)
24-24: Same mutable remote-ref concern applies here.Please apply the same immutable ref pinning approach as in
ci/lint/run-lint.sh.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci/lint/install-golangci-lint.sh` at line 24, The REMOTE_CONFIG_URL currently points to a mutable branch URL; update REMOTE_CONFIG_URL to use an immutable ref (tag or commit hash) like you did in run-lint.sh so the script fetches a fixed version of the remote lint config; locate the REMOTE_CONFIG_URL variable in this file and replace the branch-based URL with the pinned raw URL that includes the chosen tag or commit SHA.
🧹 Nitpick comments (1)
ci/lint/run-lint.sh (1)
150-151: Harden remote download with retries/timeouts and atomic write.Direct
curl -o "$CONFIG_PATH"can leave a partial config and poison cache on flaky networks.Suggested change
- echo "Downloading config: golangci-v2.yml" - curl -sSfL "${REMOTE_CONFIG_URL}/golangci-v2.yml" -o "$CONFIG_PATH" + echo "Downloading config: golangci-v2.yml" + tmp_config="$(mktemp "${CONFIG_PATH}.tmp.XXXXXX")" + if ! curl --retry 3 --retry-delay 2 --connect-timeout 10 --max-time 60 -sSfL \ + "${REMOTE_CONFIG_URL}/golangci-v2.yml" -o "$tmp_config"; then + rm -f "$tmp_config" + echo "Error: failed to download ${REMOTE_CONFIG_URL}/golangci-v2.yml" >&2 + exit 1 + fi + mv "$tmp_config" "$CONFIG_PATH"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci/lint/run-lint.sh` around lines 150 - 151, Replace the direct curl write to CONFIG_PATH with a hardened download: use curl retry and timeout flags (e.g. --retry, --retry-connrefused, --retry-delay, --connect-timeout, --max-time, -fS) targeting REMOTE_CONFIG_URL/golangci-v2.yml, download to a temporary file created via mktemp, verify curl exit status, and atomically move the temp file into CONFIG_PATH (mv) only on success so partial downloads never overwrite the cached config; update the echo/curl block around REMOTE_CONFIG_URL and CONFIG_PATH 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 `@ci/lint/install-golangci-lint.sh`:
- Around line 239-241: The message in the download-failure branch of
copy_config() is misleading: it says "Warning:" but returns 1 which under set
-eo pipefail makes the script exit; either make the failure non-fatal or make
the message reflect a hard failure. Fix by updating copy_config() so that if you
want a non-fatal fallback it echoes a true warning and does not return non-zero
(remove or change the return 1), or if it must abort then change the message
prefix to "Error:" (and keep return 1) so the log matches the behavior; update
the echo lines that reference REMOTE_CONFIG_URL/CONFIG_FILE and LOCAL_CONFIG
accordingly and ensure callers of copy_config() (the calls at lines where
copy_config() is invoked) rely on the chosen behavior.
In `@ci/lint/run-lint.sh`:
- Line 23: Update the REMOTE_CONFIG_URL in ci/lint/run-lint.sh (variable
REMOTE_CONFIG_URL) to reference an immutable ref (commit SHA or release tag)
instead of the mutable branch "main"; also update the matching URL in
ci/lint/install-golangci-lint.sh to the same pinned tag/SHA so both scripts
fetch a deterministic config. Replace the "main" path component with the chosen
tag or SHA and verify the URL resolves before committing.
---
Duplicate comments:
In `@ci/lint/install-golangci-lint.sh`:
- Line 24: The REMOTE_CONFIG_URL currently points to a mutable branch URL;
update REMOTE_CONFIG_URL to use an immutable ref (tag or commit hash) like you
did in run-lint.sh so the script fetches a fixed version of the remote lint
config; locate the REMOTE_CONFIG_URL variable in this file and replace the
branch-based URL with the pinned raw URL that includes the chosen tag or commit
SHA.
---
Nitpick comments:
In `@ci/lint/run-lint.sh`:
- Around line 150-151: Replace the direct curl write to CONFIG_PATH with a
hardened download: use curl retry and timeout flags (e.g. --retry,
--retry-connrefused, --retry-delay, --connect-timeout, --max-time, -fS)
targeting REMOTE_CONFIG_URL/golangci-v2.yml, download to a temporary file
created via mktemp, verify curl exit status, and atomically move the temp file
into CONFIG_PATH (mv) only on success so partial downloads never overwrite the
cached config; update the echo/curl block around REMOTE_CONFIG_URL and
CONFIG_PATH accordingly.
zhujian7
reviewed
Feb 25, 2026
ci/lint/install-golangci-lint.sh
Outdated
|
|
||
| # Resolve the directory where this script is located | ||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| REMOTE_CONFIG_URL="https://raw.githubusercontent.com/stolostron/sdk-go/main/ci/lint" |
Member
There was a problem hiding this comment.
I think we should avoid using stolostron?
Member
Author
There was a problem hiding this comment.
Good point! Updated. Please take another look, thanks!
- Use open-cluster-management-io/sdk-go instead of stolostron/sdk-go for remote config download URLs - Change return 1 to return 0 in copy_config() so the warning is truly non-fatal under set -eo pipefail Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: xuezhaojun <zxue@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ci/lint/install-golangci-lint.sh (1)
24-24:REMOTE_CONFIG_URLis pinned tomain— consider a tagged/versioned ref.Repos that copy only the scripts (the primary use-case for this fallback) will always pull whatever
golangci-v2.ymlis currently onmain. A breaking change to the config file will silently affect those repos the next time they run the scripts.🔧 Optional: use a versioned ref
-REMOTE_CONFIG_URL="https://raw.githubusercontent.com/open-cluster-management-io/sdk-go/main/ci/lint" +# Pin to a release tag so downstream repos get a stable config. +REMOTE_CONFIG_URL="https://raw.githubusercontent.com/open-cluster-management-io/sdk-go/v0.x.y/ci/lint"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci/lint/install-golangci-lint.sh` at line 24, REMOTE_CONFIG_URL is pinned to the main branch; change it to use a versioned ref so downstream repos don't pick up breaking config changes: introduce a REMOTE_CONFIG_REF (env-overridable) defaulting to a stable tag (e.g., "v1.0.0") and set REMOTE_CONFIG_URL to "https://raw.githubusercontent.com/open-cluster-management-io/sdk-go/${REMOTE_CONFIG_REF}/ci/lint" (update the REMOTE_CONFIG_URL assignment and any uses of REMOTE_CONFIG_URL accordingly) so callers can override REMOTE_CONFIG_REF if they need a different release.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ci/lint/install-golangci-lint.sh`:
- Around line 238-242: The curl download can create a zero-byte or partial
LOCAL_CONFIG when using -o directly; change the logic so you first download to a
temporary file (e.g., "${LOCAL_CONFIG}.tmp" or use mktemp), verify curl exits
successfully and the temp file is non-empty, then atomically move the temp file
to LOCAL_CONFIG (mv) and clean up on failure; use the existing REMOTE_CONFIG_URL
and CONFIG_FILE variables for the download target and ensure proper permissions
on the final LOCAL_CONFIG.
---
Nitpick comments:
In `@ci/lint/install-golangci-lint.sh`:
- Line 24: REMOTE_CONFIG_URL is pinned to the main branch; change it to use a
versioned ref so downstream repos don't pick up breaking config changes:
introduce a REMOTE_CONFIG_REF (env-overridable) defaulting to a stable tag
(e.g., "v1.0.0") and set REMOTE_CONFIG_URL to
"https://raw.githubusercontent.com/open-cluster-management-io/sdk-go/${REMOTE_CONFIG_REF}/ci/lint"
(update the REMOTE_CONFIG_URL assignment and any uses of REMOTE_CONFIG_URL
accordingly) so callers can override REMOTE_CONFIG_REF if they need a different
release.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ci/lint/install-golangci-lint.shci/lint/run-lint.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- ci/lint/run-lint.sh
Member
|
/lgtm |
openshift-merge-bot
bot
merged commit 284a1f4
into
open-cluster-management-io:main
13 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
2 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.
Summary
golangci-v2.ymlconfig in bothrun-lint.shandinstall-golangci-lint.shProblem
When other repos use the lint scripts from sdk-go, they may not have
golangci-v2.ymlalongside the scripts, causing:Fix
Both scripts now follow a local-first, remote-fallback strategy:
golangci-v2.ymlexists locally alongside the script → use local copyhttps://raw.githubusercontent.com/open-cluster-management-io/sdk-go/main/ci/lint/golangci-v2.ymlThe download failure in
install-golangci-lint.shis non-fatal (returns 0 with a warning) to avoid script exit underset -eo pipefail.Test plan
./ci/lint/run-lint.shin sdk-go — should use local config (no download)golangci-v2.yml) to another repo and run — should download from GitHubmake verifyin sdk-go to confirm no regressions🤖 Generated with Claude Code
Summary by CodeRabbit