fix(datastore): stop writing Maven's interim plaintext columns part 1/2 (S04-A per-format columns plan: 5/11)

Stacked MRs

Part MR Scope Reviewable LOC Target
1/2 this MR stop binding the interim pair 322 main
2/2 !2630 (merged) drop the columns pending its rebase dmeshcharakou/s04a-maven-stop-writing-plaintext

This MR merges first. !2630 (merged) targets this branch, so GitLab retargets it to main once this one lands. It must not be merged into this branch before then: that would carry both halves into main as a single merge and collapse the two releases the split exists to keep apart. The GitLab dependency on this MR holds that shut.

The one-release gap rests on !2630 (merged)'s Draft flag, not on the dependency. The dependency only orders the two: the moment this MR merges, !2630 (merged) is retargeted to main, its blocker clears, and nothing mechanical stops it merging the same day. Keep it Draft until this release is out.

This MR's row is additions plus deletions, re-derived at c345af8a3, the tip of the second rebase: 163 added and 150 removed against its merge base, main at a9f002cb7. The 296 this row carried was read at c7848c3ef, the first rebase's tip, and re-reading there now gives 297, so that figure was one line low as well as stale.

!2630 (merged)'s figure is withdrawn rather than re-derived, because both rebases moved the tree it is measured against. That row is measured tree-to-tree against this branch, so it needs a !2630 (merged) that sits on this branch's tip. !2630 (merged) is still at d6955e3db, one commit sitting directly on 3cac52f7b, a pre-rebase tip of this branch, and its merge base with the rebased branch is 85530efe0. A tree-to-tree reading taken today therefore reports 39,327, which is the two rebases' own churn and not !2630 (merged)'s change; the 30,254 this line carried was the same reading taken at the first rebase's tip. The readings before that, 2,788 at 937b71323 and 2,802 against main, predate both rebases. Measured against its own merge base with main it reads 2,871 today, which is a different quantity from the row's and is not a substitute for it. Re-derive the row once !2630 (merged) rebases onto this branch's tip, naming the tip it read, because every push here moves it. Two of the three files go-jet regenerates carry a Code generated ... DO NOT EDIT header and are excluded from that measurement (jet/.../model/maven_remote_repositories.go and its table/ twin); structure.sql is counted, because it carries none. !2630 (merged) targeting this branch is an exception to docs/dev/development-model.md's "every MR opens against main".

What this changes

Maven's credential writes stop naming tmp_plaintext_username and tmp_plaintext_password. The columns stay, the schema does not change, and the leftover-plaintext boot report stays with them.

Two places lose the pair, both statement builders: updateMavenRemoteCredentialsStmt and mavenRemoteCredentialUnitAssignments. Each stops naming tmp_plaintext_username and tmp_plaintext_password in its column list. The bind struct behind them is no longer Maven's: main moved it to the shared credentialUnitBinds in internal/datastore/credential_table.go, which Container instantiates too, so it keeps its plaintextUsername and plaintextPassword fields and Maven simply stops reading them. npm is not a third instantiation: it has its own npmCredentialUnitBinds (npm_remote_repositories.go:1300). With Maven's assignments no longer reading the pair, neither caller of credentialUnitBindsFor does, so this MR also rewrites that field comment to say the two fields and the four pg.CAST(pg.NULL).AS_TEXT() calls behind them are dead and are left for whichever change drops the columns. See the merge-order section for what the rebase onto that shape settled.

Why this is its own release

The pair was bound to NULL on every write arm, and that was right while the encrypted unit sat beside a populated pair: nulling it stopped a URL change leaving the old host's credential readable to a pod on the previous image.

It is also what makes the pair undroppable. docs/dev/database-migrations.md wants release N to stop reading and writing a column and release N+1 to drop it. With these SET assignments still in the deployed release, dropping the columns gives every in-flight Maven credential set, replace and clear SQLSTATE 42703 for the length of a rolling deploy, and a URL change with it, because the update path renders the whole unit on its ClearCredentials || urlChanged arm.

