Skip to content

feat: validate restore schema-version compatibility - #440

Open
WentingWu666666 wants to merge 10 commits into
documentdb:mainfrom
WentingWu666666:developer/wentingwu/issue-434-restore-schema-validation
Open

feat: validate restore schema-version compatibility#440
WentingWu666666 wants to merge 10 commits into
documentdb:mainfrom
WentingWu666666:developer/wentingwu/issue-434-restore-schema-validation

Conversation

@WentingWu666666

@WentingWu666666 WentingWu666666 commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

What & why

Fixes #434.

A restore is a physical recovery: the restored catalog (including the documentdb extension schema) comes back at the backup-time schema version, while the new cluster's binary version is chosen independently. Running an older binary against a newer, irreversible schema risks data corruption, and the existing rollback guard (validateImageRollback) can't catch it on a fresh restore because the new cluster's status is empty.

This PR records the schema version at backup time (and on retained PVs) and validates binary-vs-schema compatibility at admission for both restore paths.

Changes

1. Backup-CR restores (commit e78ec1e)

  • Record the source cluster's schema version on Backup.Status.SchemaVersion at backup time (from DocumentDB.Status.SchemaVersion; captured once, then immutable). Added as a print column.
  • New create-only webhook check validateRestoreSchemaCompatibility: compares the restore's binary version against Backup.Status.SchemaVersion and rejects binary < schema; warns when the value is unknown.
  • Thread admission Warnings out of ValidateCreate.

2. PersistentVolume / volume-snapshot restores (commit 7fe7db6)

  • Stamp retained PVs with a documentdb.io/schema-version annotation (from DocumentDB.Status.SchemaVersion) in the PV controller — set once the schema version is known, never cleared.
  • Broaden the PV controller's DocumentDB watch predicate to also fire on status.schemaVersion changes, so schema upgrades re-stamp retained PVs.
  • Webhook reads the PV annotation for PV restores and applies the same binary < schema rejection; falls back to require an explicit documentDBVersion / image.documentDB + warn when the annotation is absent (e.g. a PV imported from outside the operator).

Docs

  • backup-and-restore.md: new "Version compatibility" section + behavior table.
  • restore-deleted-cluster.md: PV-restore schema-version validation and explicit-version fallback.

Behavior

Restore binary vs. schema Behavior
binary == schema Allowed
binary > schema Allowed (schema stays until you opt into an upgrade via spec.schemaVersion)
binary < schema Rejected when schema version is known (Backup status, or PV annotation)
schema version unknown Allowed with a warning; PV restores with no annotation require an explicit binary version

Testing

Unit (make test — all green, webhook coverage 96.6%):

  • Backup schema population; webhook Backup-restore cases (==, >, <, unknown, missing backup, undeterminable binary); PV annotation stamping (+ no-clear); watch-predicate schemaVersion trigger; webhook PV-annotation cases (==, >, <, fallback).

E2E (test/e2e/tests/backup/schema_validation_test.go, backup matrix, Medium level):

  • Backup path: assert the operator records Backup.Status.SchemaVersion, then a restore pinned to an older binary is rejected at admission and one matching the schema version is admitted.
  • PV path: assert the PV controller stamps the documentdb.io/schema-version annotation, delete the source (Retain), then a PV restore pinned to an older binary is rejected and one matching the schema version is admitted.
  • Restores are pinned via spec.documentDBVersion (image cleared) so the admission decision is deterministic regardless of the CI-injected image; the older version is rejected before any pull.
  • The broader backup matrix gaps (multi-replica, retention, cross-size, failed-snapshot, …) remain tracked separately in e2e: backup spec matrix is too thin — only 4 specs covering minimal happy path #368.

Notes

  • RBAC: the manager SA already has get persistentvolumes (shared with the PV controller); no RBAC change needed.
  • The PV annotation is advisory-grade (like the recorded Backup value): a live-cluster mid-upgrade PV restore can transiently read a stale value, but the dominant case — a retained PV from a deleted cluster — carries the correct final schema version.

Refs #434

@documentdb-triage-tool documentdb-triage-tool Bot added documentation Improvements or additions to documentation enhancement New feature or request go Pull requests that update go code test labels Aug 4, 2026
@documentdb-triage-tool

Copy link
Copy Markdown

🤖 Auto-triaged by documentdb-triage-tool.

Applied: go, test, documentation, enhancement
Project fields suggested: Component controllers · Priority P0 · Effort L · Status In Progress
Confidence: 0.95 (deterministic)

Reasoning

component from path globs (controllers, test, docs, api, manifests); P0 keyword "data corruption" in body; effort from diff stats (651+13 LOC, 12 files)

If a label is wrong, remove it manually and ping @patty-chow so the rules can be tuned. The bot will not re-label items that already have component labels.

@WentingWu666666
WentingWu666666 force-pushed the developer/wentingwu/issue-434-restore-schema-validation branch 6 times, most recently from 06aece6 to e093257 Compare August 5, 2026 15:22
Restore is a physical recovery: the restored catalog comes back at the
backup-time schema version, while the new cluster's binary version is
chosen independently. Running an older binary against a newer, irreversible
schema risks data corruption, and validateImageRollback cannot catch it on a
fresh restore (empty status).

- Record the source cluster's schema version on Backup.Status.SchemaVersion
  at backup time (populated from DocumentDB.Status.SchemaVersion, captured
  once and left immutable).
- Add a create-only webhook validation (validateRestoreSchemaCompatibility):
  - Backup-CR restore: compare restore binary vs Backup.Status.SchemaVersion;
    reject binary < schema, warn when the value is unknown.
  - PV/volume-snapshot restore: no admission-readable schema metadata, so
    require an explicit spec.documentDBVersion / spec.image.documentDB and
    otherwise warn.
- Thread admission Warnings out of ValidateCreate.
- Docs: add a "Version compatibility" section to backup-and-restore.md and a
  PV-requires-explicit-version note to restore-deleted-cluster.md.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b653a2f9-cc1a-400a-8e93-11232229023a
Signed-off-by: Wenting Wu <wentingwu@microsoft.com>
@WentingWu666666
WentingWu666666 force-pushed the developer/wentingwu/issue-434-restore-schema-validation branch 7 times, most recently from f30ca65 to d8e28ec Compare August 5, 2026 19:10
wentingwu000 and others added 2 commits August 5, 2026 15:16
Stamp retained PersistentVolumes with a documentdb.io/schema-version
annotation (from DocumentDB.Status.SchemaVersion) in the PV controller,
and read it at admission time so a PV/volume-snapshot restore can be
validated the same way as a Backup-CR restore: reject when the target
binary version is older than the schema on the PV.

- pv_recovery.go: add AnnotationSchemaVersion constant.
- pv_controller.go: stamp the annotation in applyDesiredPVConfiguration
  (set once schema version is known, never clear); broaden the DocumentDB
  watch predicate to also fire on status.schemaVersion changes so schema
  upgrades re-stamp the annotation.
- documentdb_webhook.go: read the PV annotation for PV restores and reuse
  the binary-vs-schema comparison; fall back to require-explicit-version +
  warn when the annotation is absent.
- Tests for annotation stamping, predicate trigger, and webhook PV path.
- Docs: PV restores are now validated via the annotation when available.

Refs documentdb#434

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b653a2f9-cc1a-400a-8e93-11232229023a
Signed-off-by: Wenting Wu <wentingwu@microsoft.com>
Add feature-scoped e2e specs for the restore schema-version compatibility
validation, exercising the full API-server + admission-webhook + operator
path:

- Backup path: create a source cluster + on-demand backup, assert the
  operator records Backup.Status.SchemaVersion, then assert a restore
  pinned to an older binary is rejected at admission and one matching the
  schema version is admitted.
- PV path: assert the PV controller stamps the documentdb.io/schema-version
  annotation on the backing PV, delete the source (Retain), then assert a
  PV restore pinned to an older binary is rejected and one matching the
  schema version is admitted.

