SEP-1650: Pin the always-emitted ENCRYPT invariant for MySQL backup configs - #1305
Open
olucasandrade wants to merge 2 commits into
Open
SEP-1650: Pin the always-emitted ENCRYPT invariant for MySQL backup configs#1305olucasandrade wants to merge 2 commits into
olucasandrade wants to merge 2 commits into
Conversation
…onfigs Add a contract test that drives build_backup_spec directly and asserts the serialized YAML always carries an explicit ENCRYPT key for all three backup types (mydumper, xtrabackup, binlog), across an encrypt=True form, an encrypt=False form, and a form built without any encryption fields. The test fails if exclude_unset/exclude_defaults is ever introduced into the builder or any change lets the key be omitted from the emitted config. Record the deliberate fail-safe intent at the three payload settings.get( "ENCRYPT", True) sites so a future reader does not align them to the form/config model default (False). No payload behavior change and the payload snapshot golden is unchanged.
olucasandrade
marked this pull request as ready for review
August 7, 2026 19:23
olucasandrade
requested review from
marcuscruz-percona,
peter-o-addo and
yyyyyyyan
as code owners
August 7, 2026 19:23
There was a problem hiding this comment.
Pull request overview
This PR adds a contract-style regression test to ensure MySQL backup specs emitted by build_backup_spec always serialize an explicit ENCRYPT key, and documents the intentional fail-safe default (settings.get("ENCRYPT", True)) in each payload so future changes don’t accidentally weaken the safety posture.
Changes:
- Add a parametrized contract test asserting
ENCRYPTis always present in the emitted YAML config for all backup types and encryption input shapes. - Add inline comments at the three payload
settings.get("ENCRYPT", True)sites to record the deliberate “absent means encrypt” intent. - No functional payload behavior changes; existing snapshot golden remains unchanged.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/app/sep/apps/mysql_backups/test_payload_snapshot.py | Adds a new contract test + helper to assert ENCRYPT is always emitted in build_backup_spec YAML output. |
| app/sep/apps/mysql_backups/xtrabackup_payload | Documents the intentional fail-safe default for missing ENCRYPT in config. |
| app/sep/apps/mysql_backups/mydumper_payload | Documents the intentional fail-safe default for missing ENCRYPT in config. |
| app/sep/apps/mysql_backups/binlog_payload | Documents the intentional fail-safe default for missing ENCRYPT in config. |
Suppressed comments (1)
tests/app/sep/apps/mysql_backups/test_payload_snapshot.py:224
- This test introduces a bare
dicttype annotation forencryption. Please parameterize it (e.g.dict[str, object]) to avoid erasing the value type contract.
def test_build_backup_spec_always_emits_encrypt_key(backup_type: str, encryption: dict):
| ) | ||
|
|
||
|
|
||
| def _all_servers_config(backup_type: str, encryption: dict) -> dict: |
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
Summary
build_backup_specdirectly and asserts the serialized YAML config always carries an explicitENCRYPTkey — for all three backup types (mydumper, xtrabackup, binlog), across anencrypt=Trueform, anencrypt=Falseform, and a form built with no encryption fields supplied. The test fails ifexclude_unset/exclude_defaultsis ever introduced into the builder, or if any future change lets the key be omitted from the emitted config.settings.get("ENCRYPT", True)sites so a future reader does not "fix" them into alignment with the form/config model default (False, an unchecked box). An absentENCRYPTon a standalone run against hand-authored config must mean encrypt, never silently produce an unencrypted backup on a customer host.tests/app/sep/snapshots/payload/mysql_backups__spec_path.json) is untouched.Closes SEP-1650. Written against the post-SEP-1623 field shape (SEP-1623 / #1184 is merged to
main).Tested
N/A — test-and-annotation change with no user-facing or payload behavior change. Verified via automated tests:
make test PYTEST_PATHS=tests/app/sep/apps/mysql_backups/test_payload_snapshot.py— 10 passed (the newtest_build_backup_spec_always_emits_encrypt_keymatrix + the unchangedtest_spec_path_payload_matrix_matches_golden).exclude_defaults=Trueto the builder'sjsonable_encodermakes theencrypt=False/omitted cases fail, confirming the guard bites; reverted.Checklist
make test) (mysql_backups suite: 427 passed; 1 pre-existing environmental failure inrestore/test_xtrabackup_aes256_restore.pyfrom the externalxbcryptAES256 binary, unrelated to this change)make run-pre-commit)Database migrations generated if models changed(N/A — no model change)User-facing changes documented(N/A — no user-facing change)Configuration changes documented with examples(N/A)Changelog fragment added(N/A — internal test + code-comment change, no user-facing behavior)