Activator Header Fixes

cmd/queue/main.go

@@ -33,6 +33,7 @@ import (
 
 	"github.com/knative/pkg/logging/logkey"
 	"github.com/knative/serving/cmd/util"
+	activatorutil "github.com/knative/serving/pkg/activator/util"
 	"github.com/knative/serving/pkg/autoscaler"
 	"github.com/knative/serving/pkg/h2c"
 	"github.com/knative/serving/pkg/logging"
@@ -238,6 +239,9 @@ func main() {
 	h2cProxy = httputil.NewSingleHostReverseProxy(target)
 	h2cProxy.Transport = h2c.DefaultTransport
 
+	activatorutil.SetupHeaderPruning(httpProxy)
+	activatorutil.SetupHeaderPruning(h2cProxy)
+
 	// If containerConcurrency == 0 then concurrency is unlimited.
 	if *containerConcurrency > 0 {
 		// We set the queue depth to be equal to the container concurrency.

pkg/activator/handler/handler.go

@@ -20,6 +20,8 @@ import (
 	"net/url"
 
 	"github.com/knative/serving/pkg/activator"
+	"github.com/knative/serving/pkg/activator/util"
+	pkghttp "github.com/knative/serving/pkg/http"
 	"go.uber.org/zap"
 )
 
@@ -32,9 +34,9 @@ type ActivationHandler struct {
 }
 
 func (a *ActivationHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	namespace := r.Header.Get(activator.RevisionHeaderNamespace)
-	name := r.Header.Get(activator.RevisionHeaderName)
-	config := r.Header.Get(activator.ConfigurationHeader)
+	namespace := pkghttp.LastHeaderValue(r.Header, activator.RevisionHeaderNamespace)
+	name := pkghttp.LastHeaderValue(r.Header, activator.RevisionHeaderName)
+	config := pkghttp.LastHeaderValue(r.Header, activator.ConfigurationHeader)
 
 	endpoint, status, err := a.Activator.ActiveEndpoint(namespace, config, name)
 
@@ -54,5 +56,7 @@ func (a *ActivationHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	proxy := httputil.NewSingleHostReverseProxy(target)
 	proxy.Transport = a.Transport
 
+	util.SetupHeaderPruning(proxy)
+
 	proxy.ServeHTTP(w, r)
 }

pkg/activator/handler/handler_test.go

@@ -75,6 +75,7 @@ func TestActivationHandler(t *testing.T) {
 		label     string
 		namespace string
 		name      string
+		config    string
 		wantBody  string
 		wantCode  int
 		wantErr   error
@@ -83,6 +84,7 @@ func TestActivationHandler(t *testing.T) {
 			label:     "active endpoint",
 			namespace: "real-namespace",
 			name:      "real-name",
+			config:    "configuration-name",
 			wantBody:  "everything good!",
 			wantCode:  http.StatusOK,
 			wantErr:   nil,
@@ -91,6 +93,7 @@ func TestActivationHandler(t *testing.T) {
 			label:     "no active endpoint",
 			namespace: "fake-namespace",
 			name:      "fake-name",
+			config:    "configuration-name",
 			wantBody:  errMsg("not found!"),
 			wantCode:  http.StatusNotFound,
 			wantErr:   nil,
@@ -99,6 +102,7 @@ func TestActivationHandler(t *testing.T) {
 			label:     "request error",
 			namespace: "real-namespace",
 			name:      "real-name",
+			config:    "configuration-name",
 			wantBody:  "",
 			wantCode:  http.StatusBadGateway,
 			wantErr:   errors.New("request error!"),
@@ -126,6 +130,7 @@ func TestActivationHandler(t *testing.T) {
 			req := httptest.NewRequest("POST", "http://example.com", nil)
 			req.Header.Set(activator.RevisionHeaderNamespace, e.namespace)
 			req.Header.Set(activator.RevisionHeaderName, e.name)
+			req.Header.Set(activator.ConfigurationHeader, e.config)
 
 			handler.ServeHTTP(resp, req)
 

pkg/activator/handler/reporting_handler.go

@@ -19,6 +19,7 @@ import (
 	"time"
 
 	"github.com/knative/serving/pkg/activator"
+	pkghttp "github.com/knative/serving/pkg/http"
 )
 
 // ReportingHTTPHandler will forward request & response metrics
@@ -29,9 +30,10 @@ type ReportingHTTPHandler struct {
 }
 
 func (h *ReportingHTTPHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	namespace := r.Header.Get(activator.RevisionHeaderNamespace)
-	name := r.Header.Get(activator.RevisionHeaderName)
-	config := r.Header.Get(activator.ConfigurationHeader)
+	namespace := pkghttp.LastHeaderValue(r.Header, activator.RevisionHeaderNamespace)
+	name := pkghttp.LastHeaderValue(r.Header, activator.RevisionHeaderName)
+	config := pkghttp.LastHeaderValue(r.Header, activator.ConfigurationHeader)
+
 	start := time.Now()
 
 	capture := &statusCapture{

pkg/activator/util/header.go

@@ -0,0 +1,44 @@
+/*
+Copyright 2018 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+import (
+	"net/http"
+	"net/http/httputil"
+
+	"github.com/knative/serving/pkg/activator"
+)
+
+var headersToRemove = []string{
+	activator.ConfigurationHeader,
+	activator.RevisionHeaderName,
+	activator.RevisionHeaderNamespace,
+}
+
+// SetupHeaderPruning will cause the http.ReverseProxy
+// to not forward activator headers
+func SetupHeaderPruning(p *httputil.ReverseProxy) {
+	// Director is never null - otherwise ServeHTTP panics
+	orig := p.Director
+	p.Director = func(r *http.Request) {
+		orig(r)
+
+		for _, h := range headersToRemove {
+			r.Header.Del(h)
+		}
+	}
+}

pkg/activator/util/header_test.go

@@ -0,0 +1,85 @@
+/*
+Copyright 2018 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/http/httputil"
+	"net/url"
+	"testing"
+
+	"github.com/knative/serving/pkg/activator"
+)
+
+func TestHeaderPruning(t *testing.T) {
+	var handler http.HandlerFunc = func(w http.ResponseWriter, r *http.Request) {
+		if r.Header.Get(activator.RevisionHeaderName) != "" {
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		}
+
+		if r.Header.Get(activator.RevisionHeaderNamespace) != "" {
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		}
+
+		if r.Header.Get(activator.ConfigurationHeader) != "" {
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		}
+
+		w.WriteHeader(http.StatusOK)
+	}
+
+	server := httptest.NewServer(handler)
+	serverURL, _ := url.Parse(server.URL)
+
+	defer server.Close()
+
+	tests := []struct {
+		name   string
+		header string
+	}{{
+		name:   "revision name header",
+		header: activator.RevisionHeaderName,
+	}, {
+		name:   "revision namespace header",
+		header: activator.RevisionHeaderNamespace,
+	}, {
+		name:   "configuratio name header",
+		header: activator.ConfigurationHeader,
+	}}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			proxy := httputil.NewSingleHostReverseProxy(serverURL)
+			SetupHeaderPruning(proxy)
+
+			resp := httptest.NewRecorder()
+
+			req := httptest.NewRequest("POST", "http://example.com", nil)
+			req.Header.Set(test.header, "some-value")
+
+			proxy.ServeHTTP(resp, req)
+
+			if resp.Code != http.StatusOK {
+				t.Errorf("expected header %q to be filtered", test.header)
+			}
+		})
+	}
+}

pkg/http/header.go

@@ -0,0 +1,37 @@
+/*
+Copyright 2018 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package http
+
+import "net/http"
+
+// LastHeaderValue gets the last value associated with the given key.
+// It is case insensitive; textproto.CanonicalMIMEHeaderKey is used
+// to canonicalize the provided key.
+// If there are no values associated with the key, Get returns "".
+func LastHeaderValue(header http.Header, key string) string {
+	if header == nil {
+		return ""
+	}
+
+	v := header[http.CanonicalHeaderKey(key)]
+
+	if len(v) == 0 {
+		return ""
+	}
+
+	return v[len(v)-1]
+}

pkg/http/header_test.go

@@ -0,0 +1,61 @@
+/*
+Copyright 2018 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package http
+
+import (
+	"net/http"
+	"testing"
+)
+
+func TestLastHeaderValue(t *testing.T) {
+	headerKey := "Header-Key"
+
+	tests := []struct {
+		name     string
+		expected string
+		headers  http.Header
+	}{{
+		name:     "nil headers",
+		expected: "",
+		headers:  nil,
+	}, {
+		name:     "empty header value ",
+		expected: "",
+		headers: http.Header{
+			headerKey: nil,
+		},
+	}, {
+		name:     "single header value ",
+		expected: "first",
+		headers: http.Header{
+			headerKey: {"first"},
+		},
+	}, {
+		name:     "multi header value ",
+		expected: "second",
+		headers: http.Header{
+			headerKey: {"first", "second"},
+		},
+	}}
+
+	for _, test := range tests {
+		got := LastHeaderValue(test.headers, headerKey)
+		if got != test.expected {
+			t.Errorf("Unexpected header value got - %q want - %q", got, test.expected)
+		}
+	}
+}