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 50The 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.mdclaimed the encrypted unit and the interim pair are exclusive after any post-move write, namingmavenRemoteCredentialUnitAssignmentsandupdateMavenRemoteCredentialsStmt(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:140the one Maven sentencemaindoes 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 operatorUPDATEas the remedy that survives this release, with the partition-key and all-or-none constraints behind its shape.mainalready carries that step for Container, so Maven folds into the one block rather than sitting beside it: theUPDATEnames either table, the constraint sentence names bothcheck_container_remote_repositories_credentials_all_or_noneandcheck_maven_remote_repositories_credentials_all_or_none, and the boot-line sentence readsformat=containerorformat=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 aremain's. The only addition to them is that an emptyRETURNINGhas two readings the statement cannot tell apart, the pair already clear or theUPDATEnaming the other format's table, with the boot line'sformatas the check. npm is not named in the constraint sentence because its singletmp_plaintext_auth_tokenhas no all-or-none pairing, whichcolumn-encryption.mdstates. 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.mdhad 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 subtesta clear over a populated interim pair leaves the pair standingasserts exactly that, callingassertCredentialColumnsNullbeforeassertInterimPlaintextPair.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 countstill 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.mdkeyed the clearingUPDATEon the ids the warn line samples. That sample stops atLeftoverPlaintextSampleLimitwhile the count is the true total, whichTestRemoteCredentialStore_LeftoverPlaintextCredentialsReportexists to pin, so above the cap it is a spot check and not a way to reach the zero the gate wants.mainalready carried that fact in the "Those pairs are a starting point rather than the list" passage (:204onmain,:210atc396c0efa), 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 casea url change with no new pair clears the pair and resets healthsaid "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"; itswantSetalways 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.mdsaid 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 becausefk_maven_remote_repositories_repository_id_repositoriesisON DELETE CASCADE, andRepositoryReaper.finalize(internal/datastore/lifecycle_reap_repository.go:540) is the productionDELETEbehind 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.
mainreplaced Maven's localmavenCredentialUnitBindswith the sharedcredentialUnitBindsandcredentialUnitBindsForininternal/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.mavenCredentialUnitBindsForkeepsmain's one-line delegation, and the un-bind now lives entirely in the two statement builders, which stop namingtmp_plaintext_usernameandtmp_plaintext_passwordin their column lists. The behaviour this MR ships is unchanged; the "three places lose the pair" above is two places onmain's shape. - Every format's write path has moved, and Container has already un-bound.
Step 7 merged in
e88b33bf8and Step 10 in96b7aaff8, soInterimPlaintextWritePathMovedreturnstrueon 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 onmainas a result, and the second rebase takesmain'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.mdentry 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. Onmainit 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)