allow filestore to automatically update certificates#6598
Conversation
yegorskii
added
the
filestore
|
Note This is an automated comment that will be appended during run. Note All workloads for linux-x86_64-relwithdebinfo have completed. Tip Planned checks for linux-x86_64-relwithdebinfo.
|
|
Note This is an automated comment that will be appended during run. Note All workloads for linux-x86_64-relwithdebinfo have completed. Tip Planned checks for linux-x86_64-relwithdebinfo. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6807f76a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return CreateCertificateProvider( | ||
| Logging, | ||
| LogComponent, | ||
| Scheduler, | ||
| LongRunningTaskExecutor, |
There was a problem hiding this comment.
When vhost is configured with RefreshCertsPeriod and a remote service endpoint uses SecurePort, this helper now returns a periodic certificate provider owned only by the filestore client. TFileStoreEndpoints::Start() only starts the durable endpoint, and the client start path never calls CertificateProvider->Start() (unlike the top-level daemon CertificateProvider started by TBootstrapCommon), so the provider never publishes its initial materials or schedules reloads; TLS clients created from it can miss certificates and automatic refresh does not work. Store/start/stop these client providers with the endpoint lifecycle before using them for secure channels.
Useful? React with 👍 / 👎.
| LongRunningTaskExecutor = CreateLongRunningTaskExecutor("CertRefresh"); | ||
| } | ||
|
|
||
| CertificateProvider = CreateCertificateProvider( |
There was a problem hiding this comment.
When SecurePort is 0 and Certs is empty but RootCertsFile is still set (for example from a shared config template or a stale secret path), this unconditional provider construction now reads and validates that root file even though the server will never create secure credentials. Previously this path left CertificateProvider unset unless certs were configured, so such non-TLS deployments could start without the root file; keep the old guard or skip TLS material validation when secure serving is disabled.
Useful? React with 👍 / 👎.
| GetComponentName( | ||
| NStorage::TFileStoreComponents::TLS_CERTIFICATE_PROVIDER), | ||
| Scheduler, | ||
| LongRunningTaskExecutor, |
There was a problem hiding this comment.
For vhost remote endpoints with RefreshCertsPeriod enabled, InitEndpoints() builds this factory before InitComponents() creates LongRunningTaskExecutor later in the method, so the factory captures a null task queue. Any periodic client certificate provider created from it will dereference that null queue from ScheduleUpdateAt() once its lifecycle is started or an update is requested, preventing client-side certificate refresh from working reliably; create the executor before initializing endpoints or pass a live queue into the factory.
Useful? React with 👍 / 👎.
|
Note This is an automated comment that will be appended during run. Note All workloads for linux-x86_64-relwithdebinfo have completed. Tip Planned checks for linux-x86_64-relwithdebinfo. 🟢 linux-x86_64-relwithdebinfo target: cloud/filestore/ (test time: 7024s): all tests PASSED for commit 0ea29bb.
|
1 participant
Notes
Configure periodic certificates update for filestore
Issue
#5722