Loki: Common Config (#4347)

* refactor the pkg/util/cfg package to simplify and make it accessable outside of this package.
create a pkg/cfg which handles building the Loki config, this is created separately from pkg/util (which is Apache 2 licensed) to maintain proper separations of AGPL and Apache 2 code
Update the main.go for several of the apps with the refactoring

* refactor how dynamic/common config is loaded

Signed-off-by: Trevor Whitney <[email protected]>

* Add common config logic for path prefix

Signed-off-by: Trevor Whitney <[email protected]>

* Missed a file, update common config with prefix property

Signed-off-by: Trevor Whitney <[email protected]>

* fix test, remove loki from args

Signed-off-by: Trevor Whitney <[email protected]>

* Clean up test names

Signed-off-by: Trevor Whitney <[email protected]>

* fix typo in filename, make linter happy

Signed-off-by: Trevor Whitney <[email protected]>

* Update pkg/loki/config_wrapper.go

Co-authored-by: Owen Diehl <[email protected]>

* Update pkg/loki/config_wrapper.go

Co-authored-by: Owen Diehl <[email protected]>

* Update pkg/loki/config_wrapper_test.go

Co-authored-by: Owen Diehl <[email protected]>

* remove temp files in test

Signed-off-by: Trevor Whitney <[email protected]>

Co-authored-by: Trevor Whitney <[email protected]>
Co-authored-by: Owen Diehl <[email protected]>

clients/cmd/promtail/main.go

@@ -65,7 +65,7 @@ func (c *Config) Clone() flagext.Registerer {
 func main() {
 	// Load config, merging config file and CLI flags
 	var config Config
-	if err := cfg.Parse(&config); err != nil {
+	if err := cfg.DefaultUnmarshal(&config, os.Args[1:], flag.CommandLine); err != nil {
 		fmt.Println("Unable to parse config:", err)
 		os.Exit(1)
 	}

cmd/loki/main.go

@@ -7,7 +7,6 @@ import (
 	"reflect"
 
 	"github.com/go-kit/kit/log/level"
-	"github.com/grafana/dskit/flagext"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/common/version"
 	"github.com/weaveworks/common/logging"
@@ -26,43 +25,14 @@ func init() {
 	prometheus.MustRegister(version.NewCollector("loki"))
 }
 
-type Config struct {
-	loki.Config     `yaml:",inline"`
-	printVersion    bool
-	verifyConfig    bool
-	printConfig     bool
-	logConfig       bool
-	configFile      string
-	configExpandEnv bool
-}
-
-func (c *Config) RegisterFlags(f *flag.FlagSet) {
-	f.BoolVar(&c.printVersion, "version", false, "Print this builds version information")
-	f.BoolVar(&c.verifyConfig, "verify-config", false, "Verify config file and exits")
-	f.BoolVar(&c.printConfig, "print-config-stderr", false, "Dump the entire Loki config object to stderr")
-	f.BoolVar(&c.logConfig, "log-config-reverse-order", false, "Dump the entire Loki config object at Info log "+
-		"level with the order reversed, reversing the order makes viewing the entries easier in Grafana.")
-	f.StringVar(&c.configFile, "config.file", "", "yaml file to load")
-	f.BoolVar(&c.configExpandEnv, "config.expand-env", false, "Expands ${var} in config according to the values of the environment variables.")
-	c.Config.RegisterFlags(f)
-}
-
-// Clone takes advantage of pass-by-value semantics to return a distinct *Config.
-// This is primarily used to parse a different flag set without mutating the original *Config.
-func (c *Config) Clone() flagext.Registerer {
-	return func(c Config) *Config {
-		return &c
-	}(*c)
-}
-
 func main() {
-	var config Config
+	var config loki.ConfigWrapper
 
-	if err := cfg.Parse(&config); err != nil {
+	if err := cfg.DynamicUnmarshal(&config, os.Args[1:], flag.CommandLine); err != nil {
 		fmt.Fprintf(os.Stderr, "failed parsing config: %v\n", err)
 		os.Exit(1)
 	}
-	if config.printVersion {
+	if config.PrintVersion {
 		fmt.Println(version.Print("loki"))
 		os.Exit(0)
 	}
@@ -87,19 +57,19 @@ func main() {
 		os.Exit(1)
 	}
 
-	if config.verifyConfig {
+	if config.VerifyConfig {
 		level.Info(util_log.Logger).Log("msg", "config is valid")
 		os.Exit(0)
 	}
 
-	if config.printConfig {
+	if config.PrintConfig {
 		err := logutil.PrintConfig(os.Stderr, &config)
 		if err != nil {
 			level.Error(util_log.Logger).Log("msg", "failed to print config to stderr", "err", err.Error())
 		}
 	}
 
-	if config.logConfig {
+	if config.LogConfig {
 		err := logutil.LogConfig(&config)
 		if err != nil {
 			level.Error(util_log.Logger).Log("msg", "failed to log config object", "err", err.Error())

cmd/migrate/main.go

@@ -52,7 +52,7 @@ func main() {
 	flag.Parse()
 
 	// Create a set of defaults
-	if err := cfg.Unmarshal(&defaultsConfig, cfg.Defaults()); err != nil {
+	if err := cfg.Unmarshal(&defaultsConfig, cfg.Defaults(flag.CommandLine)); err != nil {
 		log.Println("Failed parsing defaults config:", err)
 		os.Exit(1)
 	}

pkg/loki/common/common.go

@@ -0,0 +1,6 @@
+package common
+
+// Config holds common config that can be shared between multiple other config sections
+type Config struct {
+	PathPrefix string `yaml:"path_prefix"`
+}

pkg/loki/config_wrapper.go

@@ -0,0 +1,74 @@
+package loki
+
+import (
+	"flag"
+	"fmt"
+
+	"github.com/grafana/dskit/flagext"
+	"github.com/pkg/errors"
+
+	"github.com/grafana/loki/pkg/util/cfg"
+)
+
+// ConfigWrapper is a struct containing the Loki config along with other values that can be set on the command line
+// for interacting with the config file or the application directly.
+// ConfigWrapper implements cfg.DynamicCloneable, allowing configuration to be dynamically set based
+// on the logic in ApplyDynamicConfig, which receives values set in config file
+type ConfigWrapper struct {
+	Config          `yaml:",inline"`
+	PrintVersion    bool
+	VerifyConfig    bool
+	PrintConfig     bool
+	LogConfig       bool
+	ConfigFile      string
+	ConfigExpandEnv bool
+}
+
+func (c *ConfigWrapper) RegisterFlags(f *flag.FlagSet) {
+	f.BoolVar(&c.PrintVersion, "version", false, "Print this builds version information")
+	f.BoolVar(&c.VerifyConfig, "verify-config", false, "Verify config file and exits")
+	f.BoolVar(&c.PrintConfig, "print-config-stderr", false, "Dump the entire Loki config object to stderr")
+	f.BoolVar(&c.LogConfig, "log-config-reverse-order", false, "Dump the entire Loki config object at Info log "+
+		"level with the order reversed, reversing the order makes viewing the entries easier in Grafana.")
+	f.StringVar(&c.ConfigFile, "config.file", "", "yaml file to load")
+	f.BoolVar(&c.ConfigExpandEnv, "config.expand-env", false, "Expands ${var} in config according to the values of the environment variables.")
+	c.Config.RegisterFlags(f)
+}
+
+// Clone takes advantage of pass-by-value semantics to return a distinct *Config.
+// This is primarily used to parse a different flag set without mutating the original *Config.
+func (c *ConfigWrapper) Clone() flagext.Registerer {
+	return func(c ConfigWrapper) *ConfigWrapper {
+		return &c
+	}(*c)
+}
+
+// ApplyDynamicConfig satisfies WithCommonCloneable interface, and applies all rules for setting Loki
+// config values from the common section of the Loki config file.
+// This method's purpose is to simplify Loki's config in an opinionated way so that Loki can be run
+// with the minimal amount of config options for most use cases. It also aims to reduce redundancy where
+// some values are set multiple times through the Loki config.
+func (c *ConfigWrapper) ApplyDynamicConfig() cfg.Source {
+	defaults := ConfigWrapper{}
+	flagext.DefaultValues(&defaults)
+
+	return func(dst cfg.Cloneable) error {
+		r, ok := dst.(*ConfigWrapper)
+		if !ok {
+			return errors.New("dst is not a Loki ConfigWrapper")
+		}
+
+		// Apply all our custom logic here to set values in the Loki config from values in the common config
+		if r.Common.PathPrefix != "" {
+			if r.Ruler.RulePath == defaults.Ruler.RulePath {
+				r.Ruler.RulePath = fmt.Sprintf("%s/rules", r.Common.PathPrefix)
+			}
+
+			if r.Ingester.WAL.Dir == defaults.Ingester.WAL.Dir {
+				r.Ingester.WAL.Dir = fmt.Sprintf("%s/wal", r.Common.PathPrefix)
+			}
+		}
+
+		return nil
+	}
+}

pkg/loki/config_wrapper_test.go

@@ -0,0 +1,87 @@
+package loki
+
+import (
+	"flag"
+	"io/ioutil"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/grafana/loki/pkg/util/cfg"
+)
+
+func Test_CommonConfig(t *testing.T) {
+	testContext := func(configFileString string, args []string) (ConfigWrapper, ConfigWrapper) {
+		config := ConfigWrapper{}
+		fs := flag.NewFlagSet(t.Name(), flag.PanicOnError)
+
+		file, err := ioutil.TempFile("", "config.yaml")
+		defer func() {
+			os.Remove(file.Name())
+		}()
+
+		require.NoError(t, err)
+		_, err = file.WriteString(configFileString)
+		require.NoError(t, err)
+
+		configFileArgs := []string{"-config.file", file.Name()}
+		if args == nil {
+			args = configFileArgs
+		} else {
+			args = append(args, configFileArgs...)
+		}
+		err = cfg.DynamicUnmarshal(&config, args, fs)
+		require.NoError(t, err)
+
+		defaults := ConfigWrapper{}
+		freshFlags := flag.NewFlagSet(t.Name(), flag.PanicOnError)
+		err = cfg.DefaultUnmarshal(&defaults, args, freshFlags)
+		require.NoError(t, err)
+
+		return config, defaults
+	}
+
+	t.Run("common path prefix config", func(t *testing.T) {
+		t.Run("does not override defaults for file paths when not provided", func(t *testing.T) {
+			configFileString := `---`
+			config, defaults := testContext(configFileString, nil)
+
+			assert.EqualValues(t, defaults.Ruler.RulePath, config.Ruler.RulePath)
+			assert.EqualValues(t, defaults.Ingester.WAL.Dir, config.Ingester.WAL.Dir)
+		})
+
+		t.Run("when provided, rewrites all default file paths to use common prefix", func(t *testing.T) {
+			configFileString := `---
+common:
+  path_prefix: /opt/loki`
+			config, _ := testContext(configFileString, nil)
+
+			assert.EqualValues(t, "/opt/loki/rules", config.Ruler.RulePath)
+			assert.EqualValues(t, "/opt/loki/wal", config.Ingester.WAL.Dir)
+		})
+
+		t.Run("does not rewrite custom (non-default) paths passed via config file", func(t *testing.T) {
+			configFileString := `---
+common:
+  path_prefix: /opt/loki
+ruler:
+  rule_path: /etc/ruler/rules`
+			config, _ := testContext(configFileString, nil)
+
+			assert.EqualValues(t, "/etc/ruler/rules", config.Ruler.RulePath)
+			assert.EqualValues(t, "/opt/loki/wal", config.Ingester.WAL.Dir)
+		})
+
+		t.Run("does not rewrite custom (non-default) paths passed via the command line", func(t *testing.T) {
+			configFileString := `---
+common:
+  path_prefix: /opt/loki`
+			config, _ := testContext(configFileString, []string{"-ruler.rule-path", "/etc/ruler/rules"})
+
+			assert.EqualValues(t, "/etc/ruler/rules", config.Ruler.RulePath)
+			assert.EqualValues(t, "/opt/loki/wal", config.Ingester.WAL.Dir)
+		})
+	})
+}

pkg/loki/loki.go

@@ -34,6 +34,7 @@ import (
 	"github.com/grafana/loki/pkg/distributor"
 	"github.com/grafana/loki/pkg/ingester"
 	"github.com/grafana/loki/pkg/ingester/client"
+	"github.com/grafana/loki/pkg/loki/common"
 	"github.com/grafana/loki/pkg/lokifrontend"
 	"github.com/grafana/loki/pkg/querier"
 	"github.com/grafana/loki/pkg/querier/queryrange"
@@ -53,6 +54,7 @@ type Config struct {
 	AuthEnabled bool                   `yaml:"auth_enabled,omitempty"`
 	HTTPPrefix  string                 `yaml:"http_prefix"`
 
+	Common           common.Config            `yaml:"common,omitempty"`
 	Server           server.Config            `yaml:"server,omitempty"`
 	Distributor      distributor.Config       `yaml:"distributor,omitempty"`
 	Querier          querier.Config           `yaml:"querier,omitempty"`

pkg/util/cfg/cfg.go

@@ -2,7 +2,6 @@ package cfg
 
 import (
 	"flag"
-	"os"
 	"reflect"
 
 	"github.com/grafana/dskit/flagext"
@@ -44,27 +43,11 @@ func Unmarshal(dst Cloneable, sources ...Source) error {
 	return nil
 }
 
-// Parse is a higher level wrapper for Unmarshal that automatically parses flags and a .yaml file
-func Parse(dst Cloneable) error {
-	return dParse(dst,
-		dDefaults(flag.CommandLine),
-		YAMLFlag(os.Args[1:], "config.file"),
-		Flags(),
-	)
-}
-
-// dParse is the same as Parse, but with dependency injection for testing
-func dParse(dst Cloneable, defaults, yaml, flags Source) error {
-	// check dst is a pointer
-	v := reflect.ValueOf(dst)
-	if v.Kind() != reflect.Ptr {
-		return ErrNotPointer
-	}
-
-	// unmarshal config
+// DefaultUnmarshal is a higher level wrapper for Unmarshal that automatically parses flags and a .yaml file
+func DefaultUnmarshal(dst Cloneable, args []string, fs *flag.FlagSet) error {
 	return Unmarshal(dst,
-		defaults,
-		yaml,
-		flags,
+		Defaults(fs),
+		YAMLFlag(args, "config.file"),
+		Flags(args, fs),
 	)
 }

pkg/util/cfg/cfg_test.go

@@ -22,8 +22,8 @@ tls:
 	flagSource := dFlags(fs, []string{"-verbose", "-server.port=21"})
 
 	data := Data{}
-	err := dParse(&data,
-		dDefaults(fs),
+	err := Unmarshal(&data,
+		Defaults(fs),
 		yamlSource,
 		flagSource,
 	)
@@ -55,8 +55,8 @@ tls:
 	flagSource := dFlags(fs, []string{"-verbose", "-server.port=21"})
 
 	data := Data{}
-	err := dParse(&data,
-		dDefaults(fs),
+	err := Unmarshal(&data,
+		Defaults(fs),
 		yamlSource,
 		flagSource,
 	)

pkg/util/cfg/dynamic.go

@@ -0,0 +1,36 @@
+package cfg
+
+import (
+	"flag"
+)
+
+// DynamicCloneable must be implemented by config structs that can be dynamically unmarshalled
+type DynamicCloneable interface {
+	Cloneable
+	ApplyDynamicConfig() Source
+}
+
+// DynamicUnmarshal handles populating a config based on the following precedence:
+// 1. Defaults provided by the `RegisterFlags` interface
+// 2. Sections populated by dynamic logic. Configs passed to this function must implement ApplyDynamicConfig()
+// 3. Any config options specified directly in the config file
+// 4. Any config options specified on the command line.
+func DynamicUnmarshal(dst DynamicCloneable, args []string, fs *flag.FlagSet) error {
+	return Unmarshal(dst,
+		// First populate the config with defaults including flags from the command line
+		Defaults(fs),
+		// Next populate the config from the config file, we do this to populate the `common`
+		// section of the config file by taking advantage of the code in YAMLFlag which will load
+		// and process the config file.
+		YAMLFlag(args, "config.file"),
+		// Apply any dynamic logic to set other defaults in the config. This function is called after parsing the
+		// config files so that values from a common, or shared, section can be used in
+		// the dynamic evaluation
+		dst.ApplyDynamicConfig(),
+		// Load configs from the config file a second time, this will supersede anything set by the common
+		// config with values specified in the config file.
+		YAMLFlag(args, "config.file"),
+		// Load the flags again, this will supersede anything set from config file with flags from the command line.
+		Flags(args, fs),
+	)
+}