fix(cli): add backwards compat for old telemetry env and tests

mafredri · mafredri · commit d0a2b4bb962a · 2024-11-11T16:47:16.000+02:00
diff --git a/cli/server_test.go b/cli/server_test.go
@@ -1620,6 +1620,40 @@ func TestServer_Production(t *testing.T) {
 	require.NoError(t, err)
 }
 
+//nolin:tparallel,paralleltest // This test sets environment variables.
+func TestServer_TelemetryDisable(t *testing.T) {
+	// Set the default telemetry to true (normally disabled in tests).
+	t.Setenv("CODER_TEST_TELEMETRY_DEFAULT_ENABLE", "true")
+
+	for _, tt := range []struct {
+		key  string
+		val  string
+		want bool
+	}{
+		{"", "", true},
+		{"CODER_TELEMETRY_ENABLE", "true", true},
+		{"CODER_TELEMETRY_ENABLE", "false", false},
+		{"CODER_TELEMETRY", "true", true},
+		{"CODER_TELEMETRY", "false", false},
+	} {
+		t.Run(fmt.Sprintf("%s=%s", tt.key, tt.val), func(t *testing.T) {
+			t.Parallel()
+			var b bytes.Buffer
+			inv, _ := clitest.New(t, "server", "--write-config")
+			inv.Stdout = &b
+			inv.Environ.Set(tt.key, tt.val)
+			clitest.Run(t, inv)
+
+			t.Logf("config:\n%s", b.String())
+
+			var dv codersdk.DeploymentValues
+			err := yaml.Unmarshal(b.Bytes(), &dv)
+			require.NoError(t, err)
+			assert.Equal(t, tt.want, dv.Telemetry.Enable.Value())
+		})
+	}
+}
+
 //nolint:tparallel,paralleltest // This test cannot be run in parallel due to signal handling.
 func TestServer_InterruptShutdown(t *testing.T) {
 	t.Skip("This test issues an interrupt signal which will propagate to the test runner.")
diff --git a/codersdk/deployment.go b/codersdk/deployment.go
@@ -1143,6 +1143,16 @@ when required by your organization's security policy.`,
 		Group:       &deploymentGroupEmailTLS,
 		YAML:        "certKeyFile",
 	}
+	telemetryEnable := serpent.Option{
+		Name:        "Telemetry Enable",
+		Description: "Whether telemetry is enabled or not. Coder collects anonymized usage data to help improve our product.",
+		Flag:        "telemetry",
+		Env:         "CODER_TELEMETRY_ENABLE",
+		Default:     strconv.FormatBool(flag.Lookup("test.v") == nil || os.Getenv("CODER_TEST_TELEMETRY_DEFAULT_ENABLE") == "true"),
+		Value:       &c.Telemetry.Enable,
+		Group:       &deploymentGroupTelemetry,
+		YAML:        "enable",
+	}
 	opts := serpent.OptionSet{
 		{
 			Name:        "Access URL",
@@ -1903,15 +1913,19 @@ when required by your organization's security policy.`,
 			YAML:  "dangerousSkipIssuerChecks",
 		},
 		// Telemetry settings
+		telemetryEnable,
 		{
-			Name:        "Telemetry Enable",
-			Description: "Whether telemetry is enabled or not. Coder collects anonymized usage data to help improve our product.",
-			Flag:        "telemetry",
-			Env:         "CODER_TELEMETRY_ENABLE",
-			Default:     strconv.FormatBool(flag.Lookup("test.v") == nil),
-			Value:       &c.Telemetry.Enable,
-			Group:       &deploymentGroupTelemetry,
-			YAML:        "enable",
+			Hidden: true,
+			Name:   "Telemetry (backwards compatibility)",
+			// Note the flip-flop of flag and env to maintain backwards
+			// compatibility and consistency. Inconsistently, the env
+			// was renamed to CODER_TELEMETRY_ENABLE in the past, but
+			// the flag was not renamed -enable.
+			Flag:       "telemetry-enable",
+			Env:        "CODER_TELEMETRY",
+			Value:      &c.Telemetry.Enable,
+			Group:      &deploymentGroupTelemetry,
+			UseInstead: []serpent.Option{telemetryEnable},
 		},
 		{
 			Name:        "Telemetry URL",
