Merge cockroachdb#100920

100920: cli/sql: announce the target tenant in the output r=rafiss a=knz

Fixes cockroachdb#93129.
Epic: CRDB-23559

See individual commits for details.

Co-authored-by: Raphael 'kena' Poss <[email protected]>

pkg/cli/clisqlclient/conn.go

@@ -80,6 +80,11 @@ type sqlConn struct {
 	clusterID           string
 	clusterOrganization string
 
+	// isSystemTenantUnderSecondaryTenants is true if the current
+	// connection is to the system tenant and there are secondary
+	// tenants defined.
+	isSystemTenantUnderSecondaryTenants bool
+
 	// infow and errw are the streams where informational, error and
 	// warning messages are printed.
 	// Echoed queries, if Echo is enabled, are printed to errw too.
@@ -271,6 +276,30 @@ func (c *sqlConn) tryEnableServerExecutionTimings(ctx context.Context) error {
 func (c *sqlConn) GetServerMetadata(
 	ctx context.Context,
 ) (nodeID int32, version, clusterID string, retErr error) {
+	c.isSystemTenantUnderSecondaryTenants = false
+	val, err := c.QueryRow(ctx, `
+	SELECT EXISTS (SELECT 1 FROM system.information_schema.tables WHERE table_name='tenants')`)
+	if c.conn.IsClosed() {
+		return 0, "", "", MarkWithConnectionClosed(err)
+	}
+	if err != nil {
+		return 0, "", "", err
+	}
+	// We use toString() instead of casting val[0] to string because we
+	// get either a go string or bool depending on the SQL driver in
+	// use.
+	if toString(val[0])[0] == 't' {
+		val, err = c.QueryRow(ctx, `
+	SELECT EXISTS (SELECT 1 FROM system.public.tenants LIMIT 1)`)
+		if c.conn.IsClosed() {
+			return 0, "", "", MarkWithConnectionClosed(err)
+		}
+		if err != nil {
+			return 0, "", "", err
+		}
+		c.isSystemTenantUnderSecondaryTenants = toString(val[0])[0] == 't'
+	}
+
 	// Retrieve the node ID and server build info.
 	// Be careful to query against the empty database string, which avoids taking
 	// a lease against the current database (in case it's currently unavailable).
@@ -328,6 +357,7 @@ func (c *sqlConn) GetServerMetadata(
 		c.serverBuild = fmt.Sprintf("CockroachDB %s %s (%s, built %s, %s)",
 			v10fields[0], version, v10fields[2], v10fields[3], v10fields[4])
 	}
+
 	return nodeID, version, clusterID, nil
 }
 
@@ -395,6 +425,12 @@ func (c *sqlConn) checkServerMetadata(ctx context.Context) error {
 		fmt.Fprintf(c.errw, "warning: unable to retrieve the server's version: %s\n", err)
 	}
 
+	if c.isSystemTenantUnderSecondaryTenants {
+		fmt.Fprintln(c.infow, "#\n# ATTENTION: YOU ARE CONNECTED TO THE SYSTEM TENANT OF A MULTI-TENANT CLUSTER.\n"+
+			"# PROCEED WITH CAUTION. YOU ARE RESPONSIBLE FOR ENSURING THAT YOU DO NOT\n"+
+			"# PERFORM ANY OPERATIONS THAT COULD DAMAGE THE CLUSTER OR OTHER TENANTS.\n#")
+	}
+
 	// Report the server version only if it the revision has been
 	// fetched successfully, and the revision has changed since the last
 	// connection.

pkg/cli/clisqlshell/sql.go

@@ -16,7 +16,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"net/url"
 	"os"
 	"os/exec"
 	"os/signal"
@@ -130,10 +129,10 @@ Commands specific to the demo shell (EXPERIMENTAL):
   \demo add <locality>         add a node (locality specified as "region=<region>,zone=<zone>").
 `
 
-	defaultPromptPattern = "%n@%M/%/%x>"
+	defaultPromptPattern = "%n@%M/%C%/%x>"
 
 	// debugPromptPattern avoids substitution patterns that require a db roundtrip.
-	debugPromptPattern = "%n@%M>"
+	debugPromptPattern = "%n@%M %C>"
 )
 
 // cliState defines the current state of the CLI during
@@ -934,7 +933,7 @@ func (c *cliState) doRefreshPrompts(nextState cliStateEnum) cliStateEnum {
 	} else {
 		// Configure the editor to use the new prompt.
 
-		parsedURL, err := url.Parse(c.conn.GetURL())
+		parsedURL, err := pgurl.Parse(c.conn.GetURL())
 		if err != nil {
 			// If parsing fails, we'll keep the entire URL. The Open call succeeded, and that
 			// is the important part.
@@ -943,10 +942,7 @@ func (c *cliState) doRefreshPrompts(nextState cliStateEnum) cliStateEnum {
 			return nextState
 		}
 
-		userName := ""
-		if parsedURL.User != nil {
-			userName = parsedURL.User.Username()
-		}
+		userName := parsedURL.GetUsername()
 
 		dbName := unknownDbName
 		c.lastKnownTxnStatus = unknownTxnStatus
@@ -961,14 +957,35 @@ func (c *cliState) doRefreshPrompts(nextState cliStateEnum) cliStateEnum {
 			dbName = c.refreshDatabaseName()
 		}
 
+		// Do we have a "cluster" option; either as an options= parameter
+		// or as a prefix to the database name?
+		opts := parsedURL.GetExtraOptions()
+		var logicalCluster string
+		if extOptsS := opts.Get("options"); extOptsS != "" {
+			extOpts, err := pgurl.ParseExtendedOptions(extOptsS)
+			if err == nil {
+				logicalCluster = extOpts.Get("cluster")
+			}
+		}
+		if urlDB := parsedURL.GetDatabase(); strings.HasPrefix(urlDB, "cluster:") {
+			parts := strings.SplitN(urlDB, "/", 2)
+			logicalCluster = parts[0][len("cluster:"):]
+		}
+		if logicalCluster != "" {
+			logicalCluster += "/"
+		}
+
 		c.fullPrompt = rePromptFmt.ReplaceAllStringFunc(c.iCtx.customPromptPattern, func(m string) string {
 			switch m {
 			case "%M":
-				return parsedURL.Host // full host name.
+				_, host, port := parsedURL.GetNetworking()
+				return host + ":" + port // server:port
 			case "%m":
-				return parsedURL.Hostname() // host name.
+				_, host, _ := parsedURL.GetNetworking()
+				return host
 			case "%>":
-				return parsedURL.Port() // port.
+				_, _, port := parsedURL.GetNetworking()
+				return port
 			case "%n": // user name.
 				return userName
 			case "%/": // database name.
@@ -977,6 +994,8 @@ func (c *cliState) doRefreshPrompts(nextState cliStateEnum) cliStateEnum {
 				return c.lastKnownTxnStatus
 			case "%%":
 				return "%"
+			case "%C":
+				return logicalCluster
 			default:
 				err = fmt.Errorf("unrecognized format code in prompt: %q", m)
 				return ""

pkg/cli/democluster/demo_cluster.go

@@ -114,7 +114,7 @@ const demoOrg = "Cockroach Demo"
 const demoUsername = "demo"
 
 // demoTenantName is the name of the demo tenant.
-const demoTenantName = "demo-tenant"
+const demoTenantName = "demoapp"
 
 // LoggerFn is the type of a logger function to use by the
 // demo cluster to report special events.

pkg/cli/interactive_tests/test_client_side_checking.tcl

@@ -65,16 +65,18 @@ send "(echo '\\unset errexit'; echo 'begin;'; echo 'select 1+;'; echo 'select 1;
 eexpect "syntax error"
 eexpect "current transaction is aborted"
 eexpect ":/# "
-send "echo \$?\r"
-eexpect "1\r\n:/# "
+send "echo ::\$?::\r"
+eexpect "::1::"
+eexpect ":/# "
 
 send "(echo '\\unset errexit'; echo '\\set check_syntax'; echo 'begin;'; echo 'select 1+;'; echo 'select 1;'; echo 'commit;') | $argv sql\r"
 eexpect "syntax error"
 eexpect "1 row"
 eexpect "COMMIT"
 eexpect ":/# "
-send "echo \$?\r"
-eexpect "0\r\n:/# "
+send "echo ::\$?::\r"
+eexpect "::0::"
+eexpect ":/# "
 end_test
 
 start_test "Check that --debug-sql-cli sets suitable simplified client-side options."
@@ -96,7 +98,7 @@ eexpect "root@"
 send "\\set display_format csv\r\\set\r"
 eexpect "check_syntax,false"
 eexpect "echo,true"
-eexpect "prompt1,%n@%M>"
+eexpect "prompt1,%n@%M %C>"
 eexpect "root@"
 send "\\q\r"
 eexpect ":/# "

pkg/cli/interactive_tests/test_demo_cli_integration.tcl.disabled

@@ -47,28 +47,37 @@ eexpect "does not exist"
 eexpect "defaultdb>"
 send "\\q\r"
 eexpect eof
+
+spawn $argv sql --no-line-editor -u root -d cluster:demoapp
+eexpect "Welcome"
+eexpect root@
+eexpect "demoapp/defaultdb>"
+send "\\q\r"
+eexpect eof
 end_test
 
 start_test "Check that a SQL shell can connect to the system tenant without special arguments as demo"
 spawn $argv sql --no-line-editor -u demo -d cluster:system
 eexpect "Welcome"
+eexpect "ATTENTION: YOU ARE CONNECTED TO THE SYSTEM TENANT"
 eexpect demo@
-eexpect "defaultdb>"
+eexpect "system/defaultdb>"
 send "table system.tenants;\r"
 eexpect "2 rows"
-eexpect "defaultdb>"
+eexpect "system/defaultdb>"
 send "\\q\r"
 eexpect eof
 end_test
 
 start_test "Check that a SQL shell can connect to the system tenant without special arguments as root"
 spawn $argv sql --no-line-editor -u root -d cluster:system
 eexpect "Welcome"
+eexpect "ATTENTION: YOU ARE CONNECTED TO THE SYSTEM TENANT"
 eexpect root@
-eexpect "defaultdb>"
+eexpect "system/defaultdb>"
 send "table system.tenants;\r"
 eexpect "2 rows"
-eexpect "defaultdb>"
+eexpect "system/defaultdb>"
 send "\\q\r"
 eexpect eof
 end_test
@@ -93,11 +102,12 @@ system "$argv sql -d cluster:system/demo -u root -e \"alter user root with passw
 set ssldir $env(HOME)/.cockroach-demo
 spawn $argv sql --no-line-editor --url "postgresql://root:abc@/defaultdb?host=$ssldir&options=-ccluster%3Dsystem&port=26257"
 eexpect "Welcome"
+eexpect "ATTENTION: YOU ARE CONNECTED TO THE SYSTEM TENANT"
 eexpect root@
-eexpect "defaultdb>"
+eexpect "system/defaultdb>"
 send "table system.tenants;\r"
 eexpect "2 rows"
-eexpect "defaultdb>"
+eexpect "system/defaultdb>"
 send "\\q\r"
 eexpect eof
 end_test

pkg/cli/interactive_tests/test_demo_multitenant.tcl.disabled

@@ -11,7 +11,7 @@ start_test "Check --multitenant flag runs as expected"
 spawn $argv demo --no-line-editor --empty --nodes 3 --multitenant
 
 eexpect "Welcome"
-eexpect "You are connected to tenant \"demo-tenant\""
+eexpect "You are connected to tenant \"demoapp\""
 eexpect "(sql)"
 eexpect "127.0.0.1:26257/defaultdb"
 eexpect "defaultdb>"

pkg/cli/interactive_tests/test_sql_version_reporting.tcl

@@ -35,13 +35,7 @@ eexpect "connection closed unexpectedly"
 # Check that the prompt immediately succeeds the error message
 eexpect "connection lost"
 eexpect "opening new connection: all session settings will be lost"
-expect {
-    "\r\n# " {
-	report "unexpected server message"
-	exit 1
-    }
-    "root@" {}
-}
+eexpect "root@"
 
 # Check the reconnect did succeed - this also resets the connection state to good.
 send "select 1;\r"