So the nulling goes here, in a release of its own, and the columns go in the next one.

What this opens, and who owns it

Until 5b lands, nothing nulls the pair. No arm erases a plaintext value a row already holds: not a set, not a replace, not a clear, and not a URL change. The URL change is the one worth naming separately, because the value it leaves behind was entered for the host the row no longer points at, where the other arms leave a merely stale one. Re-entering the credential stops being the remedy for any of them.

Before merging this, read the leftover-plaintext boot line in every environment this release reaches, and clear whatever it names. The gate is "a line was seen and it reported zero", never "no line was found". Once this release is the only one deployed, no path writes a plaintext value and no path nulls one, so the count falls only when a row's parent is reaped or an operator runs the UPDATE below.

The gate is now read, and it does not pass. Nothing in code or CI enforces it, so these are hand readings taken from Cloud Logging against AR's own namespace (resource.labels.namespace_name="artifact-registry-gke") in the two Runway projects:

Environment Line seen Count Read at Version
staging yes, WARN 1 2026-09-11T19:57:02Z 2.43.0
production none in 90 days unknown probed 2026-09-13 2.38.3

Staging holds one row, and the warn line samples it: 01a05acf-e945-75c5-a5ad-12285ebd8994/01a06e14-d4e2-7067-85c5-a0bb67369baf, format=maven. Both pods of that boot reported the same count, and it is the newest boot staging has had, so the reading is current rather than stale. Staging runs 2.43.0, which still nulls the pair on any credential write, so re-entering that repository's credential clears the row today and the UPDATE below also does. One of the two has to happen before this MR merges, which is exactly what !2628 (merged)'s Step 5a Depends on asks for. Once this release deploys there, the UPDATE is what reaches it.

Production has produced no line at all, which is a non-answer rather than a zero. It runs 2.38.3, and the report first ships in v2.43.0 (commit eb965a43e, Step 4), so the code that emits the line is not in the deployed image. This is the case the gate's own wording rules out: "a line was seen and it reported zero", never "no line was found".

So the gate adds a merge-order dependency this MR did not previously carry: production has to deploy a release at or after v2.43.0, and emit the line, before the gate can be called passed there. It is also the sharp end of the rollback case below, because 2.38.3 sits below the v2.43.0 boundary: production's Maven read path answers Credentials from TmpPlaintextUsername today (maven_remote_credentials.go:167 at v2.38.3), so any leftover pair there is live upstream auth right now and has never been counted.

The line is one of no remote repository holds interim plaintext credentials (info, the passing reading), remote repositories still hold interim plaintext credentials (warn, carrying leftover_plaintext_count and a capped leftover_plaintext_ids sample), or reading the interim plaintext credential count failed; boot continues (error, which fails the gate rather than reading as zero). Read it only from an environment whose migrations have applied, since the report takes no dependency on migration readiness.

Both rows come from one query: a single ClickHouse datasource serves both environments and is filtered by attribute (gitlab-com-infrastructure.md).

SELECT Attributes['env']                                 AS env,
       JSONExtractString(Body, 'version')                AS version,
       JSONExtractString(Body, 'level')                  AS level,
       JSONExtractString(Body, 'msg')                    AS msg,
       JSONExtractInt(Body, 'leftover_plaintext_count')  AS leftover_count,
       JSONExtractRaw(Body, 'leftover_plaintext_ids')    AS leftover_ids,
       JSONExtractString(Body, 'error_message')          AS error_message,
       TimestampTime                                     AS read_at
FROM observability.otel_logs
WHERE ServiceName LIKE '%artifact-registry%'
  AND JSONExtractString(Body, 'format') = 'maven'
  AND JSONExtractString(Body, 'msg') IN (
        'no remote repository holds interim plaintext credentials',
        'remote repositories still hold interim plaintext credentials',
        'reading the interim plaintext credential count failed; boot continues')
