Merge branch 'bjk/refactor_config' into 'main'

Refactor Config defaults

See merge request gitlab-org/gitlab-shell!450

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