Skip to content
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

proxy config: support path regex in allowed requests #2731

Merged
merged 6 commits into from
Jul 19, 2023

Conversation

scarlettperry
Copy link
Contributor

@scarlettperry scarlettperry commented Jul 17, 2023

Description

currently we perform isallowedRequest on each proxy API request to confirm the request path exactly matches an allowed request path in the proxy config. this set up works for paths with query parameters (i.e. /car?brand=foo) but not path parameters (ie /car/{brand}).

PR adds support in the proxy config to specify either an exact path or a path regex. If it's the latter, in isallowedRequest we check if the request path string matches the path regex.

Note: the updates to the config are backwards compatible and won't need to modify existing use cases of path.

Testing Performed

manually
unit tests

@scarlettperry scarlettperry changed the title proxy API: support path params proxy API: support regexes in path Jul 17, 2023
@scarlettperry scarlettperry changed the title proxy API: support regexes in path proxy config: support path regex in allowed requests Jul 18, 2023
Comment on lines 42 to 44
case *proxyv1cfg.AllowRequest_Path:
// For exact path type, validate that each services constructs a parsable URL
_, err := url.Parse(fmt.Sprintf("%s%s", service.Host, t.Path))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

decided to just do this validation when it's an exact path type as a path regex might contain characters like ^ which would fail url.Parse

@scarlettperry scarlettperry marked this pull request as ready for review July 18, 2023 15:30
@scarlettperry scarlettperry requested a review from a team as a code owner July 18, 2023 15:30
backend/module/proxy/proxy.go Show resolved Hide resolved
backend/module/proxy/proxy.go Outdated Show resolved Hide resolved
Copy link
Contributor

@mikecutalo mikecutalo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@scarlettperry scarlettperry merged commit a97fa67 into main Jul 19, 2023
@scarlettperry scarlettperry deleted the proxy-dynamic-route-support branch July 19, 2023 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants