From 56f28bfb63b78e16ea5da79ab32571cdcd324b25 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Fri, 20 Oct 2023 08:39:43 +0800 Subject: [PATCH] address comments Signed-off-by: huabing zhao --- internal/ir/xds.go | 15 +++++------- internal/ir/zz_generated.deepcopy.go | 14 +++++------ internal/xds/translator/cors.go | 23 +++++++++---------- .../translator/testdata/in/xds-ir/cors.yaml | 3 +-- .../testdata/out/xds-ir/cors.routes.yaml | 1 - 5 files changed, 25 insertions(+), 31 deletions(-) diff --git a/internal/ir/xds.go b/internal/ir/xds.go index bdfb9e837524..08d303dd9e9b 100644 --- a/internal/ir/xds.go +++ b/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,10 +313,10 @@ 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. @@ -327,16 +327,13 @@ type CorsPolicy struct { 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) } } diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go index 237008867399..ef6987cfb7de 100644 --- a/internal/ir/zz_generated.deepcopy.go +++ b/internal/ir/zz_generated.deepcopy.go @@ -78,7 +78,7 @@ func (in *AddHeader) DeepCopy() *AddHeader { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *CorsPolicy) DeepCopyInto(out *CorsPolicy) { +func (in *Cors) DeepCopyInto(out *Cors) { *out = *in if in.AllowOrigins != nil { in, out := &in.AllowOrigins, &out.AllowOrigins @@ -113,12 +113,12 @@ func (in *CorsPolicy) DeepCopyInto(out *CorsPolicy) { } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CorsPolicy. -func (in *CorsPolicy) DeepCopy() *CorsPolicy { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Cors. +func (in *Cors) DeepCopy() *Cors { if in == nil { return nil } - out := new(CorsPolicy) + out := new(Cors) in.DeepCopyInto(out) return out } @@ -431,9 +431,9 @@ func (in *HTTPRoute) DeepCopyInto(out *HTTPRoute) { *out = new(v1.Duration) **out = **in } - if in.CorsPolicy != nil { - in, out := &in.CorsPolicy, &out.CorsPolicy - *out = new(CorsPolicy) + if in.Cors != nil { + in, out := &in.Cors, &out.Cors + *out = new(Cors) (*in).DeepCopyInto(*out) } if in.ExtensionRefs != nil { diff --git a/internal/xds/translator/cors.go b/internal/xds/translator/cors.go index 489e7668d691..44f24a0ca5ed 100644 --- a/internal/xds/translator/cors.go +++ b/internal/xds/translator/cors.go @@ -33,7 +33,7 @@ func patchHCMWithCorsFilter(mgr *hcmv3.HttpConnectionManager, irListener *ir.HTT return errors.New("ir listener is nil") } - if !listenerContainsCorsPolicy(irListener) { + if !listenerContainsCors(irListener) { return nil } @@ -74,15 +74,15 @@ func buildHCMCorsFilter() (*hcmv3.HttpFilter, error) { }, 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 } } @@ -99,7 +99,7 @@ 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 } @@ -121,7 +121,7 @@ func patchRouteWithCorsConfig(route *routev3.Route, irRoute *ir.HTTPRoute) error ) //nolint:gocritic - for _, origin := range irRoute.CorsPolicy.AllowOrigins { + for _, origin := range irRoute.Cors.AllowOrigins { if origin.Exact != nil { allowOrigins = append(allowOrigins, &matcherv3.StringMatcher{ MatchPattern: &matcherv3.StringMatcher_Exact{ @@ -151,12 +151,11 @@ func patchRouteWithCorsConfig(route *routev3.Route, irRoute *ir.HTTPRoute) error } } - allowMethods = strings.Join(irRoute.CorsPolicy.AllowMethods, ", ") - allowHeaders = strings.Join(irRoute.CorsPolicy.AllowHeaders, ", ") - exposeHeaders = strings.Join(irRoute.CorsPolicy.ExposeHeaders, ", ") - 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, diff --git a/internal/xds/translator/testdata/in/xds-ir/cors.yaml b/internal/xds/translator/testdata/in/xds-ir/cors.yaml index 4bbe3d7be843..d7dc9f71f7a7 100644 --- a/internal/xds/translator/testdata/in/xds-ir/cors.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/cors.yaml @@ -15,7 +15,7 @@ http: - endpoints: - host: "1.2.3.4" port: 50000 - corsPolicy: + cors: allowOrigins: - name: example.com stringMatch: @@ -33,5 +33,4 @@ http: - "x-header-3" - "x-header-4" maxAge: 1000s - allowCredentials: true allowPrivateNetworkAccess: false diff --git a/internal/xds/translator/testdata/out/xds-ir/cors.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/cors.routes.yaml index cb072412df7a..ad9f9f2f3082 100755 --- a/internal/xds/translator/testdata/out/xds-ir/cors.routes.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/cors.routes.yaml @@ -13,7 +13,6 @@ typedPerFilterConfig: envoy.filters.http.cors: '@type': type.googleapis.com/envoy.extensions.filters.http.cors.v3.CorsPolicy - allowCredentials: true allowHeaders: x-header-1, x-header-2 allowMethods: GET, POST allowOriginStringMatch: