-
Notifications
You must be signed in to change notification settings - Fork 360
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: Multiple RequestMirrors Filters per HTTPRoute Rule #1819
Changes from 7 commits
e7a1f39
d903853
8ea547d
c70320e
c1b816c
219949f
ba0a590
94e9217
be5748d
d0ff46d
17eff36
e86e473
1b4c8a2
cb3254a
93cc046
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,7 @@ | |
processRedirectFilter(redirect *v1beta1.HTTPRequestRedirectFilter, filterContext *HTTPFiltersContext) | ||
processRequestHeaderModifierFilter(headerModifier *v1beta1.HTTPHeaderFilter, filterContext *HTTPFiltersContext) | ||
processResponseHeaderModifierFilter(headerModifier *v1beta1.HTTPHeaderFilter, filterContext *HTTPFiltersContext) | ||
processRequestMirrorFilter(mirror *v1beta1.HTTPRequestMirrorFilter, filterContext *HTTPFiltersContext, resources *Resources) | ||
processRequestMirrorFilter(filterIdx int, mirror *v1beta1.HTTPRequestMirrorFilter, filterContext *HTTPFiltersContext, resources *Resources) | ||
processExtensionRefHTTPFilter(extRef *v1beta1.LocalObjectReference, filterContext *HTTPFiltersContext, resources *Resources) | ||
processUnsupportedHTTPFilter(filterType string, filterContext *HTTPFiltersContext) | ||
} | ||
|
@@ -56,7 +56,7 @@ | |
AddResponseHeaders []ir.AddHeader | ||
RemoveResponseHeaders []string | ||
|
||
Mirror *ir.RouteDestination | ||
Mirror []*ir.RouteDestination | ||
|
||
RequestAuthentication *ir.RequestAuthentication | ||
RateLimit *ir.RateLimit | ||
|
@@ -76,7 +76,6 @@ | |
RuleIdx: ruleIdx, | ||
HTTPFilterIR: &HTTPFilterIR{}, | ||
} | ||
|
||
for i := range filters { | ||
filter := filters[i] | ||
// If an invalid filter type has been configured then skip processing any more filters | ||
|
@@ -98,7 +97,7 @@ | |
case v1beta1.HTTPRouteFilterResponseHeaderModifier: | ||
t.processResponseHeaderModifierFilter(filter.ResponseHeaderModifier, httpFiltersContext) | ||
case v1beta1.HTTPRouteFilterRequestMirror: | ||
t.processRequestMirrorFilter(filter.RequestMirror, httpFiltersContext, resources) | ||
t.processRequestMirrorFilter(i, filter.RequestMirror, httpFiltersContext, resources) | ||
case v1beta1.HTTPRouteFilterExtensionRef: | ||
t.processExtensionRefHTTPFilter(filter.ExtensionRef, httpFiltersContext, resources) | ||
default: | ||
|
@@ -120,7 +119,6 @@ | |
|
||
HTTPFilterIR: &HTTPFilterIR{}, | ||
} | ||
|
||
for i := range filters { | ||
filter := filters[i] | ||
// If an invalid filter type has been configured then skip processing any more filters | ||
|
@@ -138,7 +136,7 @@ | |
case v1alpha2.GRPCRouteFilterResponseHeaderModifier: | ||
t.processResponseHeaderModifierFilter(filter.ResponseHeaderModifier, httpFiltersContext) | ||
case v1alpha2.GRPCRouteFilterRequestMirror: | ||
t.processRequestMirrorFilter(filter.RequestMirror, httpFiltersContext, resources) | ||
t.processRequestMirrorFilter(i, filter.RequestMirror, httpFiltersContext, resources) | ||
case v1alpha2.GRPCRouteFilterExtensionRef: | ||
t.processExtensionRefHTTPFilter(filter.ExtensionRef, httpFiltersContext, resources) | ||
default: | ||
|
@@ -808,6 +806,7 @@ | |
} | ||
|
||
func (t *Translator) processRequestMirrorFilter( | ||
filterIdx int, | ||
mirrorFilter *v1beta1.HTTPRequestMirrorFilter, | ||
filterContext *HTTPFiltersContext, | ||
resources *Resources) { | ||
|
@@ -836,27 +835,13 @@ | |
|
||
mirrorEndpoints, _ := t.processDestEndpoints(mirrorBackendRef, filterContext.ParentRef, filterContext.Route, resources) | ||
|
||
// Only add missing mirror destinations | ||
for _, mirrorEp := range mirrorEndpoints { | ||
var found bool | ||
if filterContext.Mirror != nil { | ||
for _, mirror := range filterContext.Mirror.Endpoints { | ||
if mirror != nil { | ||
if mirror.Host == mirrorEp.Host && mirror.Port == mirrorEp.Port { | ||
found = true | ||
} | ||
} | ||
} | ||
} | ||
if !found { | ||
if filterContext.Mirror == nil { | ||
filterContext.Mirror = &ir.RouteDestination{ | ||
Name: fmt.Sprintf("%s-mirror", irRouteDestinationName(filterContext.Route, filterContext.RuleIdx)), | ||
} | ||
} | ||
filterContext.Mirror.Endpoints = append(filterContext.Mirror.Endpoints, mirrorEp) | ||
} | ||
newMirror := &ir.RouteDestination{ | ||
Name: fmt.Sprintf("%s-mirror-%d", irRouteDestinationName(filterContext.Route, filterContext.RuleIdx), filterIdx), | ||
} | ||
filterContext.Mirror = append(filterContext.Mirror, newMirror) | ||
// Get the index of the last mirror added | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we need L842-L844, can't we just assign |
||
lastMirrorIdx := len(filterContext.Mirror) - 1 | ||
filterContext.Mirror[lastMirrorIdx].Endpoints = append(filterContext.Mirror[lastMirrorIdx].Endpoints, mirrorEndpoints...) | ||
} | ||
|
||
func (t *Translator) processUnresolvedHTTPFilter(errMsg string, filterContext *HTTPFiltersContext) { | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -261,7 +261,7 @@ | |||||
// Redirections to be returned for this route. Takes precedence over Destinations. | ||||||
Redirect *Redirect `json:"redirect,omitempty" yaml:"redirect,omitempty"` | ||||||
// Destination that requests to this HTTPRoute will be mirrored to | ||||||
Mirror *RouteDestination `json:"mirror,omitempty" yaml:"mirror,omitempty"` | ||||||
Mirror []*RouteDestination `json:"mirrors,omitempty" yaml:"mirrors,omitempty"` | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
// Destination associated with this matched route. | ||||||
Destination *RouteDestination `json:"destination,omitempty" yaml:"destination,omitempty"` | ||||||
// Rewrite to be changed for this route. | ||||||
|
@@ -352,8 +352,10 @@ | |||||
} | ||||||
} | ||||||
if h.Mirror != nil { | ||||||
if err := h.Mirror.Validate(); err != nil { | ||||||
errs = multierror.Append(errs, err) | ||||||
for _, mirror := range h.Mirror { | ||||||
if err := mirror.Validate(); err != nil { | ||||||
errs = multierror.Append(errs, err) | ||||||
} | ||||||
} | ||||||
} | ||||||
if len(h.AddRequestHeaders) > 0 { | ||||||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,10 @@ http: | |
endpoints: | ||
- host: "1.2.3.4" | ||
port: 50000 | ||
mirror: | ||
name: "mirror-route-dest" | ||
mirrors: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lets keep this example as is, and create one more test file for multiple-mirrors ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sound good, will do. |
||
- name: "mirror-route-dest" | ||
endpoints: | ||
- host: "2.3.4.5" | ||
- name: "mirror-route-dest1" | ||
endpoints: | ||
- host: "3.4.5.6" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,3 +12,4 @@ | |
cluster: route-dest | ||
requestMirrorPolicies: | ||
- cluster: mirror-route-dest | ||
- cluster: mirror-route-dest1 |
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.
why not just use i
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.
Yes, I can use 'i' if I don't need to match the value with the index of Mirror list.