Fix/promtail yaml config (#2471)

* promtail config cloneable

* config parsing compile time checks against cloneable

cmd/loki/main.go

@@ -50,9 +50,9 @@ func (c *Config) RegisterFlags(f *flag.FlagSet) {
 // 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 flagext.Registerer(func(c Config) *Config {
+	return func(c Config) *Config {
 		return &c
-	}(*c))
+	}(*c)
 }
 
 func main() {

cmd/promtail/main.go

@@ -9,6 +9,7 @@ import (
 	"k8s.io/klog"
 
 	"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"
@@ -26,22 +27,44 @@ func init() {
 	prometheus.MustRegister(version.NewCollector("promtail"))
 }
 
-func main() {
-	printVersion := flag.Bool("version", false, "Print this builds version information")
-	dryRun := flag.Bool("dry-run", false, "Start Promtail but print entries instead of sending them to Loki.")
-	printConfig := flag.Bool("print-config-stderr", false, "Dump the entire Loki config object to stderr")
-	logConfig := flag.Bool("log-config-reverse-order", false, "Dump the entire Loki config object at Info log "+
+type Config struct {
+	config.Config `yaml:",inline"`
+	printVersion  bool
+	printConfig   bool
+	logConfig     bool
+	dryRun        bool
+	configFile    string
+}
+
+func (c *Config) RegisterFlags(f *flag.FlagSet) {
+	f.BoolVar(&c.printVersion, "version", false, "Print this builds version information")
+	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.BoolVar(&c.dryRun, "dry-run", false, "Start Promtail but print entries instead of sending them to Loki.")
+	f.StringVar(&c.configFile, "config.file", "", "yaml file to load")
+	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() {
 
 	// Load config, merging config file and CLI flags
-	var config config.Config
+	var config Config
 	if err := cfg.Parse(&config); err != nil {
 		fmt.Println("Unable to parse config:", err)
 		os.Exit(1)
 	}
 
 	// Handle -version CLI flag
-	if *printVersion {
+	if config.printVersion {
 		fmt.Println(version.Print("promtail"))
 		os.Exit(0)
 	}
@@ -62,21 +85,21 @@ func main() {
 		stages.Debug = true
 	}
 
-	if *printConfig {
+	if config.printConfig {
 		err := logutil.PrintConfig(os.Stderr, &config)
 		if err != nil {
 			level.Error(util.Logger).Log("msg", "failed to print config to stderr", "err", err.Error())
 		}
 	}
 
-	if *logConfig {
+	if config.logConfig {
 		err := logutil.LogConfig(&config)
 		if err != nil {
 			level.Error(util.Logger).Log("msg", "failed to log config object", "err", err.Error())
 		}
 	}
 
-	p, err := promtail.New(config, *dryRun)
+	p, err := promtail.New(config.Config, config.dryRun)
 	if err != nil {
 		level.Error(util.Logger).Log("msg", "error creating promtail", "error", err)
 		os.Exit(1)

pkg/cfg/cfg.go

@@ -1,9 +1,11 @@
 package cfg
 
 import (
+	"flag"
 	"os"
 	"reflect"
 
+	"github.com/cortexproject/cortex/pkg/util/flagext"
 	"github.com/pkg/errors"
 )
 
@@ -12,15 +14,21 @@ import (
 // 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
+type Source func(Cloneable) error
+
+// Cloneable is a config which can be cloned into a flagext.Registerer
+// Contract: the cloned value must not mutate the original.
+type Cloneable interface {
+	Clone() flagext.Registerer
+}
 
 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 {
+func Unmarshal(dst Cloneable, 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!")
 	}
@@ -37,16 +45,16 @@ func Unmarshal(dst interface{}, sources ...Source) error {
 }
 
 // Parse is a higher level wrapper for Unmarshal that automatically parses flags and a .yaml file
-func Parse(dst interface{}) error {
+func Parse(dst Cloneable) error {
 	return dParse(dst,
-		Defaults(),
+		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 interface{}, defaults, yaml, flags Source) error {
+func dParse(dst Cloneable, defaults, yaml, flags Source) error {
 	// check dst is a pointer
 	v := reflect.ValueOf(dst)
 	if v.Kind() != reflect.Ptr {

pkg/cfg/data_test.go

@@ -3,6 +3,8 @@ package cfg
 import (
 	"flag"
 	"time"
+
+	"github.com/cortexproject/cortex/pkg/util/flagext"
 )
 
 // Data is a test Data structure
@@ -12,6 +14,14 @@ type Data struct {
 	TLS     TLS    `yaml:"tls"`
 }
 
+// Clone takes advantage of pass-by-value semantics to return a distinct *Data.
+// This is primarily used to parse a different flag set without mutating the original *Data.
+func (d *Data) Clone() flagext.Registerer {
+	return func(d Data) *Data {
+		return &d
+	}(*d)
+}
+
 type Server struct {
 	Port    int           `yaml:"port"`
 	Timeout time.Duration `yaml:"timeout"`

pkg/cfg/files.go

@@ -7,14 +7,13 @@ import (
 	"io/ioutil"
 	"os"
 
-	"github.com/cortexproject/cortex/pkg/util/flagext"
 	"github.com/pkg/errors"
 	"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 {
+	return func(dst Cloneable) error {
 		if f == nil {
 			return nil
 		}
@@ -31,14 +30,14 @@ func JSON(f *string) Source {
 
 // dJSON returns a JSON source and allows dependency injection
 func dJSON(y []byte) Source {
-	return func(dst interface{}) error {
+	return func(dst Cloneable) 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 {
+	return func(dst Cloneable) error {
 		y, err := ioutil.ReadFile(f)
 		if err != nil {
 			return err
@@ -51,27 +50,19 @@ func YAML(f string) Source {
 
 // dYAML returns a YAML source and allows dependency injection
 func dYAML(y []byte) Source {
-	return func(dst interface{}) error {
+	return func(dst Cloneable) error {
 		return yaml.UnmarshalStrict(y, dst)
 	}
 }
 
 func YAMLFlag(args []string, name string) Source {
-	type cloneable interface {
-		Clone() flagext.Registerer
-	}
 
-	return func(dst interface{}) error {
+	return func(dst Cloneable) error {
 		freshFlags := flag.NewFlagSet("config-file-loader", flag.ContinueOnError)
 
-		c, ok := dst.(cloneable)
-		if !ok {
-			return errors.New("dst does not satisfy cloneable")
-		}
-
 		// Ensure we register flags on a copy of the config so as to not mutate it while
 		// parsing out the config file location.
-		c.Clone().RegisterFlags(freshFlags)
+		dst.Clone().RegisterFlags(freshFlags)
 
 		usage := freshFlags.Usage
 		freshFlags.Usage = func() { /* don't do anything by default, we will print usage ourselves, but only when requested. */ }

pkg/cfg/flag.go

@@ -11,15 +11,9 @@ import (
 	"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 {
+	return func(dst Cloneable) error {
 		r, ok := dst.(flagext.Registerer)
 		if !ok {
 			return errors.New("dst does not satisfy flagext.Registerer")
@@ -40,7 +34,7 @@ func Flags() Source {
 
 // dFlags parses the flagset, applying all values set on the slice
 func dFlags(fs *flag.FlagSet, args []string) Source {
-	return func(dst interface{}) error {
+	return func(dst Cloneable) error {
 		// parse the final flagset
 		return fs.Parse(args)
 	}

pkg/logcli/query/query.go

@@ -3,6 +3,7 @@ package query
 import (
 	"context"
 	"errors"
+	"flag"
 	"fmt"
 	"log"
 	"os"
@@ -106,9 +107,7 @@ func (q *Query) printResult(value loghttp.ResultValue, out output.LogOutput) {
 func (q *Query) DoLocalQuery(out output.LogOutput, statistics bool, orgID string) error {
 
 	var conf loki.Config
-	if err := cfg.Defaults()(&conf); err != nil {
-		return err
-	}
+	conf.RegisterFlags(flag.CommandLine)
 	if q.LocalConfig == "" {
 		return errors.New("no supplied config file")
 	}

pkg/loki/loki.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"net/http"
 
+	"github.com/cortexproject/cortex/pkg/util/flagext"
 	"github.com/cortexproject/cortex/pkg/util/modules"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/weaveworks/common/signals"
@@ -88,6 +89,14 @@ func (c *Config) RegisterFlags(f *flag.FlagSet) {
 	c.Tracing.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)
+}
+
 // Validate the config and returns an error if the validation
 // doesn't pass
 func (c *Config) Validate(log log.Logger) error {