internal/envoy/v3: RouteConfiguration ignores port when choosing virtualhost

changelogs/unreleased/5437-sunjayBhatia-minor.md

@@ -0,0 +1,5 @@
+## Host header including port is passed through unmodified to backend
+
+Previously Contour would strip any port from the Host header in a downstream request for convenience in routing.
+This resulted in backends not receiving the Host header with a port.
+We no longer do this, for conformance with Gateway API (this change also applies to HTTPProxy and Ingress configuration).

internal/dag/builder_test.go

@@ -10359,7 +10359,7 @@ func TestDAGInsert(t *testing.T) {
 							&Route{
 								PathMatchCondition: prefixString("/"),
 								HeaderMatchConditions: []HeaderMatchCondition{
-									{Name: ":authority", Value: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.projectcontour\\.io", MatchType: "regex", Invert: false},
+									{Name: ":authority", Value: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.projectcontour\\.io(:[0-9]+)?", MatchType: "regex", Invert: false},
 								},
 								Clusters: clusters(service(s1)),
 							}),
@@ -10780,7 +10780,7 @@ func TestDAGInsert(t *testing.T) {
 								{
 									Name:      ":authority",
 									MatchType: HeaderMatchTypeRegex,
-									Value:     "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.example\\.com",
+									Value:     "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.example\\.com(:[0-9]+)?",
 								},
 							},
 							Clusters: clusters(service(s1)),

internal/dag/dag.go

@@ -1170,14 +1170,22 @@ type ExtensionCluster struct {
 	ClientCertificate *Secret
 }
 
+const singleDNSLabelWildcardRegex = "^[a-z0-9]([-a-z0-9]*[a-z0-9])?"
+
+var _ = regexp.MustCompile(singleDNSLabelWildcardRegex)
+
+const ignorePortRegex = "(:[0-9]+)?"
+
+var _ = regexp.MustCompile(ignorePortRegex)
+
 func wildcardDomainHeaderMatch(fqdn string) HeaderMatchCondition {
 	return HeaderMatchCondition{
 		// Internally Envoy uses the HTTP/2 ":authority" header in
 		// place of the HTTP/1 "host" header.
 		// See: https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#config-route-v3-headermatcher
 		Name:      ":authority",
 		MatchType: HeaderMatchTypeRegex,
-		Value:     singleDNSLabelWildcardRegex + regexp.QuoteMeta(fqdn[1:]),
+		Value:     singleDNSLabelWildcardRegex + regexp.QuoteMeta(fqdn[1:]) + ignorePortRegex,
 	}
 }
 

internal/dag/ingress_processor.go

@@ -14,7 +14,6 @@
 package dag
 
 import (
-	"regexp"
 	"strconv"
 	"strings"
 	"time"
@@ -203,10 +202,6 @@ func (p *IngressProcessor) computeIngressRule(ing *networking_v1.Ingress, rule n
 	}
 }
 
-const singleDNSLabelWildcardRegex = "^[a-z0-9]([-a-z0-9]*[a-z0-9])?"
-
-var _ = regexp.MustCompile(singleDNSLabelWildcardRegex)
-
 // route builds a dag.Route for the supplied Ingress.
 func (p *IngressProcessor) route(ingress *networking_v1.Ingress, host string, path string, pathType networking_v1.PathType, service *Service, clientCertSecret *Secret, serviceName string, servicePort int32, log logrus.FieldLogger) (*Route, error) {
 	log = log.WithFields(logrus.Fields{

internal/envoy/v3/listener.go

@@ -474,14 +474,6 @@ func (b *httpConnectionManagerBuilder) Get() *envoy_listener_v3.Filter {
 
 		NormalizePath: wrapperspb.Bool(true),
 
-		// We can ignore any port number supplied in the Host/:authority header
-		// before processing by filters or routing.
-		// Note that the port a listener is bound to will already be selected
-		// and that the port is stripped from the header sent upstream as well.
-		StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{
-			StripAnyHostPort: true,
-		},
-
 		// issue #1487 pass through X-Request-Id if provided.
 		PreserveExternalRequestId:  true,
 		MergeSlashes:               b.mergeSlashes,
@@ -713,6 +705,11 @@ function envoy_on_request(request_handle)
 	local host = string.lower(headers:get(":authority"))
 	local target = %s
 
+	s, e = string.find(host, ":", 1, true)
+	if s ~= nil then
+		host = string.sub(host, 1, s - 1)
+	end
+
 	if host ~= target then
 		request_handle:respond(
 			{[":status"] = "421"},

internal/envoy/v3/listener_test.go

@@ -655,9 +655,6 @@ func TestHTTPConnectionManager(t *testing.T) {
 						AccessLog:                 FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo),
 						UseRemoteAddress:          wrapperspb.Bool(true),
 						NormalizePath:             wrapperspb.Bool(true),
-						StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{
-							StripAnyHostPort: true,
-						},
 						PreserveExternalRequestId: true,
 						MergeSlashes:              false,
 					}),
@@ -705,9 +702,6 @@ func TestHTTPConnectionManager(t *testing.T) {
 						AccessLog:                 FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo),
 						UseRemoteAddress:          wrapperspb.Bool(true),
 						NormalizePath:             wrapperspb.Bool(true),
-						StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{
-							StripAnyHostPort: true,
-						},
 						RequestTimeout:            durationpb.New(10 * time.Second),
 						PreserveExternalRequestId: true,
 						MergeSlashes:              false,
@@ -755,12 +749,9 @@ func TestHTTPConnectionManager(t *testing.T) {
 						CommonHttpProtocolOptions: &envoy_core_v3.HttpProtocolOptions{
 							IdleTimeout: durationpb.New(90 * time.Second),
 						},
-						AccessLog:        FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo),
-						UseRemoteAddress: wrapperspb.Bool(true),
-						NormalizePath:    wrapperspb.Bool(true),
-						StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{
-							StripAnyHostPort: true,
-						},
+						AccessLog:                 FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo),
+						UseRemoteAddress:          wrapperspb.Bool(true),
+						NormalizePath:             wrapperspb.Bool(true),
 						PreserveExternalRequestId: true,
 						MergeSlashes:              false,
 					}),
@@ -808,9 +799,6 @@ func TestHTTPConnectionManager(t *testing.T) {
 						AccessLog:                 FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo),
 						UseRemoteAddress:          wrapperspb.Bool(true),
 						NormalizePath:             wrapperspb.Bool(true),
-						StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{
-							StripAnyHostPort: true,
-						},
 						PreserveExternalRequestId: true,
 						MergeSlashes:              false,
 						StreamIdleTimeout:         durationpb.New(90 * time.Second),
@@ -858,12 +846,9 @@ func TestHTTPConnectionManager(t *testing.T) {
 						CommonHttpProtocolOptions: &envoy_core_v3.HttpProtocolOptions{
 							MaxConnectionDuration: durationpb.New(90 * time.Second),
 						},
-						AccessLog:        FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo),
-						UseRemoteAddress: wrapperspb.Bool(true),
-						NormalizePath:    wrapperspb.Bool(true),
-						StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{
-							StripAnyHostPort: true,
-						},
+						AccessLog:                 FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo),
+						UseRemoteAddress:          wrapperspb.Bool(true),
+						NormalizePath:             wrapperspb.Bool(true),
 						PreserveExternalRequestId: true,
 						MergeSlashes:              false,
 					}),
