feat(datastore): move the Maven remote writes behind the row encryptor (S04-A per-format columns plan: 4/11)

Step 4 of the S04-A per-format credential-column plan, split in two so neither half is reviewed at the 5187 LOC the two halves carry together, measured as the merge of part 1's 2fc083494 with this branch's 320662434 against 5e8a6449a. This is part 2, and it targets part 1's branch.

The stack, in merge order

MR What it lands
1 this MR The create, update and details paths, the leftover-plaintext boot report, and the removal of part 1's interim read fallback
2 !2553 (merged) The RemoteCredentialStore read and write behind RowEncryptor, the retired-key rejection, and the health sweep's refusal verdict

Built on !2553 (merged), and it targets that branch, so this MR merges into part 1's branch first and !2553 (merged) then carries both halves into main. A merge request dependency on this MR (block 3981883) enforces that order, because a stack can merge out of order and nothing else would stop part 1 reaching main alone.

What the order bounds is the discard, not part 1's correctness. Part 1 is correct by itself: the create and the update keep writing tmp_plaintext_*, and its read falls back to them. This MR deletes that fallback, and a row holding no encrypted unit then reads anonymous, the interim pair included. So every credential written while part 1 runs without this MR lands in the interim pair with no unit beside it and reads anonymous once this MR ships. One release holds that discard to rows predating part 1; two releases widen it to every credential write in between.

What this does

CreateMavenRemoteRepository and UpdateMavenRemoteRepository encrypt the supplied pair and write the unit, nulling the interim plaintext columns in the same statement, so a revoked or re-pointed upstream leaves no secret on the row. An anonymous create still encrypts nothing and stays creatable with no encryption block, which is the common case; a credentialed one is refused before any statement runs. has_credentials becomes the unit's presence.

With the writers moved, part 1's fallback to the interim pair has no purpose and goes, along with the two plaintext columns in its projection. A row still holding only the interim pair now reads anonymous: those values are discarded rather than backfilled, as !2542 (merged) records.

The boot report is the other half of that discard. One line per boot names how many Maven remotes still hold interim plaintext and samples up to ten of their ids, carrying no credential value. It is what the plaintext-column drop gate reads, so a failed count is its own error line rather than a zero. Its read is bounded by a deadline, because the component starts before the server binds its port and the query pins no namespace_id.

wireManagementAPI takes the encryption stack so the repository store can be built with it. The Maven dispatcher's finder keeps a nil encryptor: its store field exposes one method and no credential path is reachable through it.

The mixed-version window, accepted

Part 1 moved the credential store's read and write; this MR moves the create, update and details paths and drops part 1's read fallback. The write paths therefore do not all move in one MR, so there is a window in which one pod writes a row and another reads it: a write handled by a path that has not moved lands in tmp_plaintext_username and tmp_plaintext_password and never touches the unit, while this MR's reader reads the unit and has no fallback to the pair. The window stays open while any pod serving writes runs an image that still carries an unmoved path, and a rollback to such an image re-opens it.

Five shapes follow, each a write through an unmoved path read back through the moved reader:

  • a first credential, on a create or an update, lands in the pair with no unit beside it, so the read answers anonymous and the credential never takes effect;
  • a rotation on a row that already holds a unit leaves the unit standing, so the previous credential keeps going out and the rotation is silently ignored;
  • a clear nulls the pair and leaves the unit standing, so the credential it was meant to revoke stays live;
  • a URL change carrying no new credential runs that same clear, so the previous host's credential goes to the new host, the cross-host leak the URL-change clear exists to prevent;
  • a URL change carrying a new credential leaves the unit too, so the previous host's credential goes to the new host and the supplied one never takes effect.

The boot report sees three of those five and is blind to two. It matches on the interim columns being non-NULL, so the three shapes that write the pair stay in the leftover set and the line names them, while the clear and the credential-free URL change leave both columns NULL and the row does not look left over.

This is accepted, not fixed: no stale-unit audit here, and no release gate beyond the merge order above.

For the deployer. While an old image serves traffic, credential sets, rotations and clears handled through its unmoved paths do not reach this reader, and the affected rows have to be re-applied after the roll-forward completes. Once every write path has moved and no older image serves traffic, every write goes to the unit and the asymmetry is gone. For Maven that event is this step, Step 4 of the per-format credential-column plan; container and npm close their own window at Steps 7 and 10 of it.

The plan's "Rollback is clean while the plaintext columns still exist" line is about the schema, not about this asymmetry. docs/dev/column-encryption.md, in "Rows the move leaves behind", carries the constraint for the next reader.

Size

2668 reviewable LOC across 58 files: 1824 test, 669 production Go, 158 Markdown, 17 OpenAPI YAML, measured at 320662434 against f92f3f338, the part 1 commit this branch was rebased onto and still this diff's merge base. Part 1 has since moved to 2fc083494, which does not change this diff.

Past the 500-LOC guideline, and this is the larger of the two halves by file count (58 against part 1's 29). The create, update and details paths have to flip together with the removal of the fallback, or a credentialed Maven remote reads anonymous in the window between. Just over two thirds of the diff is tests.

Before merge

  • This MR merges into !2553 (merged)'s branch, then !2553 (merged) carries both halves into main.
  • !2542 (merged) merges. Five merged spec lines still prescribe a backfill this plan discards.
  • #513 clears. Merging this step deploys it, and no environment carries the encryption block yet.

Testing

go test ./... green; integration suites compile and run under -tags=integration; golangci-lint clean for the changed files under both tag sets; comment-caps gate green.

No e2e scenario is added or affected: docs/testing/ carries no upstream-credential scenario, and this change alters no client-facing route. has_credentials keeps its name, type and position; what changes is which column makes it true.

Related to #417 (closed)

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading