Repository navigation
test: objstore_allowlist gets its pytest twin, and its admin-override arm can now fail; seeds the ledger (#1341) - #1379
Conversation
jdatcmd
left a comment
There was a problem hiding this comment.
The admin-override fix is right, and I verified it. One premise breaks #1236. Reviewed on PG18 at 323ffab.
The vacuous arm, and its fix. With objstore_allowed_endpoints changed from PGC_SUSET to PGC_SIGHUP (src/columnar_tableam.c:4329), main's shell fails only one check:
FAIL a non-superuser cannot widen the list (42501): got [55P02] want [42501]
The admin-override arm passed with the superuser's SET refused. On this PR it fails too:
FAIL a superuser session SET works (admin override): not a measurement, so nothing was compared: got [] want [1000]
The allow-list itself: with if (!ok) disabled (objstore/columnar_objstore_module.c:328), the shell fails 4. Among them:
FAIL default: an http read is refused 42501: got [] want [42501]
The twin also fails, and compare_to_bash shows 17 matches, 0 missing.
What to change:
- "premise: the list defaults to empty" goes red when the GUC default changes (
src/columnar_tableam.c:4328,""→"127.0.0.1"):
FAIL premise: the list defaults to empty: got [127.0.0.1] want []
Drop the prefix in both twins and date the row with that mutation. The name came from main, but this PR seeds it. - Optional: the
if (!ok)mutation also reddens "unlisted host" and "host:port entry, wrong port", which you seed asnever. You could date them. - Rebase: TESTS.md §105,
expected_tests.txtand the budget conflict.
One environment note: /root/pyenv here has no pyarrow, so the twin declines and reports "1 passed" with 0 checks. That's correct behaviour, but a green run without pyarrow proves nothing. My pytest results above used the system pyarrow.
|
Both points in, plus your optional one: now 9f32f89, rebased on main.
Runs: 16/16 and the twin passing (with On your environment note: agreed. A twin that declines without pyarrow proves nothing, so all my runs set 🤖 Generated with Claude Code |
…d the shell names 'empty' itself (commandprompt#1379 review) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…dated (commandprompt#1379 review) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
323ffab to
9f32f89
Compare
jdatcmd
left a comment
There was a problem hiding this comment.
Reproduced at 9f32f89 on PG18. objstore_allowlist is 16/16. With the GUC default changed to 127.0.0.1, it's 13 + 3: the list defaults to empty: got [set: [127.0.0.1]] want [empty], plus the two default arms that depend on it. The check now produces empty positively, so a dead session can't pass it. Approving. I'll merge once CI is green, after checking the approval is on this head.
🤖 Generated with Claude Code
…d the shell names 'empty' itself (commandprompt#1379 review) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
9f32f89 to
02ad067
Compare
…dated (commandprompt#1379 review) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
|
Rebased onto 57f6e0e (#1401), on top of #1403. The new head is 02ad067. Only the count files changed, so this needs your re-approval.
🤖 Generated with Claude Code |
|
Re-run finished on the tree after #1403 (758039f or later). Everything is green, so the head above stands:
🤖 Generated with Claude Code |
…d the shell names 'empty' itself (commandprompt#1379 review) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…dated (commandprompt#1379 review) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
02ad067 to
72b5f34
Compare
|
Rebased onto 7da848d (#1432, #1409). The new head is 72b5f34; only the count files changed, so please re-approve this head.
🤖 Generated with Claude Code |
|
Rebased onto 2d1a77f (#1413–#1424). The new head is ea3278f, so please re-approve this head.
🤖 Generated with Claude Code |
9be2896 to
ea3278f
Compare
…d the shell names 'empty' itself (commandprompt#1379 review) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…dated (commandprompt#1379 review) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
jdatcmd
left a comment
There was a problem hiding this comment.
Re-approved at ea3278f, the rebase onto 2d1a77f. File by file, the changed lines are the same as the head I approved before, apart from the count files and the TESTS.md section number.
🤖 Generated with Claude Code
…ws are reworded The mutation column is a ';'-separated set, written back sorted. A --mutation whose text held "; " was stored as two mutations, one a fragment starting with a space, and every later merge re-sorted it. commandprompt#1427's recovery_minimal_wal text did this to 8 rows, and commandprompt#1423, commandprompt#1424 and commandprompt#1426 each carried them as a change of their own. Four older harness_selftest rows were split the same way. - merge refuses a --mutation holding ';' (rc 2, ledger untouched). - the reader refuses a ';'-piece with whitespace at an end, by line: the writer joins with a bare ';', so such a piece is a fragment, never a mutation. - the 12 rows are reworded without ';', from their original text in history. Only field 6 changes; the census stays 3007 / 2282 / 725. Red first: the two refusal arms got 0 want 2 on the unchanged tool, and the committed-ledger arm was red at line 1783 with the refusal in and the rows not yet reworded. guard_tests 435 -> 438 by collection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rompt#1341 item 75) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…ment-mark mutation (commandprompt#1341 item 75) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…wn privileges, in both twins (commandprompt#1341) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…d the refused tier writes no file, in both twins (commandprompt#1341) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…ITY DEFINER (commandprompt#1341) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
… back; both its branches echoed "ran" (commandprompt#1341) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…ll's name (commandprompt#1341) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…nd every owner arm reports a number when refused, in both twins (commandprompt#1341) A guard widened to refuse the owner stopped the fixture's own export, so a premise reddened for the product code and the pytest twin crashed in setup rather than failing an arm by name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…e mutations of the RLS guard (commandprompt#1341) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
… in both twins (commandprompt#1341, commandprompt#1397 review) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…uals (commandprompt#1341) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…x, and the twin fails it by name rather than raising (commandprompt#1383 review) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
commandprompt#1383 review) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…name (commandprompt#1341) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rompt#1341) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… arm runs with the configured list empty, in both twins (commandprompt#1341) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n comparing two empty strings (commandprompt#1341) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…andprompt#1341) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d the shell names 'empty' itself (commandprompt#1379 review) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…dated (commandprompt#1379 review) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
|
Restated for the chain you asked for. The new head is b0303e6, so please re-approve this head.
🤖 Generated with Claude Code |
ea3278f to
b0303e6
Compare
jdatcmd
left a comment
There was a problem hiding this comment.
Re-approved at b0303e6, its place in the chain on 17420be. This PR's own commits, measured from the link below it, change the same lines as the head I approved before, apart from the count files and the TESTS.md section number.
🤖 Generated with Claude Code
Part of #1341, the object-store twin-parity item:
objstore_allowlist.shhad no pytest twin. Writing the twin turned up a vacuous arm in the shell suite, which is fixed here in both twins.The vacuous arm
"a superuser session SET works (admin override)" ran while the configured list was still
other.example, 127.0.0.1. So the read succeeded whatever the sessionSETdid, and the arm couldn't fail. Both twins now clear the configured list first, so only the session's SET can admit the read.The setting defined
PGC_SIGHUP(no session may SET it), PG17, main 4027b8d against this branch:The twin (
test_objstore_allowlist.py)One test, all 16 shell checks by name, added to
COMPLETE:finally, because the cluster is shared.http.serverin a thread, serving one Parquet file fromserver_dir. The shell's server is a script undertest/, and the two harnesses don't call each other.expect.require_module.Twin proof: an empty list that admits every endpoint reddens the two default-deny arms in the shell, on main and here, and the twin at "default: an http read is refused 42501: got ''".
Evidence
objstore_allowlisthad no rows, so it is seeded with all 16, four dated by the two mutations. 2361 → 2377 rows, the census 1801 → 1813, and suites_not_covered 246 → 245.cluster_tests534 → 535. test: parallel_copy and parallel_export_parquet refuse a table the caller holds no grant on; import_export_privilege gets its twin and its ledger (#1341) #1362 and test: objstore_userinfo gets its pytest twin, every check by the shell's name (#1341) #1378 take section 105 too, so whichever lands later renumbers.🤖 Generated with Claude Code