-
Notifications
You must be signed in to change notification settings - Fork 5
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
[NOREF] add support for queryParams and scheme #354
base: main
Are you sure you want to change the base?
Conversation
// The rule will not match if only a subsequence of the string matches the regex. | ||
// https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routematch-safe-regex | ||
r, err := regexp.Compile("^" + sm.GetRegex() + "$") | ||
if err != nil { | ||
return false, fmt.Errorf("could not compile regex %s: %v", sm.GetRegex(), err) | ||
} | ||
return r.MatchString(s), nil | ||
default: | ||
return false, fmt.Errorf("unknown matcher type %T", sm.GetMatchType()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returning an error here is a behaviour change - could also be a log
URI string | ||
Headers map[string]string | ||
Scheme string | ||
QueryParams map[string]string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a separate field for the parameters or can we use the uri
and parse from it?
Also, the same parameter can be set multiple times and interpreted differently by web frameworks as there is no apparent standard, this would be another reason not to use map[string]string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed will keep as is for now - the envoy behaviour is to check the first query param
No description provided.