RHCLOUD-34576 Loads both config and command line arguments

cmd/migrate/migrate.go

@@ -34,7 +34,5 @@ func NewCommand(options *storage.Options, logger log.Logger) *cobra.Command {
 		},
 	}
 
-	options.AddFlags(cmd.Flags(), "storage")
-
 	return cmd
 }

cmd/root.go

@@ -42,19 +42,6 @@ var (
 		Use:     Name,
 		Version: Version,
 		Short:   "A simple common inventory system",
-		PreRunE: func(cmd *cobra.Command, args []string) error {
-			if err := viper.ReadInConfig(); err != nil {
-				return err
-			} else {
-				msg := fmt.Sprintf("Using config file: %s", viper.ConfigFileUsed())
-				logger.Debug(msg)
-			}
-
-			// put the values into the options struct.
-			err := viper.Unmarshal(&options)
-
-			return err
-		},
 	}
 
 	options = struct {
@@ -88,7 +75,16 @@ func init() {
 
 	configHelp := fmt.Sprintf("config file (default is $PWD/.%s.yaml)", Name)
 	rootCmd.PersistentFlags().StringVar(&cfgFile, "config", "", configHelp)
-	err := viper.BindPFlag("config", rootCmd.PersistentFlags().Lookup("config"))
+
+	// options.Storage is used in both commands
+	// viper/cobra do not handle well when the same key is used in multiple sub-commands
+	// and the last one takes precedence
+	// Set them as persistent flags in the main command
+	// Another solution might involve into creating our own set of flags to prevent having two separate objects for
+	// the same flag.
+	options.Storage.AddFlags(rootCmd.PersistentFlags(), "storage")
+
+	err := viper.BindPFlags(rootCmd.PersistentFlags())
 	if err != nil {
 		panic(err)
 	}
@@ -135,4 +131,15 @@ func initConfig() {
 
 	viper.SetEnvPrefix(Name)
 	viper.AutomaticEnv()
+
+	if err := viper.ReadInConfig(); err != nil {
+		panic(err)
+	} else {
+		logger.Infof("Using config file: %s", viper.ConfigFileUsed())
+	}
+
+	// put the values into the options struct.
+	if err := viper.Unmarshal(&options); err != nil {
+		panic(err)
+	}
 }

cmd/root_test.go

@@ -20,7 +20,7 @@ func (m *MockedCommandRun) RunE(cmd *cobra.Command, args []string) error {
 func setupMockRunE() map[string]*MockedCommandRun {
 	mocks := make(map[string]*MockedCommandRun)
 
-	for _, cmd := range rootCmd.Commands() {
+	for _, cmd := range append(rootCmd.Commands(), rootCmd) {
 		mockedCommandRunE := new(MockedCommandRun)
 		mockedCommandRunE.On("RunE", mock.Anything, mock.Anything).Return(nil)
 		mocks[cmd.Name()] = mockedCommandRunE
@@ -43,10 +43,10 @@ func assertCommandCalled(t *testing.T, command string, mocked map[string]*Mocked
 }
 
 func TestRootCommand(t *testing.T) {
-	commands := []string{"migrate", "serve"}
+	commands := []string{"migrate", "serve", ""} // root command
 	for _, command := range commands {
 		t.Run(command+" by setting storage.database to postgres", func(t *testing.T) {
-			rootCmd.SetArgs([]string{command, "--storage.database=postgres"})
+			rootCmd.SetArgs([]string{command, "--config", "../.inventory-api.yaml", "--storage.database=postgres"})
 
 			mocked := setupMockRunE()
 			assert.Nil(t, rootCmd.Execute())
@@ -56,3 +56,10 @@ func TestRootCommand(t *testing.T) {
 		})
 	}
 }
+
+func TestInvalidConfigFile(t *testing.T) {
+	rootCmd.SetArgs([]string{"migrate", "--config", "not-found"})
+	assert.Panics(t, func() {
+		_ = rootCmd.Execute()
+	})
+}

cmd/serve/serve.go

@@ -205,7 +205,6 @@ func NewCommand(
 	}
 
 	serverOptions.AddFlags(cmd.Flags(), "server")
-	storageOptions.AddFlags(cmd.Flags(), "storage")
 	authnOptions.AddFlags(cmd.Flags(), "authn")
 	authzOptions.AddFlags(cmd.Flags(), "authz")
 	eventingOptions.AddFlags(cmd.Flags(), "eventing")