@@ -911,9 +896,6 @@ func TestHTTPConnectionManager(t *testing.T) {
 						AccessLog:                 FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo),
 						UseRemoteAddress:          wrapperspb.Bool(true),
 						NormalizePath:             wrapperspb.Bool(true),
-						StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{
-							StripAnyHostPort: true,
-						},
 						PreserveExternalRequestId: true,
 						MergeSlashes:              false,
 					}),
@@ -961,9 +943,6 @@ func TestHTTPConnectionManager(t *testing.T) {
 						AccessLog:                 FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo),
 						UseRemoteAddress:          wrapperspb.Bool(true),
 						NormalizePath:             wrapperspb.Bool(true),
-						StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{
-							StripAnyHostPort: true,
-						},
 						PreserveExternalRequestId: true,
 						MergeSlashes:              false,
 						DelayedCloseTimeout:       durationpb.New(90 * time.Second),
@@ -1012,9 +991,6 @@ func TestHTTPConnectionManager(t *testing.T) {
 						AccessLog:                 FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo),
 						UseRemoteAddress:          wrapperspb.Bool(true),
 						NormalizePath:             wrapperspb.Bool(true),
-						StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{
-							StripAnyHostPort: true,
-						},
 						PreserveExternalRequestId: true,
 						MergeSlashes:              false,
 						DrainTimeout:              durationpb.New(90 * time.Second),
@@ -1065,9 +1041,6 @@ func TestHTTPConnectionManager(t *testing.T) {
 						AccessLog:                 FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo),
 						UseRemoteAddress:          wrapperspb.Bool(true),
 						NormalizePath:             wrapperspb.Bool(true),
-						StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{
-							StripAnyHostPort: true,
-						},
 						PreserveExternalRequestId: true,
 						MergeSlashes:              false,
 						DrainTimeout:              durationpb.New(90 * time.Second),
@@ -1116,9 +1089,6 @@ func TestHTTPConnectionManager(t *testing.T) {
 						AccessLog:                 FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo),
 						UseRemoteAddress:          wrapperspb.Bool(true),
 						NormalizePath:             wrapperspb.Bool(true),
-						StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{
-							StripAnyHostPort: true,
-						},
 						PreserveExternalRequestId: true,
 						MergeSlashes:              true,
 					}),
@@ -1162,13 +1132,10 @@ func TestHTTPConnectionManager(t *testing.T) {
 							// a Host: header. See #537.
 							AcceptHttp_10: true,
 						},
