feat: configuration source precedence

cmd/loki/main.go

@@ -7,16 +7,14 @@ import (
 	"reflect"
 
 	"github.com/go-kit/kit/log/level"
-	"github.com/grafana/loki/pkg/helpers"
+	"github.com/grafana/loki/pkg/cfg"
 	"github.com/grafana/loki/pkg/loki"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/common/version"
 	"github.com/weaveworks/common/logging"
 	"github.com/weaveworks/common/tracing"
 
 	"github.com/cortexproject/cortex/pkg/util"
-	"github.com/cortexproject/cortex/pkg/util/flagext"
-
 	"github.com/grafana/loki/pkg/util/validation"
 )
 
@@ -25,56 +23,44 @@ func init() {
 }
 
 func main() {
-	var (
-		cfg        loki.Config
-		configFile = ""
-	)
-	flag.StringVar(&configFile, "config.file", "", "Configuration file to load.")
-	flagext.RegisterFlags(&cfg)
-
 	printVersion := flag.Bool("version", false, "Print this builds version information")
-	flag.Parse()
 
+	var config loki.Config
+	if err := cfg.Parse(&config); err != nil {
+		level.Error(util.Logger).Log("msg", "parsing config", "error", err)
+		os.Exit(1)
+	}
 	if *printVersion {
 		fmt.Print(version.Print("loki"))
 		os.Exit(0)
 	}
 
-	// LimitsConfig has a customer UnmarshalYAML that will set the defaults to a global.
 	// This global is set to the config passed into the last call to `NewOverrides`. If we don't
 	// call it atleast once, the defaults are set to an empty struct.
 	// We call it with the flag values so that the config file unmarshalling only overrides the values set in the config.
-	if _, err := validation.NewOverrides(cfg.LimitsConfig); err != nil {
-		level.Error(util.Logger).Log("msg", "error loading limits", "err", err)
+	if _, err := validation.NewOverrides(config.LimitsConfig); err != nil {
+		level.Error(util.Logger).Log("msg", "setting up overrides", "error", err)
 		os.Exit(1)
 	}
 
-	util.InitLogger(&cfg.Server)
-
-	if configFile != "" {
-		if err := helpers.LoadConfig(configFile, &cfg); err != nil {
-			level.Error(util.Logger).Log("msg", "error loading config", "filename", configFile, "err", err)
-			os.Exit(1)
-		}
-	}
-
-	// Re-init the logger which will now honor a different log level set in cfg.Server
-	if reflect.DeepEqual(&cfg.Server.LogLevel, &logging.Level{}) {
+	// Init the logger which will honor the log level set in config.Server
+	if reflect.DeepEqual(&config.Server.LogLevel, &logging.Level{}) {
 		level.Error(util.Logger).Log("msg", "invalid log level")
 		os.Exit(1)
 	}
-	util.InitLogger(&cfg.Server)
+	util.InitLogger(&config.Server)
 
 	// Setting the environment variable JAEGER_AGENT_HOST enables tracing
-	trace := tracing.NewFromEnv(fmt.Sprintf("loki-%s", cfg.Target))
+	trace := tracing.NewFromEnv(fmt.Sprintf("loki-%s", config.Target))
 	defer func() {
 		if err := trace.Close(); err != nil {
 			level.Error(util.Logger).Log("msg", "error closing tracing", "err", err)
 			os.Exit(1)
 		}
 	}()
 
-	t, err := loki.New(cfg)
+	// Start Loki
+	t, err := loki.New(config)
 	if err != nil {
 		level.Error(util.Logger).Log("msg", "error initialising loki", "err", err)
 		os.Exit(1)

cmd/promtail/main.go

@@ -7,13 +7,12 @@ import (
 	"reflect"
 
 	"github.com/cortexproject/cortex/pkg/util"
-	"github.com/cortexproject/cortex/pkg/util/flagext"
 	"github.com/go-kit/kit/log/level"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/common/version"
 	"github.com/weaveworks/common/logging"
 
-	"github.com/grafana/loki/pkg/helpers"
+	"github.com/grafana/loki/pkg/cfg"
 	"github.com/grafana/loki/pkg/logentry/stages"
 	"github.com/grafana/loki/pkg/promtail"
 	"github.com/grafana/loki/pkg/promtail/config"
@@ -24,31 +23,19 @@ func init() {
 }
 
 func main() {
-	var (
-		configFile = "cmd/promtail/promtail-local-config.yaml"
-		config     config.Config
-	)
-	flag.StringVar(&configFile, "config.file", "promtail.yml", "The config file.")
-	flagext.RegisterFlags(&config)
-
 	printVersion := flag.Bool("version", false, "Print this builds version information")
-	flag.Parse()
 
+	var config config.Config
+	if err := cfg.Parse(&config); err != nil {
+		level.Error(util.Logger).Log("msg", "parsing config", "error", err)
+		os.Exit(1)
+	}
 	if *printVersion {
 		fmt.Print(version.Print("promtail"))
 		os.Exit(0)
 	}
 
-	util.InitLogger(&config.ServerConfig.Config)
-
-	if configFile != "" {
-		if err := helpers.LoadConfig(configFile, &config); err != nil {
-			level.Error(util.Logger).Log("msg", "error loading config", "filename", configFile, "err", err)
-			os.Exit(1)
-		}
-	}
-
-	// Re-init the logger which will now honor a different log level set in ServerConfig.Config
+	// Init the logger which will honor the log level set in cfg.Server
 	if reflect.DeepEqual(&config.ServerConfig.Config.LogLevel, &logging.Level{}) {
 		level.Error(util.Logger).Log("msg", "invalid log level")
 		os.Exit(1)

pkg/cfg/cfg.go

@@ -0,0 +1,61 @@
+package cfg
+
+import (
+	"reflect"
+
+	"github.com/pkg/errors"
+)
+
+// Source is a generic configuration source. This function may do whatever is
+// required to obtain the configuration. It is passed a pointer to the
+// destination, which will be something compatible to `json.Unmarshal`. The
+// obtained configuration may be written to this object, it may also contain
+// data from previous sources.
+type Source func(interface{}) error
+
+var (
+	ErrNotPointer = errors.New("dst is not a pointer")
+)
+
+// Unmarshal merges the values of the various configuration sources and sets them on
+// `dst`. The object must be compatible with `json.Unmarshal`.
+func Unmarshal(dst interface{}, sources ...Source) error {
+	if len(sources) == 0 {
+		panic("No sources supplied to cfg.Unmarshal(). This is most likely a programming issue and should never happen. Check the code!")
+	}
+	if reflect.ValueOf(dst).Kind() != reflect.Ptr {
+		return ErrNotPointer
+	}
+
+	for _, source := range sources {
+		if err := source(dst); err != nil {
+			return errors.Wrap(err, "sourcing")
+		}
+	}
+	return nil
+}
+
+// Parse is a higher level wrapper for Unmarshal that automatically parses flags and a .yaml file
+func Parse(dst interface{}) error {
+	return dParse(dst,
+		Defaults(),
+		YAMLFlag("config.file", "", "yaml file to load"),
+		Flags(),
+	)
+}
+
+// dParse is the same as Parse, but with dependency injection for testing
+func dParse(dst interface{}, defaults, yaml, flags Source) error {
+	// check dst is a pointer
+	v := reflect.ValueOf(dst)
+	if v.Kind() != reflect.Ptr {
+		return ErrNotPointer
+	}
+
+	// unmarshal config
+	return Unmarshal(dst,
+		defaults,
+		yaml,
+		flags,
+	)
+}

pkg/cfg/cfg_test.go

@@ -0,0 +1,43 @@
+package cfg
+
+import (
+	"flag"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestParse(t *testing.T) {
+	yamlSource := dYAML([]byte(`
+server:
+  port: 2000
+  timeout: 60h
+tls:
+  key: YAML
+`))
+
+	fs := flag.NewFlagSet(t.Name(), flag.PanicOnError)
+	flagSource := dFlags(fs, []string{"-verbose", "-server.port=21"})
+
+	data := Data{}
+	err := dParse(&data,
+		dDefaults(fs),
+		yamlSource,
+		flagSource,
+	)
+	require.NoError(t, err)
+
+	assert.Equal(t, Data{
+		Verbose: true, // flag
+		Server: Server{
+			Port:    21,             // flag
+			Timeout: 60 * time.Hour, // defaults
+		},
+		TLS: TLS{
+			Cert: "DEFAULTCERT", // defaults
+			Key:  "YAML",        // yaml
+		},
+	}, data)
+}

pkg/cfg/data_test.go

@@ -0,0 +1,33 @@
+package cfg
+
+import (
+	"flag"
+	"time"
+)
+
+// Data is a test Data structure
+type Data struct {
+	Verbose bool   `yaml:"verbose"`
+	Server  Server `yaml:"server"`
+	TLS     TLS    `yaml:"tls"`
+}
+
+type Server struct {
+	Port    int           `yaml:"port"`
+	Timeout time.Duration `yaml:"timeout"`
+}
+
+type TLS struct {
+	Cert string `yaml:"cert"`
+	Key  string `yaml:"key"`
+}
+
+// RegisterFlags makes Data implement flagext.Registerer for using flags
+func (d *Data) RegisterFlags(fs *flag.FlagSet) {
+	fs.BoolVar(&d.Verbose, "verbose", false, "")
+	fs.IntVar(&d.Server.Port, "server.port", 80, "")
+	fs.DurationVar(&d.Server.Timeout, "server.timeout", 60*time.Second, "")
+
+	fs.StringVar(&d.TLS.Cert, "tls.cert", "DEFAULTCERT", "")
+	fs.StringVar(&d.TLS.Key, "tls.key", "DEFAULTKEY", "")
+}

pkg/cfg/files.go

@@ -0,0 +1,68 @@
+package cfg
+
+import (
+	"encoding/json"
+	"flag"
+	"io/ioutil"
+
+	yaml "gopkg.in/yaml.v2"
+)
+
+// JSON returns a Source that opens the supplied `.json` file and loads it.
+func JSON(f *string) Source {
+	return func(dst interface{}) error {
+		if f == nil {
+			return nil
+		}
+
+		j, err := ioutil.ReadFile(*f)
+		if err != nil {
+			return err
+		}
+
+		return dJSON(j)(dst)
+	}
+}
+
+// dJSON returns a JSON source and allows dependency injection
+func dJSON(y []byte) Source {
+	return func(dst interface{}) error {
+		return json.Unmarshal(y, dst)
+	}
+}
+
+// YAML returns a Source that opens the supplied `.yaml` file and loads it.
+func YAML(f *string) Source {
+	return func(dst interface{}) error {
+		if f == nil {
+			return nil
+		}
+
+		y, err := ioutil.ReadFile(*f)
+		if err != nil {
+			return err
+		}
+
+		return dYAML(y)(dst)
+	}
+}
+
+// dYAML returns a YAML source and allows dependency injection
+func dYAML(y []byte) Source {
+	return func(dst interface{}) error {
+		return yaml.Unmarshal(y, dst)
+	}
+}
+
+// YAMLFlag defines a `config.file` flag and loads this file
+func YAMLFlag(name, value, help string) Source {
+	return func(dst interface{}) error {
+		f := flag.String(name, value, help)
+		flag.Parse()
+
+		if *f == "" {
+			f = nil
+		}
+		return YAML(f)(dst)
+	}
+}

pkg/cfg/flag.go

@@ -0,0 +1,43 @@
+package cfg
+
+import (
+	"flag"
+	"os"
+
+	"github.com/cortexproject/cortex/pkg/util/flagext"
+	"github.com/pkg/errors"
+)
+
+// Defaults registers flags to the command line using dst as the
+// flagext.Registerer
+func Defaults() Source {
+	return dDefaults(flag.CommandLine)
+}
+
+// dDefaults registers flags to the flagSet using dst as the flagext.Registerer
+func dDefaults(fs *flag.FlagSet) Source {
+	return func(dst interface{}) error {
+		r, ok := dst.(flagext.Registerer)
+		if !ok {
+			return errors.New("dst does not satisfy flagext.Registerer")
+		}
+
+		// already sets the defaults on r
+		r.RegisterFlags(fs)
+		return nil
+	}
+}
+
+// Flags parses the flag from the command line, setting only user-supplied
+// values on the flagext.Registerer passed to Defaults()
+func Flags() Source {
+	return dFlags(flag.CommandLine, os.Args[1:])
+}
+
+// dFlags parses the flagset, applying all values set on the slice
+func dFlags(fs *flag.FlagSet, args []string) Source {
+	return func(dst interface{}) error {
+		// parse the final flagset
+		return fs.Parse(args)
+	}
+}