ORDER BY read_at DESC
LIMIT 50

The format filter picks Maven's line out of three rather than naming the only one emitted: leftoverPlaintextReports (wire_remote.go:653) builds one report per moved format, and RemoteCredentialStore, ContainerRemoteCredentialStore and NpmRemoteRepositoryStore all return true from InterimPlaintextWritePathMoved now, so all three emit. An earlier reading of this paragraph said Maven was the only one, which was true when the query was written and stopped being true when Step 10 landed. error_message is LabKit's fields.ErrorMessage and rides the error line alone. leftover_ids is extracted raw because it is a JSON array, which JSONExtractString reads as the empty string. Take the newest row per env and record its version beside the count: a window wide enough to catch a boot in both environments spans releases, so an older row reports an earlier image rather than the one this MR lands on.

Two readings are owed, and this table holds the first. This one is taken before this MR merges, while re-entering a credential still clears a row the line names, which is what !2628 (merged)'s Step 5a Depends on asks for. !2630 (merged) owes a second: its migration's ban-drop-column rationale requires each reading to come from a boot that postdates the completion of this release's rollout in that environment, because a count read mid-rollout can report zero on a table a pod on the previous image is still writing the pair into. !2628 (merged)'s Step 5b Acceptance currently describes one reading doing both jobs, and the two cannot be the same read.

What neither reading reaches. An environment that migrates later than them, a restored database or a long-lived review app among them, boots on this release's image, and the boot line is the only thing that names its rows. That is why this step keeps the report rather than retiring it, and !2630 (merged)'s rationale records that it takes the drop ungated for such an environment.

Any pod below v2.43.0 can still grow the set, and in production that is the pod running today. Growing it takes an image that writes a plaintext value, which is v2.42.0 and earlier: maven_remote_repositories_update.go:388 binds TmpPlaintextUsername to pg.String(*input.Username) at v2.42.0, and v2.38.3 binds it at that same line. Every release from v2.43.0 on binds the pair to a typed NULL on both arms (mavenCredentialUnitBindsFor at v2.43.0), so where the previous image is itself at or after that boundary, a pod on it, or a scale-up from an older ReplicaSet, shrinks the set rather than growing it.

Staging is in that position and production is not. Production runs 2.38.3, so a forward deploy of this release straight from it has a rolling window whose old pods write plaintext values, and this release then freezes whatever they wrote with no API path left to clear it. That is a deploy, not a rollback, so an earlier reading of this paragraph was wrong to say the frozen set can only be reopened by a rollback past v2.43.0: that holds only once the previous image is itself at or after v2.43.0. It is the second reason production has to land a release at or after v2.43.0 before this one, the first being that the gate line does not exist below it, and it is the same window !2630 (merged)'s post-rollout reading exists to catch.

Once production's previous image is at or after v2.43.0, the set this MR freezes is reopened only by a rollback past that boundary, which is the same boundary the read consequence below turns on.

Clearing a row the warn line names is SQL, because re-entering the credential stops being the remedy in this release:

UPDATE maven_remote_repositories
   SET tmp_plaintext_username = NULL, tmp_plaintext_password = NULL
 WHERE namespace_id = $1 AND repository_id = $2;

namespace_id is the hash partition key and has to be in the predicate, and check_maven_remote_repositories_credentials_all_or_none ties the two columns, so both go in the one statement. That form clears the rows one line named; the sample stops at ten ids while the count is the true total, so driving the count to zero above the cap means the same UPDATE keyed on the interim columns being non-NULL instead.