-						CommonHttpProtocolOptions: &envoy_core_v3.HttpProtocolOptions{},
-						AccessLog:                 FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo),
-						UseRemoteAddress:          wrapperspb.Bool(true),
-						NormalizePath:             wrapperspb.Bool(true),
-						StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{
-							StripAnyHostPort: true,
-						},
+						CommonHttpProtocolOptions:  &envoy_core_v3.HttpProtocolOptions{},
+						AccessLog:                  FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo),
+						UseRemoteAddress:           wrapperspb.Bool(true),
+						NormalizePath:              wrapperspb.Bool(true),
 						PreserveExternalRequestId:  true,
 						ServerHeaderTransformation: http.HttpConnectionManager_PASS_THROUGH,
 					}),
@@ -1230,9 +1197,6 @@ func TestHTTPConnectionManager(t *testing.T) {
 						AccessLog:                 FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo),
 						UseRemoteAddress:          wrapperspb.Bool(true),
 						NormalizePath:             wrapperspb.Bool(true),
-						StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{
-							StripAnyHostPort: true,
-						},
 						PreserveExternalRequestId: true,
 						MergeSlashes:              false,
 					}),
@@ -1290,9 +1254,6 @@ func TestHTTPConnectionManager(t *testing.T) {
 						AccessLog:                 FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo),
 						UseRemoteAddress:          wrapperspb.Bool(true),
 						NormalizePath:             wrapperspb.Bool(true),
-						StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{
-							StripAnyHostPort: true,
-						},
 						PreserveExternalRequestId: true,
 						MergeSlashes:              false,
 					}),
@@ -1340,9 +1301,6 @@ func TestHTTPConnectionManager(t *testing.T) {
 						AccessLog:                 FileAccessLogEnvoy("/dev/stdout", "", nil, v1alpha1.LogLevelInfo),
 						UseRemoteAddress:          wrapperspb.Bool(true),
 						NormalizePath:             wrapperspb.Bool(true),
-						StripPortMode: &http.HttpConnectionManager_StripAnyHostPort{
-							StripAnyHostPort: true,
-						},
 						PreserveExternalRequestId: true,
 						MergeSlashes:              false,
 						XffNumTrustedHops:         uint32(1),

internal/envoy/v3/route.go

@@ -649,6 +649,9 @@ func RouteConfiguration(name string, virtualhosts ...*envoy_route_v3.VirtualHost
 		RequestHeadersToAdd: headers(
 			appendHeader("x-request-start", "t=%START_TIME(%s.%3f)%"),
 		),
+		// We can ignore any port number supplied in the Host/:authority header
+		// when selecting a virtualhost.
+		IgnorePortInHostMatching: true,
 	}
 }
 

internal/envoy/v3/route_test.go

@@ -1070,6 +1070,7 @@ func TestRouteConfiguration(t *testing.T) {
 					},
 					AppendAction: envoy_core_v3.HeaderValueOption_APPEND_IF_EXISTS_OR_ADD,
 				}},
+				IgnorePortInHostMatching: true,
 			},
 		},
 		"one virtualhost": {
@@ -1089,6 +1090,7 @@ func TestRouteConfiguration(t *testing.T) {
 					},
 					AppendAction: envoy_core_v3.HeaderValueOption_APPEND_IF_EXISTS_OR_ADD,
 				}},
+				IgnorePortInHostMatching: true,
 			},
 		},
 	}

internal/featuretests/v3/wildcardhost_test.go

@@ -93,7 +93,7 @@ func TestIngressWildcardHostHTTP(t *testing.T) {
 									StringMatch: &matcher.StringMatcher{
 										MatchPattern: &matcher.StringMatcher_SafeRegex{
 											SafeRegex: &matcher.RegexMatcher{
-												Regex: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.foo\\.com",
+												Regex: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.foo\\.com(:[0-9]+)?",
 											},
 										},
 									},
@@ -154,7 +154,7 @@ func TestHTTPProxyWildcardFQDN(t *testing.T) {
 								StringMatch: &matcher.StringMatcher{
 									MatchPattern: &matcher.StringMatcher_SafeRegex{
 										SafeRegex: &matcher.RegexMatcher{
-											Regex: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.projectcontour\\.io",
+											Regex: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.projectcontour\\.io(:[0-9]+)?",
 										},
 									},
 								},
@@ -259,7 +259,7 @@ func TestIngressWildcardHostHTTPSWildcardSecret(t *testing.T) {
 									StringMatch: &matcher.StringMatcher{
 										MatchPattern: &matcher.StringMatcher_SafeRegex{
 											SafeRegex: &matcher.RegexMatcher{
-												Regex: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.foo-tls\\.com",
+												Regex: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.foo-tls\\.com(:[0-9]+)?",
 											},
 										},
 									},
@@ -287,7 +287,7 @@ func TestIngressWildcardHostHTTPSWildcardSecret(t *testing.T) {
 									StringMatch: &matcher.StringMatcher{
 										MatchPattern: &matcher.StringMatcher_SafeRegex{
 											SafeRegex: &matcher.RegexMatcher{
-												Regex: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.foo-tls\\.com",
+												Regex: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.foo-tls\\.com(:[0-9]+)?",
 											},
 										},
 									},