-
Notifications
You must be signed in to change notification settings - Fork 70
feat: watch file update on executor config #2278
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2278 +/- ##
==========================================
- Coverage 74.09% 74.07% -0.03%
==========================================
Files 121 121
Lines 6737 6789 +52
==========================================
+ Hits 4992 5029 +37
- Misses 1542 1553 +11
- Partials 203 207 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
e8bf320
to
6984230
Compare
Signed-off-by: Binbin Li <[email protected]>
There was a problem hiding this 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 implements dynamic configuration updates for the executor by adding file watcher support for both TLS certificate files and executor configuration. The changes include updating the TLS certificate watcher to use atomic pointers for improved performance and thread safety, refactoring the server structure to use a getExecutor callback for always retrieving the latest executor, and integrating a config watcher in the server startup process. Additionally, some deployment and configuration adjustments were made for security and provider updates.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
internal/httpserver/tlssecret/certwatcher_test.go | Updated tests to use NewWatcher and atomic pointer access for TLS secrets. |
internal/httpserver/tlssecret/certwatcher.go | Refactored to replace RWMutex with atomic pointers and added logging for fsnotify events. |
internal/httpserver/server.go | Replaced static executor field with a getExecutor callback and integrated a config watcher. |
internal/httpserver/handlers_test.go & handlers.go | Updated executor API calls to reference the new getExecutor callback. |
internal/httpserver/config/config_test.go & config.go | Introduced a new Watcher for executor configuration and updated related tests. |
deployments/ratify-gatekeeper-provider/templates/deployment.yaml | Changed readOnlyRootFilesystem to true for enhanced security. |
configs/config.json | Updated registry configuration settings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
What this PR does / why we need it:
server
struct by replacingexecutor
field with agetExecutor
function callback so thatserver
could always get the latest executor.TLSCertWatcher
by replacing theRWMutex
withatomic.Pointer
to improve the performance while maintaining the thread safety.Which issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when the PR gets merged):Fixes #
Type of change
Please delete options that are not relevant.
main
branch)How Has This Been Tested?
Added unit test.
Checklist:
Post Merge Requirements
Helm Chart Change