Refactor Config defaults

Use "omitempty" to allow defaults in the config file to be correctly
passed. Without this, explicitly setting an empty default like an empty
string will not work. Needed in order to allow explicitly disabling some
settings.

Related to: https://gitlab.com/gitlab-org/gitlab-shell/-/issues/121

Signed-off-by: Ben Kochie <superq@gmail.com>

2 files changed, 33 insertions, 51 deletions

diff --git a/cmd/gitlab-sshd/main.go b/cmd/gitlab-sshd/main.go
index b9ea67a..443c142 100644
--- a/cmd/gitlab-sshd/main.go
+++ b/cmd/gitlab-sshd/main.go
@@ -42,7 +42,6 @@ func main() {
 		}
 	}
 	overrideConfigFromEnvironment(cfg)
-	cfg.ApplyServerDefaults()
 	if err := cfg.IsSane(); err != nil {
 		if *configDir == "" {
 			log.Warn("note: no config-dir provided, using only environment variables")
diff --git a/internal/config/config.go b/internal/config/config.go
index ac5c985..d729ea5 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -13,14 +13,13 @@ import (
 
 const (
 	configFile            = "config.yml"
-	logFile               = "gitlab-shell.log"
 	defaultSecretFileName = ".gitlab_shell_secret"
 )
 
 type ServerConfig struct {
-	Listen                  string   `yaml:"listen"`
-	ConcurrentSessionsLimit int64    `yaml:"concurrent_sessions_limit"`
-	HostKeyFiles            []string `yaml:"host_key_files"`
+	Listen                  string   `yaml:"listen,omitempty"`
+	ConcurrentSessionsLimit int64    `yaml:"concurrent_sessions_limit,omitempty"`
+	HostKeyFiles            []string `yaml:"host_key_files,omitempty"`
 }
 
 type HttpSettingsConfig struct {
@@ -33,10 +32,10 @@ type HttpSettingsConfig struct {
 }
 
 type Config struct {
-	User                  string `yaml:"user"`
+	User                  string `yaml:"user,omitempty"`
 	RootDir               string
-	LogFile               string `yaml:"log_file"`
-	LogFormat             string `yaml:"log_format"`
+	LogFile               string `yaml:"log_file,omitempty"`
+	LogFormat             string `yaml:"log_format,omitempty"`
 	GitlabUrl             string `yaml:"gitlab_url"`
 	GitlabRelativeURLRoot string `yaml:"gitlab_relative_url_root"`
 	GitlabTracing         string `yaml:"gitlab_tracing"`
@@ -49,6 +48,26 @@ type Config struct {
 	HttpClient     *client.HttpClient `-`
 }
 
+// The defaults to apply before parsing the config file(s).
+var (
+	DefaultConfig = Config{
+		LogFile: "gitlab-shell.log",
+		LogFormat: "text",
+		Server: DefaultServerConfig,
+		User: "git",
+	}
+
+	DefaultServerConfig = ServerConfig{
+		Listen: "[::]:22",
+		ConcurrentSessionsLimit: 10,
+		HostKeyFiles: []string{
+			"/run/secrets/ssh-hostkeys/ssh_host_rsa_key",
+			"/run/secrets/ssh-hostkeys/ssh_host_ecdsa_key",
+			"/run/secrets/ssh-hostkeys/ssh_host_ed25519_key",
+		},
+	}
+)
+
 func (c *Config) GetHttpClient() *client.HttpClient {
 	if c.HttpClient != nil {
 		return c.HttpClient
@@ -74,7 +93,6 @@ func NewFromDirExternal(dir string) (*Config, error) {
 	if err != nil {
 		return nil, err
 	}
-	cfg.ApplyExternalDefaults()
 	return cfg, nil
 }
 
@@ -87,7 +105,9 @@ func NewFromDir(dir string) (*Config, error) {
 
 // newFromFile reads a new Config instance from the given file path. It doesn't apply any defaults.
 func newFromFile(path string) (*Config, error) {
-	cfg := &Config{RootDir: filepath.Dir(path)}
+	cfg := &Config{}
+	*cfg = DefaultConfig
+	cfg.RootDir = filepath.Dir(path)
 
 	configBytes, err := ioutil.ReadFile(path)
 	if err != nil {
@@ -112,6 +132,10 @@ func newFromFile(path string) (*Config, error) {
 		return nil, err
 	}
 
+	if len(cfg.LogFile) > 0 && cfg.LogFile[0] != '/' && cfg.RootDir != "" {
+		cfg.LogFile = filepath.Join(cfg.RootDir, cfg.LogFile)
+	}
+
 	return cfg, nil
 }
 
@@ -138,47 +162,6 @@ func parseSecret(cfg *Config) error {
 	return nil
 }
 
-// ApplyServerDefaults applies defaults running inside an external SSH server.
-func (cfg *Config) ApplyExternalDefaults() {
-	// Set default LogFile to a file since with an external SSH server stdout is not a possibility.
-	if cfg.LogFile == "" {
-		cfg.LogFile = logFile
-	}
-	cfg.applyGenericDefaults()
-}
-
-// applyGenericDefaults applies defaults common to all operating modes.
-func (cfg *Config) applyGenericDefaults() {
-	if cfg.LogFormat == "" {
-		cfg.LogFormat = "text"
-	}
-	// Currently only used by the built-in SSH server, but not specific to it, so let's to it here.
-	if cfg.User == "" {
-		cfg.User = "git"
-	}
-	if len(cfg.LogFile) > 0 && cfg.LogFile[0] != '/' && cfg.RootDir != "" {
-		cfg.LogFile = filepath.Join(cfg.RootDir, cfg.LogFile)
-	}
-}
-
-// ApplyServerDefaults applies defaults for the built-in SSH server.
-func (cfg *Config) ApplyServerDefaults() {
-	if cfg.Server.ConcurrentSessionsLimit == 0 {
-		cfg.Server.ConcurrentSessionsLimit = 10
-	}
-	if cfg.Server.Listen == "" {
-		cfg.Server.Listen = "[::]:22"
-	}
-	if len(cfg.Server.HostKeyFiles) == 0 {
-		cfg.Server.HostKeyFiles = []string{
-			"/run/secrets/ssh-hostkeys/ssh_host_rsa_key",
-			"/run/secrets/ssh-hostkeys/ssh_host_ecdsa_key",
-			"/run/secrets/ssh-hostkeys/ssh_host_ed25519_key",
-		}
-	}
-	cfg.applyGenericDefaults()
-}
-
 // IsSane checks if the given config fulfills the minimum requirements to be able to run.
 // Any error returned by this function should be a startup error. On the other hand
 // if this function returns nil, this doesn't guarantee the config will work, but it's