release-19.1: cli: ensure SQL commands work when defaultdb d… (#41131)

release-19.1: cli: ensure SQL commands work when `defaultdb` does not exist

pkg/cli/cli_test.go

@@ -1695,6 +1695,8 @@ func Example_user() {
 	c.Run("user ls --format=table")
 	c.Run("user rm foo")
 	c.Run("user ls --format=table")
+	c.RunWithArgs([]string{"sql", "-e", "drop database defaultdb"})
+	c.Run("user set foo")
 
 	// Output:
 	// user ls
@@ -1779,6 +1781,10 @@ func Example_user() {
 	//   table
 	//   ομηρος
 	// (10 rows)
+	// sql -e drop database defaultdb
+	// DROP DATABASE
+	// user set foo
+	// CREATE USER 1
 }
 
 func Example_cert() {
@@ -1900,6 +1906,8 @@ func Example_node() {
 	c.Run("node ls")
 	c.Run("node ls --format=table")
 	c.Run("node status 10000")
+	c.RunWithArgs([]string{"sql", "-e", "drop database defaultdb"})
+	c.Run("node ls")
 
 	// Output:
 	// node ls
@@ -1912,6 +1920,11 @@ func Example_node() {
 	// (1 row)
 	// node status 10000
 	// Error: node 10000 doesn't exist
+	// sql -e drop database defaultdb
+	// DROP DATABASE
+	// node ls
+	// id
+	// 1
 }
 
 func TestCLITimeout(t *testing.T) {

pkg/cli/dump.go

@@ -56,7 +56,7 @@ var verDump = version.MustParse("v2.1.0-alpha.20180416-0")
 // The approach here and its current flaws are summarized
 // in https://github.com/cockroachdb/cockroach/issues/28948.
 func runDump(cmd *cobra.Command, args []string) error {
-	conn, err := getPasswordAndMakeSQLClient("cockroach dump")
+	conn, err := makeSQLClient("cockroach dump", useDefaultDb)
 	if err != nil {
 		return err
 	}

pkg/cli/node.go

@@ -50,7 +50,7 @@ To retrieve the IDs for inactive members, see 'node status --decommission'.
 }
 
 func runLsNodes(cmd *cobra.Command, args []string) error {
-	conn, err := getPasswordAndMakeSQLClient("cockroach node ls")
+	conn, err := makeSQLClient("cockroach node ls", useSystemDb)
 	if err != nil {
 		return err
 	}
@@ -189,7 +189,7 @@ SELECT node_id AS id,
        draining AS is_draining
 FROM crdb_internal.gossip_liveness LEFT JOIN crdb_internal.gossip_nodes USING (node_id)`
 
-	conn, err := getPasswordAndMakeSQLClient("cockroach node status")
+	conn, err := makeSQLClient("cockroach node status", useSystemDb)
 	if err != nil {
 		return nil, nil, err
 	}

pkg/cli/sql.go

@@ -1412,7 +1412,7 @@ func runTerm(cmd *cobra.Command, args []string) error {
 		fmt.Print(infoMessage)
 	}
 
-	conn, err := getPasswordAndMakeSQLClient("cockroach sql")
+	conn, err := makeSQLClient("cockroach sql", useDefaultDb)
 	if err != nil {
 		return err
 	}

pkg/cli/sql_util.go

@@ -470,27 +470,46 @@ func makeSQLConn(url string) *sqlConn {
 	}
 }
 
-// getPasswordAndMakeSQLClient prompts for a password if running in secure mode
-// and no certificates have been supplied.
-// Attempting to use security.RootUser without valid certificates will return an error.
-func getPasswordAndMakeSQLClient(appName string) (*sqlConn, error) {
-	return makeSQLClient(appName)
-}
-
 var sqlConnTimeout = envutil.EnvOrDefaultString("COCKROACH_CONNECT_TIMEOUT", "5")
 
+// defaultSQLDb describes how a missing database part in the SQL
+// connection string is processed when creating a client connection.
+type defaultSQLDb int
+
+const (
+	// useSystemDb means that a missing database will be overridden with
+	// "system".
+	useSystemDb defaultSQLDb = iota
+	// useDefaultDb means that a missing database will be left as-is so
+	// that the server can default to "defaultdb".
+	useDefaultDb
+)
+
 // makeSQLClient connects to the database using the connection
 // settings set by the command-line flags.
+// If a password is needed, it also prompts for the password.
+//
+// If forceSystemDB is set, it also connects it to the `system`
+// database. The --database flag or database part in the URL is then
+// ignored.
 //
 // The appName given as argument is added to the URL even if --url is
 // specified, but only if the URL didn't already specify
 // application_name. It is prefixed with '$ ' to mark it as internal.
-func makeSQLClient(appName string) (*sqlConn, error) {
+func makeSQLClient(appName string, defaultMode defaultSQLDb) (*sqlConn, error) {
 	baseURL, err := cliCtx.makeClientConnURL()
 	if err != nil {
 		return nil, err
 	}
 
+	if defaultMode == useSystemDb && baseURL.Path == "" {
+		// Override the target database. This is because the current
+		// database can influence the output of CLI commands, and in the
+		// case where the database is missing it will default server-wise to
+		// `defaultdb` which may not exist.
+		baseURL.Path = "system"
+	}
+
 	// If there is no user in the URL already, fill in the default user.
 	if baseURL.User.Username() == "" {
 		baseURL.User = url.User(security.RootUser)

pkg/cli/user.go

@@ -42,7 +42,7 @@ var verRmUser = version.MustParse("v1.1.0-alpha.20170622")
 var verSetUser = version.MustParse("v1.2.0-alpha.20171113")
 
 func runGetUser(cmd *cobra.Command, args []string) error {
-	conn, err := getPasswordAndMakeSQLClient("cockroach user")
+	conn, err := makeSQLClient("cockroach user", useSystemDb)
 	if err != nil {
 		return err
 	}
@@ -73,7 +73,7 @@ List all users.
 }
 
 func runLsUsers(cmd *cobra.Command, args []string) error {
-	conn, err := getPasswordAndMakeSQLClient("cockroach user")
+	conn, err := makeSQLClient("cockroach user", useSystemDb)
 	if err != nil {
 		return err
 	}
@@ -94,7 +94,7 @@ Remove an existing user by username.
 }
 
 func runRmUser(cmd *cobra.Command, args []string) error {
-	conn, err := getPasswordAndMakeSQLClient("cockroach user")
+	conn, err := makeSQLClient("cockroach user", useSystemDb)
 	if err != nil {
 		return err
 	}
@@ -139,7 +139,7 @@ func runSetUser(cmd *cobra.Command, args []string) error {
 		}
 	}
 
-	conn, err := getPasswordAndMakeSQLClient("cockroach user")
+	conn, err := makeSQLClient("cockroach user", useSystemDb)
 	if err != nil {
 		return err
 	}

pkg/cli/zip.go

@@ -203,7 +203,7 @@ func runDebugZip(cmd *cobra.Command, args []string) error {
 	status := serverpb.NewStatusClient(conn)
 	admin := serverpb.NewAdminClient(conn)
 
-	sqlConn, err := getPasswordAndMakeSQLClient("cockroach sql")
+	sqlConn, err := makeSQLClient("cockroach sql", useSystemDb)
 	if err != nil {
 		log.Warningf(baseCtx, "unable to open a SQL session. Debug information will be incomplete: %s", err)
 	}

pkg/cli/zone.go

@@ -76,7 +76,7 @@ func runGetZone(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	conn, err := getPasswordAndMakeSQLClient("cockroach zone")
+	conn, err := makeSQLClient("cockroach zone", useSystemDb)
 	if err != nil {
 		return err
 	}
@@ -135,7 +135,7 @@ List zone configs.
 }
 
 func runLsZones(cmd *cobra.Command, args []string) error {
-	conn, err := getPasswordAndMakeSQLClient("cockroach zone")
+	conn, err := makeSQLClient("cockroach zone", useSystemDb)
 	if err != nil {
 		return err
 	}
@@ -189,7 +189,7 @@ Remove an existing zone config for the specified database or table.
 }
 
 func runRmZone(cmd *cobra.Command, args []string) error {
-	conn, err := getPasswordAndMakeSQLClient("cockroach zone")
+	conn, err := makeSQLClient("cockroach zone", useSystemDb)
 	if err != nil {
 		return err
 	}
@@ -268,7 +268,7 @@ func readZoneConfig() (conf []byte, err error) {
 // runSetZone parses the yaml input file, converts it to proto, and inserts it
 // in the system.zones table.
 func runSetZone(cmd *cobra.Command, args []string) error {
-	conn, err := getPasswordAndMakeSQLClient("cockroach zone")
+	conn, err := makeSQLClient("cockroach zone", useSystemDb)
 	if err != nil {
 		return err
 	}