add tests and improve should reload logic

helper/tlsutil/config.go

@@ -422,24 +422,26 @@ func ParseMinVersion(version string) (uint16, error) {
 	return vers, nil
 }
 
+// ShouldReloadRPCConnections compares two TLS Configurations and determines
+// whether they differ such that RPC connections should be reloaded
 func ShouldReloadRPCConnections(old, new *config.TLSConfig) (bool, error) {
-	var tlsInfoEqual bool
+	var certificateInfoEqual bool
+	var rpcInfoEqual bool
 
 	// If already configured with TLS, compare with the new TLS configuration
 	if new != nil {
 		var err error
-		tlsInfoEqual, err = new.CertificateInfoIsEqual(old)
+		certificateInfoEqual, err = new.CertificateInfoIsEqual(old)
 		if err != nil {
 			return false, err
 		}
-	} else {
-		// If not configured with TLS, compare with the new TLS configuration
-		tlsInfoEqual = new == nil && old == nil
+	} else if new == nil && old == nil {
+		certificateInfoEqual = true
 	}
 
-	if new != nil && old != nil {
-		tlsInfoEqual = new.EnableRPC == old.EnableRPC
+	if new != nil && old != nil && new.EnableRPC == old.EnableRPC {
+		rpcInfoEqual = true
 	}
 
-	return tlsInfoEqual, nil
+	return (!rpcInfoEqual || !certificateInfoEqual), nil
 }

helper/tlsutil/config_test.go

@@ -783,3 +783,89 @@ func TestConfig_NewTLSConfiguration(t *testing.T) {
 	}
 	require.Equal(tlsConf.CipherSuites, expectedCiphers)
 }
+
+func TestConfig_ShouldReloadRPCConnections(t *testing.T) {
+	require := require.New(t)
+
+	type shouldReloadTestInput struct {
+		old          *config.TLSConfig
+		new          *config.TLSConfig
+		shouldReload bool
+		errorStr     string
+	}
+
+	testInput := []*shouldReloadTestInput{
+		{
+			old: &config.TLSConfig{
+				CAFile:   cacert,
+				CertFile: badcert,
+				KeyFile:  badkey,
+			},
+			new: &config.TLSConfig{
+				CAFile:   cacert,
+				CertFile: badcert,
+				KeyFile:  badkey,
+			},
+			shouldReload: false,
+			errorStr:     "Same TLS Configuration should not reload",
+		},
+		{
+			old: &config.TLSConfig{
+				CAFile:   cacert,
+				CertFile: badcert,
+				KeyFile:  badkey,
+			},
+			new: &config.TLSConfig{
+				CAFile:   cacert,
+				CertFile: foocert,
+				KeyFile:  fookey,
+			},
+			shouldReload: true,
+			errorStr:     "Different TLS Configuration should reload",
+		},
+		{
+			old: &config.TLSConfig{
+				CAFile:    cacert,
+				CertFile:  badcert,
+				KeyFile:   badkey,
+				EnableRPC: true,
+			},
+			new: &config.TLSConfig{
+				CAFile:    cacert,
+				CertFile:  badcert,
+				KeyFile:   badkey,
+				EnableRPC: false,
+			},
+			shouldReload: true,
+			errorStr:     "Downgrading RPC connections should force reload",
+		},
+		{
+			old: nil,
+			new: &config.TLSConfig{
+				CAFile:    cacert,
+				CertFile:  badcert,
+				KeyFile:   badkey,
+				EnableRPC: true,
+			},
+			shouldReload: true,
+			errorStr:     "Upgrading RPC connections should force reload",
+		},
+		{
+			old: &config.TLSConfig{
+				CAFile:    cacert,
+				CertFile:  badcert,
+				KeyFile:   badkey,
+				EnableRPC: true,
+			},
+			new:          nil,
+			shouldReload: true,
+			errorStr:     "Downgrading RPC connections should force reload",
+		},
+	}
+
+	for _, testCase := range testInput {
+		shouldReload, err := ShouldReloadRPCConnections(testCase.old, testCase.new)
+		require.Nil(err)
+		require.Equal(shouldReload, testCase.shouldReload, testCase.errorStr)
+	}
+}