From b017ede12ebfb024653a29f4fc912a0ef0fee2bc Mon Sep 17 00:00:00 2001 From: Roger Peppe Date: Thu, 16 May 2019 16:15:39 +0100 Subject: [PATCH 1/2] cmd/regula: remove dependency on confita/backend/flags The confita flags package is flawed and will probably be removed in the future, so change to use the flag package directly instead. --- cmd/regula/cli/cli.go | 48 +++++++++++++++++++++++++++++++++++-------- cmd/regula/main.go | 11 +++++++--- 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/cmd/regula/cli/cli.go b/cmd/regula/cli/cli.go index 7dcdd1f..246f475 100644 --- a/cmd/regula/cli/cli.go +++ b/cmd/regula/cli/cli.go @@ -2,16 +2,18 @@ package cli import ( "context" + stdflag "flag" + "fmt" "io" "log" "os" "os/signal" + "strings" "syscall" "time" "github.com/heetch/confita" "github.com/heetch/confita/backend/env" - "github.com/heetch/confita/backend/flags" "github.com/heetch/regula/api/server" isatty "github.com/mattn/go-isatty" "github.com/rs/zerolog" @@ -21,7 +23,7 @@ import ( type Config struct { Etcd struct { Endpoints []string `config:"etcd-endpoints"` - Namespace string `config:"etcd-namespace,required"` + Namespace string `config:"etcd-namespace"` } Server struct { Address string `config:"addr"` @@ -32,22 +34,50 @@ type Config struct { } // LoadConfig loads the configuration from the environment or command line flags. -func LoadConfig() (*Config, error) { +// The args hold the command line arguments, as found in os.Argv. +// It returns flag.ErrHelp if the -help flag is specified on the command line. +func LoadConfig(args []string) (*Config, error) { var cfg Config - cfg.LogLevel = zerolog.DebugLevel.String() + flag := stdflag.NewFlagSet("", stdflag.ContinueOnError) + flag.StringVar(&cfg.Etcd.Namespace, "etcd-namespace", "", "etc namespace to use") + flag.StringVar(&cfg.LogLevel, "log-level", zerolog.DebugLevel.String(), "debug level") cfg.Etcd.Endpoints = []string{"127.0.0.1:2379"} - cfg.Server.Address = "0.0.0.0:5331" - cfg.Server.Timeout = 5 * time.Second - cfg.Server.WatchTimeout = 30 * time.Second + flag.Var(commaSeparatedFlag{&cfg.Etcd.Endpoints}, "etcd-endpoints", "comma separated etc endpoints") + flag.StringVar(&cfg.Server.Address, "addr", "0.0.0.0:5331", "server address to listen on") + flag.DurationVar(&cfg.Server.Timeout, "server-timeout", 5*time.Second, "server timeout (TODO)") + flag.DurationVar(&cfg.Server.WatchTimeout, "server-watch-timeout", 30*time.Second, "server watch timeout (TODO)") - err := confita.NewLoader(env.NewBackend(), flags.NewBackend()).Load(context.Background(), &cfg) + err := confita.NewLoader(env.NewBackend()).Load(context.Background(), &cfg) if err != nil { return nil, err } - + if err := flag.Parse(os.Args[1:]); err != nil { + return nil, err + } + if cfg.Etcd.Namespace == "" { + return nil, fmt.Errorf("etcdnamespace is required (use the -etc-namespace flag to set it)") + } return &cfg, nil } +type commaSeparatedFlag struct { + parts *[]string +} + +func (f commaSeparatedFlag) Set(s string) error { + *f.parts = strings.Split(s, ",") + return nil +} + +func (f commaSeparatedFlag) String() string { + if f.parts == nil { + // Note: the flag package can make a new zero value + // which is how it's possible for parts to be nil. + return "" + } + return strings.Join(*f.parts, ",") +} + // CreateLogger returns a configured logger. func CreateLogger(level string, w io.Writer) zerolog.Logger { logger := zerolog.New(w).With().Timestamp().Logger() diff --git a/cmd/regula/main.go b/cmd/regula/main.go index 5781e31..1df5d5b 100644 --- a/cmd/regula/main.go +++ b/cmd/regula/main.go @@ -1,7 +1,8 @@ package main import ( - "log" + "flag" + "fmt" "os" "time" @@ -12,9 +13,13 @@ import ( ) func main() { - cfg, err := cli.LoadConfig() + cfg, err := cli.LoadConfig(os.Args) if err != nil { - log.Fatal(err) + if err == flag.ErrHelp { + os.Exit(0) + } + fmt.Fprintf(os.Stderr, "regula: %v\n", err) + os.Exit(2) } logger := cli.CreateLogger(cfg.LogLevel, os.Stderr) From 393cd672e307c0e4b35af36587d868a6e1c4e30a Mon Sep 17 00:00:00 2001 From: Roger Peppe Date: Fri, 17 May 2019 13:05:12 +0100 Subject: [PATCH 2/2] Apply suggestions from code review Co-Authored-By: Yasss Co-Authored-By: Asdine El Hrychy --- cmd/regula/cli/cli.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/regula/cli/cli.go b/cmd/regula/cli/cli.go index 246f475..933f460 100644 --- a/cmd/regula/cli/cli.go +++ b/cmd/regula/cli/cli.go @@ -39,10 +39,10 @@ type Config struct { func LoadConfig(args []string) (*Config, error) { var cfg Config flag := stdflag.NewFlagSet("", stdflag.ContinueOnError) - flag.StringVar(&cfg.Etcd.Namespace, "etcd-namespace", "", "etc namespace to use") + flag.StringVar(&cfg.Etcd.Namespace, "etcd-namespace", "", "etcd namespace to use") flag.StringVar(&cfg.LogLevel, "log-level", zerolog.DebugLevel.String(), "debug level") cfg.Etcd.Endpoints = []string{"127.0.0.1:2379"} - flag.Var(commaSeparatedFlag{&cfg.Etcd.Endpoints}, "etcd-endpoints", "comma separated etc endpoints") + flag.Var(commaSeparatedFlag{&cfg.Etcd.Endpoints}, "etcd-endpoints", "comma separated etcd endpoints") flag.StringVar(&cfg.Server.Address, "addr", "0.0.0.0:5331", "server address to listen on") flag.DurationVar(&cfg.Server.Timeout, "server-timeout", 5*time.Second, "server timeout (TODO)") flag.DurationVar(&cfg.Server.WatchTimeout, "server-watch-timeout", 30*time.Second, "server watch timeout (TODO)") @@ -51,7 +51,7 @@ func LoadConfig(args []string) (*Config, error) { if err != nil { return nil, err } - if err := flag.Parse(os.Args[1:]); err != nil { + if err := flag.Parse(args[1:]); err != nil { return nil, err } if cfg.Etcd.Namespace == "" {