fix(backups): say how an archive is protected instead of assuming an envelope - #194
Merged
Merged
Conversation
…envelope The table and the endpoint were written assuming every archive is wrapped by a key manager. The family of backup that actually ships is not: CloudNativePG hands the archive to the object store, which encrypts it under a key the store holds. Nothing wraps a key for it. Writing the reporting side is what surfaced this. There was no honest value for key_provider on a real archive, and the only field that fit was the installation's configured key -- which unwraps nothing, and would be read back during a restore as an instruction to go and fetch it. So the distinction docs/backup-encryption.md makes in prose is made in the type. StoreManaged has no key reference because there is no key this platform could be asked for; Envelope carries one. A restore can now tell which it is holding without reading anything, and needs_a_key_manager answers the question it actually has. The columns follow, including the CHECK that makes both halves of the mistake unwritable: an envelope with no key is an archive nobody can open, and a key beside a store managed archive is a key that opens nothing. Neither shows up until a restore, which is why they are refused rather than repaired. The endpoint takes the three key fields together or not at all. A partial set is refused for the same reason.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #177. Found while writing #182, and worth landing before it.
The defect
#190 and #192 shipped
Backup.key: KeyRefand an endpoint that requireskey_provider,key_nameandkey_versionon every archive. Both assume every archive is wrapped by a key manager.The family of backup that actually ships is not. CloudNativePG hands the archive to the object store, which encrypts it under a key the store holds. Nothing wraps a data key for it, so there is no key reference to record.
Writing the reporting side is what surfaced it: I went looking for the value a data plane would put in
key_providerfor a real archive and there is none. The only field that fits is the installation's configured wrapping key, which unwraps nothing, and which a restore would read back as an instruction to go and fetch it.The fix
docs/backup-encryption.mdalready makes this distinction in prose. This makes it in the type:A restore can now tell which one it is holding without reading anything, and
needs_a_key_manager()answers the question it actually has. The variant is not a detail of how the bytes were written: it answers what a customer asks, which is whether this platform can read their archive.Both halves of the mistake are silent, so both are refused
An envelope with no key is an archive nobody can open. A key beside a store managed archive is a key that opens nothing. Neither shows up until a restore, which is the worst possible moment, so neither is repaired or filled in:
backups_envelopes_name_their_keymakes both unwritable, with a test that inserts a keyless envelope through raw SQL and asserts the database refuses it.The migration
Additive and reversible. The key columns become nullable, a
protectioncolumn says which mechanism was used rather than leaving it inferred from whether the others are there, and the default that backfills existing rows asenvelopeis dropped immediately afterwards so a new row cannot inherit a protection nobody stated.Every row written so far was written under the old assumption, and there are none in production, so the backfill is correct by construction rather than by luck.
Verification
The full run had every
REQUIRE_guard armed, so nothing skipped itself..sqlxandopenapi.jsonwere regenerated, not edited.