Why this ships the gate rather than a clearing UPDATE. Nulling those columns needs no key, so a migration in this release could zero the count by construction and none of the above would be owed. The plan settles it the other way, and the reason is measured rather than structural: the deployed environments hold no production credential to lose. All three add_*_remote_encrypted_credentials migrations record the same 2026-09-03 reading in their squawk-ignore-file note, no traffic on .com and test traffic only on staging, and the plan's Research Findings carry it too. So the expected count is zero everywhere, a clearing migration would be an irreversible data write aimed at a population that should not exist, and reading the line is what says whether that measurement still holds. A non-zero reading is the case the plan did not expect: run the UPDATE above by hand and escalate, rather than shipping a migration written in advance for it.

That zero reading is also what closes the rollback case. v2.43.0 is the first release whose Maven read path ignores the pair, where v2.42.0 still answered Credentials from TmpPlaintextUsername, so an image rollback past v2.43.0 makes a leftover pair live Maven upstream auth again. Before this MR the population could only shrink, because every credential write nulled it; after it, no write reaches it, so a non-zero count at merge time is cleared only by an operator's UPDATE or by the parent repository's reap cascading the whole row away. A zero leaves nothing to revive.

Tests

The suites that pinned the nulling assert its absence instead. Four subtests are new and one existing subtest gains the pair, which together put every arm of the four-arm claim above under an integration assertion, because a row carrying both the encrypted unit and a populated interim pair is reachable on the ordinary path from this release on rather than only in a mixed-version window:

Subtest Suite Arm State
a set over a populated interim pair leaves the pair standing TestRemoteCredentialStore_SetThenRead set / re-entry new
a clear over a populated interim pair leaves the pair standing TestRemoteCredentialStore_ClearCredentials clear / revoke new
a url change leaves a populated interim pair standing TestMavenRemoteRepositoryStore_UpdateMavenRemoteRepository url change with a supplied pair new
clears the unit on a url change with no new pair, leaving a populated interim pair TestMavenRemoteRepositoryStore_UpdateMavenRemoteRepository credential-free url change existing, renamed

Each asserts the write succeeds and that the pair reads back holding exactly the values it held. That survival is what holds the drop gate's count still, and it revives mavenRemoteCredentialUnit.plaintextUsername / .plaintextPassword, which the removed assertions had left scanned and unread.

The last row is the one this MR's own review round added. The three new subtests reach updateMavenRemoteCredentialsStmt twice and mavenRemoteUpdateAssignments once, all three on the supplied-pair or credential-store arm, so the update path's ClearCredentials || urlChanged arm had no integration subtest left over a populated pair: the subtest that used to seed one there now seeds an encrypted unit instead, and assertCredentialColumnsNull stopped asserting those columns with the rest of the nulling assertions. The pair is seeded on the existing credential-free url-change subtest rather than in a new one, because both of that arm's entry points render the same mavenRemoteCredentialUnitAssignments(nil) and only this one produces the cross-host state: the row ends up pointing at the new host while holding the old host's plaintext. The explicit-clear subtest keeps its encrypted-unit seed and says in a comment which subtest carries the arm's pair assertion.

The fourth new subtest, a url change supplying a pair on a credential-free row is a set in TestMavenRemoteRepositoryStore_UpdateMavenRemoteRepository, is not about the pair at all: it is the set half of the presence bit on the url-change arm, whose replace half is the existing a new pair supplied with the url change replaces instead of clearing. The subtest it replaced proved that same verdict on a row seeded with an interim pair, which conflated the two concerns, so the seeded case now proves survival and the unseeded one proves the verdict. Neither re-added SET below reddens it, because a row holding no pair has none to null.

The replace arm of the four-arm claim has no integration subtest of its own and needs none: mavenRemoteUpdateAssignments has two credential arms, and set and replace are the same arm (maven_remote_repositories_update.go:429-439), separated only by the audit verdict the locked read's presence bit picks. Replace is covered by the statement-level assert.NotContains(query, "tmp_plaintext") that TestUpdateMavenRemoteRepositoryStmt now runs on all seven of its cases.

