diff --git a/pkg/ccl/backupccl/backup_test.go b/pkg/ccl/backupccl/backup_test.go index 196f4b5d40f4..17a3c50d034b 100644 --- a/pkg/ccl/backupccl/backup_test.go +++ b/pkg/ccl/backupccl/backup_test.go @@ -8452,17 +8452,18 @@ func TestIncorrectAccessOfFilesInBackupMetadata(t *testing.T) { sqlDB.ExpectErr(t, "assertion: this placeholder legacy Descriptor entry should never be used", `RESTORE DATABASE r1 FROM LATEST IN 'nodelocal://0/test' WITH new_db_name = 'r2'`) } -func TestManifestTooNew(t *testing.T) { +// TestRestoringAcrossVersions test that users are only allowed to restore +// backups taken on a version >= the minimum supported binary version of the +// current active cluster version. +func TestRestoringAcrossVersions(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) - _, sqlDB, rawDir, cleanupFn := backupRestoreTestSetup(t, singleNode, 1, InitManualReplication) + tc, sqlDB, rawDir, cleanupFn := backupRestoreTestSetup(t, singleNode, 1, InitManualReplication) defer cleanupFn() + sqlDB.Exec(t, `CREATE DATABASE r1`) sqlDB.Exec(t, `BACKUP DATABASE r1 TO 'nodelocal://0/too_new'`) sqlDB.Exec(t, `DROP DATABASE r1`) - // Prove we can restore. - sqlDB.Exec(t, `RESTORE DATABASE r1 FROM 'nodelocal://0/too_new'`) - sqlDB.Exec(t, `DROP DATABASE r1`) // Load/deserialize the manifest so we can mess with it. manifestPath := filepath.Join(rawDir, "too_new", backupbase.BackupMetadataName) @@ -8473,45 +8474,61 @@ func TestManifestTooNew(t *testing.T) { var backupManifest backuppb.BackupManifest require.NoError(t, protoutil.Unmarshal(manifestData, &backupManifest)) - // Bump the version and write it back out to make it look newer. - backupManifest.ClusterVersion = roachpb.Version{Major: math.MaxInt32, Minor: 1} - manifestData, err = protoutil.Marshal(&backupManifest) - require.NoError(t, err) - require.NoError(t, os.WriteFile(manifestPath, manifestData, 0644 /* perm */)) - // Also write the checksum file to match the new manifest. - checksum, err := backupinfo.GetChecksum(manifestData) - require.NoError(t, err) - require.NoError(t, os.WriteFile(manifestPath+backupinfo.BackupManifestChecksumSuffix, checksum, 0644 /* perm */)) + setManifestClusterVersion := func(version roachpb.Version) { + backupManifest.ClusterVersion = version + manifestData, err = protoutil.Marshal(&backupManifest) + require.NoError(t, err) + require.NoError(t, os.WriteFile(manifestPath, manifestData, 0644 /* perm */)) + // Also write the checksum file to match the new manifest. + checksum, err := backupinfo.GetChecksum(manifestData) + require.NoError(t, err) + require.NoError(t, os.WriteFile(manifestPath+backupinfo.BackupManifestChecksumSuffix, checksum, 0644 /* perm */)) + } - // Verify we reject it. - sqlDB.ExpectErr(t, "backup from version 2147483647.1 is newer than current version", `RESTORE DATABASE r1 FROM 'nodelocal://0/too_new'`) + t.Run("restore-same-version", func(t *testing.T) { + // Prove we can restore a backup taken on our current version. This subtest + // when run on a development branch will also exercise the handling of the + // `clusterversion.DevOffset` that is added to the cluster version of a + // development branch. + sqlDB.Exec(t, `RESTORE DATABASE r1 FROM 'nodelocal://0/too_new'`) + sqlDB.Exec(t, `DROP DATABASE r1`) + }) - // Bump the version down and write it back out to make it look older. - backupManifest.ClusterVersion = roachpb.Version{Major: 20, Minor: 2, Internal: 2} - manifestData, err = protoutil.Marshal(&backupManifest) - require.NoError(t, err) - require.NoError(t, os.WriteFile(manifestPath, manifestData, 0644 /* perm */)) - // Also write the checksum file to match the new manifest. - checksum, err = backupinfo.GetChecksum(manifestData) - require.NoError(t, err) - require.NoError(t, os.WriteFile(manifestPath+backupinfo.BackupManifestChecksumSuffix, checksum, 0644 /* perm */)) + t.Run("restore-newer-version", func(t *testing.T) { + // Bump the version and write it back out to make it look newer. + setManifestClusterVersion(roachpb.Version{Major: math.MaxInt32, Minor: 1}) - // Prove we can restore again. - sqlDB.Exec(t, `RESTORE DATABASE r1 FROM 'nodelocal://0/too_new'`) - sqlDB.Exec(t, `DROP DATABASE r1`) + // Verify we reject it. + sqlDB.ExpectErr(t, "backup from version 2147483647.1 is newer than current version", `RESTORE DATABASE r1 FROM 'nodelocal://0/too_new'`) + }) - // Nil out the version to match an old backup that lacked it. - backupManifest.ClusterVersion = roachpb.Version{} - manifestData, err = protoutil.Marshal(&backupManifest) - require.NoError(t, err) - require.NoError(t, os.WriteFile(manifestPath, manifestData, 0644 /* perm */)) - // Also write the checksum file to match the new manifest. - checksum, err = backupinfo.GetChecksum(manifestData) - require.NoError(t, err) - require.NoError(t, os.WriteFile(manifestPath+backupinfo.BackupManifestChecksumSuffix, checksum, 0644 /* perm */)) - // Prove we can restore again. - sqlDB.Exec(t, `RESTORE DATABASE r1 FROM 'nodelocal://0/too_new'`) - sqlDB.Exec(t, `DROP DATABASE r1`) + t.Run("restore-older-major-version", func(t *testing.T) { + // Bump the version down to outside our MinBinarySupportedVersion, and write + // it back out. This makes it ineligible for restore because of our restore + // version policy. + setManifestClusterVersion(roachpb.Version{Major: 20, Minor: 2, Internal: 2}) + + // Verify we reject it. + sqlDB.ExpectErr(t, "backup from version 20.2-2 is older than the minimum restoreable version", `RESTORE DATABASE r1 FROM 'nodelocal://0/too_new'`) + }) + + t.Run("restore-min-binary-version", func(t *testing.T) { + // Bump the version down to the min supported binary version, and write it + // back out. This makes it eligible for restore because of our restore + // version policy. + minBinaryVersion := tc.Server(0).ClusterSettings().Version.BinaryMinSupportedVersion() + setManifestClusterVersion(minBinaryVersion) + sqlDB.Exec(t, `RESTORE DATABASE r1 FROM 'nodelocal://0/too_new'`) + sqlDB.Exec(t, `DROP DATABASE r1`) + }) + + t.Run("restore-nil-version-manifest", func(t *testing.T) { + // Nil out the version to match an old backup that lacked it. + setManifestClusterVersion(roachpb.Version{}) + + // Verify we reject it. + sqlDB.ExpectErr(t, "the backup is from a version older than our minimum restoreable version", `RESTORE DATABASE r1 FROM 'nodelocal://0/too_new'`) + }) } // TestManifestBitFlip tests that we can detect a corrupt manifest when a bit diff --git a/pkg/ccl/backupccl/restore_planning.go b/pkg/ccl/backupccl/restore_planning.go index 7f7176a34ac2..68d347fa4e15 100644 --- a/pkg/ccl/backupccl/restore_planning.go +++ b/pkg/ccl/backupccl/restore_planning.go @@ -15,6 +15,7 @@ import ( "strconv" "strings" + "github.com/cockroachdb/cockroach/pkg/build" "github.com/cockroachdb/cockroach/pkg/ccl/backupccl/backupbase" "github.com/cockroachdb/cockroach/pkg/ccl/backupccl/backupdest" "github.com/cockroachdb/cockroach/pkg/ccl/backupccl/backupencryption" @@ -1592,14 +1593,39 @@ func doRestorePlan( }() currentVersion := p.ExecCfg().Settings.Version.ActiveVersion(ctx) + + // We support restoring a backup that was taken on a cluster that is on a + // version >= the earliest binary version that this binary can interoperate with. + minimumRestorableVersion := p.ExecCfg().Settings.Version.BinaryMinSupportedVersion() + if !build.IsRelease() { + if minimumRestorableVersion.Major < clusterversion.DevOffset { + return errors.AssertionFailedf("minimum restoreable version %s is less than the dev offset", + minimumRestorableVersion) + } + minimumRestorableVersion.Major -= clusterversion.DevOffset + } for i := range mainBackupManifests { - if v := mainBackupManifests[i].ClusterVersion; v.Major != 0 { - // This is the "cluster" version that does not change between patches but - // rather just tracks migrations run. If the backup is more migrated than - // this cluster, then this cluster isn't ready to restore this backup. - if currentVersion.Less(v) { - return errors.Errorf("backup from version %s is newer than current version %s", v, currentVersion) + v := mainBackupManifests[i].ClusterVersion + // This is the "cluster" version that does not change between patches but + // rather just tracks migrations run. If the backup is more migrated than + // this cluster, then this cluster isn't ready to restore this backup. + if currentVersion.Less(v) { + return errors.Errorf("backup from version %s is newer than current version %s", v, currentVersion) + } + + // If the backup is from a version earlier than the minimum restoreable + // version, then we do not support restoring it. + if v.Less(minimumRestorableVersion) { + if v.Major == 0 { + // This accounts for manifests that were generated on a version before + // the `ClusterVersion` field exists. + return errors.WithHint(errors.Newf("the backup is from a version older than our "+ + "minimum restoreable version %s", minimumRestorableVersion), + "refer to our documentation about restoring across versions: https://www.cockroachlabs.com/docs/v22.2/restoring-backups-across-versions.html") } + return errors.WithHint(errors.Newf("backup from version %s is older than the "+ + "minimum restoreable version %s", v, minimumRestorableVersion), + "refer to our documentation about restoring across versions: https://www.cockroachlabs.com/docs/v22.2/restoring-backups-across-versions.html") } }