From 4640d826fc26867d272baaf120346eaa595d499d Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Fri, 22 Nov 2024 04:16:06 +0530 Subject: [PATCH 1/8] use otel config tls Signed-off-by: chahatsagarmain --- cmd/es-rollover/app/actions.go | 48 ++++++++++++++++++++++------- cmd/es-rollover/app/actions_test.go | 13 ++++---- cmd/es-rollover/main.go | 29 +++++++++-------- 3 files changed, 57 insertions(+), 33 deletions(-) diff --git a/cmd/es-rollover/app/actions.go b/cmd/es-rollover/app/actions.go index 8dbeff30e95..cecf50eab86 100644 --- a/cmd/es-rollover/app/actions.go +++ b/cmd/es-rollover/app/actions.go @@ -4,14 +4,16 @@ package app import ( + "context" "crypto/tls" + "flag" "net/http" "time" "github.com/spf13/viper" + "go.opentelemetry.io/collector/config/configtls" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/pkg/config/tlscfg" "github.com/jaegertracing/jaeger/pkg/es/client" ) @@ -35,30 +37,54 @@ type Action interface { Do() error } +type ClientConfig struct { + configtls.ClientConfig `mapstructure:",squash"` + Enabled bool +} + +func (c *ClientConfig) AddFlags(flags *flag.FlagSet) { + flags.BoolVar(&c.Enabled, "es.tls.enabled", false, "Enable TLS when talking to the remote server(s)") + flags.StringVar(&c.CAFile, "es.tls.ca", "", "Path to a TLS CA (Certification Authority) file used to verify the remote server(s) (by default will use the system truststore)") + flags.StringVar(&c.CertFile, "es.tls.cert", "", "Path to a TLS Certificate file, used to identify this process to the remote server(s)") + flags.StringVar(&c.KeyFile, "es.tls.key", "", "Path to a TLS Private Key file, used to identify this process to the remote server(s)") + flags.StringVar(&c.ServerName, "es.tls.server-name", "", "Override the TLS server name we expect in the certificate of the remote server(s)") + flags.BoolVar(&c.InsecureSkipVerify, "es.tls.skip-host-verify", false, "(insecure) Skip server's certificate chain and host name verification") +} + // ActionExecuteOptions are the options passed to the execute action function type ActionExecuteOptions struct { - Args []string - Viper *viper.Viper - Logger *zap.Logger - TLSFlags tlscfg.ClientFlagsConfig + Args []string + Viper *viper.Viper + Logger *zap.Logger + TLSConfig *ClientConfig } // ActionCreatorFunction type is the function type in charge of create the action to be executed type ActionCreatorFunction func(client.Client, Config) Action +func getTLSConfig(tlsConfig *ClientConfig, logger *zap.Logger) (*tls.Config, error) { + if tlsConfig == nil { + return nil, nil + } + + if tlsConfig.Insecure { + logger.Info("TLS is disabled") + return nil, nil + } + + ctx := context.Background() + + return tlsConfig.LoadTLSConfig(ctx) +} + // ExecuteAction execute the action returned by the createAction function func ExecuteAction(opts ActionExecuteOptions, createAction ActionCreatorFunction) error { cfg := Config{} cfg.InitFromViper(opts.Viper) - tlsOpts, err := opts.TLSFlags.InitFromViper(opts.Viper) - if err != nil { - return err - } - tlsCfg, err := tlsOpts.Config(opts.Logger) + tlsCfg, err := getTLSConfig(opts.TLSConfig, opts.Logger) if err != nil { return err } - defer tlsOpts.Close() esClient := newESClient(opts.Args[0], &cfg, tlsCfg) action := createAction(esClient, cfg) diff --git a/cmd/es-rollover/app/actions_test.go b/cmd/es-rollover/app/actions_test.go index e4b7a33c34f..c0872402e55 100644 --- a/cmd/es-rollover/app/actions_test.go +++ b/cmd/es-rollover/app/actions_test.go @@ -15,7 +15,6 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/pkg/config/tlscfg" "github.com/jaegertracing/jaeger/pkg/es/client" ) @@ -74,10 +73,10 @@ func TestExecuteAction(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { v := viper.New() - tlsFlags := tlscfg.ClientFlagsConfig{Prefix: "es"} + tlfConfig := &ClientConfig{} command := cobra.Command{} flags := &flag.FlagSet{} - tlsFlags.AddFlags(flags) + tlfConfig.AddFlags(flags) command.PersistentFlags().AddGoFlagSet(flags) v.BindPFlags(command.PersistentFlags()) cmdLine := append([]string{"--es.tls.enabled=true"}, test.flags...) @@ -85,10 +84,10 @@ func TestExecuteAction(t *testing.T) { require.NoError(t, err) executedAction := false err = ExecuteAction(ActionExecuteOptions{ - Args: args, - Viper: v, - Logger: logger, - TLSFlags: tlsFlags, + Args: args, + Viper: v, + Logger: logger, + TLSConfig: tlfConfig, }, func(c client.Client, _ Config) Action { assert.Equal(t, "https://localhost:9300", c.Endpoint) transport, ok := c.Client.Transport.(*http.Transport) diff --git a/cmd/es-rollover/main.go b/cmd/es-rollover/main.go index 5fe026d6e68..c70ac73287b 100644 --- a/cmd/es-rollover/main.go +++ b/cmd/es-rollover/main.go @@ -16,7 +16,6 @@ import ( "github.com/jaegertracing/jaeger/cmd/es-rollover/app/lookback" "github.com/jaegertracing/jaeger/cmd/es-rollover/app/rollover" "github.com/jaegertracing/jaeger/pkg/config" - "github.com/jaegertracing/jaeger/pkg/config/tlscfg" "github.com/jaegertracing/jaeger/pkg/es/client" ) @@ -30,7 +29,7 @@ func main() { Long: "Jaeger es-rollover manages Jaeger indices", } - tlsFlags := tlscfg.ClientFlagsConfig{Prefix: "es"} + tlsConfig := &app.ClientConfig{} // Init command initCfg := &initialize.Config{} @@ -42,10 +41,10 @@ func main() { SilenceUsage: true, RunE: func(_ *cobra.Command, args []string) error { return app.ExecuteAction(app.ActionExecuteOptions{ - Args: args, - Viper: v, - Logger: logger, - TLSFlags: tlsFlags, + Args: args, + Viper: v, + Logger: logger, + TLSConfig: tlsConfig, }, func(c client.Client, cfg app.Config) app.Action { initCfg.Config = cfg initCfg.InitFromViper(v) @@ -80,10 +79,10 @@ func main() { RunE: func(_ *cobra.Command, args []string) error { rolloverCfg.InitFromViper(v) return app.ExecuteAction(app.ActionExecuteOptions{ - Args: args, - Viper: v, - Logger: logger, - TLSFlags: tlsFlags, + Args: args, + Viper: v, + Logger: logger, + TLSConfig: tlsConfig, }, func(c client.Client, cfg app.Config) app.Action { rolloverCfg.Config = cfg rolloverCfg.InitFromViper(v) @@ -109,10 +108,10 @@ func main() { RunE: func(_ *cobra.Command, args []string) error { lookbackCfg.InitFromViper(v) return app.ExecuteAction(app.ActionExecuteOptions{ - Args: args, - Viper: v, - Logger: logger, - TLSFlags: tlsFlags, + Args: args, + Viper: v, + Logger: logger, + TLSConfig: tlsConfig, }, func(c client.Client, cfg app.Config) app.Action { lookbackCfg.Config = cfg lookbackCfg.InitFromViper(v) @@ -129,7 +128,7 @@ func main() { }, } - addPersistentFlags(v, rootCmd, tlsFlags.AddFlags, app.AddFlags) + addPersistentFlags(v, rootCmd, tlsConfig.AddFlags, app.AddFlags) addSubCommand(v, rootCmd, initCommand, initCfg.AddFlags) addSubCommand(v, rootCmd, rolloverCommand, rolloverCfg.AddFlags) addSubCommand(v, rootCmd, lookbackCommand, lookbackCfg.AddFlags) From b52c38885779b1749d3ca113e171c143e97d592b Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Fri, 22 Nov 2024 05:27:47 +0530 Subject: [PATCH 2/8] removed redudant code Signed-off-by: chahatsagarmain --- cmd/es-rollover/app/actions.go | 10 ---------- cmd/es-rollover/app/actions_test.go | 3 ++- cmd/es-rollover/app/flags.go | 5 +++++ cmd/es-rollover/main.go | 2 +- 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/cmd/es-rollover/app/actions.go b/cmd/es-rollover/app/actions.go index cecf50eab86..2c59ffe40fe 100644 --- a/cmd/es-rollover/app/actions.go +++ b/cmd/es-rollover/app/actions.go @@ -6,7 +6,6 @@ package app import ( "context" "crypto/tls" - "flag" "net/http" "time" @@ -42,15 +41,6 @@ type ClientConfig struct { Enabled bool } -func (c *ClientConfig) AddFlags(flags *flag.FlagSet) { - flags.BoolVar(&c.Enabled, "es.tls.enabled", false, "Enable TLS when talking to the remote server(s)") - flags.StringVar(&c.CAFile, "es.tls.ca", "", "Path to a TLS CA (Certification Authority) file used to verify the remote server(s) (by default will use the system truststore)") - flags.StringVar(&c.CertFile, "es.tls.cert", "", "Path to a TLS Certificate file, used to identify this process to the remote server(s)") - flags.StringVar(&c.KeyFile, "es.tls.key", "", "Path to a TLS Private Key file, used to identify this process to the remote server(s)") - flags.StringVar(&c.ServerName, "es.tls.server-name", "", "Override the TLS server name we expect in the certificate of the remote server(s)") - flags.BoolVar(&c.InsecureSkipVerify, "es.tls.skip-host-verify", false, "(insecure) Skip server's certificate chain and host name verification") -} - // ActionExecuteOptions are the options passed to the execute action function type ActionExecuteOptions struct { Args []string diff --git a/cmd/es-rollover/app/actions_test.go b/cmd/es-rollover/app/actions_test.go index c0872402e55..2e2015969b2 100644 --- a/cmd/es-rollover/app/actions_test.go +++ b/cmd/es-rollover/app/actions_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/zap" + "github.com/jaegertracing/jaeger/cmd/agent/app" "github.com/jaegertracing/jaeger/pkg/es/client" ) @@ -76,7 +77,7 @@ func TestExecuteAction(t *testing.T) { tlfConfig := &ClientConfig{} command := cobra.Command{} flags := &flag.FlagSet{} - tlfConfig.AddFlags(flags) + app.AddFlags(flags) command.PersistentFlags().AddGoFlagSet(flags) v.BindPFlags(command.PersistentFlags()) cmdLine := append([]string{"--es.tls.enabled=true"}, test.flags...) diff --git a/cmd/es-rollover/app/flags.go b/cmd/es-rollover/app/flags.go index bf7bfcac1f0..58de9679162 100644 --- a/cmd/es-rollover/app/flags.go +++ b/cmd/es-rollover/app/flags.go @@ -7,6 +7,8 @@ import ( "flag" "github.com/spf13/viper" + + "github.com/jaegertracing/jaeger/pkg/config/tlscfg" ) const ( @@ -35,6 +37,8 @@ type Config struct { AdaptiveSampling bool } +var tlsFlagsConfig = tlscfg.ClientFlagsConfig{Prefix: "es"} + // AddFlags adds flags func AddFlags(flags *flag.FlagSet) { flags.String(indexPrefix, "", "Index prefix") @@ -46,6 +50,7 @@ func AddFlags(flags *flag.FlagSet) { flags.Int(timeout, 120, "Number of seconds to wait for master node response") flags.Bool(skipDependencies, false, "Disable rollover for dependencies index") flags.Bool(adaptiveSampling, false, "Enable rollover for adaptive sampling index") + tlsFlagsConfig.AddFlags(flags) } // InitFromViper initializes config from viper.Viper. diff --git a/cmd/es-rollover/main.go b/cmd/es-rollover/main.go index c70ac73287b..c0cbbd7a1bc 100644 --- a/cmd/es-rollover/main.go +++ b/cmd/es-rollover/main.go @@ -128,7 +128,7 @@ func main() { }, } - addPersistentFlags(v, rootCmd, tlsConfig.AddFlags, app.AddFlags) + addPersistentFlags(v, rootCmd, app.AddFlags) addSubCommand(v, rootCmd, initCommand, initCfg.AddFlags) addSubCommand(v, rootCmd, rolloverCommand, rolloverCfg.AddFlags) addSubCommand(v, rootCmd, lookbackCommand, lookbackCfg.AddFlags) From 830f11c0ba40fd570332bbb51e8bf37605ca781e Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Sat, 23 Nov 2024 01:15:32 +0530 Subject: [PATCH 3/8] fix Signed-off-by: chahatsagarmain --- cmd/es-rollover/app/actions.go | 1 + cmd/es-rollover/app/actions_test.go | 7 +++---- cmd/es-rollover/app/flags.go | 22 +++++++++++++++++++--- cmd/es-rollover/main.go | 2 +- 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/cmd/es-rollover/app/actions.go b/cmd/es-rollover/app/actions.go index 2c59ffe40fe..e4270090a5c 100644 --- a/cmd/es-rollover/app/actions.go +++ b/cmd/es-rollover/app/actions.go @@ -71,6 +71,7 @@ func getTLSConfig(tlsConfig *ClientConfig, logger *zap.Logger) (*tls.Config, err func ExecuteAction(opts ActionExecuteOptions, createAction ActionCreatorFunction) error { cfg := Config{} cfg.InitFromViper(opts.Viper) + opts.TLSConfig.InitFromViper(opts.Viper) tlsCfg, err := getTLSConfig(opts.TLSConfig, opts.Logger) if err != nil { return err diff --git a/cmd/es-rollover/app/actions_test.go b/cmd/es-rollover/app/actions_test.go index 2e2015969b2..a8d8b567de8 100644 --- a/cmd/es-rollover/app/actions_test.go +++ b/cmd/es-rollover/app/actions_test.go @@ -15,7 +15,6 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/cmd/agent/app" "github.com/jaegertracing/jaeger/pkg/es/client" ) @@ -74,10 +73,10 @@ func TestExecuteAction(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { v := viper.New() - tlfConfig := &ClientConfig{} + tlsConfig := &ClientConfig{} command := cobra.Command{} flags := &flag.FlagSet{} - app.AddFlags(flags) + tlsConfig.AddFlags(flags) command.PersistentFlags().AddGoFlagSet(flags) v.BindPFlags(command.PersistentFlags()) cmdLine := append([]string{"--es.tls.enabled=true"}, test.flags...) @@ -88,7 +87,7 @@ func TestExecuteAction(t *testing.T) { Args: args, Viper: v, Logger: logger, - TLSConfig: tlfConfig, + TLSConfig: tlsConfig, }, func(c client.Client, _ Config) Action { assert.Equal(t, "https://localhost:9300", c.Endpoint) transport, ok := c.Client.Transport.(*http.Transport) diff --git a/cmd/es-rollover/app/flags.go b/cmd/es-rollover/app/flags.go index 58de9679162..c9962553ca6 100644 --- a/cmd/es-rollover/app/flags.go +++ b/cmd/es-rollover/app/flags.go @@ -11,6 +11,8 @@ import ( "github.com/jaegertracing/jaeger/pkg/config/tlscfg" ) +var tlsFlagsCfg = tlscfg.ClientFlagsConfig{Prefix: "es"} + const ( indexPrefix = "index-prefix" archive = "archive" @@ -37,8 +39,6 @@ type Config struct { AdaptiveSampling bool } -var tlsFlagsConfig = tlscfg.ClientFlagsConfig{Prefix: "es"} - // AddFlags adds flags func AddFlags(flags *flag.FlagSet) { flags.String(indexPrefix, "", "Index prefix") @@ -50,7 +50,10 @@ func AddFlags(flags *flag.FlagSet) { flags.Int(timeout, 120, "Number of seconds to wait for master node response") flags.Bool(skipDependencies, false, "Disable rollover for dependencies index") flags.Bool(adaptiveSampling, false, "Enable rollover for adaptive sampling index") - tlsFlagsConfig.AddFlags(flags) +} + +func (ClientConfig) AddFlags(flags *flag.FlagSet) { + tlsFlagsCfg.AddFlags(flags) } // InitFromViper initializes config from viper.Viper. @@ -68,3 +71,16 @@ func (c *Config) InitFromViper(v *viper.Viper) { c.SkipDependencies = v.GetBool(skipDependencies) c.AdaptiveSampling = v.GetBool(adaptiveSampling) } + +func (c *ClientConfig) InitFromViper(v *viper.Viper) error { + opts, err := tlsFlagsCfg.InitFromViper(v) + if err != nil { + return err + } + c.CAFile = opts.CAPath + c.CertFile = opts.CertPath + c.KeyFile = opts.KeyPath + c.ServerName = opts.ServerName + c.InsecureSkipVerify = opts.SkipHostVerify + return nil +} diff --git a/cmd/es-rollover/main.go b/cmd/es-rollover/main.go index c0cbbd7a1bc..c70ac73287b 100644 --- a/cmd/es-rollover/main.go +++ b/cmd/es-rollover/main.go @@ -128,7 +128,7 @@ func main() { }, } - addPersistentFlags(v, rootCmd, app.AddFlags) + addPersistentFlags(v, rootCmd, tlsConfig.AddFlags, app.AddFlags) addSubCommand(v, rootCmd, initCommand, initCfg.AddFlags) addSubCommand(v, rootCmd, rolloverCommand, rolloverCfg.AddFlags) addSubCommand(v, rootCmd, lookbackCommand, lookbackCfg.AddFlags) From 70b1890f34b3fd55befe21e2da1de9e8ac76f5c2 Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Sat, 23 Nov 2024 03:07:29 +0530 Subject: [PATCH 4/8] removed unused struct Signed-off-by: chahatsagarmain --- cmd/es-rollover/app/actions.go | 33 ++++++++++++----------------- cmd/es-rollover/app/actions_test.go | 11 +++++----- cmd/es-rollover/app/flags.go | 15 +++++-------- cmd/es-rollover/main.go | 25 +++++++++------------- 4 files changed, 34 insertions(+), 50 deletions(-) diff --git a/cmd/es-rollover/app/actions.go b/cmd/es-rollover/app/actions.go index e4270090a5c..0ea3ecf824c 100644 --- a/cmd/es-rollover/app/actions.go +++ b/cmd/es-rollover/app/actions.go @@ -6,6 +6,7 @@ package app import ( "context" "crypto/tls" + "fmt" "net/http" "time" @@ -36,42 +37,36 @@ type Action interface { Do() error } -type ClientConfig struct { - configtls.ClientConfig `mapstructure:",squash"` - Enabled bool -} - // ActionExecuteOptions are the options passed to the execute action function type ActionExecuteOptions struct { Args []string Viper *viper.Viper Logger *zap.Logger - TLSConfig *ClientConfig + TLSConfig configtls.ClientConfig } // ActionCreatorFunction type is the function type in charge of create the action to be executed type ActionCreatorFunction func(client.Client, Config) Action -func getTLSConfig(tlsConfig *ClientConfig, logger *zap.Logger) (*tls.Config, error) { - if tlsConfig == nil { - return nil, nil - } - - if tlsConfig.Insecure { - logger.Info("TLS is disabled") - return nil, nil - } - - ctx := context.Background() +func getTLSConfig(tlsConfig configtls.ClientConfig, logger *zap.Logger) (*tls.Config, error) { + if tlsConfig.Insecure { + logger.Info("TLS is disabled") + return nil, nil + } - return tlsConfig.LoadTLSConfig(ctx) + ctx := context.Background() + tlsCfg, err := tlsConfig.LoadTLSConfig(ctx) + if err != nil { + logger.Error("Failed to load TLS configuration", zap.Error(err)) + return nil, fmt.Errorf("error loading TLS config: %w", err) + } + return tlsCfg, nil } // ExecuteAction execute the action returned by the createAction function func ExecuteAction(opts ActionExecuteOptions, createAction ActionCreatorFunction) error { cfg := Config{} cfg.InitFromViper(opts.Viper) - opts.TLSConfig.InitFromViper(opts.Viper) tlsCfg, err := getTLSConfig(opts.TLSConfig, opts.Logger) if err != nil { return err diff --git a/cmd/es-rollover/app/actions_test.go b/cmd/es-rollover/app/actions_test.go index a8d8b567de8..983ddb69baf 100644 --- a/cmd/es-rollover/app/actions_test.go +++ b/cmd/es-rollover/app/actions_test.go @@ -73,10 +73,10 @@ func TestExecuteAction(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { v := viper.New() - tlsConfig := &ClientConfig{} command := cobra.Command{} flags := &flag.FlagSet{} - tlsConfig.AddFlags(flags) + AddTLSFlags(flags) + AddFlags(flags) command.PersistentFlags().AddGoFlagSet(flags) v.BindPFlags(command.PersistentFlags()) cmdLine := append([]string{"--es.tls.enabled=true"}, test.flags...) @@ -84,10 +84,9 @@ func TestExecuteAction(t *testing.T) { require.NoError(t, err) executedAction := false err = ExecuteAction(ActionExecuteOptions{ - Args: args, - Viper: v, - Logger: logger, - TLSConfig: tlsConfig, + Args: args, + Viper: v, + Logger: logger, }, func(c client.Client, _ Config) Action { assert.Equal(t, "https://localhost:9300", c.Endpoint) transport, ok := c.Client.Transport.(*http.Transport) diff --git a/cmd/es-rollover/app/flags.go b/cmd/es-rollover/app/flags.go index c9962553ca6..bf7bdc78a82 100644 --- a/cmd/es-rollover/app/flags.go +++ b/cmd/es-rollover/app/flags.go @@ -7,6 +7,7 @@ import ( "flag" "github.com/spf13/viper" + "go.opentelemetry.io/collector/config/configtls" "github.com/jaegertracing/jaeger/pkg/config/tlscfg" ) @@ -37,6 +38,7 @@ type Config struct { Timeout int SkipDependencies bool AdaptiveSampling bool + TLSConfig configtls.ClientConfig } // AddFlags adds flags @@ -52,12 +54,12 @@ func AddFlags(flags *flag.FlagSet) { flags.Bool(adaptiveSampling, false, "Enable rollover for adaptive sampling index") } -func (ClientConfig) AddFlags(flags *flag.FlagSet) { +func AddTLSFlags(flags *flag.FlagSet) { tlsFlagsCfg.AddFlags(flags) } // InitFromViper initializes config from viper.Viper. -func (c *Config) InitFromViper(v *viper.Viper) { +func (c *Config) InitFromViper(v *viper.Viper) error { c.IndexPrefix = v.GetString(indexPrefix) if c.IndexPrefix != "" { c.IndexPrefix += "-" @@ -70,17 +72,10 @@ func (c *Config) InitFromViper(v *viper.Viper) { c.Timeout = v.GetInt(timeout) c.SkipDependencies = v.GetBool(skipDependencies) c.AdaptiveSampling = v.GetBool(adaptiveSampling) -} - -func (c *ClientConfig) InitFromViper(v *viper.Viper) error { opts, err := tlsFlagsCfg.InitFromViper(v) if err != nil { return err } - c.CAFile = opts.CAPath - c.CertFile = opts.CertPath - c.KeyFile = opts.KeyPath - c.ServerName = opts.ServerName - c.InsecureSkipVerify = opts.SkipHostVerify + c.TLSConfig = opts.ToOtelClientConfig() return nil } diff --git a/cmd/es-rollover/main.go b/cmd/es-rollover/main.go index c70ac73287b..48099081cdc 100644 --- a/cmd/es-rollover/main.go +++ b/cmd/es-rollover/main.go @@ -29,8 +29,6 @@ func main() { Long: "Jaeger es-rollover manages Jaeger indices", } - tlsConfig := &app.ClientConfig{} - // Init command initCfg := &initialize.Config{} initCommand := &cobra.Command{ @@ -41,10 +39,9 @@ func main() { SilenceUsage: true, RunE: func(_ *cobra.Command, args []string) error { return app.ExecuteAction(app.ActionExecuteOptions{ - Args: args, - Viper: v, - Logger: logger, - TLSConfig: tlsConfig, + Args: args, + Viper: v, + Logger: logger, }, func(c client.Client, cfg app.Config) app.Action { initCfg.Config = cfg initCfg.InitFromViper(v) @@ -79,10 +76,9 @@ func main() { RunE: func(_ *cobra.Command, args []string) error { rolloverCfg.InitFromViper(v) return app.ExecuteAction(app.ActionExecuteOptions{ - Args: args, - Viper: v, - Logger: logger, - TLSConfig: tlsConfig, + Args: args, + Viper: v, + Logger: logger, }, func(c client.Client, cfg app.Config) app.Action { rolloverCfg.Config = cfg rolloverCfg.InitFromViper(v) @@ -108,10 +104,9 @@ func main() { RunE: func(_ *cobra.Command, args []string) error { lookbackCfg.InitFromViper(v) return app.ExecuteAction(app.ActionExecuteOptions{ - Args: args, - Viper: v, - Logger: logger, - TLSConfig: tlsConfig, + Args: args, + Viper: v, + Logger: logger, }, func(c client.Client, cfg app.Config) app.Action { lookbackCfg.Config = cfg lookbackCfg.InitFromViper(v) @@ -128,7 +123,7 @@ func main() { }, } - addPersistentFlags(v, rootCmd, tlsConfig.AddFlags, app.AddFlags) + addPersistentFlags(v, rootCmd, app.AddTLSFlags, app.AddFlags) addSubCommand(v, rootCmd, initCommand, initCfg.AddFlags) addSubCommand(v, rootCmd, rolloverCommand, rolloverCfg.AddFlags) addSubCommand(v, rootCmd, lookbackCommand, lookbackCfg.AddFlags) From 64b42cc2e890af03bde64d6e61c1751cdb9e51b1 Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Sun, 24 Nov 2024 01:21:23 +0530 Subject: [PATCH 5/8] fix Signed-off-by: chahatsagarmain --- cmd/es-rollover/app/actions.go | 36 +++++++++++------------------ cmd/es-rollover/app/actions_test.go | 2 -- cmd/es-rollover/app/flags.go | 6 ++--- cmd/es-rollover/main.go | 2 +- 4 files changed, 16 insertions(+), 30 deletions(-) diff --git a/cmd/es-rollover/app/actions.go b/cmd/es-rollover/app/actions.go index 0ea3ecf824c..334ff80bb60 100644 --- a/cmd/es-rollover/app/actions.go +++ b/cmd/es-rollover/app/actions.go @@ -11,7 +11,6 @@ import ( "time" "github.com/spf13/viper" - "go.opentelemetry.io/collector/config/configtls" "go.uber.org/zap" "github.com/jaegertracing/jaeger/pkg/es/client" @@ -39,37 +38,28 @@ type Action interface { // ActionExecuteOptions are the options passed to the execute action function type ActionExecuteOptions struct { - Args []string - Viper *viper.Viper - Logger *zap.Logger - TLSConfig configtls.ClientConfig + Args []string + Viper *viper.Viper + Logger *zap.Logger } // ActionCreatorFunction type is the function type in charge of create the action to be executed type ActionCreatorFunction func(client.Client, Config) Action -func getTLSConfig(tlsConfig configtls.ClientConfig, logger *zap.Logger) (*tls.Config, error) { - if tlsConfig.Insecure { - logger.Info("TLS is disabled") - return nil, nil - } - - ctx := context.Background() - tlsCfg, err := tlsConfig.LoadTLSConfig(ctx) - if err != nil { - logger.Error("Failed to load TLS configuration", zap.Error(err)) - return nil, fmt.Errorf("error loading TLS config: %w", err) - } - return tlsCfg, nil -} - // ExecuteAction execute the action returned by the createAction function func ExecuteAction(opts ActionExecuteOptions, createAction ActionCreatorFunction) error { cfg := Config{} - cfg.InitFromViper(opts.Viper) - tlsCfg, err := getTLSConfig(opts.TLSConfig, opts.Logger) + if err := cfg.InitFromViper(opts.Viper); err != nil { + opts.Logger.Error("Failed to initialize config from viper", + zap.Error(err), + ) + return fmt.Errorf("failed to initialize config: %w", err) + } + + ctx := context.Background() + tlsCfg, err := cfg.TLSConfig.LoadTLSConfig(ctx) if err != nil { - return err + return fmt.Errorf("TLS configuration failed: %w", err) } esClient := newESClient(opts.Args[0], &cfg, tlsCfg) diff --git a/cmd/es-rollover/app/actions_test.go b/cmd/es-rollover/app/actions_test.go index 983ddb69baf..2d40be7d1a9 100644 --- a/cmd/es-rollover/app/actions_test.go +++ b/cmd/es-rollover/app/actions_test.go @@ -75,7 +75,6 @@ func TestExecuteAction(t *testing.T) { v := viper.New() command := cobra.Command{} flags := &flag.FlagSet{} - AddTLSFlags(flags) AddFlags(flags) command.PersistentFlags().AddGoFlagSet(flags) v.BindPFlags(command.PersistentFlags()) @@ -99,7 +98,6 @@ func TestExecuteAction(t *testing.T) { }, } }) - assert.Equal(t, test.expectedExecuteAction, executedAction) if test.configError { require.Error(t, err) diff --git a/cmd/es-rollover/app/flags.go b/cmd/es-rollover/app/flags.go index bf7bdc78a82..17955bf7dcf 100644 --- a/cmd/es-rollover/app/flags.go +++ b/cmd/es-rollover/app/flags.go @@ -52,9 +52,6 @@ func AddFlags(flags *flag.FlagSet) { flags.Int(timeout, 120, "Number of seconds to wait for master node response") flags.Bool(skipDependencies, false, "Disable rollover for dependencies index") flags.Bool(adaptiveSampling, false, "Enable rollover for adaptive sampling index") -} - -func AddTLSFlags(flags *flag.FlagSet) { tlsFlagsCfg.AddFlags(flags) } @@ -76,6 +73,7 @@ func (c *Config) InitFromViper(v *viper.Viper) error { if err != nil { return err } - c.TLSConfig = opts.ToOtelClientConfig() + c.TLSConfig = opts.ToOtelClientConfig() + return nil } diff --git a/cmd/es-rollover/main.go b/cmd/es-rollover/main.go index 48099081cdc..411d4596d2f 100644 --- a/cmd/es-rollover/main.go +++ b/cmd/es-rollover/main.go @@ -123,7 +123,7 @@ func main() { }, } - addPersistentFlags(v, rootCmd, app.AddTLSFlags, app.AddFlags) + addPersistentFlags(v, rootCmd, app.AddFlags) addSubCommand(v, rootCmd, initCommand, initCfg.AddFlags) addSubCommand(v, rootCmd, rolloverCommand, rolloverCfg.AddFlags) addSubCommand(v, rootCmd, lookbackCommand, lookbackCfg.AddFlags) From 4b42e6d1cf48eea1e96c03fe3540c808a9e30832 Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Sun, 24 Nov 2024 04:45:10 +0530 Subject: [PATCH 6/8] panic in initviper Signed-off-by: chahatsagarmain --- cmd/es-rollover/app/init/flags.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmd/es-rollover/app/init/flags.go b/cmd/es-rollover/app/init/flags.go index a5b17d2d1df..23e2589df17 100644 --- a/cmd/es-rollover/app/init/flags.go +++ b/cmd/es-rollover/app/init/flags.go @@ -40,6 +40,9 @@ func (*Config) AddFlags(flags *flag.FlagSet) { // InitFromViper initializes config from viper.Viper. func (c *Config) InitFromViper(v *viper.Viper) { + if v == nil { + panic("viper instance cannot be nil") + } c.Indices.Spans.Shards = v.GetInt64(shards) c.Indices.Services.Shards = v.GetInt64(shards) c.Indices.Dependencies.Shards = v.GetInt64(shards) From 578a34e330cdf3285b9137eeb0cd47da20fbf07d Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Mon, 25 Nov 2024 01:36:54 +0530 Subject: [PATCH 7/8] resolved comments Signed-off-by: chahatsagarmain --- cmd/es-rollover/app/actions.go | 7 +------ cmd/es-rollover/app/flags.go | 6 ++---- cmd/es-rollover/app/init/flags.go | 3 --- 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/cmd/es-rollover/app/actions.go b/cmd/es-rollover/app/actions.go index 334ff80bb60..3b3cf6149cf 100644 --- a/cmd/es-rollover/app/actions.go +++ b/cmd/es-rollover/app/actions.go @@ -49,12 +49,7 @@ type ActionCreatorFunction func(client.Client, Config) Action // ExecuteAction execute the action returned by the createAction function func ExecuteAction(opts ActionExecuteOptions, createAction ActionCreatorFunction) error { cfg := Config{} - if err := cfg.InitFromViper(opts.Viper); err != nil { - opts.Logger.Error("Failed to initialize config from viper", - zap.Error(err), - ) - return fmt.Errorf("failed to initialize config: %w", err) - } + cfg.InitFromViper(opts.Viper) ctx := context.Background() tlsCfg, err := cfg.TLSConfig.LoadTLSConfig(ctx) diff --git a/cmd/es-rollover/app/flags.go b/cmd/es-rollover/app/flags.go index 17955bf7dcf..a1456546546 100644 --- a/cmd/es-rollover/app/flags.go +++ b/cmd/es-rollover/app/flags.go @@ -56,7 +56,7 @@ func AddFlags(flags *flag.FlagSet) { } // InitFromViper initializes config from viper.Viper. -func (c *Config) InitFromViper(v *viper.Viper) error { +func (c *Config) InitFromViper(v *viper.Viper) { c.IndexPrefix = v.GetString(indexPrefix) if c.IndexPrefix != "" { c.IndexPrefix += "-" @@ -71,9 +71,7 @@ func (c *Config) InitFromViper(v *viper.Viper) error { c.AdaptiveSampling = v.GetBool(adaptiveSampling) opts, err := tlsFlagsCfg.InitFromViper(v) if err != nil { - return err + panic(err) } c.TLSConfig = opts.ToOtelClientConfig() - - return nil } diff --git a/cmd/es-rollover/app/init/flags.go b/cmd/es-rollover/app/init/flags.go index 23e2589df17..a5b17d2d1df 100644 --- a/cmd/es-rollover/app/init/flags.go +++ b/cmd/es-rollover/app/init/flags.go @@ -40,9 +40,6 @@ func (*Config) AddFlags(flags *flag.FlagSet) { // InitFromViper initializes config from viper.Viper. func (c *Config) InitFromViper(v *viper.Viper) { - if v == nil { - panic("viper instance cannot be nil") - } c.Indices.Spans.Shards = v.GetInt64(shards) c.Indices.Services.Shards = v.GetInt64(shards) c.Indices.Dependencies.Shards = v.GetInt64(shards) From d2394b4d064edaff155c27b8f6152e7a23e2e17c Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sun, 24 Nov 2024 15:14:00 -0500 Subject: [PATCH 8/8] use viperize Signed-off-by: Yuri Shkuro --- cmd/es-rollover/app/actions_test.go | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/cmd/es-rollover/app/actions_test.go b/cmd/es-rollover/app/actions_test.go index 2d40be7d1a9..bc78ef11827 100644 --- a/cmd/es-rollover/app/actions_test.go +++ b/cmd/es-rollover/app/actions_test.go @@ -5,16 +5,14 @@ package app import ( "errors" - "flag" "net/http" "testing" - "github.com/spf13/cobra" - "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/zap" + "github.com/jaegertracing/jaeger/pkg/config" "github.com/jaegertracing/jaeger/pkg/es/client" ) @@ -72,17 +70,11 @@ func TestExecuteAction(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - v := viper.New() - command := cobra.Command{} - flags := &flag.FlagSet{} - AddFlags(flags) - command.PersistentFlags().AddGoFlagSet(flags) - v.BindPFlags(command.PersistentFlags()) + v, command := config.Viperize(AddFlags) cmdLine := append([]string{"--es.tls.enabled=true"}, test.flags...) - err := command.ParseFlags(cmdLine) - require.NoError(t, err) + require.NoError(t, command.ParseFlags(cmdLine)) executedAction := false - err = ExecuteAction(ActionExecuteOptions{ + err := ExecuteAction(ActionExecuteOptions{ Args: args, Viper: v, Logger: logger,