wip. server tests need fixing

Signed-off-by: Amit Mor <[email protected]>
amimimor · Apr 3, 2024 · e1e9762 · e1e9762
1 parent c1da8a3
commit e1e9762
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 27 deletions.
diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go
@@ -99,14 +99,6 @@ type rewriteConfig struct {
 	Rewrite string
 }
 
-// mirrorConfig contains the configuration for a location to mirror requests,
-// as specified in a Path filter
-type mirrorConfig struct {
-	// Path mirrors the request to the specified location
-	Path string
-	Host string
-}
-
 func createLocations(pathRules []dataplane.PathRule, listenerPort int32) []http.Location {
 	maxLocs, pathsAndTypes := getMaxLocationCountAndPathMap(pathRules)
 	locs := make([]http.Location, 0, maxLocs)
@@ -120,8 +112,6 @@ func createLocations(pathRules []dataplane.PathRule, listenerPort int32) []http.
 		}
 
 		extLocations := initializeExternalLocations(rule, pathsAndTypes)
-		//internal/mode/static/nginx/config/servers.go
-		//internal/mode/static/state/graph/httproute.go
 		for matchRuleIdx, r := range rule.MatchRules {
 			buildLocations := extLocations
 			if len(rule.MatchRules) != 1 || !isPathOnlyMatch(r.Match) {
@@ -254,6 +244,12 @@ func updateLocationsForFilters(
 		return buildLocations
 	}
 
+	if filters.RequestMirror != nil {
+		for i := range buildLocations {
+			buildLocations[i].MirrorPath = *filters.RequestMirror.Target
+		}
+	}
+
 	rewrites := createRewritesValForRewriteFilter(filters.RequestURLRewrite, path)
 	proxySetHeaders := generateProxySetHeaders(&matchRule.Filters)
 	for i := range buildLocations {

diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go
@@ -36,6 +36,10 @@ server {
         {{- range $r := $l.Rewrites }}
         rewrite {{ $r }};
         {{- end }}
+		
+		{{- if $l.MirrorPath }}
+		mirror $l.MirrorPath;
+		{{- end }}
 
         {{- if $l.Return }}
         return {{ $l.Return.Code }} "{{ $l.Return.Body }}";

diff --git a/internal/mode/static/state/dataplane/convert.go b/internal/mode/static/state/dataplane/convert.go
@@ -77,25 +77,33 @@ func convertHTTPHeaderFilter(filter *v1.HTTPHeaderFilter) *HTTPHeaderFilter {
 }
 
 func convertHTTPRequestMirrorFilter(filter *v1.HTTPRequestMirrorFilter) *HTTPRequestMirrorFilter {
-	var backendRefNamePtr *string
+	result := &HTTPRequestMirrorFilter{}
+
 	filterBackendRef := filter.BackendRef
-	backendRefName := string(filterBackendRef.Name)
+
+	backendRefName := (string)(filterBackendRef.Name)
 	if len(backendRefName) > 0 {
-		backendRefNamePtr = &backendRefName
+		result.Name = helpers.GetPointer(backendRefName)
 	}
 
-	// this initialization is partial and must be completed with the path
-	result := &HTTPRequestMirrorFilter{
-		Name:      backendRefNamePtr,
-		Namespace: (*string)(filterBackendRef.Namespace),
-		Port:      (*int32)(filterBackendRef.Port),
+	backendRefNamespace := (*string)(filterBackendRef.Namespace)
+	if backendRefNamespace != nil && len(*backendRefNamespace) > 0 {
+		result.Namespace = backendRefNamespace
 	}
 
+	port := (*int32)(filterBackendRef.Port)
+	if port != nil && *port > 0 {
+		result.Port = port
+	}
+
+	// this initialization is partial and must be completed with the path
 	return result
 }
 
 func updateHTTPMirrorFilterRoute(path *v1.HTTPPathMatch, f *HTTPRequestMirrorFilter) {
-	f.Target = helpers.CreateMirrorBackendPath(path.Value, *f.Namespace, *f.Name)
+	if f.Namespace != nil && f.Name != nil {
+		f.Target = helpers.CreateMirrorBackendPath(path.Value, *f.Namespace, *f.Name)
+	}
 }
 
 func convertPathType(pathType v1.PathMatchType) PathType {

diff --git a/internal/mode/static/state/dataplane/convert_test.go b/internal/mode/static/state/dataplane/convert_test.go
@@ -296,22 +296,57 @@ func TestConvertHTTPMirrorFilter(t *testing.T) {
 			},
 			expected: &HTTPRequestMirrorFilter{
 				Namespace: helpers.GetPointer("backend"),
+				Port:      nil,
 			},
 			name: "WithoutPort",
 		},
 		{
 			filter: &v1.HTTPRequestMirrorFilter{
 				BackendRef: v1.BackendObjectReference{
-					Name: "backend",
-					Port: helpers.GetPointer[v1.PortNumber](8080),
+					Name:      "backend",
+					Namespace: helpers.GetPointer[v1.Namespace]("backend"),
+					Port:      helpers.GetPointer[v1.PortNumber](8080),
+				},
+			},
+			expected: &HTTPRequestMirrorFilter{
+				Name:      helpers.GetPointer("service"),
+				Namespace: helpers.GetPointer("backend"),
+				Port:      helpers.GetPointer[int32](8080),
+				Target:    helpers.GetPointer("/backend-service-mirror"),
+			},
+			name: "AllDataIntact",
+		},
+		{
+			filter: &v1.HTTPRequestMirrorFilter{
+				BackendRef: v1.BackendObjectReference{
+					Name:      "",
+					Namespace: helpers.GetPointer[v1.Namespace]("backend"),
+					Port:      helpers.GetPointer[v1.PortNumber](8080),
 				},
 			},
 			expected: &HTTPRequestMirrorFilter{
+				Name:      nil,
 				Namespace: helpers.GetPointer("backend"),
 				Port:      helpers.GetPointer[int32](8080),
-				Target:    helpers.GetPointer("backend"), // TODO:  test this
+				Target:    nil,
+			},
+			name: "MissingName",
+		},
+		{
+			filter: &v1.HTTPRequestMirrorFilter{
+				BackendRef: v1.BackendObjectReference{
+					Name:      "service",
+					Namespace: nil,
+					Port:      helpers.GetPointer[v1.PortNumber](8080),
+				},
+			},
+			expected: &HTTPRequestMirrorFilter{
+				Name:      helpers.GetPointer("service"),
+				Namespace: nil,
+				Port:      helpers.GetPointer[int32](8080),
+				Target:    helpers.GetPointer("/service-mirror"),
 			},
-			name: "WithNameAndPort",
+			name: "MissingNamespace",
 		},
 	}
 
@@ -320,6 +355,7 @@ func TestConvertHTTPMirrorFilter(t *testing.T) {
 			g := NewWithT(t)
 
 			result := convertHTTPRequestMirrorFilter(test.filter)
+			updateHTTPMirrorFilterRoute(&v1.HTTPPathMatch{Value: helpers.GetPointer("/")}, result)
 			g.Expect(result).To(Equal(test.expected))
 		})
 	}

diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go
@@ -158,9 +158,9 @@ type PathModifierType string
 type HTTPRequestMirrorFilter struct {
 	// Name is the service name
 	Name *string
-	// Namespace is the hostname of the redirect.
+	// Namespace is the hostname of the service.
 	Namespace *string
-	// Port is the port of the redirect.
+	// Port is the port of the service (TODO: might be redundant).
 	Port *int32
 	// Target is the target of the mirror (path with hostname and service name).
 	Target *string

diff --git a/internal/mode/static/state/graph/graph_test.go b/internal/mode/static/state/graph/graph_test.go
@@ -1179,8 +1179,8 @@ func TestBuildGraphWithMirror(t *testing.T) {
 				client.ObjectKeyFromObject(gw1): gw1,
 				client.ObjectKeyFromObject(gw2): gw2,
 			},
-			// hr1MirrorFilterCreated & hr3MirrorFilterCreated should be created implicitly, therefore they are not list
-			// here, but are added to the graph and the expected graph state
+			// hr1MirrorFilterCreated & hr3MirrorFilterCreated should be created implicitly, therefore they are not
+			// listed here, but they are expected to be present in the final graph
 			HTTPRoutes: map[types.NamespacedName]*gatewayv1.HTTPRoute{
 				client.ObjectKeyFromObject(hr1):             hr1,
 				client.ObjectKeyFromObject(hr1MirrorFilter): hr1MirrorFilter,