Checked to redden with one re-added SET per site, run separately, because the two statement builders back different suites. The unit half is re-run at c345af8a3, the second rebase's tip, against a baseline in which all 17 subtests of the three unit suites pass; it reproduces the counts below exactly. The three integration rows are carried from the c7848c3ef reading rather than re-run against a database: maven_remote_repositories_update_integration_test.go and maven_remote_credentials_integration_test.go are byte-identical between the two commits, as is maven_remote_repositories_update.go, and the only change to maven_remote_credentials.go is main's asCredentialRefusal to asCredentialFailure rename on the read path, which no row below asserts against:

Re-added SET of tmp_plaintext_username in Subtests that fail
mavenRemoteCredentialUnitAssignments 6 of 7 in TestUpdateMavenRemoteRepositoryStmt, 3 of 5 in TestMavenRemoteRepositoryWritesCarryNoPlaintextCredential, 2 of 19 in TestMavenRemoteRepositoryStore_UpdateMavenRemoteRepository
updateMavenRemoteCredentialsStmt 3 of 5 in TestUpdateMavenRemoteCredentialsStmt, 1 of 6 in TestRemoteCredentialStore_SetThenRead, 1 of 2 in TestRemoteCredentialStore_ClearCredentials

The two of 19 are the two url-change subtests in the table above, which is what the added assertion bought: before it, only one of them reddened.

The one subtest of TestUpdateMavenRemoteRepositoryStmt that stays green is "a cache window alone writes only that column", the only case rendering no credential column; the two of TestMavenRemoteRepositoryWritesCarryNoPlaintextCredential that stay green are its create cases, and the create never named the pair.

TestUpdateMavenRemoteRepositoryStmt also gains a blanket assert.NotContains(query, "tmp_plaintext") on every case rather than per-case wantAbsent entries: wantSet is a substring check, so three of the cases end their expectation at encrypted_password and would still match a rendered pair.

Integration-tagged lint, which CI never runs because .golangci.yaml leaves run.build-tags unset: golangci-lint run --build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false ./internal/datastore/... on 2.13.2, per-run GOLANGCI_LINT_CACHE on both sides and no lock or cache warning in either output. Against main the diff adds no lll, wsl, whitespace, noinlineerr or gosec finding: those five are identical on the two sides, file by file. One count moves, by one: contextcheck on maven_remote_credentials_integration_test.go, 82 to 83. That file already carries the same finding on every seed and assert call in its other suites with no directive anywhere, so the new subtests match the file rather than introducing a selective suppression, and the net is +1 rather than one per added call because the subtests this MR removes carried such calls too. No finding at all lands on maven_remote_repositories_update_integration_test.go.

No e2e scenario is added or affected: docs/testing/ covers GitLab-authenticated flows, and nothing here changes a request, a response or an upstream fetch. The interim columns are not read by any route.

Documentation

