address comments

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

internal/ir/xds.go

@@ -279,7 +279,7 @@ type HTTPRoute struct {
 	// Timeout is the time until which entire response is received from the upstream.
 	Timeout *metav1.Duration `json:"timeout,omitempty" yaml:"timeout,omitempty"`
 	// Cors policy for the route.
-	CorsPolicy *CorsPolicy `json:"corsPolicy,omitempty" yaml:"corsPolicy,omitempty"`
+	Cors *Cors `json:"cors,omitempty" yaml:"cors,omitempty"`
 	// ExtensionRefs holds unstructured resources that were introduced by an extension and used on the HTTPRoute as extensionRef filters
 	ExtensionRefs []*UnstructuredRef `json:"extensionRefs,omitempty" yaml:"extensionRefs,omitempty"`
 }
@@ -313,30 +313,27 @@ type JwtRequestAuthentication struct {
 	Providers []egv1a1.JwtAuthenticationFilterProvider `json:"providers,omitempty" yaml:"providers,omitempty"`
 }
 
-// CorsPolicy holds the Cross-Origin Resource Sharing (CORS) policy for the route.
+// Cors holds the Cross-Origin Resource Sharing (CORS) policy for the route.
 //
 // +k8s:deepcopy-gen=true
-type CorsPolicy struct {
+type Cors struct {
 	// AllowOrigins defines the origins that are allowed to make requests.
 	AllowOrigins []*StringMatch `json:"allowOrigins,omitempty" yaml:"allowOrigins,omitempty"`
 	// AllowMethods defines the methods that are allowed to make requests.
 	AllowMethods []string `json:"allowMethods,omitempty" yaml:"allowMethods,omitempty"`
 	// AllowHeaders defines the headers that are allowed to be sent with requests.
-	AllowedHeaders []string `json:"allowedHeaders,omitempty" yaml:"allowedHeaders,omitempty"`
+	AllowHeaders []string `json:"allowHeaders,omitempty" yaml:"allowHeaders,omitempty"`
 	// ExposeHeaders defines the headers that can be exposed in the responses.
-	ExposedHeaders []string `json:"exposedHeaders,omitempty" yaml:"exposedHeaders,omitempty"`
+	ExposeHeaders []string `json:"exposeHeaders,omitempty" yaml:"exposeHeaders,omitempty"`
 	// MaxAge defines how long the results of a preflight request can be cached.
 	MaxAge *metav1.Duration `json:"maxAge,omitempty" yaml:"maxAge,omitempty"`
-	// AllowCredentials defines whether the resource allows credentials.
-	// Defaults to false.
-	AllowCredentials bool `json:"allowCredentials,omitempty" yaml:"allowCredentials,omitempty"`
 	// AllowPrivateNetwork defines whether allow whose target server’s IP address
 	// is more private than that from which the request initiator was fetched.
 	// Defaults to false.
 	AllowPrivateNetworkAccess bool `json:"allowPrivateNetwork,omitempty" yaml:"allowPrivateNetwork,omitempty"`
 }
 
-func (c *CorsPolicy) Validate() error {
+func (c *Cors) Validate() error {
 	var errs error
 
 	if len(c.AllowOrigins) == 0 {
@@ -460,8 +457,8 @@ func (h HTTPRoute) Validate() error {
 			}
 		}
 	}
-	if h.CorsPolicy != nil {
-		if err := h.CorsPolicy.Validate(); err != nil {
+	if h.Cors != nil {
+		if err := h.Cors.Validate(); err != nil {
 			errs = multierror.Append(errs, err)
 		}
 	}

internal/xds/translator/cors.go

@@ -15,18 +15,15 @@ import (
 	corsv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/cors/v3"
 	hcmv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3"
 	matcherv3 "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3"
+	"github.com/envoyproxy/go-control-plane/pkg/wellknown"
 	"github.com/golang/protobuf/ptypes/wrappers"
 	"google.golang.org/protobuf/types/known/anypb"
 
 	"github.com/envoyproxy/gateway/internal/ir"
 )
 
-const (
-	corsFilter = "envoy.filters.http.cors"
-)
-
 // patchHCMWithCorsFilter builds and appends the Cors Filter to the HTTP
-// Connection Manager if applicable, and it does not already exist.
+// Connection Manager if applicable.
 func patchHCMWithCorsFilter(mgr *hcmv3.HttpConnectionManager, irListener *ir.HTTPListener) error {
 	if mgr == nil {
 		return errors.New("hcm is nil")
@@ -36,30 +33,30 @@ func patchHCMWithCorsFilter(mgr *hcmv3.HttpConnectionManager, irListener *ir.HTT
 		return errors.New("ir listener is nil")
 	}
 
-	if !listenerContainsCorsPolicy(irListener) {
+	if !listenerContainsCors(irListener) {
 		return nil
 	}
 
 	// Return early if filter already exists.
 	for _, httpFilter := range mgr.HttpFilters {
-		if httpFilter.Name == corsFilter {
+		if httpFilter.Name == wellknown.CORS {
 			return nil
 		}
 	}
 
-	corsFilter, err := buildHCMCorsFilter(irListener)
+	corsFilter, err := buildHCMCorsFilter()
 	if err != nil {
 		return err
 	}
 
-	// Ensure the cors filter is the first one in the chain.
+	// Ensure the cors filter is the first one in the filter chain.
 	mgr.HttpFilters = append([]*hcmv3.HttpFilter{corsFilter}, mgr.HttpFilters...)
 
 	return nil
 }
 
 // buildHCMCorsFilter returns a Cors filter from the provided IR listener.
-func buildHCMCorsFilter(irListener *ir.HTTPListener) (*hcmv3.HttpFilter, error) {
+func buildHCMCorsFilter() (*hcmv3.HttpFilter, error) {
 	corsProto := &corsv3.Cors{}
 
 	corsAny, err := anypb.New(corsProto)
@@ -68,22 +65,22 @@ func buildHCMCorsFilter(irListener *ir.HTTPListener) (*hcmv3.HttpFilter, error)
 	}
 
 	return &hcmv3.HttpFilter{
-		Name: corsFilter,
+		Name: wellknown.CORS,
 		ConfigType: &hcmv3.HttpFilter_TypedConfig{
 			TypedConfig: corsAny,
 		},
 	}, nil
 }
 
-// listenerContainsCorsPolicy returns true if the provided listener has Cors
+// listenerContainsCors returns true if the provided listener has Cors
 // policies attached to its routes.
-func listenerContainsCorsPolicy(irListener *ir.HTTPListener) bool {
+func listenerContainsCors(irListener *ir.HTTPListener) bool {
 	if irListener == nil {
 		return false
 	}
 
 	for _, route := range irListener.Routes {
-		if route.CorsPolicy != nil {
+		if route.Cors != nil {
 			return true
 		}
 	}
@@ -100,12 +97,14 @@ func patchRouteWithCorsConfig(route *routev3.Route, irRoute *ir.HTTPRoute) error
 	if irRoute == nil {
 		return errors.New("ir route is nil")
 	}
-	if irRoute.CorsPolicy == nil {
+	if irRoute.Cors == nil {
 		return nil
 	}
 
 	filterCfg := route.GetTypedPerFilterConfig()
-	if _, ok := filterCfg[corsFilter]; ok {
+	if _, ok := filterCfg[wellknown.CORS]; ok {
+		// This should not happen since this is the only place where the cors
+		// filter is added in a route.
 		return fmt.Errorf("route already contains cors config: %+v", route)
 	}
 
@@ -119,42 +118,17 @@ func patchRouteWithCorsConfig(route *routev3.Route, irRoute *ir.HTTPRoute) error
 		allowPrivateNetworkAccess *wrappers.BoolValue
 	)
 
-	for _, origin := range irRoute.CorsPolicy.AllowOrigins {
-		if origin.Exact != nil {
-			allowOrigins = append(allowOrigins, &matcherv3.StringMatcher{
-				MatchPattern: &matcherv3.StringMatcher_Exact{
-					Exact: *origin.Exact,
-				},
-			})
-		} else if origin.Prefix != nil {
-			allowOrigins = append(allowOrigins, &matcherv3.StringMatcher{
-				MatchPattern: &matcherv3.StringMatcher_Prefix{
-					Prefix: *origin.Prefix,
-				},
-			})
-		} else if origin.SafeRegex != nil {
-			allowOrigins = append(allowOrigins, &matcherv3.StringMatcher{
-				MatchPattern: &matcherv3.StringMatcher_SafeRegex{
-					SafeRegex: &matcherv3.RegexMatcher{
-						Regex: *origin.SafeRegex,
-					},
-				},
-			})
-		} else if origin.Suffix != nil {
-			allowOrigins = append(allowOrigins, &matcherv3.StringMatcher{
-				MatchPattern: &matcherv3.StringMatcher_Suffix{
-					Suffix: *origin.Suffix,
-				},
-			})
-		}
+	//nolint:gocritic
+
+	for _, origin := range irRoute.Cors.AllowOrigins {
+		allowOrigins = append(allowOrigins, buildXdsStringMatcher(origin))
 	}
 
-	allowMethods = strings.Join(irRoute.CorsPolicy.AllowMethods, " ,")
-	allowHeaders = strings.Join(irRoute.CorsPolicy.AllowedHeaders, " ,")
-	exposeHeaders = strings.Join(irRoute.CorsPolicy.ExposedHeaders, " ,")
-	maxAge = strconv.Itoa(int(irRoute.CorsPolicy.MaxAge.Seconds()))
-	allowPrivateNetworkAccess = &wrappers.BoolValue{Value: irRoute.CorsPolicy.AllowPrivateNetworkAccess}
-	allowCredentials = &wrappers.BoolValue{Value: irRoute.CorsPolicy.AllowCredentials}
+	allowMethods = strings.Join(irRoute.Cors.AllowMethods, ", ")
+	allowHeaders = strings.Join(irRoute.Cors.AllowHeaders, ", ")
+	exposeHeaders = strings.Join(irRoute.Cors.ExposeHeaders, ", ")
+	maxAge = strconv.Itoa(int(irRoute.Cors.MaxAge.Seconds()))
+	allowPrivateNetworkAccess = &wrappers.BoolValue{Value: irRoute.Cors.AllowPrivateNetworkAccess}
 
 	routeCfgProto := &corsv3.CorsPolicy{
 		AllowOriginStringMatch:    allowOrigins,
@@ -175,7 +149,7 @@ func patchRouteWithCorsConfig(route *routev3.Route, irRoute *ir.HTTPRoute) error
 		route.TypedPerFilterConfig = make(map[string]*anypb.Any)
 	}
 
-	route.TypedPerFilterConfig[corsFilter] = routeCfgAny
+	route.TypedPerFilterConfig[wellknown.CORS] = routeCfgAny
 
 	return nil
 }

internal/xds/translator/listener.go

@@ -134,8 +134,8 @@ func (t *Translator) addXdsHTTPFilterChain(xdsListener *listenerv3.Listener, irL
 		}
 	}
 
-	// Add the HTTP filters to the HCM, the filters have been sorted in the
-	// correct order.
+	// Add HTTP filters to the HCM, the filters have already been sorted in the
+	// correct order in the patchHCMWithFilters function.
 	if err := t.patchHCMWithFilters(mgr, irListener); err != nil {
 		return err
 	}

internal/xds/translator/testdata/in/xds-ir/cors.yaml

@@ -0,0 +1,36 @@
+http:
+- name: "first-listener"
+  address: "0.0.0.0"
+  port: 10080
+  hostnames:
+  - "*"
+  routes:
+  - name: "first-route"
+    hostname: "*"
+    pathMatch:
+      exact: "foo/bar"
+    destination:
+      name: "first-route-dest"
+      settings:
+      - endpoints:
+        - host: "1.2.3.4"
+          port: 50000
+    cors:
+      allowOrigins:
+      - name: example.com
+        stringMatch:
+        safeRegex: "*.example.com"
+      - name: foo.bar.com
+        stringMatch:
+        exact: foo.bar.com
+      allowMethods:
+      - GET
+      - POST
+      allowHeaders:
+      - "x-header-1"
+      - "x-header-2"
+      exposeHeaders:
+      - "x-header-3"
+      - "x-header-4"
+      maxAge: 1000s
+      allowPrivateNetworkAccess: false

internal/xds/translator/testdata/out/xds-ir/cors.routes.yaml

@@ -0,0 +1,24 @@
+- ignorePortInHostMatching: true
+  name: first-listener
+  virtualHosts:
+  - domains:
+    - '*'
+    name: first-listener/*
+    routes:
+    - match:
+        path: foo/bar
+      name: first-route
+      route:
+        cluster: first-route-dest
+      typedPerFilterConfig:
+        envoy.filters.http.cors:
+          '@type': type.googleapis.com/envoy.extensions.filters.http.cors.v3.CorsPolicy
+          allowHeaders: x-header-1, x-header-2
+          allowMethods: GET, POST
+          allowOriginStringMatch:
+          - safeRegex:
+              regex: '*.example.com'
+          - exact: foo.bar.com
+          allowPrivateNetworkAccess: false
+          exposeHeaders: x-header-3, x-header-4
+          maxAge: "1000"