From b3bf080f5ecda1bdce898cdb17bd17f88d2bb423 Mon Sep 17 00:00:00 2001 From: Rajdeep Kaur Date: Tue, 25 May 2021 20:46:34 +0100 Subject: [PATCH] Perform log.fatal if TLS flags are used when tls.enabled=false Signed-off-by: Rajdeep Kaur --- cmd/all-in-one/main.go | 10 ++++++++-- cmd/collector/app/builder_flags.go | 18 +++++++++++++----- cmd/collector/main.go | 5 ++++- cmd/query/app/flags.go | 16 ++++++++++++---- cmd/query/app/flags_test.go | 8 ++++---- cmd/query/app/mocks/Watcher.go | 1 - cmd/query/main.go | 5 ++++- pkg/config/tlscfg/flags.go | 14 +++++++++----- pkg/config/tlscfg/flags_test.go | 6 +++--- 9 files changed, 57 insertions(+), 26 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 4433194cf474..fa4c7d8c0ac9 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -119,8 +119,14 @@ 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) + cOpts, err := new(collectorApp.CollectorOptions).InitFromViper(v) + if err != nil { + logger.Fatal("Incorrect value of flag tls.enabled", zap.Error(err)) + } + qOpts, err := new(queryApp.QueryOptions).InitFromViper(v, logger) + if err != nil { + logger.Fatal("Incorrect value of flag tls.enabled", 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 550446704da7..b63134ffd1ff 100644 --- a/cmd/collector/app/builder_flags.go +++ b/cmd/collector/app/builder_flags.go @@ -16,6 +16,7 @@ package app import ( + "errors" "flag" "github.com/spf13/viper" @@ -92,7 +93,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 +103,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 + TLSGRPC, err := tlsGRPCFlagsConfig.InitFromViper(v) + if err != nil { + return cOpts, errors.New("ES_TLS_ENABLED is set to false unable to talk to server(s)") + } + cOpts.TLSGRPC = TLSGRPC + TLSHTTP, err := tlsHTTPFlagsConfig.InitFromViper(v) + if err != nil { + return cOpts, errors.New("ES_TLS_ENABLED is set to false unable to talk to server(s)") + } + cOpts.TLSHTTP = TLSHTTP + return cOpts, nil } diff --git a/cmd/collector/main.go b/cmd/collector/main.go index 87d24ba5b9a2..dc35505e477f 100644 --- 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("Incorrect value of flag tls.enabled", 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 fb9c29c80dfe..700490e9b5e2 100644 --- 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) + TLSGRPC, err := tlsGRPCFlagsConfig.InitFromViper(v) + if err != nil { + return qOpts, err + } + qOpts.TLSGRPC = TLSGRPC + TLSHTTP, err := tlsHTTPFlagsConfig.InitFromViper(v) + if err != nil { + return qOpts, err + } + qOpts.TLSHTTP = TLSHTTP 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 index ca3e3d3c7c4b..ef09e37583fb 100644 --- a/cmd/query/app/flags_test.go +++ b/cmd/query/app/flags_test.go @@ -41,7 +41,7 @@ func TestQueryBuilderFlags(t *testing.T) { "--query.additional-headers=whatever:thing", "--query.max-clock-skew-adjustment=10s", }) - qOpts := new(QueryOptions).InitFromViper(v, zap.NewNop()) + qOpts, _ := new(QueryOptions).InitFromViper(v, zap.NewNop()) assert.Equal(t, "/dev/null", qOpts.StaticAssets) assert.Equal(t, "some.json", qOpts.UIConfig) assert.Equal(t, "/jaeger", qOpts.BasePath) @@ -59,7 +59,7 @@ func TestQueryBuilderBadHeadersFlags(t *testing.T) { command.ParseFlags([]string{ "--query.additional-headers=malformedheader", }) - qOpts := new(QueryOptions).InitFromViper(v, zap.NewNop()) + qOpts, _ := new(QueryOptions).InitFromViper(v, zap.NewNop()) assert.Nil(t, qOpts.AdditionalHeaders) } @@ -92,7 +92,7 @@ func TestStringSliceAsHeader(t *testing.T) { func TestBuildQueryServiceOptions(t *testing.T) { v, _ := config.Viperize(AddFlags) - qOpts := new(QueryOptions).InitFromViper(v, zap.NewNop()) + qOpts, _ := new(QueryOptions).InitFromViper(v, zap.NewNop()) assert.NotNil(t, qOpts) qSvcOpts := qOpts.BuildQueryServiceOptions(&mocks.Factory{}, zap.NewNop()) @@ -162,7 +162,7 @@ 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, _ := new(QueryOptions).InitFromViper(v, zap.NewNop()) assert.Equal(t, test.expectedHTTPHostPort, qOpts.HTTPHostPort) assert.Equal(t, test.expectedGRPCHostPort, qOpts.GRPCHostPort) diff --git a/cmd/query/app/mocks/Watcher.go b/cmd/query/app/mocks/Watcher.go index 37b1c9b8736d..1cd05b0defc6 100644 --- a/cmd/query/app/mocks/Watcher.go +++ b/cmd/query/app/mocks/Watcher.go @@ -14,7 +14,6 @@ // See the License for the specific language governing permissions and // limitations under the License. - package mocks import ( diff --git a/cmd/query/main.go b/cmd/query/main.go index adbbfe46d086..f54015d3e555 100644 --- a/cmd/query/main.go +++ b/cmd/query/main.go @@ -85,7 +85,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("Incorrect value of flag tls.enabled", 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 index 54b8f8a933d4..be7d995adf2d 100644 --- a/pkg/config/tlscfg/flags.go +++ b/pkg/config/tlscfg/flags.go @@ -15,8 +15,8 @@ package tlscfg import ( + "errors" "flag" - "github.com/spf13/viper" ) @@ -70,10 +70,12 @@ 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 if c.ShowEnabled { p.Enabled = v.GetBool(c.Prefix + tlsEnabled) + } else { + return p, errors.New("ES_TLS_ENABLED is set to false unable to talk to server(s)") } p.CAPath = v.GetString(c.Prefix + tlsCA) p.CertPath = v.GetString(c.Prefix + tlsCert) @@ -82,19 +84,21 @@ func (c ClientFlagsConfig) InitFromViper(v *viper.Viper) Options { p.ServerName = v.GetString(c.Prefix + tlsServerName) } p.SkipHostVerify = v.GetBool(c.Prefix + tlsSkipHostVerify) - return p + 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) + } else { + return p, errors.New("ES_TLS_ENABLED is set to false unable to talk to server(s)") } p.CertPath = v.GetString(c.Prefix + tlsCert) p.KeyPath = v.GetString(c.Prefix + tlsKey) if c.ShowClientCA { p.ClientCAPath = v.GetString(c.Prefix + tlsClientCA) } - return p + return p, nil } diff --git a/pkg/config/tlscfg/flags_test.go b/pkg/config/tlscfg/flags_test.go index 6bd5d3b357c0..15e3eea283a0 100644 --- a/pkg/config/tlscfg/flags_test.go +++ b/pkg/config/tlscfg/flags_test.go @@ -57,7 +57,7 @@ func TestClientFlags(t *testing.T) { err := command.ParseFlags(append(cmdLine, test.option)) require.NoError(t, err) - tlsOpts := flagCfg.InitFromViper(v) + tlsOpts,_ := flagCfg.InitFromViper(v) assert.Equal(t, Options{ Enabled: true, CAPath: "ca-file", @@ -105,7 +105,7 @@ func TestServerFlags(t *testing.T) { cmdLine[0] = test.option err := command.ParseFlags(cmdLine) require.NoError(t, err) - tlsOpts := flagCfg.InitFromViper(v) + tlsOpts, _ := flagCfg.InitFromViper(v) assert.Equal(t, Options{ Enabled: true, CertPath: "cert-file", @@ -114,4 +114,4 @@ func TestServerFlags(t *testing.T) { }, tlsOpts) }) } -} +} \ No newline at end of file