sqlproxyccl: fix the run proxy sample script

The pkg/ccl/sqlproxyccl/run_proxy.sh sets a multi-instance environment
with a host server, proxy server and a test directory server that can
start tenant processes on demand. Some recent changes prevent the script
from running due to the issues described in cockroachdb#71385.
This PR does the following:
- changes the script so it uses secure connections between all processes
- uses separate CAs when possible
- adds a cert directory option to the test directory server so tenant
  processes can be started with the correct set of certs
- adds a KV addrs flag to the test directory so the tenant processes can
  target the correct host server
- changes the capturing of the tenant processes output to continiously
  flow into test directory server log. Previously that was happening
  when the tenant process terminates.
- adds a store argument to the tenant process
- removes the --logtostderr proxy option
- changes the proxy http listen port to not interfere with default
  cockroach db port
- minor fix for the "tenant client cert not found" message to show the
  directory that is being searched for the cert

Fixes cockroachdb#71385

Release note: None

pkg/ccl/sqlproxyccl/proxy_handler_test.go

@@ -1131,7 +1131,7 @@ func newDirectoryServer(
 
 	// Create the tenant directory server.
 	tdsStopper := stop.NewStopper()
-	tds, err := tenantdirsvr.New(tdsStopper)
+	tds, err := tenantdirsvr.New(tdsStopper, tenantdirsvr.TestDirectoryCfg{})
 	require.NoError(t, err)
 
 	// Override the tenant starter function to start a new tenant process using

pkg/ccl/sqlproxyccl/run_proxy.sh

@@ -1,51 +1,83 @@
 #!/bin/bash
+set -euo pipefail
 
-# Sample script to run a minimal cockroachdb deployment using sqlproxy
-# listening on its default port at localhost:46257.
-# This consists of a single-node db cluster, a sql tenant server, and a proxy.
-# The proxy listens on :46257, forwarding to tenant SQL server at :36257.
-# Finally a sql shell is opened for tenant id 123 and user `bob`.
-# The password for `bob` is `builder`.
-#
-# WARNING: directory `~/.cockroach-certs` will be DELETED.
-# WARNING: all cockroach and sqlproxy processes will be killed.
-
-COCKROACH=${1:-'./cockroach'}
-SQLPROXY=${2:-$COCKROACH mt start-proxy --target-addr 127.0.0.1:36257}
-
-set -euxo pipefail
-
-# Prep work.
-rm -rf ~/.cockroach-certs cockroach-data
-killall -9 cockroach || true
-killall -9 sqlproxy || true
-
-# Create certificates.
-export CERTSDIR=$HOME/.cockroach-certs
-export COCKROACH_CA_KEY=$CERTSDIR/ca.key
-$COCKROACH cert create-ca
-$COCKROACH cert create-client root
-$COCKROACH cert create-node 127.0.0.1 localhost
-$COCKROACH mt cert create-tenant-client 123 --certs-dir=${HOME}/.cockroach-certs
-
-#  Start KV layer. (:26257)
-$COCKROACH start-single-node --host 127.0.0.1 --background
-# Create tenant
-$COCKROACH sql --host 127.0.0.1 -e 'select crdb_internal.create_tenant(123);'
-
-# Spawn tenant SQL server (:36257) pointing at KV layer (:26257)
-COCKROACH_TRUST_CLIENT_PROVIDED_SQL_REMOTE_ADDR=true $COCKROACH mt start-sql --tenant-id 123 --kv-addrs 127.0.0.1:26257 --sql-addr 127.0.0.1:36257 --logtostderr=NONE &
+COCKROACH=${1:-'cockroach'}
+BASE=${2:-$(mktemp -d -t 'tenant-test-XXXX')}
+
+# Setup host
+HOST=$BASE/host
+CLIENT=$BASE/client
+TENANT=$BASE/tenant
+DIR=$BASE/dir
+PROXY=$BASE/proxy
+
+mkdir -p $HOST
+mkdir -p $CLIENT
+mkdir -p $TENANT
+mkdir -p $DIR
+mkdir -p $PROXY
+
+#Ports
+HOST_P=55501
+HOST_HTTP_P=55502
+TENANT_P=55503
+TENANT_HTTP_P=55504
+DIR_P=55505
+PROXY_P=55506
+PROXY_HTTP_P=55507
+
+echo Create node CA and node cert
+COCKROACH_CA_KEY=$HOST/ca.key COCKROACH_CERTS_DIR=$HOST $COCKROACH cert create-ca 
+COCKROACH_CA_KEY=$HOST/ca.key COCKROACH_CERTS_DIR=$HOST $COCKROACH cert create-node 127.0.0.1 localhost
+
+echo Create client CA
+COCKROACH_CA_KEY=$CLIENT/ca.key COCKROACH_CERTS_DIR=$CLIENT $COCKROACH cert create-client-ca
+mv $CLIENT/ca-client.crt $CLIENT/ca.crt
+# SQL connections may have client CA or host CA issued certs on either side. 
+cat $HOST/ca.crt >> $CLIENT/ca.crt
+cp $CLIENT/ca.crt $HOST/ca-client.crt
+
+echo Create tenant CA
+COCKROACH_CA_KEY=$TENANT/ca.key COCKROACH_CERTS_DIR=$TENANT $COCKROACH cert create-tenant-client-ca
+mv $TENANT/ca-client-tenant.crt $TENANT/ca.crt
+cp $TENANT/ca.crt $HOST/ca-client-tenant.crt
+cat $HOST/ca.crt >> $TENANT/ca.crt
+
+echo Start KV layer
+$COCKROACH start-single-node --listen-addr=localhost:$HOST_P --http-addr=:$HOST_HTTP_P --background --certs-dir=$HOST --store=$HOST/store
+
+echo Create client host root cert
+COCKROACH_CA_KEY=$CLIENT/ca.key COCKROACH_CERTS_DIR=$CLIENT $COCKROACH cert create-client root
+echo Connect as root to the host cluster and create tenant
+COCKROACH_CA_KEY=$CLIENT/ca.key COCKROACH_CERTS_DIR=$CLIENT $COCKROACH sql --port=$HOST_P  -e " 
+SELECT CASE WHEN NOT EXISTS(SELECT * FROM system.tenants WHERE id = 123)
+THEN crdb_internal.create_tenant(123)
+END" > /dev/null
+
+echo Create cert for tenant 123
+COCKROACH_CA_KEY=$TENANT/ca.key COCKROACH_CERTS_DIR=$TENANT $COCKROACH cert create-tenant-client 123 127.0.0.1 ::1 localhost *.local
+echo Start the tenant to set a root password
+$COCKROACH mt start-sql  --certs-dir=$TENANT --kv-addrs=:$HOST_P --sql-addr=:$TENANT_P --http-addr=:$TENANT_HTTP_P --tenant-id=123 --log="{sinks: {stderr: {}}}" 2>$TENANT/tenant.stderr.log &
+TENANT_PID=$!
+echo Tenant PID is $TENANT_PID
+sleep 1
+
+echo Create client tenant root cert
+COCKROACH_CA_KEY=$TENANT/ca.key COCKROACH_CERTS_DIR=$TENANT $COCKROACH cert create-client root
+echo Set the password
+COCKROACH_CA_KEY=$TENANT/ca.key COCKROACH_CERTS_DIR=$TENANT $COCKROACH sql --port=$TENANT_P -e "alter user root with password 'secret'" > $TENANT/alter_root.log
+
+echo Shutdown the tenant
+kill $TENANT_PID
 sleep 1
 
-# Create user on tenant (proxy does not forward client certs)
-$COCKROACH sql --port 36257 -e "create user bob with password 'builder';"
+echo Start test directory server
+$COCKROACH mt test-directory --port=$DIR_P --kv-addrs=localhost:$HOST_P --certs-dir=$TENANT --base-dir=$TENANT --log="{sinks: {file-groups: {default: {dir: $DIR, channels: ALL}}}}" 2>$DIR/stderr.log &
 
-# Spawn the proxy on :46257, forwarding to tenant SQL server (:36257)
-$SQLPROXY --listen-addr 127.0.0.1:46257 --listen-cert $CERTSDIR/node.crt --listen-key $CERTSDIR/node.key &
+echo "Start the sql proxy server (with self signed client facing cert)"
+$COCKROACH mt start-proxy  --listen-addr=localhost:$PROXY_P --listen-cert=* --listen-key=* --directory=:$DIR_P --listen-metrics=:$PROXY_HTTP_P --skip-verify --log="{sinks: {file-groups: {default: {dir: $PROXY, channels: ALL}}}}" 2>$PROXY/stderr.log &
 
-sleep 2
+echo "All files are in $BASE"
+echo "To connect:"
+echo "  $COCKROACH sql --url=\"postgresql://root:secret@127.0.0.1:$PROXY_P?sslmode=require&sslrootcert=a&options=--cluster%3Dtenant-cluster-123\""
 
-# Connect to proxy to `defaultdb`. Note the need to prefix the db name with the
-# magic phrase 'prancing-pony'.
-# Password for user `bob` is `builder`.
-$COCKROACH sql --url "postgresql://bob:builder@127.0.0.1:46257/prancing-pony.defaultdb"

pkg/ccl/sqlproxyccl/tenant/directory_test.go

@@ -528,7 +528,7 @@ func newTestDirectory(
 	})
 	clusterStopper := tc.Stopper()
 	var err error
