uri_template: Allow uri_template variables in literal expression
Commit Message: Allow uri_template variables in literal expression Additional Description: UriTemplateMatch now allows for variables to be defined in a literal. Before this change, all variables had to be clamped by path separators (/).
This is not according RFC6570.
Even though the spec is not implemented by Envoy (*, ** are not a glob in the file system sense), it is good to have more feature parity with URI Templates.
The change is to treat path separators as literals. The restriction can then be easily lifted to allow variables inside literals.
2 checks have been rewritten, double slashes are not allowed, /abc//d and /abc/{var=/d}, and that a PathGlob can only exist between two path separators.
Risk Level: Low
Testing: Tests added and adjusted where necessary
Docs Changes:
Release Notes:
Optional Fixes #Issue: #34507
@yanavlasov friendly ping.
I've asked the original author @silverstar194 to review as well, as I do not fully understand the consequences of this change.
/wait-any
Hi @yanavlasov @silverstar194 is there anything I can do to move this PR forward? Let me know!
This changes some validation logic which we don't know whether is currently safe or not in production with existing configs, so can you add a knob to the code to have the option to preserve the current behavior? Ideally in a way that keeping the knob around won't lead to duplicated code.
Also please add test cases for the things changed.
Thanks for the review! I'll need some time to process and work on. Do you have a similar set-up in Envoy for this 'preserve' current behavior? It sounds like a feature-flag. Any hints would be appreciated.
#34974 is an example of using feature-flag for a breaking change.
/wait-any
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!
I still need to go through the pathrewrite implications. With holidays ongoing it's slower than expected. I'll mark this as draft, or when autoclose kicks in ask for reopening when done.
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!
This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!