Repository: envoyproxy/gateway

Issue:
Implicitly allow OPTIONS method when cors filter attached to a path to serve preflight requests
*Description*:
Hi, 

When i use httproute with cors filter attached, i have to explicitly create a option method on the paths to support cors. Otherwise i get 404. 

For example
for the following HttpRoute
```
matches:
  - path:
      type: PathPrefix
      value: /hello
    method: "GET"
```

when i attach a cors filter i will get a 404 for preflight requests.

In order to make the cors filter works i have to explicitly add a OPTION method to the same path. like below
```
matches:
  - path:
      type: PathPrefix
      value: /hello
    method: "GET"
  - path:
      type: PathPrefix
      value: /hello
    method: "OPTIONS"
```

I suggest the following to be implemented in the EG operator. If a user attach a CORs filter to a httproute the operator should implicitly create a xds route that will enable OPTION method on the paths in the httproute and it will look match for origin and Access-Control-Request-Method. So this way we can distinguise normal requests vs preflight requests. 

Example xds:
```
routes:
- match:
    path: "/hello"
    headers:
      - name: ":method"
        exact_match: "OPTIONS"
      - name: origin
      - name: access-control-request-method
```

Discussion:
@kkk777-7: /assign@zhaohuabing: @kkk777-7 Let's hold off until we reach an agreement in the [Gateway API upstream discussion](https://github.com/kubernetes-sigs/gateway-api/issues/3857).@kkk777-7: @zhaohuabing thanks for comment! I'll hold off on the work.@zhaohuabing: If we explicitly added "OPTIONS" to the match, then the preflight requests shouldn't be forwarded to the backends. However, the current Envoy CORS implementation [only handles preflights matching the origin](https://github.com/envoyproxy/envoy/blob/fe228896f2761751c488f0abe0195f34db87597b/source/extensions/filters/http/cors/cors_filter.cc#L129-L131). As a result, preflight requests with non-matching origins are passed through to the backend service —  which goes against the user's original intention of not sending them to the backend.
@arkodg: @zhaohuabing they are not, we fixed this with `forward_not_matching_preflights`@zhaohuabing: > [@zhaohuabing](https://github.com/zhaohuabing) they are not, we fixed this with `forward_not_matching_preflights`

Ha, I haven't noticed this option. That works!@github-actions: This issue has been automatically marked as stale because it has not had activity in the last 30 days.

Solution patch:
diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go
index bbee3f7d0..e85181845 100644
--- a/internal/gatewayapi/route.go
+++ b/internal/gatewayapi/route.go
@@ -623,6 +623,52 @@ func (t *Translator) processHTTPRouteRule(
 		}
 		applyHTTPFiltersContextToIRRoute(httpFiltersContext, irRoute)
 		ruleRoutes = append(ruleRoutes, irRoute)
+
+		// When using a CORS filter with method matching that excludes OPTIONS,
+		// users must explicitly specify OPTIONS method match to handle CORS preflight requests.
+		// - https://github.com/kubernetes-sigs/gateway-api/issues/3857
+		//
+		// Envoy Gateway improves user experience by implicitly creating the envoy route for CORS preflight.
+		if (httpFiltersContext != nil && httpFiltersContext.CORS != nil) &&
+			(match.Method != nil && string(*match.Method) != "OPTIONS") {
+			corsRoute := &ir.HTTPRoute{
+				Name:              irRouteName(httpRoute, ruleIdx, matchIdx) + "/cors-preflight",
+				Metadata:          routeRuleMetadata,
+				PathMatch:         irRoute.PathMatch,
+				QueryParamMatches: irRoute.QueryParamMatches,
+				CORS:              httpFiltersContext.CORS,
+			}
+
+			// Create header matches:
+			// copy original headers (excluding :method) + add CORS headers (:method=OPTIONS, origin, access-control-request-method)
+			headerMatches := make([]*ir.StringMatch, 0, len(irRoute.HeaderMatches)+2)
+			for _, headerMatch := range irRoute.HeaderMatches {
+				// Skip the original method match for CORS preflight route to avoid conflicting method requirements.
+				if headerMatch.Name == ":method" {
+					continue
+				}
+				headerMatches = append(headerMatches, headerMatch)
+			}
+
+			corsHeaders := []*ir.StringMatch{
+				{
+					Name:  ":method",
+					Exact: ptr.To("OPTIONS"),
+				},
+				{
+					Name:      "origin",
+					SafeRegex: ptr.To(".*"),
+				},
+				{
+					Name:      "access-control-request-method",
+					SafeRegex: ptr.To(".*"),
+				},
+			}
+			headerMatches = append(headerMatches, corsHeaders...)
+
+			corsRoute.HeaderMatches = headerMatches
+			ruleRoutes = append(ruleRoutes, corsRoute)
+		}
 	}
 
 	return ruleRoutes, nil
diff --git a/release-notes/current.yaml b/release-notes/current.yaml
index e835c286f..3976a861d 100644
--- a/release-notes/current.yaml
+++ b/release-notes/current.yaml
@@ -29,6 +29,7 @@ new features: |
   Added support for Envoy PreconnectPolicy in BackendTrafficPolicy.
   Added support for binaryData in ConfigMap referenced by HTTPRouteFilter for direct response.
   Added support PDB for Ratelimit service.
