Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add reload interval to OTel server certificates #4898

Merged
merged 4 commits into from
Oct 27, 2023

Conversation

james-ryans
Copy link
Contributor

@james-ryans james-ryans commented Oct 26, 2023

Which problem is this PR solving?

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

Signed-off-by: James Ryans <james.ryans2012@gmail.com>
@james-ryans james-ryans requested a review from a team as a code owner October 26, 2023 20:25
@codecov
Copy link

codecov bot commented Oct 26, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Files Coverage Δ
cmd/collector/app/flags/flags.go 100.00% <ø> (ø)
cmd/collector/app/handler/otlp_receiver.go 100.00% <100.00%> (ø)
pkg/config/tlscfg/flags.go 100.00% <100.00%> (ø)
pkg/config/tlscfg/options.go 100.00% <ø> (ø)

... and 2 files with indirect coverage changes

📢 Thoughts on this report? Let us know!.

@@ -66,6 +67,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)")
flags.Duration(c.Prefix+tlsReloadInterval, 0, "The duration after which the certificate will be reloaded (0s means will not be reloaded)")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to add a capability, e.g. ServerFlagsConfig.EnableCertReloadInterval, to enable/disable this option. Otherwise it will show up in all uses of the tlscfg, including the many places which do NOT implement the reload functionality today (but may implement file watching).

…d otlp.grpc.tls flags

Signed-off-by: James Ryans <james.ryans2012@gmail.com>
Signed-off-by: James Ryans <james.ryans2012@gmail.com>
@james-ryans james-ryans changed the title [WIP] Add reload interval to OTel server certificates Add reload interval to OTel server certificates Oct 27, 2023
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Signed-off-by: James Ryans <james.ryans2012@gmail.com>
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@yurishkuro yurishkuro merged commit 107c91b into jaegertracing:main Oct 27, 2023
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]: Reloading of server certificates for server endpoints
2 participants