[ci] Refactor RPM nfpm configs for reuse#5179
Conversation
maru-ava
left a comment
There was a problem hiding this comment.
My understanding of the split from #5145 was that this first PR would prepare the existing packaging path for the addition of a revised DEB packaging workflow without regressing current behavior. However, the current workflow-sharing approach appears to change the bootstrap behavior for historical-tag builds.
f2c029c to
402dd0c
Compare
402dd0c to
d336cf3
Compare
maru-ava
left a comment
There was a problem hiding this comment.
Good progress! Most of the comments inline are relatively minor. My main concern would be the coherency of this PR - I'm not sure it makes sense to add support for debian packaging (including signing) in this refactor PR. Can there be a cleaner split between this PR enabling resuse of packaging machinery and 5180 using those changes for deb support?
|
This PR continues to block #5180, and I've been waiting since my last review for you to respond to the comments. |
RPM refactor. After rebasing this branch onto the post-excision tip of #5179, the hooks need to return as commits owned by #5180 so they survive the eventual merge into master. build-package.sh: source lib-build-${fmt_lower}.sh on FORMAT=DEB, add the DEB arm to the GPG_KEY_FILE case, configure gpg-agent via setup_deb_gpg_agent before setup_gpg, cache the passphrase via cache_deb_gpg_passphrase for non-interactive dpkg-sig signing, and invoke sign_deb_package post-nfpm (nfpm's openpgp signatures are incompatible with dpkg-sig --verify). lib-validate-common.sh: re-add the DEB arm of detect_host_arch's format dispatch (x86_64 -> amd64, aarch64/arm64 -> arm64). The corresponding helper functions are defined in lib-build-deb.sh. Verified locally: task -t .github/packaging/Taskfile.yml test-build-debs passes end-to-end. Both .debs built, dpkg-sig signatures verify (GOODSIG), and install + smoke test pass in fresh ubuntu:22.04 (jammy) and ubuntu:24.04 (noble) containers.
397af7d to
71fe97b
Compare
RPM refactor. After rebasing this branch onto the post-excision tip of #5179, the hooks need to return as commits owned by #5180 so they survive the eventual merge into master. build-package.sh: source lib-build-${fmt_lower}.sh on FORMAT=DEB, add the DEB arm to the GPG_KEY_FILE case, configure gpg-agent via setup_deb_gpg_agent before setup_gpg, cache the passphrase via cache_deb_gpg_passphrase for non-interactive dpkg-sig signing, and invoke sign_deb_package post-nfpm (nfpm's openpgp signatures are incompatible with dpkg-sig --verify). lib-validate-common.sh: re-add the DEB arm of detect_host_arch's format dispatch (x86_64 -> amd64, aarch64/arm64 -> arm64). The corresponding helper functions are defined in lib-build-deb.sh. Verified locally: task -t .github/packaging/Taskfile.yml test-build-debs passes end-to-end. Both .debs built, dpkg-sig signatures verify (GOODSIG), and install + smoke test pass in fresh ubuntu:22.04 (jammy) and ubuntu:24.04 (noble) containers.
RPM refactor. After rebasing this branch onto the post-excision tip of #5179, the hooks need to return as commits owned by #5180 so they survive the eventual merge into master. build-package.sh: source lib-build-${fmt_lower}.sh on FORMAT=DEB, add the DEB arm to the GPG_KEY_FILE case, configure gpg-agent via setup_deb_gpg_agent before setup_gpg, cache the passphrase via cache_deb_gpg_passphrase for non-interactive dpkg-sig signing, and invoke sign_deb_package post-nfpm (nfpm's openpgp signatures are incompatible with dpkg-sig --verify). lib-validate-common.sh: re-add the DEB arm of detect_host_arch's format dispatch (x86_64 -> amd64, aarch64/arm64 -> arm64). The corresponding helper functions are defined in lib-build-deb.sh. Verified locally: task -t .github/packaging/Taskfile.yml test-build-debs passes end-to-end. Both .debs built, dpkg-sig signatures verify (GOODSIG), and install + smoke test pass in fresh ubuntu:22.04 (jammy) and ubuntu:24.04 (noble) containers.
maru-ava
left a comment
There was a problem hiding this comment.
Minor comments, my main concern would be ensuring that actionable issues identified in this review that you think are more appropriate for a follow-up are tracked to resolution.
You may also want to consider some of the comments from the most recent review of #5180 as relevant to this PR e.g. #5180 (comment)
8bf1586 to
6fa1034
Compare
maru-ava
left a comment
There was a problem hiding this comment.
Inline comments are mechanical in nature, no substantive issues remaining.
7eebd8c to
ec91c03
Compare
- build-rpm-release.yml: clarify in the comment block that the two- step overlay is split because actions/checkout and the copy cannot be combined, so the comment now visibly applies to both steps. - build-package.sh + Taskfile.yml: rename FORMAT/fmt_lower to PKG_FORMAT/pkg_format_lower. Bare FORMAT collides with nfpm's own field name, and fmt_lower read like a function rather than a derived value. - lib-build-common.sh: annotate the SC2154 disable next to the git_commit echo so a reader sees it comes from the sourced git_commit.sh. - lib-build-common.sh: drop the SUBNET_EVM_BINARY intermediate; pass BINARY_PATH directly to subnet-evm's build script. - lib-build-common.sh: replace exit with return in build_binary's unknown-package case and resolve_subnet_evm_vm_id's empty-result case. Callers' set -e propagates the non-zero return. - lib-build-common.sh: hoist "Ava Labs" / "security@avalabs.org" to PACKAGER_NAME / PACKAGER_EMAIL constants instead of repeating the literals across the changelog template, ephemeral GPG key generation, and key export. Verified locally: task -t .github/packaging/Taskfile.yml test-build-rpms passes end-to-end (RPMs built, GPG signatures verified, install + smoke test green in a fresh rockylinux:9 container).
Drops the per-package ${*_BINARY} envs mapping in build-package.sh;
both templates reference ${BINARY_PATH} which the library already sets.
Extract DEFAULT_VM_* into graft/subnet-evm/scripts/default-vm-data.sh — a side-effect-free file safe to source from anywhere. Replaces the grep-based lookup in resolve_subnet_evm_vm_id with a direct source.
Replace doc-comment with runtime guard on REPO_ROOT at file scope, and add NFPM_CHANGELOG / NFPM_SIGNING_KEY guards at the top of generate_changelog and setup_gpg respectively. Each fires with an explicit message if the caller forgets to set the variable.
- Bump actions/checkout@v4 -> @v5 on the overlay step, restoring parity with the initial checkout step and master. - Drop the elaborate header comment in default-vm-data.sh; the file is self-evidently pure data. - Remove redundant 'expand: true' from both nfpm configs — envsubst preprocesses the YAML, so by the time nfpm reads dst:, there are no ${VAR} references left to expand. - Update stale path in subnet-evm-rpm.yml comment to point at default-vm-data.sh (where DEFAULT_VM_ID now lives).
- Document where avalanchego_path comes from (set by scripts/constants.sh, sourced in init_build_env). - Sanity-check the generated changelog actually contains the expected semver line; catches silent heredoc substitution failures. - assert_files_exist now collects all missing files and reports them together instead of failing on the first one.
RPM_GPG_KEY_FILE -> GPG_KEY_FILE.
NFPM_RPM_PASSPHRASE -> GPG_KEY_PASSPHRASE (external);
build-package.sh translates this to the nfpm-required
NFPM_${PKG_FORMAT}_PASSPHRASE before invoking nfpm.
Drop the duplicate env-var docstring block from build-package.sh (the asserts and PKG_FORMAT comment now self-document). Move run_nfpm_package's Args block inline above each local declaration.
Drop the Dockerfile.rpm default; Taskfile entrypoints already pass DOCKERFILE explicitly, so the default just hides accidental misuse.
${PKG_FORMAT}-GPG-KEY-avalanchego -> GPG-KEY-avalanchego.
Both RPM and DEB share the same signing key; the format prefix
was cosmetic.
Drop the marker-file + secret_key_file_is_usable check; reuse the existing ephemeral key when the file exists, generate a new one otherwise. The marker/usability machinery added state without materially helping the multi-build flow (where two RPM builds in the same task invocation must share one signing key).
Taskfile now passes PACKAGE_ARCH to validate-rpms (defaulting to PACKAGING_HOST_ARCH). validate-rpm.sh asserts the var is set instead of duplicating the uname mapping; lib-validate-common.sh loses detect_host_arch entirely.
Mirrors the GPG_PRIVATE_KEY condition in the setup step; secrets are unavailable on PR builds, so the passphrase shouldn't be referenced unconditionally.
workflow-setup-packaging.sh accepts a new RELEASE flag; when set and GPG_PRIVATE_KEY is empty, the script exits 1 instead of publishing an empty key path. The workflow passes RELEASE=true on the same condition that gates GPG_PRIVATE_KEY (github.event_name != 'pull_request'), preventing silent ephemeral-key fallback on tag pushes / workflow_dispatch runs.
Pass GPG_KEY_PASSPHRASE through the task's env block and use Docker's value-less -e form. Whitespace and shell metachars in the passphrase no longer corrupt the command or expose secret content via argv. Applied to both RPM build tasks.
Source the file inside a {} command group whose stdout is sent to
stderr; the explicit echo of ${DEFAULT_VM_ID} is the only thing the
outer $() captures. Prevents the v1.14.1-style 'Using branch: …'
line from being concatenated into SUBNET_EVM_VM_ID when overlaid
packaging scripts process an older tag via workflow_dispatch.
Every caller (Taskfile RPM tasks here, DEB tasks in #5180) passes PKG_FORMAT explicitly. Replace the RPM default with a `${VAR:?…}` assert so the script fails fast on misuse, mirroring the DOCKERFILE treatment in 37f63aa. Addresses https://github.com/ava-labs/avalanchego/pull/5179/changes#r3266314592
After removing detect_host_arch in ea5364a, lib-validate-common.sh held a single assert_files_exist function — not enough of a boundary to justify a separate file. Move it into lib-build-common.sh (which the validator already sources) and drop the now-unused file plus its second source line in validate-rpm.sh. Also document the cross-container key-sharing constraint that motivates the ephemeral GPG key reuse branch in setup_gpg: the two RPM builds run as separate docker invocations and share the bind- mounted key so the validator's single exported public key verifies both packages. Addresses - https://github.com/ava-labs/avalanchego/pull/5179/changes#r3276193371 - https://github.com/ava-labs/avalanchego/pull/5179/changes#r3267417392
- Convert the "Skipping GPG verification" else branch to a fatal error. The current pipeline always produces a signed key (ephemeral for PR/local, provided for releases per the release-key gate in dec7b29), so a missing GPG-KEY-avalanchego is now a real anomaly. - Drop the "Required/Optional env vars" header block. The ${VAR:?…} assert messages now carry per-var role descriptions inline. This also removes the stale "PACKAGE_ARCH defaults to host" claim (PACKAGE_ARCH was promoted to required in ea5364a). Addresses - https://github.com/ava-labs/avalanchego/pull/5179/changes#r3276233720 - https://github.com/ava-labs/avalanchego/pull/5179/changes#r3276331218 - https://github.com/ava-labs/avalanchego/pull/5179/changes#r3276336503
Add `${GITHUB_REF:?…}` / `${GITHUB_SHA:?…}` asserts so the script
fails fast when invoked outside GitHub Actions without supplying
them, mirroring the build-package.sh pattern from 43de524. Drop
the duplicate "Required/Optional env vars" header block; the
optional vars (GITHUB_OUTPUT, TAG_INPUT, GPG_PRIVATE_KEY, RELEASE)
keep a brief inline annotation above their ${VAR:-…} defaults.
Addresses https://github.com/ava-labs/avalanchego/pull/5179/changes#r3276254348
- Reword the comment block to make the replacement semantics explicit: the tag's .github/packaging tree (which may be stale or absent) is wholesale replaced with the workflow branch's version before running task test-build-rpms. - Remove `mkdir -p .github`. The directory is guaranteed to exist in the checked-out tree (the workflow yaml itself lives at .github/workflows/build-rpm-release.yml), and rm -rf .github/packaging removes only the subdir. Addresses - https://github.com/ava-labs/avalanchego/pull/5179/changes#r3276034324 - https://github.com/ava-labs/avalanchego/pull/5179/changes#r3276081744
- Drop the duplicate `# Args:` header docstring. The ${N:?…}
positional-arg asserts now carry per-arg roles inline, matching
the pattern from build-package.sh in 43de524.
- Shrink the arg surface from 4 to 3: caller passes the composed
subnet-evm plugin binary path directly. The smoke test no longer
needs to know how to join PLUGIN_DIR with SUBNET_EVM_VM_ID, and
validate-rpm.sh (which already resolves the VM ID) composes the
path inline.
Addresses
- https://github.com/ava-labs/avalanchego/pull/5179/changes#r3276183928
- https://github.com/ava-labs/avalanchego/pull/5179/changes#r3276202809
8a1f3c0 to
5f653ff
Compare
RPM refactor. After rebasing this branch onto the post-excision tip of #5179, the hooks need to return as commits owned by #5180 so they survive the eventual merge into master. build-package.sh: source lib-build-${fmt_lower}.sh on FORMAT=DEB, add the DEB arm to the GPG_KEY_FILE case, configure gpg-agent via setup_deb_gpg_agent before setup_gpg, cache the passphrase via cache_deb_gpg_passphrase for non-interactive dpkg-sig signing, and invoke sign_deb_package post-nfpm (nfpm's openpgp signatures are incompatible with dpkg-sig --verify). lib-validate-common.sh: re-add the DEB arm of detect_host_arch's format dispatch (x86_64 -> amd64, aarch64/arm64 -> arm64). The corresponding helper functions are defined in lib-build-deb.sh. Verified locally: task -t .github/packaging/Taskfile.yml test-build-debs passes end-to-end. Both .debs built, dpkg-sig signatures verify (GOODSIG), and install + smoke test pass in fresh ubuntu:22.04 (jammy) and ubuntu:24.04 (noble) containers.
PR #5179 (Thread #7, pushed as 7245fa9) shrank smoke-test.sh from 4 args to 3: the caller now passes the composed subnet-evm plugin binary path instead of the plugin dir + VM ID. validate-rpm.sh was updated in the same commit; validate-deb.sh was missed and kept the old 4-arg invocation. After rebasing onto PR #5179's tip the contract drift surfaced in build-deb-packages CI: positional-arg slip made $2 the plugins *directory*, and smoke-test.sh tried to exec it ("Is a directory", exit 126). Mirror the validate-rpm.sh fix.
Why this should be merged
Enables DEB packaging (follow-up PR) to reuse the same nfpm config template pattern. Currently nfpm configs hardcode container paths (
/build/build/...), which prevents reuse by DEB builds running in a different container. This refactor is a prerequisite for adding GPG-signed DEB packages.How this works
changelogandkey_filein nfpm configs with${NFPM_CHANGELOG}/${NFPM_SIGNING_KEY}env varsenvsubstpreprocessing inbuild-rpm.shbefore calling nfpm (nfpm doesn't expand env vars in top-level fields)gettextto RPM builder Dockerfile (providesenvsubst)DOCKERFILEparameter tobuild-builder-image.shso the DEB follow-up can point atDockerfile.debbuild/gpgin RPM workflow cleanup stepNo functional change to RPM output — the resolved paths are identical to the previous hardcoded values.
How this was tested
task packaging:test-build-rpmslocally on macOS (Docker): full build + signature verification + install + smoke test passed.github/packaging/**Need to be documented in RELEASES.md?
No