Merge #159492

159492: roachprod/cli: honor COCKROACH_ROACHPROD_INSECURE for ephemeral clusters r=shailendra-patel a=shailendra-patel

Previously, clusters in `cockroach-ephemeral` always defaulted to insecure mode, ignoring the `COCKROACH_ROACHPROD_INSECURE` env var. The only way to force secure mode was passing `--secure` explicitly.

Now, if `COCKROACH_ROACHPROD_INSECURE` is explicitly set, it takes precedence over the ephemeral project default. Setting it to `false` forces secure mode, `true` forces insecure. CLI flags still have highest precedence.

Release note: None
Epic: None

Co-authored-by: Shailendra Patel <[email protected]>

Author: craig[bot]

pkg/cmd/drtprod/configs/drt_pua_mr.yaml

@@ -17,7 +17,7 @@ environment:
   # variables used by tpcc_run_multiregion.sh
   NUM_REGIONS: 3
   NODES_PER_REGION: 5
-  REGIONS: us-central1,us-east5,us-east1
+  REGIONS: us-central1,us-east4,us-east1
   TPCC_WAREHOUSES: 10000
   TPCC_ACTIVE_WAREHOUSES: 7000
   DB_NAME: cct_tpcc
@@ -55,7 +55,7 @@ targets:
           clouds: gce
           gce-managed: true
           gce-enable-multiple-stores: true
-          gce-zones: "us-central1-a:2,us-central1-b:2,us-central1-c:1,us-east5-a:2,us-east5-b:2,us-east5-c:1,us-east1-b:2,us-east1-c:2,us-east1-d:1"
+          gce-zones: "us-central1-a:2,us-central1-b:2,us-central1-c:1,us-east4-a:2,us-east4-b:2,us-east4-c:1,us-east1-b:2,us-east1-c:2,us-east1-d:1"
           nodes: $CLUSTER_NODES
           gce-machine-type: n2-standard-16
           local-ssd: true
@@ -147,7 +147,7 @@ targets:
           - $WORKLOAD_CLUSTER
         flags:
           clouds: gce
-          gce-zones: "us-central1-a,us-east5-a,us-east1-b"
+          gce-zones: "us-central1-a,us-east4-a,us-east1-b"
           nodes: $NUM_REGIONS
           gce-machine-type: n2d-standard-4
           os-volume-size: 100
@@ -261,7 +261,7 @@ targets:
           - -e
           - |
             BACKUP INTO ('gs://$BUCKET_US_CENTRAL1/$CLUSTER?AUTH=implicit&COCKROACH_LOCALITY=default',
-                            'gs://$BUCKET_US_EAST_5/$CLUSTER?AUTH=implicit&COCKROACH_LOCALITY=region%3Dus-east5',
+                            'gs://$BUCKET_US_EAST_5/$CLUSTER?AUTH=implicit&COCKROACH_LOCALITY=region%3Dus-east4',
                             'gs://$BUCKET_US_EAST_1/$CLUSTER?AUTH=implicit&COCKROACH_LOCALITY=region%3Dus-east1')
             WITH OPTIONS (revision_history = true, detached)
         wait_after: 1500

pkg/cmd/roachprod/cli/flags.go

