From 0ad7bdb5b2782576e1477ed39847160dd186329c Mon Sep 17 00:00:00 2001 From: "Alex Ellis (OpenFaaS Ltd)" Date: Tue, 8 Aug 2023 15:13:54 +0100 Subject: [PATCH 1/2] 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) --- proxy/handler_test.go | 49 ++++++++++++++++++++++++++++++++++++++++++- proxy/proxy.go | 15 ++++++++++--- proxy/proxy_test.go | 10 ++++----- 3 files changed, 65 insertions(+), 9 deletions(-) diff --git a/proxy/handler_test.go b/proxy/handler_test.go index 2c9fa9b..cd021fb 100644 --- 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 index fdd5559..4ca099f 100644 --- 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 index 3898cf0..35d4296 100644 --- 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)) From 59a610197bc201a45d7d90a237c9b5fcd9232f69 Mon Sep 17 00:00:00 2001 From: "Alex Ellis (OpenFaaS Ltd)" Date: Tue, 8 Aug 2023 15:15:58 +0100 Subject: [PATCH 2/2] Modify to uppercase for X-OpenFaaS-Internal Signed-off-by: Alex Ellis (OpenFaaS Ltd) --- proxy/proxy.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/proxy/proxy.go b/proxy/proxy.go index 4ca099f..8cf1888 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -137,7 +137,7 @@ 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") + w.Header().Add("X-OpenFaaS-Internal", "proxy") httputil.Errorf(w, http.StatusBadRequest, "Provide function name in the request path") return @@ -145,7 +145,7 @@ func proxyRequest(w http.ResponseWriter, originalReq *http.Request, proxyClient functionAddr, err := resolver.Resolve(functionName) if err != nil { - w.Header().Add("x-openfaas-internal", "proxy") + 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, err.Error()) @@ -156,7 +156,7 @@ func proxyRequest(w http.ResponseWriter, originalReq *http.Request, proxyClient proxyReq, err := buildProxyRequest(originalReq, functionAddr, pathVars["params"]) if err != nil { - w.Header().Add("x-openfaas-internal", "proxy") + w.Header().Add("X-OpenFaaS-Internal", "proxy") httputil.Errorf(w, http.StatusInternalServerError, "Failed to resolve service: %s.", functionName) return @@ -173,7 +173,7 @@ 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") + w.Header().Add("X-OpenFaaS-Internal", "proxy") httputil.Errorf(w, http.StatusInternalServerError, "Can't reach service for: %s.", functionName) return