If applied this PR will allow dynamic host header rewrite on the rout…

…e level for HTTPProxies.

Fixes #5673

Signed-off-by: Clayton Gonsalves <[email protected]>

apis/projectcontour/v1/httpproxy.go

@@ -551,6 +551,10 @@ type Route struct {
 	// +optional
 	PathRewritePolicy *PathRewritePolicy `json:"pathRewritePolicy,omitempty"`
 	// The policy for managing request headers during proxying.
+	// NOTE: You can set the custom values to the host header on a request using
+	// the below format "%REQ(X-Header-Namee)%". If the value of the header
+	// is empty, it is ignored.
+	//
 	// +optional
 	RequestHeadersPolicy *HeadersPolicy `json:"requestHeadersPolicy,omitempty"`
 	// The policy for managing response headers during proxying.
@@ -1268,7 +1272,7 @@ type LoadBalancerPolicy struct {
 }
 
 // HeadersPolicy defines how headers are managed during forwarding.
-// The `Host` header is treated specially and if set in a HTTP response
+// The `Host` header is treated specially and if set in a HTTP request
 // will be used as the SNI server name when forwarding over TLS. It is an
 // error to attempt to set the `Host` header in a HTTP response.
 type HeadersPolicy struct {

changelogs/unreleased/XXXX-clayton-gonsalves-minor.md

@@ -0,0 +1,24 @@
+## HTTPProxy:Allow Host header rewrite with dynamic headers.
+
+This Change allows the host header to be rewritten on requests using dynamic headers on the only route level.
+
+#### Example
+```yaml
+apiVersion: projectcontour.io/v1
+kind: HTTPProxy
+metadata:
+  name: dynamic-host-header-rewrite
+spec:
+  fqdn: local.projectcontour.io
+  routes:
+    - conditions:
+      - prefix: /
+      services:
+        - name: s1
+          port: 80
+    - requestHeaderPolicy:
+        set:
+        - name: host
+          value: "%REQ(x-rewrite-header)%"
+```
+

examples/contour/01-crds.yaml

@@ -5883,8 +5883,10 @@ spec:
                           type: object
                       type: object
                     requestHeadersPolicy:
-                      description: The policy for managing request headers during
-                        proxying.
+                      description: 'The policy for managing request headers during
+                        proxying. NOTE: You can set the custom values to the host
+                        header on a request using the below format "%REQ(X-Header-Namee)%".
+                        If the value of the header is empty, it is ignored.'
                       properties:
                         remove:
                           description: Remove specifies a list of HTTP header names

examples/render/contour-deployment.yaml

@@ -6096,8 +6096,10 @@ spec:
                           type: object
                       type: object
                     requestHeadersPolicy:
-                      description: The policy for managing request headers during
-                        proxying.
+                      description: 'The policy for managing request headers during
+                        proxying. NOTE: You can set the custom values to the host
+                        header on a request using the below format "%REQ(X-Header-Namee)%".
+                        If the value of the header is empty, it is ignored.'
                       properties:
                         remove:
                           description: Remove specifies a list of HTTP header names

examples/render/contour-gateway-provisioner.yaml

@@ -5897,8 +5897,10 @@ spec:
                           type: object
                       type: object
                     requestHeadersPolicy:
-                      description: The policy for managing request headers during
-                        proxying.
+                      description: 'The policy for managing request headers during
+                        proxying. NOTE: You can set the custom values to the host
+                        header on a request using the below format "%REQ(X-Header-Namee)%".
+                        If the value of the header is empty, it is ignored.'
                       properties:
                         remove:
                           description: Remove specifies a list of HTTP header names

examples/render/contour-gateway.yaml

@@ -6102,8 +6102,10 @@ spec:
                           type: object
                       type: object
                     requestHeadersPolicy:
-                      description: The policy for managing request headers during
-                        proxying.
+                      description: 'The policy for managing request headers during
+                        proxying. NOTE: You can set the custom values to the host
+                        header on a request using the below format "%REQ(X-Header-Namee)%".
+                        If the value of the header is empty, it is ignored.'
                       properties:
                         remove:
                           description: Remove specifies a list of HTTP header names

examples/render/contour.yaml

@@ -6096,8 +6096,10 @@ spec:
                           type: object
                       type: object
                     requestHeadersPolicy:
-                      description: The policy for managing request headers during
-                        proxying.
+                      description: 'The policy for managing request headers during
+                        proxying. NOTE: You can set the custom values to the host
+                        header on a request using the below format "%REQ(X-Header-Namee)%".
+                        If the value of the header is empty, it is ignored.'
                       properties:
                         remove:
                           description: Remove specifies a list of HTTP header names

internal/dag/dag.go

@@ -454,6 +454,10 @@ type HeadersPolicy struct {
 	// HostRewrite defines if a host should be rewritten on upstream requests
 	HostRewrite string
 
+	// HostRewriteHeader defines if a host should be rewritten on upstream requests
+	// via a header value. only applicable for routes.
+	HostRewriteHeader string
+
 	Add    map[string]string
 	Set    map[string]string
 	Remove []string

internal/dag/policy.go

@@ -127,6 +127,11 @@ func headersPolicyService(defaultPolicy *HeadersPolicy, policy *contour_api_v1.H
 				return nil, fmt.Errorf("rewriting %q header is not supported", key)
 			}
 			if len(userPolicy.HostRewrite) == 0 {
+				// check for the hostRewriteHeader on the service. Return error if set since this
+				// is not supported on envoy.
+				if HostRewriteHeader := extractHostRewriteHeaderValue(v); HostRewriteHeader != "" {
+					return nil, fmt.Errorf("rewriting %q host header with dynamic value is not supported on service", key)
+				}
 				userPolicy.HostRewrite = v
 			}
 			continue
@@ -164,6 +169,7 @@ func headersPolicyRoute(policy *contour_api_v1.HeadersPolicy, allowHostRewrite b
 
 	set := make(map[string]string, len(policy.Set))
 	hostRewrite := ""
+	hostRewriteHeader := ""
 	for _, entry := range policy.Set {
 		key := http.CanonicalHeaderKey(entry.Name)
 		if _, ok := set[key]; ok {
@@ -173,8 +179,13 @@ func headersPolicyRoute(policy *contour_api_v1.HeadersPolicy, allowHostRewrite b
 			if !allowHostRewrite {
 				return nil, fmt.Errorf("rewriting %q header is not supported", key)
 			}
-			hostRewrite = entry.Value
-			continue
+			if extractedHostRewriteHeader := extractHostRewriteHeaderValue(entry.Value); extractedHostRewriteHeader != "" {
+				hostRewriteHeader = extractedHostRewriteHeader
+				continue
+			} else {
+				hostRewrite = entry.Value
+				continue
+			}
 		}
 		if msgs := validation.IsHTTPHeaderName(key); len(msgs) != 0 {
 			return nil, fmt.Errorf("invalid set header %q: %v", key, msgs)
@@ -203,12 +214,26 @@ func headersPolicyRoute(policy *contour_api_v1.HeadersPolicy, allowHostRewrite b
 	}
 
 	return &HeadersPolicy{
-		Set:         set,
-		HostRewrite: hostRewrite,
-		Remove:      rl,
+		Set:               set,
+		HostRewrite:       hostRewrite,
+		HostRewriteHeader: hostRewriteHeader,
+		Remove:            rl,
 	}, nil
 }
 
+// extractHostRewriteHeaderValue returns the value of the header
+func extractHostRewriteHeaderValue(s string) string {
+	// match "%REQ(<X-Foo-Bar>)%"
+	re := regexp.MustCompile(`%REQ\((\S+)\)%`)
+	matches := re.FindStringSubmatch(s)
+
+	if len(matches) == 2 {
+		return strings.TrimSpace(matches[1])
+	}
+
+	return ""
+}
+
 // headersPolicyGatewayAPI builds a *HeaderPolicy for the supplied HTTPHeaderFilter.
 // TODO: Take care about the order of operators once https://github.com/kubernetes-sigs/gateway-api/issues/480 was solved.
 func headersPolicyGatewayAPI(hf *gatewayapi_v1beta1.HTTPHeaderFilter, headerPolicyType string) (*HeadersPolicy, error) {

internal/dag/policy_test.go

@@ -15,6 +15,7 @@ package dag
 
 import (
 	"errors"
+	"fmt"
 	"io"
 	"testing"
 	"time"
@@ -1249,6 +1250,36 @@ func TestValidateHeaderAlteration(t *testing.T) {
 				"K-Foo": "100%%",
 			},
 		},
+	}, {
+		name: "rewrite host header with dynamic headers",
+		in: &contour_api_v1.HeadersPolicy{
+			Set: []contour_api_v1.HeaderValue{{
+				Name:  "K-Foo",
+				Value: "100%",
+			}},
+		},
+		dyn: map[string]string{
+			"CONTOUR_NAMESPACE": "myns",
+		},
+		dhp: &HeadersPolicy{
+			Set: map[string]string{
+				"K-Foo": "50%",
+			},
+		},
+		want: &HeadersPolicy{
+			Set: map[string]string{
+				"K-Foo": "100%%",
+			},
+		},
+	}, {
+		name: "Host header rewrite via dynamic header",
+		in: &contour_api_v1.HeadersPolicy{
+			Set: []contour_api_v1.HeaderValue{{
+				Name:  "Host",
+				Value: "%REQ(foo)%",
+			}},
+		},
+		wantErr: fmt.Errorf("rewriting \"Host\" header is not supported"),
 	}}
 
 	for _, test := range tests {
@@ -1259,3 +1290,124 @@ func TestValidateHeaderAlteration(t *testing.T) {
 		})
 	}
 }
+
+func TestExtractHeaderValue(t *testing.T) {
+	tests := map[string]string{
+		"%REQ(X-Header-Name)%":  "X-Header-Name",
+		"%req(X-Header-Name)%":  "",
+		"%REQ( Content-Type )%": "",
+		"REQ(Content-Type)":     "",
+		"%REQ(Content-Type%":    "",
+		"SomeOtherValue":        "",
+	}
+
+	for input, expected := range tests {
+		t.Run(input, func(t *testing.T) {
+			actual := extractHostRewriteHeaderValue(input)
+			if actual != expected {
+				t.Errorf("For input %q, expected %q, got %q", input, expected, actual)
+			}
+		})
+	}
+}
+
+func TestHeadersPolicyRoute(t *testing.T) {
+	tests := []struct {
+		name         string
+		policy       *contour_api_v1.HeadersPolicy
+		allowRewrite bool
+		dynHeaders   map[string]string
+		expected     *HeadersPolicy
+		expectedErr  error
+	}{
+		{
+			name:     "nil policy",
+			policy:   nil,
+			expected: nil,
+		},
+		{
+			name: "duplicate set headers",
+			policy: &contour_api_v1.HeadersPolicy{
+				Set: []contour_api_v1.HeaderValue{{Name: "X-Header", Value: "Test"}, {Name: "X-Header", Value: "Test2"}},
+			},
+			expectedErr: fmt.Errorf("duplicate header addition: %q", "X-Header"),
+		},
+		{
+			name: "host rewrite not allowed",
+			policy: &contour_api_v1.HeadersPolicy{
+				Set: []contour_api_v1.HeaderValue{{Name: "Host", Value: "Test"}},
+			},
+			allowRewrite: false,
+			expectedErr:  fmt.Errorf("rewriting %q header is not supported", "Host"),
+		},
+		{
+			name: "host rewrite allowed",
+			policy: &contour_api_v1.HeadersPolicy{
+				Set: []contour_api_v1.HeaderValue{{Name: "Host", Value: "Test"}},
+			},
+			allowRewrite: true,
+			expected: &HeadersPolicy{
+				HostRewrite: "Test",
+				Remove:      nil,
+			},
+		},
+		{
+			name: "host rewrite allowed, by header",
+			policy: &contour_api_v1.HeadersPolicy{
+				Set: []contour_api_v1.HeaderValue{{Name: "Host", Value: "%REQ(Test)%"}},
+			},
+			allowRewrite: true,
+			expected: &HeadersPolicy{
+				HostRewrite:       "",
+				HostRewriteHeader: "Test",
+				Remove:            nil,
+			},
+		},
+		{
+			name: "host rewrite allowed, by header. invalid",
+			policy: &contour_api_v1.HeadersPolicy{
+				Set: []contour_api_v1.HeaderValue{{Name: "Host", Value: "%REQ (Test"}},
+			},
+			allowRewrite: true,
+			expected: &HeadersPolicy{
+				HostRewrite:       "%REQ (Test",
+				HostRewriteHeader: "",
+				Remove:            nil,
+			},
+		},
+		{
+			name: "invalid header name",
+			policy: &contour_api_v1.HeadersPolicy{
+				Set: []contour_api_v1.HeaderValue{{Name: " Invalid-Header ", Value: "Test"}},
+			},
+			expectedErr: fmt.Errorf(`invalid set header " Invalid-Header ": [a valid HTTP header must consist of alphanumeric characters or '-' (e.g. 'X-Header-Name', regex used for validation is '[-A-Za-z0-9]+')]`),
+		},
+		{
+			name: "duplicate remove headers",
+			policy: &contour_api_v1.HeadersPolicy{
+				Remove: []string{"X-Header", "X-Header"},
+			},
+			expectedErr: fmt.Errorf("duplicate header removal: %q", "X-Header"),
+		},
+		{
+			name: "valid set and remove headers",
+			policy: &contour_api_v1.HeadersPolicy{
+				Set:    []contour_api_v1.HeaderValue{{Name: "X-Header", Value: "Test"}},
+				Remove: []string{"Y-Header"},
+			},
+			expected: &HeadersPolicy{
+				Set:         map[string]string{"X-Header": "Test"},
+				HostRewrite: "",
+				Remove:      []string{"Y-Header"},
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			result, err := headersPolicyRoute(tc.policy, tc.allowRewrite, tc.dynHeaders)
+			assert.Equal(t, tc.expected, result)
+			assert.Equal(t, tc.expectedErr, err)
+		})
+	}
+}

internal/envoy/route.go

@@ -19,13 +19,20 @@ import (
 	"google.golang.org/protobuf/types/known/durationpb"
 )
 
-func HostReplaceHeader(hp *dag.HeadersPolicy) string {
+func HostRewriteLiteral(hp *dag.HeadersPolicy) string {
 	if hp == nil {
 		return ""
 	}
 	return hp.HostRewrite
 }
 
+func HostRewriteHeader(hp *dag.HeadersPolicy) string {
+	if hp == nil {
+		return ""
+	}
+	return hp.HostRewriteHeader
+}
+
 // Timeout converts a timeout.Setting to a protobuf.Duration
 // that's appropriate for Envoy. In general (though there are
 // exceptions), Envoy uses the following semantics: