tenantcapabilitiesccl: rewrite some capability tests to datadriven tests

Now that we have a datadriven framework available to exercise tenant capabilities, we can move tests in `TestMultiTenantAdminFunction` to make use of this. This patch rewrites SPLIT/SCATTER related tests to make use of the datadriven test framework. As is, the construction is cumbersome to work with. The move is done with the next commit in mind, where we will change the default behavior of split/scatter capabilities. To that end, I've only moved the tests that were in my way. At some point we should get rid of this entire thing and rewrite it to use the datadriven test framework. That day is not today. Epic: none Release note: None
knz · Mar 14, 2023 · 6c148e3 · 6c148e3
1 parent c31c1ac
commit 6c148e3
Show file tree

Hide file tree

Showing 5 changed files with 168 additions and 41 deletions.
diff --git a/pkg/ccl/multitenantccl/tenantcapabilitiesccl/capabilities_test.go b/pkg/ccl/multitenantccl/tenantcapabilitiesccl/capabilities_test.go
@@ -100,6 +100,7 @@ func TestDataDriven(t *testing.T) {
 		// us from being able to do so.
 		settings := cluster.MakeTestingClusterSettings()
 		sql.SecondaryTenantSplitAtEnabled.Override(ctx, &settings.SV, true)
+		sql.SecondaryTenantScatterEnabled.Override(ctx, &settings.SV, true)
 		tenantArgs := base.TestTenantArgs{
 			TenantID: serverutils.TestTenantID(),
 			Settings: settings,
@@ -115,7 +116,7 @@ func TestDataDriven(t *testing.T) {
 		}()
 		require.NoError(t, err)
 
-		var lastUpdateTS hlc.Timestamp
+		lastUpdateTS := tc.Server(0).Clock().Now() // ensure watcher isn't starting out empty
 		datadriven.RunTest(t, path, func(t *testing.T, d *datadriven.TestData) string {
 
 			switch d.Cmd {

diff --git a/pkg/ccl/multitenantccl/tenantcapabilitiesccl/testdata/can_admin_scatter b/pkg/ccl/multitenantccl/tenantcapabilitiesccl/testdata/can_admin_scatter
@@ -0,0 +1,79 @@
+query-sql-system
+SELECT * FROM [SHOW TENANT [10] WITH CAPABILITIES] WHERE capability_id = 'can_admin_scatter'
+----
+10 tenant-10 ready none can_admin_scatter true
+
+exec-sql-tenant
+CREATE TABLE t(a INT)
+----
+ok
+
+exec-sql-tenant
+CREATE INDEX idx on t(a)
+----
+ok
+
+# By default, we should be able to scatter.
+exec-privileged-op-tenant
+ALTER TABLE t SCATTER
+----
+ok
+
+# ditto for the index.
+exec-privileged-op-tenant
+ALTER INDEX t@idx SCATTER
+----
+ok
+
+
+update-capabilities
+ALTER TENANT [10] GRANT CAPABILITY can_admin_scatter=false
+----
+ok
+
+exec-privileged-op-tenant
+ALTER TABLE t SCATTER
+----
+pq: ba: AdminScatter [/Tenant/10/Table/104/1,/Tenant/10/Table/104/2) RPC error: rpc error: code = Unauthenticated desc = client tenant does not have capability "can_admin_scatter" (*kvpb.AdminScatterRequest)
+
+# Check the index as well.
+exec-privileged-op-tenant
+ALTER INDEX t@idx SCATTER
+----
+pq: ba: AdminScatter [/Tenant/10/Table/104/2,/Tenant/10/Table/104/3) RPC error: rpc error: code = Unauthenticated desc = client tenant does not have capability "can_admin_scatter" (*kvpb.AdminScatterRequest)
+
+# Grant the capability without providing an explicit value.
+update-capabilities
+ALTER TENANT [10] GRANT CAPABILITY can_admin_scatter
+----
+ok
+
+# Scatters should work now.
+exec-privileged-op-tenant
+ALTER TABLE t SCATTER
+----
+ok
+
+# Revoke the capability using REVOKE syntax.
+update-capabilities
+ALTER TENANT [10] REVOKE CAPABILITY can_admin_scatter
+----
+ok
+
+# Scatters should no longer work.
+exec-privileged-op-tenant
+ALTER TABLE t SCATTER
+----
+pq: ba: AdminScatter [/Tenant/10/Table/104/1,/Tenant/10/Table/104/2) RPC error: rpc error: code = Unauthenticated desc = client tenant does not have capability "can_admin_scatter" (*kvpb.AdminScatterRequest)
+
+# Lastly, use the explicitly set to true syntax.
+update-capabilities
+ALTER TENANT [10] GRANT CAPABILITY can_admin_scatter=true
+----
+ok
+
+# Scatters should now work.
+exec-privileged-op-tenant
+ALTER TABLE t SCATTER
+----
+ok
diff --git a/pkg/ccl/multitenantccl/tenantcapabilitiesccl/testdata/can_admin_split b/pkg/ccl/multitenantccl/tenantcapabilitiesccl/testdata/can_admin_split
@@ -1,34 +1,79 @@
 query-sql-system
 SELECT * FROM [SHOW TENANT [10] WITH CAPABILITIES] WHERE capability_id = 'can_admin_split'
 ----
-10 tenant-10 ready none can_admin_split false
+10 tenant-10 ready none can_admin_split true
 
 exec-sql-tenant
 CREATE TABLE t(a INT)
 ----
 ok
 
+exec-sql-tenant
+CREATE INDEX idx on t(a)
+----
+ok
+
+# By default, we should be able to split.
+exec-privileged-op-tenant
+ALTER TABLE t SPLIT AT VALUES (0)
+----
+ok
+
+# ditto for the index.
+exec-privileged-op-tenant
+ALTER INDEX t@idx SPLIT AT VALUES (1)
+----
+ok
+
+
+update-capabilities
+ALTER TENANT [10] GRANT CAPABILITY can_admin_split=false
+----
+ok
+
 exec-privileged-op-tenant
 ALTER TABLE t SPLIT AT VALUES (0)
 ----
 pq: ba: AdminSplit [/Tenant/10/Table/104/1/0,/Min) RPC error: rpc error: code = Unauthenticated desc = client tenant does not have capability "can_admin_split" (*kvpb.AdminSplitRequest)
 
+# Check the index as well.
+exec-privileged-op-tenant
+ALTER INDEX t@idx SPLIT AT VALUES (1)
+----
+pq: ba: AdminSplit [/Tenant/10/Table/104/2/1,/Min) RPC error: rpc error: code = Unauthenticated desc = client tenant does not have capability "can_admin_split" (*kvpb.AdminSplitRequest)
+
+# Grant the capability without providing an explicit value.
 update-capabilities
-ALTER TENANT [10] GRANT CAPABILITY can_admin_split=true
+ALTER TENANT [10] GRANT CAPABILITY can_admin_split
 ----
 ok
 
+# Splits should work now.
 exec-privileged-op-tenant
 ALTER TABLE t SPLIT AT VALUES (0)
 ----
 ok
 
+# Revoke the capability using REVOKE syntax.
 update-capabilities
 ALTER TENANT [10] REVOKE CAPABILITY can_admin_split
 ----
 ok
 
+# Splits should no longer work.
 exec-privileged-op-tenant
 ALTER TABLE t SPLIT AT VALUES (0)
 ----
 pq: ba: AdminSplit [/Tenant/10/Table/104/1/0,/Min) RPC error: rpc error: code = Unauthenticated desc = client tenant does not have capability "can_admin_split" (*kvpb.AdminSplitRequest)
+
+# Lastly, use the explicitly set to true syntax.
+update-capabilities
+ALTER TENANT [10] GRANT CAPABILITY can_admin_split=true
+----
+ok
+
+# Splits should now work.
+exec-privileged-op-tenant
+ALTER TABLE t SPLIT AT VALUES (0)
+----
+ok
diff --git a/pkg/ccl/multitenantccl/tenantcapabilitiesccl/testdata/can_admin_unsplit b/pkg/ccl/multitenantccl/tenantcapabilitiesccl/testdata/can_admin_unsplit
@@ -0,0 +1,34 @@
+query-sql-system
+SELECT * FROM [SHOW TENANT [10] WITH CAPABILITIES] WHERE capability_id = 'can_admin_unsplit'
+----
+10 tenant-10 ready none can_admin_unsplit false
+
+exec-sql-tenant
+CREATE TABLE t(a INT)
+----
+ok
+
+exec-privileged-op-tenant
+ALTER TABLE t UNSPLIT AT VALUES (0)
+----
+pq: could not UNSPLIT AT (0): ba: AdminUnsplit [/Tenant/10/Table/104/1/0,/Min) RPC error: grpc: client tenant does not have capability "can_admin_unsplit" (*kvpb.AdminUnsplitRequest) [code 16/Unauthenticated]
+
+update-capabilities
+ALTER TENANT [10] GRANT CAPABILITY can_admin_unsplit=true
+----
+ok
+
+exec-privileged-op-tenant
+ALTER TABLE t UNSPLIT AT VALUES (0)
+----
+pq: could not UNSPLIT AT (0): key /Tenant/10/Table/104/1/0 is not the start of a range
+
+update-capabilities
+ALTER TENANT [10] REVOKE CAPABILITY can_admin_unsplit
+----
+ok
+
+exec-privileged-op-tenant
+ALTER TABLE t UNSPLIT AT VALUES (0)
+----
+pq: could not UNSPLIT AT (0): ba: AdminUnsplit [/Tenant/10/Table/104/1/0,/Min) RPC error: grpc: client tenant does not have capability "can_admin_unsplit" (*kvpb.AdminUnsplitRequest) [code 16/Unauthenticated]
diff --git a/pkg/sql/multitenant_admin_function_test.go b/pkg/sql/multitenant_admin_function_test.go
@@ -370,7 +370,9 @@ func (tc testCase) runTest(
 	}
 
 	execQueries(testCluster, systemDB, "system", tc.system)
-	execQueries(testCluster, secondaryDB, "secondary", tc.secondary)
+	if tc.secondary.isSet() {
+		execQueries(testCluster, secondaryDB, "secondary", tc.secondary)
+	}
 	if tc.secondaryWithoutClusterSetting.isSet() {
 		execQueries(testCluster, secondaryWithoutClusterSettingDB, "secondary_without_cluster_setting", tc.secondaryWithoutClusterSetting)
 	}
@@ -402,7 +404,8 @@ func (te tenantExpected) validate(t *testing.T, rows *gosql.Rows, err error, mes
 	}
 }
 
-// TestMultiTenantAdminFunction tests the "simple" admin functions that do not require complex setup.
+// TestMultiTenantAdminFunction tests the "simple" admin functions that do not
+// require complex setup.
 func TestMultiTenantAdminFunction(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
@@ -453,17 +456,10 @@ func TestMultiTenantAdminFunction(t *testing.T) {
 			system: tenantExpected{
 				result: [][]string{{"\xf0\x89\x89", "/1", maxTimestamp}},
 			},
-			secondary: tenantExpected{
-				result: [][]string{{"\xfe\x92\xf0\x89\x89", "/104/1/1", maxTimestamp}},
-			},
 			secondaryWithoutClusterSetting: tenantExpected{
 				errorMessage: "tenant cluster setting sql.split_at.allow_for_secondary_tenant.enabled disabled",
 			},
-			secondaryWithoutCapability: tenantExpected{
-				errorMessage: `does not have capability "can_admin_split"`,
-			},
 			queryClusterSetting: sql.SecondaryTenantSplitAtEnabled,
-			queryCapability:     tenantcapabilities.CanAdminSplit,
 		},
 		{
 			desc:  "ALTER INDEX x SPLIT AT",
@@ -472,17 +468,10 @@ func TestMultiTenantAdminFunction(t *testing.T) {
 			system: tenantExpected{
 				result: [][]string{{"\xf0\x8a\x89", "/1", maxTimestamp}},
 			},
-			secondary: tenantExpected{
-				result: [][]string{{"\xf0\x8a\x89", "/104/2/1", maxTimestamp}},
-			},
 			secondaryWithoutClusterSetting: tenantExpected{
 				errorMessage: "tenant cluster setting sql.split_at.allow_for_secondary_tenant.enabled disabled",
 			},
-			secondaryWithoutCapability: tenantExpected{
-				errorMessage: `does not have capability "can_admin_split"`,
-			},
 			queryClusterSetting: sql.SecondaryTenantSplitAtEnabled,
-			queryCapability:     tenantcapabilities.CanAdminSplit,
 		},
 		{
 			desc:  "ALTER TABLE x UNSPLIT AT",
@@ -564,17 +553,10 @@ func TestMultiTenantAdminFunction(t *testing.T) {
 			system: tenantExpected{
 				result: [][]string{{systemKey, systemKeyPretty}},
 			},
-			secondary: tenantExpected{
-				result: [][]string{{"\xfe\x92", secondaryKeyPretty}},
-			},
 			secondaryWithoutClusterSetting: tenantExpected{
 				errorMessage: "tenant cluster setting sql.scatter.allow_for_secondary_tenant.enabled disabled",
 			},
-			secondaryWithoutCapability: tenantExpected{
-				errorMessage: `does not have capability "can_admin_scatter"`,
-			},
-			queryClusterSetting: sql.SecondaryTenantScatterEnabled,
-			queryCapability:     tenantcapabilities.CanAdminScatter,
+			queryCapability: tenantcapabilities.CanAdminScatter,
 		},
 		{
 			desc:  "ALTER INDEX x SCATTER",
@@ -583,17 +565,10 @@ func TestMultiTenantAdminFunction(t *testing.T) {
 			system: tenantExpected{
 				result: [][]string{{"\xf0\x8a", "/Table/104/2"}},
 			},
-			secondary: tenantExpected{
-				result: [][]string{{"\xfe\x92", "/Tenant/10"}},
-			},
 			secondaryWithoutClusterSetting: tenantExpected{
 				errorMessage: "tenant cluster setting sql.scatter.allow_for_secondary_tenant.enabled disabled",
 			},
-			secondaryWithoutCapability: tenantExpected{
-				errorMessage: `does not have capability "can_admin_scatter"`,
-			},
 			queryClusterSetting: sql.SecondaryTenantScatterEnabled,
-			queryCapability:     tenantcapabilities.CanAdminScatter,
 		},
 	}
 
@@ -641,14 +616,7 @@ func TestTruncateTable(t *testing.T) {
 				{"…/104/2/1", "<after:/Tenant/11>"},
 			},
 		},
-		secondaryWithoutCapability: tenantExpected{
-			// CanAdminScatter will default to true so this will open happen if it is
-			// set to false.
-			errorMessage: `does not have capability "can_admin_scatter"`,
-		},
 		setupClusterSetting: sql.SecondaryTenantSplitAtEnabled,
-		setupCapability:     tenantcapabilities.CanAdminSplit,
-		queryCapability:     tenantcapabilities.CanAdminScatter,
 	}
 	tc.runTest(
 		t,