fix: ensure identical version across matrix builds#226
Conversation
Generate build timestamp once in a pre-job and propagate it via BUILD_TIMESTAMP env var so all matrix runners (amd64, arm64) produce the same version string. Previously each runner called date(1) independently, causing the per-arch image tags to diverge and breaking the multi-arch manifest creation step. Signed-off-by: Leela Venkaiah G <leelavg@thoughtexpo.com>
📝 WalkthroughWalkthroughA release workflow job now computes a UTC build timestamp and passes it to ChangesBuild timestamp propagation
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant GenerateVersion as generate-version job
participant BuildMicroshift as build-microshift job
participant SRPMBuild as srpm target
participant Containerfile as packaging/srpm.Containerfile
participant BuildRpms as build-rpms.sh
GenerateVersion->>BuildMicroshift: provide build-timestamp output
BuildMicroshift->>SRPMBuild: set BUILD_TIMESTAMP env var
SRPMBuild->>Containerfile: pass --build-arg BUILD_TIMESTAMP
Containerfile->>BuildRpms: expose BUILD_TIMESTAMP to container build
BuildRpms->>BuildRpms: use BUILD_TIMESTAMP or fallback to date -u
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 |
|
/hold sample failure https://github.com/leelavg/microshift/actions/runs/28642468715/job/84945912204 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/release.yaml (1)
28-36: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd explicit
permissionsblock togenerate-versionjob.Static analysis flags this job for using default (broad) permissions. Since it only computes and outputs a timestamp, scope it down.
🔒 Proposed fix
generate-version: name: Generate build version runs-on: ubuntu-24.04 + permissions: + contents: read outputs: build-timestamp: ${{ steps.timestamp.outputs.value }}🤖 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 @.github/workflows/release.yaml around lines 28 - 36, The generate-version job is relying on the workflow’s default GitHub token permissions even though it only runs the timestamp step. Add an explicit permissions block to the generate-version job in release.yaml and scope it to the minimum required access (ideally none) so the Generate build version / timestamp step remains read-only and cannot use broader defaults.Source: Linters/SAST tools
🤖 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.
Nitpick comments:
In @.github/workflows/release.yaml:
- Around line 28-36: The generate-version job is relying on the workflow’s
default GitHub token permissions even though it only runs the timestamp step.
Add an explicit permissions block to the generate-version job in release.yaml
and scope it to the minimum required access (ideally none) so the Generate build
version / timestamp step remains read-only and cannot use broader defaults.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bc200e8f-b705-4f75-8af0-b5a96e08fea1
📒 Files selected for processing (2)
.github/workflows/release.yamlsrc/image/build-rpms.sh
build-rpms.sh runs inside the srpm container image, so the host environment variable is not inherited. Thread BUILD_TIMESTAMP through as a podman --build-arg and declare it as ARG in the Containerfile so the script picks it up instead of calling date(1) independently. Signed-off-by: Leela Venkaiah G <leelavg@thoughtexpo.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packaging/srpm.Containerfile (1)
86-86: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider grouping with other
ARGdeclarations.Functionally correct —
BUILD_TIMESTAMPwill be visible tobuild-rpms.shon the next line. For consistency, other build args (USHIFT_GITREF,OKD_VERSION_TAG, etc.) are grouped near the top (Lines 10-23); grouping this one there too would improve readability, though its current placement next to its sole consumer is also reasonable.🤖 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/srpm.Containerfile` at line 86, Move BUILD_TIMESTAMP into the existing grouped ARG declarations near the top of the Containerfile alongside USHIFT_GITREF, OKD_VERSION_TAG, and the other build args. Keep the build script invocation in the same place, but declare BUILD_TIMESTAMP with the rest of the inputs so the argument definitions stay consistent and easier to scan.
🤖 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.
Nitpick comments:
In `@packaging/srpm.Containerfile`:
- Line 86: Move BUILD_TIMESTAMP into the existing grouped ARG declarations near
the top of the Containerfile alongside USHIFT_GITREF, OKD_VERSION_TAG, and the
other build args. Keep the build script invocation in the same place, but
declare BUILD_TIMESTAMP with the rest of the inputs so the argument definitions
stay consistent and easier to scan.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7db447cb-5656-4ea5-8e0b-96b0ae3ba3c0
📒 Files selected for processing (2)
Makefilepackaging/srpm.Containerfile
Generate build timestamp once in a pre-job and propagate it via BUILD_TIMESTAMP env var so all matrix runners (amd64, arm64) produce the same version string. Previously each runner called date(1) independently, causing the per-arch image tags to diverge and breaking the multi-arch manifest creation step.
Summary by CodeRabbit