Use visitor to bind flags to Viper (#92)

Simplifies the logic and no longer requires that flags be listed individually in order to get bound. Signed-off-by: Reinhard Nägele <[email protected]>
helm · Jan 24, 2019 · 9e980f8 · 9e980f8
1 parent 868038e
commit 9e980f8
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 36 deletions.
diff --git a/app/cmd/install.go b/app/cmd/install.go
@@ -18,8 +18,6 @@ import (
 	"fmt"
 	"os"
 
-	"github.com/spf13/viper"
-
 	"github.com/MakeNowJust/heredoc"
 	"github.com/helm/chart-testing/pkg/chart"
 	"github.com/helm/chart-testing/pkg/config"
@@ -74,7 +72,7 @@ func addInstallFlags(flags *flag.FlagSet) {
 func install(cmd *cobra.Command, args []string) {
 	fmt.Println("Installing charts...")
 
-	configuration, err := config.LoadConfiguration(cfgFile, cmd, bindRootFlags, bindInstallFlags)
+	configuration, err := config.LoadConfiguration(cfgFile, cmd)
 	if err != nil {
 		fmt.Printf("Error loading configuration: %s\n", err)
 		os.Exit(1)
@@ -94,8 +92,3 @@ func install(cmd *cobra.Command, args []string) {
 		os.Exit(1)
 	}
 }
-
-func bindInstallFlags(flagSet *flag.FlagSet, v *viper.Viper) error {
-	options := []string{"build-id", "helm-extra-args", "namespace", "release-label"}
-	return bindFlags(options, flagSet, v)
-}
diff --git a/app/cmd/lint.go b/app/cmd/lint.go
@@ -23,7 +23,6 @@ import (
 	"github.com/helm/chart-testing/pkg/config"
 	"github.com/spf13/cobra"
 	flag "github.com/spf13/pflag"
-	"github.com/spf13/viper"
 )
 
 func newLintCmd() *cobra.Command {
@@ -76,7 +75,7 @@ func addLintFlags(flags *flag.FlagSet) {
 func lint(cmd *cobra.Command, args []string) {
 	fmt.Println("Linting charts...")
 
-	configuration, err := config.LoadConfiguration(cfgFile, cmd, bindRootFlags, bindLintFlags)
+	configuration, err := config.LoadConfiguration(cfgFile, cmd)
 	if err != nil {
 		fmt.Printf("Error loading configuration: %s\n", err)
 		os.Exit(1)
@@ -97,7 +96,3 @@ func lint(cmd *cobra.Command, args []string) {
 	}
 }
 
-func bindLintFlags(flagSet *flag.FlagSet, v *viper.Viper) error {
-	options := []string{"lint-conf", "chart-yaml-schema", "validate-maintainers", "check-version-increment", "validate-chart-schema", "validate-yaml"}
-	return bindFlags(options, flagSet, v)
-}
diff --git a/app/cmd/lintAndInstall.go b/app/cmd/lintAndInstall.go
@@ -43,7 +43,7 @@ func newLintAndInstallCmd() *cobra.Command {
 func lintAndInstall(cmd *cobra.Command, args []string) {
 	fmt.Println("Linting and installing charts...")
 
-	configuration, err := config.LoadConfiguration(cfgFile, cmd, bindRootFlags, bindLintFlags, bindInstallFlags)
+	configuration, err := config.LoadConfiguration(cfgFile, cmd)
 	if err != nil {
 		fmt.Printf("Error loading configuration: %s\n", err)
 		os.Exit(1)

diff --git a/app/cmd/root.go b/app/cmd/root.go
@@ -21,8 +21,6 @@ import (
 	"github.com/MakeNowJust/heredoc"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
-	flag "github.com/spf13/pflag"
-	"github.com/spf13/viper"
 )
 
 var (
@@ -90,17 +88,3 @@ func addCommonLintAndInstallFlags(flags *pflag.FlagSet) {
 		Print CLI calls of external tools to stdout (Note: depending on helm-extra-args
 		passed, this may reveal sensitive data)`))
 }
-
-func bindFlags(options []string, flagSet *flag.FlagSet, v *viper.Viper) error {
-	for _, option := range options {
-		if err := v.BindPFlag(option, flagSet.Lookup(option)); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
-func bindRootFlags(flagSet *flag.FlagSet, v *viper.Viper) error {
-	options := []string{"remote", "target-branch", "all", "charts", "chart-dirs", "chart-repos", "helm-repo-extra-args", "excluded-charts", "debug"}
-	return bindFlags(options, flagSet, v)
-}
diff --git a/pkg/config/config.go b/pkg/config/config.go
@@ -60,13 +60,18 @@ type Configuration struct {
 	ReleaseLabel          string   `mapstructure:"release-label"`
 }
 
-func LoadConfiguration(cfgFile string, cmd *cobra.Command, bindFlagsFunc ...func(flagSet *flag.FlagSet, viper *viper.Viper) error) (*Configuration, error) {
+func LoadConfiguration(cfgFile string, cmd *cobra.Command) (*Configuration, error) {
 	v := viper.New()
-	for _, bindFunc := range bindFlagsFunc {
-		if err := bindFunc(cmd.Flags(), v); err != nil {
-			return nil, errors.Wrap(err, "Error binding flags")
+
+	cmd.Flags().VisitAll(func(flag *flag.Flag) {
+		flagName := flag.Name
+		if flagName != "config" && flagName != "help" {
+			if err := v.BindPFlag(flagName, flag); err != nil {
+				// can't really happen
+				panic(fmt.Sprintln(errors.Wrapf(err, "Error binding flag '%s'", flagName)))
+			}
 		}
-	}
+	})
 
 	v.AutomaticEnv()
 	v.SetEnvKeyReplacer(strings.NewReplacer("-", "_"))