backupccl: disallow restore of backups older than the minimum supported binary version

pkg/ccl/backupccl/BUILD.bazel

@@ -126,6 +126,7 @@ go_library(
         "//pkg/util/bulk",
         "//pkg/util/contextutil",
         "//pkg/util/ctxgroup",
+        "//pkg/util/envutil",
         "//pkg/util/hlc",
         "//pkg/util/interval",
         "//pkg/util/json",

pkg/ccl/backupccl/backup_test.go

@@ -8286,17 +8286,18 @@ func TestIncorrectAccessOfFilesInBackupMetadata(t *testing.T) {
 	sqlDB.ExpectErr(t, "assertion: this placeholder legacy Files entry should never be opened", `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)
@@ -8307,45 +8308,57 @@ 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.
+		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 > 1 major version, 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
@@ -10735,3 +10748,104 @@ $$;
 	require.NoError(t, err)
 
 }
+
+func TestBackupRestoreForeignKeys(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	params := base.TestServerArgs{}
+	const numAccounts = 1000
+	_, sqlDB, _, cleanup := backupRestoreTestSetupWithParams(t, singleNode, numAccounts,
+		InitManualReplication, base.TestClusterArgs{ServerArgs: params})
+	defer cleanup()
+
+	sqlDB.Exec(t, `CREATE DATABASE test`)
+	sqlDB.Exec(t, `SET database = test`)
+	sqlDB.Exec(t, `
+CREATE TABLE circular (k INT8 PRIMARY KEY, selfid INT8 UNIQUE);
+ALTER TABLE circular ADD CONSTRAINT self_fk FOREIGN KEY (selfid) REFERENCES circular (selfid);
+CREATE TABLE parent (k INT8 PRIMARY KEY, j INT8 UNIQUE);
+CREATE TABLE child (k INT8 PRIMARY KEY, parent_i INT8 REFERENCES parent, parent_j INT8 REFERENCES parent (j));
+CREATE TABLE child_pk (k INT8 PRIMARY KEY REFERENCES parent);
+`)
+
+	sqlDB.Exec(t, `BACKUP INTO $1 WITH revision_history`, localFoo)
+
+	sqlDB.Exec(t, `CREATE TABLE rev_times (id INT PRIMARY KEY, logical_time DECIMAL);`)
+	sqlDB.Exec(t, `INSERT INTO rev_times VALUES (1, cluster_logical_timestamp());`)
+
+	sqlDB.Exec(t, `
+	CREATE USER newuser;
+	GRANT ALL ON circular TO newuser;
+	GRANT ALL ON parent TO newuser;
+`)
+
+	sqlDB.Exec(t, `BACKUP INTO LATEST IN $1 WITH revision_history`, localFoo)
+	sqlDB.Exec(t, `INSERT INTO rev_times VALUES (2, cluster_logical_timestamp())`)
+
+	sqlDB.Exec(t, `GRANT ALL ON child TO newuser;`)
+
+	sqlDB.Exec(t, `BACKUP INTO LATEST IN $1 WITH revision_history`, localFoo)
+	sqlDB.Exec(t, `INSERT INTO rev_times VALUES (3, cluster_logical_timestamp())`)
+
+	sqlDB.Exec(t, `GRANT ALL ON child_pk TO newuser`)
+
+	sqlDB.Exec(t, `BACKUP INTO LATEST IN $1 WITH revision_history`, localFoo)
+
+	// Test that `SHOW BACKUP` displays the foreign keys correctly.
+	type testCase struct {
+		table                     string
+		expectedForeignKeyPattern string
+	}
+	for _, tc := range []testCase{
+		{
+			"circular",
+			"CONSTRAINT self_fk FOREIGN KEY \\(selfid\\) REFERENCES public\\.circular\\(selfid\\) NOT VALID",
+		},
+		{
+			"child",
+			"CONSTRAINT \\w+ FOREIGN KEY \\(\\w+\\) REFERENCES public\\.parent\\(\\w+\\)",
+		},
+		{
+			"child_pk",
+			"CONSTRAINT \\w+ FOREIGN KEY \\(\\w+\\) REFERENCES public\\.parent\\(\\w+\\)",
+		},
+	} {
+		results := sqlDB.QueryStr(t, `
+				SELECT
+					create_statement
+				FROM
+					[SHOW BACKUP SCHEMAS FROM LATEST IN $1]
+				WHERE
+					object_type = 'table' AND object_name = $2
+				`, localFoo, tc.table)
+		require.NotEmpty(t, results)
+		require.Regexp(t, regexp.MustCompile(tc.expectedForeignKeyPattern), results[0][0])
+	}
+	sqlDB.Exec(t, `DROP DATABASE test`)
+
+	// Test restoring different objects from the backup.
+	sqlDB.Exec(t, `CREATE DATABASE ts`)
+	sqlDB.Exec(t, `RESTORE test.rev_times FROM LATEST IN $1 WITH into_db = 'ts'`, localFoo)
+	for _, ts := range sqlDB.QueryStr(t, `SELECT logical_time FROM ts.rev_times`) {
+		sqlDB.Exec(t, fmt.Sprintf(`RESTORE DATABASE test FROM LATEST IN $1 AS OF SYSTEM TIME %s`, ts[0]), localFoo)
+		// Just rendering the constraints loads and validates schema.
+		sqlDB.Exec(t, `SELECT * FROM pg_catalog.pg_constraint`)
+		sqlDB.Exec(t, `DROP DATABASE test`)
+
+		// Restore a couple tables, including parent but not child_pk.
+		sqlDB.Exec(t, `CREATE DATABASE test`)
+		sqlDB.Exec(t, fmt.Sprintf(`RESTORE test.circular FROM LATEST IN $1 AS OF SYSTEM TIME %s`, ts[0]), localFoo)
+		sqlDB.Exec(t, fmt.Sprintf(`RESTORE test.parent, test.child FROM LATEST IN $1 AS OF SYSTEM TIME %s  WITH skip_missing_foreign_keys`, ts[0]), localFoo)
+		sqlDB.Exec(t, `SELECT * FROM pg_catalog.pg_constraint`)
+		sqlDB.Exec(t, `DROP DATABASE test`)
+
+		// Now do each table on its own with skip_missing_foreign_keys.
+		sqlDB.Exec(t, `CREATE DATABASE test`)
+		for _, name := range []string{"child_pk", "child", "circular", "parent"} {
+			sqlDB.Exec(t, fmt.Sprintf(`RESTORE test.%s FROM LATEST IN $1 AS OF SYSTEM TIME %s WITH skip_missing_foreign_keys`, name, ts[0]), localFoo)
+		}
+		sqlDB.Exec(t, `SELECT * FROM pg_catalog.pg_constraint`)
+		sqlDB.Exec(t, `DROP DATABASE test`)
+	}
+}

pkg/ccl/backupccl/restore_mid_schema_change_test.go

@@ -31,11 +31,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-var (
-	clusterRestoreVersion = version.MustParse("v20.1.0")
-	noNonMVCCAddSSTable   = version.MustParse("v22.1.0")
-)
-
 // TestRestoreMidSchemaChanges attempts to RESTORE several BACKUPs that are
 // already constructed and store in
 // ccl/backupccl/testdata/restore_mid_schema_change. These backups were taken on
@@ -68,12 +63,14 @@ func TestRestoreMidSchemaChange(t *testing.T) {
 		testdataBase = datapathutils.TestDataPath(t, "restore_mid_schema_change")
 		exportDirs   = testdataBase + "/exports"
 	)
-	for _, isSchemaOnly := range []bool{true, false} {
+	testutils.RunTrueAndFalse(t, "schema-only", func(t *testing.T, isSchemaOnly bool) {
 		name := "regular-"
 		if isSchemaOnly {
 			name = "schema-only-"
 		}
-		for _, isClusterRestore := range []bool{true, false} {
+		testutils.RunTrueAndFalse(t, "cluster-restore", func(t *testing.T, isClusterRestore bool) {
+			// TODO(adityamaru): Regenerate fixtures with cluster backups.
+			isClusterRestore = false
 			name = name + "table"
 			if isClusterRestore {
 				name = name + "cluster"
@@ -89,10 +86,6 @@ func TestRestoreMidSchemaChange(t *testing.T) {
 							clusterVersion, err := parseMajorVersion(clusterVersionDir.Name())
 							require.NoError(t, err)
 
-							if !clusterVersion.AtLeast(clusterRestoreVersion) && isClusterRestore {
-								continue
-							}
-
 							t.Run(clusterVersionDir.Name(), func(t *testing.T) {
 								require.True(t, clusterVersionDir.IsDir())
 								fullClusterVersionDir, err := filepath.Abs(
@@ -115,8 +108,8 @@ func TestRestoreMidSchemaChange(t *testing.T) {
 					})
 				}
 			})
-		}
-	}
+		})
+	})
 }
 
 // parseMajorVersion parses our major-versioned directory names as if they were
@@ -139,15 +132,10 @@ func expectedSCJobCount(scName string, ver *version.Version) int {
 	case "midmultitable":
 		expNumSCJobs = 2 // this test perform a schema change for each table
 	case "midprimarykeyswap":
-		if ver.AtLeast(noNonMVCCAddSSTable) {
-			// PK change and PK cleanup
-			expNumSCJobs = 2
-		} else {
-			// This will fail so we expect no cleanup job.
-			expNumSCJobs = 1
-		}
+		// PK change and PK cleanup
+		expNumSCJobs = 2
 	case "midprimarykeyswapcleanup":
-		expNumSCJobs = 1
+		expNumSCJobs = 2
 	default:
 		// Most test cases only have 1 schema change under test.
 		expNumSCJobs = 1
@@ -257,14 +245,14 @@ func restoreMidSchemaChange(
 		require.NoError(t, err)
 
 		sqlDB.Exec(t, "USE defaultdb")
-		restoreQuery := "RESTORE defaultdb.* FROM $1"
+		restoreQuery := "RESTORE defaultdb.* FROM LATEST IN $1"
 		if isClusterRestore {
-			restoreQuery = "RESTORE FROM $1"
+			restoreQuery = "RESTORE FROM LATEST IN $1"
 		}
 		if isSchemaOnly {
 			restoreQuery = restoreQuery + "with schema_only"
 		}
-		log.Infof(context.Background(), "%+v", sqlDB.QueryStr(t, "SHOW BACKUP $1", localFoo))
+		log.Infof(context.Background(), "%+v", sqlDB.QueryStr(t, "SHOW BACKUP LATEST IN $1", localFoo))
 		sqlDB.Exec(t, restoreQuery, localFoo)
 		// Wait for all jobs to terminate. Some may fail since we don't restore
 		// adding spans.

pkg/ccl/backupccl/restore_old_sequences_test.go

@@ -14,6 +14,8 @@ import (
 	"testing"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
+	"github.com/cockroachdb/cockroach/pkg/clusterversion"
+	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/testutils/datapathutils"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -30,7 +32,7 @@ import (
 //
 //	VERSION=...
 //	roachprod create local
-//	roachprod wipe localÅ
+//	roachprod wipe local
 //	roachprod stage local release ${VERSION}
 //	roachprod start local
 //	roachprod sql local:1 -- -e "$(cat pkg/ccl/backupccl/testdata/restore_old_sequences/create.sql)"
@@ -66,6 +68,8 @@ func TestRestoreOldSequences(t *testing.T) {
 func restoreOldSequencesTest(exportDir string, isSchemaOnly bool) func(t *testing.T) {
 	return func(t *testing.T) {
 		params := base.TestServerArgs{}
+		params.Settings = cluster.MakeTestingClusterSettingsWithVersions(clusterversion.TestingBinaryVersion,
+			clusterversion.TestingBinaryMinSupportedVersion, false /* initializeVersion */)
 		const numAccounts = 1000
 		_, sqlDB, dir, cleanup := backupRestoreTestSetupWithParams(t, singleNode, numAccounts,
 			InitManualReplication, base.TestClusterArgs{ServerArgs: params})
@@ -75,7 +79,7 @@ func restoreOldSequencesTest(exportDir string, isSchemaOnly bool) func(t *testin
 		sqlDB.Exec(t, `CREATE DATABASE test`)
 		var unused string
 		var importedRows int
-		restoreQuery := `RESTORE test.* FROM $1`
+		restoreQuery := `RESTORE test.* FROM LATEST IN $1`
 		if isSchemaOnly {
 			restoreQuery = restoreQuery + " with schema_only"
 		}