TLS config pointer is never nil which regresses TLS Support (envoypro…

…xy#318) * TLS config pointer is never nil, fixes regression in pull envoyproxy#289 fixes isssue envoyproxy#303 Signed-off-by: Vito Sabella <[email protected]> * Accidentally put the initialization before the environment variable reading. Signed-off-by: Vito Sabella <[email protected]> * Unit test for settings tlsConfig fix Signed-off-by: Vito Sabella <[email protected]> * Fixing pre-commits Signed-off-by: Vito Sabella <[email protected]> Co-authored-by: Vito Sabella <[email protected]>
barroca · Sep 1, 2023 · 44c2512 · 44c2512
1 parent 7f9c71f
commit 44c2512
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 7 deletions.
diff --git a/src/redis/driver_impl.go b/src/redis/driver_impl.go
@@ -71,11 +71,7 @@ func NewClientImpl(scope stats.Scope, useTls bool, auth, redisSocketType, redisT
 		var dialOpts []radix.DialOpt
 
 		if useTls {
-			if tlsConfig != nil {
-				dialOpts = append(dialOpts, radix.DialUseTLS(tlsConfig))
-			} else {
-				dialOpts = append(dialOpts, radix.DialUseTLS(&tls.Config{}))
-			}
+			dialOpts = append(dialOpts, radix.DialUseTLS(tlsConfig))
 		}
 
 		if auth != "" {

diff --git a/src/settings/settings.go b/src/settings/settings.go
@@ -109,8 +109,13 @@ type Option func(*Settings)
 
 func NewSettings() Settings {
 	var s Settings
-
 	err := envconfig.Process("", &s)
+
+	// Golang copy-by-value causes the RootCAs to no longer be nil
+	// which isn't the expected default behavior of continuing to use system roots
+	// so let's just initialize to what we want the correct value to be.
+	s.RedisTlsConfig = &tls.Config{}
+
 	if err != nil {
 		panic(err)
 	}

diff --git a/src/settings/settings_test.go b/src/settings/settings_test.go
@@ -0,0 +1,13 @@
+package settings
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSettingsTlsConfigUnmodified(t *testing.T) {
+	settings := NewSettings()
+	assert.NotNil(t, settings.RedisTlsConfig)
+	assert.Nil(t, settings.RedisTlsConfig.RootCAs)
+}
diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go
@@ -1,8 +1,10 @@
+//go:build integration
 // +build integration
 
 package integration_test
 
 import (
+	"crypto/tls"
 	"fmt"
 	"io"
 	"math/rand"
@@ -236,7 +238,7 @@ func TestMultiNodeMemcache(t *testing.T) {
 
 func testBasicConfigAuthTLS(perSecond bool, local_cache_size int) func(*testing.T) {
 	s := makeSimpleRedisSettings(16381, 16382, perSecond, local_cache_size)
-	s.RedisTlsConfig = nil
+	s.RedisTlsConfig = &tls.Config{}
 	s.RedisAuth = "password123"
 	s.RedisTls = true
 	s.RedisPerSecondAuth = "password123"