Skip to content

fix(release): make deb/rpm install paths correct, gate them in CI, drop apk (musl-incompatible)#464

Merged
Matovidlo merged 8 commits into
mainfrom
martinvasko-fix-apk-index-untrusted
Jun 24, 2026
Merged

fix(release): make deb/rpm install paths correct, gate them in CI, drop apk (musl-incompatible)#464
Matovidlo merged 8 commits into
mainfrom
martinvasko-fix-apk-index-untrusted

Conversation

@Matovidlo

@Matovidlo Matovidlo commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Started as the apk-indexing follow-up to #449, but validating the release pipeline end-to-end on dev tags surfaced that the entire Linux-package publish path had never run end-to-end and hid several latent bugs. This PR fixes the deb/rpm paths to actually install, adds the CI gate that was missing, and drops apk (the frozen binary can't run on Alpine).

What changed

  • Drop apk packaging. The apk repo signed/indexed/installed correctly, but the PyInstaller binary is glibc-linked (/lib64/ld-linux-x86-64.so.2) and won't execute on musl Alpine (sh: kbagent: not found after a clean install). Rather than ship an unrunnable package, apk is removed from nfpm formats, build_packages.sh, index.sh, the package/publish key wiring, and the test legs. Supporting Alpine would require a separate musl freeze — out of scope.
  • deb multi-arch index (dpkg-scanpackages -m): without it, same-version amd64/arm64 debs collide ("is repeat; ignored") and one arch is dropped, leaving apt with no candidate.
  • deb /./ 404: Filename: ./pkg.deb made apt fetch <repo>/deb/./pkg.deb; S3-behind-CloudFront treats the literal /./ as a missing key. Strip the leading ./.
  • CloudFront stale metadata: the dist domain has a 24h default TTL and S3 uploads carry no Cache-Control, so stable index paths (Packages, repodata/*) served the previous version after a publish (dnf installed dev2 when dev3 was live). Index/metadata now uploaded no-cache; packages stay immutable (version-unique names). Within existing s3:* perms — no CloudFront invalidation permission needed.
  • Pipeline gating for dev tags: pre-release tags now run publish-s3 + test-install under a separate keboola-cli2-dev S3 prefix (wiped per run, serialized via a concurrency group), so release-pipeline changes can be validated on a dev tag without a prod release or new secrets. PyPI/Homebrew/Chocolatey/WinGet stay prod-only.
  • test-install now exercises real client installs (apt + rpm) trusting the published keys and verifying --version — the gate whose absence let all the above ship undetected.

Validation

v0.65.2.dev5: publish-s3 ✅ and test-install ✅ (apt ✅, rpm ✅; Homebrew correctly skipped on pre-release).

Change type

Bug fix — release/CD (Linux package publishing).

Impact analysis

  • build/package/nfpm.yaml, build/package/linux/{build_packages,index}.sh: deb/rpm only; apk removed.
  • .github/workflows/release-kbagent.yml: dev-prefix routing + wipe + concurrency on publish-s3; apt/rpm test-install legs; APK key wiring removed.
  • No source/runtime changes. deb/rpm/brew/choco/winget unaffected for real releases.

Test plan

Validated end-to-end via v0.65.2.dev1..dev5 dev-tag pipeline runs (dev S3 prefix, no prod impact). apt + rpm client installs verify signature + version.

Rollback plan

Revert of this PR.

Post release support plan

Watch publish-s3 + test-install on the next real tag.

The .apk packages are signed by our own key via nfpm (apk.signature in
nfpm.yaml). The throwaway alpine:3 container used to build the index only
trusts Alpine's official keys, so 'apk index' rejected our packages with
'UNTRUSTED signature' (exit 99). Indexing only reads package metadata, so
pass --allow-untrusted to skip the trust check; the index is still signed
by abuild-sign and clients still verify packages against the published key.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Matovidlo

Copy link
Copy Markdown
Contributor Author

@claude review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes APK repository indexing during release publishing by allowing apk index (run inside an Alpine container) to process self-signed .apk packages created by nfpm, preventing failures like UNTRUSTED signature during real tag releases.

Changes:

  • Add --allow-untrusted to the apk index invocation used to generate APKINDEX.tar.gz in the Alpine container.
  • Document why the flag is needed (container trusts only Alpine’s default keys; indexing only reads metadata).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…ontainer

Mirrors the proven keboola-as-code apk setup. Three names must agree for both
apk index (build time) and apk add (client side) to verify:

  - nfpm.yaml: add apk.signature.key_name=keboola so packages are signed as
    .SIGN.RSA.keboola.rsa.pub (was unset → nfpm default, mismatched the
    published pubkey).
  - index.sh: import APK_KEY_PUBLIC as /etc/apk/keys/keboola.rsa.pub inside the
    Alpine container so 'apk index' trusts the self-signed packages (fixes the
    'UNTRUSTED signature' exit-99 crash) WITHOUT --allow-untrusted; name the
    abuild-sign key keboola.rsa so the index signature matches the published
    keboola.rsa.pub clients download.

Replaces the earlier --allow-untrusted band-aid, which would have let indexing
pass but left clients unable to verify (index signed .SIGN.RSA.key.rsa.pub vs
published keboola.rsa.pub).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Matovidlo Matovidlo changed the title fix(release): apk index --allow-untrusted for self-signed packages fix(release): consistent 'keboola' apk key naming so index + clients verify Jun 23, 2026
…k/rpm install tests

The apk path failed at successive stages because it had never run end-to-end.
Fixes the remaining gaps so a dev tag can validate the whole chain:

- index.sh: build the apk repo PER-ARCH (apk/<arch>/APKINDEX.tar.gz; amd64->x86_64,
  arm64->aarch64) — apk clients fetch <repo>/<arch>/APKINDEX.tar.gz, so the previous
  flat index was never installable. Import the pubkey into the container so packages
  are trusted (no --allow-untrusted); publish keboola.rsa.pub at the apk root.
- release-kbagent.yml: pre-releases now publish under a separate dev prefix
  (keboola-cli2-dev) and run publish-s3 + test-install, so a dev tag exercises the
  real signing + client install without touching prod repos. Homebrew/Chocolatey/
  WinGet/PyPI stay prod-only.
- test-install: add apk (Alpine) and rpm (Fedora) client-install legs that trust the
  published keys and verify --version; brew leg is prod-only. This is the CI gate that
  would have caught every apk failure before a tag was cut.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Matovidlo Matovidlo marked this pull request as draft June 23, 2026 19:11
Matovidlo and others added 2 commits June 23, 2026 21:23
…nt test legs

dpkg-scanpackages dedups on package name+version IGNORING architecture, so the
same-version amd64+arm64 debs collided ('is repeat; ignored') and only one arch
landed in Packages — apt on the other arch had no candidate. -m/--multiversion
emits a stanza per file. Also make the apk/rpm/brew test-install legs run with
!cancelled() so one run surfaces all client-install results instead of aborting
at the first failing leg.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…naming, wipe dev prefix

Two install-time 404s, both unrelated to apk signing (which works):
- deb: dpkg-scanpackages emits 'Filename: ./pkg.deb', so apt fetches <repo>/deb/./pkg.deb;
  S3/CloudFront treats the literal /./ as a missing key -> 404. Strip the leading ./.
- apk: clients fetch <repo>/<arch>/<name>-<version>.apk, but nfpm names the file
  keboola-cli2_<ver>_linux_<arch>.apk -> 404. Rename to apk's convention on copy.
Also wipe the keboola-cli2-dev prefix on pre-release runs so dev tags start clean and
version resolution isn't confused by leftovers (guarded to the -dev suffix; never prod).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@padak padak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of #464 — fix(release): consistent 'keboola' apk key naming so index + clients verify

Generated by kbagent-pr-reviewer subagent. Verdict and findings below
are advisory; the human author retains every veto. CI-coverable issues
(lint, format, tests) are confirmed via make check, not duplicated here.

Summary

This PR fixes a broken apk signing chain introduced in the #449 follow-up: nfpm was signing packages with a default key name derived from the key file's basename (apk.key.SIGN.RSA.apk.key.rsa.pub), while clients expected the published keboola.rsa.pub pubkey. The three-point fix — key_name: keboola in nfpm.yaml, renaming the abuild-sign key to /keboola.rsa, and importing the pubkey into the Alpine container — closes the verification gap cleanly and mirrors the proven keboola-as-code pattern 1:1. The PR also adds a dev-prefix routing for pre-release tags, a wipe step for clean-slate dev runs, and new apk + rpm test-install legs. The core apk fix is mechanically correct and the shell logic is sound. Two nits, zero blocking issues.

Verdict

  • Verdict: APPROVE
  • Blocking findings: 0
  • Non-blocking findings: 0
  • Nits: 2

Blocking findings

(none)

Non-blocking findings

(none)

Nits

  • [NIT-1] build/package/linux/index.sh:20-23 — The comment "publish-s3 only runs on real (non-pre-release) tags, where the repo + key must exist for the downstream test-install job" is now stale. This PR changes the publish-s3 job's if: condition to startsWith(github.ref, 'refs/tags/'), so pre-release tags also run that job (writing to the dev prefix). The guard itself is functionally correct — DEB_KEY_PRIVATE is required in the release environment for any tag — but the comment misrepresents when the script is called and will confuse the next reader. Suggest replacing the second sentence with "Fail loudly: if DEB_KEY_PRIVATE is absent the signing environment is misconfigured."

  • [NIT-2] release-kbagent.yml:83 — The debug echo line uses a command-substitution inside a double-quoted string to conditionally append -dev: echo "version=$VERSION prerelease=$PRE prefix=${S3_PREFIX}$([ "$PRE" = true ] && echo -dev)". The subshell expression is difficult to read and non-obvious that it's the debug log (not the GITHUB_OUTPUT line above it). An explicit if/else for the debug line — or a pre-computed $EFFECTIVE_PREFIX variable — would be clearer. No correctness issue; the >> "$GITHUB_OUTPUT" redirect correctly applies to the full compound command on line 82 (verified with bash -c test).

Verification log

  • gh pr view 464 --json title,body,files,additions,deletions,baseRefName,headRefName,labels,state → 3 files, +113/-22, fix(release): conventional prefix ✓, state OPEN ✓
  • git rev-parse --abbrev-ref HEAD after gh pr checkout 464martinvasko-fix-apk-index-untrusted ✓ (working tree on PR branch)
  • bash -n build/package/linux/index.sh → exit 0 ✓
  • python3 -c "import yaml; yaml.safe_load(open('.github/workflows/release-kbagent.yml'))"YAML parse OK
  • Shell operator precedence test: if [ "$PRE" = true ]; then echo "PUBLISH_PREFIX=X-dev"; else echo "PUBLISH_PREFIX=X"; fi >> "$GITHUB_OUTPUT" — the >> file redirect applies to the entire compound if…fi command in bash; both PRE=true and PRE=false write correctly to $GITHUB_OUTPUT. Verified by local simulation. ✓
  • APK filename version extraction (base="${f##*/}"; ver="${base#keboola-cli2_}"; ver="${ver%_linux_${deb_arch}.apk}"): input keboola-cli2_0.65.1_linux_amd64.apkver=0.65.1 → renamed to keboola-cli2-0.65.1.apk
  • Key naming chain: nfpm key_name=keboola.SIGN.RSA.keboola.rsa.pub per nfpm source convention; abuild-sign -k /keboola.rsa.SIGN.RSA.keboola.rsa.pub (basename + .pub); client fetches keboola.rsa.pub — three names agree ✓
  • make check → 4177 passed, 8 skipped, 0 failed ✓ (no Python files touched; this is expected)
  • Behavior reproduction: publish-s3 only runs on real tag pushes (pre-releases now included, writing to ${S3_PREFIX}-dev); test-install has if: always() && needs.publish-s3.result == 'success' so it runs even when homebrew is skipped for pre-releases ✓
  • Plugin synchronization map: not applicable — no CLI command surface changes in this PR ✓
  • Layer violation scan: no Python changes — 3-layer rules not applicable ✓
  • Security: no tokens in new code; WORK=$(mktemp -d); trap 'rm -rf "$WORK"' EXIT correctly cleans up all key material including $WORK/keboola.rsa ✓; key mounted read-only in Docker (-v ... :ro) ✓

Open questions for the author

(none)

Matovidlo and others added 2 commits June 24, 2026 07:00
The dist domain is CloudFront-fronted with default_ttl 24h and S3 uploads carry no
Cache-Control, so the stable index paths (deb/Packages, apk/<arch>/APKINDEX.tar.gz,
rpm/repodata/*) were cached ~24h — clients installed the PREVIOUS version after a
publish (dnf pulled dev2 when dev3 was live). Upload index/metadata with
Cache-Control: no-cache (effective TTL ~min_ttl=1s) and packages as immutable
(version-unique names). Needs only s3:PutObject (already granted) — no CloudFront
invalidation permission required.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The apk repo signed/indexed/installed correctly, but the PyInstaller binary is
glibc-linked (interpreter /lib64/ld-linux-x86-64.so.2) and won't execute on musl
Alpine ('sh: kbagent: not found' after a successful install). Rather than ship a
package that can't run on its target, drop apk: remove it from nfpm formats,
build_packages.sh, the index.sh publisher, the package-linux/publish-s3 APK key
wiring, and the apk test-install leg. deb/rpm/brew/choco/winget unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

This comment was marked as resolved.

…clearer debug line

- Add a concurrency group to publish-s3 keyed by PUBLISH_PREFIX (cancel-in-progress:
  false) so concurrent tag runs can't wipe/overwrite a shared repo prefix mid-publish
  (Copilot).
- Correct the now-stale comments: the DEB_KEY guard in index.sh and the version-job
  IS_PRERELEASE doc both said publish-s3 is real-tags-only; it now runs on every tag
  (dev prefix for pre-releases) (Padak NIT-1, Copilot).
- Replace the command-substitution debug echo with a precomputed PUBLISH_PREFIX var
  (Padak NIT-2).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Matovidlo Matovidlo changed the title fix(release): consistent 'keboola' apk key naming so index + clients verify fix(release): make deb/rpm install paths correct, gate them in CI, drop apk (musl-incompatible) Jun 24, 2026
@Matovidlo

Copy link
Copy Markdown
Contributor Author

Thanks @padak — both nits addressed (and the scope has since grown; title/description updated):

  • NIT-1 (index.sh stale guard comment): fixed — the comment claimed publish-s3 is real-tags-only; it now runs on every tag (dev prefix for pre-releases). Rewrote it to your suggestion: "Fail loudly: the key is required in the release environment for any tag." Also corrected the matching version-job IS_PRERELEASE doc.
  • NIT-2 (release-kbagent.yml:83 command-substitution debug echo): replaced the inline $([ "$PRE" = true ] && echo -dev) with a precomputed PUBLISH_PREFIX var used for both the GITHUB_OUTPUT line and the debug echo — much clearer.

Also picked up Copilot's catch and added a concurrency group on publish-s3 keyed by PUBLISH_PREFIX (queue, don't cancel) so concurrent tag runs can't clobber a shared repo prefix.

Heads-up on scope drift since your APPROVE: validating on dev tags revealed the apk binary can't run on musl Alpine (glibc PyInstaller build), so this PR now drops apk entirely and instead fixes the deb/rpm install paths (multi-arch -m, /./ 404, CloudFront no-cache metadata) + adds apt/rpm test-install legs. Validated green on v0.65.2.dev5. Given the changed scope, a re-glance would be appreciated before merge.

This comment was marked as resolved.

@Matovidlo Matovidlo marked this pull request as ready for review June 24, 2026 06:49
@Matovidlo Matovidlo requested a review from padak June 24, 2026 06:49

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Open in Devin Review

Comment thread .github/workflows/release-kbagent.yml

@padak padak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. The apk key-naming chain is consistent — keboola.rsa.pub agrees across nfpm (key_name=keboola), the abuild-sign key (/keboola.rsa), and the pubkey imported into the Alpine container, which is the precondition for client-side index verification. Only 2 non-blocking nits (stale comment in index.sh, debug-echo clarity in release-kbagent.yml) — see the detailed review comment.

@Matovidlo Matovidlo merged commit 407ddd3 into main Jun 24, 2026
6 checks passed
@Matovidlo Matovidlo deleted the martinvasko-fix-apk-index-untrusted branch June 24, 2026 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants