internal/dag: remove Envoy header name quoting

Remove the '%' quoting on Envoy header names so that operators can
configure Envoy header variables.

This fixes projectcontour#2516.

Signed-off-by: James Peach <jpeach@vmware.com>

_integration/testsuite/httpproxy/011-header-envoy-variables.yaml

@@ -0,0 +1,152 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: ingress-conformance-echo
+$apply:
+  fixture:
+    as: echo
+
+---
+
+apiVersion: v1
+kind: Service
+metadata:
+  name: ingress-conformance-echo
+$apply:
+  fixture:
+    as: echo
+
+---
+
+apiVersion: projectcontour.io/v1
+kind: HTTPProxy
+metadata:
+   name: echo
+spec:
+  virtualhost:
+    fqdn: echo.projectcontour.io
+  routes:
+  - services:
+    - name: echo
+      port: 80
+    requestHeadersPolicy:
+      set:
+      - name: Test-User-Agent
+        value: '%REQ(User-Agent)%'
+      remove:
+      - Test-Removed-Header
+    responseHeadersPolicy:
+      set:
+      - name: Test-Response-Protocol
+        value: '%PROTOCOL%'
+      remove:
+      - X-Envoy-Upstream-Service-Time
+
+---
+
+import data.contour.resources
+
+fatal_proxy_is_not_valid[msg] {
+  name := "echo"
+  proxy := resources.get("httpproxies", name)
+  status := object.get(proxy, "status", {})
+
+  object.get(status, "currentStatus", "") != "valid"
+
+  msg := sprintf("HTTPProxy '%s' is not valid\n%s", [
+    name, yaml.marshal(status)
+  ])
+}
+
+---
+
+import data.contour.http.client
+import data.contour.http.client.url
+import data.contour.http.response
+
+Response := client.Get({
+  "url": url.http(sprintf("/header-envoy-variables/%d", [time.now_ns()])),
+  "headers": {
+    "Host": "echo.projectcontour.io",
+    "User-Agent": client.ua("header-envoy-variables"),
+    "Test-Removed-Header": "remove-me",
+  }
+})
+
+# Return the value (if it is scalar) or the first element (if it is an array).
+head(value) = first {
+  is_array(value)
+  first := value[0]
+} else = first {
+  first := value
+}
+
+# requestval returns the value for the named header in the HTTP request.
+# These values come from the response body.
+requestval(name) = value {
+  body := response.body(Response)
+  headers := object.get(body, "Headers", {})
+  value := object.get(headers, name, "Not-Present")
+}
+
+# responseval returnes the value for the named header in the HTTP response.
+responseval(name) = value {
+  headers := response.headers(Response)
+  value := head(object.get(headers, name, "Not-Present"))
+}
+
+error_non_200_response [msg] {
+  not Response
+  msg := "no response"
+}
+
+error_non_200_response [msg] {
+  status := object.get(Response, "status_code", 000)
+  status != 200
+  msg := sprintf("got status %d, wanted %d", [status, 200])
+}
+
+error_wrong_routing[msg] {
+  response.testid(Response) != "echo"
+  msg := sprintf("got test ID %q, wanted %q", [
+    response.testid(Response), "echo",
+  ])
+}
+
+# Request header removed.
+error_request_header[msg] {
+  requestval("Test-Removed-Header") != "Not-Present"
+  msg := sprintf("removed header %q is still present with value %q", [
+    "Test-Removed-Header",
+    requestval("Test-Removed-Header"),
+  ])
+}
+
+# Request header copied using %REQ%.
+error_request_header[msg] {
+  requestval("User-Agent") != requestval("Test-User-Agent")
+  msg := sprintf("User-Agent header %q doesn't match copy %q", [
+    requestval("User-Agent"),
+    requestval("Test-User-Agent"),
+  ])
+}
+
+# NOTE(jpeach): Not all Envoy headers can be removed from the response
+# (e.g. removing  "Server" doesn't work).
+error_response_header[msg] {
+  responseval("X-Envoy-Upstream-Service-Time") != "Not-Present"
+  msg := sprintf("removed header %q is still present with value %q", [
+    "X-Envoy-Upstream-Service-Time",
+    requestval("X-Envoy-Upstream-Service-Time"),
+  ])
+}
+
+# Header injected into the response.
+error_response_header[msg] {
+  responseval("Test-Response-Protocol") != "HTTP/1.1"
+  msg := sprintf("injected response header %q value %q doesn't match %q", [
+    "Test-Response-Protocol",
+    responseval("Test-Response-Protocol"),
+    "HTTP/1.1",
+  ])
+}

_integration/testsuite/policies/contour-response.rego

@@ -16,14 +16,8 @@ package contour.http.response
 
 has_testid(resp) = true {
   resp.body.TestId
-}
-
-has_testid(resp) = false {
-  not resp.body
-}
-
-has_testid(resp) = false {
-  not resp.body.TestId
+} else = false {
+  true
 }
 
 # Return the HTTP response body, or an empty object if there is no body.
@@ -38,6 +32,18 @@ body(resp) = value {
   value := resp.body
 }
 
+# Return the HTTP response headers, or an empty object if there are no headers.
+headers(resp) = value {
+  is_null(resp.headers)
+  value := {}
+}
+
+# Return the HTTP response body, or an empty object if there are no headers.
+headers(resp) = value {
+  not is_null(resp.headers)
+  value := resp.headers
+}
+
 # Get the TestId element from a ingress-conformance-echo response
 # body. Returns "" if there is no body or no TestId response.
 testid(resp) = value {

internal/dag/builder.go

@@ -933,12 +933,6 @@ func determineSNI(routeRequestHeaders *HeadersPolicy, clusterRequestHeaders *Hea
 	return service.ExternalName
 }
 
-func escapeHeaderValue(value string) string {
-	// Envoy supports %-encoded variables, so literal %'s in the header's value must be escaped.  See:
-	// https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_conn_man/headers#custom-request-response-headers
-	return strings.Replace(value, "%", "%%", -1)
-}
-
 func includeConditionsIdentical(includes []projcontour.Include) bool {
 	j := 0
 	for i := 1; i < len(includes); i++ {

internal/dag/policy.go

@@ -60,7 +60,7 @@ func headersPolicy(policy *projcontour.HeadersPolicy, allowHostRewrite bool) (*H
 		if msgs := validation.IsHTTPHeaderName(key); len(msgs) != 0 {
 			return nil, fmt.Errorf("invalid set header %q: %v", key, msgs)
 		}
-		set[key] = escapeHeaderValue(entry.Value)
+		set[key] = entry.Value
 	}
 
 	remove := sets.NewString()