Merge pull request #105211 from cockroachdb/blathers/backport-release…

…-23.1-105116

pkg/cli/democluster/BUILD.bazel

@@ -23,6 +23,7 @@ go_library(
         "//pkg/cli/democluster/api",
         "//pkg/jobs",
         "//pkg/kv/kvserver/liveness/livenesspb",
+        "//pkg/multitenant",
         "//pkg/roachpb",
         "//pkg/rpc",
         "//pkg/security",

pkg/cli/democluster/demo_cluster.go

@@ -28,6 +28,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/cli/cliflags"
 	"github.com/cockroachdb/cockroach/pkg/jobs"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness/livenesspb"
+	"github.com/cockroachdb/cockroach/pkg/multitenant"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/rpc"
 	"github.com/cockroachdb/cockroach/pkg/security"
@@ -517,7 +518,7 @@ func (c *transientCluster) Start(ctx context.Context) (err error) {
 			// Choose the tenant to use when no tenant is specified on a
 			// connection or web URL.
 			if _, err := ie.Exec(ctx, "default-tenant", nil,
-				`SET CLUSTER SETTING `+server.DefaultTenantSelectSettingName+` = $1`,
+				`SET CLUSTER SETTING `+multitenant.DefaultTenantSelectSettingName+` = $1`,
 				demoTenantName); err != nil {
 				return err
 			}

pkg/multitenant/BUILD.bazel

@@ -7,6 +7,7 @@ go_library(
         "constants.go",
         "cost_controller.go",
         "doc.go",
+        "tenant_config.go",
         "tenant_usage.go",
     ],
     importpath = "github.com/cockroachdb/cockroach/pkg/multitenant",
@@ -18,6 +19,7 @@ go_library(
         "//pkg/roachpb",
         "//pkg/settings",
         "//pkg/sql/isql",
+        "//pkg/sql/sem/catconstants",
         "//pkg/sql/sqlliveness",
         "//pkg/util/metric",
         "//pkg/util/stop",

pkg/multitenant/tenant_config.go

@@ -0,0 +1,40 @@
+// Copyright 2023 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package multitenant
+
+import (
+	"github.com/cockroachdb/cockroach/pkg/settings"
+	"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
+)
+
+// DefaultTenantSelectSettingName is the name of the setting that
+// configures the default tenant to use when a client does not specify
+// a specific tenant.
+var DefaultTenantSelectSettingName = "server.controller.default_tenant"
+
+// DefaultTenantSelect determines which tenant serves requests from
+// clients that do not specify explicitly the tenant they want to use.
+var DefaultTenantSelect = settings.RegisterStringSetting(
+	settings.SystemOnly,
+	DefaultTenantSelectSettingName,
+	"name of the tenant to use to serve requests when clients don't specify a tenant",
+	catconstants.SystemTenantName,
+).WithPublic()
+
+// VerifyTenantService determines whether there should be an advisory
+// interlock between changes to the tenant service and changes to the
+// above cluster setting.
+var VerifyTenantService = settings.RegisterBoolSetting(
+	settings.SystemOnly,
+	"server.controller.default_tenant.check_service.enabled",
+	"verify that a tenant's service is coherently set with the value of "+DefaultTenantSelectSettingName,
+	true,
+)

pkg/server/authentication.go

@@ -24,6 +24,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/base"
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
+	"github.com/cockroachdb/cockroach/pkg/multitenant"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/security/password"
@@ -845,7 +846,7 @@ func findAndDecodeSessionCookie(
 // findSessionCookieValueForTenant finds the encoded session in the provided
 // aggregated session cookie value established in multi-tenant clusters that's
 // associated with the provided tenant name. If an empty tenant name is provided,
-// we default to the defaultTenantSelect cluster setting value.
+// we default to the DefaultTenantSelect cluster setting value.
 //
 // If the method cannot find a match between the tenant name and session, or
 // if the provided session cookie is nil, it will return an empty string.
@@ -875,7 +876,7 @@ func findSessionCookieValueForTenant(
 			return mtSessionStr, nil
 		}
 		if tenantName == "" {
-			tenantName = defaultTenantSelect.Get(&st.SV)
+			tenantName = multitenant.DefaultTenantSelect.Get(&st.SV)
 		}
 		var encodedSession string
 		for idx, val := range sessionSlice {

pkg/server/server_controller.go

@@ -17,7 +17,6 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/base"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
-	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgwirecancel"
@@ -131,18 +130,6 @@ func newServerController(
 	return c
 }
 
-// DefaultTenantSelectSettingName is the name of the setting that
-// configures the default tenant to use when a client does not specify
-// a specific tenant.
-var DefaultTenantSelectSettingName = "server.controller.default_tenant"
-
-var defaultTenantSelect = settings.RegisterStringSetting(
-	settings.SystemOnly,
-	DefaultTenantSelectSettingName,
-	"name of the tenant to use to serve requests when clients don't specify a tenant",
-	catconstants.SystemTenantName,
-).WithPublic()
-
 // tenantServerWrapper implements the onDemandServer interface for SQLServerWrapper.
 type tenantServerWrapper struct {
 	stopper *stop.Stopper

pkg/server/server_controller_http.go

@@ -16,6 +16,7 @@ import (
 	"net/http"
 	"strings"
 
+	"github.com/cockroachdb/cockroach/pkg/multitenant"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -93,7 +94,7 @@ func (c *serverController) httpMux(w http.ResponseWriter, r *http.Request) {
 		// tenant names or sessions (common during development). Otherwise the user can
 		// get into a state where they cannot load DB Console assets at all due to invalid
 		// cookies.
-		defaultTenantName := roachpb.TenantName(defaultTenantSelect.Get(&c.st.SV))
+		defaultTenantName := roachpb.TenantName(multitenant.DefaultTenantSelect.Get(&c.st.SV))
 		s, err = c.getServer(ctx, defaultTenantName)
 		if err != nil {
 			log.Warningf(ctx, "unable to find server for default tenant %q: %v", defaultTenantName, err)
@@ -123,7 +124,7 @@ func getTenantNameFromHTTPRequest(st *cluster.Settings, r *http.Request) roachpb
 	}
 
 	// No luck so far. Use the configured default.
-	return roachpb.TenantName(defaultTenantSelect.Get(&st.SV))
+	return roachpb.TenantName(multitenant.DefaultTenantSelect.Get(&st.SV))
 }
 
 func getSessionFromCookie(sessionStr string, name roachpb.TenantName) string {
@@ -221,7 +222,7 @@ func (c *serverController) attemptLoginToAllTenants() http.Handler {
 			// if it's one of the valid logins. Otherwise, we just use the
 			// first one in the list.
 			tenantSelection := tenantNameToSetCookieSlice[0].name
-			defaultName := defaultTenantSelect.Get(&c.st.SV)
+			defaultName := multitenant.DefaultTenantSelect.Get(&c.st.SV)
 			for _, t := range tenantNameToSetCookieSlice {
 				if t.name == defaultName {
 					tenantSelection = t.name

pkg/server/server_controller_sql.go

@@ -14,6 +14,7 @@ import (
 	"context"
 	"net"
 
+	"github.com/cockroachdb/cockroach/pkg/multitenant"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgwirecancel"
@@ -60,7 +61,7 @@ func (c *serverController) sqlMux(
 	case pgwire.PreServeReady:
 		tenantName := roachpb.TenantName(status.GetTenantName())
 		if tenantName == "" {
-			tenantName = roachpb.TenantName(defaultTenantSelect.Get(&c.st.SV))
+			tenantName = roachpb.TenantName(multitenant.DefaultTenantSelect.Get(&c.st.SV))
 		}
 
 		s, err := c.getServer(ctx, tenantName)

pkg/sql/logictest/testdata/logic_test/tenant

@@ -502,3 +502,50 @@ RESET disable_drop_tenant
 
 statement ok
 DROP TENANT nodelete
+
+subtest regression_105115
+
+statement ok
+CREATE TENANT noservice
+
+statement error shared service not enabled for tenant "noservice"
+SET CLUSTER SETTING server.controller.default_tenant = noservice
+
+statement ok
+SET CLUSTER SETTING server.controller.default_tenant.check_service.enabled = false
+
+statement ok
+SET CLUSTER SETTING server.controller.default_tenant = noservice
+
+statement ok
+RESET CLUSTER SETTING server.controller.default_tenant.check_service.enabled
+
+statement ok
+RESET CLUSTER SETTING server.controller.default_tenant
+
+statement ok
+DROP TENANT noservice;
+CREATE TENANT withservice;
+ALTER TENANT withservice START SERVICE SHARED
+
+statement ok
+SET CLUSTER SETTING server.controller.default_tenant = withservice
+
+statement error cannot stop service while tenant is selected as default
+ALTER TENANT withservice STOP SERVICE
+
+statement ok
+SET CLUSTER SETTING server.controller.default_tenant.check_service.enabled = false
+
+statement ok
+ALTER TENANT withservice STOP SERVICE
+
+# clean up
+statement ok
+RESET CLUSTER SETTING server.controller.default_tenant.check_service.enabled
+
+statement ok
+RESET CLUSTER SETTING server.controller.default_tenant
+
+statement ok
+DROP TENANT withservice

pkg/sql/set_cluster_setting.go

@@ -20,6 +20,8 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/docs"
+	"github.com/cockroachdb/cockroach/pkg/multitenant"
+	"github.com/cockroachdb/cockroach/pkg/multitenant/mtinfopb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
 	"github.com/cockroachdb/cockroach/pkg/server/telemetry"
@@ -146,7 +148,7 @@ func checkPrivilegesForSetting(ctx context.Context, p *planner, name string, act
 	return nil
 }
 
-// SetClusterSetting sets session variables.
+// SetClusterSetting sets cluster settings.
 // Privileges: super user.
 func (p *planner) SetClusterSetting(
 	ctx context.Context, n *tree.SetClusterSetting,
@@ -250,7 +252,6 @@ func (p *planner) getAndValidateTypedClusterSetting(
 }
 
 func (n *setClusterSettingNode) startExec(params runParams) error {
-
 	if strings.HasPrefix(n.name, "sql.defaults") {
 		params.p.BufferClientNotice(
 			params.ctx,
@@ -299,6 +300,17 @@ func (n *setClusterSettingNode) startExec(params runParams) error {
 	// Report tracked cluster settings via telemetry.
 	// TODO(justin): implement a more general mechanism for tracking these.
 	switch n.name {
+	case multitenant.DefaultTenantSelectSettingName:
+		if multitenant.VerifyTenantService.Get(&n.st.SV) && expectedEncodedValue != "" {
+			tr, err := GetTenantRecordByName(params.ctx, n.st, params.p.InternalSQLTxn(), roachpb.TenantName(expectedEncodedValue))
+			if err != nil {
+				return errors.Wrapf(err, "failed to lookup tenant %q", expectedEncodedValue)
+			}
+			if tr.ServiceMode != mtinfopb.ServiceModeShared {
+				return pgerror.Newf(pgcode.ObjectNotInPrerequisiteState,
+					"shared service not enabled for tenant %q", expectedEncodedValue)
+			}
+		}
 	case catpb.AutoStatsEnabledSettingName:
 		switch expectedEncodedValue {
 		case "true":

pkg/sql/tenant_update.go

@@ -16,6 +16,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/keys"
+	"github.com/cockroachdb/cockroach/pkg/multitenant"
 	"github.com/cockroachdb/cockroach/pkg/multitenant/mtinfopb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
@@ -111,6 +112,23 @@ func validateTenantInfo(
 		}
 	}
 
+	// Sanity check. Note that this interlock is not a guarantee that
+	// the cluster setting will never be set to an invalid tenant. There
+	// is a race condition between changing the cluster setting and the
+	// check here. Generally, other subsystems should always tolerate
+	// when the cluster setting is set to a tenant without service (or
+	// even one that doesn't exist).
+	if multitenant.VerifyTenantService.Get(&settings.SV) &&
+		info.ServiceMode == mtinfopb.ServiceModeNone &&
+		info.Name != "" &&
+		multitenant.DefaultTenantSelect.Get(&settings.SV) == string(info.Name) {
+		return errors.WithHintf(
+			pgerror.Newf(pgcode.ObjectNotInPrerequisiteState,
+				"cannot stop service while tenant is selected as default"),
+			"Update the cluster setting %q to a different value.",
+			multitenant.DefaultTenantSelectSettingName)
+	}
+
 	return nil
 }