Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli/sql: announce the target tenant in the output #100920

Merged
merged 4 commits into from
Apr 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions pkg/cli/clisqlclient/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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.
Expand Down
41 changes: 30 additions & 11 deletions pkg/cli/clisqlshell/sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"context"
"fmt"
"io"
"net/url"
"os"
"os/exec"
"os/signal"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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 ""
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/democluster/demo_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 7 additions & 5 deletions pkg/cli/interactive_tests/test_client_side_checking.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand All @@ -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 ":/# "
Expand Down
22 changes: 16 additions & 6 deletions pkg/cli/interactive_tests/test_demo_cli_integration.tcl.disabled
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>"
Expand Down
8 changes: 1 addition & 7 deletions pkg/cli/interactive_tests/test_sql_version_reporting.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down