From 003135087d49cdfa390b7787695d5c52ded39aa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josejulio=20Mart=C3=ADnez?= Date: Thu, 15 Aug 2024 14:30:17 -0600 Subject: [PATCH 1/2] RHCLOUD-34576 Loads both config and command line arguments The original fix was using the rootcmd prerun - but that is not executed when using a sub command. Config was not being loaded at all. The original bug still aplies, repeated flags are overriden. The flags are honored just before loading the config from viper. But once It is loaded the overriden repeated flags are set to the default values. Adding storage.* flags as PersistentFlags since is used in both commands. --- cmd/migrate/migrate.go | 2 -- cmd/root.go | 35 +++++++++++++++++++++-------------- cmd/root_test.go | 13 ++++++++++--- cmd/serve/serve.go | 1 - 4 files changed, 31 insertions(+), 20 deletions(-) diff --git a/cmd/migrate/migrate.go b/cmd/migrate/migrate.go index e5514a9a..4bbc42a1 100644 --- a/cmd/migrate/migrate.go +++ b/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 } diff --git a/cmd/root.go b/cmd/root.go index 7b5b2144..5266f086 100644 --- a/cmd/root.go +++ b/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) + } } diff --git a/cmd/root_test.go b/cmd/root_test.go index 84497339..f52019a6 100644 --- a/cmd/root_test.go +++ b/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() + }) +} diff --git a/cmd/serve/serve.go b/cmd/serve/serve.go index 244ebdac..8ffb238f 100644 --- a/cmd/serve/serve.go +++ b/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") From 31d60e1306177bcb6a24d2c95e0c903398df2ce9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josejulio=20Mart=C3=ADnez?= Date: Thu, 15 Aug 2024 14:49:49 -0600 Subject: [PATCH 2/2] fix lint --- cmd/root_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/root_test.go b/cmd/root_test.go index f52019a6..91d3e708 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -60,6 +60,6 @@ func TestRootCommand(t *testing.T) { func TestInvalidConfigFile(t *testing.T) { rootCmd.SetArgs([]string{"migrate", "--config", "not-found"}) assert.Panics(t, func() { - rootCmd.Execute() + _ = rootCmd.Execute() }) }