From 107c91b1c874af1e9c48cc110b41f13b603dc2e9 Mon Sep 17 00:00:00 2001 From: James Ryans <46216691+james-ryans@users.noreply.github.com> Date: Sat, 28 Oct 2023 00:13:49 +0700 Subject: [PATCH] Add reload interval to OTel server certificates (#4898) ## Which problem is this PR solving? - Resolves #4554 ## Description of the changes - Enable reload interval functionality to OTel server certificates by adding `otlp.http.tls.reload-interval` and `otlp.grpc.tls.reload-interval` flags (0s means disabled) ## How was this change tested? - Tested manually in local - Added `*.tls.reload-interval` flag unit tests to `flags.go` and `otel_receiver.go` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] 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`: `yarn lint` and `yarn test` --------- Signed-off-by: James Ryans --- cmd/collector/app/flags/flags.go | 6 ++-- cmd/collector/app/flags/flags_test.go | 29 ++++++++++++++++ cmd/collector/app/handler/otlp_receiver.go | 11 +++--- .../app/handler/otlp_receiver_test.go | 32 +++++++++-------- pkg/config/tlscfg/flags.go | 8 ++++- pkg/config/tlscfg/flags_test.go | 34 +++++++++++++++++++ pkg/config/tlscfg/options.go | 24 +++++++------ 7 files changed, 111 insertions(+), 33 deletions(-) diff --git a/cmd/collector/app/flags/flags.go b/cmd/collector/app/flags/flags.go index f252380fb7b..6c5e5a2babd 100644 --- a/cmd/collector/app/flags/flags.go +++ b/cmd/collector/app/flags/flags.go @@ -83,13 +83,15 @@ var otlpServerFlagsCfg = struct { GRPC: serverFlagsConfig{ prefix: "collector.otlp.grpc", tls: tlscfg.ServerFlagsConfig{ - Prefix: "collector.otlp.grpc", + Prefix: "collector.otlp.grpc", + EnableCertReloadInterval: true, }, }, HTTP: serverFlagsConfig{ prefix: "collector.otlp.http", tls: tlscfg.ServerFlagsConfig{ - Prefix: "collector.otlp.http", + Prefix: "collector.otlp.http", + EnableCertReloadInterval: true, }, }, } diff --git a/cmd/collector/app/flags/flags_test.go b/cmd/collector/app/flags/flags_test.go index faa1148e354..6916358e490 100644 --- a/cmd/collector/app/flags/flags_test.go +++ b/cmd/collector/app/flags/flags_test.go @@ -81,6 +81,35 @@ 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 { + require.Error(t, err) + assert.Contains(t, err.Error(), "unknown flag") + } else { + require.NoError(t, err) + } + }) + } +} + func TestCollectorOptionsWithFlags_CheckMaxReceiveMessageLength(t *testing.T) { c := &CollectorOptions{} v, command := config.Viperize(AddFlags) diff --git a/cmd/collector/app/handler/otlp_receiver.go b/cmd/collector/app/handler/otlp_receiver.go index 3f3c2323de6..e7513660b89 100644 --- a/cmd/collector/app/handler/otlp_receiver.go +++ b/cmd/collector/app/handler/otlp_receiver.go @@ -143,11 +143,12 @@ func applyHTTPSettings(cfg *confighttp.HTTPServerSettings, opts *flags.HTTPOptio func applyTLSSettings(opts *tlscfg.Options) *configtls.TLSServerSetting { return &configtls.TLSServerSetting{ TLSSetting: configtls.TLSSetting{ - CAFile: opts.CAPath, - CertFile: opts.CertPath, - KeyFile: opts.KeyPath, - MinVersion: opts.MinVersion, - MaxVersion: opts.MaxVersion, + CAFile: opts.CAPath, + CertFile: opts.CertPath, + KeyFile: opts.KeyPath, + MinVersion: opts.MinVersion, + MaxVersion: opts.MaxVersion, + ReloadInterval: opts.ReloadInterval, }, ClientCAFile: opts.ClientCAPath, } diff --git a/cmd/collector/app/handler/otlp_receiver_test.go b/cmd/collector/app/handler/otlp_receiver_test.go index 4cae3d0ff0e..7f3fd15b64c 100644 --- a/cmd/collector/app/handler/otlp_receiver_test.go +++ b/cmd/collector/app/handler/otlp_receiver_test.go @@ -164,13 +164,14 @@ func TestApplyOTLPGRPCServerSettings(t *testing.T) { MaxConnectionAge: 33 * time.Second, MaxConnectionAgeGrace: 37 * time.Second, TLS: tlscfg.Options{ - Enabled: true, - CAPath: "ca", - CertPath: "cert", - KeyPath: "key", - ClientCAPath: "clientca", - MinVersion: "1.1", - MaxVersion: "1.3", + Enabled: true, + CAPath: "ca", + CertPath: "cert", + KeyPath: "key", + ClientCAPath: "clientca", + MinVersion: "1.1", + MaxVersion: "1.3", + ReloadInterval: 24 * time.Hour, }, } applyGRPCSettings(otlpReceiverConfig.GRPC, grpcOpts) @@ -188,6 +189,7 @@ func TestApplyOTLPGRPCServerSettings(t *testing.T) { assert.Equal(t, out.TLSSetting.ClientCAFile, "clientca") assert.Equal(t, out.TLSSetting.MinVersion, "1.1") assert.Equal(t, out.TLSSetting.MaxVersion, "1.3") + assert.Equal(t, out.TLSSetting.ReloadInterval, 24*time.Hour) } func TestApplyOTLPHTTPServerSettings(t *testing.T) { @@ -197,13 +199,14 @@ func TestApplyOTLPHTTPServerSettings(t *testing.T) { httpOpts := &flags.HTTPOptions{ HostPort: ":12345", TLS: tlscfg.Options{ - Enabled: true, - CAPath: "ca", - CertPath: "cert", - KeyPath: "key", - ClientCAPath: "clientca", - MinVersion: "1.1", - MaxVersion: "1.3", + Enabled: true, + CAPath: "ca", + CertPath: "cert", + KeyPath: "key", + ClientCAPath: "clientca", + MinVersion: "1.1", + MaxVersion: "1.3", + ReloadInterval: 24 * time.Hour, }, CORS: corscfg.Options{ AllowedOrigins: []string{"http://example.domain.com", "http://*.domain.com"}, @@ -223,6 +226,7 @@ func TestApplyOTLPHTTPServerSettings(t *testing.T) { assert.Equal(t, out.TLSSetting.ClientCAFile, "clientca") assert.Equal(t, out.TLSSetting.MinVersion, "1.1") assert.Equal(t, out.TLSSetting.MaxVersion, "1.3") + assert.Equal(t, out.TLSSetting.ReloadInterval, 24*time.Hour) assert.Equal(t, out.CORS.AllowedHeaders, []string{"Content-Type", "Accept", "X-Requested-With"}) assert.Equal(t, out.CORS.AllowedOrigins, []string{"http://example.domain.com", "http://*.domain.com"}) } diff --git a/pkg/config/tlscfg/flags.go b/pkg/config/tlscfg/flags.go index b3c313a9aa5..96de6b7cf4e 100644 --- a/pkg/config/tlscfg/flags.go +++ b/pkg/config/tlscfg/flags.go @@ -35,6 +35,7 @@ const ( tlsCipherSuites = tlsPrefix + ".cipher-suites" tlsMinVersion = tlsPrefix + ".min-version" tlsMaxVersion = tlsPrefix + ".max-version" + tlsReloadInterval = tlsPrefix + ".reload-interval" ) // ClientFlagsConfig describes which CLI flags for TLS client should be generated. @@ -44,7 +45,8 @@ type ClientFlagsConfig struct { // ServerFlagsConfig describes which CLI flags for TLS server should be generated. type ServerFlagsConfig struct { - Prefix string + Prefix string + EnableCertReloadInterval bool } // AddFlags adds flags for TLS to the FlagSet. @@ -66,6 +68,9 @@ 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)") + } } // InitFromViper creates tls.Config populated with values retrieved from Viper. @@ -100,6 +105,7 @@ func (c ServerFlagsConfig) InitFromViper(v *viper.Viper) (Options, error) { } p.MinVersion = v.GetString(c.Prefix + tlsMinVersion) p.MaxVersion = v.GetString(c.Prefix + tlsMaxVersion) + p.ReloadInterval = v.GetDuration(c.Prefix + tlsReloadInterval) if !p.Enabled { var empty Options diff --git a/pkg/config/tlscfg/flags_test.go b/pkg/config/tlscfg/flags_test.go index 9bd9f3b47a1..be5adeb868a 100644 --- a/pkg/config/tlscfg/flags_test.go +++ b/pkg/config/tlscfg/flags_test.go @@ -122,6 +122,40 @@ 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 { + require.Error(t, err) + assert.Contains(t, err.Error(), "unknown flag") + } else { + require.NoError(t, err) + } + }) + } +} + // TestFailedTLSFlags verifies that TLS options cannot be used when tls.enabled=false func TestFailedTLSFlags(t *testing.T) { clientTests := []string{ diff --git a/pkg/config/tlscfg/options.go b/pkg/config/tlscfg/options.go index fc0422be942..aab23cd8db9 100644 --- a/pkg/config/tlscfg/options.go +++ b/pkg/config/tlscfg/options.go @@ -21,23 +21,25 @@ import ( "io" "os" "path/filepath" + "time" "go.uber.org/zap" ) // Options describes the configuration properties for TLS Connections. type Options struct { - Enabled bool `mapstructure:"enabled"` - CAPath string `mapstructure:"ca"` - CertPath string `mapstructure:"cert"` - KeyPath string `mapstructure:"key"` - ServerName string `mapstructure:"server_name"` // only for client-side TLS config - ClientCAPath string `mapstructure:"client_ca"` // only for server-side TLS config for client auth - CipherSuites []string `mapstructure:"cipher_suites"` - MinVersion string `mapstructure:"min_version"` - MaxVersion string `mapstructure:"max_version"` - SkipHostVerify bool `mapstructure:"skip_host_verify"` - certWatcher *certWatcher `mapstructure:"-"` + Enabled bool `mapstructure:"enabled"` + CAPath string `mapstructure:"ca"` + CertPath string `mapstructure:"cert"` + KeyPath string `mapstructure:"key"` + ServerName string `mapstructure:"server_name"` // only for client-side TLS config + ClientCAPath string `mapstructure:"client_ca"` // only for server-side TLS config for client auth + CipherSuites []string `mapstructure:"cipher_suites"` + MinVersion string `mapstructure:"min_version"` + MaxVersion string `mapstructure:"max_version"` + SkipHostVerify bool `mapstructure:"skip_host_verify"` + ReloadInterval time.Duration `mapstructure:"reload_interval"` + certWatcher *certWatcher `mapstructure:"-"` } var systemCertPool = x509.SystemCertPool // to allow overriding in unit test