-
Notifications
You must be signed in to change notification settings - Fork 349
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
feat(translator): implement httproutefilter and path regex rewrite #4258
Conversation
a238a26
to
11acbb1
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4258 +/- ##
==========================================
- Coverage 65.75% 65.74% -0.01%
==========================================
Files 197 197
Lines 23565 23763 +198
==========================================
+ Hits 15494 15623 +129
- Misses 6967 7020 +53
- Partials 1104 1120 +16 ☔ View full report in Codecov by Sentry. |
2a9fe55
to
3c5c126
Compare
Signed-off-by: Guy Daich <[email protected]>
3c5c126
to
657bbf0
Compare
for _, rule := range httproute.Spec.Rules { | ||
for _, filter := range rule.Filters { | ||
if filter.ExtensionRef != nil && string(filter.ExtensionRef.Kind) == resource.KindHTTPRouteFilter { | ||
// If an explicit Backend namespace is not provided, use the HTTPRoute namespace to |
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.
Nit: remove this comment. Cross-namespace filter reference is not supported.
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.
This PR looks great! I just have a nit on the comment, which is non-blocking.
internal/ir/xds.go
Outdated
type ExtendedHTTPPathModifier struct { | ||
HTTPPathModifier `json:",inline" yaml:",inline"` | ||
// RegexMatchReplace provides a regex to match an a replacement to perform on the path. | ||
RegexMatchReplace *RegexMatchReplace `json:"regexMatchReplace" yaml:"regexMatchReplace"` |
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.
omitempty
one nit around tag, else LGTM, thanks for adding this ! |
Signed-off-by: Guy Daich <[email protected]>
What type of PR is this?
What this PR does / why we need it:
HTTPRouteFilter
Which issue(s) this PR fixes:
Fixes #4120, #1794