From f92ac6ab4eda8c8353c3abe4bfca5afd668eb9b9 Mon Sep 17 00:00:00 2001
From: Rajdeep Kaur <rajdeep51994@gmail.com>
Date: Sun, 20 Jun 2021 21:08:43 +0100
Subject: [PATCH 1/8] Perform log.fatal if TLS flags are used when
 tls.enabled=false(#2893)

Signed-off-by: Rajdeep Kaur <rajdeep51994@gmail.com>
---
 cmd/agent/app/builder_test.go                 |  3 +-
 cmd/agent/app/reporter/grpc/flags.go          | 10 ++-
 cmd/agent/app/reporter/grpc/flags_test.go     |  3 +-
 cmd/agent/main.go                             |  5 +-
 cmd/all-in-one/main.go                        | 15 +++-
 cmd/collector/app/builder_flags.go            | 17 ++--
 .../app/span_handler_builder_test.go          |  3 +-
 cmd/collector/main.go                         |  5 +-
 cmd/query/app/flags.go                        | 16 +++-
 cmd/query/app/flags_test.go                   | 13 ++-
 cmd/query/main.go                             |  5 +-
 pkg/config/tlscfg/flags.go                    | 17 +++-
 pkg/config/tlscfg/flags_test.go               | 82 ++++++++++++++++++-
 pkg/kafka/auth/config.go                      |  2 +-
 plugin/metrics/prometheus/options.go          |  9 +-
 plugin/storage/cassandra/options.go           |  9 +-
 plugin/storage/es/options.go                  | 10 ++-
 17 files changed, 186 insertions(+), 38 deletions(-)

diff --git a/cmd/agent/app/builder_test.go b/cmd/agent/app/builder_test.go
index 49c2e9d58be..425381fa11c 100644
--- a/cmd/agent/app/builder_test.go
+++ b/cmd/agent/app/builder_test.go
@@ -245,7 +245,8 @@ func TestCreateCollectorProxy(t *testing.T) {
 		require.NoError(t, err)
 
 		rOpts := new(reporter.Options).InitFromViper(v, zap.NewNop())
-		grpcBuilder := grpc.NewConnBuilder().InitFromViper(v)
+		grpcBuilder, err := grpc.NewConnBuilder().InitFromViper(v)
+		require.NoError(t, err)
 
 		metricsFactory := metricstest.NewFactory(time.Microsecond)
 
diff --git a/cmd/agent/app/reporter/grpc/flags.go b/cmd/agent/app/reporter/grpc/flags.go
index 1d63d879b83..48be9aeffcc 100644
--- a/cmd/agent/app/reporter/grpc/flags.go
+++ b/cmd/agent/app/reporter/grpc/flags.go
@@ -44,13 +44,17 @@ func AddFlags(flags *flag.FlagSet) {
 }
 
 // InitFromViper initializes Options with properties retrieved from Viper.
-func (b *ConnBuilder) InitFromViper(v *viper.Viper) *ConnBuilder {
+func (b *ConnBuilder) InitFromViper(v *viper.Viper) (*ConnBuilder, error) {
 	hostPorts := v.GetString(collectorHostPort)
 	if hostPorts != "" {
 		b.CollectorHostPorts = strings.Split(hostPorts, ",")
 	}
 	b.MaxRetry = uint(v.GetInt(retry))
-	b.TLS = tlsFlagsConfig.InitFromViper(v)
+	if tls, err := tlsFlagsConfig.InitFromViper(v); err == nil {
+		b.TLS = tls
+	} else {
+		return b, err
+	}
 	b.DiscoveryMinPeers = v.GetInt(discoveryMinPeers)
-	return b
+	return b, nil
 }
diff --git a/cmd/agent/app/reporter/grpc/flags_test.go b/cmd/agent/app/reporter/grpc/flags_test.go
index cf082cd0b0c..cc879837e92 100644
--- a/cmd/agent/app/reporter/grpc/flags_test.go
+++ b/cmd/agent/app/reporter/grpc/flags_test.go
@@ -46,7 +46,8 @@ func TestBindFlags(t *testing.T) {
 
 		err := command.ParseFlags(test.cOpts)
 		require.NoError(t, err)
-		b := new(ConnBuilder).InitFromViper(v)
+		b, err := new(ConnBuilder).InitFromViper(v)
+		require.NoError(t, err)
 		assert.Equal(t, test.expected, b)
 	}
 }
diff --git a/cmd/agent/main.go b/cmd/agent/main.go
index cb63ce2c9d4..d8f525cb61c 100644
--- a/cmd/agent/main.go
+++ b/cmd/agent/main.go
@@ -61,7 +61,10 @@ func main() {
 			version.NewInfoMetrics(mFactory)
 
 			rOpts := new(reporter.Options).InitFromViper(v, logger)
-			grpcBuilder := grpc.NewConnBuilder().InitFromViper(v)
+			grpcBuilder, err := grpc.NewConnBuilder().InitFromViper(v)
+			if err != nil {
+				logger.Fatal("Failed to configure connection for grpc", zap.Error(err))
+			}
 			builders := map[reporter.Type]app.CollectorProxyBuilder{
 				reporter.GRPC: app.GRPCCollectorProxyBuilder(grpcBuilder),
 			}
diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go
index aed66c2ced2..82192faa6da 100644
--- a/cmd/all-in-one/main.go
+++ b/cmd/all-in-one/main.go
@@ -132,9 +132,18 @@ by default uses only in-memory database.`,
 
 			aOpts := new(agentApp.Builder).InitFromViper(v)
 			repOpts := new(agentRep.Options).InitFromViper(v, logger)
-			grpcBuilder := agentGrpcRep.NewConnBuilder().InitFromViper(v)
-			cOpts := new(collectorApp.CollectorOptions).InitFromViper(v)
-			qOpts := new(queryApp.QueryOptions).InitFromViper(v, logger)
+			grpcBuilder, err := agentGrpcRep.NewConnBuilder().InitFromViper(v)
+			if err != nil {
+				logger.Fatal("Failed to configure connection for grpc", zap.Error(err))
+			}
+			cOpts, err := new(collectorApp.CollectorOptions).InitFromViper(v)
+			if err != nil {
+				logger.Fatal("Failed to initialize collector", zap.Error(err))
+			}
+			qOpts, err := new(queryApp.QueryOptions).InitFromViper(v, logger)
+			if err != nil {
+				logger.Fatal("Failed to configure query service", zap.Error(err))
+			}
 
 			// collector
 			c := collectorApp.New(&collectorApp.CollectorParams{
diff --git a/cmd/collector/app/builder_flags.go b/cmd/collector/app/builder_flags.go
index 8599d864549..9b98c655eb2 100644
--- a/cmd/collector/app/builder_flags.go
+++ b/cmd/collector/app/builder_flags.go
@@ -88,7 +88,7 @@ func AddFlags(flags *flag.FlagSet) {
 }
 
 // InitFromViper initializes CollectorOptions with properties from viper
-func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper) *CollectorOptions {
+func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper) (*CollectorOptions, error) {
 	cOpts.CollectorGRPCHostPort = ports.FormatHostPort(v.GetString(collectorGRPCHostPort))
 	cOpts.CollectorHTTPHostPort = ports.FormatHostPort(v.GetString(collectorHTTPHostPort))
 	cOpts.CollectorTags = flags.ParseJaegerTags(v.GetString(collectorTags))
@@ -98,8 +98,15 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper) *CollectorOptions {
 	cOpts.DynQueueSizeMemory = v.GetUint(collectorDynQueueSizeMemory) * 1024 * 1024 // we receive in MiB and store in bytes
 	cOpts.NumWorkers = v.GetInt(collectorNumWorkers)
 	cOpts.QueueSize = v.GetInt(collectorQueueSize)
-	cOpts.TLSGRPC = tlsGRPCFlagsConfig.InitFromViper(v)
-	cOpts.TLSHTTP = tlsHTTPFlagsConfig.InitFromViper(v)
-
-	return cOpts
+	if tlsGrpc, err := tlsGRPCFlagsConfig.InitFromViper(v); err != nil {
+		cOpts.TLSGRPC = tlsGrpc
+	} else {
+		return cOpts, err
+	}
+	if tlsHTTP, err := tlsGRPCFlagsConfig.InitFromViper(v); err != nil {
+		cOpts.TLSHTTP = tlsHTTP
+	} else {
+		return cOpts, err
+	}
+	return cOpts, nil
 }
diff --git a/cmd/collector/app/span_handler_builder_test.go b/cmd/collector/app/span_handler_builder_test.go
index 772f9ce0835..cac63f818ec 100644
--- a/cmd/collector/app/span_handler_builder_test.go
+++ b/cmd/collector/app/span_handler_builder_test.go
@@ -32,7 +32,8 @@ func TestNewSpanHandlerBuilder(t *testing.T) {
 	v, command := config.Viperize(flags.AddFlags, AddFlags)
 
 	require.NoError(t, command.ParseFlags([]string{}))
-	cOpts := new(CollectorOptions).InitFromViper(v)
+	cOpts, err := new(CollectorOptions).InitFromViper(v)
+	require.NoError(t, err)
 
 	spanWriter := memory.NewStore()
 
diff --git a/cmd/collector/main.go b/cmd/collector/main.go
index 08172f489bd..f477bbe4933 100644
--- a/cmd/collector/main.go
+++ b/cmd/collector/main.go
@@ -97,7 +97,10 @@ func main() {
 				StrategyStore:  strategyStore,
 				HealthCheck:    svc.HC(),
 			})
-			collectorOpts := new(app.CollectorOptions).InitFromViper(v)
+			collectorOpts, err := new(app.CollectorOptions).InitFromViper(v)
+			if err != nil {
+				logger.Fatal("Failed to initialize collector", zap.Error(err))
+			}
 			if err := c.Start(collectorOpts); err != nil {
 				logger.Fatal("Failed to start collector", zap.Error(err))
 			}
diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go
index 2225fd31150..e63f06fb89b 100644
--- a/cmd/query/app/flags.go
+++ b/cmd/query/app/flags.go
@@ -96,11 +96,19 @@ func AddFlags(flagSet *flag.FlagSet) {
 }
 
 // InitFromViper initializes QueryOptions with properties from viper
-func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) *QueryOptions {
+func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) (*QueryOptions, error) {
 	qOpts.HTTPHostPort = v.GetString(queryHTTPHostPort)
 	qOpts.GRPCHostPort = v.GetString(queryGRPCHostPort)
-	qOpts.TLSGRPC = tlsGRPCFlagsConfig.InitFromViper(v)
-	qOpts.TLSHTTP = tlsHTTPFlagsConfig.InitFromViper(v)
+	if tlsGrpc, err := tlsGRPCFlagsConfig.InitFromViper(v); err == nil {
+		qOpts.TLSGRPC = tlsGrpc
+	} else {
+		return qOpts, err
+	}
+	if tlsHTTP, err := tlsGRPCFlagsConfig.InitFromViper(v); err == nil {
+		qOpts.TLSHTTP = tlsHTTP
+	} else {
+		return qOpts, err
+	}
 	qOpts.BasePath = v.GetString(queryBasePath)
 	qOpts.StaticAssets = v.GetString(queryStaticFiles)
 	qOpts.UIConfig = v.GetString(queryUIConfig)
@@ -114,7 +122,7 @@ func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) *Qu
 	} else {
 		qOpts.AdditionalHeaders = headers
 	}
-	return qOpts
+	return qOpts, nil
 }
 
 // BuildQueryServiceOptions creates a QueryServiceOptions struct with appropriate adjusters and archive config
diff --git a/cmd/query/app/flags_test.go b/cmd/query/app/flags_test.go
index ca3e3d3c7c4..f125503baa1 100644
--- a/cmd/query/app/flags_test.go
+++ b/cmd/query/app/flags_test.go
@@ -21,6 +21,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
 
 	"github.com/jaegertracing/jaeger/pkg/config"
@@ -41,7 +42,8 @@ func TestQueryBuilderFlags(t *testing.T) {
 		"--query.additional-headers=whatever:thing",
 		"--query.max-clock-skew-adjustment=10s",
 	})
-	qOpts := new(QueryOptions).InitFromViper(v, zap.NewNop())
+	qOpts, err := new(QueryOptions).InitFromViper(v, zap.NewNop())
+	require.NoError(t, err)
 	assert.Equal(t, "/dev/null", qOpts.StaticAssets)
 	assert.Equal(t, "some.json", qOpts.UIConfig)
 	assert.Equal(t, "/jaeger", qOpts.BasePath)
@@ -59,7 +61,8 @@ func TestQueryBuilderBadHeadersFlags(t *testing.T) {
 	command.ParseFlags([]string{
 		"--query.additional-headers=malformedheader",
 	})
-	qOpts := new(QueryOptions).InitFromViper(v, zap.NewNop())
+	qOpts, err := new(QueryOptions).InitFromViper(v, zap.NewNop())
+	require.NoError(t, err)
 	assert.Nil(t, qOpts.AdditionalHeaders)
 }
 
@@ -92,7 +95,8 @@ func TestStringSliceAsHeader(t *testing.T) {
 
 func TestBuildQueryServiceOptions(t *testing.T) {
 	v, _ := config.Viperize(AddFlags)
-	qOpts := new(QueryOptions).InitFromViper(v, zap.NewNop())
+	qOpts, err := new(QueryOptions).InitFromViper(v, zap.NewNop())
+	require.NoError(t, err)
 	assert.NotNil(t, qOpts)
 
 	qSvcOpts := qOpts.BuildQueryServiceOptions(&mocks.Factory{}, zap.NewNop())
@@ -162,7 +166,8 @@ func TestQueryOptionsPortAllocationFromFlags(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			v, command := config.Viperize(AddFlags)
 			command.ParseFlags(test.flagsArray)
-			qOpts := new(QueryOptions).InitFromViper(v, zap.NewNop())
+			qOpts, err := new(QueryOptions).InitFromViper(v, zap.NewNop())
+			require.NoError(t, err)
 
 			assert.Equal(t, test.expectedHTTPHostPort, qOpts.HTTPHostPort)
 			assert.Equal(t, test.expectedGRPCHostPort, qOpts.GRPCHostPort)
diff --git a/cmd/query/main.go b/cmd/query/main.go
index 4659ad84bab..2921cb71763 100644
--- a/cmd/query/main.go
+++ b/cmd/query/main.go
@@ -93,7 +93,10 @@ func main() {
 			}
 			defer closer.Close()
 			opentracing.SetGlobalTracer(tracer)
-			queryOpts := new(app.QueryOptions).InitFromViper(v, logger)
+			queryOpts, err := new(app.QueryOptions).InitFromViper(v, logger)
+			if err != nil {
+				logger.Fatal("Failed to configure query service", zap.Error(err))
+			}
 			// TODO: Need to figure out set enable/disable propagation on storage plugins.
 			v.Set(spanstore.StoragePropagationKey, queryOpts.BearerTokenPropagation)
 			storageFactory.InitFromViper(v, logger)
diff --git a/pkg/config/tlscfg/flags.go b/pkg/config/tlscfg/flags.go
index a59cca7d4f7..d2ac56093ad 100644
--- a/pkg/config/tlscfg/flags.go
+++ b/pkg/config/tlscfg/flags.go
@@ -16,6 +16,7 @@ package tlscfg
 
 import (
 	"flag"
+	"fmt"
 
 	"github.com/spf13/viper"
 )
@@ -60,7 +61,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 {
+func (c ClientFlagsConfig) InitFromViper(v *viper.Viper) (Options, error) {
 	var p Options
 	p.Enabled = v.GetBool(c.Prefix + tlsEnabled)
 	p.CAPath = v.GetString(c.Prefix + tlsCA)
@@ -68,15 +69,23 @@ func (c ClientFlagsConfig) InitFromViper(v *viper.Viper) Options {
 	p.KeyPath = v.GetString(c.Prefix + tlsKey)
 	p.ServerName = v.GetString(c.Prefix + tlsServerName)
 	p.SkipHostVerify = v.GetBool(c.Prefix + tlsSkipHostVerify)
-	return p
+	areTLSFlagsUsed := v.IsSet(c.Prefix+tlsCA) || v.IsSet(c.Prefix+tlsCert) || v.IsSet(c.Prefix+tlsKey) || v.IsSet(c.Prefix+tlsServerName)
+	if (!p.Enabled) && (p.SkipHostVerify && areTLSFlagsUsed) {
+		return p, fmt.Errorf("%s has been disable", c.Prefix+tlsEnabled)
+	}
+	return p, nil
 }
 
 // InitFromViper creates tls.Config populated with values retrieved from Viper.
-func (c ServerFlagsConfig) InitFromViper(v *viper.Viper) Options {
+func (c ServerFlagsConfig) InitFromViper(v *viper.Viper) (Options, error) {
 	var p Options
 	p.Enabled = v.GetBool(c.Prefix + tlsEnabled)
 	p.CertPath = v.GetString(c.Prefix + tlsCert)
 	p.KeyPath = v.GetString(c.Prefix + tlsKey)
 	p.ClientCAPath = v.GetString(c.Prefix + tlsClientCA)
-	return p
+	areTLSFlagsUsed := v.IsSet(c.Prefix+tlsCA) || v.IsSet(c.Prefix+tlsCert) || v.IsSet(c.Prefix+tlsKey)
+	if !p.Enabled && areTLSFlagsUsed {
+		return p, fmt.Errorf("%s has been disable", c.Prefix+tlsEnabled)
+	}
+	return p, nil
 }
diff --git a/pkg/config/tlscfg/flags_test.go b/pkg/config/tlscfg/flags_test.go
index 908c848e954..395f1915651 100644
--- a/pkg/config/tlscfg/flags_test.go
+++ b/pkg/config/tlscfg/flags_test.go
@@ -55,7 +55,8 @@ func TestClientFlags(t *testing.T) {
 
 			err := command.ParseFlags(append(cmdLine, test.option))
 			require.NoError(t, err)
-			tlsOpts := flagCfg.InitFromViper(v)
+			tlsOpts, err := flagCfg.InitFromViper(v)
+			require.NoError(t, err)
 			assert.Equal(t, Options{
 				Enabled:        true,
 				CAPath:         "ca-file",
@@ -101,7 +102,8 @@ func TestServerFlags(t *testing.T) {
 			cmdLine[0] = test.option
 			err := command.ParseFlags(cmdLine)
 			require.NoError(t, err)
-			tlsOpts := flagCfg.InitFromViper(v)
+			tlsOpts, err := flagCfg.InitFromViper(v)
+			require.NoError(t, err)
 			assert.Equal(t, Options{
 				Enabled:      true,
 				CertPath:     "cert-file",
@@ -111,3 +113,79 @@ func TestServerFlags(t *testing.T) {
 		})
 	}
 }
+
+func TestTLSFailClientFlags(t *testing.T) {
+	cmdLine := []string{
+		"--prefix.tls.ca=ca-file",
+		"--prefix.tls.cert=cert-file",
+		"--prefix.tls.key=key-file",
+		"--prefix.tls.server-name=HAL1",
+		"--prefix.tls.skip-host-verify=true",
+	}
+
+	tests := []struct {
+		option string
+	}{
+		{
+			option: "--prefix.tls.enabled=false",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.option, func(t *testing.T) {
+			v := viper.New()
+			command := cobra.Command{}
+			flagSet := &flag.FlagSet{}
+			flagCfg := ClientFlagsConfig{
+				Prefix: "prefix",
+			}
+			flagCfg.AddFlags(flagSet)
+			command.PersistentFlags().AddGoFlagSet(flagSet)
+			v.BindPFlags(command.PersistentFlags())
+
+			err := command.ParseFlags(append(cmdLine, test.option))
+			require.NoError(t, err)
+			_, err = flagCfg.InitFromViper(v)
+			require.Error(t, err, "prefix.tls.enabled has been disable")
+		})
+	}
+}
+
+func TestTLSFailServerFlags(t *testing.T) {
+	cmdLine := []string{
+		"##placeholder##", // replaced in each test below
+		"--prefix.tls.enabled=false",
+		"--prefix.tls.cert=cert-file",
+		"--prefix.tls.key=key-file",
+	}
+
+	tests := []struct {
+		option string
+		file   string
+	}{
+		{
+			option: "--prefix.tls.client-ca=client-ca-file",
+			file:   "client-ca-file",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.file, func(t *testing.T) {
+			v := viper.New()
+			command := cobra.Command{}
+			flagSet := &flag.FlagSet{}
+			flagCfg := ServerFlagsConfig{
+				Prefix: "prefix",
+			}
+			flagCfg.AddFlags(flagSet)
+			command.PersistentFlags().AddGoFlagSet(flagSet)
+			v.BindPFlags(command.PersistentFlags())
+
+			cmdLine[0] = test.option
+			err := command.ParseFlags(cmdLine)
+			require.NoError(t, err)
+			_, err = flagCfg.InitFromViper(v)
+			require.Error(t, err, "prefix.tls.enabled has been disable")
+		})
+	}
+}
diff --git a/pkg/kafka/auth/config.go b/pkg/kafka/auth/config.go
index 88de4aee006..31f98aebfbb 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.
 		Prefix: configPrefix,
 	}
 
-	config.TLS = tlsClientConfig.InitFromViper(v)
+	config.TLS, _ = tlsClientConfig.InitFromViper(v)
 	if config.Authentication == tls {
 		config.TLS.Enabled = true
 	}
diff --git a/plugin/metrics/prometheus/options.go b/plugin/metrics/prometheus/options.go
index 879343430c5..8a094376d2a 100644
--- a/plugin/metrics/prometheus/options.go
+++ b/plugin/metrics/prometheus/options.go
@@ -68,11 +68,16 @@ func (opt *Options) AddFlags(flagSet *flag.FlagSet) {
 }
 
 // InitFromViper initializes the options struct with values from Viper.
-func (opt *Options) InitFromViper(v *viper.Viper) {
+func (opt *Options) InitFromViper(v *viper.Viper) error {
 	cfg := &opt.Primary
 	cfg.ServerURL = stripWhiteSpace(v.GetString(cfg.namespace + suffixServerURL))
 	cfg.ConnectTimeout = v.GetDuration(cfg.namespace + suffixConnectTimeout)
-	cfg.TLS = cfg.getTLSFlagsConfig().InitFromViper(v)
+	if tls, err := cfg.getTLSFlagsConfig().InitFromViper(v); err == nil {
+		cfg.TLS = tls
+	} else {
+		return err
+	}
+	return nil
 }
 
 func (config *namespaceConfig) getTLSFlagsConfig() tlscfg.ClientFlagsConfig {
diff --git a/plugin/storage/cassandra/options.go b/plugin/storage/cassandra/options.go
index 8f1a5ee376e..d0dce726297 100644
--- a/plugin/storage/cassandra/options.go
+++ b/plugin/storage/cassandra/options.go
@@ -234,7 +234,7 @@ func tlsFlagsConfig(namespace string) tlscfg.ClientFlagsConfig {
 	}
 }
 
-func (cfg *namespaceConfig) initFromViper(v *viper.Viper) {
+func (cfg *namespaceConfig) initFromViper(v *viper.Viper) error {
 	var tlsFlagsConfig = tlsFlagsConfig(cfg.namespace)
 	if cfg.namespace != primaryStorageConfig {
 		cfg.Enabled = v.GetBool(cfg.namespace + suffixEnabled)
@@ -254,7 +254,12 @@ func (cfg *namespaceConfig) initFromViper(v *viper.Viper) {
 	cfg.Authenticator.Basic.Username = v.GetString(cfg.namespace + suffixUsername)
 	cfg.Authenticator.Basic.Password = v.GetString(cfg.namespace + suffixPassword)
 	cfg.DisableCompression = v.GetBool(cfg.namespace + suffixDisableCompression)
-	cfg.TLS = tlsFlagsConfig.InitFromViper(v)
+	if tls, err := tlsFlagsConfig.InitFromViper(v); err == nil {
+		cfg.TLS = tls
+	} else {
+		return err
+	}
+	return nil
 }
 
 // GetPrimary returns primary configuration.
diff --git a/plugin/storage/es/options.go b/plugin/storage/es/options.go
index 5c15528d2fd..5d464529a8e 100644
--- a/plugin/storage/es/options.go
+++ b/plugin/storage/es/options.go
@@ -290,7 +290,7 @@ func (opt *Options) InitFromViper(v *viper.Viper) {
 	}
 }
 
-func initFromViper(cfg *namespaceConfig, v *viper.Viper) {
+func initFromViper(cfg *namespaceConfig, v *viper.Viper) error {
 	cfg.Username = v.GetString(cfg.namespace + suffixUsername)
 	cfg.Password = v.GetString(cfg.namespace + suffixPassword)
 	cfg.TokenFilePath = v.GetString(cfg.namespace + suffixTokenPath)
@@ -321,7 +321,6 @@ func initFromViper(cfg *namespaceConfig, v *viper.Viper) {
 
 	// TODO: Need to figure out a better way for do this.
 	cfg.AllowTokenFromContext = v.GetBool(spanstore.StoragePropagationKey)
-	cfg.TLS = cfg.getTLSFlagsConfig().InitFromViper(v)
 
 	remoteReadClusters := stripWhiteSpace(v.GetString(cfg.namespace + suffixRemoteReadClusters))
 	if len(remoteReadClusters) > 0 {
@@ -337,6 +336,13 @@ func initFromViper(cfg *namespaceConfig, v *viper.Viper) {
 
 	// Dependencies calculation should be daily, and this index size is very small
 	cfg.IndexDateLayoutDependencies = initDateLayout(defaultIndexRolloverFrequency, separator)
+
+	if tls, err := cfg.getTLSFlagsConfig().InitFromViper(v); err == nil {
+		cfg.TLS = tls
+	} else {
+		return err
+	}
+	return nil
 }
 
 // GetPrimary returns primary configuration.

From f8f1cdfff26c7e08d9de859840a3b8d2f7cac4e5 Mon Sep 17 00:00:00 2001
From: Rajdeep Kaur <rajdeep51994@gmail.com>
Date: Tue, 22 Jun 2021 06:13:46 +0100
Subject: [PATCH 2/8] Add TLS flags test

This commit adds tls flag test in collector/app/builder_flag_test.go and query/app/flag_test.go
Also fixes InitFromViper check in builder_flag.go (#2893)

Signed-off-by: Rajdeep Kaur <rajdeep51994@gmail.com>
---
 cmd/agent/app/reporter/grpc/flags_test.go | 18 ++++++
 cmd/collector/app/builder_flags.go        |  4 +-
 cmd/collector/app/builder_flags_test.go   | 31 ++++++++++
 cmd/query/app/flags.go                    |  2 +-
 cmd/query/app/flags_test.go               | 72 +++++++++++++++++++++++
 plugin/metrics/prometheus/options.go      |  9 +--
 plugin/storage/cassandra/options.go       |  9 +--
 plugin/storage/es/options.go              |  9 +--
 8 files changed, 130 insertions(+), 24 deletions(-)

diff --git a/cmd/agent/app/reporter/grpc/flags_test.go b/cmd/agent/app/reporter/grpc/flags_test.go
index cc879837e92..13bcb4dc1aa 100644
--- a/cmd/agent/app/reporter/grpc/flags_test.go
+++ b/cmd/agent/app/reporter/grpc/flags_test.go
@@ -51,3 +51,21 @@ func TestBindFlags(t *testing.T) {
 		assert.Equal(t, test.expected, b)
 	}
 }
+
+func TestBindTLSFlagFailure(t *testing.T) {
+	v := viper.New()
+	command := cobra.Command{}
+	flags := &flag.FlagSet{}
+	AddFlags(flags)
+	command.ParseFlags([]string{
+		"--reporter.grpc-server.host-port=127.0.0.1:1234",
+	})
+	v.Set("reporter.grpc.tls.enabled", "false")
+	v.Set("reporter.grpc.tls.cert", "abc")
+	v.Set("reporter.grpc.tls.ca", "def")
+	v.Set("reporter.grpc.tls.key", "xyz")
+	v.Set("reporter.grpc.tls.server-name", "xyz")
+	v.Set("reporter.grpc.tls.skip-host-verify", "true")
+	_, err := new(ConnBuilder).InitFromViper(v)
+	require.Error(t, err, "collector.grpc.tls.enabled has been disable")
+}
diff --git a/cmd/collector/app/builder_flags.go b/cmd/collector/app/builder_flags.go
index 9b98c655eb2..6d3a80ee2d2 100644
--- a/cmd/collector/app/builder_flags.go
+++ b/cmd/collector/app/builder_flags.go
@@ -98,12 +98,12 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper) (*CollectorOptions,
 	cOpts.DynQueueSizeMemory = v.GetUint(collectorDynQueueSizeMemory) * 1024 * 1024 // we receive in MiB and store in bytes
 	cOpts.NumWorkers = v.GetInt(collectorNumWorkers)
 	cOpts.QueueSize = v.GetInt(collectorQueueSize)
-	if tlsGrpc, err := tlsGRPCFlagsConfig.InitFromViper(v); err != nil {
+	if tlsGrpc, err := tlsGRPCFlagsConfig.InitFromViper(v); err == nil {
 		cOpts.TLSGRPC = tlsGrpc
 	} else {
 		return cOpts, err
 	}
-	if tlsHTTP, err := tlsGRPCFlagsConfig.InitFromViper(v); err != nil {
+	if tlsHTTP, err := tlsHTTPFlagsConfig.InitFromViper(v); err == nil {
 		cOpts.TLSHTTP = tlsHTTP
 	} else {
 		return cOpts, err
diff --git a/cmd/collector/app/builder_flags_test.go b/cmd/collector/app/builder_flags_test.go
index 0ffa0e8ec1b..e24ead3d88c 100644
--- a/cmd/collector/app/builder_flags_test.go
+++ b/cmd/collector/app/builder_flags_test.go
@@ -18,6 +18,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/jaegertracing/jaeger/pkg/config"
 )
@@ -51,3 +52,33 @@ func TestCollectorOptionsWithFlags_CheckFullHostPort(t *testing.T) {
 	assert.Equal(t, "127.0.0.1:1234", c.CollectorGRPCHostPort)
 	assert.Equal(t, "0.0.0.0:3456", c.CollectorZipkinHTTPHostPort)
 }
+
+func TestCollectorOptionsWithFailedHTTPFlags(t *testing.T) {
+	c := &CollectorOptions{}
+	v, command := config.Viperize(AddFlags)
+	command.ParseFlags([]string{
+		"--collector.http-server.host-port=:5678",
+		"--collector.grpc-server.host-port=127.0.0.1:1234",
+	})
+	v.Set("collector.http.tls.enabled", "false")
+	v.Set("collector.http.tls.cert", "abc")
+	v.Set("collector.http.tls.ca", "def")
+	v.Set("collector.http.tls.key", "xyz")
+	_, err := c.InitFromViper(v)
+	require.Error(t, err, "query.http.tls.enabled has been disable")
+}
+
+func TestCollectorOptionsWithFailedGRPCFlags(t *testing.T) {
+	c := &CollectorOptions{}
+	v, command := config.Viperize(AddFlags)
+	command.ParseFlags([]string{
+		"--collector.http-server.host-port=:5678",
+		"--collector.grpc-server.host-port=127.0.0.1:1234",
+	})
+	v.Set("collector.grpc.tls.enabled", "false")
+	v.Set("collector.grpc.tls.cert", "abc")
+	v.Set("collector.grpc.tls.ca", "def")
+	v.Set("collector.grpc.tls.key", "xyz")
+	_, err := c.InitFromViper(v)
+	require.Error(t, err, "query.grpc.tls.enabled has been disable")
+}
diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go
index e63f06fb89b..846e38457b1 100644
--- a/cmd/query/app/flags.go
+++ b/cmd/query/app/flags.go
@@ -104,7 +104,7 @@ func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) (*Q
 	} else {
 		return qOpts, err
 	}
-	if tlsHTTP, err := tlsGRPCFlagsConfig.InitFromViper(v); err == nil {
+	if tlsHTTP, err := tlsHTTPFlagsConfig.InitFromViper(v); err == nil {
 		qOpts.TLSHTTP = tlsHTTP
 	} else {
 		return qOpts, err
diff --git a/cmd/query/app/flags_test.go b/cmd/query/app/flags_test.go
index f125503baa1..390a4fe8c0c 100644
--- a/cmd/query/app/flags_test.go
+++ b/cmd/query/app/flags_test.go
@@ -175,3 +175,75 @@ func TestQueryOptionsPortAllocationFromFlags(t *testing.T) {
 		})
 	}
 }
+
+func TestQueryOptionsPortAllocationFromFailedGRPCFlags(t *testing.T) {
+	var flagPortCases = []struct {
+		name                 string
+		flagsArray           []string
+		expectedHTTPHostPort string
+		expectedGRPCHostPort string
+		verifyCommonPort     bool
+		expectedHostPort     string
+	}{
+		{
+			// Allows usage of common host-ports.  Flags allow this irrespective of TLS status
+			// The server with TLS enabled with equal HTTP & GRPC host-ports, is still an acceptable flag configuration
+			name: "Common equal host-port specified, TLS enabled in atleast one server",
+			flagsArray: []string{
+				"--query.grpc.tls.enabled=false",
+				"--query.http-server.host-port=127.0.0.1:8081",
+				"--query.grpc-server.host-port=127.0.0.1:8081",
+			},
+			expectedHTTPHostPort: "127.0.0.1:8081",
+			expectedGRPCHostPort: "127.0.0.1:8081",
+		},
+	}
+	for _, test := range flagPortCases {
+		t.Run(test.name, func(t *testing.T) {
+			v, command := config.Viperize(AddFlags)
+			command.ParseFlags(test.flagsArray)
+			v.Set("query.grpc.tls.enabled", "false")
+			v.Set("query.grpc.tls.cert", "abc")
+			v.Set("query.grpc.tls.ca", "def")
+			v.Set("query.grpc.tls.key", "xyz")
+			_, err := new(QueryOptions).InitFromViper(v, zap.NewNop())
+			require.Error(t, err, "query.grpc.tls.enabled has been disable")
+		})
+	}
+}
+
+func TestQueryOptionsPortAllocationFromFailedHTTPFlags(t *testing.T) {
+	var flagPortCases = []struct {
+		name                 string
+		flagsArray           []string
+		expectedHTTPHostPort string
+		expectedGRPCHostPort string
+		verifyCommonPort     bool
+		expectedHostPort     string
+	}{
+		{
+			// Allows usage of common host-ports.  Flags allow this irrespective of TLS status
+			// The server with TLS enabled with equal HTTP & GRPC host-ports, is still an acceptable flag configuration
+			name: "Common equal host-port specified, TLS enabled in atleast one server",
+			flagsArray: []string{
+				"--query.http.tls.enabled=false",
+				"--query.http-server.host-port=127.0.0.1:8081",
+				"--query.grpc-server.host-port=127.0.0.1:8081",
+			},
+			expectedHTTPHostPort: "127.0.0.1:8081",
+			expectedGRPCHostPort: "127.0.0.1:8081",
+		},
+	}
+	for _, test := range flagPortCases {
+		t.Run(test.name, func(t *testing.T) {
+			v, command := config.Viperize(AddFlags)
+			command.ParseFlags(test.flagsArray)
+			v.Set("query.http.tls.enabled", "false")
+			v.Set("query.http.tls.cert", "abc")
+			v.Set("query.http.tls.ca", "def")
+			v.Set("query.http.tls.key", "xyz")
+			_, err := new(QueryOptions).InitFromViper(v, zap.NewNop())
+			require.Error(t, err, "query.http.tls.enabled has been disable")
+		})
+	}
+}
diff --git a/plugin/metrics/prometheus/options.go b/plugin/metrics/prometheus/options.go
index 8a094376d2a..49761ca6d8a 100644
--- a/plugin/metrics/prometheus/options.go
+++ b/plugin/metrics/prometheus/options.go
@@ -68,16 +68,11 @@ func (opt *Options) AddFlags(flagSet *flag.FlagSet) {
 }
 
 // InitFromViper initializes the options struct with values from Viper.
-func (opt *Options) InitFromViper(v *viper.Viper) error {
+func (opt *Options) InitFromViper(v *viper.Viper) {
 	cfg := &opt.Primary
 	cfg.ServerURL = stripWhiteSpace(v.GetString(cfg.namespace + suffixServerURL))
 	cfg.ConnectTimeout = v.GetDuration(cfg.namespace + suffixConnectTimeout)
-	if tls, err := cfg.getTLSFlagsConfig().InitFromViper(v); err == nil {
-		cfg.TLS = tls
-	} else {
-		return err
-	}
-	return nil
+	cfg.TLS, _ = cfg.getTLSFlagsConfig().InitFromViper(v)
 }
 
 func (config *namespaceConfig) getTLSFlagsConfig() tlscfg.ClientFlagsConfig {
diff --git a/plugin/storage/cassandra/options.go b/plugin/storage/cassandra/options.go
index d0dce726297..f17a5114139 100644
--- a/plugin/storage/cassandra/options.go
+++ b/plugin/storage/cassandra/options.go
@@ -234,7 +234,7 @@ func tlsFlagsConfig(namespace string) tlscfg.ClientFlagsConfig {
 	}
 }
 
-func (cfg *namespaceConfig) initFromViper(v *viper.Viper) error {
+func (cfg *namespaceConfig) initFromViper(v *viper.Viper) {
 	var tlsFlagsConfig = tlsFlagsConfig(cfg.namespace)
 	if cfg.namespace != primaryStorageConfig {
 		cfg.Enabled = v.GetBool(cfg.namespace + suffixEnabled)
@@ -254,12 +254,7 @@ func (cfg *namespaceConfig) initFromViper(v *viper.Viper) error {
 	cfg.Authenticator.Basic.Username = v.GetString(cfg.namespace + suffixUsername)
 	cfg.Authenticator.Basic.Password = v.GetString(cfg.namespace + suffixPassword)
 	cfg.DisableCompression = v.GetBool(cfg.namespace + suffixDisableCompression)
-	if tls, err := tlsFlagsConfig.InitFromViper(v); err == nil {
-		cfg.TLS = tls
-	} else {
-		return err
-	}
-	return nil
+	cfg.TLS, _ = tlsFlagsConfig.InitFromViper(v)
 }
 
 // GetPrimary returns primary configuration.
diff --git a/plugin/storage/es/options.go b/plugin/storage/es/options.go
index 5d464529a8e..d34bb349b3d 100644
--- a/plugin/storage/es/options.go
+++ b/plugin/storage/es/options.go
@@ -290,7 +290,7 @@ func (opt *Options) InitFromViper(v *viper.Viper) {
 	}
 }
 
-func initFromViper(cfg *namespaceConfig, v *viper.Viper) error {
+func initFromViper(cfg *namespaceConfig, v *viper.Viper) {
 	cfg.Username = v.GetString(cfg.namespace + suffixUsername)
 	cfg.Password = v.GetString(cfg.namespace + suffixPassword)
 	cfg.TokenFilePath = v.GetString(cfg.namespace + suffixTokenPath)
@@ -337,12 +337,7 @@ func initFromViper(cfg *namespaceConfig, v *viper.Viper) error {
 	// Dependencies calculation should be daily, and this index size is very small
 	cfg.IndexDateLayoutDependencies = initDateLayout(defaultIndexRolloverFrequency, separator)
 
-	if tls, err := cfg.getTLSFlagsConfig().InitFromViper(v); err == nil {
-		cfg.TLS = tls
-	} else {
-		return err
-	}
-	return nil
+	cfg.TLS, _ = cfg.getTLSFlagsConfig().InitFromViper(v)
 }
 
 // GetPrimary returns primary configuration.

From d4584f59128df64f46ad8404b2bc3dfce0b05f0f Mon Sep 17 00:00:00 2001
From: Yuri Shkuro <github@ysh.us>
Date: Sun, 17 Apr 2022 00:06:23 -0400
Subject: [PATCH 3/8] fix test

Signed-off-by: Yuri Shkuro <github@ysh.us>
---
 pkg/config/tlscfg/flags.go | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/pkg/config/tlscfg/flags.go b/pkg/config/tlscfg/flags.go
index cf0c0b80f09..25bdd2664fe 100644
--- a/pkg/config/tlscfg/flags.go
+++ b/pkg/config/tlscfg/flags.go
@@ -95,7 +95,9 @@ func (c ServerFlagsConfig) InitFromViper(v *viper.Viper) (Options, error) {
 	p.CertPath = v.GetString(c.Prefix + tlsCert)
 	p.KeyPath = v.GetString(c.Prefix + tlsKey)
 	p.ClientCAPath = v.GetString(c.Prefix + tlsClientCA)
-	p.CipherSuites = strings.Split(stripWhiteSpace(v.GetString(c.Prefix+tlsCipherSuites)), ",")
+	if s := v.GetString(c.Prefix + tlsCipherSuites); s != "" {
+		p.CipherSuites = strings.Split(stripWhiteSpace(v.GetString(c.Prefix+tlsCipherSuites)), ",")
+	}
 	p.MinVersion = v.GetString(c.Prefix + tlsMinVersion)
 	p.MaxVersion = v.GetString(c.Prefix + tlsMaxVersion)
 

From ca1dfac335562c03a1a8807cb7fd4998272c7f7c Mon Sep 17 00:00:00 2001
From: Yuri Shkuro <github@ysh.us>
Date: Sun, 17 Apr 2022 00:42:04 -0400
Subject: [PATCH 4/8] fix tests

Signed-off-by: Yuri Shkuro <github@ysh.us>
---
 cmd/es-rollover/app/actions.go      |  5 ++++-
 cmd/es-rollover/app/actions_test.go |  3 ++-
 plugin/storage/grpc/factory.go      |  4 +++-
 plugin/storage/grpc/options.go      | 10 ++++++++--
 4 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/cmd/es-rollover/app/actions.go b/cmd/es-rollover/app/actions.go
index 2186bd68884..108e03d9053 100644
--- a/cmd/es-rollover/app/actions.go
+++ b/cmd/es-rollover/app/actions.go
@@ -61,7 +61,10 @@ type ActionCreatorFunction func(client.Client, Config) Action
 func ExecuteAction(opts ActionExecuteOptions, createAction ActionCreatorFunction) error {
 	cfg := Config{}
 	cfg.InitFromViper(opts.Viper)
-	tlsOpts := opts.TLSFlags.InitFromViper(opts.Viper)
+	tlsOpts, err := opts.TLSFlags.InitFromViper(opts.Viper)
+	if err != nil {
+		return err
+	}
 	tlsCfg, err := tlsOpts.Config(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 b1fd51c3238..c3d7eea3787 100644
--- a/cmd/es-rollover/app/actions_test.go
+++ b/cmd/es-rollover/app/actions_test.go
@@ -92,7 +92,8 @@ func TestExecuteAction(t *testing.T) {
 			tlsFlags.AddFlags(flags)
 			command.PersistentFlags().AddGoFlagSet(flags)
 			v.BindPFlags(command.PersistentFlags())
-			err := command.ParseFlags(test.flags)
+			cmdLine := append([]string{"--es.tls.enabled=true"}, test.flags...)
+			err := command.ParseFlags(cmdLine)
 			require.NoError(t, err)
 			executedAction := false
 			err = ExecuteAction(ActionExecuteOptions{
diff --git a/plugin/storage/grpc/factory.go b/plugin/storage/grpc/factory.go
index 0967f382307..093584aecbe 100644
--- a/plugin/storage/grpc/factory.go
+++ b/plugin/storage/grpc/factory.go
@@ -57,7 +57,9 @@ func (f *Factory) AddFlags(flagSet *flag.FlagSet) {
 
 // InitFromViper implements plugin.Configurable
 func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) {
-	f.options.InitFromViper(v)
+	if err := f.options.InitFromViper(v); err != nil {
+		logger.Fatal("unable to initialize gRPC storage factory", zap.Error(err))
+	}
 	f.builder = &f.options.Configuration
 }
 
diff --git a/plugin/storage/grpc/options.go b/plugin/storage/grpc/options.go
index 07daacf4b85..a0b847555fd 100644
--- a/plugin/storage/grpc/options.go
+++ b/plugin/storage/grpc/options.go
@@ -16,6 +16,7 @@ package grpc
 
 import (
 	"flag"
+	"fmt"
 	"time"
 
 	"github.com/spf13/viper"
@@ -60,11 +61,16 @@ func (opt *Options) AddFlags(flagSet *flag.FlagSet) {
 }
 
 // InitFromViper initializes Options with properties from viper
-func (opt *Options) InitFromViper(v *viper.Viper) {
+func (opt *Options) InitFromViper(v *viper.Viper) error {
 	opt.Configuration.PluginBinary = v.GetString(pluginBinary)
 	opt.Configuration.PluginConfigurationFile = v.GetString(pluginConfigurationFile)
 	opt.Configuration.PluginLogLevel = v.GetString(pluginLogLevel)
 	opt.Configuration.RemoteServerAddr = v.GetString(remoteServer)
-	opt.Configuration.RemoteTLS = tlsFlagsConfig().InitFromViper(v)
+	var err error
+	opt.Configuration.RemoteTLS, err = tlsFlagsConfig().InitFromViper(v)
+	if err != nil {
+		return fmt.Errorf("cannot parse gRPC storage TLS options: %w", err)
+	}
 	opt.Configuration.RemoteConnectTimeout = v.GetDuration(remoteConnectionTimeout)
+	return nil
 }

From 5fbd1790511ca71be7423ffb1465e8d9ddde4737 Mon Sep 17 00:00:00 2001
From: Yuri Shkuro <github@ysh.us>
Date: Sun, 17 Apr 2022 00:59:19 -0400
Subject: [PATCH 5/8] fix test

Signed-off-by: Yuri Shkuro <github@ysh.us>
---
 cmd/es-index-cleaner/main.go | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/cmd/es-index-cleaner/main.go b/cmd/es-index-cleaner/main.go
index a48ec5715e9..7b910549a63 100644
--- a/cmd/es-index-cleaner/main.go
+++ b/cmd/es-index-cleaner/main.go
@@ -52,7 +52,10 @@ func main() {
 			}
 
 			cfg.InitFromViper(v)
-			tlsOpts := tlsFlags.InitFromViper(v)
+			tlsOpts, err := tlsFlags.InitFromViper(v)
+			if err != nil {
+				return err
+			}
 			tlsCfg, err := tlsOpts.Config(logger)
 			if err != nil {
 				return err

From fad9357eff8f7e2dafcb34ff6d1da8c257495ab8 Mon Sep 17 00:00:00 2001
From: Yuri Shkuro <github@ysh.us>
Date: Sun, 17 Apr 2022 13:36:47 -0400
Subject: [PATCH 6/8] fix tests

Signed-off-by: Yuri Shkuro <github@ysh.us>
---
 cmd/agent/app/reporter/grpc/flags.go      |  3 +-
 cmd/agent/app/reporter/grpc/flags_test.go | 23 +++----
 cmd/agent/main.go                         |  6 +-
 cmd/collector/app/builder_flags_test.go   | 32 ++++-----
 cmd/query/app/flags.go                    |  4 +-
 cmd/query/app/flags_test.go               | 81 ++++-------------------
 pkg/config/tlscfg/flags_test.go           | 48 ++------------
 7 files changed, 48 insertions(+), 149 deletions(-)

diff --git a/cmd/agent/app/reporter/grpc/flags.go b/cmd/agent/app/reporter/grpc/flags.go
index 48be9aeffcc..72289e4b2fd 100644
--- a/cmd/agent/app/reporter/grpc/flags.go
+++ b/cmd/agent/app/reporter/grpc/flags.go
@@ -16,6 +16,7 @@ package grpc
 
 import (
 	"flag"
+	"fmt"
 	"strings"
 
 	"github.com/spf13/viper"
@@ -53,7 +54,7 @@ func (b *ConnBuilder) InitFromViper(v *viper.Viper) (*ConnBuilder, error) {
 	if tls, err := tlsFlagsConfig.InitFromViper(v); err == nil {
 		b.TLS = tls
 	} else {
-		return b, err
+		return b, fmt.Errorf("failed to process TLS options: %w", err)
 	}
 	b.DiscoveryMinPeers = v.GetInt(discoveryMinPeers)
 	return b, nil
diff --git a/cmd/agent/app/reporter/grpc/flags_test.go b/cmd/agent/app/reporter/grpc/flags_test.go
index 13bcb4dc1aa..303bc3805c7 100644
--- a/cmd/agent/app/reporter/grpc/flags_test.go
+++ b/cmd/agent/app/reporter/grpc/flags_test.go
@@ -18,6 +18,7 @@ import (
 	"flag"
 	"testing"
 
+	"github.com/jaegertracing/jaeger/pkg/config"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
@@ -53,19 +54,13 @@ func TestBindFlags(t *testing.T) {
 }
 
 func TestBindTLSFlagFailure(t *testing.T) {
-	v := viper.New()
-	command := cobra.Command{}
-	flags := &flag.FlagSet{}
-	AddFlags(flags)
-	command.ParseFlags([]string{
-		"--reporter.grpc-server.host-port=127.0.0.1:1234",
+	v, command := config.Viperize(AddFlags)
+	err := command.ParseFlags([]string{
+		"--reporter.grpc.tls.enabled=false",
+		"--reporter.grpc.tls.cert=blah", // invalid unless tls.enabled
 	})
-	v.Set("reporter.grpc.tls.enabled", "false")
-	v.Set("reporter.grpc.tls.cert", "abc")
-	v.Set("reporter.grpc.tls.ca", "def")
-	v.Set("reporter.grpc.tls.key", "xyz")
-	v.Set("reporter.grpc.tls.server-name", "xyz")
-	v.Set("reporter.grpc.tls.skip-host-verify", "true")
-	_, err := new(ConnBuilder).InitFromViper(v)
-	require.Error(t, err, "collector.grpc.tls.enabled has been disable")
+	require.NoError(t, err)
+	_, err = new(ConnBuilder).InitFromViper(v)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "failed to process TLS options")
 }
diff --git a/cmd/agent/main.go b/cmd/agent/main.go
index d8f525cb61c..9d07c8d5147 100644
--- a/cmd/agent/main.go
+++ b/cmd/agent/main.go
@@ -63,7 +63,7 @@ func main() {
 			rOpts := new(reporter.Options).InitFromViper(v, logger)
 			grpcBuilder, err := grpc.NewConnBuilder().InitFromViper(v)
 			if err != nil {
-				logger.Fatal("Failed to configure connection for grpc", zap.Error(err))
+				logger.Fatal("Failed to configure gRPC connection", zap.Error(err))
 			}
 			builders := map[reporter.Type]app.CollectorProxyBuilder{
 				reporter.GRPC: app.GRPCCollectorProxyBuilder(grpcBuilder),
@@ -74,7 +74,7 @@ func main() {
 				Metrics: mFactory,
 			}, builders)
 			if err != nil {
-				logger.Fatal("Could not create collector proxy", zap.Error(err))
+				logger.Fatal("Failed to create collector proxy", zap.Error(err))
 			}
 
 			// TODO illustrate discovery service wiring
@@ -82,7 +82,7 @@ func main() {
 			builder := new(app.Builder).InitFromViper(v)
 			agent, err := builder.CreateAgent(cp, logger, mFactory)
 			if err != nil {
-				return fmt.Errorf("unable to initialize Jaeger Agent: %w", err)
+				return fmt.Errorf("failed to initialize Jaeger Agent: %w", err)
 			}
 
 			logger.Info("Starting agent")
diff --git a/cmd/collector/app/builder_flags_test.go b/cmd/collector/app/builder_flags_test.go
index ec1e6a915b7..e2572fb1afd 100644
--- a/cmd/collector/app/builder_flags_test.go
+++ b/cmd/collector/app/builder_flags_test.go
@@ -57,31 +57,27 @@ func TestCollectorOptionsWithFlags_CheckFullHostPort(t *testing.T) {
 func TestCollectorOptionsWithFailedHTTPFlags(t *testing.T) {
 	c := &CollectorOptions{}
 	v, command := config.Viperize(AddFlags)
-	command.ParseFlags([]string{
-		"--collector.http-server.host-port=:5678",
-		"--collector.grpc-server.host-port=127.0.0.1:1234",
+	err := command.ParseFlags([]string{
+		"--collector.http.tls.enabled=false",
+		"--collector.http.tls.cert=blah", // invalid unless tls.enabled
 	})
-	v.Set("collector.http.tls.enabled", "false")
-	v.Set("collector.http.tls.cert", "abc")
-	v.Set("collector.http.tls.ca", "def")
-	v.Set("collector.http.tls.key", "xyz")
-	_, err := c.InitFromViper(v)
-	require.Error(t, err, "query.http.tls.enabled has been disable")
+	require.NoError(t, err)
+	_, err = c.InitFromViper(v)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "failed to parse HTTP TLS options")
 }
 
 func TestCollectorOptionsWithFailedGRPCFlags(t *testing.T) {
 	c := &CollectorOptions{}
 	v, command := config.Viperize(AddFlags)
-	command.ParseFlags([]string{
-		"--collector.http-server.host-port=:5678",
-		"--collector.grpc-server.host-port=127.0.0.1:1234",
+	err := command.ParseFlags([]string{
+		"--collector.grpc.tls.enabled=false",
+		"--collector.grpc.tls.cert=blah", // invalid unless tls.enabled
 	})
-	v.Set("collector.grpc.tls.enabled", "false")
-	v.Set("collector.grpc.tls.cert", "abc")
-	v.Set("collector.grpc.tls.ca", "def")
-	v.Set("collector.grpc.tls.key", "xyz")
-	_, err := c.InitFromViper(v)
-	require.Error(t, err, "query.grpc.tls.enabled has been disable")
+	require.NoError(t, err)
+	_, err = c.InitFromViper(v)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "failed to parse gRPC TLS options")
 }
 
 func TestCollectorOptionsWithFlags_CheckMaxReceiveMessageLength(t *testing.T) {
diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go
index 846e38457b1..f9daa5041e9 100644
--- a/cmd/query/app/flags.go
+++ b/cmd/query/app/flags.go
@@ -102,12 +102,12 @@ func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) (*Q
 	if tlsGrpc, err := tlsGRPCFlagsConfig.InitFromViper(v); err == nil {
 		qOpts.TLSGRPC = tlsGrpc
 	} else {
-		return qOpts, err
+		return qOpts, fmt.Errorf("failed to process gRPC TLS options: %w", err)
 	}
 	if tlsHTTP, err := tlsHTTPFlagsConfig.InitFromViper(v); err == nil {
 		qOpts.TLSHTTP = tlsHTTP
 	} else {
-		return qOpts, err
+		return qOpts, fmt.Errorf("failed to process HTTP TLS options: %w", err)
 	}
 	qOpts.BasePath = v.GetString(queryBasePath)
 	qOpts.StaticAssets = v.GetString(queryStaticFiles)
diff --git a/cmd/query/app/flags_test.go b/cmd/query/app/flags_test.go
index 390a4fe8c0c..90b71c07ec1 100644
--- a/cmd/query/app/flags_test.go
+++ b/cmd/query/app/flags_test.go
@@ -17,6 +17,7 @@ package app
 
 import (
 	"net/http"
+	"strings"
 	"testing"
 	"time"
 
@@ -123,6 +124,7 @@ func TestBuildQueryServiceOptions(t *testing.T) {
 	assert.NotNil(t, qSvcOpts.ArchiveSpanWriter)
 }
 
+// TODO (yurishkuro) it is not clear what this test is testing (added in PR # 3030)
 func TestQueryOptionsPortAllocationFromFlags(t *testing.T) {
 	var flagPortCases = []struct {
 		name                 string
@@ -176,74 +178,19 @@ func TestQueryOptionsPortAllocationFromFlags(t *testing.T) {
 	}
 }
 
-func TestQueryOptionsPortAllocationFromFailedGRPCFlags(t *testing.T) {
-	var flagPortCases = []struct {
-		name                 string
-		flagsArray           []string
-		expectedHTTPHostPort string
-		expectedGRPCHostPort string
-		verifyCommonPort     bool
-		expectedHostPort     string
-	}{
-		{
-			// Allows usage of common host-ports.  Flags allow this irrespective of TLS status
-			// The server with TLS enabled with equal HTTP & GRPC host-ports, is still an acceptable flag configuration
-			name: "Common equal host-port specified, TLS enabled in atleast one server",
-			flagsArray: []string{
-				"--query.grpc.tls.enabled=false",
-				"--query.http-server.host-port=127.0.0.1:8081",
-				"--query.grpc-server.host-port=127.0.0.1:8081",
-			},
-			expectedHTTPHostPort: "127.0.0.1:8081",
-			expectedGRPCHostPort: "127.0.0.1:8081",
-		},
-	}
-	for _, test := range flagPortCases {
-		t.Run(test.name, func(t *testing.T) {
-			v, command := config.Viperize(AddFlags)
-			command.ParseFlags(test.flagsArray)
-			v.Set("query.grpc.tls.enabled", "false")
-			v.Set("query.grpc.tls.cert", "abc")
-			v.Set("query.grpc.tls.ca", "def")
-			v.Set("query.grpc.tls.key", "xyz")
-			_, err := new(QueryOptions).InitFromViper(v, zap.NewNop())
-			require.Error(t, err, "query.grpc.tls.enabled has been disable")
-		})
-	}
-}
-
-func TestQueryOptionsPortAllocationFromFailedHTTPFlags(t *testing.T) {
-	var flagPortCases = []struct {
-		name                 string
-		flagsArray           []string
-		expectedHTTPHostPort string
-		expectedGRPCHostPort string
-		verifyCommonPort     bool
-		expectedHostPort     string
-	}{
-		{
-			// Allows usage of common host-ports.  Flags allow this irrespective of TLS status
-			// The server with TLS enabled with equal HTTP & GRPC host-ports, is still an acceptable flag configuration
-			name: "Common equal host-port specified, TLS enabled in atleast one server",
-			flagsArray: []string{
-				"--query.http.tls.enabled=false",
-				"--query.http-server.host-port=127.0.0.1:8081",
-				"--query.grpc-server.host-port=127.0.0.1:8081",
-			},
-			expectedHTTPHostPort: "127.0.0.1:8081",
-			expectedGRPCHostPort: "127.0.0.1:8081",
-		},
-	}
-	for _, test := range flagPortCases {
-		t.Run(test.name, func(t *testing.T) {
+func TestQueryOptions_FailedTLSFlags(t *testing.T) {
+	for _, test := range []string{"gRPC", "HTTP"} {
+		t.Run(test, func(t *testing.T) {
+			proto := strings.ToLower(test)
 			v, command := config.Viperize(AddFlags)
-			command.ParseFlags(test.flagsArray)
-			v.Set("query.http.tls.enabled", "false")
-			v.Set("query.http.tls.cert", "abc")
-			v.Set("query.http.tls.ca", "def")
-			v.Set("query.http.tls.key", "xyz")
-			_, err := new(QueryOptions).InitFromViper(v, zap.NewNop())
-			require.Error(t, err, "query.http.tls.enabled has been disable")
+			err := command.ParseFlags([]string{
+				"--query." + proto + ".tls.enabled=false",
+				"--query." + proto + ".tls.cert=blah", // invalid unless tls.enabled
+			})
+			require.NoError(t, err)
+			_, err = new(QueryOptions).InitFromViper(v, zap.NewNop())
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), "failed to process "+test+" TLS options")
 		})
 	}
 }
diff --git a/pkg/config/tlscfg/flags_test.go b/pkg/config/tlscfg/flags_test.go
index f7a932caf9b..e0c8c546b26 100644
--- a/pkg/config/tlscfg/flags_test.go
+++ b/pkg/config/tlscfg/flags_test.go
@@ -18,6 +18,7 @@ import (
 	"flag"
 	"testing"
 
+	"github.com/jaegertracing/jaeger/pkg/config"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
@@ -120,44 +121,8 @@ func TestServerFlags(t *testing.T) {
 	}
 }
 
-func TestTLSFailClientFlags(t *testing.T) {
-	cmdLine := []string{
-		"--prefix.tls.ca=ca-file",
-		"--prefix.tls.cert=cert-file",
-		"--prefix.tls.key=key-file",
-		"--prefix.tls.server-name=HAL1",
-		"--prefix.tls.skip-host-verify=true",
-	}
-
-	tests := []struct {
-		option string
-	}{
-		{
-			option: "--prefix.tls.enabled=false",
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.option, func(t *testing.T) {
-			v := viper.New()
-			command := cobra.Command{}
-			flagSet := &flag.FlagSet{}
-			flagCfg := ClientFlagsConfig{
-				Prefix: "prefix",
-			}
-			flagCfg.AddFlags(flagSet)
-			command.PersistentFlags().AddGoFlagSet(flagSet)
-			v.BindPFlags(command.PersistentFlags())
-
-			err := command.ParseFlags(append(cmdLine, test.option))
-			require.NoError(t, err)
-			_, err = flagCfg.InitFromViper(v)
-			require.Error(t, err, "prefix.tls.enabled has been disable")
-		})
-	}
-}
-
-func TestTLSFailServerFlags(t *testing.T) {
+// TestFailedTLSFlags verifies that TLS options cannot be used when tls.enabled=false
+func TestFailedTLSFlags(t *testing.T) {
 	clientTests := []string{
 		".ca=blah",
 		".cert=blah",
@@ -185,9 +150,6 @@ func TestTLSFailServerFlags(t *testing.T) {
 		t.Run(metaTest.side, func(t *testing.T) {
 			for _, test := range metaTest.tests {
 				t.Run(test, func(t *testing.T) {
-					v := viper.New()
-					command := cobra.Command{}
-					flagSet := &flag.FlagSet{}
 					type UnderTest interface {
 						AddFlags(flags *flag.FlagSet)
 						InitFromViper(v *viper.Viper) (Options, error)
@@ -202,9 +164,7 @@ func TestTLSFailServerFlags(t *testing.T) {
 							Prefix: "prefix",
 						}
 					}
-					underTest.AddFlags(flagSet)
-					command.PersistentFlags().AddGoFlagSet(flagSet)
-					v.BindPFlags(command.PersistentFlags())
+					v, command := config.Viperize(underTest.AddFlags)
 
 					cmdLine := []string{
 						"--prefix.tls.enabled=true",

From 283475249e2c3026b784f75d39f6b2779d8b72e8 Mon Sep 17 00:00:00 2001
From: Yuri Shkuro <github@ysh.us>
Date: Sun, 17 Apr 2022 13:55:09 -0400
Subject: [PATCH 7/8] fix tests

Signed-off-by: Yuri Shkuro <github@ysh.us>
---
 cmd/agent/app/reporter/grpc/flags_test.go |  3 ++-
 pkg/config/tlscfg/flags_test.go           |  3 ++-
 pkg/kafka/auth/config.go                  |  9 +++++++--
 plugin/metrics/prometheus/factory.go      |  4 ++--
 plugin/metrics/prometheus/factory_test.go | 18 +++++++++++++++++-
 plugin/metrics/prometheus/options.go      | 10 ++++++++--
 plugin/storage/cassandra/options.go       |  8 +++++++-
 plugin/storage/es/options.go              |  9 +++++++--
 plugin/storage/grpc/options.go            |  2 +-
 plugin/storage/grpc/options_test.go       | 14 ++++++++++++++
 plugin/storage/kafka/options.go           |  4 +++-
 11 files changed, 70 insertions(+), 14 deletions(-)

diff --git a/cmd/agent/app/reporter/grpc/flags_test.go b/cmd/agent/app/reporter/grpc/flags_test.go
index 303bc3805c7..f63f8107fa4 100644
--- a/cmd/agent/app/reporter/grpc/flags_test.go
+++ b/cmd/agent/app/reporter/grpc/flags_test.go
@@ -18,11 +18,12 @@ import (
 	"flag"
 	"testing"
 
-	"github.com/jaegertracing/jaeger/pkg/config"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/jaegertracing/jaeger/pkg/config"
 )
 
 func TestBindFlags(t *testing.T) {
diff --git a/pkg/config/tlscfg/flags_test.go b/pkg/config/tlscfg/flags_test.go
index e0c8c546b26..9bd9f3b47a1 100644
--- a/pkg/config/tlscfg/flags_test.go
+++ b/pkg/config/tlscfg/flags_test.go
@@ -18,11 +18,12 @@ import (
 	"flag"
 	"testing"
 
-	"github.com/jaegertracing/jaeger/pkg/config"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/jaegertracing/jaeger/pkg/config"
 )
 
 func TestClientFlags(t *testing.T) {
diff --git a/pkg/kafka/auth/config.go b/pkg/kafka/auth/config.go
index eb369836db2..9ddad3e1a1f 100644
--- a/pkg/kafka/auth/config.go
+++ b/pkg/kafka/auth/config.go
@@ -75,7 +75,7 @@ func (config *AuthenticationConfig) SetConfiguration(saramaConfig *sarama.Config
 }
 
 // InitFromViper loads authentication configuration from viper flags.
-func (config *AuthenticationConfig) InitFromViper(configPrefix string, v *viper.Viper) {
+func (config *AuthenticationConfig) InitFromViper(configPrefix string, v *viper.Viper) error {
 	config.Authentication = v.GetString(configPrefix + suffixAuthentication)
 	config.Kerberos.ServiceName = v.GetString(configPrefix + kerberosPrefix + suffixKerberosServiceName)
 	config.Kerberos.Realm = v.GetString(configPrefix + kerberosPrefix + suffixKerberosRealm)
@@ -89,7 +89,11 @@ func (config *AuthenticationConfig) InitFromViper(configPrefix string, v *viper.
 		Prefix: configPrefix,
 	}
 
-	config.TLS, _ = tlsClientConfig.InitFromViper(v)
+	var err error
+	config.TLS, err = tlsClientConfig.InitFromViper(v)
+	if err != nil {
+		return fmt.Errorf("failed to process Kafka TLS options: %w", err)
+	}
 	if config.Authentication == tls {
 		config.TLS.Enabled = true
 	}
@@ -97,4 +101,5 @@ func (config *AuthenticationConfig) InitFromViper(configPrefix string, v *viper.
 	config.PlainText.Username = v.GetString(configPrefix + plainTextPrefix + suffixPlainTextUsername)
 	config.PlainText.Password = v.GetString(configPrefix + plainTextPrefix + suffixPlainTextPassword)
 	config.PlainText.Mechanism = v.GetString(configPrefix + plainTextPrefix + suffixPlainTextMechanism)
+	return nil
 }
diff --git a/plugin/metrics/prometheus/factory.go b/plugin/metrics/prometheus/factory.go
index cfae05928f8..99f74b3956b 100644
--- a/plugin/metrics/prometheus/factory.go
+++ b/plugin/metrics/prometheus/factory.go
@@ -43,8 +43,8 @@ func (f *Factory) AddFlags(flagSet *flag.FlagSet) {
 }
 
 // InitFromViper implements plugin.Configurable.
-func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) {
-	f.options.InitFromViper(v)
+func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) error {
+	return f.options.InitFromViper(v)
 }
 
 // Initialize implements storage.MetricsFactory.
diff --git a/plugin/metrics/prometheus/factory_test.go b/plugin/metrics/prometheus/factory_test.go
index bd8f6f1405b..d57dbca2593 100644
--- a/plugin/metrics/prometheus/factory_test.go
+++ b/plugin/metrics/prometheus/factory_test.go
@@ -62,7 +62,23 @@ func TestWithConfiguration(t *testing.T) {
 	})
 	require.NoError(t, err)
 
-	f.InitFromViper(v, zap.NewNop())
+	err = f.InitFromViper(v, zap.NewNop())
+
+	require.NoError(t, err)
 	assert.Equal(t, f.options.Primary.ServerURL, "http://localhost:1234")
 	assert.Equal(t, f.options.Primary.ConnectTimeout, 5*time.Second)
 }
+
+func TestFailedTLSOptions(t *testing.T) {
+	f := NewFactory()
+	v, command := config.Viperize(f.AddFlags)
+	err := command.ParseFlags([]string{
+		"--prometheus.tls.enabled=false",
+		"--prometheus.tls.cert=blah", // not valid unless tls.enabled=true
+	})
+	require.NoError(t, err)
+
+	err = f.InitFromViper(v, zap.NewNop())
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "failed to process Prometheus TLS options")
+}
diff --git a/plugin/metrics/prometheus/options.go b/plugin/metrics/prometheus/options.go
index 49761ca6d8a..ad13f6ee0e9 100644
--- a/plugin/metrics/prometheus/options.go
+++ b/plugin/metrics/prometheus/options.go
@@ -16,6 +16,7 @@ package prometheus
 
 import (
 	"flag"
+	"fmt"
 	"strings"
 	"time"
 
@@ -68,11 +69,16 @@ func (opt *Options) AddFlags(flagSet *flag.FlagSet) {
 }
 
 // InitFromViper initializes the options struct with values from Viper.
-func (opt *Options) InitFromViper(v *viper.Viper) {
+func (opt *Options) InitFromViper(v *viper.Viper) error {
 	cfg := &opt.Primary
 	cfg.ServerURL = stripWhiteSpace(v.GetString(cfg.namespace + suffixServerURL))
 	cfg.ConnectTimeout = v.GetDuration(cfg.namespace + suffixConnectTimeout)
-	cfg.TLS, _ = cfg.getTLSFlagsConfig().InitFromViper(v)
+	var err error
+	cfg.TLS, err = cfg.getTLSFlagsConfig().InitFromViper(v)
+	if err != nil {
+		return fmt.Errorf("failed to process Prometheus TLS options: %w", err)
+	}
+	return nil
 }
 
 func (config *namespaceConfig) getTLSFlagsConfig() tlscfg.ClientFlagsConfig {
diff --git a/plugin/storage/cassandra/options.go b/plugin/storage/cassandra/options.go
index f17a5114139..81d3d1b8744 100644
--- a/plugin/storage/cassandra/options.go
+++ b/plugin/storage/cassandra/options.go
@@ -17,6 +17,7 @@ package cassandra
 
 import (
 	"flag"
+	"log"
 	"strings"
 	"time"
 
@@ -254,7 +255,12 @@ func (cfg *namespaceConfig) initFromViper(v *viper.Viper) {
 	cfg.Authenticator.Basic.Username = v.GetString(cfg.namespace + suffixUsername)
 	cfg.Authenticator.Basic.Password = v.GetString(cfg.namespace + suffixPassword)
 	cfg.DisableCompression = v.GetBool(cfg.namespace + suffixDisableCompression)
-	cfg.TLS, _ = tlsFlagsConfig.InitFromViper(v)
+	var err error
+	cfg.TLS, err = tlsFlagsConfig.InitFromViper(v)
+	if err != nil {
+		// TODO refactor to be able to return error
+		log.Fatal(err)
+	}
 }
 
 // GetPrimary returns primary configuration.
diff --git a/plugin/storage/es/options.go b/plugin/storage/es/options.go
index 64e705e217a..e34ad962477 100644
--- a/plugin/storage/es/options.go
+++ b/plugin/storage/es/options.go
@@ -17,6 +17,7 @@ package es
 
 import (
 	"flag"
+	"log"
 	"strings"
 	"time"
 
@@ -345,8 +346,12 @@ func initFromViper(cfg *namespaceConfig, v *viper.Viper) {
 	// Dependencies calculation should be daily, and this index size is very small
 	cfg.IndexDateLayoutDependencies = initDateLayout(defaultIndexRolloverFrequency, separator)
 
-	// TODO handle error
-	cfg.TLS, _ = cfg.getTLSFlagsConfig().InitFromViper(v)
+	var err error
+	cfg.TLS, err = cfg.getTLSFlagsConfig().InitFromViper(v)
+	if err != nil {
+		// TODO refactor to be able to return error
+		log.Fatal(err)
+	}
 }
 
 // GetPrimary returns primary configuration.
diff --git a/plugin/storage/grpc/options.go b/plugin/storage/grpc/options.go
index a0b847555fd..af302f8a0c4 100644
--- a/plugin/storage/grpc/options.go
+++ b/plugin/storage/grpc/options.go
@@ -69,7 +69,7 @@ func (opt *Options) InitFromViper(v *viper.Viper) error {
 	var err error
 	opt.Configuration.RemoteTLS, err = tlsFlagsConfig().InitFromViper(v)
 	if err != nil {
-		return fmt.Errorf("cannot parse gRPC storage TLS options: %w", err)
+		return fmt.Errorf("failed to parse gRPC storage TLS options: %w", err)
 	}
 	opt.Configuration.RemoteConnectTimeout = v.GetDuration(remoteConnectionTimeout)
 	return nil
diff --git a/plugin/storage/grpc/options_test.go b/plugin/storage/grpc/options_test.go
index 7f83a874498..d68032e9022 100644
--- a/plugin/storage/grpc/options_test.go
+++ b/plugin/storage/grpc/options_test.go
@@ -19,6 +19,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/jaegertracing/jaeger/pkg/config"
 )
@@ -72,3 +73,16 @@ func TestRemoteOptionsNoTLSWithFlags(t *testing.T) {
 	assert.Equal(t, opts.Configuration.RemoteTLS.Enabled, false)
 	assert.Equal(t, opts.Configuration.RemoteConnectTimeout, 60*time.Second)
 }
+
+func TestFailedTLSFlags(t *testing.T) {
+	opts := &Options{}
+	v, command := config.Viperize(opts.AddFlags)
+	err := command.ParseFlags([]string{
+		"--grpc-storage.tls.enabled=false",
+		"--grpc-storage.tls.cert=blah", // invalid unless tls.enabled=true
+	})
+	require.NoError(t, err)
+	err = opts.InitFromViper(v)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "failed to parse gRPC storage TLS options")
+}
diff --git a/plugin/storage/kafka/options.go b/plugin/storage/kafka/options.go
index ed6df488fb8..5ad78645d12 100644
--- a/plugin/storage/kafka/options.go
+++ b/plugin/storage/kafka/options.go
@@ -176,7 +176,9 @@ func (opt *Options) AddFlags(flagSet *flag.FlagSet) {
 // InitFromViper initializes Options with properties from viper
 func (opt *Options) InitFromViper(v *viper.Viper) {
 	authenticationOptions := auth.AuthenticationConfig{}
-	authenticationOptions.InitFromViper(configPrefix, v)
+	if err := authenticationOptions.InitFromViper(configPrefix, v); err != nil {
+		log.Fatal(err)
+	}
 
 	requiredAcks, err := getRequiredAcks(v.GetString(configPrefix + suffixRequiredAcks))
 	if err != nil {

From 8ef54be9ae1f21ba7744eb2c822f004b38246666 Mon Sep 17 00:00:00 2001
From: Yuri Shkuro <github@ysh.us>
Date: Sun, 17 Apr 2022 18:37:38 -0400
Subject: [PATCH 8/8] remove comment

Signed-off-by: Yuri Shkuro <github@ysh.us>
---
 cmd/query/app/flags_test.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/cmd/query/app/flags_test.go b/cmd/query/app/flags_test.go
index 90b71c07ec1..94bc4c0acea 100644
--- a/cmd/query/app/flags_test.go
+++ b/cmd/query/app/flags_test.go
@@ -124,7 +124,6 @@ func TestBuildQueryServiceOptions(t *testing.T) {
 	assert.NotNil(t, qSvcOpts.ArchiveSpanWriter)
 }
 
-// TODO (yurishkuro) it is not clear what this test is testing (added in PR # 3030)
 func TestQueryOptionsPortAllocationFromFlags(t *testing.T) {
 	var flagPortCases = []struct {
 		name                 string