Restores are pinned via spec.documentDBVersion (clearing spec.image so the
image tag can't win in resolveBinaryVersion), so the admission decision is
deterministic regardless of the CI-injected DOCUMENTDB_IMAGE. The older
version is rejected before any image pull, so it need not be pullable.

Refactor helpers_test.go to split buildRecoveryDocumentDB (render only)
from createRecoveryDocumentDB (render + create) so negative specs can
mutate the CR before Create.

Scope note: the broader backup matrix gaps remain tracked in documentdb#368.

Refs documentdb#434

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b653a2f9-cc1a-400a-8e93-11232229023a
Signed-off-by: Wenting Wu <wentingwu@microsoft.com>
@WentingWu666666
WentingWu666666 force-pushed the developer/wentingwu/issue-434-restore-schema-validation branch from d8e28ec to 9048fc6 Compare August 5, 2026 19:16
wentingwu000 and others added 2 commits August 5, 2026 15:25
Stamp retained PersistentVolumes with a documentdb.io/schema-version
annotation (from DocumentDB.Status.SchemaVersion) in the PV controller,
and read it at admission time so a PV/volume-snapshot restore can be
validated the same way as a Backup-CR restore: reject when the target
binary version is older than the schema on the PV.

- pv_recovery.go: add AnnotationSchemaVersion constant.
- pv_controller.go: stamp the annotation in applyDesiredPVConfiguration
  (set once schema version is known, never clear); broaden the DocumentDB
  watch predicate to also fire on status.schemaVersion changes so schema
  upgrades re-stamp the annotation.
- documentdb_webhook.go: read the PV annotation for PV restores and reuse
  the binary-vs-schema comparison; fall back to require-explicit-version +
  warn when the annotation is absent.
- Tests for annotation stamping, predicate trigger, and webhook PV path.
- Docs: PV restores are now validated via the annotation when available.

Refs documentdb#434

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b653a2f9-cc1a-400a-8e93-11232229023a
Signed-off-by: Wenting Wu <wentingwu@microsoft.com>
Add feature-scoped e2e specs for the restore schema-version compatibility
validation, exercising the full API-server + admission-webhook + operator
path:

- Backup path: create a source cluster + on-demand backup, assert the
  operator records Backup.Status.SchemaVersion, then assert a restore
  pinned to an older binary is rejected at admission and one matching the
  schema version is admitted.
- PV path: assert the PV controller stamps the documentdb.io/schema-version
  annotation on the backing PV, delete the source (Retain), then assert a
  PV restore pinned to an older binary is rejected and one matching the
  schema version is admitted.

Restores are pinned via spec.documentDBVersion (clearing spec.image so the
image tag can't win in resolveBinaryVersion), so the admission decision is
deterministic regardless of the CI-injected DOCUMENTDB_IMAGE. The older
version is rejected before any image pull, so it need not be pullable.

Refactor helpers_test.go to split buildRecoveryDocumentDB (render only)
from createRecoveryDocumentDB (render + create) so negative specs can
mutate the CR before Create.

Scope note: the broader backup matrix gaps remain tracked in documentdb#368.

Refs documentdb#434

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b653a2f9-cc1a-400a-8e93-11232229023a
Signed-off-by: Wenting Wu <wentingwu@microsoft.com>
@WentingWu666666
WentingWu666666 force-pushed the developer/wentingwu/issue-434-restore-schema-validation branch from 9048fc6 to 1fb1030 Compare August 5, 2026 19:25
wentingwu000 and others added 4 commits August 6, 2026 12:42
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Wenting Wu <wentingwu@microsoft.com>
…ssue-434-restore-schema-validation

Signed-off-by: Wenting Wu <wentingwu@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Wenting Wu <wentingwu@microsoft.com>
In preview there is no GA history to upgrade from, so the "older backups"
framing referenced a migration that does not apply. Keep the still-valid
externally-imported PV case as the reason a schema version can be unknown.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b653a2f9-cc1a-400a-8e93-11232229023a
Signed-off-by: Wenting Wu <wentingwu@microsoft.com>
@WentingWu666666
WentingWu666666 requested a lite review from Copilot August 6, 2026 17:01
…e specs

The restore_from_backup and restore_from_pv specs already stand up a source
cluster and a Backup / retained PV, so the standalone schema_validation spec
was re-creating the suite's most expensive fixtures (two extra clusters and a
backup) just to reach the same state.

Fold the two unique documentdb#434 assertions into those specs instead:
- record check: Backup.Status.SchemaVersion / PV annotation equals the source
  cluster's schema version;
- negative admission: a restore pinned to an older binary is rejected. The
  existing happy-path restore (default version >= schema) already covers admit.

Move the shared helpers into helpers_test.go and delete schema_validation_test.go.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b653a2f9-cc1a-400a-8e93-11232229023a
Signed-off-by: Wenting Wu <wentingwu@microsoft.com>
@WentingWu666666 WentingWu666666 changed the title feat: validate restore schema-version compatibility (#434) feat: validate restore schema-version compatibility Aug 6, 2026
@WentingWu666666
WentingWu666666 marked this pull request as ready for review August 6, 2026 17:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request go Pull requests that update go code test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs: document version compatibility for backup/restore (restore into >= backup schema version)

4 participants