Add native RPM/DEB packaging and release pipeline for ColdFront#44
Add native RPM/DEB packaging and release pipeline for ColdFront#44maqeel75 wants to merge 15 commits into
Conversation
…e per distro) and ship coldfront-bin config
…O link was hitting No space left on device) + a note to start using /mnt in future if required.
…r needs the patched extensions at runtime
…o env.RPM_IMAGES/DEB_IMAGES
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds a compactor binary, shared build helpers, packaging assets for five components, and a tag-routed release workflow that builds, verifies, packages, publishes, backs up, and reports release artifacts. ChangesPackaging and Release Pipeline
Estimated code review effort: 4 (Complex) | ~75 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8).github/workflows/release.ymlTraceback (most recent call last): Comment |
Up to standards ✅🟢 Issues
|
# Conflicts: # .github/workflows/release.yml
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (7)
packaging/coldfront-bin/rpm/coldfront.spec (1)
5-5: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueDrop the redundant
BuildArch: %{_arch}
rpmbuildalready targets the native arch here, so this line is unnecessary. Removing it keeps the spec cleaner and avoids the extra spec re-parse caveat if macros are added above it later.🤖 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/coldfront-bin/rpm/coldfront.spec` at line 5, Remove the redundant BuildArch declaration from the spec so the package relies on rpmbuild’s native architecture handling. Update the top-level RPM spec definition near the existing debug_package macro, keeping the rest of the package metadata unchanged and ensuring no other macros depend on the removed BuildArch line..github/workflows/release.yml (1)
123-126: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSet
persist-credentials: falseon checkouts.None of the
actions/checkoutsteps disable credential persistence, so the defaultGITHUB_TOKENis written into.git/config. The workflow subsequently clones private repos using an explicitPGEDGE_BUILDER_TOKENand uploads workspace artifacts, so the persisted default credential isn't needed and is best dropped as defense-in-depth. This applies to everyactions/checkoutstep in the file (also flagged at lines 190, 318, 363, 427, 525, 625, 688, 752, 833).🔒 Example
- name: Checkout uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 with: fetch-depth: 0 + persist-credentials: false🤖 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 123 - 126, The Checkout steps in this workflow are persisting the default GITHUB_TOKEN in .git/config, which is unnecessary since the workflow uses explicit credentials elsewhere. Update every actions/checkout usage in the release workflow to set persist-credentials to false, keeping the existing fetch-depth and other settings intact. Use the Checkout step entries as the targets for this change across the file.Source: Linters/SAST tools
packaging/coldfront-duckdb-extensions/build-deb.sh (1)
80-81: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valuePredictable /tmp path (static analysis hint).
/tmp/pg_deb_buildis a fixed, predictable path; a local attacker on a shared runner could pre-create/symlink it. Low real risk on isolated ephemeral CI runners, butmktemp -dwould be more robust.🤖 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/coldfront-duckdb-extensions/build-deb.sh` around lines 80 - 81, The build script uses a predictable fixed /tmp path for the deb build directory, which should be replaced with an unpredictable temporary directory. Update the build flow around rename_ddeb_packages in build-deb.sh to create the workspace with mktemp -d (or equivalent), store it in a variable, and pass that variable instead of hardcoding /tmp/pg_deb_build so the temporary location is unique per run.Source: Linters/SAST tools
packaging/lakekeeper/build-rpm.sh (1)
26-34: 🔒 Security & Privacy | 🔵 Trivial | ⚖️ Poor tradeoffNo integrity verification of downloaded upstream binary before signing/shipping.
The Lakekeeper release tarball, LICENSE, and NOTICE are fetched with
wgetand used directly (no checksum or signature check) before the resulting package is GPG-signed and published. If the upstream release asset were ever corrupted in transit or the wrong artifact served, it would be repackaged and signed as trusted pgEdge output without detection. GitHub now publishes SHA256 digests for release assets, and Lakekeeper's release notes may include checksums — verifying against these before packaging would close this gap.🤖 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/lakekeeper/build-rpm.sh` around lines 26 - 34, The download flow in build-rpm.sh fetches the Lakekeeper tarball, LICENSE, and NOTICE with wget and packages them without verifying integrity. Update the packaging step around the tarball download and the LICENSE/NOTICE fetches to validate the artifacts against an expected SHA256 digest or release-published checksum before they are copied into rpmbuild/SOURCES and signed; use the existing build-rpm.sh download logic and the tarball/REPO_RAW variables to keep the verification tied to the fetched release version.packaging/coldfront-duckdb-extensions/build-extensions.sh (1)
41-46: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant submodule + unbounded vcpkg clone.
git submodule update --init --recursive --depth 1(line 41) fully clones theduckdbsubmodule insideduckdb-iceberg, which is then immediately discarded and re-cloned pinned atv${DUCKDB_VERSION}(lines 43-44) — wasted bandwidth/time on every build. Separately,git clone "${VCPKG_REPO}"(line 45) clones the full vcpkg history/ports tree with no--depth 1, which is a large repo and slows every CI run unnecessarily.Consider excluding the
duckdbpath from the submodule update (or deinit it before line 41) and shallow-cloning vcpkg.♻️ Suggested tweak
-git -C "$ICE" submodule update --init --recursive --depth 1 --jobs 8 +git -C "$ICE" submodule update --init --recursive --depth 1 --jobs 8 -- $(git -C "$ICE" config --file .gitmodules --get-regexp path | awk '{print $2}' | grep -v '^duckdb$') ... -git clone "${VCPKG_REPO}" "${BUILD_ROOT}/vcpkg" +git clone --depth 1 "${VCPKG_REPO}" "${BUILD_ROOT}/vcpkg"🤖 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/coldfront-duckdb-extensions/build-extensions.sh` around lines 41 - 46, The build script is doing extra work by fully updating the nested duckdb submodule only to delete and re-clone it later, and it is also cloning vcpkg without a shallow fetch. Update build-extensions.sh so the initial git submodule update for the ICE checkout excludes or deinitializes the duckdb path before the clone replacement, and change the vcpkg clone to a shallow checkout. Keep the fix localized around the submodule handling and the vcpkg clone steps in the build flow.packaging/coldfront-duckdb-extensions/deb/debian/rules (1)
22-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSBOM+GPG signing block duplicated across every packaging script.
This exact syft/gpg sequence is repeated near-verbatim in
packaging/pg_duckdb/deb/debian/rules,packaging/lakekeeper/deb/debian/rules, and the RPM%buildsections. Consider extracting a shared helper (e.g. acommon/sign-sbom.shsourced/called with$SNAMEand output dir) to avoid drift when the signing logic needs to change (e.g. key selection strategy).♻️ Sketch of a shared helper
# common/sign-sbom.sh sign_sbom() { local sname="$1" outdir="$2" mkdir -p "$outdir" syft dir:"${3:-$PWD}" -o cyclonedx-json > "$outdir/${sname}-sbom.json" || exit 1 local key_id key_id=$(gpg --list-secret-keys --with-colons | awk -F: '/^sec/{print $5}' | head -n 1) gpg --armor --detach-sign --local-user "$key_id" \ --output "$outdir/${sname}-sbom.json.asc" "$outdir/${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 `@packaging/coldfront-duckdb-extensions/deb/debian/rules` around lines 22 - 29, The SBOM generation and GPG signing logic in this debian/rules fragment is duplicated across multiple packaging scripts, so move the syft/gpg sequence into a shared helper and call it from execute_before_dh_install. Add a reusable helper such as common/sign-sbom.sh that accepts the package name and output directory (and optionally the source dir), then update the packaging entry points like execute_before_dh_install and the other debian/rules/RPM build sections to invoke that helper instead of inlining the signing steps. Keep the helper responsible for key selection, SBOM output naming, and signature file creation so future signing changes happen in one place.packaging/pg_duckdb/build-rpm.sh (1)
34-36: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winVerify curl tarball integrity before use.
The curl source is fetched over HTTPS but never checksummed/verified against a known-good hash or signature before being staged into
SOURCESand built. Given this build exists specifically to remediate a curl CVE, pinning a hash guards against a compromised/mirrored download.🔒 Proposed fix: verify SHA256 checksum
echo "Staging curl ${CURL_VERSION} source (build-time, EL9 only)..." wget -q "https://curl.se/download/curl-${CURL_VERSION}.tar.gz" \ -O ~/rpmbuild/SOURCES/curl-${CURL_VERSION}.tar.gz + wget -q "https://curl.se/download/curl-${CURL_VERSION}.tar.gz.sha256" -O /tmp/curl.sha256 + (cd ~/rpmbuild/SOURCES && sha256sum -c <(sed "s|curl-${CURL_VERSION}.tar.gz|curl-${CURL_VERSION}.tar.gz|" /tmp/curl.sha256))🤖 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/pg_duckdb/build-rpm.sh` around lines 34 - 36, The curl staging step in build-rpm.sh downloads the tarball in the build flow without verifying its integrity, so add a SHA256 (or similar) verification immediately after the wget in the curl staging block. Use the existing CURL_VERSION-specific download path and update the build logic to compare the fetched tarball against a known-good pinned digest before placing it into ~/rpmbuild/SOURCES, so the check is tied to the curl download step rather than the later RPM build.
🤖 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 167-168: The key selection in sign_rpms is ambiguous because it
re-derives KEY_ID from gpg --list-secret-keys and head -n 1, which can pick the
wrong secret key on a populated keyring. Update import_gpg_keys to capture the
imported key’s fingerprint/key ID at import time, then pass or export that value
explicitly into sign_rpms so it uses the intended key instead of listing the
keyring again. Keep the fix centered around import_gpg_keys and sign_rpms so the
signing step is tied to the specific imported key.
- Around line 108-113: The GPG key file path variables are mislabeled: PRI_FILE
points to public.key and PUB_FILE points to private.key, which is confusing for
future edits. Rename the variables in the key-loading block of the shell script
to match their actual contents, and update the corresponding cat and rm -f uses
so the public/private key handling is immediately clear. Keep the existing
behavior intact while aligning the variable names with the files they reference.
- Around line 3-11: The install_syft function still executes a remote installer
directly with curl piped into sudo sh, which leaves the build host exposed to
unverified code. Update install_syft to first download the syft install script
to a local temporary file, verify its integrity or authenticity before use, and
only then run it with sudo; keep the existing SYFT_VERSION pinning and reuse the
install_syft symbol so the fix is localized.
- Around line 61-79: The repo setup helpers are installing packages from
unverified remote locations and `configure_pgedge_apt_repo` uses a predictable
`/tmp/pgedge-release.deb` path. Update `configure_pgedge_apt_repo` to download
into a unique temporary file created with `mktemp` and ensure it is cleaned up
after `sudo dpkg -i`; also review `configure_pgedge_dnf_repo` to avoid direct
root installation from a URL if possible by downloading and verifying first.
Keep the changes scoped to `configure_pgedge_dnf_repo` and
`configure_pgedge_apt_repo`.
- Around line 146-174: The sign_rpms cleanup path can fail under set -u when GPG
setup variables are unset, which masks the real missing-key error. Update
sign_rpms to guard the cleanup in the KEY_ID check so it only removes
PRIVATE_KEY_FILE and GNUPGHOME when those variables are set, and keep the
failure flow intact after the gpg --list-secret-keys lookup. Use the sign_rpms
function and the KEY_ID empty-check block to locate the fix.
In `@packaging/coldfront-duckdb-extensions/build-deb.sh`:
- Around line 77-83: The post_build flow in build-deb.sh currently swallows the
missing-.deb case by falling back to echo, which lets failed package builds pass
silently. Update post_build and the sudo cp step so that when /tmp/pg_deb_build
has no .deb artifacts the script exits with a non-zero status instead of
continuing, while preserving the existing copy behavior for successful builds.
Use the existing rename_ddeb_packages and post_build functions as the hook
points for the check.
In `@packaging/lakekeeper/build-deb.sh`:
- Around line 70-75: The post_build step in build-deb.sh currently swallows
missing .deb artifacts by falling back to a log message, which lets the build
succeed silently. Update post_build so the .deb copy step in the packaging flow
fails the script when no package is produced, using the existing post_build and
rename_ddeb_packages flow to locate the problem and removing the silent-success
behavior from the sudo cp command.
- Around line 33-40: The release download flow in build-deb.sh extracts the
tarball and fetches LICENSE/NOTICE without verifying integrity first. Update the
packaging step around the tarball download/extraction and the LICENSE/NOTICE
fetches to validate the release asset against a published checksum or signature
before any extraction or packaging. Use the existing tarball download path and
the RELEASE_URL/REPO_RAW retrieval points to locate the change, and fail the
build if verification does not pass.
In `@packaging/pg_duckdb/build-deb.sh`:
- Around line 95-101: The post_build artifact staging in post_build is
swallowing real copy errors by using a broad fallback after sudo cp, so only the
expected “no .deb files” case should be handled specially. Update the copy step
to distinguish between an empty glob in "$BUILD_DIR" and genuine cp failures to
"/output", and make unexpected errors fail the script instead of printing the
fallback message. Keep the logic localized around post_build,
rename_ddeb_packages, and the sudo cp invocation.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 123-126: The Checkout steps in this workflow are persisting the
default GITHUB_TOKEN in .git/config, which is unnecessary since the workflow
uses explicit credentials elsewhere. Update every actions/checkout usage in the
release workflow to set persist-credentials to false, keeping the existing
fetch-depth and other settings intact. Use the Checkout step entries as the
targets for this change across the file.
In `@packaging/coldfront-bin/rpm/coldfront.spec`:
- Line 5: Remove the redundant BuildArch declaration from the spec so the
package relies on rpmbuild’s native architecture handling. Update the top-level
RPM spec definition near the existing debug_package macro, keeping the rest of
the package metadata unchanged and ensuring no other macros depend on the
removed BuildArch line.
In `@packaging/coldfront-duckdb-extensions/build-deb.sh`:
- Around line 80-81: The build script uses a predictable fixed /tmp path for the
deb build directory, which should be replaced with an unpredictable temporary
directory. Update the build flow around rename_ddeb_packages in build-deb.sh to
create the workspace with mktemp -d (or equivalent), store it in a variable, and
pass that variable instead of hardcoding /tmp/pg_deb_build so the temporary
location is unique per run.
In `@packaging/coldfront-duckdb-extensions/build-extensions.sh`:
- Around line 41-46: The build script is doing extra work by fully updating the
nested duckdb submodule only to delete and re-clone it later, and it is also
cloning vcpkg without a shallow fetch. Update build-extensions.sh so the initial
git submodule update for the ICE checkout excludes or deinitializes the duckdb
path before the clone replacement, and change the vcpkg clone to a shallow
checkout. Keep the fix localized around the submodule handling and the vcpkg
clone steps in the build flow.
In `@packaging/coldfront-duckdb-extensions/deb/debian/rules`:
- Around line 22-29: The SBOM generation and GPG signing logic in this
debian/rules fragment is duplicated across multiple packaging scripts, so move
the syft/gpg sequence into a shared helper and call it from
execute_before_dh_install. Add a reusable helper such as common/sign-sbom.sh
that accepts the package name and output directory (and optionally the source
dir), then update the packaging entry points like execute_before_dh_install and
the other debian/rules/RPM build sections to invoke that helper instead of
inlining the signing steps. Keep the helper responsible for key selection, SBOM
output naming, and signature file creation so future signing changes happen in
one place.
In `@packaging/lakekeeper/build-rpm.sh`:
- Around line 26-34: The download flow in build-rpm.sh fetches the Lakekeeper
tarball, LICENSE, and NOTICE with wget and packages them without verifying
integrity. Update the packaging step around the tarball download and the
LICENSE/NOTICE fetches to validate the artifacts against an expected SHA256
digest or release-published checksum before they are copied into
rpmbuild/SOURCES and signed; use the existing build-rpm.sh download logic and
the tarball/REPO_RAW variables to keep the verification tied to the fetched
release version.
In `@packaging/pg_duckdb/build-rpm.sh`:
- Around line 34-36: The curl staging step in build-rpm.sh downloads the tarball
in the build flow without verifying its integrity, so add a SHA256 (or similar)
verification immediately after the wget in the curl staging block. Use the
existing CURL_VERSION-specific download path and update the build logic to
compare the fetched tarball against a known-good pinned digest before placing it
into ~/rpmbuild/SOURCES, so the check is tied to the curl download step rather
than the later RPM build.
🪄 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: 251d16d8-f180-4243-aff6-620e18aa7c8e
📒 Files selected for processing (54)
.github/workflows/release.yml.goreleaser.yamlcommon/build.shcommon/common-functions.shpackaging/coldfront-bin/build-deb.shpackaging/coldfront-bin/build-rpm.shpackaging/coldfront-bin/common.shpackaging/coldfront-bin/deb/debian/controlpackaging/coldfront-bin/deb/debian/pgedge-coldfront.conffilespackaging/coldfront-bin/deb/debian/pgedge-coldfront.installpackaging/coldfront-bin/deb/debian/rulespackaging/coldfront-bin/deb/debian/source/formatpackaging/coldfront-bin/rpm/coldfront.specpackaging/coldfront-duckdb-extensions/build-deb.shpackaging/coldfront-duckdb-extensions/build-extensions.shpackaging/coldfront-duckdb-extensions/build-rpm.shpackaging/coldfront-duckdb-extensions/common.shpackaging/coldfront-duckdb-extensions/config/coldfront-duckdb-extensions.conf.samplepackaging/coldfront-duckdb-extensions/deb/debian/controlpackaging/coldfront-duckdb-extensions/deb/debian/rulespackaging/coldfront-duckdb-extensions/deb/debian/source/formatpackaging/coldfront-duckdb-extensions/rpm/coldfront-duckdb-extensions.specpackaging/coldfront/build-deb.shpackaging/coldfront/build-rpm.shpackaging/coldfront/common.shpackaging/coldfront/deb/debian/control.inpackaging/coldfront/deb/debian/pgedge-postgresql-coldfront.installpackaging/coldfront/deb/debian/pgversionspackaging/coldfront/deb/debian/rulespackaging/coldfront/deb/debian/source/formatpackaging/coldfront/rpm/coldfront.specpackaging/lakekeeper/README.mdpackaging/lakekeeper/build-deb.shpackaging/lakekeeper/build-rpm.shpackaging/lakekeeper/common.shpackaging/lakekeeper/common/lakekeeper.envpackaging/lakekeeper/common/lakekeeper.servicepackaging/lakekeeper/deb/debian/controlpackaging/lakekeeper/deb/debian/pgedge-lakekeeper.conffilespackaging/lakekeeper/deb/debian/pgedge-lakekeeper.docspackaging/lakekeeper/deb/debian/pgedge-lakekeeper.installpackaging/lakekeeper/deb/debian/postinstpackaging/lakekeeper/deb/debian/rulespackaging/lakekeeper/deb/debian/source/formatpackaging/lakekeeper/rpm/lakekeeper.specpackaging/pg_duckdb/build-deb.shpackaging/pg_duckdb/build-rpm.shpackaging/pg_duckdb/common.shpackaging/pg_duckdb/deb/debian/control.inpackaging/pg_duckdb/deb/debian/pgedge-postgresql-pg-duckdb.installpackaging/pg_duckdb/deb/debian/pgversionspackaging/pg_duckdb/deb/debian/rulespackaging/pg_duckdb/deb/debian/source/formatpackaging/pg_duckdb/rpm/pg_duckdb.spec
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
.github/workflows/release.yml (4)
735-740: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not block publication when an optional packaging matrix is skipped.
If
has_ext_rpm/has_ext_debis false, the corresponding package job is skipped, but these conditions still require its result to besuccess. That prevents publishing valid source packages when no extension cells exist. Allowsuccessor an intentionalskippedresult, while still failing on real build failures.Also applies to: 817-821
🤖 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 735 - 740, Update the publication job conditions near the shown needs checks to treat optional package jobs as successful when their result is either success or skipped, including the corresponding conditions around the later referenced block. Preserve the forced-push behavior and continue blocking publication when either packaging job has a real failure or is cancelled.
1029-1034: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUpload the manifest even when publishing fails.
The artifact upload runs after the external publisher without
if: always(). A publisher or Slack failure can therefore hide the manifest needed for debugging or promotion.🤖 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 1029 - 1034, Update the “Upload aggregated manifest as workflow artifact” step to run unconditionally by adding the workflow’s always() condition, so aggregated/manifest.json is uploaded even when publishing or Slack notification steps fail.
736-740: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAllow
force_pushto override the tag-only gate.Because
github.ref_type == 'tag'is required unconditionally,force_push=trueon a manual branch dispatch can never publish anything. Make the tag check an alternative to the explicitly enabled force-push path.Also applies to: 818-821
🤖 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 736 - 740, Update the release job’s if condition to make the tag requirement apply only to the normal successful package path, while allowing the explicitly enabled force_push path to run for manual branch dispatches. Preserve the existing cancelled-job exclusions and package success requirements, and apply the same condition change to the corresponding block noted in the comment.
80-91: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPin the release helper repos to immutable commits. These
git clone --depth 1steps pull whatever is current on the default ormulti-targetbranch and then execute it with signing keys and AWS credentials; a branch change could alter the release payload or expose secrets. Also applies to: 192-200, 474-484, 637-647, 700-710, 969-977🤖 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 80 - 91, Update each release-helper checkout, including the pgedge-parse-release-tag step and the other referenced clone steps, to fetch and check out an immutable, reviewed commit SHA rather than the moving default or multi-target branch. Preserve the existing local action paths and subsequent uses, and apply the pinned revision consistently to every listed helper repository.
🤖 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.
Outside diff comments:
In @.github/workflows/release.yml:
- Around line 735-740: Update the publication job conditions near the shown
needs checks to treat optional package jobs as successful when their result is
either success or skipped, including the corresponding conditions around the
later referenced block. Preserve the forced-push behavior and continue blocking
publication when either packaging job has a real failure or is cancelled.
- Around line 1029-1034: Update the “Upload aggregated manifest as workflow
artifact” step to run unconditionally by adding the workflow’s always()
condition, so aggregated/manifest.json is uploaded even when publishing or Slack
notification steps fail.
- Around line 736-740: Update the release job’s if condition to make the tag
requirement apply only to the normal successful package path, while allowing the
explicitly enabled force_push path to run for manual branch dispatches. Preserve
the existing cancelled-job exclusions and package success requirements, and
apply the same condition change to the corresponding block noted in the comment.
- Around line 80-91: Update each release-helper checkout, including the
pgedge-parse-release-tag step and the other referenced clone steps, to fetch and
check out an immutable, reviewed commit SHA rather than the moving default or
multi-target branch. Preserve the existing local action paths and subsequent
uses, and apply the pinned revision consistently to every listed helper
repository.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4471ca14-d827-454b-b376-7d00be160c8f
📒 Files selected for processing (1)
.github/workflows/release.yml
Fixed one valid issue, others either won't reach or as per design. |
1 participant
Summary
Adds native RPM/DEB packaging and a full release pipeline for ColdFront, replacing the
previous GoReleaser-only release workflow. A single tag push now builds, signs, and publishes
packages for all ColdFront components across the supported EL and Debian/Ubuntu platforms.
Components
pgedge-coldfrontpgedge-postgresql-<N>-coldfrontpgedge-pg-duckdb_<N>/pgedge-postgresql-<N>-pg-duckdbpgedge-coldfront-duckdb-extensions.duckdb_extensionpgedge-lakekeeperTarget matrix
Debian bullseye is intentionally excluded (EOL, and the prebuilt lakekeeper binary requires glibc >= 2.34).
Release pipeline (
.github/workflows/release.yml)pgedge-parse-release-tag, detect matrix viapgedge-detect-build-matrix,merged into one combined per-(component, PG major, distro, arch) matrix.
verified by a load-test gate that
LOADs all four extensions in the real DuckDB engine on everysupported distro, then repackaged per distro without recompiling.
aggregated promotion manifest + Slack notification.
env.RPM_IMAGES/env.DEB_IMAGES(single source of truth, consumedby every detect step and the load-test gate).
Configuration / runtime notes
/etc/lakekeeper/lakekeeper.env(a%config/conffile, so edits survive upgrades)and a systemd unit that is not enabled or started automatically. Requires an external PostgreSQL
15+ database; the admin sets
LAKEKEEPER__PG_DATABASE_URL_WRITE+LAKEKEEPER__PG_ENCRYPTION_KEYandruns the one-time
lakekeeper migratebefore starting. No hard PostgreSQL package dependency (it is anetwork service). Port configurable via
LAKEKEEPER__LISTEN_PORT(default 8181)./usr/lib/pgedge/coldfront/duckdb-extensions/v<ver>/linux_<arch>/;a shipped config sample documents pointing pg_duckdb at them via
duckdb.extension_directory(plus
allow_unsigned_extensions = trueandautoinstall_known_extensions = false).Testing
containers (el9 / bullseye→now noble / trixie as applicable), including signed RPMs validated with
rpm --checksigand signed SBOMs embedded in both formats.release download URL now uses the clean upstream version, not the
~-decorated package version).Follow-ups
ubuntu:resoluterequires a matchingresolutesuite on apt.pgedge.com and in the apt-repo-builderreprepro config for the DEB push to land.