SEP-1672: Reject an encrypted base backup before a fast_restore incremental

[marcuscruz-percona]

Summary

• _is_good_base_backup promised to reject a base that is "fully prepared or encrypted" but only read xtrabackup_checkpoints, a file kept in plaintext so the base's LSN stays readable — so it could not see encryption at all. _is_compressed_backup's existence fallback caught the base anyway (encryption renames ibdata1), so a fast_restore job did fall back to a full backup, but the log blamed compression. The guard now detects both encryption formats and reports the real reason.
• Detection is independent of the job's current encryption settings: a base encrypted by an earlier run stays encrypted after those settings are turned off. xbcrypt is checked by rename (stat-only, so no gpg is spawned for such a base); gpg is verified with the real per-file probe, falling back to the .gpg rename the encryptor leaves behind on hosts where gpg is not installed — otherwise a missing binary would either crash the run or permanently reject good plaintext bases.
• less_space is deliberately exempt. It takes its LSN from the plaintext checkpoints, never reads the base directory, and restore decrypts the whole chain before preparing it, so rejecting an encrypted base there would turn every scheduled run into a full backup.
• The canonical payload sat 6 bytes under the 16 KiB Nomad dispatch limit, so the guard is funded by dead code in the same file: the unused strict_mode parameter and its unreachable raise, the encrypted-suffix names in _is_compressed_backup that the new guard makes unreachable, and a dead local in DirectoryEncryptor._load_config. Net 16,367 / 16,384 bytes (17 bytes headroom), down from 16,378; all 7 generated variants improved.
• New test module covering both guards, which had zero coverage before: 34 tests over the base-state matrix, nested schema directories, checkpoint-parsing edges, the plaintext-metadata exclusion set, the gpg-unavailable fallback in both directions, and the fallback message itself. payload_harness.gpg_probe is now shared, replacing the duplicated Popen fake in test_xtrabackup_aes256_encrypt.py.

Verified locally beyond the unit suite:

• Mutation sweep — six regressions applied to the real payload, each caught by the new tests (toggle-gated gpg probe, both wrong OSError fallbacks, dropped aes256 probe, top-level-only scan, guard ignoring incremental_method). None missed.
• Real gpg — a base encrypted with DirectoryEncryptor's own argv (recipient-keyed) is rejected under fast_restore and accepted under less_space. Confirmed gpg --list-only --decrypt exits 0 on a host holding no private key, which is how backup hosts are set up; had it needed the secret key, detection would have failed in production while passing every faked test.
• make test (10,143 passed), make lint, make run-pre-commit, make smoke-xtrabackup-variants (8 variant imports + 4 real rsync uploads) all pass.

Tested

• XTRABACKUP_AES256_KEYFILE set, XTRABACKUP_INCREMENTAL_METHOD=fast_restore: run twice — the second run logs the "fully prepared or encrypted base" line, not the compressed one, and produces a full backup
• POST_RUN_ENCRYPT set with gpg, fast_restore: same expectation
• gpg-encrypted base with POST_RUN_ENCRYPT since removed, fast_restore: still names the encrypted base
• No encryption, fast_restore: the second run still produces an incremental and merges it
• No encryption, compressed base (ibdata1.qp), fast_restore: the compressed-base line still fires
• XTRABACKUP_AES256_KEYFILE set, less_space: the incremental chain still builds against the encrypted base, and a restore of that chain succeeds

Checklist

• New/modified functions have type hints and rST docstrings
• New tests added for new features or bug fixes
• All tests pass locally (make test)
• Pre-commit hooks pass (make run-pre-commit)
• Database migrations generated if models changed (make makemigrations) — N/A, no models touched
• User-facing changes documented (README, inline help, UI text) — the run log is the user-facing surface; wording already named encryption
• Configuration changes documented with examples — N/A, no new settings
• Changelog fragment added under changelog.d/ if the change is user-facing (make changelog-add), or confirmed N/A

[Copilot]

Pull request overview

Adds encrypted-base detection for fast_restore, with broad guard coverage and synchronized payload variants.

Changes:

• Detects xbcrypt/GPG-encrypted bases before incremental merging.
• Preserves less_space behavior and improves fallback logging.
• Adds guard tests and a shared GPG test harness.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
app/sep/apps/mysql_backups/xtrabackup_payload	Implements canonical encrypted-base guard.
app/sep/apps/mysql_backups/xtrabackup_s3_payload	Updates generated S3 variant.
app/sep/apps/mysql_backups/xtrabackup_gsutil_payload	Updates generated gsutil variant.
app/sep/apps/mysql_backups/xtrabackup_rsync_payload	Updates generated rsync variant.
app/sep/apps/mysql_backups/xtrabackup_noupload_payload	Updates generated no-upload variant.
app/sep/apps/mysql_backups/xtrabackup_s3_gsutil_payload	Updates generated S3/gsutil variant.
app/sep/apps/mysql_backups/xtrabackup_rsync_s3_payload	Updates generated rsync/S3 variant.
app/sep/apps/mysql_backups/xtrabackup_rsync_gsutil_payload	Updates generated rsync/gsutil variant.
tests/app/sep/apps/mysql_backups/test_xtrabackup_incremental_base_guard.py	Adds base-state and fallback tests.
tests/app/sep/apps/mysql_backups/test_xtrabackup_aes256_encrypt.py	Reuses shared GPG probe.
tests/app/sep/apps/mysql_backups/payload_harness.py	Adds reusable GPG process harness.
changelog.d/SEP-1672.fixed.md	Documents corrected fallback reporting.

Suppressed comments (2)

app/sep/apps/mysql_backups/xtrabackup_payload:2933

• The return documentation is narrower than the implementation. The final fallback also returns True for encrypted or malformed/metadata-only bases with no ibdata1, none of which “would have to be decompressed.” Describe this as detecting a base that cannot be merged as uncompressed plaintext, or narrow the fallback to actual compression.

        The closing fallback still catches an encrypted base the guard could not
        see, since a renamed ``ibdata1`` cannot be merged either way.

        :param backup_path: Base backup directory.
        :return: True if the base would have to be decompressed before a merge.

app/sep/apps/mysql_backups/xtrabackup_payload:2869

• Add path: str and -> bool to this new method. Without them, the newly introduced encryption decision is outside the payload’s typed interface and static checks cannot validate its callers.

    def _is_encrypted_base(self, path):

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
app/sep
inventory.py
app/sep/sync/syncers
pmm.py
Project Total

This report was generated by python-coverage-comment-action