From 0bdc1abc0519aef29198cbf253d512ec2fd7e44d 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 | 33 ++++++--- internal/xds/translator/authorization.go | 26 +++---- internal/xds/translator/basicauth.go | 5 +- internal/xds/translator/cluster.go | 9 +-- internal/xds/translator/custom_response.go | 11 +-- internal/xds/translator/fault.go | 5 +- internal/xds/translator/jwt.go | 11 ++- internal/xds/translator/listener.go | 68 +++++++++++++------ internal/xds/translator/listener_test.go | 19 +++++- internal/xds/translator/oidc.go | 4 +- .../in/xds-ir/accesslog-without-format.yaml | 3 +- .../testdata/in/xds-ir/accesslog.yaml | 3 +- .../authorization-multiple-principals.yaml | 4 +- .../accesslog-without-format.listeners.yaml | 2 + .../out/xds-ir/accesslog.listeners.yaml | 2 + ...horization-multiple-principals.routes.yaml | 20 +++--- internal/xds/translator/tracing.go | 6 +- internal/xds/translator/translator.go | 29 ++++++-- 19 files changed, 184 insertions(+), 112 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 6660ba8fab6..076eb659d83 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,9 @@ var ( } ) -func buildXdsAccessLog(al *ir.AccessLog, accessLogType ir.ProxyAccessLogType) []*accesslog.AccessLog { +func buildXdsAccessLog(al *ir.AccessLog, accessLogType ir.ProxyAccessLogType) ([]*accesslog.AccessLog, error) { if al == nil { - return nil + return nil, nil } totalLen := len(al.Text) + len(al.JSON) + len(al.OpenTelemetry) @@ -133,8 +132,10 @@ func buildXdsAccessLog(al *ir.AccessLog, accessLogType ir.ProxyAccessLogType) [] 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{ @@ -185,7 +186,10 @@ func buildXdsAccessLog(al *ir.AccessLog, accessLogType ir.ProxyAccessLogType) [] 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{ @@ -228,7 +232,10 @@ func buildXdsAccessLog(al *ir.AccessLog, accessLogType ir.ProxyAccessLogType) [] 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{ @@ -241,7 +248,10 @@ func buildXdsAccessLog(al *ir.AccessLog, accessLogType ir.ProxyAccessLogType) [] 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{ @@ -297,7 +307,10 @@ func buildXdsAccessLog(al *ir.AccessLog, accessLogType ir.ProxyAccessLogType) [] 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{ @@ -307,7 +320,7 @@ func buildXdsAccessLog(al *ir.AccessLog, accessLogType ir.ProxyAccessLogType) [] }) } - 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 0d2d19dc571..e19d1dbaf53 100644 --- a/internal/xds/translator/authorization.go +++ b/internal/xds/translator/authorization.go @@ -26,6 +26,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" ) @@ -75,7 +76,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 } @@ -133,7 +134,7 @@ func (*rbac) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { return err } - if cfgAny, err = anypb.New(rbacPerRoute); err != nil { + if cfgAny, err = protocov.ToAnyWithValidation(rbacPerRoute); err != nil { return err } @@ -159,7 +160,7 @@ func buildRBACPerRoute(authorization *ir.Authorization) (*rbacv3.RBACPerRoute, e Name: "ALLOW", Action: rbacconfigv3.RBAC_ALLOW, } - if allowAction, err = anypb.New(allow); err != nil { + if allowAction, err = protocov.ToAnyWithValidation(allow); err != nil { return nil, err } @@ -167,7 +168,7 @@ func buildRBACPerRoute(authorization *ir.Authorization) (*rbacv3.RBACPerRoute, e Name: "DENY", Action: rbacconfigv3.RBAC_DENY, } - if denyAction, err = anypb.New(deny); err != nil { + if denyAction, err = protocov.ToAnyWithValidation(deny); err != nil { return nil, err } @@ -287,11 +288,6 @@ func buildRBACPerRoute(authorization *ir.Authorization) (*rbacv3.RBACPerRoute, e rbac.Rbac.Matcher.MatcherType = nil } - // We need to validate the RBACPerRoute message before converting it to an Any. - if err = rbac.ValidateAll(); err != nil { - return nil, err - } - return rbac, nil } @@ -316,11 +312,11 @@ func buildIPPredicate(clientCIDRs []*ir.CIDRMatch) (*matcherv3.Matcher_MatcherLi }) } - if ipMatcher, err = anypb.New(ipRangeMatcher); err != nil { + if ipMatcher, err = protocov.ToAnyWithValidation(ipRangeMatcher); err != nil { return nil, err } - if sourceIPInput, err = anypb.New(&networkinput.SourceIPInput{}); err != nil { + if sourceIPInput, err = protocov.ToAnyWithValidation(&networkinput.SourceIPInput{}); err != nil { return nil, err } @@ -389,11 +385,11 @@ func buildJWTPredicate(jwt egv1a1.JWTPrincipal) ([]*matcherv3.Matcher_MatcherLis }, } - if inputPb, err = anypb.New(input); err != nil { + if inputPb, err = protocov.ToAnyWithValidation(input); err != nil { return nil, err } - if matcherPb, err = anypb.New(scopeMatcher); err != nil { + if matcherPb, err = protocov.ToAnyWithValidation(scopeMatcher); err != nil { return nil, err } @@ -454,7 +450,7 @@ func buildJWTPredicate(jwt egv1a1.JWTPrincipal) ([]*matcherv3.Matcher_MatcherLis Path: path, } - if inputPb, err = anypb.New(input); err != nil { + if inputPb, err = protocov.ToAnyWithValidation(input); err != nil { return nil, err } @@ -492,7 +488,7 @@ func buildJWTPredicate(jwt egv1a1.JWTPrincipal) ([]*matcherv3.Matcher_MatcherLis } } - if matcherPb, err = anypb.New(&metadatav3.Metadata{ + if matcherPb, err = protocov.ToAnyWithValidation(&metadatav3.Metadata{ Value: valueMatcher, }); err != nil { return nil, err diff --git a/internal/xds/translator/basicauth.go b/internal/xds/translator/basicauth.go index 50c4935140b..31a421ae8a9 100644 --- a/internal/xds/translator/basicauth.go +++ b/internal/xds/translator/basicauth.go @@ -17,6 +17,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" ) @@ -84,7 +85,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 } @@ -134,7 +135,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 145d616bde7..2a182ce5b75 100644 --- a/internal/xds/translator/cluster.go +++ b/internal/xds/translator/cluster.go @@ -30,6 +30,7 @@ import ( egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/envoyproxy/gateway/internal/ir" + "github.com/envoyproxy/gateway/internal/utils/protocov" ) const ( @@ -509,7 +510,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{ @@ -562,7 +563,7 @@ func buildTypedExtensionProtocolOptions(args *xdsClusterArgs) map[string]*anypb. } } - anyProtocolOptions, _ := anypb.New(&protocolOptions) + anyProtocolOptions, _ := protocov.ToAnyWithValidation(&protocolOptions) extensionOptions := map[string]*anypb.Any{ extensionOptionsKey: anyProtocolOptions, @@ -593,7 +594,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 } @@ -608,7 +609,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/custom_response.go b/internal/xds/translator/custom_response.go index e5d48d21bfd..6cca67982e9 100644 --- a/internal/xds/translator/custom_response.go +++ b/internal/xds/translator/custom_response.go @@ -24,6 +24,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" ) @@ -85,7 +86,7 @@ func (c *customResponse) buildHCMCustomResponseFilter(ro *ir.ResponseOverride) ( return nil, err } - any, err := anypb.New(proto) + any, err := protocov.ToAnyWithValidation(proto) if err != nil { return nil, err } @@ -237,7 +238,7 @@ func (c *customResponse) buildHTTPAttributeCELInput() (*cncfv3.TypedExtensionCon err error ) - if pb, err = anypb.New(&matcherv3.HttpAttributesCelMatchInput{}); err != nil { + if pb, err = protocov.ToAnyWithValidation(&matcherv3.HttpAttributesCelMatchInput{}); err != nil { return nil, err } @@ -253,7 +254,7 @@ func (c *customResponse) buildStatusCodeInput() (*cncfv3.TypedExtensionConfig, e err error ) - if pb, err = anypb.New(&envoymatcherv3.HttpResponseStatusCodeMatchInput{}); err != nil { + if pb, err = protocov.ToAnyWithValidation(&envoymatcherv3.HttpResponseStatusCodeMatchInput{}); err != nil { return nil, err } @@ -364,7 +365,7 @@ func (c *customResponse) buildStatusCodeCELMatcher(codeRange ir.StatusCodeRange) return nil, err } - if pb, err = anypb.New(matcher); err != nil { + if pb, err = protocov.ToAnyWithValidation(matcher); err != nil { return nil, err } @@ -403,7 +404,7 @@ func (c *customResponse) buildAction(r ir.ResponseOverrideRule) (*matcherv3.Matc return nil, err } - if pb, err = anypb.New(response); err != nil { + if pb, err = protocov.ToAnyWithValidation(response); err != nil { return nil, err } diff --git a/internal/xds/translator/fault.go b/internal/xds/translator/fault.go index e0acbd6c840..192ce5bf8e9 100644 --- a/internal/xds/translator/fault.go +++ b/internal/xds/translator/fault.go @@ -20,6 +20,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" ) @@ -71,7 +72,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 } @@ -165,7 +166,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/jwt.go b/internal/xds/translator/jwt.go index 53a20808ff6..f3f16b20c6f 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" ) @@ -76,11 +77,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 } @@ -214,7 +211,7 @@ func buildXdsUpstreamTLSSocket(sni string) (*corev3.TransportSocket, error) { }, } - tlsCtxAny, err := anypb.New(tlsCtxProto) + tlsCtxAny, err := protocov.ToAnyWithValidation(tlsCtxProto) if err != nil { return nil, err } @@ -247,7 +244,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 c855d3ddf92..9a68c5f3c1f 100644 --- a/internal/xds/translator/listener.go +++ b/internal/xds/translator/listener.go @@ -29,7 +29,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" @@ -66,7 +65,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{ @@ -131,7 +130,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, @@ -179,9 +178,19 @@ func setAddressByIPFamily(socketAddress *corev3.SocketAddress, ipFamily *ir.IPFa // buildXdsTCPListener creates a xds Listener resource // TODO: Improve function parameters -func buildXdsTCPListener(name, address string, port uint32, ipFamily *ir.IPFamily, keepalive *ir.TCPKeepalive, connection *ir.ClientConnection, accesslog *ir.AccessLog) *listenerv3.Listener { +func buildXdsTCPListener( + name, address string, + port uint32, + ipFamily *ir.IPFamily, + keepalive *ir.TCPKeepalive, + connection *ir.ClientConnection, + accesslog *ir.AccessLog, +) (*listenerv3.Listener, error) { socketOptions := buildTCPSocketOptions(keepalive) - al := buildXdsAccessLog(accesslog, ir.ProxyAccessLogTypeListener) + al, err := buildXdsAccessLog(accesslog, ir.ProxyAccessLogTypeListener) + if err != nil { + return nil, err + } bufferLimitBytes := buildPerConnectionBufferLimitBytes(connection) listener := &listenerv3.Listener{ Name: name, @@ -203,7 +212,7 @@ func buildXdsTCPListener(name, address string, port uint32, ipFamily *ir.IPFamil socketAddress := listener.Address.GetSocketAddress() listener.AdditionalAddresses = setAddressByIPFamily(socketAddress, ipFamily, port) - return listener + return listener, nil } func buildPerConnectionBufferLimitBytes(connection *ir.ClientConnection) *wrapperspb.UInt32Value { @@ -214,10 +223,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) { + log, err := buildXdsAccessLog(accesslog, ir.ProxyAccessLogTypeListener) + if err != nil { + return nil, err + } xdsListener := &listenerv3.Listener{ Name: name + "-quic", - AccessLog: buildXdsAccessLog(accesslog, ir.ProxyAccessLogTypeListener), + AccessLog: log, Address: &corev3.Address{ Address: &corev3.Address_SocketAddress{ SocketAddress: &corev3.SocketAddress{ @@ -238,7 +251,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 @@ -254,7 +267,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, ir.ProxyAccessLogTypeRoute) + al, err := buildXdsAccessLog(accesslog, ir.ProxyAccessLogTypeRoute) + if err != nil { + return err + } hcmTracing, err := buildHCMTracing(tracing) if err != nil { @@ -454,7 +470,7 @@ func buildEarlyHeaderMutation(headers *ir.HeaderSettings) []*corev3.TypedExtensi mutationRules = append(mutationRules, mr) } - earlyHeaderMutationAny, _ := anypb.New(&early_header_mutationv3.HeaderMutation{ + earlyHeaderMutationAny, _ := protocov.ToAnyWithValidation(&early_header_mutationv3.HeaderMutation{ Mutations: mutationRules, }) @@ -526,9 +542,12 @@ func addXdsTCPFilterChain(xdsListener *listenerv3.Listener, irRoute *ir.TCPRoute // Append port to the statPrefix. statPrefix = strings.Join([]string{statPrefix, strconv.Itoa(int(xdsListener.Address.GetSocketAddress().GetPortValue()))}, "-") - + al, error := buildXdsAccessLog(accesslog, ir.ProxyAccessLogTypeRoute) + if error != nil { + return error + } mgr := &tcpv3.TcpProxy{ - AccessLog: buildXdsAccessLog(accesslog, ir.ProxyAccessLogTypeRoute), + AccessLog: al, StatPrefix: statPrefix, ClusterSpecifier: &tcpv3.TcpProxy_Cluster{ Cluster: clusterName, @@ -612,7 +631,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 } @@ -660,7 +679,7 @@ func buildDownstreamQUICTransportSocket(tlsConfig *ir.TLSConfig) (*corev3.Transp setDownstreamTLSSessionSettings(tlsConfig, tlsCtx.DownstreamTlsContext) - tlsCtxAny, err := anypb.New(tlsCtx) + tlsCtxAny, err := protocov.ToAnyWithValidation(tlsCtx) if err != nil { return nil, err } @@ -702,7 +721,7 @@ func buildXdsDownstreamTLSSocket(tlsConfig *ir.TLSConfig) (*corev3.TransportSock setDownstreamTLSSessionSettings(tlsConfig, tlsCtx) - tlsCtxAny, err := anypb.New(tlsCtx) + tlsCtxAny, err := protocov.ToAnyWithValidation(tlsCtx) if err != nil { return nil, err } @@ -817,14 +836,18 @@ 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, error := buildXdsAccessLog(accesslog, ir.ProxyAccessLogTypeRoute) + if error != nil { + return nil, error + } udpProxy := &udpv3.UdpProxyConfig{ StatPrefix: statPrefix, - AccessLog: buildXdsAccessLog(accesslog, ir.ProxyAccessLogTypeRoute), + AccessLog: al, RouteSpecifier: &udpv3.UdpProxyConfig_Matcher{ Matcher: &matcher.Matcher{ OnNoMatch: &matcher.Matcher_OnMatch{ @@ -838,14 +861,17 @@ func buildXdsUDPListener(clusterName string, udpListener *ir.UDPListener, access }, }, } - udpProxyAny, err := anypb.New(udpProxy) + udpProxyAny, err := protocov.ToAnyWithValidation(udpProxy) if err != nil { return nil, err } + if al, err = buildXdsAccessLog(accesslog, ir.ProxyAccessLogTypeListener); err != nil { + return nil, err + } xdsListener := &listenerv3.Listener{ Name: udpListener.Name, - AccessLog: buildXdsAccessLog(accesslog, ir.ProxyAccessLogTypeListener), + AccessLog: al, Address: &corev3.Address{ Address: &corev3.Address_SocketAddress{ SocketAddress: &corev3.SocketAddress{ @@ -892,7 +918,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/oidc.go b/internal/xds/translator/oidc.go index e4e7b4a0216..a706cae662f 100644 --- a/internal/xds/translator/oidc.go +++ b/internal/xds/translator/oidc.go @@ -16,12 +16,12 @@ 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" 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" ) @@ -83,7 +83,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/testdata/in/xds-ir/accesslog-without-format.yaml b/internal/xds/translator/testdata/in/xds-ir/accesslog-without-format.yaml index 90e9f0e0c9b..434f2fb524c 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 5169bae040e..3f84816fdcf 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/in/xds-ir/authorization-multiple-principals.yaml b/internal/xds/translator/testdata/in/xds-ir/authorization-multiple-principals.yaml index c93708b4c8a..8b83e16d556 100644 --- a/internal/xds/translator/testdata/in/xds-ir/authorization-multiple-principals.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/authorization-multiple-principals.yaml @@ -44,7 +44,7 @@ http: isIPv6: false maskLen: 24 jwt: - issuer: https://one.example.com + provider: https://one.example.com scopes: - foo claims: @@ -68,7 +68,7 @@ http: isIPv6: false maskLen: 24 jwt: - issuer: https://two.example.com + provider: https://two.example.com scopes: - for - bar 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 fecb2076871..9df135e671c 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 3b52d45e8e8..0ef9cdc5fab 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/testdata/out/xds-ir/authorization-multiple-principals.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/authorization-multiple-principals.routes.yaml index c6510f63778..2b9a4906343 100644 --- a/internal/xds/translator/testdata/out/xds-ir/authorization-multiple-principals.routes.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/authorization-multiple-principals.routes.yaml @@ -59,7 +59,7 @@ '@type': type.googleapis.com/envoy.extensions.matching.common_inputs.network.v3.DynamicMetadataInput filter: envoy.filters.http.jwt_authn path: - - key: "" + - key: https://one.example.com - key: scope - orMatcher: predicate: @@ -79,7 +79,7 @@ '@type': type.googleapis.com/envoy.extensions.matching.common_inputs.network.v3.DynamicMetadataInput filter: envoy.filters.http.jwt_authn path: - - key: "" + - key: https://one.example.com - key: roles - singlePredicate: customMatch: @@ -97,7 +97,7 @@ '@type': type.googleapis.com/envoy.extensions.matching.common_inputs.network.v3.DynamicMetadataInput filter: envoy.filters.http.jwt_authn path: - - key: "" + - key: https://one.example.com - key: roles - singlePredicate: customMatch: @@ -113,7 +113,7 @@ '@type': type.googleapis.com/envoy.extensions.matching.common_inputs.network.v3.DynamicMetadataInput filter: envoy.filters.http.jwt_authn path: - - key: "" + - key: https://one.example.com - key: department - onMatch: action: @@ -155,7 +155,7 @@ '@type': type.googleapis.com/envoy.extensions.matching.common_inputs.network.v3.DynamicMetadataInput filter: envoy.filters.http.jwt_authn path: - - key: "" + - key: https://two.example.com - key: scope - singlePredicate: customMatch: @@ -173,7 +173,7 @@ '@type': type.googleapis.com/envoy.extensions.matching.common_inputs.network.v3.DynamicMetadataInput filter: envoy.filters.http.jwt_authn path: - - key: "" + - key: https://two.example.com - key: scope - orMatcher: predicate: @@ -193,7 +193,7 @@ '@type': type.googleapis.com/envoy.extensions.matching.common_inputs.network.v3.DynamicMetadataInput filter: envoy.filters.http.jwt_authn path: - - key: "" + - key: https://two.example.com - key: roles - singlePredicate: customMatch: @@ -211,7 +211,7 @@ '@type': type.googleapis.com/envoy.extensions.matching.common_inputs.network.v3.DynamicMetadataInput filter: envoy.filters.http.jwt_authn path: - - key: "" + - key: https://two.example.com - key: roles - orMatcher: predicate: @@ -229,7 +229,7 @@ '@type': type.googleapis.com/envoy.extensions.matching.common_inputs.network.v3.DynamicMetadataInput filter: envoy.filters.http.jwt_authn path: - - key: "" + - key: https://two.example.com - key: name - singlePredicate: customMatch: @@ -245,7 +245,7 @@ '@type': type.googleapis.com/envoy.extensions.matching.common_inputs.network.v3.DynamicMetadataInput filter: envoy.filters.http.jwt_authn path: - - key: "" + - key: https://two.example.com - key: name onNoMatch: action: diff --git a/internal/xds/translator/tracing.go b/internal/xds/translator/tracing.go index c7777f94ba2..3e817bad1bf 100644 --- a/internal/xds/translator/tracing.go +++ b/internal/xds/translator/tracing.go @@ -50,7 +50,7 @@ func buildHCMTracing(tracing *ir.Tracing) (*hcm.HttpConnectionManager_Tracing, e ServiceName: tracing.ServiceName, CollectorCluster: tracing.Destination.Name, } - return protocov.ToAnyWithError(config) + return protocov.ToAnyWithValidation(config) } case egv1a1.TracingProviderTypeOpenTelemetry: providerName = envoyOpenTelemetry @@ -68,7 +68,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 @@ -82,7 +82,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 30a54fe6990..27c0d3c5a04 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,13 @@ func (t *Translator) processHTTPListenerXdsTranslation( } // Create a new TCP listener for HTTP1/HTTP2 traffic. - tcpXDSListener = buildXdsTCPListener(httpListener.Name, httpListener.Address, httpListener.Port, httpListener.IPFamily, httpListener.TCPKeepalive, httpListener.Connection, accessLog) + if tcpXDSListener, err = buildXdsTCPListener( + httpListener.Name, httpListener.Address, httpListener.Port, httpListener.IPFamily, + 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 @@ -514,7 +524,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 } @@ -560,12 +570,19 @@ 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.IPFamily, tcpListener.TCPKeepalive, tcpListener.Connection, accesslog) + if xdsListener, err = buildXdsTCPListener( + tcpListener.Name, tcpListener.Address, tcpListener.Port, tcpListener.IPFamily, + 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) @@ -911,7 +928,7 @@ func buildXdsUpstreamTLSSocketWthCert(tlsConfig *ir.TLSUpstreamConfig) (*corev3. } } - tlsCtxAny, err := anypb.New(tlsCtx) + tlsCtxAny, err := protocov.ToAnyWithValidation(tlsCtx) if err != nil { return nil, err }