Repository navigation
Various fixes to unblock presubmits & COPR nightlies - #229
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR updates RPM build tooling and downstream spec filtering, preserves detected repository versions, ensures container policy placement, and adds optional authenticated GitHub release lookup with explicit failure handling to quick-start. ChangesRPM packaging updates
Quick-start release lookup
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant QuickStartAction
participant QuickstartScript
participant GitHubReleasesAPI
QuickStartAction->>QuickstartScript: Pass GITHUB_TOKEN, IMAGE, and TAG
QuickstartScript->>GitHubReleasesAPI: Request releases/latest with timeout and optional Authorization
GitHubReleasesAPI-->>QuickstartScript: Release response
QuickstartScript->>QuickstartScript: Parse and validate tag_name
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packaging/rpm.Containerfile`:
- Line 3: Update the RPM build documentation describing the make rpm target to
reference the CentOS Stream 10 builder instead of Stream 9. Review the Stream 10
release/test instructions in docs/build.md and ensure they explicitly cover
supported CentOS 9 deployments where required, without changing the packaging
configuration.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6d7583d9-7131-42c9-80b9-0aa06924a6d0
📒 Files selected for processing (2)
packaging/rpm.Containerfilesrc/image/modify-spec.py
ee4ebad to
006fc54
Compare
f481189 to
995c662
Compare
ec108aa to
f9fa0ac
Compare
f9fa0ac to
ad7c447
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packaging/rpm.Containerfile (1)
3-3: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winApply the intended Stream 10 base-image change.
packaging/rpm.Containerfilestill usesquay.io/centos/centos:stream9, while the README lists CentOS 10 RPM support using the Stream 10 image. Update this stage toquay.io/centos/centos:stream10, and update relatedstream9references if this is a Stream 10 migration.🤖 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 `@packaging/rpm.Containerfile` at line 3, Update the base image in the RPM container stage from CentOS Stream 9 to quay.io/centos/centos:stream10, and revise any related stream9 references in the same migration scope to consistently target Stream 10.
🧹 Nitpick comments (1)
packaging/rpm.Containerfile (1)
14-14: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winFail closed for unsupported architectures.
Every
uname -mvalue other thanx86_64is treated asarm64. Use explicitx86_64andaarch64cases, and exit with an error for anything else instead of downloading an incompatible archive.🤖 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 `@packaging/rpm.Containerfile` at line 14, Update the GO_ARCH assignment in the Containerfile to explicitly map x86_64 to amd64 and aarch64 to arm64, and fail the build with an error for any other uname -m value instead of defaulting to arm64.
🤖 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 @.github/actions/quick-start-clean/action.yaml:
- Line 30: Update the quick-start invocation around quickstart.sh to stop
passing GITHUB_TOKEN as a sudo command-line assignment while xtrace is enabled.
Preserve the token handoff to quickstart.sh using an environment mechanism that
does not expose its value in traced arguments or process listings, while
retaining the existing IMAGE and TAG behavior.
In `@packaging/rpm.Containerfile`:
- Around line 13-20: Update the Go installation RUN step to pin an explicit Go
version instead of querying go.dev/VERSION, derive the supported architecture,
and retrieve the matching official per-architecture checksum from
go.dev/dl/?mode=json with failure and timeout handling. Download the pinned
archive, verify it with sha256sum -c before extraction, then preserve the
existing cleanup and installation flow.
In `@src/quickstart.sh`:
- Around line 203-206: Update the curl_args initialization in the release lookup
flow to include the fail-on-HTTP-error option, preserving the existing silent,
redirect-following, and timeout behavior. Ensure the curl call used by the
response assignment rejects 4xx/5xx responses before passing data to jq.
---
Outside diff comments:
In `@packaging/rpm.Containerfile`:
- Line 3: Update the base image in the RPM container stage from CentOS Stream 9
to quay.io/centos/centos:stream10, and revise any related stream9 references in
the same migration scope to consistently target Stream 10.
---
Nitpick comments:
In `@packaging/rpm.Containerfile`:
- Line 14: Update the GO_ARCH assignment in the Containerfile to explicitly map
x86_64 to amd64 and aarch64 to arm64, and fail the build with an error for any
other uname -m value instead of defaulting to arm64.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9e09f872-0bb2-4984-b915-a71b630e341b
📒 Files selected for processing (6)
.github/actions/quick-start-clean/action.yamlpackaging/rpm.Containerfilesrc/image/modify-spec.pysrc/quickstart.shsrc/rpm/create_repos.shsrc/rpm/postinstall.sh
💤 Files with no reviewable changes (1)
- src/rpm/create_repos.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- src/rpm/postinstall.sh
- src/image/modify-spec.py
3e05ec7 to
0e80a9b
Compare
0e80a9b to
f18e718
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packaging/rpm.Containerfile`:
- Around line 13-17: Update the GO_ARCH assignment in the Go toolchain download
RUN command to explicitly map uname -m values x86_64 to amd64 and aarch64 to
arm64, and fail for all other architectures instead of defaulting to ARM64.
Preserve the existing download flow for supported architectures.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 46fbe8d7-58e1-4d40-ba38-fe10268e02d0
📒 Files selected for processing (6)
.github/actions/quick-start-clean/action.yamlpackaging/rpm.Containerfilesrc/image/modify-spec.pysrc/quickstart.shsrc/rpm/create_repos.shsrc/rpm/postinstall.sh
💤 Files with no reviewable changes (1)
- src/rpm/create_repos.sh
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/actions/quick-start-clean/action.yaml
- src/image/modify-spec.py
- src/quickstart.sh
f18e718 to
f335d4f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/quickstart.sh`:
- Around line 204-207: Update the release lookup around the curl invocation in
quickstart.sh to disable xtrace while GITHUB_TOKEN is incorporated into
curl_args and used, then restore the prior tracing state afterward. Also remove
GITHUB_TOKEN from the quick-start-clean caller’s child Bash invocation so the
token is not exposed through environment or command tracing.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 825ca721-4a90-475a-9b59-7c9bbb65eaf2
📒 Files selected for processing (2)
.github/actions/quick-start-clean/action.yamlsrc/quickstart.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/actions/quick-start-clean/action.yaml
| # If the GITHUB_TOKEN is available use it to authN and avoid anonymous request limits | ||
| [ -n "${GITHUB_TOKEN:-}" ] && curl_args+=(-H "Authorization: token ${GITHUB_TOKEN}") | ||
| response="" | ||
| if ! response="$(curl "${curl_args[@]}" "https://github.com/proxy/api.github.com/repos/${OWNER}/${REPO}/releases/latest")"; then |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
git ls-files | rg '(^|/)(quickstart\.sh|action\.ya?ml|.*check.*)$' || true
echo
echo "== quickstart relevant lines =="
if [ -f src/quickstart.sh ]; then
nl -ba src/quickstart.sh | sed -n '180,225p'
fi
echo
echo "== quick-start action files =="
fd -a 'action\.ya?ml' .github actions src 2>/dev/null | sed 's#^\./##' | while read -r f; do
if rg -n 'quickstart|GITHUB_TOKEN|xtrace|set -x|bash -x|pipefail' "$f" >/dev/null 2>&1; then
echo "--- $f"
nl -ba "$f" | sed -n '1,220p'
fi
done
echo
echo "== search for quickstart invocation and env export =="
rg -n 'quickstart|GITHUB_TOKEN|set -x|bash -x|BUNDLER|GITHUB_TOKEN\|' .github src -S || true
echo
echo "== ShellCheck availability and lint (read-only) =="
if command -v shellcheck >/dev/null 2>&1; then
shellcheck --version | head -5
shellcheck src/quickstart.sh || true
else
echo "shellcheck not available"
fiRepository: microshift-io/microshift
Length of output: 851
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== quickstart relevant lines =="
awk 'NR>=180 && NR<=225 {printf "%7d %s\n", NR, $0}' src/quickstart.sh
echo
echo "== quick-start action file =="
awk 'NR>=1 && NR<=220 {printf "%7d %s\n", NR, $0}' .github/actions/quick-start-clean/action.yaml
echo
echo "== all action entries for quickstart/quick-start-clean/GITHUB_TOKEN/bash -x/set -x =="
awk '
/quick-start-clean|quickstart|GITHUB_TOKEN|xtrace|set -x|bash -x|pipefail|RUN|env:/ {printf "%s:%s\n", FILENAME, $0}
' .github/actions src/quickstart.sh docs/quickstart.sh 2>/dev/null || true
echo
echo "== search quickstart call graph with grep =="
grep -RIn 'quickstart|quick-start-clean|GITHUB_TOKEN|set -x|bash -x|pipefail' .github src docs 2>/dev/null || trueRepository: microshift-io/microshift
Length of output: 5941
Keep GITHUB_TOKEN out of xtrace logs.
quick-start-clean runs this script with set -x and passes GITHUB_TOKEN into the child Bash invocation before the curl_args expansion that adds Authorization: token .... Disable tracing around this release lookup and remove it from the caller invocation rather than relying on masking.
🤖 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 `@src/quickstart.sh` around lines 204 - 207, Update the release lookup around
the curl invocation in quickstart.sh to disable xtrace while GITHUB_TOKEN is
incorporated into curl_args and used, then restore the prior tracing state
afterward. Also remove GITHUB_TOKEN from the quick-start-clean caller’s child
Bash invocation so the token is not exposed through environment or command
tracing.
Summary by CodeRabbit
/etc/containers/policy.jsonexists by copying from the default location when needed.