Updates release workflow to generate native packages - #12
Conversation
maqeel75
commented
Jul 10, 2026
maqeel75
commented
- Updates release workflow to generate native packages
- Makes sure that docker images don't create latest tag on pre-release tags. Only release tag will generate latest docker tag
Up to standards ✅🟢 Issues
|
📝 WalkthroughWalkthroughThe pull request adds a shared packaging framework and build scripts for RPM and Debian packages. It defines package metadata, configuration, SBOM generation, signing, and artifact collection. The release workflow now supports simulated and tag-based runs, matrix-driven packaging, DNF and APT publishing, S3 backups, aggregated manifests, workflow artifacts, and Slack notification preparation. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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
🧹 Nitpick comments (5)
.github/workflows/release.yml (2)
82-83: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSet
persist-credentials: falseon checkout steps that don't need to push.None of these
actions/checkoutsteps push back to this repo (GoReleaser pushes viaGITHUB_TOKENenv, not git), yet all default to persisting the token in.git/configfor the job's duration. Combined with the subsequentgit cloneof external action repos usingPGEDGE_BUILDER_TOKENin the same job, this widens the window during which the runner's local git credentials are exposed to other steps.🔒 Example fix (repeat for each checkout)
- name: Checkout code uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: falseAlso applies to: 185-188, 246-247, 347-348, 459-460, 555-556
🤖 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.yml around lines 82 - 83, Set persist-credentials: false on every actions/checkout step identified by the review, including the checkout step near the existing uses entry and the additional checkout steps at the referenced locations; preserve the pinned action versions and all other workflow settings.Source: Linters/SAST tools
273-277: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winInterpolate
matrix.archviaenv:instead of directly in the shell template.
${{ matrix.arch }}is expanded twice into the shell body here (case "${{ matrix.arch }}"and inside the error message), which zizmor flags as a template-injection surface. The workflow already established the safer pattern for exactly this reason at lines 142-145 (SUFFIX/EFFECTIVE_TAGpassed viaenv:"so a hostile tag value can't be template-substituted into the shell body"). Even thoughmatrix.archcurrently only takes hardcoded values (amd64/arm64) from thearchsinput, it's produced by an external action (pgedge-detect-build-matrix) and this inconsistency leaves a gap if that ever changes.🔒 Proposed fix
- name: Stage release-artifacts/ for the build container + env: + ARCH: ${{ matrix.arch }} run: | set -euo pipefail - case "${{ matrix.arch }}" in + case "${ARCH}" in amd64) tok=x86_64 ;; arm64) tok=arm64 ;; - *) echo "::error::unexpected arch ${{ matrix.arch }}"; exit 1 ;; + *) echo "::error::unexpected arch ${ARCH}"; exit 1 ;; esacAlso applies to: 486-490
🤖 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.yml around lines 273 - 277, Replace direct `${{ matrix.arch }}` interpolations in both architecture case blocks with an environment variable assigned through the step’s `env:` section, then reference that variable in the shell `case` statement and unexpected-architecture error message. Apply the same change to the blocks around both the reported locations, preserving the existing amd64/arm64 mappings and validation.Source: Linters/SAST tools
common/common-functions.sh (1)
109-159: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSwapped variable names in
import_gpg_keys(debug-only helper).
PRI_FILEis assigned thepublic.keypath andPUB_FILEtheprivate.keypath — inverted relative to their names. The subsequent reads still land in the rightGPG_PUBLIC_KEY/GPG_PRIVATE_KEYvariables because both the name and the mistake cancel out, but this is confusing enough that a future maintainer "fixing" it based on the names alone would introduce a real bug (swapping public/private key contents). Since this is explicitly a debug-only function (perpkg/build-rpm.sh's#import_gpg_keyscomment) not exercised by the CI pipeline, worth a quick rename for clarity.♻️ Proposed fix
- PRI_FILE=$(dirname "$0")/public.key - PUB_FILE=$(dirname "$0")/private.key + PUB_FILE=$(dirname "$0")/public.key + PRI_FILE=$(dirname "$0")/private.key - GPG_PUBLIC_KEY=$(cat $PRI_FILE) - GPG_PRIVATE_KEY=$(cat $PUB_FILE) + GPG_PUBLIC_KEY=$(cat $PUB_FILE) + GPG_PRIVATE_KEY=$(cat $PRI_FILE)🤖 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 `@common/common-functions.sh` around lines 109 - 159, Rename the swapped path variables in import_gpg_keys so the public.key path is assigned to PUB_FILE and private.key to PRI_FILE, then update the corresponding reads to populate GPG_PUBLIC_KEY and GPG_PRIVATE_KEY from the correctly named variables while preserving existing cleanup behavior.pkg/rpm/docloader.spec (1)
26-27: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winComputed
KEY_IDis never used for SBOM signing.
KEY_IDis looked up and exported but the subsequentgpg --armor --detach-signdoesn't pass--local-user "$KEY_ID"(or--default-key), unlikesign_rpmsincommon/common-functions.shwhich correctly uses--define "_gpg_name $KEY_ID"for the RPM signature itself. Currently harmless since only one secret key (GPG_FIPS_RPM_*) is imported per this job, but it's a latent correctness gap if the keyring ever contains more than one secret key.🔒 Proposed fix
KEY_ID=$(gpg --list-secret-keys --with-colons | awk -F: '/^sec/{print $5}' | head -n 1); export KEY_ID -gpg --armor --detach-sign --output %{_builddir}/%{sname}-sbom.json.asc %{_builddir}/%{sname}-sbom.json || exit 1 +gpg --local-user "$KEY_ID" --armor --detach-sign --output %{_builddir}/%{sname}-sbom.json.asc %{_builddir}/%{sname}-sbom.json || exit 1🤖 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 `@pkg/rpm/docloader.spec` around lines 26 - 27, Use the computed KEY_ID when signing the SBOM in the spec’s GPG command: add the appropriate local-user selection (for example, --local-user "$KEY_ID") to the gpg invocation, matching the key-selection behavior used by sign_rpms.pkg/build-deb.sh (1)
32-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueQuote unquoted variable expansions (shellcheck SC2086).
$SRC_DIR(line 33) and${COMPONENT_NAME}/common/config.yml(line 47) are unquoted, risking word-splitting/globbing.🔧 Proposed fix
rm -rf "$SRC_DIR" - mkdir -p $SRC_DIR + mkdir -p "$SRC_DIR"cp -rp "${CWD}/${COMPONENT_NAME}/deb/debian" "$SRC_DIR/" - cp ${COMPONENT_NAME}/common/config.yml "$SRC_DIR/debian/" + cp "${COMPONENT_NAME}/common/config.yml" "$SRC_DIR/debian/"Also applies to: 45-47
🤖 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 `@pkg/build-deb.sh` around lines 32 - 33, Quote all variable expansions in pkg/build-deb.sh to prevent word splitting and glob expansion: update the mkdir invocation using SRC_DIR and the paths involving COMPONENT_NAME/common/config.yml, preserving the existing command behavior.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.
Inline comments:
In `@common/common-functions.sh`:
- Around line 85-94: Update configure_pgedge_apt_repo() to run apt-get update
with sudo, matching the elevated dpkg and repository setup commands so the DEB
packaging path can complete successfully.
In `@pkg/build-deb.sh`:
- Around line 78-84: Update post_build so cp failures are not swallowed:
explicitly detect whether "$BUILD_DIR" contains any .deb files, report and fail
when none exist, and run sudo cp without an || fallback so permission, disk, or
copy errors propagate a nonzero exit status.
In `@pkg/deb/debian/rules`:
- Around line 15-18: Use the computed KEY_ID in the gpg signing command by
passing it explicitly via --local-user "$KEY_ID" (or an equivalent default-key
option) before signing the SBOM, while retaining the existing failure handling.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 82-83: Set persist-credentials: false on every actions/checkout
step identified by the review, including the checkout step near the existing
uses entry and the additional checkout steps at the referenced locations;
preserve the pinned action versions and all other workflow settings.
- Around line 273-277: Replace direct `${{ matrix.arch }}` interpolations in
both architecture case blocks with an environment variable assigned through the
step’s `env:` section, then reference that variable in the shell `case`
statement and unexpected-architecture error message. Apply the same change to
the blocks around both the reported locations, preserving the existing
amd64/arm64 mappings and validation.
In `@common/common-functions.sh`:
- Around line 109-159: Rename the swapped path variables in import_gpg_keys so
the public.key path is assigned to PUB_FILE and private.key to PRI_FILE, then
update the corresponding reads to populate GPG_PUBLIC_KEY and GPG_PRIVATE_KEY
from the correctly named variables while preserving existing cleanup behavior.
In `@pkg/build-deb.sh`:
- Around line 32-33: Quote all variable expansions in pkg/build-deb.sh to
prevent word splitting and glob expansion: update the mkdir invocation using
SRC_DIR and the paths involving COMPONENT_NAME/common/config.yml, preserving the
existing command behavior.
In `@pkg/rpm/docloader.spec`:
- Around line 26-27: Use the computed KEY_ID when signing the SBOM in the spec’s
GPG command: add the appropriate local-user selection (for example, --local-user
"$KEY_ID") to the gpg invocation, matching the key-selection behavior used by
sign_rpms.
🪄 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
Run ID: 5f7320ce-9324-454d-b6f8-a92d14fcc6f7
📒 Files selected for processing (12)
.github/workflows/release.ymlcommon/build.shcommon/common-functions.shpkg/build-deb.shpkg/build-rpm.shpkg/common.shpkg/common/config.ymlpkg/deb/debian/controlpkg/deb/debian/docspkg/deb/debian/formatpkg/deb/debian/rulespkg/rpm/docloader.spec
2 participants
