handle multiple prefixes + tests

Signed-off-by: Steve Sloka <slokas@vmware.com>
projectcontour · Jan 27, 2021 · f0ed4bc · f0ed4bc
1 parent fa7ba36
commit f0ed4bc
Show file tree

Hide file tree

Showing 2 changed files with 178 additions and 13 deletions.
diff --git a/internal/dag/builder_test.go b/internal/dag/builder_test.go
@@ -33,7 +33,6 @@ import (
 )
 
 func TestDAGInsertServiceAPIs(t *testing.T) {
-
 	type testcase struct {
 		objs                         []interface{}
 		disablePermitInsecure        bool
@@ -103,6 +102,21 @@ func TestDAGInsertServiceAPIs(t *testing.T) {
 		},
 	}
 
+	svc2 := &v1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "blogsvc",
+			Namespace: "default",
+		},
+		Spec: v1.ServiceSpec{
+			Ports: []v1.ServicePort{{
+				Name:       "http",
+				Protocol:   "TCP",
+				Port:       80,
+				TargetPort: intstr.FromInt(8080),
+			}},
+		},
+	}
+
 	gateway := &serviceapis.Gateway{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "gateway",
@@ -124,7 +138,7 @@ func TestDAGInsertServiceAPIs(t *testing.T) {
 	}
 
 	// Simple example with a single host and a single service.
-	run(t, "insert basic", testcase{
+	run(t, "insert basic single route, single hostname", testcase{
 		objs: []interface{}{
 			gateway,
 			svc1,
@@ -165,6 +179,60 @@ func TestDAGInsertServiceAPIs(t *testing.T) {
 		),
 	})
 
+	run(t, "insert basic multiple routes, single hostname", testcase{
+		objs: []interface{}{
+			gateway,
+			svc1,
+			svc2,
+			&serviceapis.HTTPRoute{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "basic",
+					Namespace: "default",
+					Labels: map[string]string{
+						"app": "contour",
+					},
+				},
+				Spec: serviceapis.HTTPRouteSpec{
+					Hostnames: []serviceapis.Hostname{
+						"test.projectcontour.io",
+					},
+					Rules: []serviceapis.HTTPRouteRule{{
+						Matches: []serviceapis.HTTPRouteMatch{{
+							Path: serviceapis.HTTPPathMatch{
+								Type:  "Prefix",
+								Value: "/",
+							},
+						}},
+						ForwardTo: []serviceapis.HTTPRouteForwardTo{{
+							ServiceName: pointer.StringPtr("kuard"),
+							Port:        8080,
+						}},
+					}, {
+						Matches: []serviceapis.HTTPRouteMatch{{
+							Path: serviceapis.HTTPPathMatch{
+								Type:  "Prefix",
+								Value: "/blog",
+							},
+						}},
+						ForwardTo: []serviceapis.HTTPRouteForwardTo{{
+							ServiceName: pointer.StringPtr("blogsvc"),
+							Port:        80,
+						}},
+					}},
+				},
+			},
+		},
+		want: listeners(
+			&Listener{
+				Port: 80,
+				VirtualHosts: virtualhosts(
+					virtualhost("test.projectcontour.io",
+						prefixroute("/", service(svc1)), prefixroute("/blog", service(svc2))),
+				),
+			},
+		),
+	})
+
 	// Multiple hostnames should be added to each route.
 	run(t, "multiple hosts", testcase{
 		objs: []interface{}{
@@ -252,6 +320,93 @@ func TestDAGInsertServiceAPIs(t *testing.T) {
 			},
 		),
 	})
+
+	// If the ServiceName referenced from an HTTPRoute is missing,
+	// the route should not be added.
+	run(t, "missing service", testcase{
+		objs: []interface{}{
+			gateway,
+			&serviceapis.HTTPRoute{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "basic",
+					Namespace: "default",
+					Labels: map[string]string{
+						"app": "contour",
+					},
+				},
+				Spec: serviceapis.HTTPRouteSpec{
+					Rules: []serviceapis.HTTPRouteRule{{
+						Matches: []serviceapis.HTTPRouteMatch{{
+							Path: serviceapis.HTTPPathMatch{
+								Type:  "Prefix",
+								Value: "/",
+							},
+						}},
+						ForwardTo: []serviceapis.HTTPRouteForwardTo{{
+							ServiceName: pointer.StringPtr("kuard"),
+							Port:        8080,
+						}},
+					}},
+				},
+			},
+		},
+		want: listeners(),
+	})
+
+	// Single host with single route containing multiple prefixes to the same service.
+	run(t, "insert basic single route with multiple prefixes, single hostname", testcase{
+		objs: []interface{}{
+			gateway,
+			svc1,
+			&serviceapis.HTTPRoute{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "basic",
+					Namespace: "default",
+					Labels: map[string]string{
+						"app": "contour",
+					},
+				},
+				Spec: serviceapis.HTTPRouteSpec{
+					Hostnames: []serviceapis.Hostname{
+						"test.projectcontour.io",
+					},
+					Rules: []serviceapis.HTTPRouteRule{{
+						Matches: []serviceapis.HTTPRouteMatch{{
+							Path: serviceapis.HTTPPathMatch{
+								Type:  "Prefix",
+								Value: "/",
+							},
+						}, {
+							Path: serviceapis.HTTPPathMatch{
+								Type:  "Prefix",
+								Value: "/blog",
+							},
+						}, {
+							Path: serviceapis.HTTPPathMatch{
+								Type:  "Prefix",
+								Value: "/tech",
+							},
+						}},
+						ForwardTo: []serviceapis.HTTPRouteForwardTo{{
+							ServiceName: pointer.StringPtr("kuard"),
+							Port:        8080,
+						}},
+					}},
+				},
+			},
+		},
+		want: listeners(
+			&Listener{
+				Port: 80,
+				VirtualHosts: virtualhosts(
+					virtualhost("test.projectcontour.io",
+						prefixroute("/", service(svc1)),
+						prefixroute("/blog", service(svc1)),
+						prefixroute("/tech", service(svc1))),
+				),
+			},
+		),
+	})
 }
 
 func TestDAGInsert(t *testing.T) {

diff --git a/internal/dag/serviceapis_processor.go b/internal/dag/serviceapis_processor.go
@@ -76,43 +76,53 @@ func (p *ServiceAPIsProcessor) computeHTTPRoute(route *serviceapis.HTTPRoute) {
 
 		for _, forward := range rule.ForwardTo {
 			// Verify the service is valid
-			// TODO: ServiceName could be nil
+			if forward.ServiceName == nil {
+				p.Error("ServiceName must not specified and is currently only implemented!")
+				break
+			}
 			meta := types.NamespacedName{Name: *forward.ServiceName, Namespace: route.Namespace}
 
 			// TODO: Refactor EnsureService to take an int32 so conversion to intstr is not needed.
 			service, err := p.dag.EnsureService(meta, intstr.FromInt(int(forward.Port)), p.source)
 			if err != nil {
-				continue
+				// TODO: Raise `ResolvedRefs` condition on Gateway with `DegradedRoutes` reason.
+				return
 			}
 			services = append(services, service)
 		}
 
-		r := p.route(pathPrefixes, services)
+		routes := p.route(pathPrefixes, services)
 
 		for _, vhost := range hosts {
 			vhost := p.dag.EnsureVirtualHost(vhost)
-			vhost.addRoute(r)
+			for _, route := range routes {
+				vhost.addRoute(route)
+			}
 		}
 	}
 }
 
-// route builds a dag.Route for the supplied HTTPRoute.
-func (p *ServiceAPIsProcessor) route(pathPrefixes []string, services []*Service) *Route {
+// route builds a []*dag.Route for the supplied HTTPRoute.
+func (p *ServiceAPIsProcessor) route(pathPrefixes []string, services []*Service) []*Route {
 	var clusters []*Cluster
+	var routes []*Route
 
 	for _, service := range services {
 		clusters = append(clusters, &Cluster{
 			Upstream: service,
 			Protocol: service.Protocol,
 		})
 	}
-	r := &Route{
-		Clusters: clusters,
+
+	for _, prefix := range pathPrefixes {
+		r := &Route{
+			Clusters: clusters,
+		}
+		r.PathMatchCondition = &PrefixMatchCondition{Prefix: prefix}
+		routes = append(routes, r)
 	}
 
-	// TODO: Allow dag.Route.pathPrefixes to be an array.
-	r.PathMatchCondition = &PrefixMatchCondition{Prefix: pathPrefixes[0]}
-	return r
+	return routes
 }
 
 // httpRoutes returns a slice of *serviceapis.HTTPRoute objects.