@@ -28,62 +28,62 @@ import (
 
 var (
 	// Do not populate providerOptsContainer here as we need to call InitProivders() first.
-	providerOptsContainer vm.ProviderOptionsContainer
-	pprofOpts             roachprod.PprofOpts
-	numNodes              int
-	numRacks              int
-	username              string
-	database              string
-	dryrun                bool
-	destroyAllMine        bool
-	destroyAllLocal       bool
-	extendLifetime        time.Duration
-	wipePreserveCerts     bool
-	grafanaConfig         string
-	grafanaArch           string
-	grafanaDumpDir        string
-	jaegerConfigNodes     string
-	listCost              bool
-	listDetails           bool
-	listJSON              bool
-	listMine              bool
-	listPattern           string
-	isSecure              install.ComplexSecureOption // Set based on the values passed to --secure and --insecure
-	secure                = true
-	insecure              = envutil.EnvOrDefaultBool("COCKROACH_ROACHPROD_INSECURE", false)
-	virtualClusterName    string
-	sqlInstance           int
-	extraSSHOptions       = ""
-	exportSSHConfig       string
-	nodeEnv               []string
-	tag                   string
-	external              = false
-	pgurlCertsDir         string
-	authMode              string
-	adminurlPath          = ""
-	adminurlIPs           = false
-	urlOpen               = false
-	useTreeDist           = true
-	sig                   = 9
-	waitFlag              = false
-	gracePeriod           = 0
-	deploySig             = 15
-	deployWaitFlag        = true
-	deployGracePeriod     = 300
-	pause                 = time.Duration(0)
-	createVMOpts          = vm.DefaultCreateOpts()
-	startOpts             = roachprod.DefaultStartOpts()
-	stageOS               string
-	stageArch             string
-	stageDir              string
-	logsDir               string
-	logsFilter            string
-	logsProgramFilter     string
-	logsFrom              time.Time
-	logsTo                time.Time
-	logsInterval          time.Duration
-	volumeCreateOpts      vm.VolumeCreateOpts
-	listOpts              vm.ListOptions
+	providerOptsContainer    vm.ProviderOptionsContainer
+	pprofOpts                roachprod.PprofOpts
+	numNodes                 int
+	numRacks                 int
+	username                 string
+	database                 string
+	dryrun                   bool
+	destroyAllMine           bool
+	destroyAllLocal          bool
+	extendLifetime           time.Duration
+	wipePreserveCerts        bool
+	grafanaConfig            string
+	grafanaArch              string
+	grafanaDumpDir           string
+	jaegerConfigNodes        string
+	listCost                 bool
+	listDetails              bool
+	listJSON                 bool
+	listMine                 bool
+	listPattern              string
+	isSecure                 install.ComplexSecureOption // Set based on the values passed to --secure and --insecure
+	secure                   = true
+	insecure, insecureEnvSet = getInsecureEnvVar() // Get both value and whether it was explicitly set
+	virtualClusterName       string
+	sqlInstance              int
+	extraSSHOptions          = ""
+	exportSSHConfig          string
+	nodeEnv                  []string
+	tag                      string
+	external                 = false
+	pgurlCertsDir            string
+	authMode                 string
+	adminurlPath             = ""
+	adminurlIPs              = false
+	urlOpen                  = false
+	useTreeDist              = true
+	sig                      = 9
+	waitFlag                 = false
+	gracePeriod              = 0
+	deploySig                = 15
+	deployWaitFlag           = true
+	deployGracePeriod        = 300
+	pause                    = time.Duration(0)
+	createVMOpts             = vm.DefaultCreateOpts()
+	startOpts                = roachprod.DefaultStartOpts()
+	stageOS                  string
+	stageArch                string
+	stageDir                 string
+	logsDir                  string
+	logsFilter               string
+	logsProgramFilter        string
+	logsFrom                 time.Time
+	logsTo                   time.Time
+	logsInterval             time.Duration
+	volumeCreateOpts         vm.VolumeCreateOpts
+	listOpts                 vm.ListOptions
 
 	monitorOpts        install.MonitorOpts
 	cachedHostsCluster string
@@ -109,11 +109,24 @@ var (
 	fetchLogsTimeout time.Duration
 )
 
+// getInsecureEnvVar returns the value of COCKROACH_ROACHPROD_INSECURE and
+// whether it was explicitly set.
+func getInsecureEnvVar() (bool, bool) {
+	val, ok := envutil.EnvString("COCKROACH_ROACHPROD_INSECURE", 1)
+	if !ok {
+		return false, false
+	}
+	return val == "true" || val == "1", true
+}
+
 // Intended to be called once from drtprod main package to update defaults which differ from roachprod.
 func UpdateFlagDefaults() {
-	// N.B. unlike roachprod, which defaults to "insecure mode", drtprod defaults to "secure mode".
+	// N.B. Both roachprod and drtprod default to secure mode via the flag defaults.
+	// However, roachprod has runtime logic in overrideBasedOnClusterSettings() that
+	// forces insecure mode for clusters in the cockroach-ephemeral GCP project.
+	// drtprod explicitly sets secure=true here to ensure secure mode is used.
 	secure = true
-	insecure = envutil.EnvOrDefaultBool("COCKROACH_ROACHPROD_INSECURE", false)
+	// insecure and insecureEnvSet are already initialized via getInsecureEnvVar()
 }
 
 func initRootCmdFlags(rootCmd *cobra.Command) {

pkg/cmd/roachprod/cli/util.go

@@ -143,6 +143,14 @@ func isSecureCluster(cmd *cobra.Command) (install.ComplexSecureOption, error) {
 	case hasInsecureFlag:
 		return install.ComplexSecureOption{ForcedInsecure: true}, nil
 
+	case insecureEnvSet:
+		// If COCKROACH_ROACHPROD_INSECURE env var was explicitly set, treat it
+		// as a forced setting that takes precedence over ephemeral project defaults.
+		if insecure {
+			return install.ComplexSecureOption{ForcedInsecure: true}, nil
+		}
+		return install.ComplexSecureOption{ForcedSecure: true}, nil
+
 	default:
 		return install.ComplexSecureOption{DefaultSecure: !insecure}, nil
 	}