Merge pull request #5543 from urso/feature/common-flags

Improve custom config flag handlers

CHANGELOG.asciidoc

@@ -88,6 +88,7 @@ https://github.com/elastic/beats/compare/v6.0.0-beta2...master[Check the HEAD di
 - Add Azure VM support for add_cloud_metadata processor {pull}5355[5355]
 - Add `output.file.permission` config option. {pull}4638[4638]
 - Refactor add_kubernetes_metadata to support autodiscovery {pull}5434[5434]
+- Improve custom flag handling and CLI flags usage message. {pull}5543[5543]
 
 *Auditbeat*
 

filebeat/fileset/flags.go

@@ -11,7 +11,7 @@ import (
 // Modules related command line flags.
 var (
 	modulesFlag     = flag.String("modules", "", "List of enabled modules (comma separated)")
-	moduleOverrides = common.NewFlagConfig(nil, nil, "M", "Module configuration overwrite")
+	moduleOverrides = common.SettingFlag(nil, "M", "Module configuration overwrite")
 )
 
 type ModuleOverrides map[string]map[string]*common.Config // module -> fileset -> Config

libbeat/cfgfile/cfgfile.go

@@ -15,8 +15,8 @@ var (
 	// The default config cannot include the beat name as it is not initialized
 	// when this variable is created. See ChangeDefaultCfgfileFlag which should
 	// be called prior to flags.Parse().
-	configfiles = flagArgList("c", "beat.yml", "Configuration file, relative to path.config")
-	overwrites  = common.NewFlagConfig(nil, nil, "E", "Configuration overwrite")
+	configfiles = common.StringArrFlag(nil, "c", "beat.yml", "Configuration file, relative to path.config")
+	overwrites  = common.SettingFlag(nil, "E", "Configuration overwrite")
 	testConfig  = flag.Bool("configtest", false, "Test configuration and exit.")
 
 	// Additional default settings, that must be available for variable expansion
@@ -37,7 +37,7 @@ var (
 func init() {
 	// add '-path.x' options overwriting paths in 'overwrites' config
 	makePathFlag := func(name, usage string) *string {
-		return common.NewFlagOverwrite(nil, overwrites, name, name, "", usage)
+		return common.ConfigOverwriteFlag(nil, overwrites, name, name, "", usage)
 	}
 
 	homePath = makePathFlag("path.home", "Home path")
@@ -107,7 +107,7 @@ func Load(path string) (*common.Config, error) {
 
 	if path == "" {
 		list := []string{}
-		for _, cfg := range configfiles.list {
+		for _, cfg := range configfiles.List() {
 			if !filepath.IsAbs(cfg) {
 				list = append(list, filepath.Join(cfgpath, cfg))
 			} else {

libbeat/cmd/instance/beat.go

@@ -364,12 +364,6 @@ func (b *Beat) Setup(bt beat.Creator, template, dashboards, machineLearning bool
 // handleFlags parses the command line flags. It handles the '-version' flag
 // and invokes the HandleFlags callback if implemented by the Beat.
 func (b *Beat) handleFlags() error {
-	// Due to a dependence upon the beat name, the default config file path
-	// must be updated prior to CLI flag handling.
-	err := cfgfile.ChangeDefaultCfgfileFlag(b.Info.Beat)
-	if err != nil {
-		return fmt.Errorf("failed to set default config file path: %v", err)
-	}
 	flag.Parse()
 
 	if printVersion {

libbeat/cmd/root.go

@@ -2,13 +2,15 @@ package cmd
 
 import (
 	"flag"
+	"fmt"
 	"os"
 	"strings"
 
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 
 	"github.com/elastic/beats/libbeat/beat"
+	"github.com/elastic/beats/libbeat/cfgfile"
 )
 
 func init() {
@@ -51,6 +53,14 @@ func GenRootCmdWithIndexPrefixWithRunFlags(name, indexPrefix, version string, be
 	rootCmd := &BeatsRootCmd{}
 	rootCmd.Use = name
 
+	// Due to a dependence upon the beat name, the default config file path
+	err := cfgfile.ChangeDefaultCfgfileFlag(name)
+	if err != nil {
+		panic(fmt.Errorf("failed to set default config file path: %v", err))
+	}
+
+	// must be updated prior to CLI flag handling.
+
 	rootCmd.RunCmd = genRunCmd(name, indexPrefix, version, beatCreator, runFlags)
 	rootCmd.SetupCmd = genSetupCmd(name, indexPrefix, version, beatCreator)
 	rootCmd.VersionCmd = genVersionCmd(name, version)

libbeat/common/config.go

@@ -10,12 +10,12 @@ import (
 	"runtime"
 	"strings"
 
+	ucfg "github.com/elastic/go-ucfg"
+	"github.com/elastic/go-ucfg/yaml"
+
 	"github.com/elastic/beats/libbeat/common/file"
 	"github.com/elastic/beats/libbeat/logp"
-	ucfg "github.com/elastic/go-ucfg"
 	"github.com/elastic/go-ucfg/cfgutil"
-	cfgflag "github.com/elastic/go-ucfg/flag"
-	"github.com/elastic/go-ucfg/yaml"
 )
 
 var flagStrictPerms = flag.Bool("strict.perms", true, "Strict permission checking on config files")
@@ -39,12 +39,6 @@ type ConfigNamespace struct {
 	config *Config
 }
 
-type flagOverwrite struct {
-	config *ucfg.Config
-	path   string
-	value  string
-}
-
 var configOpts = []ucfg.Option{
 	ucfg.PathSep("."),
 	ucfg.ResolveEnv,
@@ -102,62 +96,6 @@ func NewConfigWithYAML(in []byte, source string) (*Config, error) {
 	return fromConfig(c), err
 }
 
-func NewFlagConfig(
-	set *flag.FlagSet,
-	def *Config,
-	name string,
-	usage string,
-) *Config {
-	opts := append(
-		[]ucfg.Option{
-			ucfg.MetaData(ucfg.Meta{Source: "command line flag"}),
-		},
-		configOpts...,
-	)
-
-	var to *ucfg.Config
-	if def != nil {
-		to = def.access()
-	}
-
-	config := cfgflag.ConfigVar(set, to, name, usage, opts...)
-	return fromConfig(config)
-}
-
-func NewFlagOverwrite(
-	set *flag.FlagSet,
-	config *Config,
-	name, path, def, usage string,
-) *string {
-	if config == nil {
-		panic("Missing configuration")
-	}
-	if path == "" {
-		panic("empty path")
-	}
-
-	if def != "" {
-		err := config.SetString(path, -1, def)
-		if err != nil {
-			panic(err)
-		}
-	}
-
-	f := &flagOverwrite{
-		config: config.access(),
-		path:   path,
-		value:  def,
-	}
-
-	if set == nil {
-		flag.Var(f, name, usage)
-	} else {
-		set.Var(f, name, usage)
-	}
-
-	return &f.value
-}
-
 func LoadFile(path string) (*Config, error) {
 	if IsStrictPerms() {
 		if err := ownerHasExclusiveWritePerms(path); err != nil {
@@ -304,30 +242,6 @@ func (c *Config) GetFields() []string {
 	return c.access().GetFields()
 }
 
-func (f *flagOverwrite) String() string {
-	return f.value
-}
-
-func (f *flagOverwrite) Set(v string) error {
-	opts := append(
-		[]ucfg.Option{
-			ucfg.MetaData(ucfg.Meta{Source: "command line flag"}),
-		},
-		configOpts...,
-	)
-
-	err := f.config.SetString(f.path, -1, v, opts...)
-	if err != nil {
-		return err
-	}
-	f.value = v
-	return nil
-}
-
-func (f *flagOverwrite) Get() interface{} {
-	return f.value
-}
-
 // Unpack unpacks a configuration with at most one sub object. An sub object is
 // ignored if it is disabled by setting `enabled: false`. If the configuration
 // passed contains multiple active sub objects, Unpack will return an error.