+  Added support for allowing OPTIONS method implicitly when HTTPRoute CORS filter is used.
   Added support rate limit based on path and method in BackendTrafficPolicy.
 
 bug fixes: |



Test patch:
diff --git a/internal/gatewayapi/testdata/httproute-with-cors-filter.in.yaml b/internal/gatewayapi/testdata/httproute-with-cors-filter.in.yaml
index e143c40f4..6659df8f8 100644
--- a/internal/gatewayapi/testdata/httproute-with-cors-filter.in.yaml
+++ b/internal/gatewayapi/testdata/httproute-with-cors-filter.in.yaml
@@ -52,7 +52,6 @@ httpRoutes:
           - header4
           allowCredentials: true
           maxAge: 1000
-
 - apiVersion: gateway.networking.k8s.io/v1
   kind: HTTPRoute
   metadata:
@@ -82,3 +81,35 @@ httpRoutes:
           - POST
           - PUT
           - DELETE
+- apiVersion: gateway.networking.k8s.io/v1
+  kind: HTTPRoute
+  metadata:
+    namespace: default
+    name: httproute-cors-specific-methods
+  spec:
+    hostnames:
+    - gateway.envoyproxy.io
+    parentRefs:
+    - namespace: envoy-gateway
+      name: gateway-1
+      sectionName: http
+    rules:
+    - matches:
+      - path:
+          value: "/baz"
+        method: "GET"
+        queryParams:
+        - type: RegularExpression
+          name: id
+          value: '.*'
+      backendRefs:
+      - name: service-1
+        port: 8080
+      filters:
+      - type: CORS
+        cors:
+          allowOrigins:
+          - "https://gateway.envoyproxy.io"
+          allowMethods:
+          - OPTIONS
+          - GET
diff --git a/internal/gatewayapi/testdata/httproute-with-cors-filter.out.yaml b/internal/gatewayapi/testdata/httproute-with-cors-filter.out.yaml
index 87c6841ae..bda8e47b0 100644
--- a/internal/gatewayapi/testdata/httproute-with-cors-filter.out.yaml
+++ b/internal/gatewayapi/testdata/httproute-with-cors-filter.out.yaml
@@ -16,7 +16,7 @@ gateways:
       protocol: HTTP
   status:
     listeners:
-    - attachedRoutes: 2
+    - attachedRoutes: 3
       conditions:
       - lastTransitionTime: null
         message: Sending translated listener configuration to the data plane
@@ -142,6 +142,56 @@ httpRoutes:
         name: gateway-1
         namespace: envoy-gateway
         sectionName: http
+- apiVersion: gateway.networking.k8s.io/v1
+  kind: HTTPRoute
+  metadata:
+    name: httproute-cors-specific-methods
+    namespace: default
+  spec:
+    hostnames:
+    - gateway.envoyproxy.io
+    parentRefs:
+    - name: gateway-1
+      namespace: envoy-gateway
+      sectionName: http
+    rules:
+    - backendRefs:
+      - name: service-1
+        port: 8080
+      filters:
+      - cors:
+          allowMethods:
+          - OPTIONS
+          - GET
+          allowOrigins:
+          - https://gateway.envoyproxy.io
+        type: CORS
+      matches:
+      - method: GET
+        path:
+          value: /baz
+        queryParams:
+        - name: id
+          type: RegularExpression
+          value: .*
+  status:
+    parents:
+    - conditions:
+      - lastTransitionTime: null
+        message: Route is accepted
+        reason: Accepted
+        status: "True"
+        type: Accepted
+      - lastTransitionTime: null
+        message: Resolved all the Object references for the Route
+        reason: ResolvedRefs
+        status: "True"
+        type: ResolvedRefs
+      controllerName: gateway.envoyproxy.io/gatewayclass-controller
+      parentRef:
+        name: gateway-1
+        namespace: envoy-gateway
+        sectionName: http
 infraIR:
   envoy-gateway/gateway-1:
     proxy:
@@ -203,6 +253,104 @@ xdsIR:
         mergeSlashes: true
       port: 10080
       routes:
