Skip to content

Commit

Permalink
Add reload interval to OTel server certificates (#4898)
Browse files Browse the repository at this point in the history
## 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 <james.ryans2012@gmail.com>
  • Loading branch information
james-ryans committed Oct 27, 2023
1 parent 2aa0d90 commit 107c91b
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 33 deletions.
6 changes: 4 additions & 2 deletions cmd/collector/app/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
}
Expand Down
29 changes: 29 additions & 0 deletions cmd/collector/app/flags/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 6 additions & 5 deletions cmd/collector/app/handler/otlp_receiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
32 changes: 18 additions & 14 deletions cmd/collector/app/handler/otlp_receiver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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) {
Expand All @@ -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"},
Expand All @@ -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"})
}
8 changes: 7 additions & 1 deletion pkg/config/tlscfg/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down
34 changes: 34 additions & 0 deletions pkg/config/tlscfg/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
24 changes: 13 additions & 11 deletions pkg/config/tlscfg/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 107c91b

Please sign in to comment.