sql: use PGUrl method from the application layer

This commit replaces almost all usages of `pgurlutils` package within
the `sql` directory in tests with the corresponding `PGUrl` call on the
`ApplicationLayerInterface` since I found this to be a necessary change
in several tests when making them work with secondary tenants. Perhaps
this wasn't necessary, but it seems cleaner and more consistent this
way.

Release note: None

Author: yuzefovich

pkg/ccl/multiregionccl/cold_start_latency_test.go

@@ -323,7 +323,7 @@ COMMIT;`}
 		defer tenant.AppStopper().Stop(ctx)
 		pgURL, cleanup, err := pgurlutils.PGUrlWithOptionalClientCertsE(
 			tenant.AdvSQLAddr(), "tenantdata", url.UserPassword("foo", password),
-			false, "", // withClientCerts
+			false /* withClientCerts */, "", /* certName */
 		)
 		if !assert.NoError(t, err) {
 			return

pkg/server/testserver.go

@@ -516,6 +516,7 @@ func (ts *testServer) SQLConnE(opts ...serverutils.SQLConnOption) (*gosql.DB, er
 		ts.cfg.Insecure,
 		options.ClientCerts,
 		options.CertsDirPrefix,
+		options.CertName,
 	)
 }
 
@@ -544,6 +545,7 @@ func (ts *testServer) PGUrlE(opts ...serverutils.SQLConnOption) (url.URL, func()
 		ts.cfg.Insecure,
 		options.ClientCerts,
 		options.CertsDirPrefix,
+		options.CertName,
 	)
 }
 
@@ -1039,6 +1041,7 @@ func (t *testTenant) SQLConnE(opts ...serverutils.SQLConnOption) (*gosql.DB, err
 		t.Cfg.Insecure,
 		options.ClientCerts,
 		options.CertsDirPrefix,
+		options.CertName,
 	)
 }
 
@@ -1073,6 +1076,7 @@ func (t *testTenant) PGUrlE(opts ...serverutils.SQLConnOption) (url.URL, func(),
 		t.Cfg.Insecure,
 		options.ClientCerts,
 		options.CertsDirPrefix,
+		options.CertName,
 	)
 }
 

pkg/server/testserver_sqlconn.go

@@ -45,6 +45,7 @@ func pgURL(
 	insecure bool,
 	clientCerts bool,
 	prefix string,
+	certName string,
 ) (pgURL url.URL, cleanupFn func(), err error) {
 	opts := url.Values{}
 	if tenantName != "" && !strings.HasPrefix(dbName, "cluster:") {
@@ -65,7 +66,7 @@ func pgURL(
 	}
 
 	// No LoopbackListener
-	pgURL, cleanupFn, err = pgurlutils.PGUrlWithOptionalClientCertsE(sqlAddr, prefix, user, clientCerts, "")
+	pgURL, cleanupFn, err = pgurlutils.PGUrlWithOptionalClientCertsE(sqlAddr, prefix, user, clientCerts, certName)
 	if err != nil {
 		return pgURL, cleanupFn, err
 	}
@@ -95,9 +96,10 @@ func openTestSQLConn(
 	insecure bool,
 	clientCerts bool,
 	prefix string,
+	certName string,
 ) (*gosql.DB, error) {
 	var goDB *gosql.DB
-	u, cleanupFn, err := pgURL(dbName, user, tenantName, sqlAddr, insecure, clientCerts, prefix)
+	u, cleanupFn, err := pgURL(dbName, user, tenantName, sqlAddr, insecure, clientCerts, prefix, certName)
 	if err != nil {
 		return nil, err
 	}

pkg/sql/BUILD.bazel

@@ -942,7 +942,6 @@ go_test(
         "//pkg/testutils/jobutils",
         "//pkg/testutils/kvclientutils",
         "//pkg/testutils/pgtest",
-        "//pkg/testutils/pgurlutils",
         "//pkg/testutils/serverutils",
         "//pkg/testutils/skip",
         "//pkg/testutils/sqlutils",

pkg/sql/colenc/BUILD.bazel

@@ -80,7 +80,6 @@ go_test(
         "//pkg/sql/sem/tree",
         "//pkg/sql/sessiondata",
         "//pkg/sql/types",
-        "//pkg/testutils/pgurlutils",
         "//pkg/testutils/serverutils",
         "//pkg/testutils/sqlutils",
         "//pkg/testutils/testcluster",

pkg/sql/colenc/bench_test.go

@@ -8,7 +8,6 @@ package colenc_test
 import (
 	"context"
 	"io"
-	"net/url"
 	"testing"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
@@ -24,7 +23,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
-	"github.com/cockroachdb/cockroach/pkg/testutils/pgurlutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
@@ -90,7 +88,7 @@ func BenchmarkTCPHLineItem(b *testing.B) {
 	defer srv.Stopper().Stop(ctx)
 	s := srv.ApplicationLayer()
 
-	url, cleanup := pgurlutils.PGUrl(b, s.AdvSQLAddr(), "copytest", url.User(username.RootUser))
+	url, cleanup := s.PGUrl(b, serverutils.CertsDirPrefix("copytest"), serverutils.User(username.RootUser))
 	defer cleanup()
 	var sqlConnCtx clisqlclient.Context
 	conn := sqlConnCtx.MakeSQLConn(io.Discard, io.Discard, url.String())

pkg/sql/conn_executor_test.go

@@ -47,7 +47,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlstats/persistedsqlstats/sqlstatstestutil"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/pgtest"
-	"github.com/cockroachdb/cockroach/pkg/testutils/pgurlutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
@@ -80,11 +79,12 @@ func TestSessionFinishRollsBackTxn(t *testing.T) {
 	defer aborter.Close(t)
 	params, _ := createTestServerParamsAllowTenants()
 	params.Knobs.SQLExecutor = aborter.executorKnobs()
-	s, mainDB, _ := serverutils.StartServer(t, params)
-	defer s.Stopper().Stop(context.Background())
+	srv, mainDB, _ := serverutils.StartServer(t, params)
+	defer srv.Stopper().Stop(context.Background())
+	s := srv.ApplicationLayer()
 	{
-		pgURL, cleanup := pgurlutils.PGUrl(
-			t, s.AdvSQLAddr(), "TestSessionFinishRollsBackTxn", url.User(username.RootUser))
+		pgURL, cleanup := s.PGUrl(
+			t, serverutils.CertsDirPrefix("TestSessionFinishRollsBackTxn"), serverutils.User(username.RootUser))
 		defer cleanup()
 		if err := aborter.Init(pgURL); err != nil {
 			t.Fatal(err)
@@ -108,7 +108,7 @@ CREATE TABLE t.test (k INT PRIMARY KEY, v TEXT);
 	for _, state := range tests {
 		t.Run(state, func(t *testing.T) {
 			// Create a low-level lib/pq connection so we can close it at will.
-			pgURL, cleanup := s.ApplicationLayer().PGUrl(t)
+			pgURL, cleanup := s.PGUrl(t)
 			defer cleanup()
 			c, err := pq.Open(pgURL.String())
 			if err != nil {
@@ -960,15 +960,16 @@ func TestErrorDuringPrepareInExplicitTransactionPropagates(t *testing.T) {
 
 	ctx := context.Background()
 	filter := newDynamicRequestFilter()
-	s, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{
+	srv, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{
 		Knobs: base.TestingKnobs{
 			Store: &kvserver.StoreTestingKnobs{
 				TestingRequestFilter: filter.filter,
 			},
 		},
 	})
-	defer s.Stopper().Stop(ctx)
-	codec := s.ApplicationLayer().Codec()
+	defer srv.Stopper().Stop(ctx)
+	s := srv.ApplicationLayer()
+	codec := s.Codec()
 
 	testDB := sqlutils.MakeSQLRunner(sqlDB)
 	testDB.Exec(t, "CREATE TABLE foo (i INT PRIMARY KEY)")
@@ -981,7 +982,7 @@ func TestErrorDuringPrepareInExplicitTransactionPropagates(t *testing.T) {
 	// transaction state evolves appropriately.
 
 	// Use pgx so that we can introspect error codes returned from cockroach.
-	pgURL, cleanup := pgurlutils.PGUrl(t, s.AdvSQLAddr(), "", url.User("root"))
+	pgURL, cleanup := s.PGUrl(t, serverutils.User(username.RootUser))
 	defer cleanup()
 	conf, err := pgx.ParseConfig(pgURL.String())
 	require.NoError(t, err)
@@ -1543,9 +1544,9 @@ func TestInjectRetryErrors(t *testing.T) {
 			}
 		},
 	}
-	s, db, _ := serverutils.StartServer(t, params)
-	defer s.Stopper().Stop(ctx)
-	defer db.Close()
+	srv, db, _ := serverutils.StartServer(t, params)
+	defer srv.Stopper().Stop(ctx)
+	s := srv.ApplicationLayer()
 
 	_, err := db.Exec("SET inject_retry_errors_enabled = 'true'")
 	require.NoError(t, err)
@@ -1656,8 +1657,9 @@ func TestInjectRetryErrors(t *testing.T) {
 
 		// Choose a small results_buffer_size and make sure the statement retry
 		// does not occur.
-		pgURL, cleanupFn := pgurlutils.PGUrl(
-			t, s.AdvSQLAddr(), t.Name(), url.User(username.RootUser))
+		pgURL, cleanupFn := s.PGUrl(
+			t, serverutils.CertsDirPrefix(t.Name()), serverutils.User(username.RootUser),
+		)
 		defer cleanupFn()
 		q := pgURL.Query()
 		q.Add("results_buffer_size", "4")

pkg/sql/copy/BUILD.bazel

@@ -33,7 +33,6 @@ go_test(
         "//pkg/sql/types",
         "//pkg/testutils",
         "//pkg/testutils/datapathutils",
-        "//pkg/testutils/pgurlutils",
         "//pkg/testutils/serverutils",
         "//pkg/testutils/skip",
         "//pkg/testutils/sqlutils",
@@ -45,7 +44,6 @@ go_test(
         "//pkg/util/leaktest",
         "//pkg/util/log",
         "//pkg/util/randutil",
-        "//pkg/util/stop",
         "//pkg/util/timeofday",
         "//pkg/util/timetz",
         "@com_github_cockroachdb_apd_v3//:apd",

pkg/sql/copy/copy_in_test.go

@@ -11,7 +11,6 @@ import (
 	"io"
 	"math"
 	"math/rand"
-	"net/url"
 	"reflect"
 	"strconv"
 	"strings"
@@ -30,7 +29,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sqltestutils"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
-	"github.com/cockroachdb/cockroach/pkg/testutils/pgurlutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
 	"github.com/cockroachdb/cockroach/pkg/util/admission/admissionpb"
@@ -792,8 +790,9 @@ func TestMessageSizeTooBig(t *testing.T) {
 	ctx := context.Background()
 	srv := serverutils.StartServerOnly(t, base.TestServerArgs{})
 	defer srv.Stopper().Stop(ctx)
+	s := srv.ApplicationLayer()
 
-	url, cleanup := pgurlutils.PGUrl(t, srv.ApplicationLayer().AdvSQLAddr(), "copytest", url.User(username.RootUser))
+	url, cleanup := s.PGUrl(t, serverutils.CertsDirPrefix("copytest"), serverutils.User(username.RootUser))
 	defer cleanup()
 	var sqlConnCtx clisqlclient.Context
 	conn := sqlConnCtx.MakeSQLConn(io.Discard, io.Discard, url.String())
@@ -841,7 +840,7 @@ func TestCopyExceedsSQLMemory(t *testing.T) {
 
 					s := srv.ApplicationLayer()
 
-					url, cleanup := pgurlutils.PGUrl(t, s.AdvSQLAddr(), "copytest", url.User(username.RootUser))
+					url, cleanup := s.PGUrl(t, serverutils.CertsDirPrefix("copytest"), serverutils.User(username.RootUser))
 					defer cleanup()
 					var sqlConnCtx clisqlclient.Context
 					conn := sqlConnCtx.MakeSQLConn(io.Discard, io.Discard, url.String())

pkg/sql/copy/copy_out_test.go

@@ -24,7 +24,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/randutil"
-	"github.com/cockroachdb/cockroach/pkg/util/stop"
 	"github.com/jackc/pgx/v5"
 	"github.com/stretchr/testify/require"
 )
@@ -46,12 +45,12 @@ func TestCopyOutTransaction(t *testing.T) {
 	`)
 	require.NoError(t, err)
 
