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 13 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
|
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)
📝 WalkthroughWalkthroughThis PR adds a compactor binary, shared build helpers, packaging assets for multiple components, and a tag-routed release workflow that builds, verifies, packages, publishes, and reports release artifacts. ChangesPackaging and Release Pipeline
Estimated code review effort: 4 (Complex) | ~75 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Up to standards ✅🟢 Issues
|
# Conflicts: # .github/workflows/release.yml
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
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.