Merge #151230

151230: roachprod: default to secure clusters unless GCE cockroach-ephemeral r=herkolategan,srosenberg a=golgeek

Since #147737, to ease testing for engineering teams, roachprod defaults to insecure clusters. This change has led to security impact for other entities at CRL.

This patch brings smart selection of secure vs. insecure cluster depending on the Cloud project the cluster is provisionned in. If the cluster is provisionned in the engineering's ephemeral GCP project, it will default to insecure, while all other configurations will default to secure.

In order to achieve that, and because this depends on the cluster's configuration that is not available at the CLI package level, the CLI now keeps track of the user's arguments (--secure, --insecure, default) and passes the information to the roachprod library.

Epic: none
Release note: None

Co-authored-by: Ludovic Leroux <[email protected]>

Author: craig[bot]

pkg/cmd/drt-run/BUILD.bazel

@@ -18,6 +18,7 @@ go_library(
         "//pkg/cmd/roachtest/registry",
         "//pkg/cmd/roachtest/spec",
         "//pkg/roachprod",
+        "//pkg/roachprod/install",
         "//pkg/roachprod/logger",
         "//pkg/util/randutil",
         "//pkg/util/syncutil",

pkg/cmd/drt-run/workloads.go

@@ -14,6 +14,7 @@ import (
 	"sync"
 
 	"github.com/cockroachdb/cockroach/pkg/roachprod"
+	"github.com/cockroachdb/cockroach/pkg/roachprod/install"
 	"github.com/cockroachdb/cockroach/pkg/roachprod/logger"
 	"github.com/cockroachdb/errors"
 	"github.com/cockroachdb/logtags"
@@ -45,7 +46,7 @@ func (w *workloadRunner) runWorkloadStep(
 		return
 	}
 	pgUrl, err := roachprod.PgURL(ctx, l, w.config.ClusterName, w.config.CertsDir, roachprod.PGURLOptions{
-		Secure:   w.config.CertsDir != "",
+		Secure:   install.SimpleSecureOption(w.config.CertsDir != ""),
 		External: true,
 	})
 	if err != nil {

pkg/cmd/drtprod/cli/commands/yamlprocessor.go

@@ -283,7 +283,7 @@ func setupAndExecute(
 ) error {
 	logger := config.Logger
 	// Move the drtprod binary to /usr/bin to ensure it is available system-wide on the cluster.
-	err := roachprodRun(ctx, logger, monitorClusterName, "", "", true,
+	err := roachprodRun(ctx, logger, monitorClusterName, "", "", install.SimpleSecureOption(true),
 		os.Stdout, os.Stderr,
 		[]string{fmt.Sprintf("sudo cp %s /usr/bin", drtprodLocation)},
 		install.RunOptions{FailOption: install.FailSlow})
@@ -293,7 +293,7 @@ func setupAndExecute(
 
 	// Enable linger for the default user, so that the cloud subprocess is not
 	// killed when the user logs out.
-	err = roachprodRun(ctx, logger, monitorClusterName, "", "", true,
+	err = roachprodRun(ctx, logger, monitorClusterName, "", "", install.SimpleSecureOption(true),
 		os.Stdout, os.Stderr,
 		[]string{fmt.Sprintf("sudo loginctl enable-linger %s", config.SharedUser)},
 		install.RunOptions{FailOption: install.FailSlow})
@@ -318,7 +318,7 @@ func setupAndExecute(
 	}
 
 	// Run the systemd command on the remote cluster.
-	return roachprodRun(ctx, logger, monitorClusterName, "", "", true,
+	return roachprodRun(ctx, logger, monitorClusterName, "", "", install.SimpleSecureOption(true),
 		os.Stdout, os.Stderr,
 		[]string{executeArgs},
 		install.RunOptions{FailOption: install.FailSlow})
@@ -340,7 +340,7 @@ func uploadAllDependentFiles(
 			if strings.Contains(fl, "/") {
 				dirLocation := filepath.Dir(fl)
 				// Use roachprod to create the directory on the remote.
-				err := roachprodRun(ctx, logger, monitorClusterName, "", "", true,
+				err := roachprodRun(ctx, logger, monitorClusterName, "", "", install.SimpleSecureOption(true),
 					os.Stdout, os.Stderr,
 					[]string{fmt.Sprintf("mkdir -p %s", dirLocation)},
 					install.RunOptions{FailOption: install.FailSlow})

pkg/cmd/drtprod/cli/commands/yamlprocessor_test.go

@@ -254,7 +254,7 @@ environment:
 			return nil
 		}
 		roachprodRun = func(ctx context.Context, l *logger.Logger, clusterName,
-			SSHOptions, processTag string, secure bool, stdout, stderr io.Writer,
+			SSHOptions, processTag string, secure install.SecureOption, stdout, stderr io.Writer,
 			cmdArray []string, options install.RunOptions) error {
 			require.Equal(t, "test-monitor", clusterName)
 			if strings.HasPrefix(cmdArray[0], "mkdir -p") {
@@ -284,7 +284,7 @@ environment:
 			return nil
 		}
 		roachprodRun = func(ctx context.Context, l *logger.Logger, clusterName,
-			SSHOptions, processTag string, secure bool, stdout, stderr io.Writer,
+			SSHOptions, processTag string, secure install.SecureOption, stdout, stderr io.Writer,
 			cmdArray []string, options install.RunOptions) error {
 			require.Equal(t, "test-monitor", clusterName)
 			return nil
@@ -311,7 +311,7 @@ environment:
 			return nil
 		}
 		roachprodRun = func(ctx context.Context, l *logger.Logger, clusterName,
-			SSHOptions, processTag string, secure bool, stdout, stderr io.Writer,
+			SSHOptions, processTag string, secure install.SecureOption, stdout, stderr io.Writer,
 			cmdArray []string, options install.RunOptions) error {
 			if strings.HasPrefix(cmdArray[0], "sudo cp") {
 				return fmt.Errorf("move command failed")
@@ -342,7 +342,7 @@ environment:
 			return nil
 		}
 		roachprodRun = func(ctx context.Context, l *logger.Logger, clusterName,
-			SSHOptions, processTag string, secure bool, stdout, stderr io.Writer,
+			SSHOptions, processTag string, secure install.SecureOption, stdout, stderr io.Writer,
 			cmdArray []string, options install.RunOptions) error {
 			runCmdsLock.Lock()
 			defer runCmdsLock.Unlock()
@@ -375,7 +375,7 @@ environment:
 			return nil
 		}
 		roachprodRun = func(ctx context.Context, l *logger.Logger, clusterName,
-			SSHOptions, processTag string, secure bool, stdout, stderr io.Writer,
+			SSHOptions, processTag string, secure install.SecureOption, stdout, stderr io.Writer,
 			cmdArray []string, options install.RunOptions) error {
 			require.Equal(t, "test-monitor", clusterName)
 			if strings.HasPrefix(cmdArray[0], "sudo systemd-run") {
@@ -409,7 +409,7 @@ environment:
 			return nil
 		}
 		roachprodRun = func(ctx context.Context, l *logger.Logger, clusterName,
-			SSHOptions, processTag string, secure bool, stdout, stderr io.Writer,
+			SSHOptions, processTag string, secure install.SecureOption, stdout, stderr io.Writer,
 			cmdArray []string, options install.RunOptions) error {
 			require.Equal(t, "test-monitor", clusterName)
 			runCmdsLock.Lock()
@@ -483,7 +483,7 @@ environment:
 			return nil
 		}
 		roachprodRun = func(ctx context.Context, l *logger.Logger, clusterName,
-			SSHOptions, processTag string, secure bool, stdout, stderr io.Writer,
+			SSHOptions, processTag string, secure install.SecureOption, stdout, stderr io.Writer,
 			cmdArray []string, options install.RunOptions) error {
 			require.Equal(t, "test-monitor", clusterName)
 			runCmdsLock.Lock()
@@ -553,7 +553,7 @@ environment:
 			return nil
 		}
 		roachprodRun = func(ctx context.Context, l *logger.Logger, clusterName,
-			SSHOptions, processTag string, secure bool, stdout, stderr io.Writer,
+			SSHOptions, processTag string, secure install.SecureOption, stdout, stderr io.Writer,
 			cmdArray []string, options install.RunOptions) error {
 			require.Equal(t, "test-monitor", clusterName)
 			runCmdsLock.Lock()

pkg/cmd/roachprod-microbench/cluster/execute.go

@@ -57,7 +57,7 @@ type RemoteExecutionFunc func(
 	ctx context.Context,
 	l *logger.Logger,
 	clusterName, SSHOptions, processTag string,
-	secure bool,
+	secure install.SecureOption,
 	cmdArray []string,
 	options install.RunOptions,
 ) ([]install.RunResultDetails, error)
@@ -156,7 +156,7 @@ func remoteWorker(
 
 			runResult, err := execFunc(
 				cmdCtx, log, clusterNode, "" /* SSHOptions */, "", /* processTag */
-				false /* secure */, command.Args, runOptions,
+				install.SimpleSecureOption(false), command.Args, runOptions,
 			)
 			duration := timeutil.Since(start)
 

pkg/cmd/roachprod-microbench/cluster/executor_test.go

@@ -69,7 +69,7 @@ func TestExecutionScheduling(t *testing.T) {
 				ctx context.Context,
 				l *logger.Logger,
 				clusterName, SSHOptions, processTag string,
-				secure bool,
+				secure install.SecureOption,
 				cmdArray []string,
 				options install.RunOptions,
 			) ([]install.RunResultDetails, error) {
@@ -168,7 +168,7 @@ func TestCancelGroupsBeforeExecution(t *testing.T) {
 		ctx context.Context,
 		l *logger.Logger,
 		clusterName, SSHOptions, processTag string,
-		secure bool,
+		secure install.SecureOption,
 		cmdArray []string,
 		options install.RunOptions,
 	) ([]install.RunResultDetails, error) {
@@ -226,7 +226,7 @@ func TestCancelGroupsInFlight(t *testing.T) {
 		ctx context.Context,
 		l *logger.Logger,
 		clusterName, SSHOptions, processTag string,
-		secure bool,
+		secure install.SecureOption,
 		cmdArray []string,
 		options install.RunOptions,
 	) ([]install.RunResultDetails, error) {

pkg/cmd/roachprod-microbench/executor.go

@@ -252,7 +252,7 @@ func (e *executor) listBenchmarks(
 // cluster by inspecting the given remote directory.
 func (e *executor) listRemotePackages(log *logger.Logger, dir string) ([]string, error) {
 	ctx := context.Background()
-	results, err := roachprod.RunWithDetails(ctx, log, e.cluster, "", "", false,
+	results, err := roachprod.RunWithDetails(ctx, log, e.cluster, "", "", install.SimpleSecureOption(false),
 		[]string{"find", dir, "-name", "run.sh"}, install.WithNodes(install.Nodes{1}))
 	if err != nil {
 		return nil, err

pkg/cmd/roachprod-microbench/roachprod_util.go

@@ -26,7 +26,7 @@ func InitRoachprod() {
 func RoachprodRun(clusterName string, l *logger.Logger, cmdArray []string) error {
 	// Execute the roachprod command with the provided context, logger, cluster name, and options.
 	return roachprod.Run(
-		context.Background(), l, clusterName, "", "", false,
+		context.Background(), l, clusterName, "", "", install.SimpleSecureOption(false),
 		os.Stdout, os.Stderr, cmdArray, install.DefaultRunOptions(),
 	)
 }

pkg/cmd/roachprod-stress/main.go

@@ -135,7 +135,7 @@ func roundToSeconds(d time.Duration) time.Duration {
 
 func roachprodRun(clusterName string, cmdArray []string) error {
 	return roachprod.Run(
-		context.Background(), l, clusterName, "", "", false,
+		context.Background(), l, clusterName, "", "", install.SimpleSecureOption(false),
 		os.Stdout, os.Stderr, cmdArray, install.DefaultRunOptions(),
 	)
 }

pkg/cmd/roachprod/cli/commands.go

@@ -736,7 +736,7 @@ cluster setting will be set to its value.
 			clusterSettingsOpts := []install.ClusterSettingOption{
 				install.TagOption(tag),
 				install.PGUrlCertsDirOption(pgurlCertsDir),
-				install.SecureOption(isSecure),
+				isSecure,
 				install.UseTreeDistOption(useTreeDist),
 				install.EnvOption(nodeEnv),
 				install.NumRacksOption(numRacks),
@@ -773,7 +773,7 @@ Note that if the cluster is started in insecure mode, set the insecure mode here
 		Args: cobra.ExactArgs(1),
 		Run: Wrap(func(cmd *cobra.Command, args []string) error {
 			clusterSettingsOpts := []install.ClusterSettingOption{
-				install.SecureOption(isSecure),
+				isSecure,
 			}
 			return roachprod.UpdateTargets(context.Background(), config.Logger, args[0], clusterSettingsOpts...)
 		}),
@@ -857,7 +857,7 @@ environment variables to the cockroach process.
 			clusterSettingsOpts := []install.ClusterSettingOption{
 				install.TagOption(tag),
 				install.PGUrlCertsDirOption(pgurlCertsDir),
-				install.SecureOption(isSecure),
+				isSecure,
 				install.UseTreeDistOption(useTreeDist),
 				install.EnvOption(nodeEnv),
 				install.NumRacksOption(numRacks),
@@ -1723,7 +1723,7 @@ roachprod grafana-annotation grafana.testeng.crdb.io example-annotation-event --
 				return errors.Newf("Too many arguments for --time-range, expected 1 or 2, got: %d", len(grafanaTimeRange))
 			}
 
-			return roachprod.AddGrafanaAnnotation(context.Background(), args[0] /* host */, isSecure, req)
+			return roachprod.AddGrafanaAnnotation(context.Background(), args[0] /* host */, isSecure.DefaultSecure, req)
 		}),
 	}
 	initGrafanaAnnotationCmdFlags(grafanaAnnotationCmd)