feat: sync the served environment set from the proxy config - #18
feat: sync the served environment set from the proxy config#18gagantrivedi wants to merge 1 commit into
Conversation
d49bc46 to
134525a
Compare
|
@CodeRabbit review |
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe change adds an optional validated Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🟠 High · up to The change can allow expired environment keys to access cached documents, flags, or identity data, while overlapping refreshes may restore removed environments or stale keys. These security and correctness risks should be fixed before merging. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@CodeRabbit review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7a4fca1a-71b2-460e-9ff3-9c61e10aa615
📒 Files selected for processing (7)
src/config/settings.rssrc/environments.rssrc/main.rssrc/models/mod.rssrc/models/proxy_config.rssrc/services/environment.rstests/test_proxy_config_discovery.rs
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
matthewelwell
left a comment
matthewelwell
left a comment
There was a problem hiding this comment.
Full disclosure: I haven't reviewed the tests
| self.cache.remove_environment(&keys.client_key).await; | ||
| self.clear_endpoint_caches(&keys).await; | ||
| self.purge_environment_caches(&keys).await; |
There was a problem hiding this comment.
I guess this question is valid for the previous code here, but I don't understand the 'purge' here - shouldn't we rebuild instead?
There was a problem hiding this comment.
Also, if it simplifies it, I think we should just deprecate the endpoint caches - they are not worth keeping imo.
There was a problem hiding this comment.
gone with the endpoint cache
fbdd769 to
21dc629
Compare
With a proxy_key configured, the proxy keeps its served environments in sync with GET /api/v1/proxy/config/: new environments are served within one poll interval, rotations and removals propagate, and per-environment configuration is no longer needed. - proxy_key setting (one key; organisation-level grants make it multi-org) - models/proxy_config.rs: serde DTOs for the fields acted on - EnvironmentIndex::sync_to(desired): insert new/changed, remove absent; statically configured environments are never overridden or removed - sync runs at the top of each poll pass; a failed fetch removes nothing - environments with no usable server key are skipped, and a deactivated or expired server key stops authenticating on the next request
90f27dd to
43c1d37
Compare
2 participants
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Contributes to Flagsmith/edge-proxy#128
Stacked on #17. With a
proxy_keyconfigured, the proxy keeps its served environments in sync with the proxy config endpoint (GET /api/v1/proxy/config/,X-Proxy-Keyheader): a newly created environment is served within one poll interval, rotations and removals propagate automatically, and per-environment configuration is no longer needed.proxy_keysetting (a single key — organisation-level grants make one key multi-org).models/proxy_config.rs: serde DTOs declaring only the fields acted on; everything else the endpoint sends is ignored.EnvironmentIndex::sync_to(desired): inserts new/changed environments, removes absent ones — statically configured environments (the index remembers their keys) are never overridden, removed, or key-hijacked by the config.How did you test this code?
cargo test: 64 green — reconcile unit tests plustests/test_proxy_config_discovery.rs, wiremock against the frozen contract shape: environment added / dropped / key-rotated, static-never-removed, and fetch-failure-removes-nothing. Clippy and fmt clean.