+      - cors:
+          allowMethods:
+          - OPTIONS
+          - GET
+          allowOrigins:
+          - distinct: false
+            exact: https://gateway.envoyproxy.io
+            name: ""
+          maxAge: 0s
+        destination:
+          metadata:
+            kind: HTTPRoute
+            name: httproute-cors-specific-methods
+            namespace: default
+          name: httproute/default/httproute-cors-specific-methods/rule/0
+          settings:
+          - addressType: IP
+            endpoints:
+            - host: 7.7.7.7
+              port: 8080
+            metadata:
+              name: service-1
+              namespace: default
+              sectionName: "8080"
+            name: httproute/default/httproute-cors-specific-methods/rule/0/backend/0
+            protocol: HTTP
+            weight: 1
+        headerMatches:
+        - distinct: false
+          exact: OPTIONS
+          name: :method
+        - distinct: false
+          name: origin
+          safeRegex: .*
+        - distinct: false
+          name: access-control-request-method
+          safeRegex: .*
+        hostname: gateway.envoyproxy.io
+        isHTTP2: false
+        metadata:
+          kind: HTTPRoute
+          name: httproute-cors-specific-methods
+          namespace: default
+        name: httproute/default/httproute-cors-specific-methods/rule/0/match/0/cors-preflight/gateway_envoyproxy_io
+        pathMatch:
+          distinct: false
+          name: ""
+          prefix: /baz
+        queryParamMatches:
+        - distinct: false
+          name: id
+          safeRegex: .*
+      - cors:
+          allowMethods:
+          - OPTIONS
+          - GET
+          allowOrigins:
+          - distinct: false
+            exact: https://gateway.envoyproxy.io
+            name: ""
+          maxAge: 0s
+        destination:
+          metadata:
+            kind: HTTPRoute
+            name: httproute-cors-specific-methods
+            namespace: default
+          name: httproute/default/httproute-cors-specific-methods/rule/0
+          settings:
+          - addressType: IP
+            endpoints:
+            - host: 7.7.7.7
+              port: 8080
+            metadata:
+              name: service-1
+              namespace: default
+              sectionName: "8080"
+            name: httproute/default/httproute-cors-specific-methods/rule/0/backend/0
+            protocol: HTTP
+            weight: 1
+        headerMatches:
+        - distinct: false
+          exact: GET
+          name: :method
+        hostname: gateway.envoyproxy.io
+        isHTTP2: false
+        metadata:
+          kind: HTTPRoute
+          name: httproute-cors-specific-methods
+          namespace: default
+        name: httproute/default/httproute-cors-specific-methods/rule/0/match/0/gateway_envoyproxy_io
+        pathMatch:
+          distinct: false
+          name: ""
+          prefix: /baz
+        queryParamMatches:
+        - distinct: false
+          name: id
+          safeRegex: .*
       - cors:
           allowCredentials: true
           allowHeaders:
diff --git a/test/e2e/testdata/cors-http-cors-filter.yaml b/test/e2e/testdata/cors-http-cors-filter.yaml
index 6f42f1880..812eb0f0a 100644
--- a/test/e2e/testdata/cors-http-cors-filter.yaml
+++ b/test/e2e/testdata/cors-http-cors-filter.yaml
@@ -62,3 +62,34 @@ spec:
         - "*"
         exposeHeaders:
         - "*"
+---
+# HTTPRoute with CORS filter and method match to verify that OPTIONS method is implicitly allowed for CORS preflight
+apiVersion: gateway.networking.k8s.io/v1
+kind: HTTPRoute
+metadata:
+  name: http-with-cors-specific-method
+  namespace: gateway-conformance-infra
+spec:
+  parentRefs:
+  - name: same-namespace
+  rules:
+  - matches:
+    - path:
+        type: PathPrefix
+        value: /cors-specific-method
+      method: GET
+    backendRefs:
+    - name: infra-backend-v1
+      port: 8080
+    filters:
+    - type: CORS
+      cors:
+        allowOrigins:
+        - "https://www.foo.com"
+        allowMethods:
+        - GET
+        - OPTIONS
+        allowHeaders:
+        - "*"
+        exposeHeaders:
+        - "*"
diff --git a/test/e2e/tests/cors.go b/test/e2e/tests/cors.go
index f2421d37d..9112ae7b5 100644
--- a/test/e2e/tests/cors.go
+++ b/test/e2e/tests/cors.go
@@ -202,4 +202,44 @@ func runCORStest(t *testing.T, suite *suite.ConformanceTestSuite, withSecurityPo
 
 		http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectedResponse)
 	})
+
+	if !withSecurityPolicy {
+		t.Run("should enable cors with specific method match", func(t *testing.T) {
+			// When using HTTPCORSFilter with method match, OPTIONS method is implicitly allowed for CORS preflight
+			// even without explicit OPTIONS method match, so preflight response should be returned successfully
+			expectedResponse := http.ExpectedResponse{
+				Request: http.Request{
+					Path:   "/cors-specific-method",
+					Method: "OPTIONS",
+					Headers: map[string]string{
+						"Origin":                         "https://www.foo.com",
+						"access-control-request-method":  "GET",
+						"access-control-request-headers": "x-header-1, x-header-2",
+					},
+				},
+				// Set the expected request properties to empty strings.
+				// This is a workaround to avoid the test failure.
+				// The response body is empty because the request is a preflight request.
+				ExpectedRequest: &http.ExpectedRequest{
+					Request: http.Request{
+						Host:    "",
+						Method:  "OPTIONS",
+						Path:    "",
+						Headers: nil,
+					},
+				},
+				Response: http.Response{
+					StatusCode: 200,
+					Headers: map[string]string{
+						"access-control-allow-origin":   "https://www.foo.com",
+						"access-control-allow-methods":  "GET, OPTIONS",
+						"access-control-allow-headers":  "x-header-1, x-header-2",
+						"access-control-expose-headers": "*",
+					},
+				},
+				Namespace: "",
+			}
+			http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectedResponse)
+		})
+	}
 }