-
Notifications
You must be signed in to change notification settings - Fork 631
fix(target-allocator): add TLS certificate hot-reload for mTLS #4597
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
Merged
Merged
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
981e9df
fix(target-allocator): add TLS certificate hot-reload for mTLS
CharlieTLe da2f8a2
Merge branch 'main' into ta-mtls-bug
CharlieTLe 577a970
fix(target-allocator): debounce TLS certificate reloads
CharlieTLe dab147a
feat(target-allocator): support TLS cert files in different directories
CharlieTLe d2d4425
fix(tests): use filesystem events instead of manually calling schedul…
CharlieTLe 83296e2
Merge branch 'main' into ta-mtls-bug
CharlieTLe 6d77805
test(target-allocator): verify certificate changes instead of reload …
CharlieTLe 6575064
fix(target-allocator): prevent timer starvation in TLS cert reload de…
CharlieTLe 36815ac
Merge branch 'main' into ta-mtls-bug
CharlieTLe 98924f9
fix linting
CharlieTLe e2fc078
Merge branch 'main' into ta-mtls-bug
CharlieTLe 4a612f6
Use certwatcher from controller-runtime to handle cert rotation
CharlieTLe 4296989
Merge branch 'main' into ta-mtls-bug
CharlieTLe 2d115e5
Remove TestCAReloader_ConcurrentAccess
CharlieTLe d507cbb
Lock CAReloader.Reload() to prevent potential race condition
CharlieTLe 33a728e
Merge branch 'main' into ta-mtls-bug
CharlieTLe 81d511e
Fix mTLS certificate verification to only verify leaf certificate
CharlieTLe af68589
Update go.mod
CharlieTLe b86ce3e
Run make fmt vet lint
CharlieTLe 6a9b4d5
Merge branch 'main' into ta-mtls-bug
CharlieTLe File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | ||
| change_type: bug_fix | ||
|
|
||
| # The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action) | ||
| component: target allocator | ||
|
|
||
| # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). | ||
| note: Fix TLS certificate hot-reload for mTLS connections | ||
|
|
||
| # One or more tracking issues related to the change | ||
| issues: [4368] | ||
|
|
||
| # (Optional) One or more lines of additional information to render under the primary note. | ||
| # These lines will be padded with 2 spaces and then inserted directly into the document. | ||
| # Use pipe (|) for multiline entries. | ||
| subtext: | | ||
| The Target Allocator now automatically reloads TLS certificates when they are renewed | ||
| by cert-manager. Previously, certificate renewals required a pod restart because | ||
| certificates were only loaded once at startup. The fix uses fsnotify to watch the | ||
| certificate directory and dynamically reloads certificates via the GetCertificate | ||
| callback, enabling seamless certificate rotation without downtime. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| // Copyright The OpenTelemetry Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package config | ||
|
|
||
| import ( | ||
| "crypto/x509" | ||
| "fmt" | ||
| "os" | ||
| "sync" | ||
|
|
||
| "github.com/go-logr/logr" | ||
| ) | ||
|
|
||
| // CAReloader manages CA certificate reloading for client verification. | ||
| // It provides thread-safe access to the current CA certificate pool and can be | ||
| // triggered to reload via the Reload() method, typically called by a cert watcher callback. | ||
| type CAReloader struct { | ||
| caPath string | ||
| clientCAs *x509.CertPool | ||
| mu sync.RWMutex | ||
| logger logr.Logger | ||
| } | ||
|
|
||
| // NewCAReloader creates a new CAReloader and loads the initial CA certificate. | ||
| func NewCAReloader(caPath string, logger logr.Logger) (*CAReloader, error) { | ||
| r := &CAReloader{ | ||
| caPath: caPath, | ||
| logger: logger.WithName("ca-reloader"), | ||
| } | ||
|
|
||
| if err := r.Reload(); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return r, nil | ||
| } | ||
|
|
||
| // Reload reads the CA certificate file from disk and updates the cached certificate pool. | ||
| // This method is thread-safe and can be called concurrently. | ||
| func (r *CAReloader) Reload() error { | ||
| r.mu.Lock() | ||
| defer r.mu.Unlock() | ||
|
|
||
| caCert, err := os.ReadFile(r.caPath) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to read CA certificate: %w", err) | ||
| } | ||
|
|
||
| caCertPool := x509.NewCertPool() | ||
| if !caCertPool.AppendCertsFromPEM(caCert) { | ||
| return fmt.Errorf("failed to parse CA certificate at %s", r.caPath) | ||
| } | ||
|
|
||
| r.clientCAs = caCertPool | ||
|
|
||
| r.logger.Info("CA certificate reloaded successfully", "caPath", r.caPath) | ||
| return nil | ||
| } | ||
|
|
||
| // GetClientCAs returns the current CA certificate pool for client verification. | ||
| // This method is safe for concurrent access and is called during TLS handshakes. | ||
| func (r *CAReloader) GetClientCAs() *x509.CertPool { | ||
| r.mu.RLock() | ||
| defer r.mu.RUnlock() | ||
| return r.clientCAs | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| // Copyright The OpenTelemetry Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package config | ||
|
|
||
| import ( | ||
| "os" | ||
| "path/filepath" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ctrl "sigs.k8s.io/controller-runtime" | ||
| ) | ||
|
|
||
| func TestCAReloader_Reload(t *testing.T) { | ||
| tmpDir := t.TempDir() | ||
|
|
||
| // Generate initial CA certificate | ||
| caPEM1, _ := generateTestCertificate(t) | ||
| caPath := filepath.Join(tmpDir, "ca.crt") | ||
| require.NoError(t, os.WriteFile(caPath, caPEM1, 0600)) | ||
|
|
||
| logger := ctrl.Log.WithName("test") | ||
| reloader, err := NewCAReloader(caPath, logger) | ||
| require.NoError(t, err) | ||
|
|
||
| initialCA := reloader.GetClientCAs() | ||
| require.NotNil(t, initialCA) | ||
|
|
||
| // Generate new CA certificate | ||
| caPEM2, _ := generateTestCertificate(t) | ||
| require.NoError(t, os.WriteFile(caPath, caPEM2, 0600)) | ||
|
|
||
| // Reload CA | ||
| err = reloader.Reload() | ||
| require.NoError(t, err) | ||
|
|
||
| // Verify CA pool was updated | ||
| newCA := reloader.GetClientCAs() | ||
| require.NotNil(t, newCA) | ||
| } | ||
|
|
||
| func TestCAReloader_InvalidCA(t *testing.T) { | ||
| tmpDir := t.TempDir() | ||
|
|
||
| // Create valid CA first | ||
| validCAPEM, _ := generateTestCertificate(t) | ||
| caPath := filepath.Join(tmpDir, "ca.crt") | ||
| require.NoError(t, os.WriteFile(caPath, validCAPEM, 0600)) | ||
|
|
||
| logger := ctrl.Log.WithName("test") | ||
| reloader, err := NewCAReloader(caPath, logger) | ||
| require.NoError(t, err) | ||
|
|
||
| oldCA := reloader.GetClientCAs() | ||
| require.NotNil(t, oldCA) | ||
|
|
||
| // Write invalid CA | ||
| require.NoError(t, os.WriteFile(caPath, []byte("invalid"), 0600)) | ||
|
|
||
| // Reload should fail | ||
| err = reloader.Reload() | ||
| require.Error(t, err) | ||
|
|
||
| // Verify old CA is still in use | ||
| currentCA := reloader.GetClientCAs() | ||
| assert.Equal(t, oldCA, currentCA) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.