-	pgURL, cleanupGoDB, err := s.PGUrlE(
+	pgURL, cleanupGoDB := s.PGUrl(
+		t,
 		serverutils.CertsDirPrefix("StartServer"),
 		serverutils.User(username.RootUser),
 	)
-	require.NoError(t, err)
-	s.AppStopper().AddCloser(stop.CloserFn(func() { cleanupGoDB() }))
+	defer cleanupGoDB()
 	config, err := pgx.ParseConfig(pgURL.String())
 	require.NoError(t, err)
 
@@ -113,12 +112,12 @@ func TestCopyOutRandom(t *testing.T) {
 
 	// Use pgx for this next bit as it allows selecting rows by raw values.
 	// Furthermore, it handles CopyTo!
-	pgURL, cleanupGoDB, err := s.PGUrlE(
+	pgURL, cleanupGoDB := s.PGUrl(
+		t,
 		serverutils.CertsDirPrefix("StartServer"),
 		serverutils.User(username.RootUser),
 	)
-	require.NoError(t, err)
-	s.AppStopper().AddCloser(stop.CloserFn(func() { cleanupGoDB() }))
+	defer cleanupGoDB()
 	config, err := pgx.ParseConfig(pgURL.String())
 	require.NoError(t, err)
 	config.Database = sqlutils.TestDB