diff --git a/pkg/cmd/roachtest/cluster.go b/pkg/cmd/roachtest/cluster.go index a4c5447ef1f2..a4f310f41507 100644 --- a/pkg/cmd/roachtest/cluster.go +++ b/pkg/cmd/roachtest/cluster.go @@ -2721,9 +2721,6 @@ func (c *clusterImpl) InternalPGUrl( return c.pgURLErr(ctx, l, nodes, opts) } -// Silence unused warning. -var _ = (&clusterImpl{}).InternalPGUrl - // ExternalPGUrl returns the external Postgres endpoint for the specified nodes. func (c *clusterImpl) ExternalPGUrl( ctx context.Context, l *logger.Logger, nodes option.NodeListOption, opts roachprod.PGURLOptions, diff --git a/pkg/cmd/roachtest/tests/copyfrom.go b/pkg/cmd/roachtest/tests/copyfrom.go index 5723e2563b4f..3eff4e313991 100644 --- a/pkg/cmd/roachtest/tests/copyfrom.go +++ b/pkg/cmd/roachtest/tests/copyfrom.go @@ -168,7 +168,11 @@ func runCopyFromCRDB(ctx context.Context, t test.Test, c cluster.Cluster, sf int t.Fatal(err) } } - urls, err := c.InternalPGUrl(ctx, t.L(), c.Node(1), roachprod.PGURLOptions{Auth: install.AuthUserPassword}) + // psql doesn't support the CRDB specific "allow_unsafe_internals" parameter. + urls, err := c.InternalPGUrl(ctx, t.L(), c.Node(1), roachprod.PGURLOptions{ + Auth: install.AuthUserPassword, + DisallowUnsafeInternals: true, + }) require.NoError(t, err) m := c.NewDeprecatedMonitor(ctx, c.All()) m.Go(func(ctx context.Context) error { diff --git a/pkg/cmd/roachtest/tests/network.go b/pkg/cmd/roachtest/tests/network.go index 9773a69e82be..3f99a26f5802 100644 --- a/pkg/cmd/roachtest/tests/network.go +++ b/pkg/cmd/roachtest/tests/network.go @@ -85,10 +85,6 @@ func runNetworkAuthentication(ctx context.Context, t test.Test, c cluster.Cluste c.Start(ctx, t.L(), startOpts, settings, c.Node(1)) } - t.L().Printf("retrieving server addresses...") - serverUrls, err := c.InternalPGUrl(ctx, t.L(), serverNodes, roachprod.PGURLOptions{Auth: install.AuthUserPassword}) - require.NoError(t, err) - t.L().Printf("fetching certs...") certsDir := fmt.Sprintf("/home/ubuntu/%s", install.CockroachNodeCertsDir) localCertsDir, err := filepath.Abs("./network-certs") @@ -206,7 +202,7 @@ SELECT $1::INT = ALL ( } // Construct a connection URL to server i. - url := serverUrls[server-1] + url := fmt.Sprintf("{pgurl:%d}", server) // Attempt a client connection to that server. t.L().Printf("server %d, attempt %d; url: %s\n", server, attempt, url) diff --git a/pkg/cmd/roachtest/tests/pg_regress.go b/pkg/cmd/roachtest/tests/pg_regress.go index 9d07d307aa44..e6aefc53ce3b 100644 --- a/pkg/cmd/roachtest/tests/pg_regress.go +++ b/pkg/cmd/roachtest/tests/pg_regress.go @@ -110,7 +110,7 @@ func initPGRegress(ctx context.Context, t test.Test, c cluster.Cluster) { } } - urls, err := c.InternalPGUrl(ctx, t.L(), node, roachprod.PGURLOptions{}) + urls, err := c.InternalPGUrl(ctx, t.L(), node, roachprod.PGURLOptions{DisallowUnsafeInternals: true}) if err != nil { t.Fatal(err) } @@ -149,7 +149,8 @@ func runPGRegress(ctx context.Context, t test.Test, c cluster.Cluster) { initPGRegress(ctx, t, c) node := c.Node(1) - urls, err := c.InternalPGUrl(ctx, t.L(), node, roachprod.PGURLOptions{}) + // psql doesn't support the CRDB specific "allow_unsafe_internals" parameter. + urls, err := c.InternalPGUrl(ctx, t.L(), node, roachprod.PGURLOptions{DisallowUnsafeInternals: true}) if err != nil { t.Fatal(err) } diff --git a/pkg/roachprod/failureinjection/failures/failure.go b/pkg/roachprod/failureinjection/failures/failure.go index ed21e10c8017..cba886f7a04b 100644 --- a/pkg/roachprod/failureinjection/failures/failure.go +++ b/pkg/roachprod/failureinjection/failures/failure.go @@ -160,7 +160,7 @@ func (f *GenericFailure) Conn( if !c.Secure { authMode = install.AuthRootCert } - nodeURL := c.NodeURL(ip, desc.Port, "" /* virtualClusterName */, desc.ServiceMode, authMode, "" /* database */) + nodeURL := c.NodeURL(ip, desc.Port, "" /* virtualClusterName */, desc.ServiceMode, authMode, "" /* database */, false /* disallowUnsafeInternals */) nodeURL = strings.Trim(nodeURL, "'") pgurl, err := url.Parse(nodeURL) if err != nil { diff --git a/pkg/roachprod/install/cluster_synced.go b/pkg/roachprod/install/cluster_synced.go index d35fbe1cb24b..2ae0296ee66d 100644 --- a/pkg/roachprod/install/cluster_synced.go +++ b/pkg/roachprod/install/cluster_synced.go @@ -2304,7 +2304,7 @@ func (c *SyncedCluster) pgurls( if err != nil { return nil, err } - m[node] = c.NodeURL(host, desc.Port, virtualClusterName, desc.ServiceMode, DefaultAuthMode(), "" /* database */) + m[node] = c.NodeURL(host, desc.Port, virtualClusterName, desc.ServiceMode, DefaultAuthMode(), "" /* database */, false /* disallowUnsafeInternals */) } return m, nil } @@ -2347,7 +2347,7 @@ func (c *SyncedCluster) loadBalancerURL( if err != nil { return "", err } - loadBalancerURL := c.NodeURL(address.IP, address.Port, virtualClusterName, descs[0].ServiceMode, auth, "" /* database */) + loadBalancerURL := c.NodeURL(address.IP, address.Port, virtualClusterName, descs[0].ServiceMode, auth, "" /* database */, false /* disallowUnsafeInternals */) return loadBalancerURL, nil } diff --git a/pkg/roachprod/install/cockroach.go b/pkg/roachprod/install/cockroach.go index 1e828a13baad..ddeaacdc89d8 100644 --- a/pkg/roachprod/install/cockroach.go +++ b/pkg/roachprod/install/cockroach.go @@ -805,6 +805,7 @@ func (c *SyncedCluster) NodeURL( serviceMode ServiceMode, auth PGAuthMode, database string, + disallowUnsafeInternals bool, ) string { var u url.URL u.Scheme = "postgres" @@ -836,7 +837,11 @@ func (c *SyncedCluster) NodeURL( v.Add("sslmode", "disable") } - v.Add("allow_unsafe_internals", "true") + // We usually want to allow unsafe internals for testing environments, + // but allow an escape hatch to disallow it, e.g. if we are using psql. + if !disallowUnsafeInternals { + v.Add("allow_unsafe_internals", "true") + } // We only want to pass an explicit `cluster` name if the user provided one. if virtualClusterName != "" { // We can only pass the cluster parameter for shared processes, as SQL server @@ -894,7 +899,7 @@ func (c *SyncedCluster) ExecOrInteractiveSQL( if err != nil { return err } - url := c.NodeURL("localhost", desc.Port, virtualClusterName, desc.ServiceMode, authMode, database) + url := c.NodeURL("localhost", desc.Port, virtualClusterName, desc.ServiceMode, authMode, database, false /* disallowUnsafeInternals */) binary := cockroachNodeBinary(c, c.Nodes[0]) allArgs := []string{binary, "sql", "--url", url} allArgs = append(allArgs, ssh.Escape(args)) @@ -926,7 +931,7 @@ func (c *SyncedCluster) ExecSQL( cmd = fmt.Sprintf(`cd %s ; `, c.localVMDir(node)) } cmd += SuppressMetamorphicConstantsEnvVar() + " " + cockroachNodeBinary(c, node) + " sql --url " + - c.NodeURL("localhost", desc.Port, virtualClusterName, desc.ServiceMode, authMode, database) + " " + + c.NodeURL("localhost", desc.Port, virtualClusterName, desc.ServiceMode, authMode, database, false /* disallowUnsafeInternals */) + " " + ssh.Escape(args) return c.runCmdOnSingleNode(ctx, l, node, cmd, defaultCmdOpts("run-sql")) }) @@ -1562,7 +1567,7 @@ func (c *SyncedCluster) generateClusterSettingCmd( if err != nil { return "", err } - url := c.NodeURL("localhost", port, SystemInterfaceName /* virtualClusterName */, ServiceModeShared, AuthRootCert, "" /* database */) + url := c.NodeURL("localhost", port, SystemInterfaceName /* virtualClusterName */, ServiceModeShared, AuthRootCert, "" /* database */, false /* disallowUnsafeInternals */) // We use `mkdir -p` here since the directory may not exist if an in-memory // store is used. @@ -1584,7 +1589,7 @@ func (c *SyncedCluster) generateInitCmd(ctx context.Context, node Node) (string, if err != nil { return "", err } - url := c.NodeURL("localhost", port, SystemInterfaceName /* virtualClusterName */, ServiceModeShared, AuthRootCert, "" /* database */) + url := c.NodeURL("localhost", port, SystemInterfaceName /* virtualClusterName */, ServiceModeShared, AuthRootCert, "" /* database */, false /* disallowUnsafeInternals */) binary := cockroachNodeBinary(c, node) initCmd += fmt.Sprintf(` if ! test -e %[1]s ; then @@ -1802,7 +1807,7 @@ func (c *SyncedCluster) createFixedBackupSchedule( serviceMode = ServiceModeExternal } - url := c.NodeURL("localhost", port, startOpts.VirtualClusterName, serviceMode, AuthRootCert, "" /* database */) + url := c.NodeURL("localhost", port, startOpts.VirtualClusterName, serviceMode, AuthRootCert, "" /* database */, false /* disallowUnsafeInternals */) fullCmd := fmt.Sprintf(`%s COCKROACH_CONNECT_TIMEOUT=%d %s sql --url %s -e %q`, SuppressMetamorphicConstantsEnvVar(), startSQLTimeout, binary, url, createScheduleCmd) // Instead of using `c.ExecSQL()`, use `c.runCmdOnSingleNode()`, which allows us to diff --git a/pkg/roachprod/roachprod.go b/pkg/roachprod/roachprod.go index 815b9e1764a9..1320de23965f 100644 --- a/pkg/roachprod/roachprod.go +++ b/pkg/roachprod/roachprod.go @@ -1131,12 +1131,13 @@ func Get(ctx context.Context, l *logger.Logger, clusterName, src, dest string) e } type PGURLOptions struct { - Database string - Secure install.SecureOption - External bool - VirtualClusterName string - SQLInstance int - Auth install.PGAuthMode + Database string + Secure install.SecureOption + External bool + VirtualClusterName string + SQLInstance int + Auth install.PGAuthMode + DisallowUnsafeInternals bool } // PgURL generates pgurls for the nodes in a cluster. @@ -1171,7 +1172,7 @@ func PgURL( if ip == "" { return nil, errors.Errorf("empty ip: %v", ips) } - urls = append(urls, c.NodeURL(ip, desc.Port, opts.VirtualClusterName, desc.ServiceMode, opts.Auth, opts.Database)) + urls = append(urls, c.NodeURL(ip, desc.Port, opts.VirtualClusterName, desc.ServiceMode, opts.Auth, opts.Database, opts.DisallowUnsafeInternals)) } if len(urls) != len(nodes) { return nil, errors.Errorf("have nodes %v, but urls %v from ips %v", nodes, urls, ips) @@ -2951,7 +2952,7 @@ func LoadBalancerPgURL( if err != nil { return "", err } - return c.NodeURL(addr.IP, port, opts.VirtualClusterName, serviceMode, opts.Auth, opts.Database), nil + return c.NodeURL(addr.IP, port, opts.VirtualClusterName, serviceMode, opts.Auth, opts.Database, opts.DisallowUnsafeInternals), nil } // LoadBalancerIP resolves the IP of a load balancer serving the