Three sites asserted the nulling as live and this release falsifies all three, so they move with the code:

  • internal/datastore/credential_table.md claimed the encrypted unit and the interim pair are exclusive after any post-move write, naming mavenRemoteCredentialUnitAssignments and updateMavenRemoteCredentialsStmt (the two functions this MR edits) and called a row carrying both a mixed-version artifact. It now says the exclusivity ended and what produces such a row.
  • docs/dev/column-encryption.md's "Rows the move leaves behind" told an operator that re-entering a credential takes the row out of the leftover set. It now names the window between a format's un-bind release and its drop and says Maven is in it. main's moved-path clause already states the rule without naming a format, so this MR adds Maven alongside Container in the sentence that says whose statements no longer name the pair (:144), and adds at :140 the one Maven sentence main does not carry: that re-entry no longer clears the discarded values and operator SQL is what removes them. !2630 (merged) rewrites the same passage for the post-drop world; this MR is the release in which the un-amended text is wrong, which is why the fix cannot wait for it.
  • docs/runbooks/key-rotation.md's Step 4 of the root-key compromise response, "Treat every credential as disclosed", told an incident responder that "credentials are cleared one repository at a time through the settings update API". On Maven that clear now nulls the encrypted unit and leaves the interim pair standing, and those two columns carry no ciphertext, so no rotation ever covered them: they are precisely the rows that step exists for. The step now names the operator UPDATE as the remedy that survives this release, with the partition-key and all-or-none constraints behind its shape. main already carries that step for Container, so Maven folds into the one block rather than sitting beside it: the UPDATE names either table, the constraint sentence names both check_container_remote_repositories_credentials_all_or_none and check_maven_remote_repositories_credentials_all_or_none, and the boot-line sentence reads format=container or format=maven. RETURNING namespace_id, repository_id, the audit gap it exists for, and the note that the uncapped arm prunes no partition and writes across all 64 are main's. The only addition to them is that an empty RETURNING has two readings the statement cannot tell apart, the pair already clear or the UPDATE naming the other format's table, with the boot line's format as the check. npm is not named in the constraint sentence because its single tmp_plaintext_auth_token has no all-or-none pairing, which column-encryption.md states. This is the site !2628 (merged)'s Step 5a Files list asks for, and the one where getting it wrong costs an incident responder time.

The five shapes listed further down column-encryption.md (:195-199) are scoped to a write through an unmoved path read back through the moved reader. Every format's write path has moved now, so that list describes a rollback rather than a live window, and this MR leaves it alone, which is what the plan's Step 5a entry asks for.

A review pass on this MR found the same fact at four more sites, three of them wrong once the writes stop naming the columns, all four fixed in 1215febb9:

  • credential_table.md had a set, a replace, a clear and a URL change all leaving "a row carrying both" shapes. A clear, and the credential-free URL change that takes the same arm, null the whole unit, so what they leave is the interim pair standing alone; only a set or a replace leaves both. The new subtest a clear over a populated interim pair leaves the pair standing asserts exactly that, calling assertCredentialColumnsNull before assertInterimPlaintextPair.
  • LeftoverPlaintextRow's doc comment still named re-entering the credential as the operator's remedy, in the file this MR edits.
  • The assertion message in clearing a row's plaintext takes it out of the count still said the count falls "as rows are re-entered". The subtest clears by raw SQL, so it passed either way; the stated mechanism was the one this release removes.
  • column-encryption.md keyed the clearing UPDATE on the ids the warn line samples. That sample stops at LeftoverPlaintextSampleLimit while the count is the true total, which TestRemoteCredentialStore_LeftoverPlaintextCredentialsReport exists to pin, so above the cap it is a spot check and not a way to reach the zero the gate wants. main already carried that fact in the "Those pairs are a starting point rather than the list" passage (:204 on main, :210 at c396c0efa), so the predicate form is folded into that passage rather than written out a second time beside the id-keyed one.

Two smaller corrections in the same file. The count freezing is what this release changes: growth stopped earlier, in the release that moved the format's write paths onto the encrypted unit, so crediting this one with "cannot grow" gave a reason that does not operate. And the parent-gate paragraph still read as if a soft-deleted or wrong-format parent were what leaves a row unreachable; on Maven the gate no longer narrows that set, because no write path clears those columns at all.

A second review pass found two more sites, fixed in a9eca8506:

  • TestUpdateMavenRemoteRepositoryStmt's case a url change with no new pair clears the pair and resets health said "the pair" for the encrypted unit, in the same table whose sibling case this MR renamed from "clears the unit and the pair alone" to "clears the unit alone". A reader comparing the two read the first as still claiming the interim pair is cleared. It is now "clears the unit and resets health"; its wantSet always pinned the truth, so no assertion moved.
  • credential_table.md's sentence on a row carrying both shapes expanded the old mixed-version cause after a colon, three clauses past where that cause was last named, and never stated the new one. It is three sentences now and names the new cause: such a row is the ordinary result of re-entering a credential on a pre-move row.

