Skip to content

feat: grpc mTLS certificates are hot reloaded #6756

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 8 commits into from
May 13, 2025

Conversation

JorTurFer
Copy link
Member

Checklist

  • Commits are signed with Developer Certificate of Origin (DCO - learn more)

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
@JorTurFer JorTurFer requested a review from a team as a code owner May 2, 2025 15:27
JorTurFer and others added 4 commits May 2, 2025 17:46
Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
.
Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
Signed-off-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
@JorTurFer
Copy link
Member Author

JorTurFer commented May 4, 2025

/run-e2e
Update: You can check the progress here

@JorTurFer
Copy link
Member Author

JorTurFer commented May 4, 2025

I've tested this behavior on my local with 1 hour certs and it works, but I'm not fully sure about how to test this deterministically, as e2e tests can't ensure when the certificate has been updated without checking the logs, and that could be quite flaky. In the other hand, IDK how to unit test this as credentials.TransportCredentials doesn't expose it internals to test it (I've not seen it at least)

image

@wozniakjan wozniakjan requested a review from Copilot May 9, 2025 06:51
Copy link
Contributor

@Copilot 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 introduces hot reloading for gRPC mTLS certificates by updating TLS credential loading and integrating a file watcher for certificate changes. Key changes include:

  • Adding a context parameter to LoadGrpcTLSCredentials and updating its callers.
  • Implementing a file watcher using fsnotify to detect certificate updates.
  • Bumping the fsnotify dependency and updating the certificate generation command in BUILD.md.

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/metricsservice/utils/tls.go Updated TLS credentials loading to support certificate hot reloading with fsnotify.
pkg/metricsservice/server.go Updated Start() method to pass context for TLS credentials.
pkg/metricsservice/client.go Updated NewGrpcClient() to pass context for TLS credentials.
go.mod Upgraded fsnotify dependency to v1.9.0.
cmd/adapter/main.go Updated to pass context to NewGrpcClient.
CHANGELOG.md Documented the certificate hot-reloading improvement.
BUILD.md Modified openssl command to include subjectAltName certificate extension.

Copy link
Member

@wozniakjan wozniakjan left a comment

Choose a reason for hiding this comment

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

looks great, few minor nits below

JorTurFer and others added 2 commits May 12, 2025 22:20
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
@JorTurFer
Copy link
Member Author

JorTurFer commented May 13, 2025

/run-e2e rabbit
Update: You can check the progress here

@JorTurFer
Copy link
Member Author

It's working:
image

@JorTurFer JorTurFer merged commit bf2b3f1 into kedacore:main May 13, 2025
20 checks passed
@JorTurFer JorTurFer deleted the hot-reload-cert branch May 13, 2025 13:04
@wozniakjan wozniakjan mentioned this pull request May 19, 2025
18 tasks
@wozniakjan wozniakjan mentioned this pull request Jun 3, 2025
12 tasks
wozniakjan pushed a commit to wozniakjan/keda that referenced this pull request Jun 17, 2025
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
wozniakjan pushed a commit to wozniakjan/keda that referenced this pull request Jun 17, 2025
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>
wozniakjan added a commit that referenced this pull request Jun 18, 2025
* feat: grpc mTLS certificates are hot reloaded (#6756)

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>

* fix: temporal scaler was not setting up tls for apikey auth (#6781)

* fix: temporal scaler was not setting up tls for apikey auth

Signed-off-by: joaopaulosr95 <joaopaulosr95@gmail.com>

* Update CHANGELOG.md

Co-authored-by: Jan Wozniak <wozniak.jan@gmail.com>
Signed-off-by: João Bastos <joaopaulosr95@gmail.com>

---------

Signed-off-by: joaopaulosr95 <joaopaulosr95@gmail.com>
Signed-off-by: João Bastos <joaopaulosr95@gmail.com>
Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>
Co-authored-by: Jan Wozniak <wozniak.jan@gmail.com>
Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>

* chore: changelog and issue template v2.17.2

Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>

---------

Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>
Signed-off-by: joaopaulosr95 <joaopaulosr95@gmail.com>
Signed-off-by: João Bastos <joaopaulosr95@gmail.com>
Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: João Bastos <joaopaulosr95@gmail.com>
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.

2 participants