diff --git a/pkg/dlx/handler.go b/pkg/dlx/handler.go index e9a3119..fbf9111 100644 --- a/pkg/dlx/handler.go +++ b/pkg/dlx/handler.go @@ -79,7 +79,6 @@ func NewHandler(parentLogger logger.Logger, } func (h *Handler) handleRequest(res http.ResponseWriter, req *http.Request) { - var path string var err error var resourceNames []string @@ -100,7 +99,7 @@ func (h *Handler) handleRequest(res http.ResponseWriter, req *http.Request) { resourceNames = append(resourceNames, resourceName) resourceTargetURLMap[resourceName] = targetURL } else { - path, resourceNames, err = h.getPathAndResourceNames(req) + resourceNames, err = h.getResourceNames(req) if err != nil { h.logger.WarnWith("Failed to get resource names and path from request", "error", err.Error(), @@ -109,8 +108,11 @@ func (h *Handler) handleRequest(res http.ResponseWriter, req *http.Request) { res.WriteHeader(http.StatusBadRequest) return } + // add the path addition to the path if the header exists + // this is used when the ingress controller needs to add a suffix to the path request + pathAddition := req.Header.Get(h.targetPathHeader) for _, resourceName := range resourceNames { - targetURL, status := h.parseTargetURL(resourceName, path) + targetURL, status := h.parseTargetURL(resourceName, pathAddition) if targetURL == nil { res.WriteHeader(status) return @@ -166,11 +168,11 @@ func (h *Handler) handleRequest(res http.ResponseWriter, req *http.Request) { proxy.ServeHTTP(res, req) } -func (h *Handler) getPathAndResourceNames(req *http.Request) (string, []string, error) { - // first try to get the resource names and path from the ingress cache - path, resourceNames, err := h.getValuesFromCache(req) +func (h *Handler) getResourceNames(req *http.Request) ([]string, error) { + // first try to get the resource names from the ingress cache + resourceNames, err := h.getValuesFromCache(req) if err == nil { - return path, resourceNames, nil + return resourceNames, nil } h.logger.DebugWith("Failed to get resource names from ingress cache, trying to extract from the request headers", @@ -180,27 +182,26 @@ func (h *Handler) getPathAndResourceNames(req *http.Request) (string, []string, // old implementation for backward compatibility targetNameHeaderValue := req.Header.Get(h.targetNameHeader) - path = req.Header.Get(h.targetPathHeader) if targetNameHeaderValue == "" { - return "", nil, errors.New("No target name header found") + return nil, errors.New("No target name header found") } resourceNames = strings.Split(targetNameHeaderValue, ",") - return path, resourceNames, nil + return resourceNames, nil } -func (h *Handler) getValuesFromCache(req *http.Request) (string, []string, error) { +func (h *Handler) getValuesFromCache(req *http.Request) ([]string, error) { host := req.Host path := h.getRequestURLPath(req) resourceNames, err := h.ingressCache.Get(host, path) if err != nil { - return "", nil, errors.New("Failed to get resource names from ingress cache") + return nil, errors.New("Failed to get resource names from ingress cache") } if len(resourceNames) == 0 { - return "", nil, errors.New("No resources found in ingress cache") + return nil, errors.New("No resources found in ingress cache") } - return path, resourceNames, nil + return resourceNames, nil } func (h *Handler) parseTargetURL(resourceName, path string) (*url.URL, int) { diff --git a/pkg/dlx/handler_test.go b/pkg/dlx/handler_test.go index 8693bc8..a8c7f47 100644 --- a/pkg/dlx/handler_test.go +++ b/pkg/dlx/handler_test.go @@ -44,8 +44,8 @@ func (suite *HandlerTestSuite) SetupTest() { resourceReadinessTimeout: 3 * time.Second, } allowedPaths := map[string]struct{}{ - "/test/path/test/path": {}, - "/test/path/to/multiple/test/path/to/multiple": {}, + "/test/path": {}, + "/test/path/to/multiple": {}, } // Start a test server that always returns 200 suite.httpServer = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -130,13 +130,34 @@ func (suite *HandlerTestSuite) TestHandleRequest() { name: "Request headers flow", reqHeaders: map[string]string{ "X-Forwarded-Host": "127.0.0.1", - "X-Original-Uri": "test/path", + "X-Original-Uri": "", "X-Resource-Name": "test-targets-name-1", }, reqHost: "www.example.com", reqPath: "test/path", expectedStatus: http.StatusOK, }, + { + name: "Request headers negative flow - duplicate request path should fail", + reqHeaders: map[string]string{ + "X-Forwarded-Host": "127.0.0.1", + "X-Original-Uri": "test/path", + "X-Resource-Name": "test-targets-name-1", + }, + reqHost: "www.example.com", + reqPath: "test/path", + expectedStatus: http.StatusBadRequest, + }, + { + name: "Request headers flow with resource path header- should fail because path is duplicated", + reqHeaders: map[string]string{ + "X-Resource-Name": "test-targets-name-1", + "X-Resource-Path": "test/path", + }, + reqHost: "www.example.com", + reqPath: "test/path", + expectedStatus: http.StatusBadRequest, + }, } { suite.Run(testCase.name, func() { // test case setup @@ -166,7 +187,6 @@ func (suite *HandlerTestSuite) TestGetPathAndResourceNames() { reqHost string reqPath string expectErr bool - expectedPath string expectedResourceNames []string }{ { @@ -178,13 +198,11 @@ func (suite *HandlerTestSuite) TestGetPathAndResourceNames() { }, reqHost: "www.example.com", reqPath: "test/path", - expectedPath: "test/path", expectedResourceNames: []string{"test-targets-name-1"}, }, { name: "request headers, host and path did not found in ingress cache", reqHost: "www.example.com", reqPath: "test/path", - expectedPath: "test/path", expectedResourceNames: []string{"test-targets-name-1"}, reqHeaders: map[string]string{ "X-Resource-Name": "test-targets-name-1", @@ -209,7 +227,6 @@ func (suite *HandlerTestSuite) TestGetPathAndResourceNames() { "X-Resource-Name": "test-targets-from-headers", "X-Resource-Path": "test/path", }, - expectedPath: "test/path", expectedResourceNames: []string{"test-targets-from-cache"}, }, { @@ -230,7 +247,7 @@ func (suite *HandlerTestSuite) TestGetPathAndResourceNames() { testHandler, err := suite.createTestHandlerAndInitTestCache(suite.backendPort, testCase.initialCachedData) suite.Require().NoError(err) testRequest := suite.createTestHTTPRequest(testCase.name, testCase.reqHeaders, testCase.reqHost, testCase.reqPath) - resultPath, resultResourceNames, err := testHandler.getPathAndResourceNames(testRequest) + resultResourceNames, err := testHandler.getResourceNames(testRequest) // validate the result if testCase.expectErr { @@ -238,7 +255,6 @@ func (suite *HandlerTestSuite) TestGetPathAndResourceNames() { suite.Require().ErrorContains(err, testCase.errMsg) } else { suite.Require().NoError(err) - suite.Require().Equal(testCase.expectedPath, resultPath) suite.Require().Equal(testCase.expectedResourceNames, resultResourceNames) } }) @@ -308,7 +324,8 @@ func (suite *HandlerTestSuite) setScalerMocksBasedOnTestCase( case "No request headers, scaler fails": suite.scaler.On("ResolveServiceName", mock.Anything).Return(suite.backendHost, resolveServiceNameErr) case "No request headers, not found in ingress cache": - case "Request headers flow": + case "Request headers flow", + "Request headers negative flow - duplicate request path should fail": suite.scaler.On("SetScaleCtx", mock.Anything, mock.Anything, mock.Anything).Return(nil) default: suite.scaler.On("ResolveServiceName", mock.Anything).Return(suite.backendHost, resolveServiceNameErr)