Skip to content

Commit f180b58

Browse files
fix: same issue exists in grpcroute
Signed-off-by: Kostis Kapelonis <[email protected]>
1 parent a1d46cc commit f180b58

File tree

2 files changed

+115
-0
lines changed

2 files changed

+115
-0
lines changed

pkg/plugin/grpcroute.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ func (r *RpcPlugin) setGRPCHeaderRoute(rollout *v1alpha1.Rollout, headerRouting
130130
}
131131
grpcHeaderRouteRule := gatewayv1.GRPCRouteRule{
132132
Matches: []gatewayv1.GRPCRouteMatch{},
133+
Filters: []gatewayv1.GRPCRouteFilter{},
133134
BackendRefs: []gatewayv1.GRPCBackendRef{
134135
{
135136
BackendRef: gatewayv1.BackendRef{
@@ -143,6 +144,13 @@ func (r *RpcPlugin) setGRPCHeaderRoute(rollout *v1alpha1.Rollout, headerRouting
143144
},
144145
},
145146
}
147+
148+
// Copy filters from original route
149+
if grpcRouteRule.Filters != nil {
150+
for i := 0; i < len(grpcRouteRule.Filters); i++ {
151+
grpcHeaderRouteRule.Filters = append(grpcHeaderRouteRule.Filters, *grpcRouteRule.Filters[i].DeepCopy())
152+
}
153+
}
146154
matchLength := len(grpcRouteRule.Matches)
147155
if matchLength == 0 {
148156
grpcHeaderRouteRule.Matches = []gatewayv1.GRPCRouteMatch{

pkg/plugin/plugin_test.go

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,113 @@ func TestRunSuccessfully(t *testing.T) {
222222
assert.Equal(t, prefixedHeaderValue, rpcPluginImp.UpdatedGRPCRouteMock.Spec.Rules[1].Matches[0].Headers[0].Value)
223223
assert.Equal(t, headerValueType, *rpcPluginImp.UpdatedGRPCRouteMock.Spec.Rules[1].Matches[0].Headers[0].Type)
224224
})
225+
t.Run("SetGRPCHeaderRouteWithFilters", func(t *testing.T) {
226+
// Create a GRPCRoute mock with filters
227+
grpcRouteWithFilters := mocks.GRPCRouteObj
228+
grpcRouteWithFilters.Spec.Rules[0].Filters = []gatewayv1.GRPCRouteFilter{
229+
{
230+
Type: gatewayv1.GRPCRouteFilterRequestHeaderModifier,
231+
RequestHeaderModifier: &gatewayv1.HTTPHeaderFilter{
232+
Add: []gatewayv1.HTTPHeader{
233+
{
234+
Name: "X-Custom-Header",
235+
Value: "custom-value",
236+
},
237+
},
238+
},
239+
},
240+
{
241+
Type: gatewayv1.GRPCRouteFilterResponseHeaderModifier,
242+
ResponseHeaderModifier: &gatewayv1.HTTPHeaderFilter{
243+
Set: []gatewayv1.HTTPHeader{
244+
{
245+
Name: "X-Response-Header",
246+
Value: "response-value",
247+
},
248+
},
249+
},
250+
},
251+
}
252+
253+
// Update the plugin's GRPCRouteClient with the new mock
254+
rpcPluginImp.GRPCRouteClient = gwFake.NewSimpleClientset(&grpcRouteWithFilters).GatewayV1().GRPCRoutes(mocks.RolloutNamespace)
255+
256+
headerName := "X-Test"
257+
headerValue := "test"
258+
headerMatch := v1alpha1.StringMatch{
259+
Prefix: headerValue,
260+
}
261+
headerRouting := v1alpha1.SetHeaderRoute{
262+
Name: mocks.ManagedRouteName,
263+
Match: []v1alpha1.HeaderRoutingMatch{
264+
{
265+
HeaderName: headerName,
266+
HeaderValue: &headerMatch,
267+
},
268+
},
269+
}
270+
rollout := newRollout(mocks.StableServiceName, mocks.CanaryServiceName, &GatewayAPITrafficRouting{
271+
Namespace: mocks.RolloutNamespace,
272+
GRPCRoute: mocks.GRPCRouteName,
273+
ConfigMap: mocks.ConfigMapName,
274+
})
275+
err := pluginInstance.SetHeaderRoute(rollout, &headerRouting)
276+
277+
assert.Empty(t, err.Error())
278+
// Verify that the new header route rule (index 1) has the same filters as the original route rule (index 0)
279+
originalFilters := grpcRouteWithFilters.Spec.Rules[0].Filters
280+
newRouteFilters := rpcPluginImp.UpdatedGRPCRouteMock.Spec.Rules[1].Filters
281+
282+
assert.Equal(t, len(originalFilters), len(newRouteFilters), "New route should have same number of filters as original")
283+
284+
// Verify first filter (RequestHeaderModifier)
285+
assert.Equal(t, originalFilters[0].Type, newRouteFilters[0].Type)
286+
assert.Equal(t, originalFilters[0].RequestHeaderModifier.Add[0].Name, newRouteFilters[0].RequestHeaderModifier.Add[0].Name)
287+
assert.Equal(t, originalFilters[0].RequestHeaderModifier.Add[0].Value, newRouteFilters[0].RequestHeaderModifier.Add[0].Value)
288+
289+
// Verify second filter (ResponseHeaderModifier)
290+
assert.Equal(t, originalFilters[1].Type, newRouteFilters[1].Type)
291+
assert.Equal(t, originalFilters[1].ResponseHeaderModifier.Set[0].Name, newRouteFilters[1].ResponseHeaderModifier.Set[0].Name)
292+
assert.Equal(t, originalFilters[1].ResponseHeaderModifier.Set[0].Value, newRouteFilters[1].ResponseHeaderModifier.Set[0].Value)
293+
})
294+
t.Run("SetGRPCHeaderRouteWithoutFilters", func(t *testing.T) {
295+
// Create a GRPCRoute mock without filters (using the original mock which has no filters)
296+
grpcRouteWithoutFilters := mocks.GRPCRouteObj
297+
grpcRouteWithoutFilters.Spec.Rules[0].Filters = nil // Explicitly set to nil
298+
299+
// Update the plugin's GRPCRouteClient with the mock without filters
300+
rpcPluginImp.GRPCRouteClient = gwFake.NewSimpleClientset(&grpcRouteWithoutFilters).GatewayV1().GRPCRoutes(mocks.RolloutNamespace)
301+
302+
headerName := "X-Test"
303+
headerValue := "test"
304+
headerMatch := v1alpha1.StringMatch{
305+
Prefix: headerValue,
306+
}
307+
headerRouting := v1alpha1.SetHeaderRoute{
308+
Name: mocks.ManagedRouteName,
309+
Match: []v1alpha1.HeaderRoutingMatch{
310+
{
311+
HeaderName: headerName,
312+
HeaderValue: &headerMatch,
313+
},
314+
},
315+
}
316+
rollout := newRollout(mocks.StableServiceName, mocks.CanaryServiceName, &GatewayAPITrafficRouting{
317+
Namespace: mocks.RolloutNamespace,
318+
GRPCRoute: mocks.GRPCRouteName,
319+
ConfigMap: mocks.ConfigMapName,
320+
})
321+
err := pluginInstance.SetHeaderRoute(rollout, &headerRouting)
322+
323+
assert.Empty(t, err.Error())
324+
// Verify that the new header route rule (index 1) has no filters, same as the original route rule (index 0)
325+
originalFilters := grpcRouteWithoutFilters.Spec.Rules[0].Filters
326+
newRouteFilters := rpcPluginImp.UpdatedGRPCRouteMock.Spec.Rules[1].Filters
327+
328+
assert.Nil(t, originalFilters, "Original route should have no filters")
329+
assert.Equal(t, len(originalFilters), len(newRouteFilters), "New route should have same number of filters as original (none)")
330+
assert.Empty(t, newRouteFilters, "New route should have no filters when original has none")
331+
})
225332
t.Run("SetHTTPHeaderRouteWithFilters", func(t *testing.T) {
226333
// Create an HTTPRoute mock with filters
227334
httpRouteWithFilters := mocks.HTTPRouteObj

0 commit comments

Comments
 (0)