dag: configure 502 if include references another (#5016) (#5157)

Set 502 response if include references another root Fixes #5016 Signed-off-by: Lan Liang <gcslyp@gmail.com>
projectcontour · Apr 13, 2023 · 98f876a · 98f876a
1 parent c813560
commit 98f876a
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 9 deletions.
diff --git a/changelogs/unreleased/5157-liangyuanpeng-small.md b/changelogs/unreleased/5157-liangyuanpeng-small.md
@@ -0,0 +1 @@
+Set 502 response if include references another root.
diff --git a/internal/dag/builder_test.go b/internal/dag/builder_test.go
@@ -10722,6 +10722,53 @@ func TestDAGInsert(t *testing.T) {
 				},
 			),
 		},
+		"insert httproxy with include references another root": {
+			objs: []interface{}{
+				&contour_api_v1.HTTPProxy{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "default",
+						Name:      "example-com",
+					},
+					Spec: contour_api_v1.HTTPProxySpec{
+						VirtualHost: &contour_api_v1.VirtualHost{
+							Fqdn: "example.com",
+						},
+						Includes: []contour_api_v1.Include{{
+							Conditions: []contour_api_v1.MatchCondition{{
+								Prefix: "/finance",
+							}},
+							Name:      "other-root",
+							Namespace: "default",
+						}},
+					},
+				},
+				&contour_api_v1.HTTPProxy{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "other-root",
+						Namespace: "default",
+					},
+					Spec: contour_api_v1.HTTPProxySpec{
+						VirtualHost: &contour_api_v1.VirtualHost{
+							Fqdn: "example2.com",
+						},
+					},
+				},
+			},
+			want: listeners(
+				&Listener{
+					Name: HTTP_LISTENER_NAME,
+					Port: 8080,
+					VirtualHosts: virtualhosts(
+						virtualhost("example.com", &Route{
+							PathMatchCondition: prefixString("/finance"),
+							DirectResponse: &DirectResponse{
+								StatusCode: http.StatusBadGateway,
+							},
+						}),
+					),
+				},
+			),
+		},
 		"insert httproxy w/ conditions": {
 			objs: []interface{}{
 				proxy1c, s1,

diff --git a/internal/dag/httpproxy_processor.go b/internal/dag/httpproxy_processor.go
@@ -553,6 +553,18 @@ func addRoutes(vhost vhost, routes []*Route) {
 	}
 }
 
+func addStatusBadGatewayRoute(routes []*Route, conds []contour_api_v1.MatchCondition) []*Route {
+	if len(conds) > 0 {
+		routes = append(routes, &Route{
+			PathMatchCondition:        mergePathMatchConditions(conds),
+			HeaderMatchConditions:     mergeHeaderMatchConditions(conds),
+			QueryParamMatchConditions: mergeQueryParamMatchConditions(conds),
+			DirectResponse:            directResponse(http.StatusBadGateway, ""),
+		})
+	}
+	return routes
+}
+
 func (p *HTTPProxyProcessor) computeRoutes(
 	validCond *contour_api_v1.DetailedCondition,
 	rootProxy *contour_api_v1.HTTPProxy,
@@ -618,21 +630,15 @@ func (p *HTTPProxyProcessor) computeRoutes(
 				"include %s/%s not found", namespace, include.Name)
 
 			// Set 502 response when include was not found but include condition was valid.
-			if len(include.Conditions) > 0 {
-				routes = append(routes, &Route{
-					PathMatchCondition:        mergePathMatchConditions(include.Conditions),
-					HeaderMatchConditions:     mergeHeaderMatchConditions(include.Conditions),
-					QueryParamMatchConditions: mergeQueryParamMatchConditions(include.Conditions),
-					DirectResponse:            directResponse(http.StatusBadGateway, ""),
-				})
-			}
-
+			routes = addStatusBadGatewayRoute(routes, include.Conditions)
 			continue
 		}
 
 		if includedProxy.Spec.VirtualHost != nil {
 			validCond.AddErrorf(contour_api_v1.ConditionTypeIncludeError, "RootIncludesRoot",
 				"root httpproxy cannot include another root httpproxy")
+			// Set 502 response if include references another root
+			routes = addStatusBadGatewayRoute(routes, include.Conditions)
 			continue
 		}