Skip to content

feat: add support for reloading certs when renewed #917

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

afrancis101
Copy link

For #45 so the k8s-image-swapper can handle the rotation of webhook certificates

@github-actions github-actions bot added the enhancement New feature or request label Apr 28, 2025
@afrancis101 afrancis101 force-pushed the feat/auto-reload-certificates branch from 3a6e08e to 70267f6 Compare April 28, 2025 15:37
@estahn
Copy link
Owner

estahn commented Apr 28, 2025

Would you mind explaining how this works? Under what conditions is "getCertificate" executed?

Once a Certificate is returned it should not be modified.

@afrancis101
Copy link
Author

Would you mind explaining how this works? Under what conditions is "getCertificate" executed?

Once a Certificate is returned it should not be modified.

Sure, my understanding is that it is executed on every TLS handshake. Which does makes it pretty inefficient at present!
One of my colleagues has provided an example of caching the cert/key response and only updating the response if the files change. I'll look to update the PR to use that pattern instead if that's okay?

@afrancis101 afrancis101 force-pushed the feat/auto-reload-certificates branch from 70267f6 to ecb30d6 Compare April 29, 2025 16:55
@afrancis101 afrancis101 force-pushed the feat/auto-reload-certificates branch from ecb30d6 to ca940b6 Compare April 29, 2025 17:12
@afrancis101
Copy link
Author

Once a Certificate is returned it should not be modified.

On this, I'd argue that the returned certificate is not being modified. The cert/key on file will change over time but we are not modifying what is returned to GetCertificate. Other projects such as kubernetes-sigs/controller-runtime use similar patterns from what I can see too.

@afrancis101 afrancis101 marked this pull request as ready for review April 30, 2025 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants