Skip to content

feat: Hot reload TLS certificate when TLS cert or key files changes#111

Merged
clement0010 merged 28 commits intomasterfrom
feature/hot-reload-tls-cert-and-key
Aug 28, 2025
Merged

feat: Hot reload TLS certificate when TLS cert or key files changes#111
clement0010 merged 28 commits intomasterfrom
feature/hot-reload-tls-cert-and-key

Conversation

@clement0010
Copy link
Copy Markdown
Contributor

@clement0010 clement0010 commented Aug 19, 2025

Changes

  • Create a custom CertReloader to watch for TLS certificate and private key changes. When the watcher is removed, it will also attempt to restart the watcher periodically (every 1 minute).

Notes

When the TLS Secret is changed, we expect fsnotify to notify multiple chmod events. As such, it's not uncommon to reload the cert file multiple times during cert rotation.

Reference: K8s Dynamic Certificates

@clement0010 clement0010 requested a review from Copilot August 19, 2025 11:05

This comment was marked as outdated.

@clement0010 clement0010 requested a review from minhtule August 19, 2025 11:55
Comment thread go.mod Outdated
Comment thread go.mod Outdated
Comment thread internal/httpproxy/cert_reloader.go Outdated
Comment thread internal/httpproxy/cert_reloader.go Outdated
Comment thread internal/httpproxy/cert_reloader_test.go Outdated
Comment thread internal/httpproxy/cert_reloader_test.go Outdated
Comment thread internal/httpproxy/cert_reloader_test.go Outdated
Comment thread internal/httpproxy/cert_reloader_test.go Outdated
Comment thread internal/httpproxy/cert_reloader_test.go Outdated
Comment thread internal/httpproxy/cert_reloader_test.go Outdated
@clement0010 clement0010 changed the title feat: Hot reload TLS certificate when k8s Secret object modified feat: Hot reload TLS certificate when TLS cert or key files changes Aug 22, 2025
Comment thread internal/httpproxy/cert_reloader.go Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds hot reload functionality for TLS certificates by implementing a custom CertReloader that watches for changes to TLS certificate and private key files in the /etc/tls-secret-volume directory using fsnotify.

  • Implements a CertReloader struct that monitors file system events for certificate and key files
  • Replaces static certificate loading with dynamic certificate retrieval through GetCertificate callback
  • Adds comprehensive test coverage for certificate reloading scenarios including file changes and invalid certificate pairs

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
internal/httpproxy/cert_reloader.go Core implementation of the certificate reloader with file watching and hot reload logic
internal/httpproxy/cert_reloader_test.go Test suite covering certificate reloading scenarios and edge cases
internal/httpproxy/http_proxy.go Integration of cert reloader into the HTTP proxy server configuration
test/data/proxy/tls1.crt Additional test certificate for testing certificate rotation
test/data/proxy/tls1.key Additional test private key for testing certificate rotation
test/data/data.go Embedded test data variables for the new certificate and key files
go.mod Moved fsnotify dependency from indirect to direct requirement
Comments suppressed due to low confidence (1)

go.mod:3

  • Go version 1.24.6 does not exist. The latest Go version as of January 2025 was 1.23.x. This appears to be an invalid version number.
go 1.24.6

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread internal/httpproxy/cert_reloader.go Outdated
Comment thread internal/httpproxy/cert_reloader.go Outdated
Comment thread internal/httpproxy/cert_reloader.go
@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Aug 22, 2025

Pull Request Test Coverage Report for Build 17256260704

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 81 of 91 (89.01%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.1%) to 92.239%

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/httpproxy/http_proxy.go 8 12 66.67%
internal/httpproxy/cert_reloader.go 73 79 92.41%
Totals Coverage Status
Change from base Build 17152291955: -0.1%
Covered Lines: 1438
Relevant Lines: 1559

💛 - Coveralls

Comment thread internal/httpproxy/cert_reloader.go Outdated
Comment thread internal/httpproxy/cert_reloader.go Outdated
Comment thread internal/httpproxy/cert_reloader.go Outdated
Comment thread internal/httpproxy/cert_reloader.go Outdated
Comment thread internal/httpproxy/cert_reloader_test.go Outdated
Comment thread internal/httpproxy/cert_reloader_test.go Outdated
Comment thread internal/httpproxy/cert_reloader_test.go Outdated
@clement0010 clement0010 requested a review from minhtule August 25, 2025 06:18
Copy link
Copy Markdown
Contributor

@minhtule minhtule left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Comment thread internal/httpproxy/cert_reloader.go Outdated
Comment thread internal/httpproxy/cert_reloader.go Outdated
Comment thread internal/httpproxy/cert_reloader.go
Comment thread internal/httpproxy/cert_reloader.go Outdated
@clement0010 clement0010 requested a review from sghiocel August 27, 2025 03:57
@clement0010 clement0010 merged commit f1cc74a into master Aug 28, 2025
12 checks passed
@clement0010 clement0010 deleted the feature/hot-reload-tls-cert-and-key branch August 28, 2025 04:15
clement0010 added a commit that referenced this pull request Aug 29, 2025
…#122)

## Changes
- After merging the TLS cert hot-reload
[PR](#111), we
should remove the `checksum/tlsAlternativeNames` from `deployment.yaml`
template to avoid restarting the Gateway pod when the TLS secret changes
minhtule added a commit that referenced this pull request Dec 30, 2025
)

## Changes

Move inner TLS upgrade outside of `ProxyConn.Authenticate()` to keep
`ProxyConn.Authenticate()` only responsible for the authentication.
After that, the connection would have 2 paths
- SSH secure transport (only for SSH protocol): the connection is sent
to `SSHListener`. The SSH proxy handles both the secure transport and
application layer.
- TLS secure transport: upgrade the `ProxyConn` connection to TLS and
pass it to the `HTTPListener`. Right now only HTTPS (`kubectl`) uses
this transport but in the future, database protocols like Postgres would
use TLS as the secure transport too. We will have different listener for
different L7 procol.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants