Skip to content

ROX-34637: Hot reload TLS certificates in client connections#20661

Open
vladbologa wants to merge 3 commits into
masterfrom
vb/hot-reload-client-certs
Open

ROX-34637: Hot reload TLS certificates in client connections#20661
vladbologa wants to merge 3 commits into
masterfrom
vb/hot-reload-client-certs

Conversation

@vladbologa
Copy link
Copy Markdown
Contributor

@vladbologa vladbologa commented May 18, 2026

Description

Reload leaf certificates every time a client connection is established.

The compliance node indexer additionally switches from a tls.Config with InsecureSkipVerify to clientconn.TLSConfig, gaining both hot reload and server certificate verification against the StackRox CA.

User-facing documentation

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

Tested with a client admission-control connection to sensor:

  • Deployed a fake gRPC sensor that logs the client certificate serial from each TLS handshake
  • Scaled down the real sensor, pointed admission-control at the fake sensor via the sensor Service
  • Patched tls-cert-admission-control secret with a new cert, then killed the fake sensor to force reconnections
  • All 3 admission-control pods reconnected presenting the new client cert serial

@vladbologa
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 18, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 18, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

The PR moves client certificate loading from initialization time to TLS handshake time. Instead of eagerly loading and assigning certificates to conf.Certificates, a GetClientCertificate callback now loads certificates on-demand. Error handling is adjusted: required certificates (MustUseClientCert) cause handshake failures, while optional certificates log warnings and proceed.

Changes

Client Certificate Lazy Loading

Layer / File(s) Summary
Lazy certificate loading callback
pkg/clientconn/client.go
TLSConfig replaces eager certificate loading with a GetClientCertificate callback that loads from mtls.LeafCertificateFromFile during handshakes. Load failures are handled based on certificate requirement: mandatory certificates fail the handshake, optional certificates log warnings and return empty.

🎯 3 (Moderate) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The PR description includes a clear summary of changes and comprehensive validation details, but lacks confirmation that production readiness checks are complete. Confirm whether the change is GA or gated by a feature flag, and clarify the status of CI inspection as indicated in the testing and quality section.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: implementing hot reload functionality for TLS certificates in client connections, which aligns with the code change from eager to lazy certificate loading.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch vb/hot-reload-client-certs

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 18, 2026

🚀 Build Images Ready

Images are ready for commit 08badce. To use with deploy scripts:

export MAIN_IMAGE_TAG=4.11.x-1032-g08badce154

@vladbologa vladbologa force-pushed the vb/hot-reload-client-certs branch from 2f016b5 to 5bddc37 Compare May 18, 2026 18:48
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented May 18, 2026

@vladbologa: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/ocp-4-12-operator-e2e-tests 08badce link false /test ocp-4-12-operator-e2e-tests
ci/prow/ocp-4-21-compliance-e2e-tests 08badce link false /test ocp-4-21-compliance-e2e-tests
ci/prow/ocp-4-21-operator-e2e-tests 08badce link false /test ocp-4-21-operator-e2e-tests
ci/prow/ocp-4-21-qa-e2e-tests 08badce link false /test ocp-4-21-qa-e2e-tests
ci/prow/ocp-4-21-nongroovy-e2e-tests 08badce link false /test ocp-4-21-nongroovy-e2e-tests

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@vladbologa vladbologa force-pushed the vb/hot-reload-client-certs branch from 08badce to 752433c Compare May 19, 2026 07:39
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.

1 participant