Skip to content

test: a user mapping with half a credential is refused before any request; objstore_credentials seeds the ledger (#1341) - #1369

Merged
jdatcmd merged 3 commits into
commandprompt:mainfrom
OffgridwithJD:test/1341-objstore-half-credential
Oct 14, 2026
Merged

jdatcmd merged 3 commits into
commandprompt:mainfrom
OffgridwithJD:test/1341-objstore-half-credential

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

Part of #1341: the medium error-path item "a Parquet FDW user mapping that carries only half a credential is refused with 28000". No arm reached the pairing check in columnar_parquet_reader.c.

The arms (objstore_credentials.sh)

  • The mappings: two roles, each with USAGE on the server and SELECT on the S3-backed table. One mapping has access_key_id only, the other secret_access_key only.
  • What each read must give: 28000 columnar: the user mapping for server "csrv" carries only half a credential, the SQLSTATE and the message.
  • The fixture's request log must not grow.
  • A control runs first: a full mapping's read does grow the log, so "no request" is measured, not assumed.
  • Readiness: each arm starts on a server that accepts connections, because a crash in one arm must not fail the next for it.

What the check prevents

The check removed, PG17, main 1933577 against this branch:

main this branch
objstore_credentials.sh 21/21 the access-key-only read crashes its backend ("server closed the connection unexpectedly"); the secret-only read gets a different 28000, "no credentials for s3://…", which only the message assertion tells apart. The log arm and the control stay green.

So the pairing check is what stands between a half-configured mapping and a server crash, not only a nicer error.

Shell only: objstore_credentials has no pytest twin; that's the object-store twin-parity item.

Evidence

🤖 Generated with Claude Code

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving the content at 87a3d5f. On PG17 I reproduced the claim with the pairing check disabled:

  • main: objstore_credentials passes 21/21.
  • this head: the access-key-only read crashes its backend ("server closed the connection unexpectedly"), and the secret-only read gets the other 28000, "no credentials for …".

So that check is what stands between a half-configured mapping and a backend crash, and the message assertion is what tells the two 28000s apart. Worth a follow-up: the crash means the code after the check dereferences the missing half. Nothing else reaches it today, but it's a latent NULL dereference. Before merge: rebase and restate (seeding; the ceiling falls by one).

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Rebased onto f601281 (#1363 and #1368 merged): now 527bb71. Only the budget conflicted. Restated on main's figures: rows 2460 + 25 = 2485, never 1893 + 23 = 1916, dated 567 + 2 = 569, suites_not_covered 244 → 243. objstore_credentials.sh is unchanged from the version the 25/25 five-major runs used, and the gate is clean against origin/main. Its turn comes after #1365, which I'm restating now: #1363 seeded arrow_import, so #1365's two new checks need rows and a fresh run.

🤖 Generated with Claude Code

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Restated on f4a7591 (main is green again with #1390): now 0cc6d60. Rows 2464 + 25 = 2489; never 1918; dated 571; ceiling 244 → 243. The gate is clean, and the test file is unchanged.

🤖 Generated with Claude Code

@OffgridwithJD
OffgridwithJD force-pushed the test/1341-objstore-half-credential branch from 527bb71 to 0cc6d60 Compare October 14, 2026 17:50

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Checked 0cc6d60 against my approval of 87a3d5f. git range-diff shows the two test commits identical (=), and the third differs only in the restated budget (2489 / 1918 / 571, ceiling 243).

It's one merge behind: I merged #1365 (df6a02f) just before your push. That adds two arrow_import rows and dates reject dictionary-encoded file, so main is now 2466 / 1895 / 571, and your budget will conflict once more. Restated on df6a02f, it should give 2491 / 1918 / 573. When that head is green, I'll verify it the same way and merge.

🤖 Generated with Claude Code

@OffgridwithJD
OffgridwithJD force-pushed the test/1341-objstore-half-credential branch from 0cc6d60 to 4608bbd Compare October 14, 2026 18:03
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Restated on df6a02f (#1365 merged): now 4608bbd. Rows 2466 + 25 = 2491; never 1918; dated 573; ceiling 244 → 243. The gate is clean, and the test file is unchanged. Next in the queue.

🤖 Generated with Claude Code

@OffgridwithJD
OffgridwithJD force-pushed the test/1341-objstore-half-credential branch from 4608bbd to 1231d43 Compare October 14, 2026 18:33
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Restated on 7065fbe (#1356 merged): now 1231d43. Rows 2466 + 25 = 2491; never 1917; dated 574; ceiling 244 → 243. The gate is clean, and the test file is unchanged.

🤖 Generated with Claude Code

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verified 1231d43 against my approval of 87a3d5f. It sits on current main (7065fbe), the patch is identical outside the count files, and no main row is dropped or changed. The ledger gives 2491 / 1917 with the ceiling at 243. I'll merge once CI is green.

🤖 Generated with Claude Code

jdatcmd and others added 3 commits October 14, 2026 18:47
…uest (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…og control runs first (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… arms dated (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Restated on fc48e65 (#1385 merged): now 857b67b. Rows 2472 + 25 = 2497; never 1918; dated 579; ceiling 244 → 243. The gate is clean, and the test file is unchanged.

🤖 Generated with Claude Code

@OffgridwithJD
OffgridwithJD force-pushed the test/1341-objstore-half-credential branch from 1231d43 to 857b67b Compare October 14, 2026 18:47

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verified 857b67b against my approval of 87a3d5f. It sits on current main (fc48e65), the patch is identical outside the count files, and no main row changes (2497 / 1918, ceiling 243). I'll merge once CI is green.

🤖 Generated with Claude Code

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.

2 participants