From 0428802621921519d4dcfc873a569744bb12bcb6 Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Tue, 4 Aug 2026 14:51:57 -0400 Subject: [PATCH 1/8] feat: validate restore schema-version compatibility (#434) 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 --- .../preview/operations/backup-and-restore.md | 41 ++++++ .../operations/restore-deleted-cluster.md | 12 ++ .../crds/documentdb.io_backups.yaml | 12 ++ operator/src/api/preview/backup_funcs.go | 13 +- operator/src/api/preview/backup_funcs_test.go | 25 +++- operator/src/api/preview/backup_types.go | 9 ++ .../crd/bases/documentdb.io_backups.yaml | 12 ++ .../internal/controller/backup_controller.go | 9 +- .../controller/backup_controller_test.go | 9 +- .../internal/webhook/documentdb_webhook.go | 118 ++++++++++++++- .../webhook/documentdb_webhook_test.go | 136 ++++++++++++++++++ 11 files changed, 381 insertions(+), 15 deletions(-) diff --git a/docs/operator-public-documentation/preview/operations/backup-and-restore.md b/docs/operator-public-documentation/preview/operations/backup-and-restore.md index 8bb574810..06f2b41ee 100644 --- a/docs/operator-public-documentation/preview/operations/backup-and-restore.md +++ b/docs/operator-public-documentation/preview/operations/backup-and-restore.md @@ -166,6 +166,43 @@ List backups for your DocumentDB cluster and choose one in `completed` status: kubectl get backups -n ``` +The `SchemaVersion` column shows the DocumentDB extension schema version the backup +was taken at. Note this value — you need it to pick a compatible binary version for +the restore (see [Version compatibility](#version-compatibility) below): + +```bash +kubectl get backup my-backup -n -o jsonpath='{.status.schemaVersion}' +``` + +### Version compatibility + +A restore is a **physical** recovery: the restored database — including the +installed `documentdb` extension schema — comes back at the schema version that +existed **when the backup was taken**. The restored cluster's binary version is +chosen independently by the new cluster's `documentDBVersion` (or `image`). + +!!! danger "Restore onto a binary version >= the backup's schema version" + You must restore onto a DocumentDB binary whose version is **greater than or + equal to** the backup's schema version — ideally the **same** `documentDBVersion` + the backup was taken with. Restoring onto an **older** binary runs it against a + newer, irreversible schema and may cause **data corruption**: schema upgrades + (`ALTER EXTENSION UPDATE`) have no downgrade path. + +How the rule is enforced and reported: + +| Restore binary vs. backup schema | Behavior | +| --- | --- | +| `binary == schema` | Allowed. No schema change. | +| `binary > schema` | Allowed. The schema stays at the backup version until you opt into an upgrade via `spec.schemaVersion` (see [Upgrades](upgrades.md)). | +| `binary < schema` | **Rejected** at admission when the backup has a recorded schema version. | +| schema version unknown | Allowed with an **admission warning** — compatibility could **not** be verified. This applies when the backup predates schema-version recording. For **PersistentVolume/volume-snapshot** restores (which carry no schema metadata), an explicit `documentDBVersion` (or `image.documentDB`) is **required** and the restore is otherwise allowed with a warning — verify the version manually. | + +To determine a backup's schema version when it is not recorded on the `Backup`, +record the source cluster's `status.schemaVersion` **before** taking the backup +(recommended operational practice), or restore into a cluster whose binary is `>=` +the expected version and then read +`kubectl get documentdb -o jsonpath='{.status.schemaVersion}'`. + ### Step 2: Create a New DocumentDB Cluster ```yaml title="restore.yaml" @@ -177,6 +214,9 @@ metadata: spec: nodeCount: 1 instancesPerNode: 1 + # Set to a version >= the backup's schema version (ideally the version the + # backup was taken with). See "Version compatibility" above. + documentDBVersion: "0.110.0" resource: storage: pvcSize: 10Gi @@ -207,6 +247,7 @@ Once the status shows `Cluster in healthy state`, connect and verify your data. - The backup must be in `completed` status. - The VolumeSnapshot referenced by the backup must still exist — if it was manually deleted, the backup cannot be used for recovery. - You cannot specify both `backup` and `persistentVolume` in the same recovery spec. +- The restore binary version must be **>= the backup's schema version**. Restores from a `Backup` with a recorded schema version are enforced at admission; PV-based restores and backups without a recorded schema version are allowed with a warning only. See [Version compatibility](#version-compatibility). For additional recovery options (including PV-based recovery), see [Restore a Deleted DocumentDB Cluster](restore-deleted-cluster.md). diff --git a/docs/operator-public-documentation/preview/operations/restore-deleted-cluster.md b/docs/operator-public-documentation/preview/operations/restore-deleted-cluster.md index 28df7423f..196d2057a 100644 --- a/docs/operator-public-documentation/preview/operations/restore-deleted-cluster.md +++ b/docs/operator-public-documentation/preview/operations/restore-deleted-cluster.md @@ -48,6 +48,15 @@ The PV should be in `Released` or `Available` status. ### Step 2: Create a New DocumentDB Cluster with PV Recovery +!!! warning "PV restore requires an explicit `documentDBVersion`" + Unlike a backup, a retained PV carries **no readable schema-version metadata**, so + the operator cannot verify version compatibility at admission time. You **must** set + `spec.documentDBVersion` (or `spec.image.documentDB`) explicitly, and it must be + **>= the schema version of the data on the PV** — ideally the version the original + cluster was running. A PV restore that omits an explicit version is rejected. + Restoring onto an older binary risks **data corruption** (see + [Backup and Restore → Version compatibility](backup-and-restore.md#version-compatibility)). + ```yaml title="restore-from-pv.yaml" apiVersion: documentdb.io/preview kind: DocumentDB @@ -57,6 +66,9 @@ metadata: spec: nodeCount: 1 instancesPerNode: 1 + # Required for PV restore. Set to a version >= the schema version of the data + # on the PV (ideally the version the original cluster was running). + documentDBVersion: "0.110.0" documentDbCredentialSecret: documentdb-credentials resource: storage: diff --git a/operator/documentdb-helm-chart/crds/documentdb.io_backups.yaml b/operator/documentdb-helm-chart/crds/documentdb.io_backups.yaml index a3172baa3..feb9387c6 100644 --- a/operator/documentdb-helm-chart/crds/documentdb.io_backups.yaml +++ b/operator/documentdb-helm-chart/crds/documentdb.io_backups.yaml @@ -41,6 +41,10 @@ spec: jsonPath: .status.message name: Message type: string + - description: DocumentDB schema version at backup time + jsonPath: .status.schemaVersion + name: SchemaVersion + type: string name: preview schema: openAPIV3Schema: @@ -104,6 +108,14 @@ spec: phase: description: Phase represents the current phase of the backup operation. type: string + schemaVersion: + description: |- + SchemaVersion is the DocumentDB extension schema version of the source + cluster at backup time, captured from the source DocumentDB's + status.schemaVersion. It is used to validate restore compatibility: + a restore must target a binary version >= this schema version, otherwise + an older binary would run against a newer, irreversible schema. + type: string startedAt: description: StartedAt is the time when the backup operation started. format: date-time diff --git a/operator/src/api/preview/backup_funcs.go b/operator/src/api/preview/backup_funcs.go index f88d95506..0c4d1fe92 100644 --- a/operator/src/api/preview/backup_funcs.go +++ b/operator/src/api/preview/backup_funcs.go @@ -34,8 +34,9 @@ func (backup *Backup) CreateCNPGBackup(scheme *runtime.Scheme, clusterName strin return cnpgBackup, nil } -// UpdateStatus updates the Backup status based on the CNPG Backup status and backup configuration. -func (backup *Backup) UpdateStatus(cnpgBackup *cnpgv1.Backup, backupConfiguration *BackupConfiguration) bool { +// UpdateStatus reconciles the Backup status from the observed inputs and returns +// whether any status field changed. +func (backup *Backup) UpdateStatus(cnpgBackup *cnpgv1.Backup, backupConfiguration *BackupConfiguration, sourceSchemaVersion string) bool { needsUpdate := false if backup.Status.Phase != cnpgBackup.Status.Phase { backup.Status.Phase = cnpgBackup.Status.Phase @@ -63,6 +64,14 @@ func (backup *Backup) UpdateStatus(cnpgBackup *cnpgv1.Backup, backupConfiguratio needsUpdate = true } + // Record the source cluster's schema version at backup time: captured once + // when first available and left immutable afterward, so a later restore can + // validate binary-vs-schema compatibility. + if backup.Status.SchemaVersion == "" && sourceSchemaVersion != "" { + backup.Status.SchemaVersion = sourceSchemaVersion + needsUpdate = true + } + return needsUpdate } diff --git a/operator/src/api/preview/backup_funcs_test.go b/operator/src/api/preview/backup_funcs_test.go index bdc4e224a..0e2e72cbc 100644 --- a/operator/src/api/preview/backup_funcs_test.go +++ b/operator/src/api/preview/backup_funcs_test.go @@ -83,7 +83,7 @@ var _ = Describe("Backup", func() { }, } - needsUpdate := backup.UpdateStatus(cnpg, nil) + needsUpdate := backup.UpdateStatus(cnpg, nil, "") Expect(needsUpdate).To(BeTrue()) Expect(string(backup.Status.Phase)).To(Equal(cnpgv1.BackupPhaseCompleted)) Expect(backup.Status.StartedAt).To(Equal(&startedAt)) @@ -119,9 +119,30 @@ var _ = Describe("Backup", func() { }, } - needsUpdate := backup.UpdateStatus(cnpg, nil) + needsUpdate := backup.UpdateStatus(cnpg, nil, "") Expect(needsUpdate).To(BeFalse()) }) + + It("captures the source schema version once and leaves it immutable", func() { + cnpg := &cnpgv1.Backup{ + Status: cnpgv1.BackupStatus{Phase: cnpgv1.BackupPhaseCompleted}, + } + backup := &Backup{Spec: BackupSpec{}} + + // First observation records the source schema version. + Expect(backup.UpdateStatus(cnpg, nil, "0.110.0")).To(BeTrue()) + Expect(backup.Status.SchemaVersion).To(Equal("0.110.0")) + + // A later, differing source schema version must NOT overwrite it + // (it reflects the schema at backup time), and must not by itself + // mark the status dirty. + Expect(backup.UpdateStatus(cnpg, nil, "0.112.0")).To(BeFalse()) + Expect(backup.Status.SchemaVersion).To(Equal("0.110.0")) + + // An empty source schema version is a no-op and never clears it. + Expect(backup.UpdateStatus(cnpg, nil, "")).To(BeFalse()) + Expect(backup.Status.SchemaVersion).To(Equal("0.110.0")) + }) }) Describe("CalculateExpirationTime", func() { diff --git a/operator/src/api/preview/backup_types.go b/operator/src/api/preview/backup_types.go index b1529a2b7..7b5ea36e2 100644 --- a/operator/src/api/preview/backup_types.go +++ b/operator/src/api/preview/backup_types.go @@ -48,6 +48,14 @@ type BackupStatus struct { // For skipped backups, this explains why the backup was skipped. // +optional Message string `json:"message,omitempty"` + + // SchemaVersion is the DocumentDB extension schema version of the source + // cluster at backup time, captured from the source DocumentDB's + // status.schemaVersion. It is used to validate restore compatibility: + // a restore must target a binary version >= this schema version, otherwise + // an older binary would run against a newer, irreversible schema. + // +optional + SchemaVersion string `json:"schemaVersion,omitempty"` } // +kubebuilder:object:root=true @@ -59,6 +67,7 @@ type BackupStatus struct { // +kubebuilder:printcolumn:name="StoppedAt",type=string,JSONPath=".status.stoppedAt",description="Backup completion time" // +kubebuilder:printcolumn:name="ExpiredAt",type=string,JSONPath=".status.expiredAt",description="Backup expiration time" // +kubebuilder:printcolumn:name="Message",type=string,JSONPath=".status.message",description="Backup status message" +// +kubebuilder:printcolumn:name="SchemaVersion",type=string,JSONPath=".status.schemaVersion",description="DocumentDB schema version at backup time" // +kubebuilder:metadata:labels=app=documentdb-operator type Backup struct { metav1.TypeMeta `json:",inline"` diff --git a/operator/src/config/crd/bases/documentdb.io_backups.yaml b/operator/src/config/crd/bases/documentdb.io_backups.yaml index a3172baa3..feb9387c6 100644 --- a/operator/src/config/crd/bases/documentdb.io_backups.yaml +++ b/operator/src/config/crd/bases/documentdb.io_backups.yaml @@ -41,6 +41,10 @@ spec: jsonPath: .status.message name: Message type: string + - description: DocumentDB schema version at backup time + jsonPath: .status.schemaVersion + name: SchemaVersion + type: string name: preview schema: openAPIV3Schema: @@ -104,6 +108,14 @@ spec: phase: description: Phase represents the current phase of the backup operation. type: string + schemaVersion: + description: |- + SchemaVersion is the DocumentDB extension schema version of the source + cluster at backup time, captured from the source DocumentDB's + status.schemaVersion. It is used to validate restore compatibility: + a restore must target a binary version >= this schema version, otherwise + an older binary would run against a newer, irreversible schema. + type: string startedAt: description: StartedAt is the time when the backup operation started. format: date-time diff --git a/operator/src/internal/controller/backup_controller.go b/operator/src/internal/controller/backup_controller.go index 8bcd4e135..ebb2b279e 100644 --- a/operator/src/internal/controller/backup_controller.go +++ b/operator/src/internal/controller/backup_controller.go @@ -108,8 +108,7 @@ func (r *BackupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, err } - // Update status based on CNPG Backup status - return r.updateBackupStatus(ctx, backup, cnpgBackup, cluster.Spec.Backup) + return r.updateBackupStatus(ctx, backup, cnpgBackup, cluster.Spec.Backup, cluster.Status.SchemaVersion) } // ensureVolumeSnapshotClass creates a VolumeSnapshotClass based on the cloud environment @@ -193,10 +192,10 @@ func (r *BackupReconciler) createCNPGBackup(ctx context.Context, backup *dbprevi return ctrl.Result{RequeueAfter: 5 * time.Second}, nil } -// updateBackupStatus updates the Backup status based on CNPG Backup status -func (r *BackupReconciler) updateBackupStatus(ctx context.Context, backup *dbpreview.Backup, cnpgBackup *cnpgv1.Backup, backupConfiguration *dbpreview.BackupConfiguration) (ctrl.Result, error) { +// updateBackupStatus reconciles and persists the Backup's status, returning the reconcile result. +func (r *BackupReconciler) updateBackupStatus(ctx context.Context, backup *dbpreview.Backup, cnpgBackup *cnpgv1.Backup, backupConfiguration *dbpreview.BackupConfiguration, sourceSchemaVersion string) (ctrl.Result, error) { original := backup.DeepCopy() - needsUpdate := backup.UpdateStatus(cnpgBackup, backupConfiguration) + needsUpdate := backup.UpdateStatus(cnpgBackup, backupConfiguration, sourceSchemaVersion) if needsUpdate { if err := r.Status().Patch(ctx, backup, client.MergeFrom(original)); err != nil { diff --git a/operator/src/internal/controller/backup_controller_test.go b/operator/src/internal/controller/backup_controller_test.go index 881546272..c3ab21c7b 100644 --- a/operator/src/internal/controller/backup_controller_test.go +++ b/operator/src/internal/controller/backup_controller_test.go @@ -193,11 +193,11 @@ var _ = Describe("Backup Controller", func() { }, } - res, err := reconciler.updateBackupStatus(ctx, backup, cnpgBackup, nil) + res, err := reconciler.updateBackupStatus(ctx, backup, cnpgBackup, nil, "0.112.0") Expect(err).ToNot(HaveOccurred()) Expect(res.RequeueAfter).NotTo(Equal(0)) - // Verify status was updated with times + // Verify status was updated with times and the source schema version. updated := &dbpreview.Backup{} Expect(fakeClient.Get(ctx, client.ObjectKey{Name: backupName, Namespace: backupNamespace}, updated)).To(Succeed()) Expect(string(updated.Status.Phase)).To(Equal(string(cnpgv1.BackupPhaseCompleted))) @@ -205,6 +205,7 @@ var _ = Describe("Backup Controller", func() { Expect(updated.Status.StoppedAt).ToNot(BeNil()) Expect(updated.Status.StartedAt.Time.Unix()).To(Equal(cnpgBackup.Status.StartedAt.Time.Unix())) Expect(updated.Status.StoppedAt.Time.Unix()).To(Equal(cnpgBackup.Status.StoppedAt.Time.Unix())) + Expect(updated.Status.SchemaVersion).To(Equal("0.112.0")) }) It("stops reconciling (returns zero result) when CNPG Backup phase is Failed", func() { @@ -249,7 +250,7 @@ var _ = Describe("Backup Controller", func() { }, } - res, err := reconciler.updateBackupStatus(ctx, backup, cnpgBackup, nil) + res, err := reconciler.updateBackupStatus(ctx, backup, cnpgBackup, nil, "") Expect(err).ToNot(HaveOccurred()) Expect(res.RequeueAfter).NotTo(Equal(0)) @@ -300,7 +301,7 @@ var _ = Describe("Backup Controller", func() { }, } - res, err := reconciler.updateBackupStatus(ctx, backup, cnpgBackup, nil) + res, err := reconciler.updateBackupStatus(ctx, backup, cnpgBackup, nil, "") Expect(err).ToNot(HaveOccurred()) // Still in progress, requeue Expect(res.RequeueAfter).To(Equal(10 * time.Second)) diff --git a/operator/src/internal/webhook/documentdb_webhook.go b/operator/src/internal/webhook/documentdb_webhook.go index c4220fc39..a5fb9108e 100644 --- a/operator/src/internal/webhook/documentdb_webhook.go +++ b/operator/src/internal/webhook/documentdb_webhook.go @@ -61,10 +61,16 @@ func (v *DocumentDBValidator) ValidateCreate(ctx context.Context, documentdb *db } allErrs := v.validate(documentdb) + + // Restore-specific compatibility check. Runs only on create because bootstrap + // is immutable afterward. May both add errors (hard block) and warnings. + warnings, restoreErrs := v.validateRestoreSchemaCompatibility(ctx, documentdb) + allErrs = append(allErrs, restoreErrs...) + if len(allErrs) == 0 { - return nil, nil + return warnings, nil } - return nil, apierrors.NewInvalid( + return warnings, apierrors.NewInvalid( schema.GroupKind{Group: "documentdb.io", Kind: "DocumentDB"}, documentdb.Name, allErrs) } @@ -218,6 +224,114 @@ func (v *DocumentDBValidator) validateImageRollback(newDB, oldDB *dbpreview.Docu return nil } +// --------------------------------------------------------------------------- +// Restore validation (create-only) +// --------------------------------------------------------------------------- + +// validateRestoreSchemaCompatibility validates that a restore's target binary +// version is compatible with the schema version the source backup was taken at. +// +// A restore is a physical recovery: the restored catalog (including the +// documentdb extension schema) comes back at 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 +// rollback guard (validateImageRollback) cannot catch it because a fresh restore +// has an empty status. +// +// Rules: +// - binary >= schema → allowed (schema catch-up handled by the two-phase upgrade flow) +// - binary < schema → rejected +// - schema/binary version unknown → allowed with a warning +// +// Backup-CR restores read the schema version from Backup.Status.SchemaVersion. +// PV/volume-snapshot restores carry no admission-readable schema metadata, so +// they instead require an explicit binary version (spec.documentDBVersion or +// spec.image.documentDB) and are otherwise allowed with a warning. +func (v *DocumentDBValidator) validateRestoreSchemaCompatibility(ctx context.Context, newDB *dbpreview.DocumentDB) (admission.Warnings, field.ErrorList) { + if newDB.Spec.Bootstrap == nil || newDB.Spec.Bootstrap.Recovery == nil { + return nil, nil + } + recovery := newDB.Spec.Bootstrap.Recovery + + // Non-backup restore (PV / volume snapshot): the schema version lives inside + // the restored data directory and cannot be read at admission time. + if recovery.Backup.Name == "" { + if recovery.PersistentVolume != nil && recovery.PersistentVolume.Name != "" { + // Require an explicit binary version so the restore is not silently + // pinned to the default image, which may be older than the restored + // schema and risk data corruption. We still cannot verify the exact + // number against the disk, so we also warn. + if newDB.Spec.DocumentDBVersion == "" && specImageDocumentDB(newDB) == "" { + return nil, field.ErrorList{field.Required( + field.NewPath("spec", "documentDBVersion"), + "restoring from a PersistentVolume requires an explicit spec.documentDBVersion "+ + "(or spec.image.documentDB) that is >= the schema version of the restored data; "+ + "the schema version cannot be verified at admission time for PV restores, so it must be set deliberately", + )} + } + return admission.Warnings{ + "restoring from a PersistentVolume: schema-version compatibility cannot be verified at admission time; " + + "ensure the specified binary version is >= the schema version of the restored data to avoid data corruption", + }, nil + } + return nil, nil + } + + backupName := recovery.Backup.Name + backup := &dbpreview.Backup{} + if err := v.Get(ctx, client.ObjectKey{Name: backupName, Namespace: newDB.Namespace}, backup); err != nil { + if apierrors.IsNotFound(err) { + return admission.Warnings{ + fmt.Sprintf("backup %q not found in namespace %q: schema-version compatibility cannot be verified", + backupName, newDB.Namespace), + }, nil + } + // Transient/API error: don't hard-block the restore, but warn. + return admission.Warnings{ + fmt.Sprintf("failed to read backup %q: schema-version compatibility cannot be verified: %v", backupName, err), + }, nil + } + + backupSchemaVersion := backup.Status.SchemaVersion + if backupSchemaVersion == "" { + return admission.Warnings{ + fmt.Sprintf("backup %q has no recorded schema version: schema-version compatibility cannot be verified; "+ + "ensure the target binary version is >= the backup's schema version to avoid data corruption", backupName), + }, nil + } + + binaryVersion := resolveBinaryVersion(newDB) + if binaryVersion == "" { + return admission.Warnings{ + fmt.Sprintf("cannot determine the target binary version for restore from backup %q "+ + "(set spec.documentDBVersion or spec.image.documentDB): schema-version compatibility (backup schema %s) cannot be verified", + backupName, backupSchemaVersion), + }, nil + } + + binaryExtensionVersion := util.SemverToExtensionVersion(binaryVersion) + schemaExtensionVersion := util.SemverToExtensionVersion(backupSchemaVersion) + + cmp, err := util.CompareExtensionVersions(binaryExtensionVersion, schemaExtensionVersion) + if err != nil { + return admission.Warnings{ + fmt.Sprintf("cannot compare restore binary version %s with backup schema version %s: %v; compatibility not verified", + binaryVersion, backupSchemaVersion, err), + }, nil + } + if cmp < 0 { + return nil, field.ErrorList{field.Forbidden( + field.NewPath("spec", "bootstrap", "recovery", "backup"), + fmt.Sprintf( + "restore blocked: target binary version %s is older than the backup's schema version %s. "+ + "Restoring onto an older binary runs it against a newer, irreversible schema and may cause data corruption. "+ + "Set spec.documentDBVersion (or spec.image.documentDB) to a version >= %s.", + binaryVersion, backupSchemaVersion, backupSchemaVersion), + )} + } + return nil, nil +} + // validateImmutableFields rejects updates to fields that cannot be changed after creation. // Note: credentialSecret, storageClass, and sidecarInjectorPluginName are enforced via // CEL transition rules on the CRD schema (see documentdb_types.go). diff --git a/operator/src/internal/webhook/documentdb_webhook_test.go b/operator/src/internal/webhook/documentdb_webhook_test.go index c838f649a..6fa4431cf 100644 --- a/operator/src/internal/webhook/documentdb_webhook_test.go +++ b/operator/src/internal/webhook/documentdb_webhook_test.go @@ -568,3 +568,139 @@ var _ = Describe("resource envelope validation", func() { Expect(v.validateResources(db)).ToNot(BeEmpty()) }) }) + +func newRestoreDocumentDB(name, version, backupName string) *dbpreview.DocumentDB { + db := newTestDocumentDB(version, "", "") + db.Name = name + db.Spec.Bootstrap = &dbpreview.BootstrapConfiguration{ + Recovery: &dbpreview.RecoveryConfiguration{ + Backup: cnpgv1.LocalObjectReference{Name: backupName}, + }, + } + return db +} + +func newBackupWithSchema(name, schemaVersion string) *dbpreview.Backup { + return &dbpreview.Backup{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"}, + Status: dbpreview.BackupStatus{SchemaVersion: schemaVersion}, + } +} + +func newValidatorWithObjects(objs ...ctrlclient.Object) *DocumentDBValidator { + scheme := runtime.NewScheme() + Expect(dbpreview.AddToScheme(scheme)).To(Succeed()) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() + return &DocumentDBValidator{Client: fakeClient} +} + +var _ = Describe("restore schema compatibility validation", func() { + ctx := context.Background() + + It("is a no-op when there is no bootstrap recovery", func() { + v := newValidatorWithObjects() + db := newTestDocumentDB("0.112.0", "", "") + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(warnings).To(BeEmpty()) + Expect(errs).To(BeEmpty()) + }) + + It("allows restore when binary version equals backup schema version", func() { + backup := newBackupWithSchema("bk", "0.112.0") + v := newValidatorWithObjects(backup) + db := newRestoreDocumentDB("restored", "0.112.0", "bk") + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(warnings).To(BeEmpty()) + Expect(errs).To(BeEmpty()) + }) + + It("allows restore when binary version is newer than backup schema version", func() { + backup := newBackupWithSchema("bk", "0.110.0") + v := newValidatorWithObjects(backup) + db := newRestoreDocumentDB("restored", "0.112.0", "bk") + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(warnings).To(BeEmpty()) + Expect(errs).To(BeEmpty()) + }) + + It("rejects restore when binary version is older than backup schema version", func() { + backup := newBackupWithSchema("bk", "0.112.0") + v := newValidatorWithObjects(backup) + db := newRestoreDocumentDB("restored", "0.110.0", "bk") + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(warnings).To(BeEmpty()) + Expect(errs).To(HaveLen(1)) + Expect(errs[0].Detail).To(ContainSubstring("older than the backup's schema version")) + }) + + It("warns when the backup has no recorded schema version", func() { + backup := newBackupWithSchema("bk", "") + v := newValidatorWithObjects(backup) + db := newRestoreDocumentDB("restored", "0.110.0", "bk") + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(errs).To(BeEmpty()) + Expect(warnings).To(HaveLen(1)) + Expect(warnings[0]).To(ContainSubstring("no recorded schema version")) + }) + + It("warns when the referenced backup does not exist", func() { + v := newValidatorWithObjects() + db := newRestoreDocumentDB("restored", "0.110.0", "missing") + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(errs).To(BeEmpty()) + Expect(warnings).To(HaveLen(1)) + Expect(warnings[0]).To(ContainSubstring("not found")) + }) + + It("warns when the restore binary version cannot be determined", func() { + backup := newBackupWithSchema("bk", "0.112.0") + v := newValidatorWithObjects(backup) + db := newRestoreDocumentDB("restored", "", "bk") + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(errs).To(BeEmpty()) + Expect(warnings).To(HaveLen(1)) + Expect(warnings[0]).To(ContainSubstring("cannot determine the target binary version")) + }) + + It("warns when restoring from a PersistentVolume with an explicit version", func() { + v := newValidatorWithObjects() + db := newTestDocumentDB("0.112.0", "", "") + db.Spec.Bootstrap = &dbpreview.BootstrapConfiguration{ + Recovery: &dbpreview.RecoveryConfiguration{ + PersistentVolume: &dbpreview.PVRecoveryConfiguration{Name: "pv-1"}, + }, + } + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(errs).To(BeEmpty()) + Expect(warnings).To(HaveLen(1)) + Expect(warnings[0]).To(ContainSubstring("PersistentVolume")) + }) + + It("rejects a PersistentVolume restore that omits an explicit binary version", func() { + v := newValidatorWithObjects() + db := newTestDocumentDB("", "", "") + db.Spec.Bootstrap = &dbpreview.BootstrapConfiguration{ + Recovery: &dbpreview.RecoveryConfiguration{ + PersistentVolume: &dbpreview.PVRecoveryConfiguration{Name: "pv-1"}, + }, + } + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(warnings).To(BeEmpty()) + Expect(errs).To(HaveLen(1)) + Expect(errs[0].Detail).To(ContainSubstring("requires an explicit spec.documentDBVersion")) + }) + + It("allows a PersistentVolume restore when only image.documentDB is set", func() { + v := newValidatorWithObjects() + db := newTestDocumentDB("", "", "ghcr.io/documentdb/documentdb:0.112.0") + db.Spec.Bootstrap = &dbpreview.BootstrapConfiguration{ + Recovery: &dbpreview.RecoveryConfiguration{ + PersistentVolume: &dbpreview.PVRecoveryConfiguration{Name: "pv-1"}, + }, + } + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(errs).To(BeEmpty()) + Expect(warnings).To(HaveLen(1)) + Expect(warnings[0]).To(ContainSubstring("PersistentVolume")) + }) +}) From 42cf5f871b0b530a20699dd1213ec8afb6a5426d Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Tue, 4 Aug 2026 14:57:37 -0400 Subject: [PATCH 2/8] feat: validate PV-restore schema compatibility via PV annotation 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 #434 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b653a2f9-cc1a-400a-8e93-11232229023a Signed-off-by: Wenting Wu --- .../preview/operations/backup-and-restore.md | 6 +- .../operations/restore-deleted-cluster.md | 19 +- .../src/internal/controller/pv_controller.go | 32 ++- .../internal/controller/pv_controller_test.go | 82 ++++++- operator/src/internal/utils/pv_recovery.go | 7 + .../internal/webhook/documentdb_webhook.go | 200 ++++++++++++------ .../webhook/documentdb_webhook_test.go | 131 +++++++++++- 7 files changed, 396 insertions(+), 81 deletions(-) diff --git a/docs/operator-public-documentation/preview/operations/backup-and-restore.md b/docs/operator-public-documentation/preview/operations/backup-and-restore.md index 06f2b41ee..8cba7d9af 100644 --- a/docs/operator-public-documentation/preview/operations/backup-and-restore.md +++ b/docs/operator-public-documentation/preview/operations/backup-and-restore.md @@ -194,8 +194,8 @@ How the rule is enforced and reported: | --- | --- | | `binary == schema` | Allowed. No schema change. | | `binary > schema` | Allowed. The schema stays at the backup version until you opt into an upgrade via `spec.schemaVersion` (see [Upgrades](upgrades.md)). | -| `binary < schema` | **Rejected** at admission when the backup has a recorded schema version. | -| schema version unknown | Allowed with an **admission warning** — compatibility could **not** be verified. This applies when the backup predates schema-version recording. For **PersistentVolume/volume-snapshot** restores (which carry no schema metadata), an explicit `documentDBVersion` (or `image.documentDB`) is **required** and the restore is otherwise allowed with a warning — verify the version manually. | +| `binary < schema` | **Rejected** at admission when the schema version is known — from the backup's recorded schema version, or from a retained PV's `documentdb.io/schema-version` annotation. | +| schema version unknown | Allowed with an **admission warning** — compatibility could **not** be verified. This applies when the backup predates schema-version recording. For **PersistentVolume/volume-snapshot** restores where the PV carries no schema-version annotation (e.g. imported from outside the operator), an explicit `documentDBVersion` (or `image.documentDB`) is **required** and the restore is otherwise allowed with a warning — verify the version manually. | To determine a backup's schema version when it is not recorded on the `Backup`, record the source cluster's `status.schemaVersion` **before** taking the backup @@ -247,7 +247,7 @@ Once the status shows `Cluster in healthy state`, connect and verify your data. - The backup must be in `completed` status. - The VolumeSnapshot referenced by the backup must still exist — if it was manually deleted, the backup cannot be used for recovery. - You cannot specify both `backup` and `persistentVolume` in the same recovery spec. -- The restore binary version must be **>= the backup's schema version**. Restores from a `Backup` with a recorded schema version are enforced at admission; PV-based restores and backups without a recorded schema version are allowed with a warning only. See [Version compatibility](#version-compatibility). +- The restore binary version must be **>= the backup's schema version**. Restores from a `Backup` with a recorded schema version — and PV restores where the PV carries a `documentdb.io/schema-version` annotation — are enforced at admission; backups or PVs without a known schema version are allowed with a warning only. See [Version compatibility](#version-compatibility). For additional recovery options (including PV-based recovery), see [Restore a Deleted DocumentDB Cluster](restore-deleted-cluster.md). diff --git a/docs/operator-public-documentation/preview/operations/restore-deleted-cluster.md b/docs/operator-public-documentation/preview/operations/restore-deleted-cluster.md index 196d2057a..14b628832 100644 --- a/docs/operator-public-documentation/preview/operations/restore-deleted-cluster.md +++ b/docs/operator-public-documentation/preview/operations/restore-deleted-cluster.md @@ -48,15 +48,20 @@ The PV should be in `Released` or `Available` status. ### Step 2: Create a New DocumentDB Cluster with PV Recovery -!!! warning "PV restore requires an explicit `documentDBVersion`" - Unlike a backup, a retained PV carries **no readable schema-version metadata**, so - the operator cannot verify version compatibility at admission time. You **must** set - `spec.documentDBVersion` (or `spec.image.documentDB`) explicitly, and it must be - **>= the schema version of the data on the PV** — ideally the version the original - cluster was running. A PV restore that omits an explicit version is rejected. - Restoring onto an older binary risks **data corruption** (see +!!! warning "PV restore and schema-version compatibility" + The operator stamps retained PVs with a `documentdb.io/schema-version` annotation + (recorded from the source cluster's schema version). When that annotation is present, + admission **verifies** it against the restore's binary version and **rejects** a restore + whose `documentDBVersion` (or `spec.image.documentDB`) is older than the PV's schema + version — restoring onto an older binary risks **data corruption** (see [Backup and Restore → Version compatibility](backup-and-restore.md#version-compatibility)). + If the annotation is missing (for example, a PV imported from outside the operator, or + from a raw disk/snapshot), the schema version cannot be verified. In that case you **must** + set `spec.documentDBVersion` (or `spec.image.documentDB`) explicitly — ideally the version + the original cluster was running — and a PV restore that omits an explicit version is + rejected. + ```yaml title="restore-from-pv.yaml" apiVersion: documentdb.io/preview kind: DocumentDB diff --git a/operator/src/internal/controller/pv_controller.go b/operator/src/internal/controller/pv_controller.go index 7b23feca0..63b5330d5 100644 --- a/operator/src/internal/controller/pv_controller.go +++ b/operator/src/internal/controller/pv_controller.go @@ -151,6 +151,25 @@ func (r *PersistentVolumeReconciler) applyDesiredPVConfiguration(ctx context.Con needsUpdate = true } + // Stamp the installed schema version so a future PV restore can validate + // binary/schema compatibility at admission time. Only set it once the cluster + // has reported a schema version; never clear a previously stamped value (a + // transient empty status must not erase the last-known-good version, which is + // what a retained PV would be restored from). + if schemaVersion := documentdb.Status.SchemaVersion; schemaVersion != "" && + pv.Annotations[util.AnnotationSchemaVersion] != schemaVersion { + if pv.Annotations == nil { + pv.Annotations = make(map[string]string) + } + logger.Info("PV schema-version annotation needs update", + "pv", pv.Name, + "currentValue", pv.Annotations[util.AnnotationSchemaVersion], + "desiredValue", schemaVersion, + "documentdb", documentdb.Name) + pv.Annotations[util.AnnotationSchemaVersion] = schemaVersion + needsUpdate = true + } + // Check if reclaim policy needs update desiredPolicy := r.getDesiredReclaimPolicy(documentdb) if pv.Spec.PersistentVolumeReclaimPolicy != desiredPolicy { @@ -402,17 +421,21 @@ func (r *PersistentVolumeReconciler) SetupWithManager(mgr ctrl.Manager) error { // Apply pvPredicate only to PersistentVolume events, not globally For(&corev1.PersistentVolume{}, builder.WithPredicates(pvPredicate())). // Watch DocumentDB changes and trigger reconciliation of associated PVs + // when the reclaim policy or the installed schema version changes. Watches( &dbpreview.DocumentDB{}, handler.EnqueueRequestsFromMapFunc(r.findPVsForDocumentDB), - builder.WithPredicates(documentDBReclaimPolicyPredicate()), + builder.WithPredicates(documentDBPVRelevantPredicate()), ). Named("pv-controller"). Complete(r) } -// documentDBReclaimPolicyPredicate only triggers when the reclaim policy field changes -func documentDBReclaimPolicyPredicate() predicate.Predicate { +// documentDBPVRelevantPredicate triggers PV reconciliation when a DocumentDB +// change affects PV-level state: the reclaim policy (mutates the PV spec) or the +// installed schema version (stamped onto the PV as an annotation for restore +// compatibility validation). +func documentDBPVRelevantPredicate() predicate.Predicate { return predicate.Funcs{ UpdateFunc: func(e event.UpdateEvent) bool { oldDB, ok := e.ObjectOld.(*dbpreview.DocumentDB) @@ -423,7 +446,8 @@ func documentDBReclaimPolicyPredicate() predicate.Predicate { if !ok { return false } - return oldDB.Spec.Resource.Storage.PersistentVolumeReclaimPolicy != newDB.Spec.Resource.Storage.PersistentVolumeReclaimPolicy + return oldDB.Spec.Resource.Storage.PersistentVolumeReclaimPolicy != newDB.Spec.Resource.Storage.PersistentVolumeReclaimPolicy || + oldDB.Status.SchemaVersion != newDB.Status.SchemaVersion }, CreateFunc: func(e event.CreateEvent) bool { return false }, DeleteFunc: func(e event.DeleteEvent) bool { return false }, diff --git a/operator/src/internal/controller/pv_controller_test.go b/operator/src/internal/controller/pv_controller_test.go index fc1fd1419..f312abf5d 100644 --- a/operator/src/internal/controller/pv_controller_test.go +++ b/operator/src/internal/controller/pv_controller_test.go @@ -277,6 +277,71 @@ var _ = Describe("PersistentVolume Controller", func() { needsUpdate := reconciler.applyDesiredPVConfiguration(ctx, pv, documentdb) Expect(needsUpdate).To(BeFalse()) }) + + It("stamps the schema-version annotation from DocumentDB status", func() { + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: pvName, + Labels: map[string]string{ + util.LabelCluster: documentdbName, + util.LabelNamespace: testNamespace, + }, + }, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimRetain, + MountOptions: []string{"nodev", "noexec", "nosuid"}, + }, + } + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{Name: documentdbName, Namespace: testNamespace}, + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PersistentVolumeReclaimPolicy: "Retain", + }, + }, + }, + Status: dbpreview.DocumentDBStatus{SchemaVersion: "0.112.0"}, + } + + needsUpdate := reconciler.applyDesiredPVConfiguration(ctx, pv, documentdb) + Expect(needsUpdate).To(BeTrue()) + Expect(pv.Annotations[util.AnnotationSchemaVersion]).To(Equal("0.112.0")) + }) + + It("does not clear the schema-version annotation when status is empty", func() { + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: pvName, + Labels: map[string]string{ + util.LabelCluster: documentdbName, + util.LabelNamespace: testNamespace, + }, + Annotations: map[string]string{ + util.AnnotationSchemaVersion: "0.112.0", + }, + }, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimRetain, + MountOptions: []string{"nodev", "noexec", "nosuid"}, + }, + } + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{Name: documentdbName, Namespace: testNamespace}, + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PersistentVolumeReclaimPolicy: "Retain", + }, + }, + }, + // No Status.SchemaVersion set. + } + + needsUpdate := reconciler.applyDesiredPVConfiguration(ctx, pv, documentdb) + Expect(needsUpdate).To(BeFalse()) + Expect(pv.Annotations[util.AnnotationSchemaVersion]).To(Equal("0.112.0")) + }) }) Describe("provisionerSupportsMountOptions", func() { @@ -955,11 +1020,11 @@ var _ = Describe("PersistentVolume Controller", func() { }) }) - Describe("documentDBReclaimPolicyPredicate", func() { + Describe("documentDBPVRelevantPredicate", func() { var pred predicate.Predicate BeforeEach(func() { - pred = documentDBReclaimPolicyPredicate() + pred = documentDBPVRelevantPredicate() }) Describe("UpdateFunc", func() { @@ -1013,6 +1078,19 @@ var _ = Describe("PersistentVolume Controller", func() { Expect(pred.Update(e)).To(BeFalse()) }) + It("returns true when the installed schema version changes", func() { + oldDB := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{Name: documentdbName, Namespace: testNamespace}, + Status: dbpreview.DocumentDBStatus{SchemaVersion: "0.110.0"}, + } + newDB := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{Name: documentdbName, Namespace: testNamespace}, + Status: dbpreview.DocumentDBStatus{SchemaVersion: "0.112.0"}, + } + e := event.UpdateEvent{ObjectOld: oldDB, ObjectNew: newDB} + Expect(pred.Update(e)).To(BeTrue()) + }) + It("returns false for non-DocumentDB objects", func() { pvc := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{Name: pvcName, Namespace: testNamespace}, diff --git a/operator/src/internal/utils/pv_recovery.go b/operator/src/internal/utils/pv_recovery.go index 16cd387e3..257bec157 100644 --- a/operator/src/internal/utils/pv_recovery.go +++ b/operator/src/internal/utils/pv_recovery.go @@ -17,6 +17,13 @@ const ( // Label for identifying the DocumentDB cluster a PV/PVC belongs to LabelCluster = "documentdb.io/cluster" LabelNamespace = "documentdb.io/namespace" + + // AnnotationSchemaVersion records, on a PV, the DocumentDB extension schema + // version installed in the data on that volume. It is stamped by the PV + // controller from DocumentDB.Status.SchemaVersion and read at admission time + // to validate PV-restore binary/schema compatibility (the schema version is + // otherwise not readable from a PV without booting PostgreSQL). + AnnotationSchemaVersion = "documentdb.io/schema-version" ) // TempPVCNameForPVRecovery generates the name for a temporary PVC used during PV recovery. diff --git a/operator/src/internal/webhook/documentdb_webhook.go b/operator/src/internal/webhook/documentdb_webhook.go index a5fb9108e..ef85da430 100644 --- a/operator/src/internal/webhook/documentdb_webhook.go +++ b/operator/src/internal/webhook/documentdb_webhook.go @@ -10,6 +10,7 @@ import ( "strconv" "strings" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/runtime/schema" @@ -229,10 +230,16 @@ func (v *DocumentDBValidator) validateImageRollback(newDB, oldDB *dbpreview.Docu // --------------------------------------------------------------------------- // validateRestoreSchemaCompatibility validates that a restore's target binary -// version is compatible with the schema version the source backup was taken at. +// version is compatible with the schema version the source was taken at. +// +// "Binary version" here is the DocumentDB extension version the new cluster will +// run (resolved from spec.documentDBVersion / spec.image.documentDB, or the +// operator default) — the same value surfaced to users in messages as the +// "DocumentDB version". The "schema version" is the extension schema version the +// source data was written at. // // A restore is a physical recovery: the restored catalog (including the -// documentdb extension schema) comes back at backup-time schema version, while +// documentdb extension schema) comes back at source-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 // rollback guard (validateImageRollback) cannot catch it because a fresh restore @@ -241,92 +248,144 @@ func (v *DocumentDBValidator) validateImageRollback(newDB, oldDB *dbpreview.Docu // Rules: // - binary >= schema → allowed (schema catch-up handled by the two-phase upgrade flow) // - binary < schema → rejected -// - schema/binary version unknown → allowed with a warning +// - schema or binary version unknown → allowed with a warning // -// Backup-CR restores read the schema version from Backup.Status.SchemaVersion. -// PV/volume-snapshot restores carry no admission-readable schema metadata, so -// they instead require an explicit binary version (spec.documentDBVersion or -// spec.image.documentDB) and are otherwise allowed with a warning. +// It orchestrates three single-purpose steps: identify the restore source, +// resolve that source's schema version, and compare it against the effective +// binary version. Backup-CR restores read the schema from Backup.Status; PV +// restores read it from the source PV's annotation (stamped by the PV +// controller). Both sources follow the same logic: when the schema version +// cannot be determined the restore is allowed with a warning; only a resolved +// binary older than a known schema is rejected. func (v *DocumentDBValidator) validateRestoreSchemaCompatibility(ctx context.Context, newDB *dbpreview.DocumentDB) (admission.Warnings, field.ErrorList) { if newDB.Spec.Bootstrap == nil || newDB.Spec.Bootstrap.Recovery == nil { return nil, nil } - recovery := newDB.Spec.Bootstrap.Recovery - - // Non-backup restore (PV / volume snapshot): the schema version lives inside - // the restored data directory and cannot be read at admission time. - if recovery.Backup.Name == "" { - if recovery.PersistentVolume != nil && recovery.PersistentVolume.Name != "" { - // Require an explicit binary version so the restore is not silently - // pinned to the default image, which may be older than the restored - // schema and risk data corruption. We still cannot verify the exact - // number against the disk, so we also warn. - if newDB.Spec.DocumentDBVersion == "" && specImageDocumentDB(newDB) == "" { - return nil, field.ErrorList{field.Required( - field.NewPath("spec", "documentDBVersion"), - "restoring from a PersistentVolume requires an explicit spec.documentDBVersion "+ - "(or spec.image.documentDB) that is >= the schema version of the restored data; "+ - "the schema version cannot be verified at admission time for PV restores, so it must be set deliberately", - )} - } - return admission.Warnings{ - "restoring from a PersistentVolume: schema-version compatibility cannot be verified at admission time; " + - "ensure the specified binary version is >= the schema version of the restored data to avoid data corruption", - }, nil + src, ok := restoreSourceFor(newDB.Spec.Bootstrap.Recovery) + if !ok { + return nil, nil // no recognizable restore source to validate + } + + schemaVersion, warnings := v.resolveSourceSchemaVersion(ctx, newDB.Namespace, src) + if len(warnings) > 0 { + return warnings, nil + } + + return compareBinaryToSchema(resolveEffectiveBinaryVersion(newDB), schemaVersion, src) +} + +// restoreSource identifies where a restore draws its data — and thus its schema +// version — from, carrying the diagnostics and the spec field to flag on error. +type restoreSource struct { + kind string // sourceKindBackup or sourceKindPV + name string + fieldPath *field.Path +} + +const ( + sourceKindBackup = "backup" + sourceKindPV = "PersistentVolume" +) + +// restoreSourceFor identifies the restore source from a recovery configuration, +// returning false when neither a backup nor a PersistentVolume source is set. +func restoreSourceFor(recovery *dbpreview.RecoveryConfiguration) (restoreSource, bool) { + if recovery.Backup.Name != "" { + return restoreSource{ + kind: sourceKindBackup, + name: recovery.Backup.Name, + fieldPath: field.NewPath("spec", "bootstrap", "recovery", "backup"), + }, true + } + if recovery.PersistentVolume != nil && recovery.PersistentVolume.Name != "" { + return restoreSource{ + kind: sourceKindPV, + name: recovery.PersistentVolume.Name, + fieldPath: field.NewPath("spec", "bootstrap", "recovery", "persistentVolume"), + }, true + } + return restoreSource{}, false +} + +// resolveSourceSchemaVersion reads the restore source and returns its recorded +// schema version. It only reports warnings for I/O failures (source not found or +// unreadable); a successfully read source with no schema version returns ("", +// nil), leaving the "unknown schema" policy to compareBinaryToSchema so that both +// empty-input cases are decided in one place. Backup and PV are handled +// identically. +func (v *DocumentDBValidator) resolveSourceSchemaVersion(ctx context.Context, namespace string, src restoreSource) (string, admission.Warnings) { + var schemaVersion string + var readErr error + + switch src.kind { + case sourceKindBackup: + backup := &dbpreview.Backup{} + if err := v.Get(ctx, client.ObjectKey{Name: src.name, Namespace: namespace}, backup); err != nil { + readErr = err + } else { + schemaVersion = backup.Status.SchemaVersion + } + case sourceKindPV: + pv := &corev1.PersistentVolume{} + if err := v.Get(ctx, client.ObjectKey{Name: src.name}, pv); err != nil { + readErr = err + } else { + schemaVersion = pv.Annotations[util.AnnotationSchemaVersion] } - return nil, nil } - backupName := recovery.Backup.Name - backup := &dbpreview.Backup{} - if err := v.Get(ctx, client.ObjectKey{Name: backupName, Namespace: newDB.Namespace}, backup); err != nil { - if apierrors.IsNotFound(err) { - return admission.Warnings{ - fmt.Sprintf("backup %q not found in namespace %q: schema-version compatibility cannot be verified", - backupName, newDB.Namespace), - }, nil + if readErr != nil { + if apierrors.IsNotFound(readErr) { + return "", admission.Warnings{ + fmt.Sprintf("%s %q not found: schema-version compatibility cannot be verified", src.kind, src.name), + } } // Transient/API error: don't hard-block the restore, but warn. - return admission.Warnings{ - fmt.Sprintf("failed to read backup %q: schema-version compatibility cannot be verified: %v", backupName, err), - }, nil + return "", admission.Warnings{ + fmt.Sprintf("failed to read %s %q: schema-version compatibility cannot be verified: %v", src.kind, src.name, readErr), + } } + return schemaVersion, nil +} - backupSchemaVersion := backup.Status.SchemaVersion - if backupSchemaVersion == "" { +// compareBinaryToSchema is the single authority on the restore decision. It warns +// (and allows) when either the source schema version or the target DocumentDB +// version is unknown, and rejects only a known target version that is older than a +// known schema. +func compareBinaryToSchema(binaryVersion, schemaVersion string, src restoreSource) (admission.Warnings, field.ErrorList) { + if schemaVersion == "" { return admission.Warnings{ - fmt.Sprintf("backup %q has no recorded schema version: schema-version compatibility cannot be verified; "+ - "ensure the target binary version is >= the backup's schema version to avoid data corruption", backupName), + fmt.Sprintf("%s %q has no recorded schema version: schema-version compatibility cannot be verified; "+ + "ensure the target DocumentDB version (spec.documentDBVersion or spec.image.documentDB) is >= the source's "+ + "schema version to avoid data corruption", src.kind, src.name), }, nil } - - binaryVersion := resolveBinaryVersion(newDB) if binaryVersion == "" { return admission.Warnings{ - fmt.Sprintf("cannot determine the target binary version for restore from backup %q "+ - "(set spec.documentDBVersion or spec.image.documentDB): schema-version compatibility (backup schema %s) cannot be verified", - backupName, backupSchemaVersion), + fmt.Sprintf("cannot determine the target DocumentDB version for restore from %s %q "+ + "(set spec.documentDBVersion or spec.image.documentDB): compatibility with the %s's schema version %s cannot be verified", + src.kind, src.name, src.kind, schemaVersion), }, nil } binaryExtensionVersion := util.SemverToExtensionVersion(binaryVersion) - schemaExtensionVersion := util.SemverToExtensionVersion(backupSchemaVersion) + schemaExtensionVersion := util.SemverToExtensionVersion(schemaVersion) cmp, err := util.CompareExtensionVersions(binaryExtensionVersion, schemaExtensionVersion) if err != nil { return admission.Warnings{ - fmt.Sprintf("cannot compare restore binary version %s with backup schema version %s: %v; compatibility not verified", - binaryVersion, backupSchemaVersion, err), + fmt.Sprintf("cannot compare the target DocumentDB version %s with the %s's schema version %s: %v; compatibility not verified", + binaryVersion, src.kind, schemaVersion, err), }, nil } if cmp < 0 { return nil, field.ErrorList{field.Forbidden( - field.NewPath("spec", "bootstrap", "recovery", "backup"), + src.fieldPath, fmt.Sprintf( - "restore blocked: target binary version %s is older than the backup's schema version %s. "+ - "Restoring onto an older binary runs it against a newer, irreversible schema and may cause data corruption. "+ - "Set spec.documentDBVersion (or spec.image.documentDB) to a version >= %s.", - binaryVersion, backupSchemaVersion, backupSchemaVersion), + "restore blocked: the target DocumentDB version %s is older than the %s's schema version %s. "+ + "Restoring onto an older DocumentDB version runs it against a newer, irreversible schema and may cause data corruption. "+ + "Set spec.documentDBVersion (or spec.image.documentDB) to %s or newer.", + binaryVersion, src.kind, schemaVersion, schemaVersion), )} } return nil, nil @@ -431,7 +490,30 @@ func resolveBinaryVersion(db *dbpreview.DocumentDB) string { return db.Spec.DocumentDBVersion } -// specImageDocumentDB safely returns spec.image.documentDB or "" when unset. +// resolveEffectiveBinaryVersion returns the binary version the operator will +// actually run for db, including the operator-wide default the controller applies +// when the spec pins no version. Restore validation uses this rather than the +// spec-only resolveBinaryVersion so a restore whose effective binary would be +// older than the source schema is blocked, not merely warned. +func resolveEffectiveBinaryVersion(db *dbpreview.DocumentDB) string { + if v := resolveBinaryVersion(db); v != "" { + return v + } + // The spec pins no parseable version. Resolve the exact image the controller + // would actually run (a pinned but unparseable image, the DOCUMENTDB_VERSION + // env default, the ChangeStreams image, or the built-in default) via the shared + // helper, and read its semver tag. Anything without a parseable semver tag + // (a digest, or the changestream image) stays unknown so the restore is warned, + // not falsely blocked — keeping the webhook in step with the controller. + image := util.GetDocumentDBImageForInstance(db) + if tagIdx := strings.LastIndex(image, ":"); tagIdx >= 0 { + if semver := extractSemver(image[tagIdx+1:]); semver != "" { + return semver + } + } + return "" +} + func specImageDocumentDB(db *dbpreview.DocumentDB) string { if db == nil || db.Spec.Image == nil { return "" diff --git a/operator/src/internal/webhook/documentdb_webhook_test.go b/operator/src/internal/webhook/documentdb_webhook_test.go index 6fa4431cf..a4a207971 100644 --- a/operator/src/internal/webhook/documentdb_webhook_test.go +++ b/operator/src/internal/webhook/documentdb_webhook_test.go @@ -9,6 +9,7 @@ import ( cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/rest" @@ -18,6 +19,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" dbpreview "github.com/documentdb/documentdb-operator/api/preview" + util "github.com/documentdb/documentdb-operator/internal/utils" ) type fakeWebhookManager struct { @@ -352,6 +354,39 @@ var _ = Describe("resolveBinaryVersion helper", func() { }) }) +var _ = Describe("resolveEffectiveBinaryVersion helper", func() { + It("returns the spec version when one is set", func() { + db := newTestDocumentDB("0.112.0", "", "") + Expect(resolveEffectiveBinaryVersion(db)).To(Equal("0.112.0")) + }) + + It("falls back to the default image version when neither image nor version is set", func() { + db := newTestDocumentDB("", "", "") + Expect(resolveEffectiveBinaryVersion(db)).To(Equal("0.110.0")) + }) + + It("prefers the DOCUMENTDB_VERSION env override over the default", func() { + GinkgoT().Setenv(util.DOCUMENTDB_VERSION_ENV, "0.115.0") + db := newTestDocumentDB("", "", "") + Expect(resolveEffectiveBinaryVersion(db)).To(Equal("0.115.0")) + }) + + It("stays unknown for a digest-only image with no version (does not use the default)", func() { + db := newTestDocumentDB("", "", "ghcr.io/documentdb/documentdb@sha256:abc123") + Expect(resolveEffectiveBinaryVersion(db)).To(BeEmpty()) + }) + + It("stays unknown when the ChangeStreams gate selects a non-semver image", func() { + // With no version/env and the ChangeStreams gate on, the controller runs the + // changestream image (non-semver tag). The webhook must treat it as unknown + // (warn), not silently compare against the default, to stay in step with the + // controller. + db := newTestDocumentDB("", "", "") + db.Spec.FeatureGates = map[string]bool{dbpreview.FeatureGateChangeStreams: true} + Expect(resolveEffectiveBinaryVersion(db)).To(BeEmpty()) + }) +}) + var _ = Describe("extractSemver helper", func() { It("extracts clean semver", func() { Expect(extractSemver("0.112.0")).To(Equal("0.112.0")) @@ -590,10 +625,30 @@ func newBackupWithSchema(name, schemaVersion string) *dbpreview.Backup { func newValidatorWithObjects(objs ...ctrlclient.Object) *DocumentDBValidator { scheme := runtime.NewScheme() Expect(dbpreview.AddToScheme(scheme)).To(Succeed()) + Expect(corev1.AddToScheme(scheme)).To(Succeed()) fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() return &DocumentDBValidator{Client: fakeClient} } +func newPVRestoreDocumentDB(name, version, pvName string) *dbpreview.DocumentDB { + db := newTestDocumentDB(version, "", "") + db.Name = name + db.Spec.Bootstrap = &dbpreview.BootstrapConfiguration{ + Recovery: &dbpreview.RecoveryConfiguration{ + PersistentVolume: &dbpreview.PVRecoveryConfiguration{Name: pvName}, + }, + } + return db +} + +func newPVWithSchema(name, schemaVersion string) *corev1.PersistentVolume { + pv := &corev1.PersistentVolume{ObjectMeta: metav1.ObjectMeta{Name: name}} + if schemaVersion != "" { + pv.Annotations = map[string]string{util.AnnotationSchemaVersion: schemaVersion} + } + return pv +} + var _ = Describe("restore schema compatibility validation", func() { ctx := context.Background() @@ -652,14 +707,40 @@ var _ = Describe("restore schema compatibility validation", func() { Expect(warnings[0]).To(ContainSubstring("not found")) }) - It("warns when the restore binary version cannot be determined", func() { + It("blocks restore when no version is set and the default binary is older than the backup schema", func() { + // With no spec.documentDBVersion/image, the controller applies the operator + // default (DEFAULT_DOCUMENTDB_IMAGE, 0.110.0). Restoring a 0.112.0 schema onto + // it would run an older binary against a newer schema, so admission blocks it. + backup := newBackupWithSchema("bk", "0.112.0") + v := newValidatorWithObjects(backup) + db := newRestoreDocumentDB("restored", "", "bk") + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(warnings).To(BeEmpty()) + Expect(errs).To(HaveLen(1)) + Expect(errs[0].Detail).To(ContainSubstring("older than the backup's schema version")) + }) + + It("allows restore when no version is set and the default binary is >= the backup schema", func() { + backup := newBackupWithSchema("bk", "0.110.0") + v := newValidatorWithObjects(backup) + db := newRestoreDocumentDB("restored", "", "bk") + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(warnings).To(BeEmpty()) + Expect(errs).To(BeEmpty()) + }) + + It("warns when the restore binary version cannot be determined (digest-only image)", func() { + // A digest-only image pins an image whose version is unknown at admission, + // and it takes priority over the operator default, so compatibility can only + // be warned about, not verified. backup := newBackupWithSchema("bk", "0.112.0") v := newValidatorWithObjects(backup) db := newRestoreDocumentDB("restored", "", "bk") + db.Spec.Image = &dbpreview.ImageSpec{DocumentDB: "ghcr.io/documentdb/documentdb@sha256:abc123"} warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) Expect(errs).To(BeEmpty()) Expect(warnings).To(HaveLen(1)) - Expect(warnings[0]).To(ContainSubstring("cannot determine the target binary version")) + Expect(warnings[0]).To(ContainSubstring("cannot determine the target DocumentDB version")) }) It("warns when restoring from a PersistentVolume with an explicit version", func() { @@ -676,7 +757,7 @@ var _ = Describe("restore schema compatibility validation", func() { Expect(warnings[0]).To(ContainSubstring("PersistentVolume")) }) - It("rejects a PersistentVolume restore that omits an explicit binary version", func() { + It("warns (does not block) on a PersistentVolume restore that omits an explicit binary version", func() { v := newValidatorWithObjects() db := newTestDocumentDB("", "", "") db.Spec.Bootstrap = &dbpreview.BootstrapConfiguration{ @@ -685,9 +766,9 @@ var _ = Describe("restore schema compatibility validation", func() { }, } warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) - Expect(warnings).To(BeEmpty()) - Expect(errs).To(HaveLen(1)) - Expect(errs[0].Detail).To(ContainSubstring("requires an explicit spec.documentDBVersion")) + Expect(errs).To(BeEmpty()) + Expect(warnings).To(HaveLen(1)) + Expect(warnings[0]).To(ContainSubstring("PersistentVolume")) }) It("allows a PersistentVolume restore when only image.documentDB is set", func() { @@ -703,4 +784,42 @@ var _ = Describe("restore schema compatibility validation", func() { Expect(warnings).To(HaveLen(1)) Expect(warnings[0]).To(ContainSubstring("PersistentVolume")) }) + + It("allows a PersistentVolume restore when the PV annotation schema equals the binary version", func() { + pv := newPVWithSchema("pv-1", "0.112.0") + v := newValidatorWithObjects(pv) + db := newPVRestoreDocumentDB("restored", "0.112.0", "pv-1") + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(warnings).To(BeEmpty()) + Expect(errs).To(BeEmpty()) + }) + + It("allows a PersistentVolume restore when the binary version is newer than the PV annotation schema", func() { + pv := newPVWithSchema("pv-1", "0.110.0") + v := newValidatorWithObjects(pv) + db := newPVRestoreDocumentDB("restored", "0.112.0", "pv-1") + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(warnings).To(BeEmpty()) + Expect(errs).To(BeEmpty()) + }) + + It("rejects a PersistentVolume restore when the binary version is older than the PV annotation schema", func() { + pv := newPVWithSchema("pv-1", "0.112.0") + v := newValidatorWithObjects(pv) + db := newPVRestoreDocumentDB("restored", "0.110.0", "pv-1") + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(warnings).To(BeEmpty()) + Expect(errs).To(HaveLen(1)) + Expect(errs[0].Detail).To(ContainSubstring("older than the PersistentVolume's schema version")) + }) + + It("warns when the PV has no schema annotation (same as a backup with no recorded schema)", func() { + pv := newPVWithSchema("pv-1", "") + v := newValidatorWithObjects(pv) + db := newPVRestoreDocumentDB("restored", "", "pv-1") + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(errs).To(BeEmpty()) + Expect(warnings).To(HaveLen(1)) + Expect(warnings[0]).To(ContainSubstring("no recorded schema version")) + }) }) From 9048fc624fa99fd57760e10dac40f5018c0e1185 Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Wed, 5 Aug 2026 10:47:32 -0400 Subject: [PATCH 3/8] test(e2e): cover restore schema-version validation (#434) 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 #368. Refs #434 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b653a2f9-cc1a-400a-8e93-11232229023a Signed-off-by: Wenting Wu --- test/e2e/tests/backup/helpers_test.go | 19 +- .../tests/backup/schema_validation_test.go | 194 ++++++++++++++++++ 2 files changed, 209 insertions(+), 4 deletions(-) create mode 100644 test/e2e/tests/backup/schema_validation_test.go diff --git a/test/e2e/tests/backup/helpers_test.go b/test/e2e/tests/backup/helpers_test.go index 936d05c07..a897d2f83 100644 --- a/test/e2e/tests/backup/helpers_test.go +++ b/test/e2e/tests/backup/helpers_test.go @@ -91,10 +91,11 @@ func manifestsRoot() string { return filepath.Join(filepath.Dir(thisFile), "..", "..", "manifests") } -// createRecoveryDocumentDB renders a flat recovery_* template under -// manifests/backup/ and applies the resulting DocumentDB CR. -func createRecoveryDocumentDB( - ctx context.Context, c client.Client, +// buildRecoveryDocumentDB renders a flat recovery_* template under +// manifests/backup/ and returns the resulting DocumentDB CR *without* +// creating it, so callers (e.g. admission-rejection specs) can mutate +// the spec before calling Create themselves. +func buildRecoveryDocumentDB( ns, name, templateName string, extra map[string]string, ) *previewv1.DocumentDB { vars := baseVars(name, ns, "") @@ -112,6 +113,16 @@ func createRecoveryDocumentDB( if dd.Name == "" { dd.Name = name } + return dd +} + +// createRecoveryDocumentDB renders a flat recovery_* template under +// manifests/backup/ and applies the resulting DocumentDB CR. +func createRecoveryDocumentDB( + ctx context.Context, c client.Client, + ns, name, templateName string, extra map[string]string, +) *previewv1.DocumentDB { + dd := buildRecoveryDocumentDB(ns, name, templateName, extra) Expect(c.Create(ctx, dd)).To(Succeed(), "create recovery DocumentDB %s/%s", ns, name) return dd } diff --git a/test/e2e/tests/backup/schema_validation_test.go b/test/e2e/tests/backup/schema_validation_test.go new file mode 100644 index 000000000..c428b0f94 --- /dev/null +++ b/test/e2e/tests/backup/schema_validation_test.go @@ -0,0 +1,194 @@ +package backup + +import ( + "context" + "time" + + . "github.com/onsi/ginkgo/v2" //nolint:revive + . "github.com/onsi/gomega" //nolint:revive + + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + previewv1 "github.com/documentdb/documentdb-operator/api/preview" + "github.com/documentdb/documentdb-operator/test/e2e" + "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/assertions" + bkp "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/backup" + "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/documentdb" + "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/namespaces" + "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/timeouts" + shareddb "github.com/documentdb/documentdb-operator/test/shared/documentdb" +) + +// schemaVersionAnnotation mirrors util.AnnotationSchemaVersion in the +// operator. It is intentionally hard-coded here so this e2e spec pins +// the exact on-the-wire annotation key the restore-validation contract +// depends on — a rename on the operator side must be a conscious, +// two-place change. +const schemaVersionAnnotation = "documentdb.io/schema-version" + +// olderBinaryVersion is a semver guaranteed to be lower than any real +// DocumentDB extension schema version (which are 0.1x.y). Restores +// pinned to it are rejected at admission *before* any image pull, so it +// never needs to resolve to a pullable tag. +const olderBinaryVersion = "0.1.0" + +// sourceSchemaVersion polls a source DocumentDB's status.schemaVersion +// until it is non-empty, returning the observed value. The operator +// records this on Backups and stamps it onto retained PVs; both restore +// validations compare a restore's binary version against it. +func sourceSchemaVersion(ctx context.Context, c client.Client, key types.NamespacedName) string { + var observed string + Eventually(func() string { + dd, err := shareddb.Get(ctx, c, key) + if err != nil { + return "" + } + observed = dd.Status.SchemaVersion + return observed + }, timeouts.For(timeouts.DocumentDBReady), timeouts.PollInterval(timeouts.DocumentDBReady)). + ShouldNot(BeEmpty(), "source %s never reported status.schemaVersion", key) + return observed +} + +// pinBinaryVersion forces a restore CR's resolved binary version to v by +// clearing spec.image (whose tag would otherwise win in resolveBinaryVersion) +// and setting spec.documentDBVersion. This makes the admission decision +// deterministic regardless of the DOCUMENTDB_IMAGE the CI job injects. +func pinBinaryVersion(dd *previewv1.DocumentDB, v string) { + dd.Spec.Image = nil + dd.Spec.DocumentDBVersion = v +} + +var _ = Describe("DocumentDB restore — schema-version compatibility (#434)", + Label(e2e.BackupLabel, e2e.NeedsCSISnapshotsLabel, e2e.SlowLabel), e2e.MediumLevelLabel, + func() { + var ( + ctx context.Context + ns string + c client.Client + ) + + BeforeEach(func() { + e2e.SkipUnlessLevel(e2e.Medium) + ctx = context.Background() + c = e2e.SuiteEnv().Client + skipUnlessCSISnapshotsUsable(ctx, c) + ns = namespaces.NamespaceForSpec(e2e.BackupLabel) + createNamespace(ctx, c, ns) + createCredentialSecret(ctx, c, ns) + }) + + It("records the schema version on the Backup and blocks restore onto an older binary", func() { + const ( + sourceName = "schema-bk-src" + backupName = "schema-bk-src-backup" + ) + + // Source cluster → Ready, then capture its installed schema version. + src, err := documentdb.Create(ctx, c, ns, sourceName, documentdb.CreateOptions{ + Base: "documentdb", + Vars: baseVars(sourceName, ns, "2Gi"), + ManifestsRoot: manifestsRoot(), + }) + Expect(err).NotTo(HaveOccurred()) + DeferCleanup(func(ctx SpecContext) { _ = shareddb.Delete(ctx, c, src, 3*time.Minute) }) + + srcKey := types.NamespacedName{Namespace: ns, Name: sourceName} + Eventually(assertions.AssertDocumentDBReady(ctx, c, srcKey), + timeouts.For(timeouts.DocumentDBReady), + timeouts.PollInterval(timeouts.DocumentDBReady), + ).Should(Succeed()) + schema := sourceSchemaVersion(ctx, c, srcKey) + + // On-demand Backup → Completed, and the operator must have + // recorded the source's schema version onto its status. + _, err = bkp.Create(ctx, c, bkp.BackupVars{ + Name: backupName, Namespace: ns, ClusterName: sourceName, RetentionDays: 1, + }) + Expect(err).NotTo(HaveOccurred()) + DeferCleanup(func(ctx SpecContext) { _ = bkp.Delete(ctx, c, ns, backupName, 1*time.Minute) }) + completed, err := bkp.WaitForCompleted(ctx, c, ns, backupName, timeouts.For(timeouts.BackupComplete)) + Expect(err).NotTo(HaveOccurred(), "backup %s/%s did not complete", ns, backupName) + Expect(completed).NotTo(BeNil()) + Eventually(func() string { + b, err := bkp.Get(ctx, c, ns, backupName) + if err != nil { + return "" + } + return b.Status.SchemaVersion + }, timeouts.For(timeouts.DocumentDBReady), timeouts.PollInterval(timeouts.DocumentDBReady)). + Should(Equal(schema), "Backup.Status.SchemaVersion must record the source cluster's schema version") + + By("rejecting a restore whose binary version is older than the backup's schema version") + bad := buildRecoveryDocumentDB(ns, "schema-bk-dst-bad", + "recovery_from_backup.yaml.template", + map[string]string{"BACKUP_NAME": backupName}) + pinBinaryVersion(bad, olderBinaryVersion) + err = c.Create(ctx, bad) + Expect(err).To(HaveOccurred(), "restore onto an older binary must be rejected at admission") + Expect(err.Error()).To(ContainSubstring("older than the backup's schema version")) + + By("admitting a restore whose binary version matches the backup's schema version") + good := buildRecoveryDocumentDB(ns, "schema-bk-dst-good", + "recovery_from_backup.yaml.template", + map[string]string{"BACKUP_NAME": backupName}) + pinBinaryVersion(good, schema) + Expect(c.Create(ctx, good)).To(Succeed(), + "restore onto a binary >= the backup's schema version must be admitted") + DeferCleanup(func(ctx SpecContext) { _ = shareddb.Delete(ctx, c, good, 3*time.Minute) }) + }) + + It("stamps the schema version on the PV and blocks PV restore onto an older binary", func() { + const sourceName = "schema-pv-src" + + // Source cluster → Ready; the PV controller stamps the schema + // version onto the backing PV as an annotation. + src, err := documentdb.Create(ctx, c, ns, sourceName, documentdb.CreateOptions{ + Base: "documentdb", + Vars: baseVars(sourceName, ns, "2Gi"), + ManifestsRoot: manifestsRoot(), + }) + Expect(err).NotTo(HaveOccurred()) + srcKey := types.NamespacedName{Namespace: ns, Name: sourceName} + Eventually(assertions.AssertDocumentDBReady(ctx, c, srcKey), + timeouts.For(timeouts.DocumentDBReady), + timeouts.PollInterval(timeouts.DocumentDBReady), + ).Should(Succeed()) + schema := sourceSchemaVersion(ctx, c, srcKey) + + By("waiting for the PV controller to stamp the schema-version annotation") + var pvName string + Eventually(func(g Gomega) { + pv, err := bkp.FindRetainedPV(ctx, c, ns, sourceName) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(pv).NotTo(BeNil()) + g.Expect(pv.Annotations).To(HaveKeyWithValue(schemaVersionAnnotation, schema), + "PV must be annotated with the source's schema version") + pvName = pv.Name + }, timeouts.For(timeouts.DocumentDBReady), timeouts.PollInterval(timeouts.DocumentDBReady)). + Should(Succeed()) + + // Delete the source (reclaimPolicy Retain by default) so the + // annotated PV survives for a restore. + Expect(shareddb.Delete(ctx, c, src, 3*time.Minute)).To(Succeed()) + + By("rejecting a PV restore whose binary version is older than the PV's schema version") + bad := buildRecoveryDocumentDB(ns, "schema-pv-dst-bad", + "recovery_from_pv.yaml.template", + map[string]string{"PV_NAME": pvName}) + pinBinaryVersion(bad, olderBinaryVersion) + err = c.Create(ctx, bad) + Expect(err).To(HaveOccurred(), "PV restore onto an older binary must be rejected at admission") + Expect(err.Error()).To(ContainSubstring("older than the PersistentVolume's schema version")) + + By("admitting a PV restore whose binary version matches the PV's schema version") + good := buildRecoveryDocumentDB(ns, "schema-pv-dst-good", + "recovery_from_pv.yaml.template", + map[string]string{"PV_NAME": pvName}) + pinBinaryVersion(good, schema) + Expect(c.Create(ctx, good)).To(Succeed(), + "PV restore onto a binary >= the PV's schema version must be admitted") + DeferCleanup(func(ctx SpecContext) { _ = shareddb.Delete(ctx, c, good, 3*time.Minute) }) + }) + }) From 84937b714c5523c2649901200a7cb2271e86abac Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Tue, 4 Aug 2026 14:57:37 -0400 Subject: [PATCH 4/8] feat: validate PV-restore schema compatibility via PV annotation 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 #434 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b653a2f9-cc1a-400a-8e93-11232229023a Signed-off-by: Wenting Wu --- .../preview/operations/backup-and-restore.md | 45 ++-- .../operations/restore-deleted-cluster.md | 19 +- .../src/internal/controller/pv_controller.go | 32 ++- .../internal/controller/pv_controller_test.go | 82 ++++++- operator/src/internal/utils/pv_recovery.go | 7 + .../internal/webhook/documentdb_webhook.go | 200 ++++++++++++------ .../webhook/documentdb_webhook_test.go | 131 +++++++++++- 7 files changed, 406 insertions(+), 110 deletions(-) diff --git a/docs/operator-public-documentation/preview/operations/backup-and-restore.md b/docs/operator-public-documentation/preview/operations/backup-and-restore.md index 06f2b41ee..e79caddbb 100644 --- a/docs/operator-public-documentation/preview/operations/backup-and-restore.md +++ b/docs/operator-public-documentation/preview/operations/backup-and-restore.md @@ -166,9 +166,8 @@ List backups for your DocumentDB cluster and choose one in `completed` status: kubectl get backups -n ``` -The `SchemaVersion` column shows the DocumentDB extension schema version the backup -was taken at. Note this value — you need it to pick a compatible binary version for -the restore (see [Version compatibility](#version-compatibility) below): +The `SchemaVersion` column shows the schema version the backup was taken at. You +need it to choose a compatible version for the restore: ```bash kubectl get backup my-backup -n -o jsonpath='{.status.schemaVersion}' @@ -176,32 +175,21 @@ kubectl get backup my-backup -n -o jsonpath='{.status.schemaVersion} ### Version compatibility -A restore is a **physical** recovery: the restored database — including the -installed `documentdb` extension schema — comes back at the schema version that -existed **when the backup was taken**. The restored cluster's binary version is -chosen independently by the new cluster's `documentDBVersion` (or `image`). +A restore brings your data back at the schema version from **when the backup was +taken**. Set the new cluster's `documentDBVersion` to **that version or newer** — +ideally the same version the backup was taken with. Never restore onto an **older** +version: it runs against a newer, irreversible schema and may cause **data +corruption**. -!!! danger "Restore onto a binary version >= the backup's schema version" - You must restore onto a DocumentDB binary whose version is **greater than or - equal to** the backup's schema version — ideally the **same** `documentDBVersion` - the backup was taken with. Restoring onto an **older** binary runs it against a - newer, irreversible schema and may cause **data corruption**: schema upgrades - (`ALTER EXTENSION UPDATE`) have no downgrade path. - -How the rule is enforced and reported: - -| Restore binary vs. backup schema | Behavior | +| Restore version vs. backup schema | Behavior | | --- | --- | -| `binary == schema` | Allowed. No schema change. | -| `binary > schema` | Allowed. The schema stays at the backup version until you opt into an upgrade via `spec.schemaVersion` (see [Upgrades](upgrades.md)). | -| `binary < schema` | **Rejected** at admission when the backup has a recorded schema version. | -| schema version unknown | Allowed with an **admission warning** — compatibility could **not** be verified. This applies when the backup predates schema-version recording. For **PersistentVolume/volume-snapshot** restores (which carry no schema metadata), an explicit `documentDBVersion` (or `image.documentDB`) is **required** and the restore is otherwise allowed with a warning — verify the version manually. | +| equal or newer | Allowed | +| older | **Rejected** at admission (when the schema version is known) | +| schema version unknown | Allowed with a warning — verify the version yourself | -To determine a backup's schema version when it is not recorded on the `Backup`, -record the source cluster's `status.schemaVersion` **before** taking the backup -(recommended operational practice), or restore into a cluster whose binary is `>=` -the expected version and then read -`kubectl get documentdb -o jsonpath='{.status.schemaVersion}'`. +The schema version is known from the `Backup`'s `status.schemaVersion` or a +retained PV's `documentdb.io/schema-version` annotation. Older backups or PVs +imported from outside the operator may not have it recorded. ### Step 2: Create a New DocumentDB Cluster @@ -214,8 +202,7 @@ metadata: spec: nodeCount: 1 instancesPerNode: 1 - # Set to a version >= the backup's schema version (ideally the version the - # backup was taken with). See "Version compatibility" above. + # Set to the backup's schema version or newer (see Version compatibility). documentDBVersion: "0.110.0" resource: storage: @@ -247,7 +234,7 @@ Once the status shows `Cluster in healthy state`, connect and verify your data. - The backup must be in `completed` status. - The VolumeSnapshot referenced by the backup must still exist — if it was manually deleted, the backup cannot be used for recovery. - You cannot specify both `backup` and `persistentVolume` in the same recovery spec. -- The restore binary version must be **>= the backup's schema version**. Restores from a `Backup` with a recorded schema version are enforced at admission; PV-based restores and backups without a recorded schema version are allowed with a warning only. See [Version compatibility](#version-compatibility). +- The restore version must be **>= the backup's schema version** when that version is known (from the `Backup` or a PV's `documentdb.io/schema-version` annotation); otherwise the restore is allowed with a warning. See [Version compatibility](#version-compatibility). For additional recovery options (including PV-based recovery), see [Restore a Deleted DocumentDB Cluster](restore-deleted-cluster.md). diff --git a/docs/operator-public-documentation/preview/operations/restore-deleted-cluster.md b/docs/operator-public-documentation/preview/operations/restore-deleted-cluster.md index 196d2057a..3a3c2c70a 100644 --- a/docs/operator-public-documentation/preview/operations/restore-deleted-cluster.md +++ b/docs/operator-public-documentation/preview/operations/restore-deleted-cluster.md @@ -48,14 +48,14 @@ The PV should be in `Released` or `Available` status. ### Step 2: Create a New DocumentDB Cluster with PV Recovery -!!! warning "PV restore requires an explicit `documentDBVersion`" - Unlike a backup, a retained PV carries **no readable schema-version metadata**, so - the operator cannot verify version compatibility at admission time. You **must** set - `spec.documentDBVersion` (or `spec.image.documentDB`) explicitly, and it must be - **>= the schema version of the data on the PV** — ideally the version the original - cluster was running. A PV restore that omits an explicit version is rejected. - Restoring onto an older binary risks **data corruption** (see - [Backup and Restore → Version compatibility](backup-and-restore.md#version-compatibility)). +!!! warning "Schema-version compatibility" + Set `documentDBVersion` to the schema version of the data on the PV **or newer** — + never older, which risks **data corruption**. Retained PVs carry a + `documentdb.io/schema-version` annotation; when present, admission rejects a + restore onto an older version. If the annotation is missing (e.g. a PV imported + from outside the operator), the version can't be verified and the restore is + allowed with a warning — set the right version yourself. See + [Version compatibility](backup-and-restore.md#version-compatibility). ```yaml title="restore-from-pv.yaml" apiVersion: documentdb.io/preview @@ -66,8 +66,7 @@ metadata: spec: nodeCount: 1 instancesPerNode: 1 - # Required for PV restore. Set to a version >= the schema version of the data - # on the PV (ideally the version the original cluster was running). + # Set to the PV data's schema version or newer (see Version compatibility). documentDBVersion: "0.110.0" documentDbCredentialSecret: documentdb-credentials resource: diff --git a/operator/src/internal/controller/pv_controller.go b/operator/src/internal/controller/pv_controller.go index 7b23feca0..63b5330d5 100644 --- a/operator/src/internal/controller/pv_controller.go +++ b/operator/src/internal/controller/pv_controller.go @@ -151,6 +151,25 @@ func (r *PersistentVolumeReconciler) applyDesiredPVConfiguration(ctx context.Con needsUpdate = true } + // Stamp the installed schema version so a future PV restore can validate + // binary/schema compatibility at admission time. Only set it once the cluster + // has reported a schema version; never clear a previously stamped value (a + // transient empty status must not erase the last-known-good version, which is + // what a retained PV would be restored from). + if schemaVersion := documentdb.Status.SchemaVersion; schemaVersion != "" && + pv.Annotations[util.AnnotationSchemaVersion] != schemaVersion { + if pv.Annotations == nil { + pv.Annotations = make(map[string]string) + } + logger.Info("PV schema-version annotation needs update", + "pv", pv.Name, + "currentValue", pv.Annotations[util.AnnotationSchemaVersion], + "desiredValue", schemaVersion, + "documentdb", documentdb.Name) + pv.Annotations[util.AnnotationSchemaVersion] = schemaVersion + needsUpdate = true + } + // Check if reclaim policy needs update desiredPolicy := r.getDesiredReclaimPolicy(documentdb) if pv.Spec.PersistentVolumeReclaimPolicy != desiredPolicy { @@ -402,17 +421,21 @@ func (r *PersistentVolumeReconciler) SetupWithManager(mgr ctrl.Manager) error { // Apply pvPredicate only to PersistentVolume events, not globally For(&corev1.PersistentVolume{}, builder.WithPredicates(pvPredicate())). // Watch DocumentDB changes and trigger reconciliation of associated PVs + // when the reclaim policy or the installed schema version changes. Watches( &dbpreview.DocumentDB{}, handler.EnqueueRequestsFromMapFunc(r.findPVsForDocumentDB), - builder.WithPredicates(documentDBReclaimPolicyPredicate()), + builder.WithPredicates(documentDBPVRelevantPredicate()), ). Named("pv-controller"). Complete(r) } -// documentDBReclaimPolicyPredicate only triggers when the reclaim policy field changes -func documentDBReclaimPolicyPredicate() predicate.Predicate { +// documentDBPVRelevantPredicate triggers PV reconciliation when a DocumentDB +// change affects PV-level state: the reclaim policy (mutates the PV spec) or the +// installed schema version (stamped onto the PV as an annotation for restore +// compatibility validation). +func documentDBPVRelevantPredicate() predicate.Predicate { return predicate.Funcs{ UpdateFunc: func(e event.UpdateEvent) bool { oldDB, ok := e.ObjectOld.(*dbpreview.DocumentDB) @@ -423,7 +446,8 @@ func documentDBReclaimPolicyPredicate() predicate.Predicate { if !ok { return false } - return oldDB.Spec.Resource.Storage.PersistentVolumeReclaimPolicy != newDB.Spec.Resource.Storage.PersistentVolumeReclaimPolicy + return oldDB.Spec.Resource.Storage.PersistentVolumeReclaimPolicy != newDB.Spec.Resource.Storage.PersistentVolumeReclaimPolicy || + oldDB.Status.SchemaVersion != newDB.Status.SchemaVersion }, CreateFunc: func(e event.CreateEvent) bool { return false }, DeleteFunc: func(e event.DeleteEvent) bool { return false }, diff --git a/operator/src/internal/controller/pv_controller_test.go b/operator/src/internal/controller/pv_controller_test.go index fc1fd1419..f312abf5d 100644 --- a/operator/src/internal/controller/pv_controller_test.go +++ b/operator/src/internal/controller/pv_controller_test.go @@ -277,6 +277,71 @@ var _ = Describe("PersistentVolume Controller", func() { needsUpdate := reconciler.applyDesiredPVConfiguration(ctx, pv, documentdb) Expect(needsUpdate).To(BeFalse()) }) + + It("stamps the schema-version annotation from DocumentDB status", func() { + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: pvName, + Labels: map[string]string{ + util.LabelCluster: documentdbName, + util.LabelNamespace: testNamespace, + }, + }, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimRetain, + MountOptions: []string{"nodev", "noexec", "nosuid"}, + }, + } + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{Name: documentdbName, Namespace: testNamespace}, + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PersistentVolumeReclaimPolicy: "Retain", + }, + }, + }, + Status: dbpreview.DocumentDBStatus{SchemaVersion: "0.112.0"}, + } + + needsUpdate := reconciler.applyDesiredPVConfiguration(ctx, pv, documentdb) + Expect(needsUpdate).To(BeTrue()) + Expect(pv.Annotations[util.AnnotationSchemaVersion]).To(Equal("0.112.0")) + }) + + It("does not clear the schema-version annotation when status is empty", func() { + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: pvName, + Labels: map[string]string{ + util.LabelCluster: documentdbName, + util.LabelNamespace: testNamespace, + }, + Annotations: map[string]string{ + util.AnnotationSchemaVersion: "0.112.0", + }, + }, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimRetain, + MountOptions: []string{"nodev", "noexec", "nosuid"}, + }, + } + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{Name: documentdbName, Namespace: testNamespace}, + Spec: dbpreview.DocumentDBSpec{ + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PersistentVolumeReclaimPolicy: "Retain", + }, + }, + }, + // No Status.SchemaVersion set. + } + + needsUpdate := reconciler.applyDesiredPVConfiguration(ctx, pv, documentdb) + Expect(needsUpdate).To(BeFalse()) + Expect(pv.Annotations[util.AnnotationSchemaVersion]).To(Equal("0.112.0")) + }) }) Describe("provisionerSupportsMountOptions", func() { @@ -955,11 +1020,11 @@ var _ = Describe("PersistentVolume Controller", func() { }) }) - Describe("documentDBReclaimPolicyPredicate", func() { + Describe("documentDBPVRelevantPredicate", func() { var pred predicate.Predicate BeforeEach(func() { - pred = documentDBReclaimPolicyPredicate() + pred = documentDBPVRelevantPredicate() }) Describe("UpdateFunc", func() { @@ -1013,6 +1078,19 @@ var _ = Describe("PersistentVolume Controller", func() { Expect(pred.Update(e)).To(BeFalse()) }) + It("returns true when the installed schema version changes", func() { + oldDB := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{Name: documentdbName, Namespace: testNamespace}, + Status: dbpreview.DocumentDBStatus{SchemaVersion: "0.110.0"}, + } + newDB := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{Name: documentdbName, Namespace: testNamespace}, + Status: dbpreview.DocumentDBStatus{SchemaVersion: "0.112.0"}, + } + e := event.UpdateEvent{ObjectOld: oldDB, ObjectNew: newDB} + Expect(pred.Update(e)).To(BeTrue()) + }) + It("returns false for non-DocumentDB objects", func() { pvc := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{Name: pvcName, Namespace: testNamespace}, diff --git a/operator/src/internal/utils/pv_recovery.go b/operator/src/internal/utils/pv_recovery.go index 16cd387e3..257bec157 100644 --- a/operator/src/internal/utils/pv_recovery.go +++ b/operator/src/internal/utils/pv_recovery.go @@ -17,6 +17,13 @@ const ( // Label for identifying the DocumentDB cluster a PV/PVC belongs to LabelCluster = "documentdb.io/cluster" LabelNamespace = "documentdb.io/namespace" + + // AnnotationSchemaVersion records, on a PV, the DocumentDB extension schema + // version installed in the data on that volume. It is stamped by the PV + // controller from DocumentDB.Status.SchemaVersion and read at admission time + // to validate PV-restore binary/schema compatibility (the schema version is + // otherwise not readable from a PV without booting PostgreSQL). + AnnotationSchemaVersion = "documentdb.io/schema-version" ) // TempPVCNameForPVRecovery generates the name for a temporary PVC used during PV recovery. diff --git a/operator/src/internal/webhook/documentdb_webhook.go b/operator/src/internal/webhook/documentdb_webhook.go index a5fb9108e..ef85da430 100644 --- a/operator/src/internal/webhook/documentdb_webhook.go +++ b/operator/src/internal/webhook/documentdb_webhook.go @@ -10,6 +10,7 @@ import ( "strconv" "strings" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/runtime/schema" @@ -229,10 +230,16 @@ func (v *DocumentDBValidator) validateImageRollback(newDB, oldDB *dbpreview.Docu // --------------------------------------------------------------------------- // validateRestoreSchemaCompatibility validates that a restore's target binary -// version is compatible with the schema version the source backup was taken at. +// version is compatible with the schema version the source was taken at. +// +// "Binary version" here is the DocumentDB extension version the new cluster will +// run (resolved from spec.documentDBVersion / spec.image.documentDB, or the +// operator default) — the same value surfaced to users in messages as the +// "DocumentDB version". The "schema version" is the extension schema version the +// source data was written at. // // A restore is a physical recovery: the restored catalog (including the -// documentdb extension schema) comes back at backup-time schema version, while +// documentdb extension schema) comes back at source-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 // rollback guard (validateImageRollback) cannot catch it because a fresh restore @@ -241,92 +248,144 @@ func (v *DocumentDBValidator) validateImageRollback(newDB, oldDB *dbpreview.Docu // Rules: // - binary >= schema → allowed (schema catch-up handled by the two-phase upgrade flow) // - binary < schema → rejected -// - schema/binary version unknown → allowed with a warning +// - schema or binary version unknown → allowed with a warning // -// Backup-CR restores read the schema version from Backup.Status.SchemaVersion. -// PV/volume-snapshot restores carry no admission-readable schema metadata, so -// they instead require an explicit binary version (spec.documentDBVersion or -// spec.image.documentDB) and are otherwise allowed with a warning. +// It orchestrates three single-purpose steps: identify the restore source, +// resolve that source's schema version, and compare it against the effective +// binary version. Backup-CR restores read the schema from Backup.Status; PV +// restores read it from the source PV's annotation (stamped by the PV +// controller). Both sources follow the same logic: when the schema version +// cannot be determined the restore is allowed with a warning; only a resolved +// binary older than a known schema is rejected. func (v *DocumentDBValidator) validateRestoreSchemaCompatibility(ctx context.Context, newDB *dbpreview.DocumentDB) (admission.Warnings, field.ErrorList) { if newDB.Spec.Bootstrap == nil || newDB.Spec.Bootstrap.Recovery == nil { return nil, nil } - recovery := newDB.Spec.Bootstrap.Recovery - - // Non-backup restore (PV / volume snapshot): the schema version lives inside - // the restored data directory and cannot be read at admission time. - if recovery.Backup.Name == "" { - if recovery.PersistentVolume != nil && recovery.PersistentVolume.Name != "" { - // Require an explicit binary version so the restore is not silently - // pinned to the default image, which may be older than the restored - // schema and risk data corruption. We still cannot verify the exact - // number against the disk, so we also warn. - if newDB.Spec.DocumentDBVersion == "" && specImageDocumentDB(newDB) == "" { - return nil, field.ErrorList{field.Required( - field.NewPath("spec", "documentDBVersion"), - "restoring from a PersistentVolume requires an explicit spec.documentDBVersion "+ - "(or spec.image.documentDB) that is >= the schema version of the restored data; "+ - "the schema version cannot be verified at admission time for PV restores, so it must be set deliberately", - )} - } - return admission.Warnings{ - "restoring from a PersistentVolume: schema-version compatibility cannot be verified at admission time; " + - "ensure the specified binary version is >= the schema version of the restored data to avoid data corruption", - }, nil + src, ok := restoreSourceFor(newDB.Spec.Bootstrap.Recovery) + if !ok { + return nil, nil // no recognizable restore source to validate + } + + schemaVersion, warnings := v.resolveSourceSchemaVersion(ctx, newDB.Namespace, src) + if len(warnings) > 0 { + return warnings, nil + } + + return compareBinaryToSchema(resolveEffectiveBinaryVersion(newDB), schemaVersion, src) +} + +// restoreSource identifies where a restore draws its data — and thus its schema +// version — from, carrying the diagnostics and the spec field to flag on error. +type restoreSource struct { + kind string // sourceKindBackup or sourceKindPV + name string + fieldPath *field.Path +} + +const ( + sourceKindBackup = "backup" + sourceKindPV = "PersistentVolume" +) + +// restoreSourceFor identifies the restore source from a recovery configuration, +// returning false when neither a backup nor a PersistentVolume source is set. +func restoreSourceFor(recovery *dbpreview.RecoveryConfiguration) (restoreSource, bool) { + if recovery.Backup.Name != "" { + return restoreSource{ + kind: sourceKindBackup, + name: recovery.Backup.Name, + fieldPath: field.NewPath("spec", "bootstrap", "recovery", "backup"), + }, true + } + if recovery.PersistentVolume != nil && recovery.PersistentVolume.Name != "" { + return restoreSource{ + kind: sourceKindPV, + name: recovery.PersistentVolume.Name, + fieldPath: field.NewPath("spec", "bootstrap", "recovery", "persistentVolume"), + }, true + } + return restoreSource{}, false +} + +// resolveSourceSchemaVersion reads the restore source and returns its recorded +// schema version. It only reports warnings for I/O failures (source not found or +// unreadable); a successfully read source with no schema version returns ("", +// nil), leaving the "unknown schema" policy to compareBinaryToSchema so that both +// empty-input cases are decided in one place. Backup and PV are handled +// identically. +func (v *DocumentDBValidator) resolveSourceSchemaVersion(ctx context.Context, namespace string, src restoreSource) (string, admission.Warnings) { + var schemaVersion string + var readErr error + + switch src.kind { + case sourceKindBackup: + backup := &dbpreview.Backup{} + if err := v.Get(ctx, client.ObjectKey{Name: src.name, Namespace: namespace}, backup); err != nil { + readErr = err + } else { + schemaVersion = backup.Status.SchemaVersion + } + case sourceKindPV: + pv := &corev1.PersistentVolume{} + if err := v.Get(ctx, client.ObjectKey{Name: src.name}, pv); err != nil { + readErr = err + } else { + schemaVersion = pv.Annotations[util.AnnotationSchemaVersion] } - return nil, nil } - backupName := recovery.Backup.Name - backup := &dbpreview.Backup{} - if err := v.Get(ctx, client.ObjectKey{Name: backupName, Namespace: newDB.Namespace}, backup); err != nil { - if apierrors.IsNotFound(err) { - return admission.Warnings{ - fmt.Sprintf("backup %q not found in namespace %q: schema-version compatibility cannot be verified", - backupName, newDB.Namespace), - }, nil + if readErr != nil { + if apierrors.IsNotFound(readErr) { + return "", admission.Warnings{ + fmt.Sprintf("%s %q not found: schema-version compatibility cannot be verified", src.kind, src.name), + } } // Transient/API error: don't hard-block the restore, but warn. - return admission.Warnings{ - fmt.Sprintf("failed to read backup %q: schema-version compatibility cannot be verified: %v", backupName, err), - }, nil + return "", admission.Warnings{ + fmt.Sprintf("failed to read %s %q: schema-version compatibility cannot be verified: %v", src.kind, src.name, readErr), + } } + return schemaVersion, nil +} - backupSchemaVersion := backup.Status.SchemaVersion - if backupSchemaVersion == "" { +// compareBinaryToSchema is the single authority on the restore decision. It warns +// (and allows) when either the source schema version or the target DocumentDB +// version is unknown, and rejects only a known target version that is older than a +// known schema. +func compareBinaryToSchema(binaryVersion, schemaVersion string, src restoreSource) (admission.Warnings, field.ErrorList) { + if schemaVersion == "" { return admission.Warnings{ - fmt.Sprintf("backup %q has no recorded schema version: schema-version compatibility cannot be verified; "+ - "ensure the target binary version is >= the backup's schema version to avoid data corruption", backupName), + fmt.Sprintf("%s %q has no recorded schema version: schema-version compatibility cannot be verified; "+ + "ensure the target DocumentDB version (spec.documentDBVersion or spec.image.documentDB) is >= the source's "+ + "schema version to avoid data corruption", src.kind, src.name), }, nil } - - binaryVersion := resolveBinaryVersion(newDB) if binaryVersion == "" { return admission.Warnings{ - fmt.Sprintf("cannot determine the target binary version for restore from backup %q "+ - "(set spec.documentDBVersion or spec.image.documentDB): schema-version compatibility (backup schema %s) cannot be verified", - backupName, backupSchemaVersion), + fmt.Sprintf("cannot determine the target DocumentDB version for restore from %s %q "+ + "(set spec.documentDBVersion or spec.image.documentDB): compatibility with the %s's schema version %s cannot be verified", + src.kind, src.name, src.kind, schemaVersion), }, nil } binaryExtensionVersion := util.SemverToExtensionVersion(binaryVersion) - schemaExtensionVersion := util.SemverToExtensionVersion(backupSchemaVersion) + schemaExtensionVersion := util.SemverToExtensionVersion(schemaVersion) cmp, err := util.CompareExtensionVersions(binaryExtensionVersion, schemaExtensionVersion) if err != nil { return admission.Warnings{ - fmt.Sprintf("cannot compare restore binary version %s with backup schema version %s: %v; compatibility not verified", - binaryVersion, backupSchemaVersion, err), + fmt.Sprintf("cannot compare the target DocumentDB version %s with the %s's schema version %s: %v; compatibility not verified", + binaryVersion, src.kind, schemaVersion, err), }, nil } if cmp < 0 { return nil, field.ErrorList{field.Forbidden( - field.NewPath("spec", "bootstrap", "recovery", "backup"), + src.fieldPath, fmt.Sprintf( - "restore blocked: target binary version %s is older than the backup's schema version %s. "+ - "Restoring onto an older binary runs it against a newer, irreversible schema and may cause data corruption. "+ - "Set spec.documentDBVersion (or spec.image.documentDB) to a version >= %s.", - binaryVersion, backupSchemaVersion, backupSchemaVersion), + "restore blocked: the target DocumentDB version %s is older than the %s's schema version %s. "+ + "Restoring onto an older DocumentDB version runs it against a newer, irreversible schema and may cause data corruption. "+ + "Set spec.documentDBVersion (or spec.image.documentDB) to %s or newer.", + binaryVersion, src.kind, schemaVersion, schemaVersion), )} } return nil, nil @@ -431,7 +490,30 @@ func resolveBinaryVersion(db *dbpreview.DocumentDB) string { return db.Spec.DocumentDBVersion } -// specImageDocumentDB safely returns spec.image.documentDB or "" when unset. +// resolveEffectiveBinaryVersion returns the binary version the operator will +// actually run for db, including the operator-wide default the controller applies +// when the spec pins no version. Restore validation uses this rather than the +// spec-only resolveBinaryVersion so a restore whose effective binary would be +// older than the source schema is blocked, not merely warned. +func resolveEffectiveBinaryVersion(db *dbpreview.DocumentDB) string { + if v := resolveBinaryVersion(db); v != "" { + return v + } + // The spec pins no parseable version. Resolve the exact image the controller + // would actually run (a pinned but unparseable image, the DOCUMENTDB_VERSION + // env default, the ChangeStreams image, or the built-in default) via the shared + // helper, and read its semver tag. Anything without a parseable semver tag + // (a digest, or the changestream image) stays unknown so the restore is warned, + // not falsely blocked — keeping the webhook in step with the controller. + image := util.GetDocumentDBImageForInstance(db) + if tagIdx := strings.LastIndex(image, ":"); tagIdx >= 0 { + if semver := extractSemver(image[tagIdx+1:]); semver != "" { + return semver + } + } + return "" +} + func specImageDocumentDB(db *dbpreview.DocumentDB) string { if db == nil || db.Spec.Image == nil { return "" diff --git a/operator/src/internal/webhook/documentdb_webhook_test.go b/operator/src/internal/webhook/documentdb_webhook_test.go index 6fa4431cf..a4a207971 100644 --- a/operator/src/internal/webhook/documentdb_webhook_test.go +++ b/operator/src/internal/webhook/documentdb_webhook_test.go @@ -9,6 +9,7 @@ import ( cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/rest" @@ -18,6 +19,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" dbpreview "github.com/documentdb/documentdb-operator/api/preview" + util "github.com/documentdb/documentdb-operator/internal/utils" ) type fakeWebhookManager struct { @@ -352,6 +354,39 @@ var _ = Describe("resolveBinaryVersion helper", func() { }) }) +var _ = Describe("resolveEffectiveBinaryVersion helper", func() { + It("returns the spec version when one is set", func() { + db := newTestDocumentDB("0.112.0", "", "") + Expect(resolveEffectiveBinaryVersion(db)).To(Equal("0.112.0")) + }) + + It("falls back to the default image version when neither image nor version is set", func() { + db := newTestDocumentDB("", "", "") + Expect(resolveEffectiveBinaryVersion(db)).To(Equal("0.110.0")) + }) + + It("prefers the DOCUMENTDB_VERSION env override over the default", func() { + GinkgoT().Setenv(util.DOCUMENTDB_VERSION_ENV, "0.115.0") + db := newTestDocumentDB("", "", "") + Expect(resolveEffectiveBinaryVersion(db)).To(Equal("0.115.0")) + }) + + It("stays unknown for a digest-only image with no version (does not use the default)", func() { + db := newTestDocumentDB("", "", "ghcr.io/documentdb/documentdb@sha256:abc123") + Expect(resolveEffectiveBinaryVersion(db)).To(BeEmpty()) + }) + + It("stays unknown when the ChangeStreams gate selects a non-semver image", func() { + // With no version/env and the ChangeStreams gate on, the controller runs the + // changestream image (non-semver tag). The webhook must treat it as unknown + // (warn), not silently compare against the default, to stay in step with the + // controller. + db := newTestDocumentDB("", "", "") + db.Spec.FeatureGates = map[string]bool{dbpreview.FeatureGateChangeStreams: true} + Expect(resolveEffectiveBinaryVersion(db)).To(BeEmpty()) + }) +}) + var _ = Describe("extractSemver helper", func() { It("extracts clean semver", func() { Expect(extractSemver("0.112.0")).To(Equal("0.112.0")) @@ -590,10 +625,30 @@ func newBackupWithSchema(name, schemaVersion string) *dbpreview.Backup { func newValidatorWithObjects(objs ...ctrlclient.Object) *DocumentDBValidator { scheme := runtime.NewScheme() Expect(dbpreview.AddToScheme(scheme)).To(Succeed()) + Expect(corev1.AddToScheme(scheme)).To(Succeed()) fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() return &DocumentDBValidator{Client: fakeClient} } +func newPVRestoreDocumentDB(name, version, pvName string) *dbpreview.DocumentDB { + db := newTestDocumentDB(version, "", "") + db.Name = name + db.Spec.Bootstrap = &dbpreview.BootstrapConfiguration{ + Recovery: &dbpreview.RecoveryConfiguration{ + PersistentVolume: &dbpreview.PVRecoveryConfiguration{Name: pvName}, + }, + } + return db +} + +func newPVWithSchema(name, schemaVersion string) *corev1.PersistentVolume { + pv := &corev1.PersistentVolume{ObjectMeta: metav1.ObjectMeta{Name: name}} + if schemaVersion != "" { + pv.Annotations = map[string]string{util.AnnotationSchemaVersion: schemaVersion} + } + return pv +} + var _ = Describe("restore schema compatibility validation", func() { ctx := context.Background() @@ -652,14 +707,40 @@ var _ = Describe("restore schema compatibility validation", func() { Expect(warnings[0]).To(ContainSubstring("not found")) }) - It("warns when the restore binary version cannot be determined", func() { + It("blocks restore when no version is set and the default binary is older than the backup schema", func() { + // With no spec.documentDBVersion/image, the controller applies the operator + // default (DEFAULT_DOCUMENTDB_IMAGE, 0.110.0). Restoring a 0.112.0 schema onto + // it would run an older binary against a newer schema, so admission blocks it. + backup := newBackupWithSchema("bk", "0.112.0") + v := newValidatorWithObjects(backup) + db := newRestoreDocumentDB("restored", "", "bk") + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(warnings).To(BeEmpty()) + Expect(errs).To(HaveLen(1)) + Expect(errs[0].Detail).To(ContainSubstring("older than the backup's schema version")) + }) + + It("allows restore when no version is set and the default binary is >= the backup schema", func() { + backup := newBackupWithSchema("bk", "0.110.0") + v := newValidatorWithObjects(backup) + db := newRestoreDocumentDB("restored", "", "bk") + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(warnings).To(BeEmpty()) + Expect(errs).To(BeEmpty()) + }) + + It("warns when the restore binary version cannot be determined (digest-only image)", func() { + // A digest-only image pins an image whose version is unknown at admission, + // and it takes priority over the operator default, so compatibility can only + // be warned about, not verified. backup := newBackupWithSchema("bk", "0.112.0") v := newValidatorWithObjects(backup) db := newRestoreDocumentDB("restored", "", "bk") + db.Spec.Image = &dbpreview.ImageSpec{DocumentDB: "ghcr.io/documentdb/documentdb@sha256:abc123"} warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) Expect(errs).To(BeEmpty()) Expect(warnings).To(HaveLen(1)) - Expect(warnings[0]).To(ContainSubstring("cannot determine the target binary version")) + Expect(warnings[0]).To(ContainSubstring("cannot determine the target DocumentDB version")) }) It("warns when restoring from a PersistentVolume with an explicit version", func() { @@ -676,7 +757,7 @@ var _ = Describe("restore schema compatibility validation", func() { Expect(warnings[0]).To(ContainSubstring("PersistentVolume")) }) - It("rejects a PersistentVolume restore that omits an explicit binary version", func() { + It("warns (does not block) on a PersistentVolume restore that omits an explicit binary version", func() { v := newValidatorWithObjects() db := newTestDocumentDB("", "", "") db.Spec.Bootstrap = &dbpreview.BootstrapConfiguration{ @@ -685,9 +766,9 @@ var _ = Describe("restore schema compatibility validation", func() { }, } warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) - Expect(warnings).To(BeEmpty()) - Expect(errs).To(HaveLen(1)) - Expect(errs[0].Detail).To(ContainSubstring("requires an explicit spec.documentDBVersion")) + Expect(errs).To(BeEmpty()) + Expect(warnings).To(HaveLen(1)) + Expect(warnings[0]).To(ContainSubstring("PersistentVolume")) }) It("allows a PersistentVolume restore when only image.documentDB is set", func() { @@ -703,4 +784,42 @@ var _ = Describe("restore schema compatibility validation", func() { Expect(warnings).To(HaveLen(1)) Expect(warnings[0]).To(ContainSubstring("PersistentVolume")) }) + + It("allows a PersistentVolume restore when the PV annotation schema equals the binary version", func() { + pv := newPVWithSchema("pv-1", "0.112.0") + v := newValidatorWithObjects(pv) + db := newPVRestoreDocumentDB("restored", "0.112.0", "pv-1") + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(warnings).To(BeEmpty()) + Expect(errs).To(BeEmpty()) + }) + + It("allows a PersistentVolume restore when the binary version is newer than the PV annotation schema", func() { + pv := newPVWithSchema("pv-1", "0.110.0") + v := newValidatorWithObjects(pv) + db := newPVRestoreDocumentDB("restored", "0.112.0", "pv-1") + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(warnings).To(BeEmpty()) + Expect(errs).To(BeEmpty()) + }) + + It("rejects a PersistentVolume restore when the binary version is older than the PV annotation schema", func() { + pv := newPVWithSchema("pv-1", "0.112.0") + v := newValidatorWithObjects(pv) + db := newPVRestoreDocumentDB("restored", "0.110.0", "pv-1") + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(warnings).To(BeEmpty()) + Expect(errs).To(HaveLen(1)) + Expect(errs[0].Detail).To(ContainSubstring("older than the PersistentVolume's schema version")) + }) + + It("warns when the PV has no schema annotation (same as a backup with no recorded schema)", func() { + pv := newPVWithSchema("pv-1", "") + v := newValidatorWithObjects(pv) + db := newPVRestoreDocumentDB("restored", "", "pv-1") + warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) + Expect(errs).To(BeEmpty()) + Expect(warnings).To(HaveLen(1)) + Expect(warnings[0]).To(ContainSubstring("no recorded schema version")) + }) }) From 1fb1030df363d6ac024d4eebe1f34eb127d20564 Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Wed, 5 Aug 2026 10:47:32 -0400 Subject: [PATCH 5/8] test(e2e): cover restore schema-version validation (#434) 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 #368. Refs #434 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b653a2f9-cc1a-400a-8e93-11232229023a Signed-off-by: Wenting Wu --- test/e2e/tests/backup/helpers_test.go | 19 +- .../tests/backup/schema_validation_test.go | 194 ++++++++++++++++++ 2 files changed, 209 insertions(+), 4 deletions(-) create mode 100644 test/e2e/tests/backup/schema_validation_test.go diff --git a/test/e2e/tests/backup/helpers_test.go b/test/e2e/tests/backup/helpers_test.go index 936d05c07..a897d2f83 100644 --- a/test/e2e/tests/backup/helpers_test.go +++ b/test/e2e/tests/backup/helpers_test.go @@ -91,10 +91,11 @@ func manifestsRoot() string { return filepath.Join(filepath.Dir(thisFile), "..", "..", "manifests") } -// createRecoveryDocumentDB renders a flat recovery_* template under -// manifests/backup/ and applies the resulting DocumentDB CR. -func createRecoveryDocumentDB( - ctx context.Context, c client.Client, +// buildRecoveryDocumentDB renders a flat recovery_* template under +// manifests/backup/ and returns the resulting DocumentDB CR *without* +// creating it, so callers (e.g. admission-rejection specs) can mutate +// the spec before calling Create themselves. +func buildRecoveryDocumentDB( ns, name, templateName string, extra map[string]string, ) *previewv1.DocumentDB { vars := baseVars(name, ns, "") @@ -112,6 +113,16 @@ func createRecoveryDocumentDB( if dd.Name == "" { dd.Name = name } + return dd +} + +// createRecoveryDocumentDB renders a flat recovery_* template under +// manifests/backup/ and applies the resulting DocumentDB CR. +func createRecoveryDocumentDB( + ctx context.Context, c client.Client, + ns, name, templateName string, extra map[string]string, +) *previewv1.DocumentDB { + dd := buildRecoveryDocumentDB(ns, name, templateName, extra) Expect(c.Create(ctx, dd)).To(Succeed(), "create recovery DocumentDB %s/%s", ns, name) return dd } diff --git a/test/e2e/tests/backup/schema_validation_test.go b/test/e2e/tests/backup/schema_validation_test.go new file mode 100644 index 000000000..c428b0f94 --- /dev/null +++ b/test/e2e/tests/backup/schema_validation_test.go @@ -0,0 +1,194 @@ +package backup + +import ( + "context" + "time" + + . "github.com/onsi/ginkgo/v2" //nolint:revive + . "github.com/onsi/gomega" //nolint:revive + + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + previewv1 "github.com/documentdb/documentdb-operator/api/preview" + "github.com/documentdb/documentdb-operator/test/e2e" + "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/assertions" + bkp "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/backup" + "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/documentdb" + "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/namespaces" + "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/timeouts" + shareddb "github.com/documentdb/documentdb-operator/test/shared/documentdb" +) + +// schemaVersionAnnotation mirrors util.AnnotationSchemaVersion in the +// operator. It is intentionally hard-coded here so this e2e spec pins +// the exact on-the-wire annotation key the restore-validation contract +// depends on — a rename on the operator side must be a conscious, +// two-place change. +const schemaVersionAnnotation = "documentdb.io/schema-version" + +// olderBinaryVersion is a semver guaranteed to be lower than any real +// DocumentDB extension schema version (which are 0.1x.y). Restores +// pinned to it are rejected at admission *before* any image pull, so it +// never needs to resolve to a pullable tag. +const olderBinaryVersion = "0.1.0" + +// sourceSchemaVersion polls a source DocumentDB's status.schemaVersion +// until it is non-empty, returning the observed value. The operator +// records this on Backups and stamps it onto retained PVs; both restore +// validations compare a restore's binary version against it. +func sourceSchemaVersion(ctx context.Context, c client.Client, key types.NamespacedName) string { + var observed string + Eventually(func() string { + dd, err := shareddb.Get(ctx, c, key) + if err != nil { + return "" + } + observed = dd.Status.SchemaVersion + return observed + }, timeouts.For(timeouts.DocumentDBReady), timeouts.PollInterval(timeouts.DocumentDBReady)). + ShouldNot(BeEmpty(), "source %s never reported status.schemaVersion", key) + return observed +} + +// pinBinaryVersion forces a restore CR's resolved binary version to v by +// clearing spec.image (whose tag would otherwise win in resolveBinaryVersion) +// and setting spec.documentDBVersion. This makes the admission decision +// deterministic regardless of the DOCUMENTDB_IMAGE the CI job injects. +func pinBinaryVersion(dd *previewv1.DocumentDB, v string) { + dd.Spec.Image = nil + dd.Spec.DocumentDBVersion = v +} + +var _ = Describe("DocumentDB restore — schema-version compatibility (#434)", + Label(e2e.BackupLabel, e2e.NeedsCSISnapshotsLabel, e2e.SlowLabel), e2e.MediumLevelLabel, + func() { + var ( + ctx context.Context + ns string + c client.Client + ) + + BeforeEach(func() { + e2e.SkipUnlessLevel(e2e.Medium) + ctx = context.Background() + c = e2e.SuiteEnv().Client + skipUnlessCSISnapshotsUsable(ctx, c) + ns = namespaces.NamespaceForSpec(e2e.BackupLabel) + createNamespace(ctx, c, ns) + createCredentialSecret(ctx, c, ns) + }) + + It("records the schema version on the Backup and blocks restore onto an older binary", func() { + const ( + sourceName = "schema-bk-src" + backupName = "schema-bk-src-backup" + ) + + // Source cluster → Ready, then capture its installed schema version. + src, err := documentdb.Create(ctx, c, ns, sourceName, documentdb.CreateOptions{ + Base: "documentdb", + Vars: baseVars(sourceName, ns, "2Gi"), + ManifestsRoot: manifestsRoot(), + }) + Expect(err).NotTo(HaveOccurred()) + DeferCleanup(func(ctx SpecContext) { _ = shareddb.Delete(ctx, c, src, 3*time.Minute) }) + + srcKey := types.NamespacedName{Namespace: ns, Name: sourceName} + Eventually(assertions.AssertDocumentDBReady(ctx, c, srcKey), + timeouts.For(timeouts.DocumentDBReady), + timeouts.PollInterval(timeouts.DocumentDBReady), + ).Should(Succeed()) + schema := sourceSchemaVersion(ctx, c, srcKey) + + // On-demand Backup → Completed, and the operator must have + // recorded the source's schema version onto its status. + _, err = bkp.Create(ctx, c, bkp.BackupVars{ + Name: backupName, Namespace: ns, ClusterName: sourceName, RetentionDays: 1, + }) + Expect(err).NotTo(HaveOccurred()) + DeferCleanup(func(ctx SpecContext) { _ = bkp.Delete(ctx, c, ns, backupName, 1*time.Minute) }) + completed, err := bkp.WaitForCompleted(ctx, c, ns, backupName, timeouts.For(timeouts.BackupComplete)) + Expect(err).NotTo(HaveOccurred(), "backup %s/%s did not complete", ns, backupName) + Expect(completed).NotTo(BeNil()) + Eventually(func() string { + b, err := bkp.Get(ctx, c, ns, backupName) + if err != nil { + return "" + } + return b.Status.SchemaVersion + }, timeouts.For(timeouts.DocumentDBReady), timeouts.PollInterval(timeouts.DocumentDBReady)). + Should(Equal(schema), "Backup.Status.SchemaVersion must record the source cluster's schema version") + + By("rejecting a restore whose binary version is older than the backup's schema version") + bad := buildRecoveryDocumentDB(ns, "schema-bk-dst-bad", + "recovery_from_backup.yaml.template", + map[string]string{"BACKUP_NAME": backupName}) + pinBinaryVersion(bad, olderBinaryVersion) + err = c.Create(ctx, bad) + Expect(err).To(HaveOccurred(), "restore onto an older binary must be rejected at admission") + Expect(err.Error()).To(ContainSubstring("older than the backup's schema version")) + + By("admitting a restore whose binary version matches the backup's schema version") + good := buildRecoveryDocumentDB(ns, "schema-bk-dst-good", + "recovery_from_backup.yaml.template", + map[string]string{"BACKUP_NAME": backupName}) + pinBinaryVersion(good, schema) + Expect(c.Create(ctx, good)).To(Succeed(), + "restore onto a binary >= the backup's schema version must be admitted") + DeferCleanup(func(ctx SpecContext) { _ = shareddb.Delete(ctx, c, good, 3*time.Minute) }) + }) + + It("stamps the schema version on the PV and blocks PV restore onto an older binary", func() { + const sourceName = "schema-pv-src" + + // Source cluster → Ready; the PV controller stamps the schema + // version onto the backing PV as an annotation. + src, err := documentdb.Create(ctx, c, ns, sourceName, documentdb.CreateOptions{ + Base: "documentdb", + Vars: baseVars(sourceName, ns, "2Gi"), + ManifestsRoot: manifestsRoot(), + }) + Expect(err).NotTo(HaveOccurred()) + srcKey := types.NamespacedName{Namespace: ns, Name: sourceName} + Eventually(assertions.AssertDocumentDBReady(ctx, c, srcKey), + timeouts.For(timeouts.DocumentDBReady), + timeouts.PollInterval(timeouts.DocumentDBReady), + ).Should(Succeed()) + schema := sourceSchemaVersion(ctx, c, srcKey) + + By("waiting for the PV controller to stamp the schema-version annotation") + var pvName string + Eventually(func(g Gomega) { + pv, err := bkp.FindRetainedPV(ctx, c, ns, sourceName) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(pv).NotTo(BeNil()) + g.Expect(pv.Annotations).To(HaveKeyWithValue(schemaVersionAnnotation, schema), + "PV must be annotated with the source's schema version") + pvName = pv.Name + }, timeouts.For(timeouts.DocumentDBReady), timeouts.PollInterval(timeouts.DocumentDBReady)). + Should(Succeed()) + + // Delete the source (reclaimPolicy Retain by default) so the + // annotated PV survives for a restore. + Expect(shareddb.Delete(ctx, c, src, 3*time.Minute)).To(Succeed()) + + By("rejecting a PV restore whose binary version is older than the PV's schema version") + bad := buildRecoveryDocumentDB(ns, "schema-pv-dst-bad", + "recovery_from_pv.yaml.template", + map[string]string{"PV_NAME": pvName}) + pinBinaryVersion(bad, olderBinaryVersion) + err = c.Create(ctx, bad) + Expect(err).To(HaveOccurred(), "PV restore onto an older binary must be rejected at admission") + Expect(err.Error()).To(ContainSubstring("older than the PersistentVolume's schema version")) + + By("admitting a PV restore whose binary version matches the PV's schema version") + good := buildRecoveryDocumentDB(ns, "schema-pv-dst-good", + "recovery_from_pv.yaml.template", + map[string]string{"PV_NAME": pvName}) + pinBinaryVersion(good, schema) + Expect(c.Create(ctx, good)).To(Succeed(), + "PV restore onto a binary >= the PV's schema version must be admitted") + DeferCleanup(func(ctx SpecContext) { _ = shareddb.Delete(ctx, c, good, 3*time.Minute) }) + }) + }) From ceb841acdd82f98bd0a25ca874b5a524e0912e94 Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Thu, 6 Aug 2026 12:43:26 -0400 Subject: [PATCH 6/8] test: decouple restore validation from default version Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Wenting Wu --- operator/src/internal/webhook/documentdb_webhook_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/operator/src/internal/webhook/documentdb_webhook_test.go b/operator/src/internal/webhook/documentdb_webhook_test.go index a4a207971..0c17c71e9 100644 --- a/operator/src/internal/webhook/documentdb_webhook_test.go +++ b/operator/src/internal/webhook/documentdb_webhook_test.go @@ -362,7 +362,8 @@ var _ = Describe("resolveEffectiveBinaryVersion helper", func() { It("falls back to the default image version when neither image nor version is set", func() { db := newTestDocumentDB("", "", "") - Expect(resolveEffectiveBinaryVersion(db)).To(Equal("0.110.0")) + defaultVersion := resolveBinaryVersion(newTestDocumentDB("", "", util.DEFAULT_DOCUMENTDB_IMAGE)) + Expect(resolveEffectiveBinaryVersion(db)).To(Equal(defaultVersion)) }) It("prefers the DOCUMENTDB_VERSION env override over the default", func() { @@ -709,9 +710,9 @@ var _ = Describe("restore schema compatibility validation", func() { It("blocks restore when no version is set and the default binary is older than the backup schema", func() { // With no spec.documentDBVersion/image, the controller applies the operator - // default (DEFAULT_DOCUMENTDB_IMAGE, 0.110.0). Restoring a 0.112.0 schema onto - // it would run an older binary against a newer schema, so admission blocks it. - backup := newBackupWithSchema("bk", "0.112.0") + // default. Restoring a newer schema onto it would run an older binary against + // a newer schema, so admission blocks it. + backup := newBackupWithSchema("bk", "999.0.0") v := newValidatorWithObjects(backup) db := newRestoreDocumentDB("restored", "", "bk") warnings, errs := v.validateRestoreSchemaCompatibility(ctx, db) From 8f00c93db925bf06de79c05dbf7fc05557ff26bc Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Thu, 6 Aug 2026 12:46:58 -0400 Subject: [PATCH 7/8] docs: drop legacy-version framing for unknown restore schema 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 --- .../preview/operations/backup-and-restore.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/operator-public-documentation/preview/operations/backup-and-restore.md b/docs/operator-public-documentation/preview/operations/backup-and-restore.md index 6981685fa..7bf86874a 100644 --- a/docs/operator-public-documentation/preview/operations/backup-and-restore.md +++ b/docs/operator-public-documentation/preview/operations/backup-and-restore.md @@ -188,8 +188,9 @@ corruption**. | schema version unknown | Allowed with a warning — verify the version yourself | The schema version is known from the `Backup`'s `status.schemaVersion` or a -retained PV's `documentdb.io/schema-version` annotation. Older backups or PVs -imported from outside the operator may not have it recorded. +retained PV's `documentdb.io/schema-version` annotation. A PV imported from +outside the operator (e.g. a raw disk or external snapshot) carries no annotation, +so its schema version is unknown. ### Step 2: Create a New DocumentDB Cluster From 0eac89b1d6e64cbc4e235cb08bc208dc271d6ec4 Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Thu, 6 Aug 2026 13:10:20 -0400 Subject: [PATCH 8/8] test(e2e): fold #434 schema-validation into existing restore 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 #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 --- test/e2e/tests/backup/helpers_test.go | 42 ++++ .../tests/backup/restore_from_backup_test.go | 23 +++ test/e2e/tests/backup/restore_from_pv_test.go | 17 ++ .../tests/backup/schema_validation_test.go | 194 ------------------ 4 files changed, 82 insertions(+), 194 deletions(-) delete mode 100644 test/e2e/tests/backup/schema_validation_test.go diff --git a/test/e2e/tests/backup/helpers_test.go b/test/e2e/tests/backup/helpers_test.go index a897d2f83..ff1684077 100644 --- a/test/e2e/tests/backup/helpers_test.go +++ b/test/e2e/tests/backup/helpers_test.go @@ -11,6 +11,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/yaml" @@ -18,8 +19,49 @@ import ( bkp "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/backup" "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/clusterprobe" "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/fixtures" + "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/timeouts" + shareddb "github.com/documentdb/documentdb-operator/test/shared/documentdb" ) +// schemaVersionAnnotation mirrors util.AnnotationSchemaVersion in the +// operator. It is intentionally hard-coded here so the restore-validation +// specs pin the exact on-the-wire annotation key the contract depends on — +// a rename on the operator side must be a conscious, two-place change. +const schemaVersionAnnotation = "documentdb.io/schema-version" + +// olderBinaryVersion is a semver guaranteed to be lower than any real +// DocumentDB extension schema version (which are 0.1x.y). Restores pinned +// to it are rejected at admission *before* any image pull, so it never +// needs to resolve to a pullable tag. +const olderBinaryVersion = "0.1.0" + +// sourceSchemaVersion polls a source DocumentDB's status.schemaVersion +// until it is non-empty, returning the observed value. The operator +// records this on Backups and stamps it onto retained PVs; both restore +// validations compare a restore's binary version against it. +func sourceSchemaVersion(ctx context.Context, c client.Client, key types.NamespacedName) string { + var observed string + Eventually(func() string { + dd, err := shareddb.Get(ctx, c, key) + if err != nil { + return "" + } + observed = dd.Status.SchemaVersion + return observed + }, timeouts.For(timeouts.DocumentDBReady), timeouts.PollInterval(timeouts.DocumentDBReady)). + ShouldNot(BeEmpty(), "source %s never reported status.schemaVersion", key) + return observed +} + +// pinBinaryVersion forces a restore CR's resolved binary version to v by +// clearing spec.image (whose tag would otherwise win in resolveBinaryVersion) +// and setting spec.documentDBVersion. This makes the admission decision +// deterministic regardless of the DOCUMENTDB_IMAGE the CI job injects. +func pinBinaryVersion(dd *previewv1.DocumentDB, v string) { + dd.Spec.Image = nil + dd.Spec.DocumentDBVersion = v +} + // credentialSecretName is the secret the backup area seeds in every // source and recovery namespace. Aliased to the fixtures default so // future credential-name changes stay a single-edit concern. diff --git a/test/e2e/tests/backup/restore_from_backup_test.go b/test/e2e/tests/backup/restore_from_backup_test.go index f813039a0..fec1d8bfa 100644 --- a/test/e2e/tests/backup/restore_from_backup_test.go +++ b/test/e2e/tests/backup/restore_from_backup_test.go @@ -65,6 +65,7 @@ var _ = Describe("DocumentDB restore — recovery.backup (CSI snapshot)", timeouts.For(timeouts.DocumentDBReady), timeouts.PollInterval(timeouts.DocumentDBReady), ).Should(Succeed()) + schema := sourceSchemaVersion(ctx, c, srcKey) h, err := emongo.NewFromDocumentDB(ctx, e2e.SuiteEnv(), ns, sourceName) Expect(err).NotTo(HaveOccurred(), "connect to source DocumentDB") @@ -89,6 +90,28 @@ var _ = Describe("DocumentDB restore — recovery.backup (CSI snapshot)", Expect(err).NotTo(HaveOccurred(), "source backup %s/%s did not complete", ns, backupName) + // Schema-version compatibility (#434): the operator must record the + // source's schema version onto the Backup, and admission must reject a + // restore whose binary version is older than it. The happy-path restore + // below (default version >= schema) covers the admit case. + Eventually(func() string { + b, err := bkp.Get(ctx, c, ns, backupName) + if err != nil { + return "" + } + return b.Status.SchemaVersion + }, timeouts.For(timeouts.DocumentDBReady), timeouts.PollInterval(timeouts.DocumentDBReady)). + Should(Equal(schema), "Backup.Status.SchemaVersion must record the source cluster's schema version") + + By("rejecting a restore whose binary version is older than the backup's schema version") + bad := buildRecoveryDocumentDB(ns, "restore-dst-old-binary", + "recovery_from_backup.yaml.template", + map[string]string{"BACKUP_NAME": backupName}) + pinBinaryVersion(bad, olderBinaryVersion) + err = c.Create(ctx, bad) + Expect(err).To(HaveOccurred(), "restore onto an older binary must be rejected at admission") + Expect(err.Error()).To(ContainSubstring("older than the backup's schema version")) + // 3. Recovery DocumentDB sourced from that Backup name. dst := createRecoveryDocumentDB(ctx, c, ns, recoveryName, "recovery_from_backup.yaml.template", diff --git a/test/e2e/tests/backup/restore_from_pv_test.go b/test/e2e/tests/backup/restore_from_pv_test.go index 93603aedf..bd8ff537d 100644 --- a/test/e2e/tests/backup/restore_from_pv_test.go +++ b/test/e2e/tests/backup/restore_from_pv_test.go @@ -65,6 +65,7 @@ var _ = Describe("DocumentDB restore — recovery.persistentVolume (retained PV) timeouts.For(timeouts.DocumentDBReady), timeouts.PollInterval(timeouts.DocumentDBReady), ).Should(Succeed()) + schema := sourceSchemaVersion(ctx, c, srcKey) h, err := emongo.NewFromDocumentDB(ctx, e2e.SuiteEnv(), ns, sourceName) Expect(err).NotTo(HaveOccurred(), "connect to source DocumentDB") @@ -88,6 +89,22 @@ var _ = Describe("DocumentDB restore — recovery.persistentVolume (retained PV) "no retained PV found for deleted source cluster %s/%s", ns, sourceName) Expect(pv).NotTo(BeNil()) + // Schema-version compatibility (#434): the PV controller must stamp the + // source's schema version onto the retained PV, and admission must reject + // a PV restore whose binary version is older than it. The happy-path + // restore below (default version >= schema) covers the admit case. + Expect(pv.Annotations).To(HaveKeyWithValue(schemaVersionAnnotation, schema), + "retained PV must be annotated with the source's schema version") + + By("rejecting a PV restore whose binary version is older than the PV's schema version") + bad := buildRecoveryDocumentDB(ns, "pv-recovery-dst-old-binary", + "recovery_from_pv.yaml.template", + map[string]string{"PV_NAME": pv.Name}) + pinBinaryVersion(bad, olderBinaryVersion) + err = c.Create(ctx, bad) + Expect(err).To(HaveOccurred(), "PV restore onto an older binary must be rejected at admission") + Expect(err.Error()).To(ContainSubstring("older than the PersistentVolume's schema version")) + // 4. Create the recovery DocumentDB that points at that // PV's name. The operator should rehydrate the data, // including creating and then cleaning up a temp PVC diff --git a/test/e2e/tests/backup/schema_validation_test.go b/test/e2e/tests/backup/schema_validation_test.go deleted file mode 100644 index c428b0f94..000000000 --- a/test/e2e/tests/backup/schema_validation_test.go +++ /dev/null @@ -1,194 +0,0 @@ -package backup - -import ( - "context" - "time" - - . "github.com/onsi/ginkgo/v2" //nolint:revive - . "github.com/onsi/gomega" //nolint:revive - - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" - - previewv1 "github.com/documentdb/documentdb-operator/api/preview" - "github.com/documentdb/documentdb-operator/test/e2e" - "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/assertions" - bkp "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/backup" - "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/documentdb" - "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/namespaces" - "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/timeouts" - shareddb "github.com/documentdb/documentdb-operator/test/shared/documentdb" -) - -// schemaVersionAnnotation mirrors util.AnnotationSchemaVersion in the -// operator. It is intentionally hard-coded here so this e2e spec pins -// the exact on-the-wire annotation key the restore-validation contract -// depends on — a rename on the operator side must be a conscious, -// two-place change. -const schemaVersionAnnotation = "documentdb.io/schema-version" - -// olderBinaryVersion is a semver guaranteed to be lower than any real -// DocumentDB extension schema version (which are 0.1x.y). Restores -// pinned to it are rejected at admission *before* any image pull, so it -// never needs to resolve to a pullable tag. -const olderBinaryVersion = "0.1.0" - -// sourceSchemaVersion polls a source DocumentDB's status.schemaVersion -// until it is non-empty, returning the observed value. The operator -// records this on Backups and stamps it onto retained PVs; both restore -// validations compare a restore's binary version against it. -func sourceSchemaVersion(ctx context.Context, c client.Client, key types.NamespacedName) string { - var observed string - Eventually(func() string { - dd, err := shareddb.Get(ctx, c, key) - if err != nil { - return "" - } - observed = dd.Status.SchemaVersion - return observed - }, timeouts.For(timeouts.DocumentDBReady), timeouts.PollInterval(timeouts.DocumentDBReady)). - ShouldNot(BeEmpty(), "source %s never reported status.schemaVersion", key) - return observed -} - -// pinBinaryVersion forces a restore CR's resolved binary version to v by -// clearing spec.image (whose tag would otherwise win in resolveBinaryVersion) -// and setting spec.documentDBVersion. This makes the admission decision -// deterministic regardless of the DOCUMENTDB_IMAGE the CI job injects. -func pinBinaryVersion(dd *previewv1.DocumentDB, v string) { - dd.Spec.Image = nil - dd.Spec.DocumentDBVersion = v -} - -var _ = Describe("DocumentDB restore — schema-version compatibility (#434)", - Label(e2e.BackupLabel, e2e.NeedsCSISnapshotsLabel, e2e.SlowLabel), e2e.MediumLevelLabel, - func() { - var ( - ctx context.Context - ns string - c client.Client - ) - - BeforeEach(func() { - e2e.SkipUnlessLevel(e2e.Medium) - ctx = context.Background() - c = e2e.SuiteEnv().Client - skipUnlessCSISnapshotsUsable(ctx, c) - ns = namespaces.NamespaceForSpec(e2e.BackupLabel) - createNamespace(ctx, c, ns) - createCredentialSecret(ctx, c, ns) - }) - - It("records the schema version on the Backup and blocks restore onto an older binary", func() { - const ( - sourceName = "schema-bk-src" - backupName = "schema-bk-src-backup" - ) - - // Source cluster → Ready, then capture its installed schema version. - src, err := documentdb.Create(ctx, c, ns, sourceName, documentdb.CreateOptions{ - Base: "documentdb", - Vars: baseVars(sourceName, ns, "2Gi"), - ManifestsRoot: manifestsRoot(), - }) - Expect(err).NotTo(HaveOccurred()) - DeferCleanup(func(ctx SpecContext) { _ = shareddb.Delete(ctx, c, src, 3*time.Minute) }) - - srcKey := types.NamespacedName{Namespace: ns, Name: sourceName} - Eventually(assertions.AssertDocumentDBReady(ctx, c, srcKey), - timeouts.For(timeouts.DocumentDBReady), - timeouts.PollInterval(timeouts.DocumentDBReady), - ).Should(Succeed()) - schema := sourceSchemaVersion(ctx, c, srcKey) - - // On-demand Backup → Completed, and the operator must have - // recorded the source's schema version onto its status. - _, err = bkp.Create(ctx, c, bkp.BackupVars{ - Name: backupName, Namespace: ns, ClusterName: sourceName, RetentionDays: 1, - }) - Expect(err).NotTo(HaveOccurred()) - DeferCleanup(func(ctx SpecContext) { _ = bkp.Delete(ctx, c, ns, backupName, 1*time.Minute) }) - completed, err := bkp.WaitForCompleted(ctx, c, ns, backupName, timeouts.For(timeouts.BackupComplete)) - Expect(err).NotTo(HaveOccurred(), "backup %s/%s did not complete", ns, backupName) - Expect(completed).NotTo(BeNil()) - Eventually(func() string { - b, err := bkp.Get(ctx, c, ns, backupName) - if err != nil { - return "" - } - return b.Status.SchemaVersion - }, timeouts.For(timeouts.DocumentDBReady), timeouts.PollInterval(timeouts.DocumentDBReady)). - Should(Equal(schema), "Backup.Status.SchemaVersion must record the source cluster's schema version") - - By("rejecting a restore whose binary version is older than the backup's schema version") - bad := buildRecoveryDocumentDB(ns, "schema-bk-dst-bad", - "recovery_from_backup.yaml.template", - map[string]string{"BACKUP_NAME": backupName}) - pinBinaryVersion(bad, olderBinaryVersion) - err = c.Create(ctx, bad) - Expect(err).To(HaveOccurred(), "restore onto an older binary must be rejected at admission") - Expect(err.Error()).To(ContainSubstring("older than the backup's schema version")) - - By("admitting a restore whose binary version matches the backup's schema version") - good := buildRecoveryDocumentDB(ns, "schema-bk-dst-good", - "recovery_from_backup.yaml.template", - map[string]string{"BACKUP_NAME": backupName}) - pinBinaryVersion(good, schema) - Expect(c.Create(ctx, good)).To(Succeed(), - "restore onto a binary >= the backup's schema version must be admitted") - DeferCleanup(func(ctx SpecContext) { _ = shareddb.Delete(ctx, c, good, 3*time.Minute) }) - }) - - It("stamps the schema version on the PV and blocks PV restore onto an older binary", func() { - const sourceName = "schema-pv-src" - - // Source cluster → Ready; the PV controller stamps the schema - // version onto the backing PV as an annotation. - src, err := documentdb.Create(ctx, c, ns, sourceName, documentdb.CreateOptions{ - Base: "documentdb", - Vars: baseVars(sourceName, ns, "2Gi"), - ManifestsRoot: manifestsRoot(), - }) - Expect(err).NotTo(HaveOccurred()) - srcKey := types.NamespacedName{Namespace: ns, Name: sourceName} - Eventually(assertions.AssertDocumentDBReady(ctx, c, srcKey), - timeouts.For(timeouts.DocumentDBReady), - timeouts.PollInterval(timeouts.DocumentDBReady), - ).Should(Succeed()) - schema := sourceSchemaVersion(ctx, c, srcKey) - - By("waiting for the PV controller to stamp the schema-version annotation") - var pvName string - Eventually(func(g Gomega) { - pv, err := bkp.FindRetainedPV(ctx, c, ns, sourceName) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(pv).NotTo(BeNil()) - g.Expect(pv.Annotations).To(HaveKeyWithValue(schemaVersionAnnotation, schema), - "PV must be annotated with the source's schema version") - pvName = pv.Name - }, timeouts.For(timeouts.DocumentDBReady), timeouts.PollInterval(timeouts.DocumentDBReady)). - Should(Succeed()) - - // Delete the source (reclaimPolicy Retain by default) so the - // annotated PV survives for a restore. - Expect(shareddb.Delete(ctx, c, src, 3*time.Minute)).To(Succeed()) - - By("rejecting a PV restore whose binary version is older than the PV's schema version") - bad := buildRecoveryDocumentDB(ns, "schema-pv-dst-bad", - "recovery_from_pv.yaml.template", - map[string]string{"PV_NAME": pvName}) - pinBinaryVersion(bad, olderBinaryVersion) - err = c.Create(ctx, bad) - Expect(err).To(HaveOccurred(), "PV restore onto an older binary must be rejected at admission") - Expect(err.Error()).To(ContainSubstring("older than the PersistentVolume's schema version")) - - By("admitting a PV restore whose binary version matches the PV's schema version") - good := buildRecoveryDocumentDB(ns, "schema-pv-dst-good", - "recovery_from_pv.yaml.template", - map[string]string{"PV_NAME": pvName}) - pinBinaryVersion(good, schema) - Expect(c.Create(ctx, good)).To(Succeed(), - "PV restore onto a binary >= the PV's schema version must be admitted") - DeferCleanup(func(ctx SpecContext) { _ = shareddb.Delete(ctx, c, good, 3*time.Minute) }) - }) - })