From 0042cd3aa2fe24289f8ebd10b849a889399eadd9 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Mon, 2 Mar 2020 15:22:11 +0100 Subject: [PATCH 1/7] [fix] Enable Kafka TLS when TLS auth is specified Signed-off-by: Pavol Loffay --- cmd/ingester/app/flags_test.go | 43 ++++++++++++++++++++++++++++ pkg/kafka/auth/config.go | 4 +++ plugin/storage/kafka/options_test.go | 43 ++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+) diff --git a/cmd/ingester/app/flags_test.go b/cmd/ingester/app/flags_test.go index fab2d5ed2f1..b72a1343eda 100644 --- a/cmd/ingester/app/flags_test.go +++ b/cmd/ingester/app/flags_test.go @@ -15,12 +15,16 @@ package app import ( + "fmt" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/jaegertracing/jaeger/pkg/config" + "github.com/jaegertracing/jaeger/pkg/config/tlscfg" + "github.com/jaegertracing/jaeger/pkg/kafka/auth" "github.com/jaegertracing/jaeger/plugin/storage/kafka" ) @@ -49,6 +53,45 @@ func TestOptionsWithFlags(t *testing.T) { assert.Equal(t, kafka.EncodingJSON, o.Encoding) } +func TestTLSFlags(t *testing.T) { + tests := []struct { + flags []string + expected auth.AuthenticationConfig + }{ + { + flags: []string{}, + expected: auth.AuthenticationConfig{Authentication: "none", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}}, + }, + { + flags: []string{"--kafka.consumer.authentication=foo"}, + expected: auth.AuthenticationConfig{Authentication: "foo", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}}, + }, + { + flags: []string{"--kafka.consumer.authentication=kerberos", "--kafka.consumer.tls.enabled=true"}, + expected: auth.AuthenticationConfig{Authentication: "kerberos", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}, TLS: tlscfg.Options{Enabled: true}}, + }, + { + flags: []string{"--kafka.consumer.authentication=tls"}, + expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}, TLS: tlscfg.Options{Enabled: true}}, + }, + { + flags: []string{"--kafka.consumer.authentication=tls", "--kafka.consumer.tls.enabled=false"}, + expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}, TLS: tlscfg.Options{Enabled: true}}, + }, + } + + for _, test := range tests { + t.Run(fmt.Sprintf("%s", test.flags), func(t *testing.T) { + o := &Options{} + v, command := config.Viperize(AddFlags) + err := command.ParseFlags(test.flags) + require.NoError(t, err) + o.InitFromViper(v) + assert.Equal(t, test.expected, o.AuthenticationConfig) + }) + } +} + func TestFlagDefaults(t *testing.T) { o := &Options{} v, command := config.Viperize(AddFlags) diff --git a/pkg/kafka/auth/config.go b/pkg/kafka/auth/config.go index 7b94176f220..c938e052a07 100644 --- a/pkg/kafka/auth/config.go +++ b/pkg/kafka/auth/config.go @@ -86,6 +86,10 @@ func (config *AuthenticationConfig) InitFromViper(configPrefix string, v *viper. config.TLS = tlsClientConfig.InitFromViper(v) + if config.Authentication == tls { + config.TLS.Enabled = true + } + config.PlainText.UserName = v.GetString(configPrefix + plainTextPrefix + suffixPlainTextUserName) config.PlainText.Password = v.GetString(configPrefix + plainTextPrefix + suffixPlainTextPassword) } diff --git a/plugin/storage/kafka/options_test.go b/plugin/storage/kafka/options_test.go index e27ee91f55f..1be8b5bad1f 100644 --- a/plugin/storage/kafka/options_test.go +++ b/plugin/storage/kafka/options_test.go @@ -15,6 +15,7 @@ package kafka import ( + "fmt" "testing" "time" @@ -23,6 +24,8 @@ import ( "github.com/stretchr/testify/require" "github.com/jaegertracing/jaeger/pkg/config" + "github.com/jaegertracing/jaeger/pkg/config/tlscfg" + "github.com/jaegertracing/jaeger/pkg/kafka/auth" ) func TestOptionsWithFlags(t *testing.T) { @@ -164,3 +167,43 @@ func TestRequiredAcksFailures(t *testing.T) { _, err := getRequiredAcks("test") assert.Error(t, err) } + +func TestTLSFlags(t *testing.T) { + tests := []struct { + flags []string + expected auth.AuthenticationConfig + }{ + { + flags: []string{}, + expected: auth.AuthenticationConfig{Authentication: "none", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}}, + }, + { + flags: []string{"--kafka.producer.authentication=foo"}, + expected: auth.AuthenticationConfig{Authentication: "foo", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}}, + }, + { + flags: []string{"--kafka.producer.authentication=kerberos", "--kafka.producer.tls.enabled=true"}, + expected: auth.AuthenticationConfig{Authentication: "kerberos", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}, TLS: tlscfg.Options{Enabled: true}}, + }, + { + flags: []string{"--kafka.producer.authentication=tls"}, + expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}, TLS: tlscfg.Options{Enabled: true}}, + }, + { + flags: []string{"--kafka.producer.authentication=tls", "--kafka.producer.tls.enabled=false"}, + expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}, TLS: tlscfg.Options{Enabled: true}}, + }, + } + + for _, test := range tests { + t.Run(fmt.Sprintf("%s", test.flags), func(t *testing.T) { + o := &Options{} + v, command := config.Viperize(o.AddFlags) + err := command.ParseFlags(test.flags) + require.NoError(t, err) + o.InitFromViper(v) + assert.Equal(t, test.expected, o.config.AuthenticationConfig) + + }) + } +} From 5b484225d837288aba11254dcb73f3d70c93c068 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Mon, 2 Mar 2020 17:52:35 +0100 Subject: [PATCH 2/7] Use TLS when tls.enabled=true Signed-off-by: Pavol Loffay --- cmd/ingester/app/flags_test.go | 2 +- pkg/kafka/auth/config.go | 3 +++ plugin/storage/kafka/options_test.go | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/cmd/ingester/app/flags_test.go b/cmd/ingester/app/flags_test.go index b72a1343eda..7f910525df3 100644 --- a/cmd/ingester/app/flags_test.go +++ b/cmd/ingester/app/flags_test.go @@ -68,7 +68,7 @@ func TestTLSFlags(t *testing.T) { }, { flags: []string{"--kafka.consumer.authentication=kerberos", "--kafka.consumer.tls.enabled=true"}, - expected: auth.AuthenticationConfig{Authentication: "kerberos", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}, TLS: tlscfg.Options{Enabled: true}}, + expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}, TLS: tlscfg.Options{Enabled: true}}, }, { flags: []string{"--kafka.consumer.authentication=tls"}, diff --git a/pkg/kafka/auth/config.go b/pkg/kafka/auth/config.go index c938e052a07..e162a3f5dc5 100644 --- a/pkg/kafka/auth/config.go +++ b/pkg/kafka/auth/config.go @@ -89,6 +89,9 @@ func (config *AuthenticationConfig) InitFromViper(configPrefix string, v *viper. if config.Authentication == tls { config.TLS.Enabled = true } + if config.TLS.Enabled == true { + config.Authentication = tls + } config.PlainText.UserName = v.GetString(configPrefix + plainTextPrefix + suffixPlainTextUserName) config.PlainText.Password = v.GetString(configPrefix + plainTextPrefix + suffixPlainTextPassword) diff --git a/plugin/storage/kafka/options_test.go b/plugin/storage/kafka/options_test.go index 1be8b5bad1f..6173a87e08b 100644 --- a/plugin/storage/kafka/options_test.go +++ b/plugin/storage/kafka/options_test.go @@ -183,7 +183,7 @@ func TestTLSFlags(t *testing.T) { }, { flags: []string{"--kafka.producer.authentication=kerberos", "--kafka.producer.tls.enabled=true"}, - expected: auth.AuthenticationConfig{Authentication: "kerberos", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}, TLS: tlscfg.Options{Enabled: true}}, + expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}, TLS: tlscfg.Options{Enabled: true}}, }, { flags: []string{"--kafka.producer.authentication=tls"}, From 4e170e9ebe334d1e95061f5fa8292e0537d86191 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Tue, 3 Mar 2020 11:32:10 +0100 Subject: [PATCH 3/7] Log deprecation message Signed-off-by: Pavol Loffay --- cmd/agent/app/reporter/grpc/flags.go | 2 +- cmd/collector/app/builder_flags.go | 2 +- cmd/ingester/app/flags.go | 4 ++- cmd/ingester/app/flags_test.go | 7 +++--- cmd/ingester/main.go | 2 +- pkg/config/tlscfg/flags.go | 32 ++++++++++++++++++------ pkg/config/tlscfg/flags_test.go | 4 +-- pkg/kafka/auth/config.go | 19 ++++++++------ pkg/kafka/auth/options.go | 2 +- plugin/storage/cassandra/options.go | 2 +- plugin/storage/es/options.go | 2 +- plugin/storage/integration/kafka_test.go | 2 +- plugin/storage/kafka/factory.go | 2 +- plugin/storage/kafka/options_test.go | 2 ++ 14 files changed, 55 insertions(+), 29 deletions(-) diff --git a/cmd/agent/app/reporter/grpc/flags.go b/cmd/agent/app/reporter/grpc/flags.go index e557a8be6fb..5a3879df81e 100644 --- a/cmd/agent/app/reporter/grpc/flags.go +++ b/cmd/agent/app/reporter/grpc/flags.go @@ -33,7 +33,7 @@ const ( var tlsFlagsConfig = tlscfg.ClientFlagsConfig{ Prefix: gRPCPrefix, - ShowEnabled: true, + Enabled: tlscfg.Show, ShowServerName: true, } diff --git a/cmd/collector/app/builder_flags.go b/cmd/collector/app/builder_flags.go index ca207f6b37e..c45c1e1c784 100644 --- a/cmd/collector/app/builder_flags.go +++ b/cmd/collector/app/builder_flags.go @@ -40,7 +40,7 @@ const ( var tlsFlagsConfig = tlscfg.ServerFlagsConfig{ Prefix: "collector.grpc", - ShowEnabled: true, + ShowEnabled: tlscfg.Show, ShowClientCA: true, } diff --git a/cmd/ingester/app/flags.go b/cmd/ingester/app/flags.go index 5aca213bf2a..500e173cfe6 100644 --- a/cmd/ingester/app/flags.go +++ b/cmd/ingester/app/flags.go @@ -22,6 +22,7 @@ import ( "time" "github.com/spf13/viper" + "go.uber.org/zap" "github.com/jaegertracing/jaeger/pkg/kafka/auth" kafkaConsumer "github.com/jaegertracing/jaeger/pkg/kafka/consumer" @@ -114,7 +115,7 @@ func AddFlags(flagSet *flag.FlagSet) { } // InitFromViper initializes Builder with properties from viper -func (o *Options) InitFromViper(v *viper.Viper) { +func (o *Options) InitFromViper(v *viper.Viper, logger *zap.Logger) { o.Brokers = strings.Split(stripWhiteSpace(v.GetString(KafkaConsumerConfigPrefix+SuffixBrokers)), ",") o.Topic = v.GetString(KafkaConsumerConfigPrefix + SuffixTopic) o.GroupID = v.GetString(KafkaConsumerConfigPrefix + SuffixGroupID) @@ -126,6 +127,7 @@ func (o *Options) InitFromViper(v *viper.Viper) { o.DeadlockInterval = v.GetDuration(ConfigPrefix + SuffixDeadlockInterval) authenticationOptions := auth.AuthenticationConfig{} authenticationOptions.InitFromViper(KafkaConsumerConfigPrefix, v) + authenticationOptions.Normalize(logger) o.AuthenticationConfig = authenticationOptions } diff --git a/cmd/ingester/app/flags_test.go b/cmd/ingester/app/flags_test.go index 7f910525df3..341fedef107 100644 --- a/cmd/ingester/app/flags_test.go +++ b/cmd/ingester/app/flags_test.go @@ -21,6 +21,7 @@ import ( "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/config/tlscfg" @@ -41,7 +42,7 @@ func TestOptionsWithFlags(t *testing.T) { "--ingester.parallelism=5", "--ingester.deadlockInterval=2m", }) - o.InitFromViper(v) + o.InitFromViper(v, zap.NewNop()) assert.Equal(t, "topic1", o.Topic) assert.Equal(t, []string{"127.0.0.1:9092", "0.0.0:1234"}, o.Brokers) @@ -86,7 +87,7 @@ func TestTLSFlags(t *testing.T) { v, command := config.Viperize(AddFlags) err := command.ParseFlags(test.flags) require.NoError(t, err) - o.InitFromViper(v) + o.InitFromViper(v, zap.NewNop()) assert.Equal(t, test.expected, o.AuthenticationConfig) }) } @@ -96,7 +97,7 @@ func TestFlagDefaults(t *testing.T) { o := &Options{} v, command := config.Viperize(AddFlags) command.ParseFlags([]string{}) - o.InitFromViper(v) + o.InitFromViper(v, zap.NewNop()) assert.Equal(t, DefaultTopic, o.Topic) assert.Equal(t, []string{DefaultBroker}, o.Brokers) diff --git a/cmd/ingester/main.go b/cmd/ingester/main.go index f2179654498..8a5e2729f33 100644 --- a/cmd/ingester/main.go +++ b/cmd/ingester/main.go @@ -68,7 +68,7 @@ func main() { } options := app.Options{} - options.InitFromViper(v) + options.InitFromViper(v, logger) consumer, err := builder.CreateConsumer(logger, metricsFactory, spanWriter, options) if err != nil { logger.Fatal("Unable to create consumer", zap.Error(err)) diff --git a/pkg/config/tlscfg/flags.go b/pkg/config/tlscfg/flags.go index 266fa62372b..c8f635b9f92 100644 --- a/pkg/config/tlscfg/flags.go +++ b/pkg/config/tlscfg/flags.go @@ -33,24 +33,36 @@ const ( tlsSkipHostVerify = tlsPrefix + ".skip-host-verify" ) +type Enabled int + +const ( + Hide Enabled = iota + Show + ShowDeprecated +) + // ClientFlagsConfig describes which CLI flags for TLS client should be generated. type ClientFlagsConfig struct { Prefix string - ShowEnabled bool + Enabled Enabled ShowServerName bool } // ServerFlagsConfig describes which CLI flags for TLS server should be generated. type ServerFlagsConfig struct { Prefix string - ShowEnabled bool + ShowEnabled Enabled ShowClientCA bool } // AddFlags adds flags for TLS to the FlagSet. func (c ClientFlagsConfig) AddFlags(flags *flag.FlagSet) { - if c.ShowEnabled { - flags.Bool(c.Prefix+tlsEnabled, false, "Enable TLS when talking to the remote server(s)") + if c.Enabled >= Show { + deprecated := "" + if c.Enabled == ShowDeprecated { + deprecated = "(deprecated) " + } + flags.Bool(c.Prefix+tlsEnabled, false, deprecated+"Enable TLS when talking to the remote server(s)") flags.Bool(c.Prefix+tlsEnabledOld, false, "(deprecated) see --"+c.Prefix+tlsEnabled) } flags.String(c.Prefix+tlsCA, "", "Path to a TLS CA (Certification Authority) file used to verify the remote server(s) (by default will use the system truststore)") @@ -64,8 +76,12 @@ func (c ClientFlagsConfig) AddFlags(flags *flag.FlagSet) { // AddFlags adds flags for TLS to the FlagSet. func (c ServerFlagsConfig) AddFlags(flags *flag.FlagSet) { - if c.ShowEnabled { - flags.Bool(c.Prefix+tlsEnabled, false, "Enable TLS on the server") + if c.ShowEnabled >= Show { + deprecated := "" + if c.ShowEnabled == ShowDeprecated { + deprecated = "(deprecated) " + } + flags.Bool(c.Prefix+tlsEnabled, false, deprecated+"Enable TLS on the server") flags.Bool(c.Prefix+tlsEnabledOld, false, "(deprecated) see --"+c.Prefix+tlsEnabled) } flags.String(c.Prefix+tlsCert, "", "Path to a TLS Certificate file, used to identify this server to clients") @@ -77,7 +93,7 @@ func (c ServerFlagsConfig) AddFlags(flags *flag.FlagSet) { // InitFromViper creates tls.Config populated with values retrieved from Viper. func (c ClientFlagsConfig) InitFromViper(v *viper.Viper) Options { var p Options - if c.ShowEnabled { + if c.Enabled >= Show { p.Enabled = v.GetBool(c.Prefix + tlsEnabled) if !p.Enabled { @@ -97,7 +113,7 @@ func (c ClientFlagsConfig) InitFromViper(v *viper.Viper) Options { // InitFromViper creates tls.Config populated with values retrieved from Viper. func (c ServerFlagsConfig) InitFromViper(v *viper.Viper) Options { var p Options - if c.ShowEnabled { + if c.ShowEnabled >= Show { p.Enabled = v.GetBool(c.Prefix + tlsEnabled) if !p.Enabled { diff --git a/pkg/config/tlscfg/flags_test.go b/pkg/config/tlscfg/flags_test.go index f55ecbd819f..e952cb3e466 100644 --- a/pkg/config/tlscfg/flags_test.go +++ b/pkg/config/tlscfg/flags_test.go @@ -51,7 +51,7 @@ func TestClientFlags(t *testing.T) { flagSet := &flag.FlagSet{} flagCfg := ClientFlagsConfig{ Prefix: "prefix", - ShowEnabled: true, + Enabled: Show, ShowServerName: true, } flagCfg.AddFlags(flagSet) @@ -102,7 +102,7 @@ func TestServerFlags(t *testing.T) { flagSet := &flag.FlagSet{} flagCfg := ServerFlagsConfig{ Prefix: "prefix", - ShowEnabled: true, + ShowEnabled: Show, ShowClientCA: true, } flagCfg.AddFlags(flagSet) diff --git a/pkg/kafka/auth/config.go b/pkg/kafka/auth/config.go index e162a3f5dc5..89b63f256c5 100644 --- a/pkg/kafka/auth/config.go +++ b/pkg/kafka/auth/config.go @@ -20,6 +20,7 @@ import ( "github.com/Shopify/sarama" "github.com/spf13/viper" + "go.uber.org/zap" "github.com/jaegertracing/jaeger/pkg/config/tlscfg" ) @@ -80,19 +81,23 @@ func (config *AuthenticationConfig) InitFromViper(configPrefix string, v *viper. var tlsClientConfig = tlscfg.ClientFlagsConfig{ Prefix: configPrefix, - ShowEnabled: true, + Enabled: tlscfg.ShowDeprecated, ShowServerName: true, } config.TLS = tlsClientConfig.InitFromViper(v) - if config.Authentication == tls { - config.TLS.Enabled = true - } + config.PlainText.UserName = v.GetString(configPrefix + plainTextPrefix + suffixPlainTextUserName) + config.PlainText.Password = v.GetString(configPrefix + plainTextPrefix + suffixPlainTextPassword) +} + +// Normalize normalizes kafka options +func (config *AuthenticationConfig) Normalize(logger *zap.Logger) { if config.TLS.Enabled == true { + logger.Warn("Flag .tls.enabled is deprecated use " + suffixAuthentication + " instead.") config.Authentication = tls } - - config.PlainText.UserName = v.GetString(configPrefix + plainTextPrefix + suffixPlainTextUserName) - config.PlainText.Password = v.GetString(configPrefix + plainTextPrefix + suffixPlainTextPassword) + if config.Authentication == tls { + config.TLS.Enabled = true + } } diff --git a/pkg/kafka/auth/options.go b/pkg/kafka/auth/options.go index e2e7edb4804..0d8ab28c684 100644 --- a/pkg/kafka/auth/options.go +++ b/pkg/kafka/auth/options.go @@ -104,7 +104,7 @@ func AddFlags(configPrefix string, flagSet *flag.FlagSet) { tlsClientConfig := tlscfg.ClientFlagsConfig{ Prefix: configPrefix, - ShowEnabled: true, + Enabled: tlscfg.ShowDeprecated, ShowServerName: true, } tlsClientConfig.AddFlags(flagSet) diff --git a/plugin/storage/cassandra/options.go b/plugin/storage/cassandra/options.go index f0307f7ad7a..6b3dc69845e 100644 --- a/plugin/storage/cassandra/options.go +++ b/plugin/storage/cassandra/options.go @@ -238,7 +238,7 @@ func (opt *Options) InitFromViper(v *viper.Viper) { func tlsFlagsConfig(namespace string) tlscfg.ClientFlagsConfig { return tlscfg.ClientFlagsConfig{ Prefix: namespace, - ShowEnabled: true, + Enabled: tlscfg.Show, ShowServerName: true, } } diff --git a/plugin/storage/es/options.go b/plugin/storage/es/options.go index ab38acf051c..800dbb147c7 100644 --- a/plugin/storage/es/options.go +++ b/plugin/storage/es/options.go @@ -111,7 +111,7 @@ func NewOptions(primaryNamespace string, otherNamespaces ...string) *Options { func (config *namespaceConfig) getTLSFlagsConfig() tlscfg.ClientFlagsConfig { return tlscfg.ClientFlagsConfig{ Prefix: config.namespace, - ShowEnabled: true, + Enabled: tlscfg.Show, ShowServerName: true, } } diff --git a/plugin/storage/integration/kafka_test.go b/plugin/storage/integration/kafka_test.go index 667646d81f7..718eca04109 100644 --- a/plugin/storage/integration/kafka_test.go +++ b/plugin/storage/integration/kafka_test.go @@ -91,7 +91,7 @@ func (s *KafkaIntegrationTestSuite) initialize() error { return err } options := app.Options{} - options.InitFromViper(v) + options.InitFromViper(v, s.logger) traceStore := memory.NewStore() spanConsumer, err := builder.CreateConsumer(s.logger, metrics.NullFactory, traceStore, options) if err != nil { diff --git a/plugin/storage/kafka/factory.go b/plugin/storage/kafka/factory.go index 51c8ae5220a..cd37168a354 100644 --- a/plugin/storage/kafka/factory.go +++ b/plugin/storage/kafka/factory.go @@ -17,7 +17,6 @@ package kafka import ( "errors" "flag" - "github.com/Shopify/sarama" "github.com/spf13/viper" "github.com/uber/jaeger-lib/metrics" @@ -62,6 +61,7 @@ func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) logger.Info("Kafka factory", zap.Any("producer builder", f.Builder), zap.Any("topic", f.options.topic)) + f.options.config.Normalize(logger) p, err := f.NewProducer() if err != nil { return err diff --git a/plugin/storage/kafka/options_test.go b/plugin/storage/kafka/options_test.go index 6173a87e08b..105d0e80416 100644 --- a/plugin/storage/kafka/options_test.go +++ b/plugin/storage/kafka/options_test.go @@ -22,6 +22,7 @@ import ( "github.com/Shopify/sarama" "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/config/tlscfg" @@ -202,6 +203,7 @@ func TestTLSFlags(t *testing.T) { err := command.ParseFlags(test.flags) require.NoError(t, err) o.InitFromViper(v) + o.config.Normalize(zap.NewNop()) assert.Equal(t, test.expected, o.config.AuthenticationConfig) }) From 5dc88cb0b94fcd6b8306b4c4014cdf1f41c50f97 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Tue, 3 Mar 2020 16:37:14 +0100 Subject: [PATCH 4/7] Fix fmt and lint Signed-off-by: Pavol Loffay --- pkg/config/tlscfg/flags.go | 4 ++++ pkg/kafka/auth/config.go | 2 +- plugin/storage/kafka/factory.go | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/config/tlscfg/flags.go b/pkg/config/tlscfg/flags.go index c8f635b9f92..77cab2a4cd7 100644 --- a/pkg/config/tlscfg/flags.go +++ b/pkg/config/tlscfg/flags.go @@ -33,11 +33,15 @@ const ( tlsSkipHostVerify = tlsPrefix + ".skip-host-verify" ) +// Enabled configures TLS enabled flags type Enabled int const ( + // Hide hides the enabled TLS flags Hide Enabled = iota + // Show shows the enabled TLS flags Show + // ShowDeprecated shows deprecation annotation for tls flags ShowDeprecated ) diff --git a/pkg/kafka/auth/config.go b/pkg/kafka/auth/config.go index 89b63f256c5..27e2446eed3 100644 --- a/pkg/kafka/auth/config.go +++ b/pkg/kafka/auth/config.go @@ -93,7 +93,7 @@ func (config *AuthenticationConfig) InitFromViper(configPrefix string, v *viper. // Normalize normalizes kafka options func (config *AuthenticationConfig) Normalize(logger *zap.Logger) { - if config.TLS.Enabled == true { + if config.TLS.Enabled { logger.Warn("Flag .tls.enabled is deprecated use " + suffixAuthentication + " instead.") config.Authentication = tls } diff --git a/plugin/storage/kafka/factory.go b/plugin/storage/kafka/factory.go index cd37168a354..ef3830c4fb2 100644 --- a/plugin/storage/kafka/factory.go +++ b/plugin/storage/kafka/factory.go @@ -17,6 +17,7 @@ package kafka import ( "errors" "flag" + "github.com/Shopify/sarama" "github.com/spf13/viper" "github.com/uber/jaeger-lib/metrics" From 4c760ada63aaa80e6f5ffb363b95c8b627fa87ce Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Tue, 3 Mar 2020 16:54:30 +0100 Subject: [PATCH 5/7] Remove deprecated TLS enabled flag Signed-off-by: Pavol Loffay --- cmd/agent/app/reporter/grpc/flags.go | 2 +- cmd/collector/app/builder_flags.go | 2 +- cmd/ingester/app/flags.go | 4 +-- cmd/ingester/app/flags_test.go | 7 ++--- cmd/ingester/main.go | 2 +- pkg/config/tlscfg/flags.go | 36 ++++++------------------ pkg/config/tlscfg/flags_test.go | 4 +-- pkg/kafka/auth/config.go | 14 +++------ pkg/kafka/auth/options.go | 2 +- plugin/storage/cassandra/options.go | 2 +- plugin/storage/es/options.go | 2 +- plugin/storage/integration/kafka_test.go | 2 +- plugin/storage/kafka/factory.go | 1 - plugin/storage/kafka/options_test.go | 2 -- 14 files changed, 25 insertions(+), 57 deletions(-) diff --git a/cmd/agent/app/reporter/grpc/flags.go b/cmd/agent/app/reporter/grpc/flags.go index 5a3879df81e..e557a8be6fb 100644 --- a/cmd/agent/app/reporter/grpc/flags.go +++ b/cmd/agent/app/reporter/grpc/flags.go @@ -33,7 +33,7 @@ const ( var tlsFlagsConfig = tlscfg.ClientFlagsConfig{ Prefix: gRPCPrefix, - Enabled: tlscfg.Show, + ShowEnabled: true, ShowServerName: true, } diff --git a/cmd/collector/app/builder_flags.go b/cmd/collector/app/builder_flags.go index c45c1e1c784..ca207f6b37e 100644 --- a/cmd/collector/app/builder_flags.go +++ b/cmd/collector/app/builder_flags.go @@ -40,7 +40,7 @@ const ( var tlsFlagsConfig = tlscfg.ServerFlagsConfig{ Prefix: "collector.grpc", - ShowEnabled: tlscfg.Show, + ShowEnabled: true, ShowClientCA: true, } diff --git a/cmd/ingester/app/flags.go b/cmd/ingester/app/flags.go index 500e173cfe6..5aca213bf2a 100644 --- a/cmd/ingester/app/flags.go +++ b/cmd/ingester/app/flags.go @@ -22,7 +22,6 @@ import ( "time" "github.com/spf13/viper" - "go.uber.org/zap" "github.com/jaegertracing/jaeger/pkg/kafka/auth" kafkaConsumer "github.com/jaegertracing/jaeger/pkg/kafka/consumer" @@ -115,7 +114,7 @@ func AddFlags(flagSet *flag.FlagSet) { } // InitFromViper initializes Builder with properties from viper -func (o *Options) InitFromViper(v *viper.Viper, logger *zap.Logger) { +func (o *Options) InitFromViper(v *viper.Viper) { o.Brokers = strings.Split(stripWhiteSpace(v.GetString(KafkaConsumerConfigPrefix+SuffixBrokers)), ",") o.Topic = v.GetString(KafkaConsumerConfigPrefix + SuffixTopic) o.GroupID = v.GetString(KafkaConsumerConfigPrefix + SuffixGroupID) @@ -127,7 +126,6 @@ func (o *Options) InitFromViper(v *viper.Viper, logger *zap.Logger) { o.DeadlockInterval = v.GetDuration(ConfigPrefix + SuffixDeadlockInterval) authenticationOptions := auth.AuthenticationConfig{} authenticationOptions.InitFromViper(KafkaConsumerConfigPrefix, v) - authenticationOptions.Normalize(logger) o.AuthenticationConfig = authenticationOptions } diff --git a/cmd/ingester/app/flags_test.go b/cmd/ingester/app/flags_test.go index 341fedef107..7f910525df3 100644 --- a/cmd/ingester/app/flags_test.go +++ b/cmd/ingester/app/flags_test.go @@ -21,7 +21,6 @@ import ( "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/config/tlscfg" @@ -42,7 +41,7 @@ func TestOptionsWithFlags(t *testing.T) { "--ingester.parallelism=5", "--ingester.deadlockInterval=2m", }) - o.InitFromViper(v, zap.NewNop()) + o.InitFromViper(v) assert.Equal(t, "topic1", o.Topic) assert.Equal(t, []string{"127.0.0.1:9092", "0.0.0:1234"}, o.Brokers) @@ -87,7 +86,7 @@ func TestTLSFlags(t *testing.T) { v, command := config.Viperize(AddFlags) err := command.ParseFlags(test.flags) require.NoError(t, err) - o.InitFromViper(v, zap.NewNop()) + o.InitFromViper(v) assert.Equal(t, test.expected, o.AuthenticationConfig) }) } @@ -97,7 +96,7 @@ func TestFlagDefaults(t *testing.T) { o := &Options{} v, command := config.Viperize(AddFlags) command.ParseFlags([]string{}) - o.InitFromViper(v, zap.NewNop()) + o.InitFromViper(v) assert.Equal(t, DefaultTopic, o.Topic) assert.Equal(t, []string{DefaultBroker}, o.Brokers) diff --git a/cmd/ingester/main.go b/cmd/ingester/main.go index 8a5e2729f33..f2179654498 100644 --- a/cmd/ingester/main.go +++ b/cmd/ingester/main.go @@ -68,7 +68,7 @@ func main() { } options := app.Options{} - options.InitFromViper(v, logger) + options.InitFromViper(v) consumer, err := builder.CreateConsumer(logger, metricsFactory, spanWriter, options) if err != nil { logger.Fatal("Unable to create consumer", zap.Error(err)) diff --git a/pkg/config/tlscfg/flags.go b/pkg/config/tlscfg/flags.go index 77cab2a4cd7..266fa62372b 100644 --- a/pkg/config/tlscfg/flags.go +++ b/pkg/config/tlscfg/flags.go @@ -33,40 +33,24 @@ const ( tlsSkipHostVerify = tlsPrefix + ".skip-host-verify" ) -// Enabled configures TLS enabled flags -type Enabled int - -const ( - // Hide hides the enabled TLS flags - Hide Enabled = iota - // Show shows the enabled TLS flags - Show - // ShowDeprecated shows deprecation annotation for tls flags - ShowDeprecated -) - // ClientFlagsConfig describes which CLI flags for TLS client should be generated. type ClientFlagsConfig struct { Prefix string - Enabled Enabled + ShowEnabled bool ShowServerName bool } // ServerFlagsConfig describes which CLI flags for TLS server should be generated. type ServerFlagsConfig struct { Prefix string - ShowEnabled Enabled + ShowEnabled bool ShowClientCA bool } // AddFlags adds flags for TLS to the FlagSet. func (c ClientFlagsConfig) AddFlags(flags *flag.FlagSet) { - if c.Enabled >= Show { - deprecated := "" - if c.Enabled == ShowDeprecated { - deprecated = "(deprecated) " - } - flags.Bool(c.Prefix+tlsEnabled, false, deprecated+"Enable TLS when talking to the remote server(s)") + if c.ShowEnabled { + flags.Bool(c.Prefix+tlsEnabled, false, "Enable TLS when talking to the remote server(s)") flags.Bool(c.Prefix+tlsEnabledOld, false, "(deprecated) see --"+c.Prefix+tlsEnabled) } flags.String(c.Prefix+tlsCA, "", "Path to a TLS CA (Certification Authority) file used to verify the remote server(s) (by default will use the system truststore)") @@ -80,12 +64,8 @@ func (c ClientFlagsConfig) AddFlags(flags *flag.FlagSet) { // AddFlags adds flags for TLS to the FlagSet. func (c ServerFlagsConfig) AddFlags(flags *flag.FlagSet) { - if c.ShowEnabled >= Show { - deprecated := "" - if c.ShowEnabled == ShowDeprecated { - deprecated = "(deprecated) " - } - flags.Bool(c.Prefix+tlsEnabled, false, deprecated+"Enable TLS on the server") + if c.ShowEnabled { + flags.Bool(c.Prefix+tlsEnabled, false, "Enable TLS on the server") flags.Bool(c.Prefix+tlsEnabledOld, false, "(deprecated) see --"+c.Prefix+tlsEnabled) } flags.String(c.Prefix+tlsCert, "", "Path to a TLS Certificate file, used to identify this server to clients") @@ -97,7 +77,7 @@ func (c ServerFlagsConfig) AddFlags(flags *flag.FlagSet) { // InitFromViper creates tls.Config populated with values retrieved from Viper. func (c ClientFlagsConfig) InitFromViper(v *viper.Viper) Options { var p Options - if c.Enabled >= Show { + if c.ShowEnabled { p.Enabled = v.GetBool(c.Prefix + tlsEnabled) if !p.Enabled { @@ -117,7 +97,7 @@ func (c ClientFlagsConfig) InitFromViper(v *viper.Viper) Options { // InitFromViper creates tls.Config populated with values retrieved from Viper. func (c ServerFlagsConfig) InitFromViper(v *viper.Viper) Options { var p Options - if c.ShowEnabled >= Show { + if c.ShowEnabled { p.Enabled = v.GetBool(c.Prefix + tlsEnabled) if !p.Enabled { diff --git a/pkg/config/tlscfg/flags_test.go b/pkg/config/tlscfg/flags_test.go index e952cb3e466..f55ecbd819f 100644 --- a/pkg/config/tlscfg/flags_test.go +++ b/pkg/config/tlscfg/flags_test.go @@ -51,7 +51,7 @@ func TestClientFlags(t *testing.T) { flagSet := &flag.FlagSet{} flagCfg := ClientFlagsConfig{ Prefix: "prefix", - Enabled: Show, + ShowEnabled: true, ShowServerName: true, } flagCfg.AddFlags(flagSet) @@ -102,7 +102,7 @@ func TestServerFlags(t *testing.T) { flagSet := &flag.FlagSet{} flagCfg := ServerFlagsConfig{ Prefix: "prefix", - ShowEnabled: Show, + ShowEnabled: true, ShowClientCA: true, } flagCfg.AddFlags(flagSet) diff --git a/pkg/kafka/auth/config.go b/pkg/kafka/auth/config.go index 27e2446eed3..cc30c747122 100644 --- a/pkg/kafka/auth/config.go +++ b/pkg/kafka/auth/config.go @@ -20,7 +20,6 @@ import ( "github.com/Shopify/sarama" "github.com/spf13/viper" - "go.uber.org/zap" "github.com/jaegertracing/jaeger/pkg/config/tlscfg" ) @@ -81,23 +80,18 @@ func (config *AuthenticationConfig) InitFromViper(configPrefix string, v *viper. var tlsClientConfig = tlscfg.ClientFlagsConfig{ Prefix: configPrefix, - Enabled: tlscfg.ShowDeprecated, + ShowEnabled: true, ShowServerName: true, } config.TLS = tlsClientConfig.InitFromViper(v) - - config.PlainText.UserName = v.GetString(configPrefix + plainTextPrefix + suffixPlainTextUserName) - config.PlainText.Password = v.GetString(configPrefix + plainTextPrefix + suffixPlainTextPassword) -} - -// Normalize normalizes kafka options -func (config *AuthenticationConfig) Normalize(logger *zap.Logger) { if config.TLS.Enabled { - logger.Warn("Flag .tls.enabled is deprecated use " + suffixAuthentication + " instead.") config.Authentication = tls } if config.Authentication == tls { config.TLS.Enabled = true } + + config.PlainText.UserName = v.GetString(configPrefix + plainTextPrefix + suffixPlainTextUserName) + config.PlainText.Password = v.GetString(configPrefix + plainTextPrefix + suffixPlainTextPassword) } diff --git a/pkg/kafka/auth/options.go b/pkg/kafka/auth/options.go index 0d8ab28c684..e2e7edb4804 100644 --- a/pkg/kafka/auth/options.go +++ b/pkg/kafka/auth/options.go @@ -104,7 +104,7 @@ func AddFlags(configPrefix string, flagSet *flag.FlagSet) { tlsClientConfig := tlscfg.ClientFlagsConfig{ Prefix: configPrefix, - Enabled: tlscfg.ShowDeprecated, + ShowEnabled: true, ShowServerName: true, } tlsClientConfig.AddFlags(flagSet) diff --git a/plugin/storage/cassandra/options.go b/plugin/storage/cassandra/options.go index 6b3dc69845e..f0307f7ad7a 100644 --- a/plugin/storage/cassandra/options.go +++ b/plugin/storage/cassandra/options.go @@ -238,7 +238,7 @@ func (opt *Options) InitFromViper(v *viper.Viper) { func tlsFlagsConfig(namespace string) tlscfg.ClientFlagsConfig { return tlscfg.ClientFlagsConfig{ Prefix: namespace, - Enabled: tlscfg.Show, + ShowEnabled: true, ShowServerName: true, } } diff --git a/plugin/storage/es/options.go b/plugin/storage/es/options.go index 800dbb147c7..ab38acf051c 100644 --- a/plugin/storage/es/options.go +++ b/plugin/storage/es/options.go @@ -111,7 +111,7 @@ func NewOptions(primaryNamespace string, otherNamespaces ...string) *Options { func (config *namespaceConfig) getTLSFlagsConfig() tlscfg.ClientFlagsConfig { return tlscfg.ClientFlagsConfig{ Prefix: config.namespace, - Enabled: tlscfg.Show, + ShowEnabled: true, ShowServerName: true, } } diff --git a/plugin/storage/integration/kafka_test.go b/plugin/storage/integration/kafka_test.go index 718eca04109..667646d81f7 100644 --- a/plugin/storage/integration/kafka_test.go +++ b/plugin/storage/integration/kafka_test.go @@ -91,7 +91,7 @@ func (s *KafkaIntegrationTestSuite) initialize() error { return err } options := app.Options{} - options.InitFromViper(v, s.logger) + options.InitFromViper(v) traceStore := memory.NewStore() spanConsumer, err := builder.CreateConsumer(s.logger, metrics.NullFactory, traceStore, options) if err != nil { diff --git a/plugin/storage/kafka/factory.go b/plugin/storage/kafka/factory.go index ef3830c4fb2..51c8ae5220a 100644 --- a/plugin/storage/kafka/factory.go +++ b/plugin/storage/kafka/factory.go @@ -62,7 +62,6 @@ func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) logger.Info("Kafka factory", zap.Any("producer builder", f.Builder), zap.Any("topic", f.options.topic)) - f.options.config.Normalize(logger) p, err := f.NewProducer() if err != nil { return err diff --git a/plugin/storage/kafka/options_test.go b/plugin/storage/kafka/options_test.go index 105d0e80416..6173a87e08b 100644 --- a/plugin/storage/kafka/options_test.go +++ b/plugin/storage/kafka/options_test.go @@ -22,7 +22,6 @@ import ( "github.com/Shopify/sarama" "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/config/tlscfg" @@ -203,7 +202,6 @@ func TestTLSFlags(t *testing.T) { err := command.ParseFlags(test.flags) require.NoError(t, err) o.InitFromViper(v) - o.config.Normalize(zap.NewNop()) assert.Equal(t, test.expected, o.config.AuthenticationConfig) }) From 7e8c615a911a8ed7572bf097486949d4d1ca2969 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Wed, 4 Mar 2020 15:05:47 +0100 Subject: [PATCH 6/7] Review comments Signed-off-by: Pavol Loffay --- cmd/ingester/app/flags_test.go | 11 ++++++----- pkg/kafka/auth/config.go | 11 ++++++----- plugin/storage/kafka/options_test.go | 11 ++++++----- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/cmd/ingester/app/flags_test.go b/cmd/ingester/app/flags_test.go index 7f910525df3..93be0d13f54 100644 --- a/cmd/ingester/app/flags_test.go +++ b/cmd/ingester/app/flags_test.go @@ -54,29 +54,30 @@ func TestOptionsWithFlags(t *testing.T) { } func TestTLSFlags(t *testing.T) { + kerb := auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"} tests := []struct { flags []string expected auth.AuthenticationConfig }{ { flags: []string{}, - expected: auth.AuthenticationConfig{Authentication: "none", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}}, + expected: auth.AuthenticationConfig{Authentication: "none", Kerberos: kerb}, }, { flags: []string{"--kafka.consumer.authentication=foo"}, - expected: auth.AuthenticationConfig{Authentication: "foo", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}}, + expected: auth.AuthenticationConfig{Authentication: "foo", Kerberos: kerb}, }, { flags: []string{"--kafka.consumer.authentication=kerberos", "--kafka.consumer.tls.enabled=true"}, - expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}, TLS: tlscfg.Options{Enabled: true}}, + expected: auth.AuthenticationConfig{Authentication: "kerberos", Kerberos: kerb, TLS: tlscfg.Options{Enabled: true}}, }, { flags: []string{"--kafka.consumer.authentication=tls"}, - expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}, TLS: tlscfg.Options{Enabled: true}}, + expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: kerb, TLS: tlscfg.Options{Enabled: true}}, }, { flags: []string{"--kafka.consumer.authentication=tls", "--kafka.consumer.tls.enabled=false"}, - expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}, TLS: tlscfg.Options{Enabled: true}}, + expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: kerb, TLS: tlscfg.Options{Enabled: true}}, }, } diff --git a/pkg/kafka/auth/config.go b/pkg/kafka/auth/config.go index cc30c747122..02034954728 100644 --- a/pkg/kafka/auth/config.go +++ b/pkg/kafka/auth/config.go @@ -51,14 +51,18 @@ func (config *AuthenticationConfig) SetConfiguration(saramaConfig *sarama.Config if strings.Trim(authentication, " ") == "" { authentication = none } + if config.Authentication == tls || config.TLS.Enabled { + err := setTLSConfiguration(&config.TLS, saramaConfig) + if err != nil { + return err + } + } switch authentication { case none: return nil case kerberos: setKerberosConfiguration(&config.Kerberos, saramaConfig) return nil - case tls: - return setTLSConfiguration(&config.TLS, saramaConfig) case plaintext: setPlainTextConfiguration(&config.PlainText, saramaConfig) return nil @@ -85,9 +89,6 @@ func (config *AuthenticationConfig) InitFromViper(configPrefix string, v *viper. } config.TLS = tlsClientConfig.InitFromViper(v) - if config.TLS.Enabled { - config.Authentication = tls - } if config.Authentication == tls { config.TLS.Enabled = true } diff --git a/plugin/storage/kafka/options_test.go b/plugin/storage/kafka/options_test.go index 6173a87e08b..90b5632e5c0 100644 --- a/plugin/storage/kafka/options_test.go +++ b/plugin/storage/kafka/options_test.go @@ -169,29 +169,30 @@ func TestRequiredAcksFailures(t *testing.T) { } func TestTLSFlags(t *testing.T) { + kerb := auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"} tests := []struct { flags []string expected auth.AuthenticationConfig }{ { flags: []string{}, - expected: auth.AuthenticationConfig{Authentication: "none", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}}, + expected: auth.AuthenticationConfig{Authentication: "none", Kerberos: kerb}, }, { flags: []string{"--kafka.producer.authentication=foo"}, - expected: auth.AuthenticationConfig{Authentication: "foo", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}}, + expected: auth.AuthenticationConfig{Authentication: "foo", Kerberos: kerb}, }, { flags: []string{"--kafka.producer.authentication=kerberos", "--kafka.producer.tls.enabled=true"}, - expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}, TLS: tlscfg.Options{Enabled: true}}, + expected: auth.AuthenticationConfig{Authentication: "kerberos", Kerberos: kerb, TLS: tlscfg.Options{Enabled: true}}, }, { flags: []string{"--kafka.producer.authentication=tls"}, - expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}, TLS: tlscfg.Options{Enabled: true}}, + expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: kerb, TLS: tlscfg.Options{Enabled: true}}, }, { flags: []string{"--kafka.producer.authentication=tls", "--kafka.producer.tls.enabled=false"}, - expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: auth.KerberosConfig{ServiceName: "kafka", ConfigPath: "/etc/krb5.conf", KeyTabPath: "/etc/security/kafka.keytab"}, TLS: tlscfg.Options{Enabled: true}}, + expected: auth.AuthenticationConfig{Authentication: "tls", Kerberos: kerb, TLS: tlscfg.Options{Enabled: true}}, }, } From 1f1a6f6b1c54d8eb1056bb1681c0420bb74e406c Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Thu, 5 Mar 2020 11:49:55 +0100 Subject: [PATCH 7/7] Add tls case back Signed-off-by: Pavol Loffay --- pkg/kafka/auth/config.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/kafka/auth/config.go b/pkg/kafka/auth/config.go index 02034954728..2221cd9d7a1 100644 --- a/pkg/kafka/auth/config.go +++ b/pkg/kafka/auth/config.go @@ -60,6 +60,8 @@ func (config *AuthenticationConfig) SetConfiguration(saramaConfig *sarama.Config switch authentication { case none: return nil + case tls: + return nil case kerberos: setKerberosConfiguration(&config.Kerberos, saramaConfig) return nil