A third review pass found the count's permanence overstated at two code sites and three in this description, fixed in 2b293f8dd and in the text above:

  • column-encryption.md said the un-bind release means "the count can no longer fall", and that a non-zero reading "stays non-zero until the drop". The reap falsifies both, and the same section already said so two paragraphs further down: a soft-deleted parent's row leaves on its own because fk_maven_remote_repositories_repository_id_repositories is ON DELETE CASCADE, and RepositoryReaper.finalize (internal/datastore/lifecycle_reap_repository.go:540) is the production DELETE behind it. What this release actually stops is every write clearing a pair in place, which is what the text says now.
  • LeftoverPlaintextRow's doc comment said only SQL clears "the row". SQL is what clears the pair; the reap removes the row.

One site is deliberately left: the plan's own "an operator re-enters credentials from the ids", in docs/plans/2026-09-03-s04a-per-format-credential-columns.md, which !2628 (merged)'s rewrite still carries. Guardrail 4 keeps a step MR out of the plan file, so it belongs in !2628 (merged). Quoted rather than cited by line, because !2628 (merged) is still moving and the line number went 206, 211, 230 across three readings of it.

Merge order

!2628 (merged), the plan amendment that splits step 5 into these two MRs, merged on 2026-09-14. This MR merges after the gate reading above. !2630 (merged) merges after this one is released, not merely merged.

The step-10 chain landed first, and this branch is rebased onto it

This is settled, not pending. An earlier reading of this section asked for this MR to merge before !2625 (merged), because !2625 (merged) rewrote internal/datastore/credential_table.md wholesale and its new text carried the stale exclusivity claim forward: "That shape and the encrypted one are exclusive after any post-move write, because every such write nulls the interim columns in the statement that writes the unit. On Maven that is mavenRemoteCredentialUnitAssignments ... updateMavenRemoteCredentialsStmt." Those are the two functions this MR edits.

That order did not happen. !2625 (merged) and !2626 (merged) are merged, so this MR is the one landing second and owning the resolution, which is the outcome that section called the more expensive of the two. The resolution is done twice over. The first rebase landed on main at 9b2540d92; !2627 (merged) and Container's Step 8a merged within hours of it, so the branch was hand-resolved onto main again. It now sits on main at a9f002cb7, which is both its merge base and main's tip, and merges into it cleanly. That second rebase replayed every commit, so every branch SHA quoted below is a replayed one and the pre-rebase SHAs an earlier revision of this description quoted are no longer on the branch.

Three things the rebase had to settle, none of them mechanical:

  • The bind struct is gone from Maven's file. main replaced Maven's local mavenCredentialUnitBinds with the shared credentialUnitBinds and credentialUnitBindsFor in internal/datastore/credential_table.go, which Container also instantiates. This MR can no longer drop the interim pair from the struct, because that struct is no longer Maven's to change. mavenCredentialUnitBindsFor keeps main's one-line delegation, and the un-bind now lives entirely in the two statement builders, which stop naming tmp_plaintext_username and tmp_plaintext_password in their column lists. The behaviour this MR ships is unchanged; the "three places lose the pair" above is two places on main's shape.
  • Every format's write path has moved, and Container has already un-bound. Step 7 merged in e88b33bf8 and Step 10 in 96b7aaff8, so InterimPlaintextWritePathMoved returns true on all three stores (maven_remote_credentials.go:314, container_remote_credential_table.go:192, npm_remote_credential_table.go:102). Container went a release further: Step 8a (eb549ff49) stopped its statements naming the pair. Two generations of sentence on this branch are false on main as a result, and the second rebase takes main's wording for both formats: the original "Container and npm still write their interim columns from paths that have not moved", and the first rebase's replacement saying Container still nulls its pair while npm has not moved.
  • Neither branch the plan offers is what shipped, and the record is Generalize. The plan's docs/dev/column-encryption.md entry names two: Narrow the moved-path clause whenever Step 7 or Step 10 has landed, and Delete it only while both are still open. Both were open at this plan's basis, so the authored commits took Delete, and the first rebase switched to Narrow and named Container and npm. Step 8a then landed and did neither: it generalized the clause. On main it names no format at all, reading "A credential write through a moved path whose statements still name its interim columns nulls them alongside the unit it writes", and the formats that have left the nulling side get a sentence of their own under it. This MR takes that shape and adds Maven to that sentence rather than reintroducing a format list. So the record the plan asks for, and what Step 11a inherits, is Generalize: a format-free rule plus one sentence naming the formats whose statements have stopped nulling. 11a adds npm to that sentence at its own un-bind and has no format name to remove. Amending the plan's Narrow/Delete pair to match is a plan edit and out of scope for a step MR under guardrail 4.

