Add a header to mark when a response is from openfaas

* The function proxy will now add an extra header to show when an error occurred during invocation. * Added unit tests and peer reviewed with @welteki Signed-off-by: Alex Ellis (OpenFaaS Ltd) <[email protected]>
openfaas · Aug 8, 2023 · 0ad7bdb · 0ad7bdb
1 parent 8a2dc38
commit 0ad7bdb
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 9 deletions.
diff --git a/proxy/handler_test.go b/proxy/handler_test.go
@@ -30,6 +30,7 @@ func (tr *testBaseURLResolver) Resolve(name string) (url.URL, error) {
 		Host:   tr.testServerBase,
 	}, nil
 }
+
 func Test_NewHandlerFunc_Panic(t *testing.T) {
 	defer func() {
 		if r := recover(); r == nil {
@@ -126,6 +127,12 @@ func Test_ProxyHandler_ResolveError(t *testing.T) {
 	if !strings.Contains(logs.String(), resolveErr.Error()) {
 		t.Errorf("expected logs to contain `%s`", resolveErr.Error())
 	}
+
+	internalErrorHeader := w.Header().Get("X-OpenFaaS-Internal")
+	wantHeaderValue := "proxy"
+	if internalErrorHeader != wantHeaderValue {
+		t.Errorf("expected X-OpenFaaS-Internal header to be %s, got %s", wantHeaderValue, internalErrorHeader)
+	}
 }
 
 func Test_ProxyHandler_Proxy_Success(t *testing.T) {
@@ -160,8 +167,48 @@ func Test_ProxyHandler_Proxy_Success(t *testing.T) {
 			proxyFunc(w, req)
 			resp := w.Result()
 			if resp.StatusCode != http.StatusNoContent {
-				t.Errorf("expected status code `%d`, got `%d`", http.StatusNoContent, resp.StatusCode)
+				t.Fatalf("expected status code `%d`, got `%d`", http.StatusNoContent, resp.StatusCode)
 			}
+
+			if v := resp.Header.Get("X-OpenFaaS-Internal"); v != "" {
+				t.Fatalf("expected X-OpenFaaS-Internal header to be empty, got %q", v)
+			}
+
 		})
 	}
 }
+
+func Test_ProxyHandler_Proxy_FailsMidFlight(t *testing.T) {
+	var svr *httptest.Server
+
+	testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+
+		svr.Close()
+		// w.WriteHeader(http.StatusOK)
+	})
+	svr = httptest.NewServer(testHandler)
+
+	config := types.FaaSConfig{
+		ReadTimeout:  100 * time.Millisecond,
+		WriteTimeout: 100 * time.Millisecond,
+	}
+
+	serverURL := strings.TrimPrefix(svr.URL, "http://")
+	proxyFunc := NewHandlerFunc(config, &testBaseURLResolver{serverURL, nil})
+
+	w := httptest.NewRecorder()
+
+	req := httptest.NewRequest(http.MethodPost, "http://example.com/foo", nil)
+	req = mux.SetURLVars(req, map[string]string{"name": "foo"})
+
+	proxyFunc(w, req)
+	resp := w.Result()
+	wantCode := http.StatusInternalServerError
+	if resp.StatusCode != wantCode {
+		t.Fatalf("want status code `%d`, got `%d`", wantCode, resp.StatusCode)
+	}
+
+	if v := resp.Header.Get("X-OpenFaaS-Internal"); v != "proxy" {
+		t.Errorf("expected X-OpenFaaS-Internal header to be `proxy`, got %s", v)
+	}
+}
diff --git a/proxy/proxy.go b/proxy/proxy.go
@@ -137,20 +137,27 @@ func proxyRequest(w http.ResponseWriter, originalReq *http.Request, proxyClient
 	pathVars := mux.Vars(originalReq)
 	functionName := pathVars["name"]
 	if functionName == "" {
+		w.Header().Add("x-openfaas-internal", "proxy")
+
 		httputil.Errorf(w, http.StatusBadRequest, "Provide function name in the request path")
 		return
 	}
 
-	functionAddr, resolveErr := resolver.Resolve(functionName)
-	if resolveErr != nil {
+	functionAddr, err := resolver.Resolve(functionName)
+	if err != nil {
+		w.Header().Add("x-openfaas-internal", "proxy")
+
 		// TODO: Should record the 404/not found error in Prometheus.
-		log.Printf("resolver error: no endpoints for %s: %s\n", functionName, resolveErr.Error())
+		log.Printf("resolver error: no endpoints for %s: %s\n", functionName, err.Error())
 		httputil.Errorf(w, http.StatusServiceUnavailable, "No endpoints available for: %s.", functionName)
 		return
 	}
 
 	proxyReq, err := buildProxyRequest(originalReq, functionAddr, pathVars["params"])
 	if err != nil {
+
+		w.Header().Add("x-openfaas-internal", "proxy")
+
 		httputil.Errorf(w, http.StatusInternalServerError, "Failed to resolve service: %s.", functionName)
 		return
 	}
@@ -166,6 +173,8 @@ func proxyRequest(w http.ResponseWriter, originalReq *http.Request, proxyClient
 	if err != nil {
 		log.Printf("error with proxy request to: %s, %s\n", proxyReq.URL.String(), err.Error())
 
+		w.Header().Add("x-openfaas-internal", "proxy")
+
 		httputil.Errorf(w, http.StatusInternalServerError, "Can't reach service for: %s.", functionName)
 		return
 	}

diff --git a/proxy/proxy_test.go b/proxy/proxy_test.go
@@ -3,7 +3,7 @@ package proxy
 import (
 	"bytes"
 	"fmt"
-	"io/ioutil"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -186,7 +186,7 @@ func Test_buildProxyRequest_Body_Method_Query(t *testing.T) {
 		t.Fail()
 	}
 
-	upstreamBytes, _ := ioutil.ReadAll(upstream.Body)
+	upstreamBytes, _ := io.ReadAll(upstream.Body)
 
 	if string(upstreamBytes) != string(srcBytes) {
 		t.Errorf("Body - want: %s, got: %s", string(upstreamBytes), string(srcBytes))
@@ -357,7 +357,7 @@ func Test_buildProxyRequest_WithPathNoQuery(t *testing.T) {
 		t.Fail()
 	}
 
-	upstreamBytes, _ := ioutil.ReadAll(upstream.Body)
+	upstreamBytes, _ := io.ReadAll(upstream.Body)
 
 	if string(upstreamBytes) != string(srcBytes) {
 		t.Errorf("Body - want: %s, got: %s", string(upstreamBytes), string(srcBytes))
@@ -409,7 +409,7 @@ func Test_buildProxyRequest_WithNoPathNoQuery(t *testing.T) {
 		t.Fail()
 	}
 
-	upstreamBytes, _ := ioutil.ReadAll(upstream.Body)
+	upstreamBytes, _ := io.ReadAll(upstream.Body)
 
 	if string(upstreamBytes) != string(srcBytes) {
 		t.Errorf("Body - want: %s, got: %s", string(upstreamBytes), string(srcBytes))
@@ -459,7 +459,7 @@ func Test_buildProxyRequest_WithPathAndQuery(t *testing.T) {
 		t.Fail()
 	}
 
-	upstreamBytes, _ := ioutil.ReadAll(upstream.Body)
+	upstreamBytes, _ := io.ReadAll(upstream.Body)
 
 	if string(upstreamBytes) != string(srcBytes) {
 		t.Errorf("Body - want: %s, got: %s", string(upstreamBytes), string(srcBytes))