Skip to content

Commit 8913ec0

Browse files
DarrylWongDarrylWong
committed
roachprod: allow opting out of allow_unsafe_internals
We sometimes need to construct pgurls for psql or postgres dbs which don't support this CRDB only parameter.
1 parent c0b0acf commit 8913ec0

File tree

7 files changed

+31
-23
lines changed

7 files changed

+31
-23
lines changed

‎pkg/cmd/roachtest/cluster.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2721,9 +2721,6 @@ func (c *clusterImpl) InternalPGUrl(
27212721
return c.pgURLErr(ctx, l, nodes, opts)
27222722
}
27232723

2724-
// Silence unused warning.
2725-
var _ = (&clusterImpl{}).InternalPGUrl
2726-
27272724
// ExternalPGUrl returns the external Postgres endpoint for the specified nodes.
27282725
func (c *clusterImpl) ExternalPGUrl(
27292726
ctx context.Context, l *logger.Logger, nodes option.NodeListOption, opts roachprod.PGURLOptions,

‎pkg/cmd/roachtest/tests/copyfrom.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,11 @@ func runCopyFromCRDB(ctx context.Context, t test.Test, c cluster.Cluster, sf int
168168
t.Fatal(err)
169169
}
170170
}
171-
urls, err := c.InternalPGUrl(ctx, t.L(), c.Node(1), roachprod.PGURLOptions{Auth: install.AuthUserPassword})
171+
// psql doesn't support the CRDB specific "allow_unsafe_internals" parameter.
172+
urls, err := c.InternalPGUrl(ctx, t.L(), c.Node(1), roachprod.PGURLOptions{
173+
Auth: install.AuthUserPassword,
174+
DisallowUnsafeInternals: true,
175+
})
172176
require.NoError(t, err)
173177
m := c.NewDeprecatedMonitor(ctx, c.All())
174178
m.Go(func(ctx context.Context) error {

‎pkg/cmd/roachtest/tests/pg_regress.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ func initPGRegress(ctx context.Context, t test.Test, c cluster.Cluster) {
110110
}
111111
}
112112

113-
urls, err := c.InternalPGUrl(ctx, t.L(), node, roachprod.PGURLOptions{})
113+
urls, err := c.InternalPGUrl(ctx, t.L(), node, roachprod.PGURLOptions{DisallowUnsafeInternals: true})
114114
if err != nil {
115115
t.Fatal(err)
116116
}
@@ -149,7 +149,8 @@ func runPGRegress(ctx context.Context, t test.Test, c cluster.Cluster) {
149149
initPGRegress(ctx, t, c)
150150

151151
node := c.Node(1)
152-
urls, err := c.InternalPGUrl(ctx, t.L(), node, roachprod.PGURLOptions{})
152+
// psql doesn't support the CRDB specific "allow_unsafe_internals" parameter.
153+
urls, err := c.InternalPGUrl(ctx, t.L(), node, roachprod.PGURLOptions{DisallowUnsafeInternals: true})
153154
if err != nil {
154155
t.Fatal(err)
155156
}

‎pkg/roachprod/failureinjection/failures/failure.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ func (f *GenericFailure) Conn(
160160
if !c.Secure {
161161
authMode = install.AuthRootCert
162162
}
163-
nodeURL := c.NodeURL(ip, desc.Port, "" /* virtualClusterName */, desc.ServiceMode, authMode, "" /* database */)
163+
nodeURL := c.NodeURL(ip, desc.Port, "" /* virtualClusterName */, desc.ServiceMode, authMode, "" /* database */, false /* disallowUnsafeInternals */)
164164
nodeURL = strings.Trim(nodeURL, "'")
165165
pgurl, err := url.Parse(nodeURL)
166166
if err != nil {

‎pkg/roachprod/install/cluster_synced.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2304,7 +2304,7 @@ func (c *SyncedCluster) pgurls(
23042304
if err != nil {
23052305
return nil, err
23062306
}
2307-
m[node] = c.NodeURL(host, desc.Port, virtualClusterName, desc.ServiceMode, DefaultAuthMode(), "" /* database */)
2307+
m[node] = c.NodeURL(host, desc.Port, virtualClusterName, desc.ServiceMode, DefaultAuthMode(), "" /* database */, false /* disallowUnsafeInternals */)
23082308
}
23092309
return m, nil
23102310
}
@@ -2347,7 +2347,7 @@ func (c *SyncedCluster) loadBalancerURL(
23472347
if err != nil {
23482348
return "", err
23492349
}
2350-
loadBalancerURL := c.NodeURL(address.IP, address.Port, virtualClusterName, descs[0].ServiceMode, auth, "" /* database */)
2350+
loadBalancerURL := c.NodeURL(address.IP, address.Port, virtualClusterName, descs[0].ServiceMode, auth, "" /* database */, false /* disallowUnsafeInternals */)
23512351
return loadBalancerURL, nil
23522352
}
23532353

‎pkg/roachprod/install/cockroach.go‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -805,6 +805,7 @@ func (c *SyncedCluster) NodeURL(
805805
serviceMode ServiceMode,
806806
auth PGAuthMode,
807807
database string,
808+
disallowUnsafeInternals bool,
808809
) string {
809810
var u url.URL
810811
u.Scheme = "postgres"
@@ -836,7 +837,11 @@ func (c *SyncedCluster) NodeURL(
836837
v.Add("sslmode", "disable")
837838
}
838839

839-
v.Add("allow_unsafe_internals", "true")
840+
// We usually want to allow unsafe internals for testing environments,
841+
// but allow an escape hatch to disallow it, e.g. if we are using psql.
842+
if !disallowUnsafeInternals {
843+
v.Add("allow_unsafe_internals", "true")
844+
}
840845
// We only want to pass an explicit `cluster` name if the user provided one.
841846
if virtualClusterName != "" {
842847
// We can only pass the cluster parameter for shared processes, as SQL server
@@ -894,7 +899,7 @@ func (c *SyncedCluster) ExecOrInteractiveSQL(
894899
if err != nil {
895900
return err
896901
}
897-
url := c.NodeURL("localhost", desc.Port, virtualClusterName, desc.ServiceMode, authMode, database)
902+
url := c.NodeURL("localhost", desc.Port, virtualClusterName, desc.ServiceMode, authMode, database, false /* disallowUnsafeInternals */)
898903
binary := cockroachNodeBinary(c, c.Nodes[0])
899904
allArgs := []string{binary, "sql", "--url", url}
900905
allArgs = append(allArgs, ssh.Escape(args))
@@ -926,7 +931,7 @@ func (c *SyncedCluster) ExecSQL(
926931
cmd = fmt.Sprintf(`cd %s ; `, c.localVMDir(node))
927932
}
928933
cmd += SuppressMetamorphicConstantsEnvVar() + " " + cockroachNodeBinary(c, node) + " sql --url " +
929-
c.NodeURL("localhost", desc.Port, virtualClusterName, desc.ServiceMode, authMode, database) + " " +
934+
c.NodeURL("localhost", desc.Port, virtualClusterName, desc.ServiceMode, authMode, database, false /* disallowUnsafeInternals */) + " " +
930935
ssh.Escape(args)
931936
return c.runCmdOnSingleNode(ctx, l, node, cmd, defaultCmdOpts("run-sql"))
932937
})
@@ -1562,7 +1567,7 @@ func (c *SyncedCluster) generateClusterSettingCmd(
15621567
if err != nil {
15631568
return "", err
15641569
}
1565-
url := c.NodeURL("localhost", port, SystemInterfaceName /* virtualClusterName */, ServiceModeShared, AuthRootCert, "" /* database */)
1570+
url := c.NodeURL("localhost", port, SystemInterfaceName /* virtualClusterName */, ServiceModeShared, AuthRootCert, "" /* database */, false /* disallowUnsafeInternals */)
15661571

15671572
// We use `mkdir -p` here since the directory may not exist if an in-memory
15681573
// store is used.
@@ -1584,7 +1589,7 @@ func (c *SyncedCluster) generateInitCmd(ctx context.Context, node Node) (string,
15841589
if err != nil {
15851590
return "", err
15861591
}
1587-
url := c.NodeURL("localhost", port, SystemInterfaceName /* virtualClusterName */, ServiceModeShared, AuthRootCert, "" /* database */)
1592+
url := c.NodeURL("localhost", port, SystemInterfaceName /* virtualClusterName */, ServiceModeShared, AuthRootCert, "" /* database */, false /* disallowUnsafeInternals */)
15881593
binary := cockroachNodeBinary(c, node)
15891594
initCmd += fmt.Sprintf(`
15901595
if ! test -e %[1]s ; then
@@ -1802,7 +1807,7 @@ func (c *SyncedCluster) createFixedBackupSchedule(
18021807
serviceMode = ServiceModeExternal
18031808
}
18041809

1805-
url := c.NodeURL("localhost", port, startOpts.VirtualClusterName, serviceMode, AuthRootCert, "" /* database */)
1810+
url := c.NodeURL("localhost", port, startOpts.VirtualClusterName, serviceMode, AuthRootCert, "" /* database */, false /* disallowUnsafeInternals */)
18061811
fullCmd := fmt.Sprintf(`%s COCKROACH_CONNECT_TIMEOUT=%d %s sql --url %s -e %q`,
18071812
SuppressMetamorphicConstantsEnvVar(), startSQLTimeout, binary, url, createScheduleCmd)
18081813
// Instead of using `c.ExecSQL()`, use `c.runCmdOnSingleNode()`, which allows us to

‎pkg/roachprod/roachprod.go‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,12 +1131,13 @@ func Get(ctx context.Context, l *logger.Logger, clusterName, src, dest string) e
11311131
}
11321132

11331133
type PGURLOptions struct {
1134-
Database string
1135-
Secure install.SecureOption
1136-
External bool
1137-
VirtualClusterName string
1138-
SQLInstance int
1139-
Auth install.PGAuthMode
1134+
Database string
1135+
Secure install.SecureOption
1136+
External bool
1137+
VirtualClusterName string
1138+
SQLInstance int
1139+
Auth install.PGAuthMode
1140+
DisallowUnsafeInternals bool
11401141
}
11411142

11421143
// PgURL generates pgurls for the nodes in a cluster.
@@ -1171,7 +1172,7 @@ func PgURL(
11711172
if ip == "" {
11721173
return nil, errors.Errorf("empty ip: %v", ips)
11731174
}
1174-
urls = append(urls, c.NodeURL(ip, desc.Port, opts.VirtualClusterName, desc.ServiceMode, opts.Auth, opts.Database))
1175+
urls = append(urls, c.NodeURL(ip, desc.Port, opts.VirtualClusterName, desc.ServiceMode, opts.Auth, opts.Database, opts.DisallowUnsafeInternals))
11751176
}
11761177
if len(urls) != len(nodes) {
11771178
return nil, errors.Errorf("have nodes %v, but urls %v from ips %v", nodes, urls, ips)
@@ -2951,7 +2952,7 @@ func LoadBalancerPgURL(
29512952
if err != nil {
29522953
return "", err
29532954
}
2954-
return c.NodeURL(addr.IP, port, opts.VirtualClusterName, serviceMode, opts.Auth, opts.Database), nil
2955+
return c.NodeURL(addr.IP, port, opts.VirtualClusterName, serviceMode, opts.Auth, opts.Database, opts.DisallowUnsafeInternals), nil
29552956
}
29562957

29572958
// LoadBalancerIP resolves the IP of a load balancer serving the

0 commit comments

Comments
 (0)