You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat: Exclude verified Edge Proxy requests from API usage tracking - #8407
I have added information to docs/ if required so people know about the feature.
I have filled in the "Changes" section below.
I have filled in the "How did you test this code" section below.
Changes
Contributes to Flagsmith/flagsmith-private#256
An Edge Proxy reports the requests it serves through its own usage endpoint, so counting its /environment-document/ fetches here would double-count.
APIUsageMiddleware skips a request when the private edge_control_plane app's is_valid_edge_proxy_request() verifies it as the proxy's own: a valid X-Proxy-Key whose grants cover the presented environment.
Bare header presence is never trusted, so the header can't be spoofed to dodge metering.
Wired only on non-SaaS deployments with edge_control_plane installed; SaaS is untouched.
How did you test this code?
Middleware unit tests: verifier wired only when installed and non-SaaS; verified requests untracked, unverified tracked; a proxy header with no verifier still tracked. The verifier itself is tested in flagsmith-private.
APIUsageMiddleware conditionally wires is_valid_edge_proxy_request when the Edge Control Plane app is installed and the deployment is not SaaS. It skips usage tracking for a request with an X-Environment-Key header only when the validator confirms that the request is valid. Tests cover verifier wiring and tracking behaviour.
Merge/deploy order: #282 and 8407 (with the flagsmith-private pin bump) before any proxy build with #19 is deployed, else document polls double-count during the overlap.
An Edge Proxy sends X-Proxy-Key on its environment-document fetches
and reports the requests it serves through its own usage endpoint, so
counting its fetches here would double-count. The private edge_proxy
app's is_edge_proxy_request() decides the exemption — it verifies the
key and that its grants cover the presented environment, so a spoofed
header never dodges metering. Wired only on non-SaaS deployments with
the edge_proxy app installed; requires the flagsmith-private release
that ships is_edge_proxy_request (bump the lock before merge).
Core's lock pins a flagsmith-private wheel that does not ship the
helper yet, so a hard import broke every request in CI. Feature-detect
instead: on an older wheel the proxy's fetches are simply counted as
before, and the exclusion activates when the pin catches up.
184-210: Trivial · Maintainability & Code Quality · Low value
Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.
Test the verifier through the real wiring path.
This test sets settings.EDGE_CONTROL_PLANE_INSTALLED = False and then assigns middleware.is_valid_edge_proxy_request by hand. It checks the branch in __call__, but not that __init__ wires the verifier. A separate test covers the wiring, so the coverage is acceptable.
The settings.EDGE_CONTROL_PLANE_INSTALLED = False assignment on Line 195 duplicates the autouse fixture. Remove it to reduce noise. The same applies to Line 222.
Review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: b5ee4f57-0e49-4094-83c0-2a9e76543882
Commits
Reviewing files that changed from the base of the PR and between b0a2a51 and 9fd86fb.
Files selected for processing: 2
api/app_analytics/middleware.py
api/tests/unit/app_analytics/test_middleware.py
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
24 screenshots compared. See report for details. View full report
This branch has not been deployed
No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
apiIssue related to the REST APIfeatureNew feature or request
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
github-actions
Bot
added
feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Contributes to Flagsmith/flagsmith-private#256
An Edge Proxy reports the requests it serves through its own usage endpoint, so counting its
/environment-document/fetches here would double-count.APIUsageMiddlewareskips a request when the privateedge_control_planeapp'sis_valid_edge_proxy_request()verifies it as the proxy's own: a validX-Proxy-Keywhose grants cover the presented environment.edge_control_planeinstalled; SaaS is untouched.How did you test this code?
Middleware unit tests: verifier wired only when installed and non-SaaS; verified requests untracked, unverified tracked; a proxy header with no verifier still tracked. The verifier itself is tested in flagsmith-private.