fix(config): fix loading config precedence

cmd/root.go

@@ -2,6 +2,7 @@ package cmd
 
 import (
 	"fmt"
+	"os"
 	"strings"
 	"time"
 
@@ -17,8 +18,17 @@ const (
 	defaultConfigFilename = "config"
 )
 
-var (
-	rootCmd = &cobra.Command{
+var conf = config.NewConfig()
+
+func Execute() {
+	cmd := NewRootCommand()
+	if err := cmd.Execute(); err != nil {
+		os.Exit(1)
+	}
+}
+
+func NewRootCommand() *cobra.Command {
+	rootCmd := &cobra.Command{
 		Use:   "sablier",
 		Short: "A webserver to start container on demand",
 		Long: `Sablier is an API that start containers on demand.
@@ -28,18 +38,8 @@ It provides an integrations with multiple reverse proxies and different loading
 			return initializeConfig(cmd)
 		},
 	}
-)
 
-// Execute executes the root command.
-func Execute() error {
-	return rootCmd.Execute()
-}
-
-var conf = config.NewConfig()
-
-func init() {
-
-	rootCmd.AddCommand(startCmd)
+	startCmd := newStartCommand()
 	// Provider flags
 	startCmd.Flags().StringVar(&conf.Provider.Name, "provider.name", "docker", fmt.Sprintf("Provider to use to manage containers %v", config.GetProviders()))
 	viper.BindPFlag("provider.name", startCmd.Flags().Lookup("provider.name"))
@@ -54,7 +54,7 @@ func init() {
 	// Sessions flags
 	startCmd.Flags().DurationVar(&conf.Sessions.DefaultDuration, "sessions.default-duration", time.Duration(5)*time.Minute, "The default session duration")
 	viper.BindPFlag("sessions.default-duration", startCmd.Flags().Lookup("sessions.default-duration"))
-	startCmd.Flags().DurationVar(&conf.Sessions.DefaultDuration, "sessions.expiration-interval", time.Duration(20)*time.Second, "The expiration checking interval. Higher duration gives less stress on CPU. If you only use sessions of 1h, setting this to 5m is a good trade-off.")
+	startCmd.Flags().DurationVar(&conf.Sessions.ExpirationInterval, "sessions.expiration-interval", time.Duration(20)*time.Second, "The expiration checking interval. Higher duration gives less stress on CPU. If you only use sessions of 1h, setting this to 5m is a good trade-off.")
 	viper.BindPFlag("sessions.expiration-interval", startCmd.Flags().Lookup("sessions.expiration-interval"))
 
 	// logging level
@@ -71,7 +71,10 @@ func init() {
 	startCmd.Flags().DurationVar(&conf.Strategy.Blocking.DefaultTimeout, "strategy.blocking.default-timeout", 1*time.Minute, "Default timeout used for blocking strategy")
 	viper.BindPFlag("strategy.blocking.default-timeout", startCmd.Flags().Lookup("strategy.blocking.default-timeout"))
 
+	rootCmd.AddCommand(startCmd)
 	rootCmd.AddCommand(versionCmd)
+
+	return rootCmd
 }
 
 func initializeConfig(cmd *cobra.Command) error {

cmd/root_test.go

@@ -0,0 +1,183 @@
+package cmd
+
+import (
+	"bufio"
+	"bytes"
+	"encoding/json"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/acouvreur/sablier/config"
+	"github.com/spf13/cobra"
+	"github.com/spf13/viper"
+	"github.com/stretchr/testify/require"
+	"gotest.tools/v3/assert"
+)
+
+func TestPrecedence(t *testing.T) {
+	// Run the tests in a temporary directory
+	tmpDir := os.TempDir()
+	testDir, err := os.Getwd()
+	require.NoError(t, err, "error getting the current working directory")
+	defer os.Chdir(testDir)
+	err = os.Chdir(tmpDir)
+	require.NoError(t, err, "error changing to the temporary test directory")
+
+	// CHANGE `startCmd` behavior to only print the config, this is for testing purposes only
+	newStartCommand = mockStartCommand
+
+	t.Run("config file", func(t *testing.T) {
+		configB, err := os.ReadFile(filepath.Join(testDir, "testdata", "config.yml"))
+		require.NoError(t, err, "error reading test config file")
+		err = ioutil.WriteFile(filepath.Join(tmpDir, "config.yml"), configB, 0644)
+		require.NoError(t, err, "error writing test config file")
+		defer os.Remove(filepath.Join(tmpDir, "config.yml"))
+
+		wantConfig, err := ioutil.ReadFile(filepath.Join(testDir, "testdata", "config_yaml_wanted.json"))
+		require.NoError(t, err, "error reading test config file")
+
+		conf = config.NewConfig()
+		cmd := NewRootCommand()
+		output := &bytes.Buffer{}
+		cmd.SetOut(output)
+		cmd.SetArgs([]string{"start"})
+		cmd.Execute()
+
+		gotOutput := output.String()
+
+		assert.Equal(t, string(wantConfig), gotOutput)
+	})
+
+	t.Run("env var", func(t *testing.T) {
+		// 1. Load Config file for precedence assertions
+		configB, err := os.ReadFile(filepath.Join(testDir, "testdata", "config.yml"))
+		require.NoError(t, err, "error reading test config file")
+		err = ioutil.WriteFile(filepath.Join(tmpDir, "config.yml"), configB, 0644)
+		require.NoError(t, err, "error writing test config file")
+		defer os.Remove(filepath.Join(tmpDir, "config.yml"))
+
+		setEnvsFromFile(filepath.Join(testDir, "testdata", "config.env"))
+		defer unsetEnvsFromFile(filepath.Join(testDir, "testdata", "config.env"))
+
+		wantConfig, err := ioutil.ReadFile(filepath.Join(testDir, "testdata", "config_env_wanted.json"))
+		require.NoError(t, err, "error reading test config file")
+
+		conf = config.NewConfig()
+		cmd := NewRootCommand()
+		output := &bytes.Buffer{}
+		cmd.SetOut(output)
+		cmd.SetArgs([]string{"start"})
+		cmd.Execute()
+
+		gotOutput := output.String()
+
+		assert.Equal(t, string(wantConfig), gotOutput)
+	})
+
+	t.Run("flag", func(t *testing.T) {
+		// 1. Load Config file for precedence assertions
+		configB, err := os.ReadFile(filepath.Join(testDir, "testdata", "config.yml"))
+		require.NoError(t, err, "error reading test config file")
+		err = ioutil.WriteFile(filepath.Join(tmpDir, "config.yml"), configB, 0644)
+		require.NoError(t, err, "error writing test config file")
+		defer os.Remove(filepath.Join(tmpDir, "config.yml"))
+
+		// 2. Load envs variable for precedence assertions
+		setEnvsFromFile(filepath.Join(testDir, "testdata", "config.env"))
+		defer unsetEnvsFromFile(filepath.Join(testDir, "testdata", "config.env"))
+
+		wantConfig, err := ioutil.ReadFile(filepath.Join(testDir, "testdata", "config_cli_wanted.json"))
+		require.NoError(t, err, "error reading test config file")
+
+		cmd := NewRootCommand()
+		output := &bytes.Buffer{}
+		conf = config.NewConfig()
+		cmd.SetOut(output)
+		cmd.SetArgs([]string{
+			"start",
+			"--provider.name", "cli",
+			"--server.port", "3333",
+			"--server.base-path", "/cli/",
+			"--storage.file", "/tmp/cli.json",
+			"--sessions.default-duration", "3h",
+			"--sessions.expiration-interval", "3h",
+			"--logging.level", "info",
+			"--strategy.dynamic.custom-themes-path", "/tmp/cli/themes",
+			"--strategy.dynamic.default-theme", "cli",
+			"--strategy.dynamic.default-refresh-frequency", "3h",
+			"--strategy.blocking.default-timeout", "3h",
+		})
+		cmd.Execute()
+
+		gotOutput := output.String()
+
+		assert.Equal(t, string(wantConfig), gotOutput)
+	})
+}
+
+func setEnvsFromFile(path string) {
+	readFile, err := os.Open(path)
+
+	if err != nil {
+		panic(err)
+	}
+
+	defer readFile.Close()
+
+	if err != nil {
+		panic(err)
+	}
+
+	fileScanner := bufio.NewScanner(readFile)
+
+	fileScanner.Split(bufio.ScanLines)
+
+	for fileScanner.Scan() {
+		splitted := strings.Split(fileScanner.Text(), "=")
+		os.Setenv(splitted[0], splitted[1])
+	}
+}
+
+func unsetEnvsFromFile(path string) {
+	readFile, err := os.Open(path)
+
+	if err != nil {
+		panic(err)
+	}
+
+	defer readFile.Close()
+
+	if err != nil {
+		panic(err)
+	}
+
+	fileScanner := bufio.NewScanner(readFile)
+
+	fileScanner.Split(bufio.ScanLines)
+
+	for fileScanner.Scan() {
+		splitted := strings.Split(fileScanner.Text(), "=")
+		os.Unsetenv(splitted[0])
+	}
+}
+
+func mockStartCommand() *cobra.Command {
+	cmd := &cobra.Command{
+		Use:   "start",
+		Short: "Start the Sablier server",
+		Run: func(cmd *cobra.Command, args []string) {
+			viper.Unmarshal(&conf)
+
+			out := cmd.OutOrStdout()
+
+			encoder := json.NewEncoder(out)
+
+			encoder.SetIndent("", "  ")
+			encoder.Encode(conf)
+		},
+	}
+	return cmd
+}

cmd/start.go

@@ -2,21 +2,23 @@ package cmd
 
 import (
 	"github.com/acouvreur/sablier/app"
-	"github.com/acouvreur/sablier/config"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 )
 
-var startCmd = &cobra.Command{
-	Use:   "start",
-	Short: "Start the Sablier server",
-	Run: func(cmd *cobra.Command, args []string) {
-		conf := config.NewConfig()
-		viper.Unmarshal(&conf)
+var newStartCommand = func() *cobra.Command {
+	cmd := &cobra.Command{
 
-		err := app.Start(conf)
-		if err != nil {
-			panic(err)
-		}
-	},
+		Use:   "start",
+		Short: "Start the Sablier server",
+		Run: func(cmd *cobra.Command, args []string) {
+			viper.Unmarshal(&conf)
+
+			err := app.Start(conf)
+			if err != nil {
+				panic(err)
+			}
+		},
+	}
+	return cmd
 }

cmd/testdata/config.env

@@ -0,0 +1,11 @@
+PROVIDER_NAME=envvar
+SERVER_PORT=2222
+SERVER_BASE_PATH=/envvar/
+STORAGE_FILE=/tmp/envvar.json
+SESSIONS_DEFAULT_DURATION=2h
+SESSIONS_EXPIRATION_INTERVAL=2h
+LOGGING_LEVEL=debug
+STRATEGY_DYNAMIC_CUSTOM_THEMES_PATH=/tmp/envvar/themes
+STRATEGY_DYNAMIC_DEFAULT_THEME=envvar
+STRATEGY_DYNAMIC_DEFAULT_REFRESH_FREQUENCY=2h
+STRATEGY_BLOCKING_DEFAULT_TIMEOUT=2h

cmd/testdata/config.yml

@@ -0,0 +1,19 @@
+provider:
+  name: configfile
+server:
+  port: 1111
+  base-path: /configfile/
+storage:
+  file: /tmp/configfile.json
+sessions:
+  default-duration: 1h
+  expiration-interval: 1h
+logging:
+  level: trace
+strategy:
+  dynamic:
+    custom-themes-path: /tmp/configfile/themes
+    default-theme: configfile
+    default-refresh-frequency: 1h
+  blocking:
+    default-timeout: 1h

cmd/testdata/config_cli_wanted.json

@@ -0,0 +1,29 @@
+{
+  "Server": {
+    "Port": 3333,
+    "BasePath": "/cli/"
+  },
+  "Storage": {
+    "File": "/tmp/cli.json"
+  },
+  "Provider": {
+    "Name": "cli"
+  },
+  "Sessions": {
+    "DefaultDuration": 10800000000000,
+    "ExpirationInterval": 10800000000000
+  },
+  "Logging": {
+    "Level": "info"
+  },
+  "Strategy": {
+    "Dynamic": {
+      "CustomThemesPath": "/tmp/cli/themes",
+      "DefaultTheme": "cli",
+      "DefaultRefreshFrequency": 10800000000000
+    },
+    "Blocking": {
+      "DefaultTimeout": 10800000000000
+    }
+  }
+}

cmd/testdata/config_env_wanted.json

@@ -0,0 +1,29 @@
+{
+  "Server": {
+    "Port": 2222,
+    "BasePath": "/envvar/"
+  },
+  "Storage": {
+    "File": "/tmp/envvar.json"
+  },
+  "Provider": {
+    "Name": "envvar"
+  },
+  "Sessions": {
+    "DefaultDuration": 7200000000000,
+    "ExpirationInterval": 7200000000000
+  },
+  "Logging": {
+    "Level": "debug"
+  },
+  "Strategy": {
+    "Dynamic": {
+      "CustomThemesPath": "/tmp/envvar/themes",
+      "DefaultTheme": "envvar",
+      "DefaultRefreshFrequency": 7200000000000
+    },
+    "Blocking": {
+      "DefaultTimeout": 7200000000000
+    }
+  }
+}

cmd/testdata/config_yaml_wanted.json

@@ -0,0 +1,29 @@
+{
+  "Server": {
+    "Port": 1111,
+    "BasePath": "/configfile/"
+  },
+  "Storage": {
+    "File": "/tmp/configfile.json"
+  },
+  "Provider": {
+    "Name": "configfile"
+  },
+  "Sessions": {
+    "DefaultDuration": 3600000000000,
+    "ExpirationInterval": 3600000000000
+  },
+  "Logging": {
+    "Level": "trace"
+  },
+  "Strategy": {
+    "Dynamic": {
+      "CustomThemesPath": "/tmp/configfile/themes",
+      "DefaultTheme": "configfile",
+      "DefaultRefreshFrequency": 3600000000000
+    },
+    "Blocking": {
+      "DefaultTimeout": 3600000000000
+    }
+  }
+}

config/server.go

@@ -2,7 +2,7 @@ package config
 
 type Server struct {
 	Port     int    `mapstructure:"PORT" yaml:"port" default:"10000"`
-	BasePath string `mapstructure:"BASEPATH" yaml:"basePath" default:"/"`
+	BasePath string `mapstructure:"BASE_PATH" yaml:"basePath" default:"/"`
 }
 
 func NewServerConfig() Server {

config/strategy.go

@@ -3,13 +3,13 @@ package config
 import "time"
 
 type DynamicStrategy struct {
-	CustomThemesPath        string        `mapstructure:"CUSTOMTHEMESPATH" yaml:"customThemesPath"`
-	DefaultTheme            string        `mapstructure:"DEFAULTTHEME" yaml:"defaultTheme" default:"hacker-terminal"`
-	DefaultRefreshFrequency time.Duration `mapstructure:"DEFAULTREFRESHFREQUENCY" yaml:"defaultRefreshFrequency" default:"5s"`
+	CustomThemesPath        string        `mapstructure:"CUSTOM_THEMES_PATH" yaml:"customThemesPath"`
+	DefaultTheme            string        `mapstructure:"DEFAULT_THEME" yaml:"defaultTheme" default:"hacker-terminal"`
+	DefaultRefreshFrequency time.Duration `mapstructure:"DEFAULT_REFRESH_FREQUENCY" yaml:"defaultRefreshFrequency" default:"5s"`
 }
 
 type BlockingStrategy struct {
-	DefaultTimeout time.Duration `mapstructure:"DEFAULTTIMEOUT" yaml:"defaultTimeout" default:"1m"`
+	DefaultTimeout time.Duration `mapstructure:"DEFAULT_TIMEOUT" yaml:"defaultTimeout" default:"1m"`
 }
 
 type Strategy struct {