Overlap with the open merge requests

A file-overlap scan over the other 70 open MRs, re-run at c345af8a3, finds three. Each is checked by merging it into main first and then merging this branch into the result, so a branch's own staleness against main is not reported as a conflict with this one:

MR Overlapping paths After it merges
!2662 (merged) (s04a-container-drop-plaintext-columns) docs/dev/column-encryption.md, docs/runbooks/key-rotation.md conflict in both; see below
!2630 (merged) (part 2/2) all eleven of this MR's files stack lag: it targets this branch and sits on a pre-rebase tip
!2642 (merged) (s04b-sweep-stuck-row-recovery) docs/runbooks/key-rotation.md clean; see below

!2627 (merged) was the third hit on the previous reading and has merged, which is what the second rebase settled.

!2662 (merged) is the new one, and it has no dependency ordering it against this MR. It is Container's Step 8b, the drop half, 2,084 reviewable LOC, and it is Draft. It rewrites exactly the passages this MR touches: the moved-path clause and the parent-gate sentence in docs/dev/column-encryption.md, and the whole interim-plaintext SQL block in docs/runbooks/key-rotation.md. It generalizes them one step further than Step 8a did, replacing Container's name with "a format" throughout on the grounds that Container's columns are gone.

So this is a "whichever lands second owns the resolution" case, and no pipeline reports it: both branches merge into main cleanly on their own and only conflict with each other. This MR should land first. It is not Draft and !2662 (merged) is, and the resolution is cheaper in that direction: taking this MR's Maven additions into !2662 (merged)'s generic text is adding one format's sentence to a passage that has already dropped its format names, where the other order asks !2662 (merged) to re-derive which formats a narrower text still names. If !2662 (merged) lands first instead, the work here is to add Maven to its generic wording and to keep this MR's :140 sentence about operator SQL, which !2662 (merged) does not carry.

This MR carries a hard GitLab dependency on !2627 (merged), which has merged, so that dependency no longer gates anything.

!2642 (merged) overlaps but does not conflict with this branch. It adds one row to the sweep-troubleshooting table under ## Failure triage, landing at key-rotation.md:692 on main, over 300 lines below the interim-plaintext block this MR edits at :356. It merges into main cleanly on its own now, and this branch merges cleanly on top of that result, so there is no merge order to coordinate. An earlier reading of this paragraph put the table at :648 and reported !2642 (merged) as conflicting with main; both were true of an earlier main and neither holds today.

!2630 (merged)'s conflict is stack lag, and its own next rebase clears it. Its tip d6955e3db is one commit sitting directly on 3cac52f7b, a pre-rebase tip of this branch, so its merge base with this branch falls back to 85530efe0 and all eleven of this MR's files overlap. It conflicts with main on its own across eighteen files for the same reason, which is the lag rather than a disagreement. Nothing here is a merge order to coordinate: !2630 (merged) cannot merge before this branch, and rebasing it onto c345af8a3 is what resolves it. Its reviewable-LOC row in the table at the top is withdrawn until that rebase, for the same reason.

Related to #417 (closed)

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading