-
Notifications
You must be signed in to change notification settings - Fork 100
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
fix: Fix reload errors due long matching conditions #1829
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1829 +/- ##
==========================================
- Coverage 86.20% 86.12% -0.09%
==========================================
Files 83 83
Lines 5540 5586 +46
Branches 52 50 -2
==========================================
+ Hits 4776 4811 +35
- Misses 715 726 +11
Partials 49 49 ☔ View full report in Codecov by Sentry. |
62958e7
to
5abcc8a
Compare
45d073f
to
8df5af2
Compare
Please rebase and ensure the pipeline runs. I'm assuming you tested with the large example from the issue, not just the advanced routing example? |
Co-authored-by: Saylor Berman <[email protected]>
Co-authored-by: Saylor Berman <[email protected]>
d57350d
to
81b5655
Compare
Co-authored-by: Michael Pleshakov <[email protected]>
Co-authored-by: Michael Pleshakov <[email protected]>
Co-authored-by: Ciara Stacke <[email protected]>
Co-authored-by: Ciara Stacke <[email protected]>
Co-authored-by: Ciara Stacke <[email protected]>
Co-authored-by: Ciara Stacke <[email protected]>
Co-authored-by: Ciara Stacke <[email protected]>
yes have addressed those ! |
Proposed changes
Problem: A large number of match conditions for a single hostname/path cause reload errors with NGINX about being parameter being too long
Solution: The solution is to use
js_preload_objects
to store thehttp_matches
and then parse them through NGINX using NJS to map the correct match condition.Changes:
match.json
to/etc/nginx/conf.d
http_matches
as key-value pairs inmatch.json
/
,.
, and replace=
withEXACT
. Reasons being NJS throws variable errors if the key contains/
,.
and having a=
confuses NGINX separating the key into two words.Testing: Manual testing
Testing with long matching conditions httproute
Closes #1107
Checklist
Before creating a PR, run through this checklist and mark each as complete.
Release notes
If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.