Enable reload-interval flag for all TLS server configs (#6525)

## Which problem is this PR solving? - Resolves #6521 ## Description of the changes - Added the field EnableCertReloadInterval: true everywhere ServerFlagsConfig is used - As a dual check for future use, set the value of c.EnableCertReloadInterval to true in AddFlags function in pkg/config/tlscfg/flags.go ## How was this change tested? - ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [ ] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` --------- Signed-off-by: cs-308-2023 <[email protected]> Signed-off-by: Yuri Shkuro <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]>
jaegertracing · Jan 12, 2025 · c7e0b78 · c7e0b78
1 parent 89c4ee4
commit c7e0b78
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 66 deletions.
diff --git a/cmd/collector/app/flags/flags.go b/cmd/collector/app/flags/flags.go
@@ -77,15 +77,13 @@ var otlpServerFlagsCfg = struct {
 	GRPC: serverFlagsConfig{
 		prefix: "collector.otlp.grpc",
 		tls: tlscfg.ServerFlagsConfig{
-			Prefix:                   "collector.otlp.grpc",
-			EnableCertReloadInterval: true,
+			Prefix: "collector.otlp.grpc",
 		},
 	},
 	HTTP: serverFlagsConfig{
 		prefix: "collector.otlp.http",
 		tls: tlscfg.ServerFlagsConfig{
-			Prefix:                   "collector.otlp.http",
-			EnableCertReloadInterval: true,
+			Prefix: "collector.otlp.http",
 		},
 		corsCfg: corscfg.Flags{
 			Prefix: "collector.otlp.http",

diff --git a/cmd/collector/app/flags/flags_test.go b/cmd/collector/app/flags/flags_test.go
@@ -70,34 +70,6 @@ func TestCollectorOptionsWithFailedTLSFlags(t *testing.T) {
 	}
 }
 
-func TestCollectorOptionsWithFlags_CheckTLSReloadInterval(t *testing.T) {
-	prefixes := []string{
-		"--collector.http",
-		"--collector.grpc",
-		"--collector.zipkin",
-		"--collector.otlp.http",
-		"--collector.otlp.grpc",
-	}
-	otlpPrefixes := map[string]struct{}{
-		"--collector.otlp.http": {},
-		"--collector.otlp.grpc": {},
-	}
-	for _, prefix := range prefixes {
-		t.Run(prefix, func(t *testing.T) {
-			_, command := config.Viperize(AddFlags)
-			err := command.ParseFlags([]string{
-				prefix + ".tls.enabled=true",
-				prefix + ".tls.reload-interval=24h",
-			})
-			if _, ok := otlpPrefixes[prefix]; !ok {
-				assert.ErrorContains(t, err, "unknown flag")
-			} else {
-				require.NoError(t, err)
-			}
-		})
-	}
-}
-
 func TestCollectorOptionsWithFlags_CheckMaxReceiveMessageLength(t *testing.T) {
 	c := &CollectorOptions{}
 	v, command := config.Viperize(AddFlags)

diff --git a/pkg/config/tlscfg/flags.go b/pkg/config/tlscfg/flags.go
@@ -35,8 +35,7 @@ type ClientFlagsConfig struct {
 
 // ServerFlagsConfig describes which CLI flags for TLS server should be generated.
 type ServerFlagsConfig struct {
-	Prefix                   string
-	EnableCertReloadInterval bool
+	Prefix string
 }
 
 // AddFlags adds flags for TLS to the FlagSet.
@@ -58,9 +57,7 @@ func (c ServerFlagsConfig) AddFlags(flags *flag.FlagSet) {
 	flags.String(c.Prefix+tlsCipherSuites, "", "Comma-separated list of cipher suites for the server, values are from tls package constants (https://golang.org/pkg/crypto/tls/#pkg-constants).")
 	flags.String(c.Prefix+tlsMinVersion, "", "Minimum TLS version supported (Possible values: 1.0, 1.1, 1.2, 1.3)")
 	flags.String(c.Prefix+tlsMaxVersion, "", "Maximum TLS version supported (Possible values: 1.0, 1.1, 1.2, 1.3)")
-	if c.EnableCertReloadInterval {
-		flags.Duration(c.Prefix+tlsReloadInterval, 0, "The duration after which the certificate will be reloaded (0s means will not be reloaded)")
-	}
+	flags.Duration(c.Prefix+tlsReloadInterval, 0, "The duration after which the certificate will be reloaded (0s means will not be reloaded)")
 }
 
 // InitFromViper creates tls.Config populated with values retrieved from Viper.

diff --git a/pkg/config/tlscfg/flags_test.go b/pkg/config/tlscfg/flags_test.go
@@ -6,6 +6,7 @@ package tlscfg
 import (
 	"flag"
 	"testing"
+	"time"
 
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
@@ -125,36 +126,18 @@ func TestServerFlags(t *testing.T) {
 }
 
 func TestServerCertReloadInterval(t *testing.T) {
-	tests := []struct {
-		config ServerFlagsConfig
-	}{
-		{
-			config: ServerFlagsConfig{
-				Prefix:                   "enabled",
-				EnableCertReloadInterval: true,
-			},
-		},
-		{
-			config: ServerFlagsConfig{
-				Prefix:                   "disabled",
-				EnableCertReloadInterval: false,
-			},
-		},
-	}
-	for _, test := range tests {
-		t.Run(test.config.Prefix, func(t *testing.T) {
-			_, command := config.Viperize(test.config.AddFlags)
-			err := command.ParseFlags([]string{
-				"--" + test.config.Prefix + ".tls.enabled=true",
-				"--" + test.config.Prefix + ".tls.reload-interval=24h",
-			})
-			if !test.config.EnableCertReloadInterval {
-				assert.ErrorContains(t, err, "unknown flag")
-			} else {
-				require.NoError(t, err)
-			}
-		})
+	cfg := ServerFlagsConfig{
+		Prefix: "prefix",
 	}
+	v, command := config.Viperize(cfg.AddFlags)
+	err := command.ParseFlags([]string{
+		"--prefix.tls.enabled=true",
+		"--prefix.tls.reload-interval=24h",
+	})
+	require.NoError(t, err)
+	tlscfg, err := cfg.InitFromViper(v)
+	require.NoError(t, err)
+	assert.Equal(t, 24*time.Hour, tlscfg.ReloadInterval)
 }
 
 // TestFailedTLSFlags verifies that TLS options cannot be used when tls.enabled=false