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>
jaegertracing · Jun 22, 2021 · ebbd75c · ebbd75c
1 parent c93467d
commit ebbd75c
Show file tree

Hide file tree

Showing 4 changed files with 106 additions and 3 deletions.
diff --git a/cmd/collector/app/builder_flags.go b/cmd/collector/app/builder_flags.go
@@ -102,12 +102,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
@@ -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
@@ -108,7 +108,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
@@ -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")
+		})
+	}
+}