-	tds, err = tenantdirsvr.New(clusterStopper)
+	tds, err = tenantdirsvr.New(clusterStopper, tenantdirsvr.TestDirectoryCfg{})
 	require.NoError(t, err)
 	tds.TenantStarterFunc = func(ctx context.Context, tenantID uint64) (*tenantdirsvr.Process, error) {
 		t.Logf("starting tenant %d", tenantID)

pkg/ccl/sqlproxyccl/tenantdirsvr/BUILD.bazel

@@ -11,7 +11,6 @@ go_library(
         "//pkg/util/log",
         "//pkg/util/stop",
         "//pkg/util/syncutil",
-        "@com_github_cockroachdb_errors//:errors",
         "@com_github_cockroachdb_logtags//:logtags",
         "@org_golang_google_grpc//:go_default_library",
     ],

pkg/ccl/sqlproxyccl/tenantdirsvr/test_directory_svr.go

@@ -9,21 +9,21 @@
 package tenantdirsvr
 
 import (
-	"bytes"
+	"bufio"
 	"container/list"
 	"context"
 	"fmt"
 	"net"
 	"os"
 	"os/exec"
+	"strings"
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/ccl/sqlproxyccl/tenant"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/stop"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
-	"github.com/cockroachdb/errors"
 	"github.com/cockroachdb/logtags"
 	"google.golang.org/grpc"
 )
@@ -63,9 +63,19 @@ func NewSubStopper(parentStopper *stop.Stopper) *stop.Stopper {
 	return subStopper
 }
 
+// TestDirectoryCfg is used to pass configuration parameters to the
+// test directory server
+type TestDirectoryCfg struct {
+	// CertsDir if set will be passed to the tenant process when instantiated.
+	CertsDir      string
+	KVAddrs       string
+	TenantBaseDir string
+}
+
 // TestDirectoryServer is a directory server implementation that is used for
 // testing.
 type TestDirectoryServer struct {
+	TestDirectoryCfg
 	stopper             *stop.Stopper
 	grpcServer          *grpc.Server
 	cockroachExecutable string
@@ -85,13 +95,14 @@ type TestDirectoryServer struct {
 }
 
 // New will create a new server.
-func New(stopper *stop.Stopper) (*TestDirectoryServer, error) {
+func New(stopper *stop.Stopper, cfg TestDirectoryCfg) (*TestDirectoryServer, error) {
 	// Determine the path to cockroach executable.
 	cockroachExecutable, err := os.Executable()
 	if err != nil {
 		return nil, err
 	}
 	dir := &TestDirectoryServer{
+		TestDirectoryCfg:    cfg,
 		grpcServer:          grpc.NewServer(),
 		stopper:             stopper,
 		cockroachExecutable: cockroachExecutable,
@@ -382,6 +393,10 @@ func (s *TestDirectoryServer) deregisterInstance(tenantID uint64, sql net.Addr)
 	}
 }
 
+type writerFunc func(p []byte) (int, error)
+
+func (wf writerFunc) Write(p []byte) (int, error) { return wf(p) }
+
 // startTenantLocked is the default tenant process startup logic that runs the
 // cockroach db executable out of process.
 func (s *TestDirectoryServer) startTenantLocked(
@@ -398,11 +413,23 @@ func (s *TestDirectoryServer) startTenantLocked(
 	}
 	process := &Process{SQL: sql.Addr(), FakeLoad: 0.01}
 	args := []string{
-		"mt", "start-sql", "--kv-addrs=127.0.0.1:26257", "--idle-exit-after=30s",
+		"mt", "start-sql",
 		fmt.Sprintf("--sql-addr=%s", sql.Addr().String()),
 		fmt.Sprintf("--http-addr=%s", http.Addr().String()),
 		fmt.Sprintf("--tenant-id=%d", tenantID),
-		"--insecure",
+	}
+	if s.KVAddrs != "" {
+		args = append(args, fmt.Sprintf("--kv-addrs=%s", s.KVAddrs))
+	} else {
+		args = append(args, "--kv-addrs=localhost:26257")
+	}
+	if s.TenantBaseDir != "" {
+		args = append(args, fmt.Sprintf("--store=%s/store.0.%d", s.TenantBaseDir, tenantID))
+	}
+	if s.CertsDir != "" {
+		args = append(args, fmt.Sprintf("--certs-dir=%s", s.CertsDir))
+	} else {
+		args = append(args, "--insecure")
 	}
 	if err = sql.Close(); err != nil {
 		return nil, err
@@ -414,16 +441,15 @@ func (s *TestDirectoryServer) startTenantLocked(
 	c := exec.Command(s.cockroachExecutable, args...)
 	process.Cmd = c
 	c.Env = append(os.Environ(), "COCKROACH_TRUST_CLIENT_PROVIDED_SQL_REMOTE_ADDR=true")
-
-	if c.Stdout != nil {
-		return nil, errors.New("exec: Stdout already set")
-	}
-	if c.Stderr != nil {
-		return nil, errors.New("exec: Stderr already set")
+	var f writerFunc = func(p []byte) (int, error) {
+		sc := bufio.NewScanner(strings.NewReader(string(p)))
+		for sc.Scan() {
+			log.Infof(ctx, "%s", sc.Text())
+		}
+		return len(p), nil
 	}
-	var b bytes.Buffer
-	c.Stdout = &b
-	c.Stderr = &b
+	c.Stdout = f
+	c.Stderr = f
 	err = c.Start()
 	if err != nil {
 		return nil, err
@@ -436,7 +462,6 @@ func (s *TestDirectoryServer) startTenantLocked(
 	err = s.stopper.RunAsyncTask(ctx, "cmd-wait", func(ctx context.Context) {
 		if err := c.Wait(); err != nil {
 			log.Infof(ctx, "finished %s with err %s", process.Cmd.Args, err)
-			log.Infof(ctx, "output %s", b.Bytes())
 			return
 		}
 		log.Infof(ctx, "finished %s with success", process.Cmd.Args)

pkg/cli/cliflags/flags_mt.go

@@ -102,4 +102,14 @@ This rule must include the port of the SQL pod.`,
 		Name:        "port",
 		Description: "Test directory server binds and listens on this port.",
 	}
+
+	TestDirectoryTenantCertsDir = FlagInfo{
+		Name:        "certs-dir",
+		Description: CertsDir.Description,
+	}
+
+	TestDirectoryTenantBaseDir = FlagInfo{
+		Name:        "base-dir",
+		Description: "If set, the tenant processes will use it as a store location.",
+	}
 )

pkg/cli/context.go

@@ -635,7 +635,10 @@ func setProxyContextDefaults() {
 }
 
 var testDirectorySvrContext struct {
-	port int
+	port          int
+	certsDir      string
+	kvAddrs       string
+	tenantBaseDir string
 }
 
 func setTestDirectorySvrContextDefaults() {

pkg/cli/flags.go

@@ -990,6 +990,9 @@ func init() {
 	{
 		f := mtTestDirectorySvr.Flags()
 		intFlag(f, &testDirectorySvrContext.port, cliflags.TestDirectoryListenPort)
+		stringFlag(f, &testDirectorySvrContext.certsDir, cliflags.TestDirectoryTenantCertsDir)
+		stringFlag(f, &testDirectorySvrContext.tenantBaseDir, cliflags.TestDirectoryTenantBaseDir)
+		stringFlag(f, &testDirectorySvrContext.kvAddrs, cliflags.KVAddrs)
 	}
 
 	// userfile upload command.

pkg/cli/mt_test_directory.go

@@ -42,7 +42,12 @@ func runDirectorySvr(cmd *cobra.Command, args []string) (returnErr error) {
 	}
 	defer stopper.Stop(ctx)
 
-	tds, err := tenantdirsvr.New(stopper)
+	tds, err := tenantdirsvr.New(stopper,
+		tenantdirsvr.TestDirectoryCfg{
+			CertsDir:      testDirectorySvrContext.certsDir,
+			KVAddrs:       testDirectorySvrContext.kvAddrs,
+			TenantBaseDir: testDirectorySvrContext.tenantBaseDir,
+		})
 	if err != nil {
 		return err
 	}

pkg/security/certificate_manager.go

@@ -666,7 +666,7 @@ func (cm *CertificateManager) LoadCertificates() error {
 	}
 
 	if tenantCert == nil && cm.tenantIdentifier != 0 {
-		return makeErrorf(errors.New("tenant client cert not found"), "for %d", cm.tenantIdentifier)
+		return makeErrorf(errors.New("tenant client cert not found"), "for %d in %s", cm.tenantIdentifier, cm.certsDir)
 	}
 
 	if nodeClientCert == nil && nodeCert != nil {