Review feedback

nginxinc · May 2, 2024 · 8a9a1ad · 8a9a1ad
1 parent 07cbf95
commit 8a9a1ad
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 6 deletions.
diff --git a/internal/mode/static/state/graph/grpcroute.go b/internal/mode/static/state/graph/grpcroute.go
@@ -86,11 +86,9 @@ func processGRPCRouteRules(
 			matchesErrs = append(matchesErrs, validateGRPCMatch(validator, match, matchPath)...)
 		}
 
-		if len(rule.Filters) > 0 {
-			for j, filter := range rule.Filters {
-				filterPath := rulePath.Child("filters").Index(j)
-				filtersErrs = append(filtersErrs, validateGRPCFilter(validator, filter, filterPath)...)
-			}
+		for j, filter := range rule.Filters {
+			filterPath := rulePath.Child("filters").Index(j)
+			filtersErrs = append(filtersErrs, validateGRPCFilter(validator, filter, filterPath)...)
 		}
 
 		backendRefs := make([]RouteBackendRef, 0, len(rule.BackendRefs))

diff --git a/internal/mode/static/state/graph/grpcroute_test.go b/internal/mode/static/state/graph/grpcroute_test.go
@@ -252,6 +252,15 @@ func TestBuildGRPCRoute(t *testing.T) {
 		[]v1alpha2.GRPCRouteRule{grValidFilterRule},
 	)
 
+	convertedFilters := []v1.HTTPRouteFilter{
+		{
+			Type: v1.HTTPRouteFilterRequestHeaderModifier,
+			RequestHeaderModifier: &v1.HTTPHeaderFilter{
+				Remove: []string{"header"},
+			},
+		},
+	}
+
 	createAllValidValidator := func() *validationfakes.FakeHTTPFieldsValidator {
 		v := &validationfakes.FakeHTTPFieldsValidator{}
 		v.ValidateMethodInMatchReturns(true, nil)
@@ -351,7 +360,7 @@ func TestBuildGRPCRoute(t *testing.T) {
 							ValidFilters:     true,
 							Matches:          convertGRPCMatches(grValidFilter.Spec.Rules[0].Matches),
 							RouteBackendRefs: []RouteBackendRef{},
-							Filters:          convertGRPCFilters(grValidFilter.Spec.Rules[0].Filters),
+							Filters:          convertedFilters,
 						},
 					},
 				},
@@ -630,3 +639,74 @@ func TestBuildGRPCRoute(t *testing.T) {
 		})
 	}
 }
+
+func TestConvertGRPCMatches(t *testing.T) {
+	methodMatch := createGRPCMethodMatch("myService", "myMethod", "Exact").Matches
+
+	headersMatch := createGRPCHeadersMatch("Exact", "MyHeader", "SomeValue").Matches
+
+	expectedHTTPMatches := []v1.HTTPRouteMatch{
+		{
+			Path: &v1.HTTPPathMatch{
+				Type:  helpers.GetPointer(v1.PathMatchExact),
+				Value: helpers.GetPointer("/myService/myMethod"),
+			},
+			Headers: []v1.HTTPHeaderMatch{},
+		},
+	}
+
+	expectedHeadersMatches := []v1.HTTPRouteMatch{
+		{
+			Path: &v1.HTTPPathMatch{
+				Type:  helpers.GetPointer(v1.PathMatchPathPrefix),
+				Value: helpers.GetPointer("/"),
+			},
+			Headers: []v1.HTTPHeaderMatch{
+				{
+					Value: "SomeValue",
+					Name:  v1.HTTPHeaderName("MyHeader"),
+				},
+			},
+		},
+	}
+
+	expectedEmptyMatches := []v1.HTTPRouteMatch{
+		{
+			Path: &v1.HTTPPathMatch{
+				Type:  helpers.GetPointer(v1.PathMatchPathPrefix),
+				Value: helpers.GetPointer("/"),
+			},
+		},
+	}
+
+	tests := []struct {
+		name          string
+		methodMatches []v1alpha2.GRPCRouteMatch
+		expected      []v1.HTTPRouteMatch
+	}{
+		{
+			name:          "exact match",
+			methodMatches: methodMatch,
+			expected:      expectedHTTPMatches,
+		},
+		{
+			name:          "headers matches",
+			methodMatches: headersMatch,
+			expected:      expectedHeadersMatches,
+		},
+		{
+			name:          "empty matches",
+			methodMatches: []v1alpha2.GRPCRouteMatch{},
+			expected:      expectedEmptyMatches,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			httpMatches := convertGRPCMatches(test.methodMatches)
+			g.Expect(helpers.Diff(test.expected, httpMatches)).To(BeEmpty())
+		})
+	}
+}