fix(clibase): allow empty values to unset defaults

ammario · ammario · commit 8e86eab6343c · 2023-03-28T01:30:19.000Z
diff --git a/cli/clibase/option.go b/cli/clibase/option.go
@@ -121,12 +121,7 @@ func (s *OptionSet) ParseEnv(vs []EnvVar) error {
 		}
 
 		envVal, ok := envs[opt.Env]
-		// Currently, empty values are treated as if the environment variable is
-		// unset. This behavior is technically not correct as there is now no
-		// way for a user to change a Default value to an empty string from
-		// the environment. Unfortunately, we have old configuration files
-		// that rely on the faulty behavior.
-		if !ok || envVal == "" {
+		if !ok {
 			continue
 		}
 
diff --git a/cli/clibase/option_test.go b/cli/clibase/option_test.go
@@ -97,7 +97,7 @@ func TestOptionSet_ParseEnv(t *testing.T) {
 		require.EqualValues(t, "foo", workspaceName)
 	})
 
-	t.Run("EmptyValue", func(t *testing.T) {
+	t.Run("EmptyValueUnsets", func(t *testing.T) {
 		t.Parallel()
 
 		var workspaceName clibase.String
@@ -116,6 +116,6 @@ func TestOptionSet_ParseEnv(t *testing.T) {
 
 		err = os.ParseEnv(clibase.ParseEnviron([]string{"CODER_WORKSPACE_NAME="}, "CODER_"))
 		require.NoError(t, err)
-		require.EqualValues(t, "defname", workspaceName)
+		require.EqualValues(t, "", workspaceName)
 	})
 }
diff --git a/coder.env b/coder.env
@@ -1,11 +1,11 @@
 # Coder must be reachable from an external URL for users and workspaces to connect.
 # e.g. https://coder.example.com
-CODER_ACCESS_URL=
+# CODER_ACCESS_URL=
 
-CODER_ADDRESS=
-CODER_PG_CONNECTION_URL=
-CODER_TLS_CERT_FILE=
-CODER_TLS_ENABLE=
-CODER_TLS_KEY_FILE=
+# CODER_ADDRESS=
+# CODER_PG_CONNECTION_URL=
+# CODER_TLS_CERT_FILE=
+# CODER_TLS_ENABLE=
+# CODER_TLS_KEY_FILE=
 
 # Run "coder server --help" for flag information.

Original file line number	Diff line number	Diff line change
`@@ -121,12 +121,7 @@ func (s *OptionSet) ParseEnv(vs []EnvVar) error {`
`121`	`121`	`}`
`122`	`122`
`123`	`123`	`envVal, ok := envs[opt.Env]`
`124`		`- // Currently, empty values are treated as if the environment variable is`
`125`		`- // unset. This behavior is technically not correct as there is now no`
`126`		`- // way for a user to change a Default value to an empty string from`
`127`		`- // the environment. Unfortunately, we have old configuration files`
`128`		`- // that rely on the faulty behavior.`
`129`		`- if !ok \|\| envVal == "" {`
	`124`	`+ if !ok {`
`130`	`125`	`continue`
`131`	`126`	`}`
`132`	`127`