Pin and verify SHAs for CI image dependencies#167
Pin and verify SHAs for CI image dependencies#167ay901246 wants to merge 2 commits intocloudfoundry:windows-2019from
Conversation
Made-with: Cursor
Made-with: Cursor
|
@coderabbitai review |
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
|
There might be some extremely subtle edges I'm not seeing here, but this lgtm. I'm going to hold off an official thumbs up to give other folks a chance to review as well. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis change introduces automated binary dependency management with SHA-256 integrity verification. A new GitHub Actions workflow executes a bash script that automatically checks for updated releases of six tools (bosh-cli, meta4-cli, yq, ruby-install, golangci-lint, govc) and updates their download URLs and SHA-256 checksums in a new JSON manifest file. The Docker image build process is updated to use this manifest instead of dynamic GitHub API queries, and the Dockerfile now validates downloaded artifacts against their checksums before installation. A pull request is opened automatically when dependencies are updated. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/bump-binaries.yml:
- Around line 31-32: Remove the mock azstemcell file after the Docker build and
restrict the PR action to only include the intended file: add a cleanup step
that runs rm -f ci/docker/azstemcell (to remove the file created by the touch
ci/docker/azstemcell) immediately after the Docker build step, and update the PR
creation action to include add-paths: ci/docker/dependencies.json so only
dependencies.json is added to the PR; locate the touch call in
.github/workflows/bump-binaries.yml and add the rm -f cleanup and the add-paths
setting in the PR step that references the created artifacts.
In `@ci/scripts/bump-binaries.sh`:
- Around line 75-77: The jq invocation should pass url/sha (and the dynamic key)
via --arg instead of interpolating shell variables into the program; update the
invocation that currently uses jq ".${key}.url = \"${url}\" | .${key}.sha256 =
\"${sha}\"" to use --arg key "$key" --arg url "$url" --arg sha "$sha" and change
the program to use the jq variables (e.g. .[$key].url = $url | .[$key].sha256 =
$sha) writing to TMP_FILE and then mv TMP_FILE to DEPS_FILE as before.
- Around line 51-54: The current extraction uses head which can cause jq to
receive a SIGPIPE under set -o pipefail; replace the pipeline that uses head
with a jq-only selection that returns the first matching asset inside jq.
Specifically, update the assignment to url (where release_json and pattern are
used) to use a jq expression such as: ".assets | map(select(.name |
test(\"${pattern}\"))) | .[0].browser_download_url" (and keep -r) so the first
match is selected inside jq and no external head/cutting of the pipe is needed.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a156fd5d-9cc0-4372-8c54-69f7a6e4e9b7
📒 Files selected for processing (5)
.github/workflows/bump-binaries.ymlci/docker/Dockerfileci/docker/dependencies.jsonci/scripts/bump-binaries.shci/tasks/build-docker-args/run.sh
| # Mock the azstemcell binary for the docker build context | ||
| touch ci/docker/azstemcell |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether the workflow's mock Docker context file would be excluded from commits.
set -euo pipefail
echo "Tracked status:"
git ls-files --stage -- ci/docker/azstemcell || true
echo "Ignore status:"
git check-ignore -v ci/docker/azstemcell || true
echo "Relevant ignore rules:"
rg -n -C2 '(^|/)azstemcell$|ci/docker/azstemcell' .gitignore .github ci || trueRepository: cloudfoundry/bosh-windows-stemcell-builder
Length of output: 1347
🏁 Script executed:
cat -n .github/workflows/bump-binaries.yml | sed -n '25,85p'Repository: cloudfoundry/bosh-windows-stemcell-builder
Length of output: 3314
Remove the mock azstemcell file before creating the PR.
Line 32 creates ci/docker/azstemcell for the Docker build, but it remains as an untracked, unignored file when the PR creation step executes. Without cleanup or path restriction, it will be committed to the pull request alongside the intended dependencies.json changes.
Add rm -f ci/docker/azstemcell after the Docker build step (after line 67) and add add-paths: ci/docker/dependencies.json to the PR action to ensure only the intended file is committed.
Proposed fix
docker build \
--build-arg BOSH_CLI_URL="$BOSH_CLI_URL" \
--build-arg BOSH_CLI_SHA256="$BOSH_CLI_SHA" \
--build-arg META4_CLI_URL="$META4_CLI_URL" \
--build-arg META4_CLI_SHA256="$META4_CLI_SHA" \
--build-arg YQ_CLI_URL="$YQ_CLI_URL" \
--build-arg YQ_CLI_SHA256="$YQ_CLI_SHA" \
--build-arg RUBY_INSTALL_URL="$RUBY_INSTALL_URL" \
--build-arg RUBY_INSTALL_SHA256="$RUBY_INSTALL_SHA" \
--build-arg GOLANGCI_LINT_INSTALL_URL="$GOLANGCI_LINT_URL" \
--build-arg GOLANGCI_LINT_INSTALL_SHA256="$GOLANGCI_LINT_SHA" \
--build-arg GOVC_INSTALL_URL="$GOVC_URL" \
--build-arg GOVC_INSTALL_SHA256="$GOVC_SHA" \
--build-arg RUBY_VERSION="$RUBY_VERSION" \
--build-arg GEM_HOME="$GEM_HOME" \
-f ci/docker/Dockerfile ci/docker/
+
+ rm -f ci/docker/azstemcell
- name: Create Pull Request
uses: peter-evans/create-pull-request@c5a7806660adbe173f04e3e038b0ccdcd758773c # v6.1.0
with:
token: ${{ secrets.GITHUB_TOKEN }}
+ add-paths: ci/docker/dependencies.json
commit-message: "Update binary dependencies in dependencies.json"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/bump-binaries.yml around lines 31 - 32, Remove the mock
azstemcell file after the Docker build and restrict the PR action to only
include the intended file: add a cleanup step that runs rm -f
ci/docker/azstemcell (to remove the file created by the touch
ci/docker/azstemcell) immediately after the Docker build step, and update the PR
creation action to include add-paths: ci/docker/dependencies.json so only
dependencies.json is added to the PR; locate the touch call in
.github/workflows/bump-binaries.yml and add the rm -f cleanup and the add-paths
setting in the PR step that references the created artifacts.
| # 2. Extract the URL | ||
| local url | ||
| url=$(echo "${release_json}" | jq -r ".assets[] | select(.name | test(\"${pattern}\")) | .browser_download_url" | head -n 1) | ||
|
|
There was a problem hiding this comment.
Avoid head under pipefail for asset selection.
If multiple assets match, head -n 1 can close the pipe early and make jq fail with SIGPIPE under set -o pipefail, aborting the bump workflow. Select the first match inside jq instead.
Proposed fix
- url=$(echo "${release_json}" | jq -r ".assets[] | select(.name | test(\"${pattern}\")) | .browser_download_url" | head -n 1)
+ url=$(echo "${release_json}" | jq -r --arg pattern "${pattern}" '
+ first(.assets[] | select(.name | test($pattern)) | .browser_download_url) // empty
+ ')📝 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.
| # 2. Extract the URL | |
| local url | |
| url=$(echo "${release_json}" | jq -r ".assets[] | select(.name | test(\"${pattern}\")) | .browser_download_url" | head -n 1) | |
| # 2. Extract the URL | |
| local url | |
| url=$(echo "${release_json}" | jq -r --arg pattern "${pattern}" ' | |
| first(.assets[] | select(.name | test($pattern)) | .browser_download_url) // empty | |
| ') | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ci/scripts/bump-binaries.sh` around lines 51 - 54, The current extraction
uses head which can cause jq to receive a SIGPIPE under set -o pipefail; replace
the pipeline that uses head with a jq-only selection that returns the first
matching asset inside jq. Specifically, update the assignment to url (where
release_json and pattern are used) to use a jq expression such as: ".assets |
map(select(.name | test(\"${pattern}\"))) | .[0].browser_download_url" (and keep
-r) so the first match is selected inside jq and no external head/cutting of the
pipe is needed.
| echo " Updating ${DEPS_FILE}..." | ||
| jq ".${key}.url = \"${url}\" | .${key}.sha256 = \"${sha}\"" "${DEPS_FILE}" > "${TMP_FILE}" | ||
| mv "${TMP_FILE}" "${DEPS_FILE}" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Pass updated values to jq with --arg.
Directly interpolating ${url} into the jq program makes the update fragile if GitHub asset URLs ever contain quoting or escape characters.
Proposed fix
- jq ".${key}.url = \"${url}\" | .${key}.sha256 = \"${sha}\"" "${DEPS_FILE}" > "${TMP_FILE}"
+ jq --arg key "${key}" --arg url "${url}" --arg sha "${sha}" '
+ .[$key].url = $url | .[$key].sha256 = $sha
+ ' "${DEPS_FILE}" > "${TMP_FILE}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ci/scripts/bump-binaries.sh` around lines 75 - 77, The jq invocation should
pass url/sha (and the dynamic key) via --arg instead of interpolating shell
variables into the program; update the invocation that currently uses jq
".${key}.url = \"${url}\" | .${key}.sha256 = \"${sha}\"" to use --arg key "$key"
--arg url "$url" --arg sha "$sha" and change the program to use the jq variables
(e.g. .[$key].url = $url | .[$key].sha256 = $sha) writing to TMP_FILE and then
mv TMP_FILE to DEPS_FILE as before.
github-project-automation
Bot
moved this from Inbox
to Waiting for Changes | Open for Contribution
in Foundational Infrastructure Working Group
3 participants
Motivation
Currently, the CI pipeline dynamically fetches the "latest" release binaries (BOSH CLI, meta4, yq, etc.) via the GitHub API every time the Docker image is built. This approach has a few drawbacks:
mainCI pipeline.Key Changes
This PR overhauls how we handle CI image dependencies by pinning them explicitly and automating their updates:
dependencies.json: All CI binary URLs and SHA-256 hashes are now hardcoded in a single source of truth.Dockerfileto calculate and verify thesha256sumof every downloaded binary before installation. The build will now fail fast if a checksum mismatches.bump-binaries.yml) that runs a bash script to query the latest releases, calculate their new SHAs, and automatically open a PR with the updateddependencies.json.docker buildstep. It will only open a PR if the new URLs and hashes successfully build the Docker image, preventing broken PRs from being submitted.Testing Performed
ci/scripts/bump-binaries.shsuccessfully updates the JSON.Example of it working on my fork: https://github.com/ay901246/bosh-windows-stemcell-builder/actions/workflows/bump-binaries.yml