Fix config dir bug

coder · ammario · Mar 7, 2023 · Feb 25, 2023 · Feb 25, 2023 · Feb 25, 2023
commit 05b3a10adf5956186a621554744a9c8b6895c4f4
diff --git a/cli/bigcli/option.go b/cli/bigcli/option.go
@@ -169,7 +169,7 @@ func (os *OptionSet) ParseEnv(globalPrefix string, environ []string) error {
 }
 
 // SetDefaults sets the default values for each Option.
-// It should be called after all parsing (e.g. ParseFlags, ParseEnv).
+// It should be called before all parsing (e.g. ParseFlags, ParseEnv).
 func (os *OptionSet) SetDefaults() error {
 	var merr *multierror.Error
 	for _, opt := range *os {

diff --git a/cli/bigcli/values.go b/cli/bigcli/values.go
@@ -149,9 +149,26 @@ func (b *BindAddress) Set(v string) error {
 }
 
 func (b *BindAddress) String() string {
+	if b.Host == "" && b.Port == "" {
+		return ""
+	}
 	return b.Host + ":" + b.Port
 }
 
 func (*BindAddress) Type() string {
 	return "bind-address"
 }
+
+type DiscardValue struct{}
+
+func (DiscardValue) Set(string) error {
+	return nil
+}
+
+func (DiscardValue) String() string {
+	return ""
+}
+
+func (DiscardValue) Type() string {
+	return "nop"
+}
diff --git a/cli/config/file.go b/cli/config/file.go
@@ -4,6 +4,8 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+
+	"github.com/kirsle/configdir"
 )
 
 const (
@@ -98,3 +100,7 @@ func read(path string) ([]byte, error) {
 	defer fi.Close()
 	return io.ReadAll(fi)
 }
+
+func DefaultDir() string {
+	return configdir.LocalConfig("coderv2")
+}
diff --git a/cli/resetpassword_test.go b/cli/resetpassword_test.go
@@ -32,7 +32,6 @@ func TestResetPassword(t *testing.T) {
 	const newPassword = "MyNewPassword!"
 
 	// start postgres and coder server processes
-
 	connectionURL, closeFunc, err := postgres.Open()
 	require.NoError(t, err)
 	defer closeFunc()

diff --git a/cli/root.go b/cli/root.go
@@ -22,7 +22,6 @@ import (
 	"cdr.dev/slog"
 
 	"github.com/charmbracelet/lipgloss"
-	"github.com/kirsle/configdir"
 	"github.com/mattn/go-isatty"
 	"github.com/spf13/cobra"
 
@@ -169,7 +168,7 @@ func Root(subcommands []*cobra.Command) *cobra.Command {
 	_ = cmd.PersistentFlags().MarkHidden(varAgentToken)
 	cliflag.String(cmd.PersistentFlags(), varAgentURL, "", "CODER_AGENT_URL", "", "URL for an agent to access your deployment.")
 	_ = cmd.PersistentFlags().MarkHidden(varAgentURL)
-	cliflag.String(cmd.PersistentFlags(), config.FlagName, "", "CODER_CONFIG_DIR", configdir.LocalConfig("coderv2"), "Path to the global `coder` config directory.")
+	cliflag.String(cmd.PersistentFlags(), config.FlagName, "", "CODER_CONFIG_DIR", config.DefaultDir(), "Path to the global `coder` config directory.")
 	cliflag.StringArray(cmd.PersistentFlags(), varHeader, "", "CODER_HEADER", []string{}, "HTTP headers added to all requests. Provide as \"Key=Value\"")
 	cmd.PersistentFlags().Bool(varForceTty, false, "Force the `coder` command to run as if connected to a TTY.")
 	_ = cmd.PersistentFlags().MarkHidden(varForceTty)

diff --git a/cli/server.go b/cli/server.go
@@ -11,6 +11,7 @@ import (
 	"crypto/x509"
 	"database/sql"
 	"errors"
+	"flag"
 	"fmt"
 	"io"
 	"log"
@@ -96,19 +97,31 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, io.Close
 			cfg := codersdk.NewDeploymentConfig()
 			cliOpts := cfg.ConfigOptions()
 
-			_, err := cliOpts.ParseFlags(args...)
+			var configDir bigcli.String
+			// This is a hack to get around the fact that the Cobra-defined
+			// flags are not available.
+			cliOpts.Add(bigcli.Option{
+				Name:    "Global Config",
+				Flag:    config.FlagName,
+				Usage:   "Global Config is ignored in server mode.",
+				Hidden:  true,
+				Default: config.DefaultDir(),
+				Value:   &configDir,
+			})
+
+			err := cliOpts.SetDefaults()
 			if err != nil {
-				return xerrors.Errorf("parse flags: %w", err)
+				return xerrors.Errorf("set defaults: %w", err)
 			}
 
-			err = cliOpts.ParseEnv("CODER_", os.Environ())
+			_, err = cliOpts.ParseFlags(args...)
 			if err != nil {
-				return xerrors.Errorf("parse env: %w", err)
+				return xerrors.Errorf("parse flags: %w", err)
 			}
 
-			err = cliOpts.SetDefaults()
+			err = cliOpts.ParseEnv("CODER_", os.Environ())
 			if err != nil {
-				return xerrors.Errorf("set defaults: %w", err)
+				return xerrors.Errorf("parse env: %w", err)
 			}
 
 			// Print deprecation warnings.
@@ -134,7 +147,7 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, io.Close
 			go dumpHandler(ctx)
 
 			// Validate bind addresses.
-			if cfg.Address.Host != "" {
+			if cfg.Address.String() != "" {
 				if cfg.TLS.Enable {
 					cfg.HTTPAddress.Host = ""
 					cfg.TLS.Address = cfg.Address
@@ -143,10 +156,10 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, io.Close
 					cfg.TLS.Address.Host = ""
 				}
 			}
-			if cfg.TLS.Enable && cfg.TLS.Address.Host == "" {
+			if cfg.TLS.Enable && cfg.TLS.Address.String() == "" {
 				return xerrors.Errorf("TLS address must be set if TLS is enabled")
 			}
-			if !cfg.TLS.Enable && cfg.HTTPAddress.Host == "" {
+			if !cfg.TLS.Enable && cfg.HTTPAddress.String() == "" {
 				return xerrors.Errorf("TLS is disabled. Enable with --tls-enable or specify a HTTP address")
 			}
 
@@ -155,7 +168,6 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, io.Close
 			loginRateLimit := 60
 			filesRateLimit := 12
 			if cfg.RateLimit.DisableAll {
-
 				cfg.RateLimit.API = -1
 				loginRateLimit = -1
 				filesRateLimit = -1
@@ -233,7 +245,7 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, io.Close
 				}
 			}
 
-			config := createConfig(cmd)
+			config := config.Root(configDir)
 			builtinPostgres := false
 			// Only use built-in if PostgreSQL URL isn't specified!
 			if !cfg.InMemoryDatabase && cfg.PostgresURL == "" {
@@ -264,7 +276,7 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, io.Close
 				httpListener net.Listener
 				httpURL      *url.URL
 			)
-			if cfg.HTTPAddress.Host != "" {
+			if cfg.HTTPAddress.String() != "" {
 				httpListener, err = net.Listen("tcp", cfg.HTTPAddress.String())
 				if err != nil {
 					return xerrors.Errorf("listen %q: %w", cfg.HTTPAddress.String(), err)
@@ -304,7 +316,7 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, io.Close
 				httpsURL      *url.URL
 			)
 			if cfg.TLS.Enable {
-				if cfg.TLS.Address.Host == "" {
+				if cfg.TLS.Address.String() == "" {
 					return xerrors.New("tls address must be set if tls is enabled")
 				}
 
@@ -736,7 +748,10 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, io.Close
 			// This is helpful for tests, but can be silently ignored.
 			// Coder may be ran as users that don't have permission to write in the homedir,
 			// such as via the systemd service.
-			_ = config.URL().Write(client.URL.String())
+			err = config.URL().Write(client.URL.String())
+			if err != nil && flag.Lookup("test.v") != nil {
+				return xerrors.Errorf("write config url: %w", err)
+			}
 
 			// Since errCh only has one buffered slot, all routines
 			// sending on it must be wrapped in a select/default to

diff --git a/coderd/deploymentconfig.go b/coderd/deploymentconfig.go
@@ -20,5 +20,11 @@ func (api *API) deploymentConfig(rw http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	httpapi.Write(r.Context(), rw, http.StatusOK, api.DeploymentConfig)
+	scrubbedConfig, err := api.DeploymentConfig.Scrub()
+	if err != nil {
+		httpapi.InternalServerError(rw, err)
+		return
+	}
+
+	httpapi.Write(r.Context(), rw, http.StatusOK, scrubbedConfig)
 }
diff --git a/coderd/deploymentconfig_test.go b/coderd/deploymentconfig_test.go
@@ -31,10 +31,10 @@ func TestDeploymentConfig(t *testing.T) {
 	scrubbed, err := client.DeploymentConfig(ctx)
 	require.NoError(t, err)
 	// ensure normal values pass through
-	require.EqualValues(t, true, scrubbed.BrowserOnly.Value)
+	require.EqualValues(t, true, scrubbed.BrowserOnly.Value())
 	// ensure secrets are removed
-	require.Empty(t, scrubbed.OAuth2.Github.ClientSecret.Value)
-	require.Empty(t, scrubbed.OIDC.ClientSecret.Value)
-	require.Empty(t, scrubbed.PostgresURL.Value)
-	require.Empty(t, scrubbed.SCIMAPIKey.Value)
+	require.Empty(t, scrubbed.OAuth2.Github.ClientSecret.Value())
+	require.Empty(t, scrubbed.OIDC.ClientSecret.Value())
+	require.Empty(t, scrubbed.PostgresURL.Value())
+	require.Empty(t, scrubbed.SCIMAPIKey.Value())
 }
diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go
@@ -1318,8 +1318,8 @@ func TestAppSharing(t *testing.T) {
 		deploymentConfig, err := ownerClient.DeploymentConfig(context.Background())
 		require.NoError(t, err)
 
-		assert.Equal(t, pathAppSharingEnabled, deploymentConfig.Dangerous.AllowPathAppSharing.Value)
-		assert.Equal(t, siteOwnerPathAppAccessEnabled, deploymentConfig.Dangerous.AllowPathAppSiteOwnerAccess.Value)
+		assert.Equal(t, pathAppSharingEnabled, deploymentConfig.Dangerous.AllowPathAppSharing.Value())
+		assert.Equal(t, siteOwnerPathAppAccessEnabled, deploymentConfig.Dangerous.AllowPathAppSiteOwnerAccess.Value())
 
 		t.Run("LevelOwner", func(t *testing.T) {
 			t.Parallel()