Add basic error handling to serverless reverse proxy

5 files changed, 41 insertions, 18 deletions

diff --git a/internal/serving/serverless/director.go b/internal/serving/serverless/director.go
index 0d046af9..68887a13 100644
--- a/internal/serving/serverless/director.go
+++ b/internal/serving/serverless/director.go
@@ -11,8 +11,10 @@ import (
 // request
 func NewDirectorFunc(cluster Cluster) func(*http.Request) {
 	return func(request *http.Request) {
-		request.Host = cluster.Address
-		request.URL.Host = strings.Join([]string{cluster.Address, cluster.Port}, ":")
+		location := strings.Join([]string{cluster.Address, cluster.Port}, ":")
+
+		request.Host = location
+		request.URL.Host = location
 		request.URL.Scheme = "https"
 		request.Header.Set("User-Agent", "GitLab Pages Daemon")
 		request.Header.Set("X-Forwarded-For", realip.FromRequest(request))
diff --git a/internal/serving/serverless/errors.go b/internal/serving/serverless/errors.go
new file mode 100644
index 00000000..dad07fee
--- /dev/null
+++ b/internal/serving/serverless/errors.go
@@ -0,0 +1,16 @@
+package serverless
+
+import (
+	"net/http"
+
+	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
+)
+
+// NewErrorHandler returns a func(http.ResponseWriter, *http.Request, error)
+// responsible for handling proxy errors
+func NewErrorHandler() func(http.ResponseWriter, *http.Request, error) {
+	return func(w http.ResponseWriter, r *http.Request, err error) {
+		// TODO provide serialized error message
+		httperrors.Serve500(w)
+	}
+}
diff --git a/internal/serving/serverless/serverless.go b/internal/serving/serverless/serverless.go
index 77150996..3b7b7d43 100644
--- a/internal/serving/serverless/serverless.go
+++ b/internal/serving/serverless/serverless.go
@@ -16,8 +16,9 @@ type Serverless struct {
 // New returns a new serving instance
 func New(cluster Cluster) serving.Serving {
 	proxy := httputil.ReverseProxy{
-		Director:  NewDirectorFunc(cluster),
-		Transport: NewTransport(cluster),
+		Director:     NewDirectorFunc(cluster),
+		Transport:    NewTransport(cluster),
+		ErrorHandler: NewErrorHandler(),
 	}
 
 	return &Serverless{proxy: &proxy}
diff --git a/internal/serving/serverless/serverless_test.go b/internal/serving/serverless/serverless_test.go
index fc604608..d69a4516 100644
--- a/internal/serving/serverless/serverless_test.go
+++ b/internal/serving/serverless/serverless_test.go
@@ -18,11 +18,8 @@ func TestServeFileHTTP(t *testing.T) {
 	config, err := NewClusterConfig(fixture.Certificate, fixture.Key)
 	require.NoError(t, err)
 
-	cluster := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		assert.Equal(t, "GitLab Pages Daemon", r.Header.Get("User-Agent"))
-		assert.Equal(t, "https", r.Header.Get("X-Forwarded-Proto"))
-		assert.Contains(t, r.Header.Get("X-Forwarded-For"), "127.0.0.123")
-	}))
+	mux := http.NewServeMux()
+	cluster := httptest.NewUnstartedServer(mux)
 
 	cluster.TLS = &tls.Config{
 		Certificates: []tls.Certificate{config.Certificate},
@@ -36,18 +33,29 @@ func TestServeFileHTTP(t *testing.T) {
 	require.NoError(t, err)
 
 	serverless := New(Cluster{
-		Address:  clusterURL.Hostname(),
 		Hostname: "knative.gitlab-example.com",
+		Address:  clusterURL.Hostname(),
 		Port:     clusterURL.Port(),
 		Config:   config,
 	})
 
 	t.Run("when proxying simple request to a cluster", func(t *testing.T) {
 		writer := httptest.NewRecorder()
-		request := httptest.NewRequest("GET", "http://example.gitlab.com", nil)
-		request.Header.Set("X-Real-IP", "127.0.0.123")
+		request := httptest.NewRequest("GET", "http://example.gitlab.com/simple/proxy", nil)
+		request.Header.Set("X-Real-IP", "127.0.0.105")
+
 		handler := serving.Handler{Writer: writer, Request: request}
 
-		assert.True(t, serverless.ServeFileHTTP(handler))
+		mux.HandleFunc("/simple/proxy", func(w http.ResponseWriter, r *http.Request) {
+			assert.Equal(t, "GitLab Pages Daemon", r.Header.Get("User-Agent"))
+			assert.Equal(t, "https", r.Header.Get("X-Forwarded-Proto"))
+			assert.Contains(t, r.Header.Get("X-Forwarded-For"), "127.0.0.105")
+		})
+
+		served := serverless.ServeFileHTTP(handler)
+		result := writer.Result()
+
+		assert.True(t, served)
+		assert.Equal(t, http.StatusOK, result.StatusCode)
 	})
 }
diff --git a/internal/serving/serverless/transport.go b/internal/serving/serverless/transport.go
index 8d195688..d98663f5 100644
--- a/internal/serving/serverless/transport.go
+++ b/internal/serving/serverless/transport.go
@@ -22,11 +22,6 @@ func NewTransport(cluster Cluster) *Transport {
 	}
 
 	dialContext := func(ctx context.Context, network, address string) (net.Conn, error) {
-		// TODO
-		// if address == domain+":443" {
-		// 	address = cluster + ":443"
-		// }
-
 		return dialer.DialContext(ctx, network, address)
 	}
 
@@ -44,5 +39,6 @@ func NewTransport(cluster Cluster) *Transport {
 func (t *Transport) RoundTrip(request *http.Request) (*http.Response, error) {
 	response, err := t.transport.RoundTrip(request)
 
+	// TODO add prometheus metrics for round trip timing
 	return response, err
 }