Skip to content

pg_tle deployment smoke test for extension_drop (chained on cat_tools) - #21

Merged
jnasbyupgrade merged 1 commit into
masterfrom
pg-tle-ci
Aug 10, 2026
Merged

pg_tle deployment smoke test for extension_drop (chained on cat_tools)#21
jnasbyupgrade merged 1 commit into
masterfrom
pg-tle-ci

Conversation

@jnasbyupgrade

@jnasbyupgrade jnasbyupgrade commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Adds a pg-tle-test CI job proving extension_drop can be deployed with zero filesystem footprint via pg_tle (AWS's Trusted Language Extensions — a database-backed catalog for installing an extension with no .control file on disk; relevant for RDS/Aurora-style managed deployments). This is a third deployment dimension, orthogonal to the fresh/update/existing testing already in test/install: not how the extension got there across time, but filesystem-installed vs. registered purely through pg_tle's catalog.

extension_drop requires cat_tools (.control's requires = 'cat_tools'), so unlike a leaf extension's pg_tle test (e.g. cat_tools's own pg-tle-test job, the reference this is modeled on), CREATE EXTENSION extension_drop CASCADE only resolves cleanly if cat_tools is also already registered as a pg_tle extension — otherwise the CASCADE either fails outright or silently resolves cat_tools from a stray filesystem install, defeating the point of the test. The job registers, in order, pg_tlecat_tools (cloned at the same CAT_TOOLS_GIT_REF the Makefile's own filesystem cat_tools target pins to, read via make print-CAT_TOOLS_GIT_REF so the two never drift) → extension_drop, all against template1 before any database that needs them is created.

bin/assert_fs_clean (snapshot/verify subcommands, diffing *.control files against a pre-pg_tle baseline) is modeled on cat_tools's script of the same name: a stray filesystem control file silently wins over a pg_tle registration of the same name, so this checks that nothing landed on disk, bracketing every step that could plausibly write one.

Explicitly out of scope for now:

  • The update path via pg_tle — extension_drop has no prior released version to update from yet.
  • Binary pg_upgrade of a pg_tle-deployed extension — a separate, heavier concern from a fresh-install smoke test.

PG matrix is the intersection of two independently-moving ranges: extension_drop's own tested range and pg_tle 1.5.2's supported PostgreSQL range (12–18, dropped PG11 — see pgxntool/pgtle_versions.md); 18 is left out since the existing test job doesn't cover it either.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: dac40f40-acf8-4e94-947c-71eaa6439ec4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@jnasbyupgrade
jnasbyupgrade force-pushed the test-install-foundation branch 2 times, most recently from 43749f2 to 187577a Compare August 6, 2026 17:13
@jnasbyupgrade
jnasbyupgrade force-pushed the test-install-foundation branch from 3fcef44 to 8f4e50d Compare August 6, 2026 21:05
@jnasbyupgrade
jnasbyupgrade force-pushed the test-install-foundation branch from 8f4e50d to 67855c7 Compare August 6, 2026 23:03
@jnasbyupgrade
jnasbyupgrade force-pushed the test-install-foundation branch 3 times, most recently from 2925fca to 03b0355 Compare August 7, 2026 22:14
@jnasbyupgrade
jnasbyupgrade changed the base branch from test-install-foundation to master August 9, 2026 17:44
@jnasbyupgrade
jnasbyupgrade marked this pull request as ready for review August 9, 2026 17:49
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

Claude finished @jnasbyupgrade's task in 6m 30s —— View job


Code Review

  • Gather context (PR diff, CLAUDE.md, referenced files)
  • Review .github/workflows/ci.yml changes
  • Review bin/assert_fs_clean script
  • Cross-check against actual CI run
  • Post review feedback

Summary

This is a well-designed, carefully-reasoned addition. I traced the dependency chain through pgxntool/base.mk (run-pgtlepgtleall control.mk, which notably does not pull in this repo's own deps: cat_tools target — so the chained registration genuinely can't fall back to a filesystem install), verified the extension_drop__add/get/remove calls in sql/extension_drop.sql:172-236 match the signatures exercised in the smoke test, and confirmed print-%/print-PGXNVERSION (pgxntool/base.mk:609) is the documented, intended interface rather than an internal detail being relied on. bin/assert_fs_clean's baseline/diff logic (snapshot before the pg_tle build, verify after, excluding only pg_tle.control) is sound.

I also checked actual CI (run 31327195286): all six 🧩 pg_tle {12..17} legs are green, along with the pre-existing test/lint legs and all-checks-passed. That empirically confirms the parts I couldn't verify from the diff alone — e.g. apt-get install -y flex bison libkrb5-dev (ci.yml:140) succeeding without a preceding apt-get update, and the container user having sufficient privileges for make install, editing postgresql.conf, and pg_ctlcluster restart.

Two minor, non-blocking observations:

  1. Makefile's CAT_TOOLS_SKIP_INSTALL (pre-existing, from test-install-foundation, not touched here) appears unused by this job — run-pgtle's dependency chain (pgtle: all control.mk ...) never reaches deps/install, so cat_tools's real filesystem-install path is never invoked here regardless. That's fine (assert_fs_clean proves the filesystem stays clean either way), but worth a quick confirmation of whether that variable was meant to be wired into this job or is reserved for a different caller — otherwise it's dead code.
  2. actions/checkout@v5 in the new pg-tle-test job (ci.yml:129) vs. @v7 in the test job — a pre-existing inconsistency in this repo (the lint job also uses v5), not introduced by this PR, but worth aligning at some point.

Nothing here blocks merge — the design matches the proven cat_tools reference pattern, the chained-dependency registration order is correct, and CI is green.

…at_tools)

Proves extension_drop can be deployed with zero filesystem footprint via
pg_tle (AWS's Trusted Language Extensions), the same fresh-install proof
cat_tools' own pg-tle-test job (PR #47) does for itself -- but extension_drop
REQUIRES cat_tools, so this job also registers cat_tools as a pg_tle
extension first (a leaf extension's pg_tle test never has to solve chained
dependency resolution). Every step that could write an extension file to
disk is bracketed by a new bin/assert_fs_clean check (modeled on cat_tools'
script of the same name), since a stray filesystem .control file silently
wins over a pg_tle registration of the same name.

Verified end to end locally: built pg_tle 1.5.2 from source, registered
pg_tle+cat_tools+extension_drop against template1, CASCADE-installed
extension_drop in a smoke database, and called extension_drop__add/__get/
__remove against a real installed extension -- all with zero filesystem
control files present throughout (confirmed via bin/assert_fs_clean).

Explicitly out of scope: the update path via pg_tle (extension_drop has no
prior released version to update from yet) and binary pg_upgrade of a
pg_tle-deployed extension -- both noted as follow-up work in the PR.
@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown

Claude finished @jnasbyupgrade's task in 2m 14s —— View job


Code Review

  • Gather context (PR diff, CLAUDE.md, referenced files)
  • Review .github/workflows/ci.yml changes
  • Review bin/assert_fs_clean script
  • Cross-check against actual CI run
  • Post review feedback

Summary

Re-verified this PR (no new commits since the last review — same diff: .github/workflows/ci.yml +155/-1, bin/assert_fs_clean +85/-0). The design is sound and matches the proven cat_tools pg-tle-test reference pattern.

Verified independently this pass:

  • Dependency-chain correctness: traced run-pgtlepgtleall control.mk $(PGXNTOOL_CONTROL_FILES) in pgxntool/base.mk:450-480 — this chain never reaches deps/cat_tools/install (Makefile:74-76), so the ordered registration (pg_tlecat_toolsextension_drop, all against template1) genuinely can't fall back to a filesystem install. The CASCADE in ci.yml's Install extension_drop purely via pg_tle step can only resolve through pg_tle's catalog.
  • Function signatures match: extension_drop__add(extension_name, sql), extension_drop__get(extension_name), extension_drop__remove(extension_name) (sql/extension_drop.sql:172-236) line up exactly with the smoke test's calls (extension_drop__add('pg_tle', 'SELECT 1'), etc.). __add/__get/__remove all invoke extension_drop__sanity_assert(), which requires the named extension to actually exist in pg_extension (sql/extension_drop.sql:120-123) — since pg_tle is genuinely CREATE EXTENSION'd in that database, this is a real functional check, not just a successful INSERT.
  • bin/assert_fs_clean: the snapshot/verify baseline-diff logic (bin/assert_fs_clean:42-66) is correct — comm -13 catches anything new relative to baseline, explicitly excluding only pg_tle.control (the one legitimate filesystem install), and the EXIT trap correctly bakes in the tempfile path via double-quoting so it survives function return under set -e.
  • CI confirmation: checked run 31420461363 directly — all six 🧩 pg_tle {12..17} legs, the pre-existing test/lint/pg-upgrade-test legs, and all-checks-passed are green.

Same two minor, non-blocking observations as the prior pass on this PR (still unresolved, still not blocking):

  1. Makefile's CAT_TOOLS_SKIP_INSTALL (pre-existing, from test-install-foundation, not touched here) is unused by this job — confirmed again via the run-pgtle chain trace above. Worth a quick check on whether it was meant to be wired in here or reserved for a different caller, otherwise it's dead code.
  2. actions/checkout@v5 in pg-tle-test (ci.yml:129) vs. @v7 in the test job — pre-existing inconsistency (the lint job also uses v5), not introduced by this PR, worth aligning eventually.

Nothing blocks merge — chained-dependency registration order is correct, filesystem-isolation proof is sound, and CI is green.

@jnasbyupgrade jnasbyupgrade changed the title ci: pg_tle deployment smoke test for extension_drop (chained on cat_tools) pg_tle deployment smoke test for extension_drop (chained on cat_tools) Aug 10, 2026
@jnasbyupgrade
jnasbyupgrade merged commit cdbf348 into master Aug 10, 2026
20 checks passed
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.

1 participant