From aa1e61890679fe8fe272244e5507a23f4dfe8c51 Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Tue, 29 Oct 2024 09:10:12 +0800 Subject: [PATCH] fix: validate proto messages before converting them to anypb.Any (#4499) * validate proto message before converting to any Signed-off-by: Huabing Zhao (cherry picked from commit 05817fcc42d803caba384d54eee6d9f0c562c1ef) Signed-off-by: Huabing Zhao --- internal/utils/protocov/protocov.go | 36 +++++------ internal/xds/translator/accesslog.go | 34 +++++++---- internal/xds/translator/authorization.go | 18 +++--- internal/xds/translator/basicauth.go | 5 +- internal/xds/translator/cluster.go | 9 +-- internal/xds/translator/cors.go | 5 +- internal/xds/translator/extauth.go | 7 +-- internal/xds/translator/extproc.go | 7 +-- internal/xds/translator/fault.go | 5 +- internal/xds/translator/healthcheck.go | 6 +- internal/xds/translator/jwt.go | 11 ++-- internal/xds/translator/listener.go | 60 +++++++++++++------ internal/xds/translator/listener_test.go | 19 +++++- internal/xds/translator/local_ratelimit.go | 5 +- internal/xds/translator/oidc.go | 4 +- internal/xds/translator/proxy_protocol.go | 4 +- internal/xds/translator/ratelimit.go | 6 +- .../in/xds-ir/accesslog-without-format.yaml | 3 +- .../testdata/in/xds-ir/accesslog.yaml | 3 +- .../accesslog-without-format.listeners.yaml | 2 + .../out/xds-ir/accesslog.listeners.yaml | 2 + internal/xds/translator/tracing.go | 4 +- internal/xds/translator/translator.go | 25 ++++++-- internal/xds/translator/wasm.go | 9 ++- 24 files changed, 173 insertions(+), 116 deletions(-) diff --git a/internal/utils/protocov/protocov.go b/internal/utils/protocov/protocov.go index 6533f84c543..2c5693ee9a3 100644 --- a/internal/utils/protocov/protocov.go +++ b/internal/utils/protocov/protocov.go @@ -12,30 +12,30 @@ import ( "google.golang.org/protobuf/types/known/anypb" ) -const ( - APIPrefix = "type.googleapis.com/" -) - -var marshalOpts = proto.MarshalOptions{} +// Deprecated: error should not be ignored, use ToAnyWithValidation instead. +func ToAny(msg proto.Message) *anypb.Any { + res, err := ToAnyWithValidation(msg) + if err != nil { + return nil + } + return res +} -func ToAnyWithError(msg proto.Message) (*anypb.Any, error) { +func ToAnyWithValidation(msg proto.Message) (*anypb.Any, error) { if msg == nil { return nil, errors.New("empty message received") } - b, err := marshalOpts.Marshal(msg) - if err != nil { - return nil, err + + // If the message has a ValidateAll method, call it before marshaling. + if validator, ok := msg.(interface{ ValidateAll() error }); ok { + if err := validator.ValidateAll(); err != nil { + return nil, err + } } - return &anypb.Any{ - TypeUrl: APIPrefix + string(msg.ProtoReflect().Descriptor().FullName()), - Value: b, - }, nil -} -func ToAny(msg proto.Message) *anypb.Any { - res, err := ToAnyWithError(msg) + any, err := anypb.New(msg) if err != nil { - return nil + return nil, err } - return res + return any, nil } diff --git a/internal/xds/translator/accesslog.go b/internal/xds/translator/accesslog.go index 01c448b65e9..505537731e1 100644 --- a/internal/xds/translator/accesslog.go +++ b/internal/xds/translator/accesslog.go @@ -22,7 +22,6 @@ import ( "github.com/envoyproxy/go-control-plane/pkg/wellknown" otlpcommonv1 "go.opentelemetry.io/proto/otlp/common/v1" "golang.org/x/exp/maps" - "google.golang.org/protobuf/types/known/anypb" "google.golang.org/protobuf/types/known/structpb" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" @@ -90,9 +89,10 @@ var ( } ) -func buildXdsAccessLog(al *ir.AccessLog, forListener bool) []*accesslog.AccessLog { + +func buildXdsAccessLog(al *ir.AccessLog, forListener bool) ([]*accesslog.AccessLog, error) { if al == nil { - return nil + return nil, nil } totalLen := len(al.Text) + len(al.JSON) + len(al.OpenTelemetry) @@ -124,8 +124,10 @@ func buildXdsAccessLog(al *ir.AccessLog, forListener bool) []*accesslog.AccessLo filelog.GetLogFormat().Formatters = formatters } - // TODO: find a better way to handle this - accesslogAny, _ := anypb.New(filelog) + accesslogAny, err := protocov.ToAnyWithValidation(filelog) + if err != nil { + return nil, err + } accessLogs = append(accessLogs, &accesslog.AccessLog{ Name: wellknown.FileAccessLog, ConfigType: &accesslog.AccessLog_TypedConfig{ @@ -168,7 +170,10 @@ func buildXdsAccessLog(al *ir.AccessLog, forListener bool) []*accesslog.AccessLo filelog.GetLogFormat().Formatters = formatters } - accesslogAny, _ := anypb.New(filelog) + accesslogAny, err := protocov.ToAnyWithValidation(filelog) + if err != nil { + return nil, err + } accessLogs = append(accessLogs, &accesslog.AccessLog{ Name: wellknown.FileAccessLog, ConfigType: &accesslog.AccessLog_TypedConfig{ @@ -203,7 +208,10 @@ func buildXdsAccessLog(al *ir.AccessLog, forListener bool) []*accesslog.AccessLo alCfg.AdditionalResponseTrailersToLog = als.HTTP.ResponseTrailers } - accesslogAny, _ := anypb.New(alCfg) + accesslogAny, err := protocov.ToAnyWithValidation(alCfg) + if err != nil { + return nil, err + } accessLogs = append(accessLogs, &accesslog.AccessLog{ Name: wellknown.HTTPGRPCAccessLog, ConfigType: &accesslog.AccessLog_TypedConfig{ @@ -216,7 +224,10 @@ func buildXdsAccessLog(al *ir.AccessLog, forListener bool) []*accesslog.AccessLo CommonConfig: cc, } - accesslogAny, _ := anypb.New(alCfg) + accesslogAny, err := protocov.ToAnyWithValidation(alCfg) + if err != nil { + return nil, err + } accessLogs = append(accessLogs, &accesslog.AccessLog{ Name: tcpGRPCAccessLog, ConfigType: &accesslog.AccessLog_TypedConfig{ @@ -264,7 +275,10 @@ func buildXdsAccessLog(al *ir.AccessLog, forListener bool) []*accesslog.AccessLo al.Formatters = formatters } - accesslogAny, _ := anypb.New(al) + accesslogAny, err := protocov.ToAnyWithValidation(al) + if err != nil { + return nil, err + } accessLogs = append(accessLogs, &accesslog.AccessLog{ Name: otelAccessLog, ConfigType: &accesslog.AccessLog_TypedConfig{ @@ -274,7 +288,7 @@ func buildXdsAccessLog(al *ir.AccessLog, forListener bool) []*accesslog.AccessLo }) } - return accessLogs + return accessLogs, nil } func celAccessLogFilter(expr string) *accesslog.AccessLogFilter { diff --git a/internal/xds/translator/authorization.go b/internal/xds/translator/authorization.go index cfafb272529..e3977470ab0 100644 --- a/internal/xds/translator/authorization.go +++ b/internal/xds/translator/authorization.go @@ -23,6 +23,7 @@ import ( egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/envoyproxy/gateway/internal/ir" + "github.com/envoyproxy/gateway/internal/utils/protocov" "github.com/envoyproxy/gateway/internal/xds/types" ) @@ -72,7 +73,7 @@ func (*rbac) patchHCM( // buildHCMRBACFilter returns a RBAC filter from the provided IR listener. func buildHCMRBACFilter() (*hcmv3.HttpFilter, error) { rbacProto := &rbacv3.RBAC{} - rbacAny, err := anypb.New(rbacProto) + rbacAny, err := protocov.ToAnyWithValidation(rbacProto) if err != nil { return nil, err } @@ -134,7 +135,7 @@ func (*rbac) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { Name: "ALLOW", Action: rbacconfigv3.RBAC_ALLOW, } - if allowAction, err = anypb.New(allow); err != nil { + if allowAction, err = protocov.ToAnyWithValidation(allow); err != nil { return err } @@ -142,7 +143,7 @@ func (*rbac) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { Name: "DENY", Action: rbacconfigv3.RBAC_DENY, } - if denyAction, err = anypb.New(deny); err != nil { + if denyAction, err = protocov.ToAnyWithValidation(deny); err != nil { return err } @@ -166,11 +167,11 @@ func (*rbac) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { }) } - if ipMatcher, err = anypb.New(ipRangeMatcher); err != nil { + if ipMatcher, err = protocov.ToAnyWithValidation(ipRangeMatcher); err != nil { return err } - if sourceIPInput, err = anypb.New(&networkinput.SourceIPInput{}); err != nil { + if sourceIPInput, err = protocov.ToAnyWithValidation(&networkinput.SourceIPInput{}); err != nil { return err } @@ -243,12 +244,7 @@ func (*rbac) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { routeCfgProto.Rbac.Matcher.MatcherType = nil } - // We need to validate the RBACPerRoute message before converting it to an Any. - if err = routeCfgProto.ValidateAll(); err != nil { - return err - } - - routeCfgAny, err := anypb.New(routeCfgProto) + routeCfgAny, err := protocov.ToAnyWithValidation(routeCfgProto) if err != nil { return err } diff --git a/internal/xds/translator/basicauth.go b/internal/xds/translator/basicauth.go index e22febfca4b..d1219d27f48 100644 --- a/internal/xds/translator/basicauth.go +++ b/internal/xds/translator/basicauth.go @@ -16,6 +16,7 @@ import ( "google.golang.org/protobuf/types/known/anypb" "github.com/envoyproxy/gateway/internal/ir" + "github.com/envoyproxy/gateway/internal/utils/protocov" "github.com/envoyproxy/gateway/internal/xds/types" ) @@ -87,7 +88,7 @@ func buildHCMBasicAuthFilter(basicAuth *ir.BasicAuth) (*hcmv3.HttpFilter, error) if err = basicAuthProto.ValidateAll(); err != nil { return nil, err } - if basicAuthAny, err = anypb.New(basicAuthProto); err != nil { + if basicAuthAny, err = protocov.ToAnyWithValidation(basicAuthProto); err != nil { return nil, err } @@ -137,7 +138,7 @@ func (*basicAuth) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error return err } - if basicAuthAny, err = anypb.New(basicAuthProto); err != nil { + if basicAuthAny, err = protocov.ToAnyWithValidation(basicAuthProto); err != nil { return err } diff --git a/internal/xds/translator/cluster.go b/internal/xds/translator/cluster.go index e646f410944..9620cb29622 100644 --- a/internal/xds/translator/cluster.go +++ b/internal/xds/translator/cluster.go @@ -29,6 +29,7 @@ import ( "k8s.io/utils/ptr" "github.com/envoyproxy/gateway/internal/ir" + "github.com/envoyproxy/gateway/internal/utils/protocov" ) const ( @@ -478,7 +479,7 @@ func buildTypedExtensionProtocolOptions(args *xdsClusterArgs) map[string]*anypb. if args.http1Settings != nil { http1opts.EnableTrailers = args.http1Settings.EnableTrailers if args.http1Settings.PreserveHeaderCase { - preservecaseAny, _ := anypb.New(&preservecasev3.PreserveCaseFormatterConfig{}) + preservecaseAny, _ := protocov.ToAnyWithValidation(&preservecasev3.PreserveCaseFormatterConfig{}) http1opts.HeaderKeyFormat = &corev3.Http1ProtocolOptions_HeaderKeyFormat{ HeaderFormat: &corev3.Http1ProtocolOptions_HeaderKeyFormat_StatefulFormatter{ StatefulFormatter: &corev3.TypedExtensionConfig{ @@ -529,7 +530,7 @@ func buildTypedExtensionProtocolOptions(args *xdsClusterArgs) map[string]*anypb. } } - anyProtocolOptions, _ := anypb.New(&protocolOptions) + anyProtocolOptions, _ := protocov.ToAnyWithValidation(&protocolOptions) extensionOptions := map[string]*anypb.Any{ extensionOptionsKey: anyProtocolOptions, @@ -560,7 +561,7 @@ func buildProxyProtocolSocket(proxyProtocol *ir.ProxyProtocol, tSocket *corev3.T // If existing transport socket does not exist wrap around raw buffer if tSocket == nil { rawCtx := &rawbufferv3.RawBuffer{} - rawCtxAny, err := anypb.New(rawCtx) + rawCtxAny, err := protocov.ToAnyWithValidation(rawCtx) if err != nil { return nil } @@ -575,7 +576,7 @@ func buildProxyProtocolSocket(proxyProtocol *ir.ProxyProtocol, tSocket *corev3.T ppCtx.TransportSocket = tSocket } - ppCtxAny, err := anypb.New(ppCtx) + ppCtxAny, err := protocov.ToAnyWithValidation(ppCtx) if err != nil { return nil } diff --git a/internal/xds/translator/cors.go b/internal/xds/translator/cors.go index cda5ae8a40a..e664e18264b 100644 --- a/internal/xds/translator/cors.go +++ b/internal/xds/translator/cors.go @@ -20,6 +20,7 @@ import ( "google.golang.org/protobuf/types/known/wrapperspb" "github.com/envoyproxy/gateway/internal/ir" + "github.com/envoyproxy/gateway/internal/utils/protocov" "github.com/envoyproxy/gateway/internal/xds/types" ) @@ -70,7 +71,7 @@ func (*cors) patchHCM( func buildHCMCORSFilter() (*hcmv3.HttpFilter, error) { corsProto := &corsv3.Cors{} - corsAny, err := anypb.New(corsProto) + corsAny, err := protocov.ToAnyWithValidation(corsProto) if err != nil { return nil, err } @@ -152,7 +153,7 @@ func (*cors) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { ForwardNotMatchingPreflights: &wrapperspb.BoolValue{Value: false}, } - routeCfgAny, err := anypb.New(routeCfgProto) + routeCfgAny, err := protocov.ToAnyWithValidation(routeCfgProto) if err != nil { return err } diff --git a/internal/xds/translator/extauth.go b/internal/xds/translator/extauth.go index 5da500ddfc9..4ce42468cca 100644 --- a/internal/xds/translator/extauth.go +++ b/internal/xds/translator/extauth.go @@ -14,10 +14,10 @@ import ( extauthv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/ext_authz/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" - "google.golang.org/protobuf/types/known/anypb" "google.golang.org/protobuf/types/known/durationpb" "github.com/envoyproxy/gateway/internal/ir" + "github.com/envoyproxy/gateway/internal/utils/protocov" "github.com/envoyproxy/gateway/internal/xds/types" ) @@ -75,11 +75,8 @@ func (*extAuth) patchHCM(mgr *hcmv3.HttpConnectionManager, irListener *ir.HTTPLi // buildHCMExtAuthFilter returns an ext_authz HTTP filter from the provided IR HTTPRoute. func buildHCMExtAuthFilter(extAuth *ir.ExtAuth) (*hcmv3.HttpFilter, error) { extAuthProto := extAuthConfig(extAuth) - if err := extAuthProto.ValidateAll(); err != nil { - return nil, err - } - extAuthAny, err := anypb.New(extAuthProto) + extAuthAny, err := protocov.ToAnyWithValidation(extAuthProto) if err != nil { return nil, err } diff --git a/internal/xds/translator/extproc.go b/internal/xds/translator/extproc.go index 2eeeb585590..f8978a827f3 100644 --- a/internal/xds/translator/extproc.go +++ b/internal/xds/translator/extproc.go @@ -12,10 +12,10 @@ import ( routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" extprocv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/ext_proc/v3" hcmv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3" - "google.golang.org/protobuf/types/known/anypb" "google.golang.org/protobuf/types/known/durationpb" "github.com/envoyproxy/gateway/internal/ir" + "github.com/envoyproxy/gateway/internal/utils/protocov" "github.com/envoyproxy/gateway/internal/xds/types" ) @@ -72,11 +72,8 @@ func (*extProc) patchHCM(mgr *hcmv3.HttpConnectionManager, irListener *ir.HTTPLi // buildHCMExtProcFilter returns an ext_proc HTTP filter from the provided IR HTTPRoute. func buildHCMExtProcFilter(extProc ir.ExtProc) (*hcmv3.HttpFilter, error) { extAuthProto := extProcConfig(extProc) - if err := extAuthProto.ValidateAll(); err != nil { - return nil, err - } - extAuthAny, err := anypb.New(extAuthProto) + extAuthAny, err := protocov.ToAnyWithValidation(extAuthProto) if err != nil { return nil, err } diff --git a/internal/xds/translator/fault.go b/internal/xds/translator/fault.go index 43fae199c47..13d08d12ed8 100644 --- a/internal/xds/translator/fault.go +++ b/internal/xds/translator/fault.go @@ -19,6 +19,7 @@ import ( "google.golang.org/protobuf/types/known/durationpb" "github.com/envoyproxy/gateway/internal/ir" + "github.com/envoyproxy/gateway/internal/utils/protocov" "github.com/envoyproxy/gateway/internal/xds/types" ) @@ -74,7 +75,7 @@ func buildHCMFaultFilter() (*hcmv3.HttpFilter, error) { return nil, err } - faultAny, err := anypb.New(faultProto) + faultAny, err := protocov.ToAnyWithValidation(faultProto) if err != nil { return nil, err } @@ -168,7 +169,7 @@ func (*fault) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { return nil } - routeCfgAny, err := anypb.New(routeCfgProto) + routeCfgAny, err := protocov.ToAnyWithValidation(routeCfgProto) if err != nil { return err } diff --git a/internal/xds/translator/healthcheck.go b/internal/xds/translator/healthcheck.go index 3356730a6e6..f095b122f63 100644 --- a/internal/xds/translator/healthcheck.go +++ b/internal/xds/translator/healthcheck.go @@ -17,6 +17,7 @@ import ( "google.golang.org/protobuf/types/known/wrapperspb" "github.com/envoyproxy/gateway/internal/ir" + "github.com/envoyproxy/gateway/internal/utils/protocov" "github.com/envoyproxy/gateway/internal/xds/types" ) @@ -82,10 +83,7 @@ func buildHealthCheckFilter(healthCheck *ir.HealthCheckSettings) (*hcmv3.HttpFil }}, } - if err = healthCheckProto.ValidateAll(); err != nil { - return nil, err - } - if healthCheckAny, err = anypb.New(healthCheckProto); err != nil { + if healthCheckAny, err = protocov.ToAnyWithValidation(healthCheckProto); err != nil { return nil, err } diff --git a/internal/xds/translator/jwt.go b/internal/xds/translator/jwt.go index ee6c52326b7..0e6e370d90b 100644 --- a/internal/xds/translator/jwt.go +++ b/internal/xds/translator/jwt.go @@ -22,6 +22,7 @@ import ( egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/envoyproxy/gateway/internal/ir" + "github.com/envoyproxy/gateway/internal/utils/protocov" "github.com/envoyproxy/gateway/internal/xds/types" ) @@ -77,11 +78,7 @@ func buildHCMJWTFilter(irListener *ir.HTTPListener) (*hcmv3.HttpFilter, error) { return nil, err } - if err := jwtAuthnProto.ValidateAll(); err != nil { - return nil, err - } - - jwtAuthnAny, err := anypb.New(jwtAuthnProto) + jwtAuthnAny, err := protocov.ToAnyWithValidation(jwtAuthnProto) if err != nil { return nil, err } @@ -211,7 +208,7 @@ func buildXdsUpstreamTLSSocket(sni string) (*corev3.TransportSocket, error) { }, } - tlsCtxAny, err := anypb.New(tlsCtxProto) + tlsCtxAny, err := protocov.ToAnyWithValidation(tlsCtxProto) if err != nil { return nil, err } @@ -244,7 +241,7 @@ func (*jwt) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { RequirementSpecifier: &jwtauthnv3.PerRouteConfig_RequirementName{RequirementName: irRoute.Name}, } - routeCfgAny, err := anypb.New(routeCfgProto) + routeCfgAny, err := protocov.ToAnyWithValidation(routeCfgProto) if err != nil { return err } diff --git a/internal/xds/translator/listener.go b/internal/xds/translator/listener.go index a19ac7cfadd..cb11cd84d88 100644 --- a/internal/xds/translator/listener.go +++ b/internal/xds/translator/listener.go @@ -25,7 +25,6 @@ import ( "github.com/envoyproxy/go-control-plane/pkg/resource/v3" "github.com/envoyproxy/go-control-plane/pkg/wellknown" "google.golang.org/protobuf/proto" - "google.golang.org/protobuf/types/known/anypb" "google.golang.org/protobuf/types/known/durationpb" "google.golang.org/protobuf/types/known/wrapperspb" "k8s.io/utils/ptr" @@ -62,7 +61,7 @@ func http1ProtocolOptions(opts *ir.HTTP1Settings) *corev3.Http1ProtocolOptions { EnableTrailers: opts.EnableTrailers, } if opts.PreserveHeaderCase { - preservecaseAny, _ := anypb.New(&preservecasev3.PreserveCaseFormatterConfig{}) + preservecaseAny, _ := protocov.ToAnyWithValidation(&preservecasev3.PreserveCaseFormatterConfig{}) r.HeaderKeyFormat = &corev3.Http1ProtocolOptions_HeaderKeyFormat{ HeaderFormat: &corev3.Http1ProtocolOptions_HeaderKeyFormat_StatefulFormatter{ StatefulFormatter: &corev3.TypedExtensionConfig{ @@ -119,7 +118,7 @@ func originalIPDetectionExtensions(clientIPDetection *ir.ClientIPDetectionSettin rejectWithStatus = &typev3.HttpStatus{Code: typev3.StatusCode_Forbidden} } - customHeaderConfigAny, _ := anypb.New(&customheaderv3.CustomHeaderConfig{ + customHeaderConfigAny, _ := protocov.ToAnyWithValidation(&customheaderv3.CustomHeaderConfig{ HeaderName: clientIPDetection.CustomHeader.Name, RejectWithStatus: rejectWithStatus, @@ -137,9 +136,12 @@ func originalIPDetectionExtensions(clientIPDetection *ir.ClientIPDetectionSettin // buildXdsTCPListener creates a xds Listener resource // TODO: Improve function parameters -func buildXdsTCPListener(name, address string, port uint32, keepalive *ir.TCPKeepalive, connection *ir.ClientConnection, accesslog *ir.AccessLog) *listenerv3.Listener { +func buildXdsTCPListener(name, address string, port uint32, keepalive *ir.TCPKeepalive, connection *ir.ClientConnection, accesslog *ir.AccessLog) (*listenerv3.Listener,error) { socketOptions := buildTCPSocketOptions(keepalive) - al := buildXdsAccessLog(accesslog, true) + al,err := buildXdsAccessLog(accesslog, true) + if err != nil { + return nil, err + } bufferLimitBytes := buildPerConnectionBufferLimitBytes(connection) return &listenerv3.Listener{ Name: name, @@ -157,7 +159,7 @@ func buildXdsTCPListener(name, address string, port uint32, keepalive *ir.TCPKee }, }, }, - } + },nil } func buildPerConnectionBufferLimitBytes(connection *ir.ClientConnection) *wrapperspb.UInt32Value { @@ -168,10 +170,14 @@ func buildPerConnectionBufferLimitBytes(connection *ir.ClientConnection) *wrappe } // buildXdsQuicListener creates a xds Listener resource for quic -func buildXdsQuicListener(name, address string, port uint32, accesslog *ir.AccessLog) *listenerv3.Listener { +func buildXdsQuicListener(name, address string, port uint32, accesslog *ir.AccessLog) (*listenerv3.Listener,error) { + al, err := buildXdsAccessLog(accesslog, true) + if err != nil { + return nil, err + } xdsListener := &listenerv3.Listener{ Name: name + "-quic", - AccessLog: buildXdsAccessLog(accesslog, true), + AccessLog: al, Address: &corev3.Address{ Address: &corev3.Address_SocketAddress{ SocketAddress: &corev3.SocketAddress{ @@ -192,7 +198,7 @@ func buildXdsQuicListener(name, address string, port uint32, accesslog *ir.Acces DrainType: listenerv3.Listener_MODIFY_ONLY, } - return xdsListener + return xdsListener, nil } // addHCMToXDSListener adds a HCM filter to the listener's filter chain, and adds @@ -208,7 +214,10 @@ func buildXdsQuicListener(name, address string, port uint32, accesslog *ir.Acces func (t *Translator) addHCMToXDSListener(xdsListener *listenerv3.Listener, irListener *ir.HTTPListener, accesslog *ir.AccessLog, tracing *ir.Tracing, http3Listener bool, connection *ir.ClientConnection, ) error { - al := buildXdsAccessLog(accesslog, false) + al,err := buildXdsAccessLog(accesslog, false) + if err != nil { + return err + } hcmTracing, err := buildHCMTracing(tracing) if err != nil { @@ -407,8 +416,12 @@ func addXdsTCPFilterChain(xdsListener *listenerv3.Listener, irRoute *ir.TCPRoute statPrefix = "terminate" } + al,err := buildXdsAccessLog(accesslog, false) + if err != nil { + return err + } mgr := &tcpv3.TcpProxy{ - AccessLog: buildXdsAccessLog(accesslog, false), + AccessLog: al, StatPrefix: statPrefix, ClusterSpecifier: &tcpv3.TcpProxy_Cluster{ Cluster: clusterName, @@ -492,7 +505,7 @@ func addXdsTLSInspectorFilter(xdsListener *listenerv3.Listener) error { } tlsInspector := &tls_inspectorv3.TlsInspector{} - tlsInspectorAny, err := anypb.New(tlsInspector) + tlsInspectorAny, err := protocov.ToAnyWithValidation(tlsInspector) if err != nil { return err } @@ -538,7 +551,7 @@ func buildDownstreamQUICTransportSocket(tlsConfig *ir.TLSConfig) (*corev3.Transp } } - tlsCtxAny, err := anypb.New(tlsCtx) + tlsCtxAny, err := protocov.ToAnyWithValidation(tlsCtx) if err != nil { return nil, err } @@ -578,7 +591,7 @@ func buildXdsDownstreamTLSSocket(tlsConfig *ir.TLSConfig) (*corev3.TransportSock } } - tlsCtxAny, err := anypb.New(tlsCtx) + tlsCtxAny, err := protocov.ToAnyWithValidation(tlsCtx) if err != nil { return nil, err } @@ -680,14 +693,19 @@ func buildXdsUDPListener(clusterName string, udpListener *ir.UDPListener, access route := &udpv3.Route{ Cluster: clusterName, } - routeAny, err := anypb.New(route) + routeAny, err := protocov.ToAnyWithValidation(route) + if err != nil { + return nil, err + } + + al,err := buildXdsAccessLog(accesslog, false) if err != nil { return nil, err } udpProxy := &udpv3.UdpProxyConfig{ StatPrefix: statPrefix, - AccessLog: buildXdsAccessLog(accesslog, false), + AccessLog: al, RouteSpecifier: &udpv3.UdpProxyConfig_Matcher{ Matcher: &matcher.Matcher{ OnNoMatch: &matcher.Matcher_OnMatch{ @@ -701,14 +719,18 @@ func buildXdsUDPListener(clusterName string, udpListener *ir.UDPListener, access }, }, } - udpProxyAny, err := anypb.New(udpProxy) + udpProxyAny, err := protocov.ToAnyWithValidation(udpProxy) if err != nil { return nil, err } + al,err = buildXdsAccessLog(accesslog, true) + if err != nil { + return nil, err + } xdsListener := &listenerv3.Listener{ Name: udpListener.Name, - AccessLog: buildXdsAccessLog(accesslog, true), + AccessLog: al, Address: &corev3.Address{ Address: &corev3.Address_SocketAddress{ SocketAddress: &corev3.SocketAddress{ @@ -755,7 +777,7 @@ func translateEscapePath(in ir.PathEscapedSlashAction) hcmv3.HttpConnectionManag } func toNetworkFilter(filterName string, filterProto proto.Message) (*listenerv3.Filter, error) { - filterAny, err := protocov.ToAnyWithError(filterProto) + filterAny, err := protocov.ToAnyWithValidation(filterProto) if err != nil { return nil, err } diff --git a/internal/xds/translator/listener_test.go b/internal/xds/translator/listener_test.go index 28572bb06be..fbb716c1ac4 100644 --- a/internal/xds/translator/listener_test.go +++ b/internal/xds/translator/listener_test.go @@ -10,6 +10,7 @@ import ( "reflect" "testing" + routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" hcmv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3" typev3 "github.com/envoyproxy/go-control-plane/envoy/type/v3" "github.com/stretchr/testify/assert" @@ -25,12 +26,24 @@ func Test_toNetworkFilter(t *testing.T) { wantErr error }{ { - name: "valid filter", - proto: &hcmv3.HttpConnectionManager{}, + name: "valid filter", + proto: &hcmv3.HttpConnectionManager{ + StatPrefix: "stats", + RouteSpecifier: &hcmv3.HttpConnectionManager_RouteConfig{ + RouteConfig: &routev3.RouteConfiguration{ + Name: "route", + }, + }, + }, wantErr: nil, }, { name: "invalid proto msg", + proto: &hcmv3.HttpConnectionManager{}, + wantErr: errors.New("invalid HttpConnectionManager.StatPrefix: value length must be at least 1 runes; invalid HttpConnectionManager.RouteSpecifier: value is required"), + }, + { + name: "nil proto msg", proto: nil, wantErr: errors.New("empty message received"), }, @@ -39,7 +52,7 @@ func Test_toNetworkFilter(t *testing.T) { t.Run(tt.name, func(t *testing.T) { _, err := toNetworkFilter("name", tt.proto) if tt.wantErr != nil { - assert.Equalf(t, tt.wantErr, err, "toNetworkFilter(%v)", tt.proto) + assert.Containsf(t, err.Error(), tt.wantErr.Error(), "toNetworkFilter(%v)", tt.proto) } else { assert.NoErrorf(t, err, "toNetworkFilter(%v)", tt.proto) } diff --git a/internal/xds/translator/local_ratelimit.go b/internal/xds/translator/local_ratelimit.go index 231bda44f60..bb768cc464c 100644 --- a/internal/xds/translator/local_ratelimit.go +++ b/internal/xds/translator/local_ratelimit.go @@ -21,6 +21,7 @@ import ( egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/envoyproxy/gateway/internal/ir" + "github.com/envoyproxy/gateway/internal/utils/protocov" "github.com/envoyproxy/gateway/internal/xds/types" ) @@ -62,7 +63,7 @@ func (*localRateLimit) patchHCM(mgr *hcmv3.HttpConnectionManager, irListener *ir StatPrefix: localRateLimitFilterStatPrefix, } - localRlAny, err := anypb.New(localRl) + localRlAny, err := protocov.ToAnyWithValidation(localRl) if err != nil { return err } @@ -176,7 +177,7 @@ func (*localRateLimit) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) e }, } - localRlAny, err := anypb.New(localRl) + localRlAny, err := protocov.ToAnyWithValidation(localRl) if err != nil { return err } diff --git a/internal/xds/translator/oidc.go b/internal/xds/translator/oidc.go index 340caaf4c40..00e89122272 100644 --- a/internal/xds/translator/oidc.go +++ b/internal/xds/translator/oidc.go @@ -16,11 +16,11 @@ import ( tlsv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3" matcherv3 "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" "github.com/golang/protobuf/ptypes/wrappers" - "google.golang.org/protobuf/types/known/anypb" "google.golang.org/protobuf/types/known/durationpb" "k8s.io/utils/ptr" "github.com/envoyproxy/gateway/internal/ir" + "github.com/envoyproxy/gateway/internal/utils/protocov" "github.com/envoyproxy/gateway/internal/xds/types" ) @@ -86,7 +86,7 @@ func buildHCMOAuth2Filter(oidc *ir.OIDC) (*hcmv3.HttpFilter, error) { return nil, err } - OAuth2Any, err := anypb.New(oauth2Proto) + OAuth2Any, err := protocov.ToAnyWithValidation(oauth2Proto) if err != nil { return nil, err } diff --git a/internal/xds/translator/proxy_protocol.go b/internal/xds/translator/proxy_protocol.go index c48fa5e2f8c..c215fb250f5 100644 --- a/internal/xds/translator/proxy_protocol.go +++ b/internal/xds/translator/proxy_protocol.go @@ -6,10 +6,10 @@ package translator import ( + "github.com/envoyproxy/gateway/internal/utils/protocov" listenerv3 "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3" proxyprotocolv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/listener/proxy_protocol/v3" "github.com/envoyproxy/go-control-plane/pkg/wellknown" - "google.golang.org/protobuf/types/known/anypb" ) // patchProxyProtocolFilter builds and appends the Proxy Protocol Filter to the @@ -39,7 +39,7 @@ func patchProxyProtocolFilter(xdsListener *listenerv3.Listener, enableProxyProto func buildProxyProtocolFilter() *listenerv3.ListenerFilter { pp := &proxyprotocolv3.ProxyProtocol{} - ppAny, err := anypb.New(pp) + ppAny, err := protocov.ToAnyWithValidation(pp) if err != nil { return nil } diff --git a/internal/xds/translator/ratelimit.go b/internal/xds/translator/ratelimit.go index 8e3e661f9d7..2b97ff9001b 100644 --- a/internal/xds/translator/ratelimit.go +++ b/internal/xds/translator/ratelimit.go @@ -21,13 +21,13 @@ import ( "github.com/envoyproxy/go-control-plane/pkg/wellknown" rlsconfv3 "github.com/envoyproxy/go-control-plane/ratelimit/config/ratelimit/v3" "github.com/envoyproxy/ratelimit/src/config" - "google.golang.org/protobuf/types/known/anypb" "google.golang.org/protobuf/types/known/durationpb" "google.golang.org/protobuf/types/known/wrapperspb" goyaml "gopkg.in/yaml.v3" // nolint: depguard "k8s.io/utils/ptr" "github.com/envoyproxy/gateway/internal/ir" + "github.com/envoyproxy/gateway/internal/utils/protocov" "github.com/envoyproxy/gateway/internal/xds/types" ) @@ -113,7 +113,7 @@ func (t *Translator) buildRateLimitFilter(irListener *ir.HTTPListener) *hcmv3.Ht rateLimitFilterProto.FailureModeDeny = t.GlobalRateLimit.FailClosed } - rateLimitFilterAny, err := anypb.New(rateLimitFilterProto) + rateLimitFilterAny, err := protocov.ToAnyWithValidation(rateLimitFilterProto) if err != nil { return nil } @@ -451,7 +451,7 @@ func buildRateLimitTLSocket() (*corev3.TransportSocket, error) { } tlsCtx.CommonTlsContext.TlsCertificates = append(tlsCtx.CommonTlsContext.TlsCertificates, tlsCert) - tlsCtxAny, err := anypb.New(tlsCtx) + tlsCtxAny, err := protocov.ToAnyWithValidation(tlsCtx) if err != nil { return nil, err } diff --git a/internal/xds/translator/testdata/in/xds-ir/accesslog-without-format.yaml b/internal/xds/translator/testdata/in/xds-ir/accesslog-without-format.yaml index 40aef558e3e..d2a935ebe83 100644 --- a/internal/xds/translator/testdata/in/xds-ir/accesslog-without-format.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/accesslog-without-format.yaml @@ -11,7 +11,8 @@ accesslog: protocol: "%PROTOCOL%" response_code: "%RESPONSE_CODE%" als: - - destination: + - name: als + destination: name: accesslog/monitoring/envoy-als/port/9000 settings: - addressType: IP diff --git a/internal/xds/translator/testdata/in/xds-ir/accesslog.yaml b/internal/xds/translator/testdata/in/xds-ir/accesslog.yaml index 26f0f5663f8..2ff95c7be91 100644 --- a/internal/xds/translator/testdata/in/xds-ir/accesslog.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/accesslog.yaml @@ -13,7 +13,8 @@ accesslog: protocol: "%PROTOCOL%" response_code: "%RESPONSE_CODE%" als: - - destination: + - name: als + destination: name: accesslog/monitoring/envoy-als/port/9000 settings: - addressType: IP diff --git a/internal/xds/translator/testdata/out/xds-ir/accesslog-without-format.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/accesslog-without-format.listeners.yaml index ee31f03f2d6..87269dcd9c7 100644 --- a/internal/xds/translator/testdata/out/xds-ir/accesslog-without-format.listeners.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/accesslog-without-format.listeners.yaml @@ -43,6 +43,7 @@ grpcService: envoyGrpc: clusterName: accesslog/monitoring/envoy-als/port/9000 + logName: als transportApiVersion: V3 - filter: responseFlagFilter: @@ -119,6 +120,7 @@ grpcService: envoyGrpc: clusterName: accesslog/monitoring/envoy-als/port/9000 + logName: als transportApiVersion: V3 - name: envoy.access_loggers.open_telemetry typedConfig: diff --git a/internal/xds/translator/testdata/out/xds-ir/accesslog.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/accesslog.listeners.yaml index 3bdbdc36126..f8a576c2923 100644 --- a/internal/xds/translator/testdata/out/xds-ir/accesslog.listeners.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/accesslog.listeners.yaml @@ -43,6 +43,7 @@ grpcService: envoyGrpc: clusterName: accesslog/monitoring/envoy-als/port/9000 + logName: als transportApiVersion: V3 - filter: responseFlagFilter: @@ -119,6 +120,7 @@ grpcService: envoyGrpc: clusterName: accesslog/monitoring/envoy-als/port/9000 + logName: als transportApiVersion: V3 - name: envoy.access_loggers.open_telemetry typedConfig: diff --git a/internal/xds/translator/tracing.go b/internal/xds/translator/tracing.go index b2a52ec6a18..4ddcee8ba70 100644 --- a/internal/xds/translator/tracing.go +++ b/internal/xds/translator/tracing.go @@ -57,7 +57,7 @@ func buildHCMTracing(tracing *ir.Tracing) (*hcm.HttpConnectionManager_Tracing, e ServiceName: tracing.ServiceName, } - return protocov.ToAnyWithError(config) + return protocov.ToAnyWithValidation(config) } case egv1a1.TracingProviderTypeZipkin: providerName = envoyZipkin @@ -71,7 +71,7 @@ func buildHCMTracing(tracing *ir.Tracing) (*hcm.HttpConnectionManager_Tracing, e CollectorEndpointVersion: tracecfg.ZipkinConfig_HTTP_JSON, } - return protocov.ToAnyWithError(config) + return protocov.ToAnyWithValidation(config) } default: return nil, fmt.Errorf("unknown tracing provider type: %s", tracing.Provider.Type) diff --git a/internal/xds/translator/translator.go b/internal/xds/translator/translator.go index a58903e88ad..11edb94bacb 100644 --- a/internal/xds/translator/translator.go +++ b/internal/xds/translator/translator.go @@ -217,7 +217,11 @@ func (t *Translator) processHTTPListenerXdsTranslation( case !xdsListenerOnSameAddressPortExists: // Create a new UDP(QUIC) listener for HTTP3 traffic if HTTP3 is enabled if http3Enabled { - quicXDSListener = buildXdsQuicListener(httpListener.Name, httpListener.Address, httpListener.Port, accessLog) + if quicXDSListener, err = buildXdsQuicListener(httpListener.Name, httpListener.Address, httpListener.Port, accessLog); err != nil { + errs = errors.Join(errs, err) + continue + } + if err = tCtx.AddXdsResource(resourcev3.ListenerType, quicXDSListener); err != nil { errs = errors.Join(errs, err) continue @@ -225,7 +229,11 @@ func (t *Translator) processHTTPListenerXdsTranslation( } // Create a new TCP listener for HTTP1/HTTP2 traffic. - tcpXDSListener = buildXdsTCPListener(httpListener.Name, httpListener.Address, httpListener.Port, httpListener.TCPKeepalive, httpListener.Connection, accessLog) + if tcpXDSListener, err = buildXdsTCPListener(httpListener.Name, httpListener.Address, httpListener.Port, httpListener.TCPKeepalive, httpListener.Connection, accessLog); err != nil { + errs = errors.Join(errs, err) + continue + } + if err = tCtx.AddXdsResource(resourcev3.ListenerType, tcpXDSListener); err != nil { errs = errors.Join(errs, err) continue @@ -506,7 +514,7 @@ func (t *Translator) addHTTPFiltersToHCM(filterChain *listenerv3.FilterChain, ht for i, filter := range filterChain.Filters { if filter.Name == wellknown.HTTPConnectionManager { var mgrAny *anypb.Any - if mgrAny, err = protocov.ToAnyWithError(hcm); err != nil { + if mgrAny, err = protocov.ToAnyWithValidation(hcm); err != nil { return err } @@ -552,12 +560,17 @@ func (t *Translator) processTCPListenerXdsTranslation( ) error { // The XDS translation is done in a best-effort manner, so we collect all // errors and return them at the end. - var errs error + var errs, err error for _, tcpListener := range tcpListeners { // Search for an existing listener, if it does not exist, create one. xdsListener := findXdsListenerByHostPort(tCtx, tcpListener.Address, tcpListener.Port, corev3.SocketAddress_TCP) if xdsListener == nil { - xdsListener = buildXdsTCPListener(tcpListener.Name, tcpListener.Address, tcpListener.Port, tcpListener.TCPKeepalive, tcpListener.Connection, accesslog) + if xdsListener, err = buildXdsTCPListener(tcpListener.Name, tcpListener.Address, tcpListener.Port, tcpListener.TCPKeepalive, tcpListener.Connection, accesslog); err != nil { + // skip this listener if failed to build xds listener + errs = errors.Join(errs, err) + continue + } + if err := tCtx.AddXdsResource(resourcev3.ListenerType, xdsListener); err != nil { // skip this listener if failed to add xds listener to the errs = errors.Join(errs, err) @@ -947,7 +960,7 @@ func buildXdsUpstreamTLSSocketWthCert(tlsConfig *ir.TLSUpstreamConfig) (*corev3. } } - tlsCtxAny, err := anypb.New(tlsCtx) + tlsCtxAny, err := protocov.ToAnyWithValidation(tlsCtx) if err != nil { return nil, err } diff --git a/internal/xds/translator/wasm.go b/internal/xds/translator/wasm.go index 4d6434c95d3..977174b0f04 100644 --- a/internal/xds/translator/wasm.go +++ b/internal/xds/translator/wasm.go @@ -18,6 +18,7 @@ import ( "google.golang.org/protobuf/types/known/wrapperspb" "github.com/envoyproxy/gateway/internal/ir" + "github.com/envoyproxy/gateway/internal/utils/protocov" "github.com/envoyproxy/gateway/internal/xds/types" ) @@ -80,10 +81,8 @@ func buildHCMWasmFilter(wasm ir.Wasm) (*hcmv3.HttpFilter, error) { if wasmProto, err = wasmConfig(wasm); err != nil { return nil, err } - if err = wasmProto.ValidateAll(); err != nil { - return nil, err - } - if wasmAny, err = anypb.New(wasmProto); err != nil { + + if wasmAny, err = protocov.ToAnyWithValidation(wasmProto); err != nil { return nil, err } @@ -114,7 +113,7 @@ func wasmConfig(wasm ir.Wasm) (*wasmfilterv3.Wasm, error) { pluginConfig = string(wasm.Config.Raw) } - if configAny, err = anypb.New(wrapperspb.String(pluginConfig)); err != nil { + if configAny, err = protocov.ToAnyWithValidation(wrapperspb.String(pluginConfig)); err != nil { return nil, err }