Perform log.fatal if TLS flags are used when tls.enabled=false(#2893)

Signed-off-by: Rajdeep Kaur <rajdeep51994@gmail.com>
jaegertracing · Jun 21, 2021 · 68279e2 · 68279e2
1 parent 13885e5
commit 68279e2
Show file tree

Hide file tree

Showing 17 changed files with 139 additions and 45 deletions.
diff --git 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
@@ -46,13 +46,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
@@ -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
@@ -60,7 +60,10 @@ func main() {
 				baseFactory)
 
 			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
@@ -131,9 +131,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
@@ -92,7 +92,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))
@@ -102,8 +102,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
@@ -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
@@ -96,7 +96,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
@@ -100,11 +100,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)
@@ -118,7 +126,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
@@ -16,6 +16,7 @@
 package app
 
 import (
+	"github.com/stretchr/testify/require"
 	"net/http"
 	"testing"
 	"time"
@@ -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
@@ -92,7 +92,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)

diff --git 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"
 )
@@ -70,31 +71,61 @@ 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
+	var checkAnyFieldIsEmpty bool
 	if c.ShowEnabled {
 		p.Enabled = v.GetBool(c.Prefix + tlsEnabled)
 	}
-	p.CAPath = v.GetString(c.Prefix + tlsCA)
-	p.CertPath = v.GetString(c.Prefix + tlsCert)
-	p.KeyPath = v.GetString(c.Prefix + tlsKey)
+	if cAPATH := v.GetString(c.Prefix + tlsCA); len(cAPATH) > 0 {
+		p.CAPath = cAPATH
+	}
+	if certPath := v.GetString(c.Prefix + tlsCert); len(certPath) > 0 {
+		p.CertPath = certPath
+	} else {
+		checkAnyFieldIsEmpty = true
+	}
+	if keypath := v.GetString(c.Prefix + tlsKey); len(keypath) > 0 {
+		p.KeyPath = keypath
+	} else {
+		checkAnyFieldIsEmpty = true
+	}
 	if c.ShowServerName {
 		p.ServerName = v.GetString(c.Prefix + tlsServerName)
+	} else {
+		checkAnyFieldIsEmpty = true
 	}
 	p.SkipHostVerify = v.GetBool(c.Prefix + tlsSkipHostVerify)
-	return p
+	if (!p.Enabled || !c.ShowEnabled) && (p.SkipHostVerify && !checkAnyFieldIsEmpty) {
+		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
-	if c.ShowEnabled {
-		p.Enabled = v.GetBool(c.Prefix + tlsEnabled)
+	var checkAnyFieldIsEmpty bool
+	if certPath := v.GetString(c.Prefix + tlsCert); len(certPath) > 0 {
+		p.CertPath = certPath
+	} else {
+		checkAnyFieldIsEmpty = true
+	}
+	if keypath := v.GetString(c.Prefix + tlsKey); len(keypath) > 0 {
+		p.KeyPath = keypath
+	} else {
+		checkAnyFieldIsEmpty = true
 	}
-	p.CertPath = v.GetString(c.Prefix + tlsCert)
-	p.KeyPath = v.GetString(c.Prefix + tlsKey)
 	if c.ShowClientCA {
 		p.ClientCAPath = v.GetString(c.Prefix + tlsClientCA)
+	} else {
+		checkAnyFieldIsEmpty = true
+	}
+	if c.ShowEnabled {
+		p.Enabled = v.GetBool(c.Prefix + tlsEnabled)
+	}
+	if (!p.Enabled || !c.ShowEnabled) && !checkAnyFieldIsEmpty {
+		return p, fmt.Errorf("%s has been disable", c.Prefix+tlsEnabled)
 	}
-	return p
+	return p, nil
 }
diff --git a/pkg/config/tlscfg/flags_test.go b/pkg/config/tlscfg/flags_test.go
@@ -57,7 +57,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",
@@ -105,7 +106,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",

diff --git a/pkg/kafka/auth/config.go b/pkg/kafka/auth/config.go
@@ -95,7 +95,7 @@ func (config *AuthenticationConfig) InitFromViper(configPrefix string, v *viper.
 		ShowServerName: true,
 	}
 
-	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
@@ -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
@@ -236,7 +236,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)
@@ -256,7 +256,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.