[Scaler] Fix path duplication for request http

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) {

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,15 +247,14 @@ 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 {
 				suite.Require().Error(err)
 				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)