Merge #110005

110005: externalconn/testutils: fix a mistake in SetSQLDBForUser r=stevendanna a=knz All commits but the last are from #110004 (review only the last commit here) Prior to this patch, this method was incorrectly swapping a SQL connection to the secondary tenant for one to the system tenant. This patch fixes it. Epic: CRDB-18499 Co-authored-by: Raphael 'kena' Poss <[email protected]>
cockroachdb · Sep 5, 2023 · 2b71df6 · 2b71df6
2 parents 6e0dfb1 + 547a048
commit 2b71df6
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 11 deletions.
diff --git a/pkg/ccl/partitionccl/zone_test.go b/pkg/ccl/partitionccl/zone_test.go
@@ -386,11 +386,14 @@ func TestZoneConfigAppliesToTemporaryIndex(t *testing.T) {
 		},
 	}
 
-	s, sqlDB, kvDB := serverutils.StartServer(t, params)
-	defer s.Stopper().Stop(context.Background())
+	srv, sqlDB, kvDB := serverutils.StartServer(t, params)
+	defer srv.Stopper().Stop(context.Background())
+
+	s := srv.ApplicationLayer()
+
 	tdb := sqlutils.MakeSQLRunner(sqlDB)
-	codec := s.ApplicationLayer().Codec()
-	sv := &s.ApplicationLayer().ClusterSettings().SV
+	codec := s.Codec()
+	sv := &s.ClusterSettings().SV
 	sql.SecondaryTenantZoneConfigsEnabled.Override(context.Background(), sv, true)
 
 	if _, err := sqlDB.Exec(`

diff --git a/pkg/cloud/externalconn/testutils/cluster.go b/pkg/cloud/externalconn/testutils/cluster.go
@@ -58,7 +58,7 @@ func (h *Handle) SetSQLDBForUser(tenantID roachpb.TenantID, user string) func()
 		return resetToRootUser
 	}
 
-	userSQLDB := h.tc.Server(0).SQLConnForUser(h.t, user, "")
+	userSQLDB := tenantState.ApplicationLayerInterface.SQLConnForUser(h.t, user, "")
 	tenantState.curDB = sqlutils.MakeSQLRunner(userSQLDB)
 	tenantState.userToDB[user] = tenantState.curDB
 

diff --git a/pkg/server/server_controller_new_server.go b/pkg/server/server_controller_new_server.go
@@ -245,6 +245,7 @@ func makeSharedProcessTenantServerConfig(
 	baseCfg.Locality = kvServerCfg.BaseConfig.Locality
 	baseCfg.SpanConfigsDisabled = kvServerCfg.BaseConfig.SpanConfigsDisabled
 	baseCfg.EnableDemoLoginEndpoint = kvServerCfg.BaseConfig.EnableDemoLoginEndpoint
+	baseCfg.DefaultZoneConfig = kvServerCfg.BaseConfig.DefaultZoneConfig
 
 	// TODO(knz): use a single network interface for all tenant servers.
 	// See: https://github.com/cockroachdb/cockroach/issues/92524

diff --git a/pkg/server/testserver.go b/pkg/server/testserver.go
@@ -1476,6 +1476,7 @@ func (ts *testServer) StartTenant(
 	baseCfg.StartDiagnosticsReporting = params.StartDiagnosticsReporting
 	baseCfg.DisableTLSForHTTP = params.DisableTLSForHTTP
 	baseCfg.EnableDemoLoginEndpoint = params.EnableDemoLoginEndpoint
+	baseCfg.DefaultZoneConfig = ts.Cfg.DefaultZoneConfig
 
 	// Waiting for capabilities can time To avoid paying this cost in all
 	// cases, we only set the nodelocal storage capability if the caller has

diff --git a/pkg/testutils/sqlutils/BUILD.bazel b/pkg/testutils/sqlutils/BUILD.bazel
@@ -24,6 +24,7 @@ go_library(
         "//pkg/sql/lexbase",
         "//pkg/sql/parser",
         "//pkg/sql/pgwire/pgerror",
+        "//pkg/sql/protoreflect",
         "//pkg/sql/sem/catconstants",
         "//pkg/sql/sem/tree",
         "//pkg/testutils",

diff --git a/pkg/testutils/sqlutils/zone.go b/pkg/testutils/sqlutils/zone.go
@@ -17,7 +17,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/config/zonepb"
 	"github.com/cockroachdb/cockroach/pkg/sql/lexbase"
-	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
+	"github.com/cockroachdb/cockroach/pkg/sql/protoreflect"
 )
 
 // ZoneRow represents a row returned by SHOW ZONE CONFIGURATION.
@@ -27,13 +27,17 @@ type ZoneRow struct {
 }
 
 func (row ZoneRow) sqlRowString() ([]string, error) {
-	configProto, err := protoutil.Marshal(&row.Config)
+	// Make the JSON comparable with the output of crdb_internal.pb_to_json.
+	configJSON, err := protoreflect.MessageToJSON(
+		&row.Config,
+		protoreflect.FmtFlags{EmitDefaults: false, EmitRedacted: false},
+	)
 	if err != nil {
 		return nil, err
 	}
 	return []string{
 		fmt.Sprintf("%d", row.ID),
-		string(configProto),
+		configJSON.String(),
 	}, nil
 }
 
@@ -83,8 +87,8 @@ func VerifyZoneConfigForTarget(t testing.TB, sqlDB *SQLRunner, target string, ro
 		t.Fatal(err)
 	}
 	sqlDB.CheckQueryResults(t, fmt.Sprintf(`
-SELECT zone_id, raw_config_protobuf
-  FROM [SHOW ZONE CONFIGURATION FOR %s]`, target),
+SELECT zone_id, crdb_internal.pb_to_json('cockroach.config.zonepb.ZoneConfig', raw_config_protobuf)::STRING
+FROM [SHOW ZONE CONFIGURATION FOR %s]`, target),
 		[][]string{sqlRow})
 }
 
@@ -100,7 +104,9 @@ func VerifyAllZoneConfigs(t testing.TB, sqlDB *SQLRunner, rows ...ZoneRow) {
 			t.Fatal(err)
 		}
 	}
-	sqlDB.CheckQueryResults(t, `SELECT zone_id, raw_config_protobuf FROM crdb_internal.zones`, expected)
+	sqlDB.CheckQueryResults(t, `
+SELECT zone_id, crdb_internal.pb_to_json('cockroach.config.zonepb.ZoneConfig', raw_config_protobuf)::STRING
+FROM crdb_internal.zones`, expected)
 }
 
 // ZoneConfigExists returns whether a zone config with the provided name exists.