solo-io / gloo

The Feature-rich, Kubernetes-native, Next-Generation API Gateway Built on Envoy
https://docs.solo.io/
Apache License 2.0
4.07k stars 437 forks source link

Envoy 1.29 #9232

Closed jbohanon closed 5 months ago

jbohanon commented 6 months ago

Gloo Edge Product

Open Source

Gloo Edge Version

1.17

Is your feature request related to a problem? Please describe.

I want Gloo Edge to use the latest stable Envoy version

Describe the solution you'd like

Describe alternatives you've considered

No response

Additional Context

Leapfrogs https://github.com/solo-io/gloo/issues/8979

┆Issue is synchronized with this Asana task by Unito

nfuden commented 6 months ago

Closed existing issue from the cost of doing business and updated this to be part of 1.17 cost of doing business.

jbohanon commented 6 months ago

Potential Problem

Incompatible behavior changes

Minor behavior changes

Bug fixes

Warrants Investigation

Incompatible behavior changes

Minor behavior changes

Bug fixes

New Features (unfiltered)

jbohanon commented 6 months ago

Comparing the commit our current go-control-plane-fork-v2 forked from to the commit synced with upstream v1.29.2 go-control-plane changes https://github.com/envoyproxy/go-control-plane/compare/989e83d4a05c74448fdc72e5a67df5529387c021...5401a878d8bb6ef0ee8ae8df2f916aa1faaa1890

ben-taussig-solo commented 5 months ago

@jbohanon @ashishb-solo and I met to discuss the changes listed in Jacob's comment above (see here)

Here are our notes -- they are pretty messy, but the general conclusion is that we need to look further into the envoy.reloadable_features.no_downgrade_to_canonical_name change and the header mutation change. We determined that we are either unaffected by other changes, or that those changes should be communicated to end users and field engineers

Meeting Notes

Solo issues -- to look into

Unaffected

jbohanon commented 5 months ago

No downgrade to canonical name

This change will be considered unsafe for us if there are any places where we are using custom filter names which might not align with one or more overrides in TypedPerFilterConfig.

To start, I have checked that we are always setting the TypedPerFilterConfig with the canonical name as the key. I validated this by grepping through gloo and solo-projects for calls into pluginutils[.].*PerFilterConfig[(] which covers all of these helper functions. solo-projects:

grep -rE "pluginutils[.].*PerFilterConfig[(]" ./
./projects/gloo/pkg/plugins/graphql/plugin.go:  return pluginutils.SetRoutePerFilterConfig(out, FilterName, routeConf)
./projects/gloo/pkg/plugins/dlp/plugin.go:              pluginutils.SetVhostPerFilterConfig(out, FilterName, dlpConfig)
./projects/gloo/pkg/plugins/dlp/plugin.go:              pluginutils.SetRoutePerFilterConfig(out, FilterName, dlpConfig)
./projects/gloo/pkg/plugins/extauth/sanitize_filter.go: return pluginutils.SetVhostPerFilterConfig(out, SanitizeFilterName, customAuthConfig)
./projects/gloo/pkg/plugins/extauth/sanitize_filter.go: return pluginutils.SetWeightedClusterPerFilterConfig(out, SanitizeFilterName, customAuthConfig)
./projects/gloo/pkg/plugins/extauth/sanitize_filter.go: return pluginutils.SetRoutePerFilterConfig(out, SanitizeFilterName, customAuthConfig)
./projects/gloo/pkg/plugins/extauth/plugin.go:  return pluginutils.SetVhostPerFilterConfig(out, wellknown.HTTPExternalAuthorization, extAuthPerRouteConfig)
./projects/gloo/pkg/plugins/extauth/plugin.go:  return pluginutils.SetRoutePerFilterConfig(out, wellknown.HTTPExternalAuthorization, extAuthPerRouteConfig)
./projects/gloo/pkg/plugins/extauth/plugin.go:  return pluginutils.SetWeightedClusterPerFilterConfig(out, wellknown.HTTPExternalAuthorization, extAuthPerRouteConfig)
./projects/gloo/pkg/plugins/rbac/plugin.go:             pluginutils.SetVhostPerFilterConfig(out, FilterName, perRouteRbac)
./projects/gloo/pkg/plugins/rbac/plugin.go:     pluginutils.SetVhostPerFilterConfig(out, FilterName, perRouteRbac)
./projects/gloo/pkg/plugins/rbac/plugin.go:             pluginutils.SetRoutePerFilterConfig(out, FilterName, perRouteRbac)
./projects/gloo/pkg/plugins/jwt/plugin.go:      return pluginutils.SetRoutePerFilterConfig(out, SoloJwtFilterName, stagedCfg)
./projects/gloo/pkg/plugins/jwt/plugin.go:      return pluginutils.SetVhostPerFilterConfig(out, SoloJwtFilterName, stagedCfg)
./projects/gloo/pkg/plugins/waf/plugin.go:      pluginutils.SetVhostPerFilterConfig(out, FilterName, perVhostCfg)
./projects/gloo/pkg/plugins/waf/plugin.go:      pluginutils.SetRoutePerFilterConfig(out, FilterName, perRouteCfg)
./projects/gloo/pkg/plugins/extproc/plugin.go:  return pluginutils.SetVhostPerFilterConfig(out, FilterName, extProcPerRoute)
./projects/gloo/pkg/plugins/extproc/plugin.go:  return pluginutils.SetRoutePerFilterConfig(out, FilterName, extProcPerRoute)

gloo:

grep -rE "pluginutils[.].*PerFilterConfig[(]" ./
./projects/gloo/pkg/plugins/rest/plugin.go:     return pluginutils.MarkPerFilterConfig(params.Ctx, params.Snapshot, in, out, transformation.FilterName,
./projects/gloo/pkg/plugins/buffer/plugin.go:           return pluginutils.SetRoutePerFilterConfig(out, wellknown.Buffer, getNoBufferConfig())
./projects/gloo/pkg/plugins/buffer/plugin.go:           return pluginutils.SetRoutePerFilterConfig(out, wellknown.Buffer, config)
./projects/gloo/pkg/plugins/buffer/plugin.go:           return pluginutils.SetVhostPerFilterConfig(out, wellknown.Buffer, getNoBufferConfig())
./projects/gloo/pkg/plugins/buffer/plugin.go:           return pluginutils.SetVhostPerFilterConfig(out, wellknown.Buffer, config)
./projects/gloo/pkg/plugins/buffer/plugin.go:           return pluginutils.SetWeightedClusterPerFilterConfig(out, wellknown.Buffer, getNoBufferConfig())
./projects/gloo/pkg/plugins/buffer/plugin.go:           return pluginutils.SetWeightedClusterPerFilterConfig(out, wellknown.Buffer, config)
./projects/gloo/pkg/plugins/dynamic_forward_proxy/plugin.go:    return pluginutils.SetRoutePerFilterConfig(out, FilterName, dfpRouteCfg)
./projects/gloo/pkg/plugins/azure/plugin.go:    return pluginutils.MarkPerFilterConfig(p.ctx, params.Snapshot, in, out, transformation.FilterName,
./projects/gloo/pkg/plugins/extauth/plugin.go:  return pluginutils.SetVhostPerFilterConfig(out, wellknown.HTTPExternalAuthorization, extAuthPerRouteConfig)
./projects/gloo/pkg/plugins/extauth/plugin.go:  return pluginutils.SetRoutePerFilterConfig(out, wellknown.HTTPExternalAuthorization, extAuthPerRouteConfig)
./projects/gloo/pkg/plugins/extauth/plugin.go:  return pluginutils.SetWeightedClusterPerFilterConfig(out, wellknown.HTTPExternalAuthorization, extAuthPerRouteConfig)
./projects/gloo/pkg/plugins/grpc/plugin.go:     return pluginutils.MarkPerFilterConfig(params.Ctx, params.Snapshot, in, out, transformation.FilterName,
./projects/gloo/pkg/plugins/csrf/plugin.go:     return pluginutils.SetRoutePerFilterConfig(out, FilterName, envoyCsrfConfig)
./projects/gloo/pkg/plugins/csrf/plugin.go:     return pluginutils.SetVhostPerFilterConfig(out, FilterName, envoyCsrfConfig)
./projects/gloo/pkg/plugins/csrf/plugin.go:     return pluginutils.SetWeightedClusterPerFilterConfig(out, FilterName, envoyCsrfConfig)
./projects/gloo/pkg/plugins/faultinjection/plugin.go:   return pluginutils.MarkPerFilterConfig(params.Ctx, params.Snapshot, in, out, wellknown.Fault, markFilterConfigFunc)
./projects/gloo/pkg/plugins/transformation/plugin.go:   return pluginutils.ModifyVhostPerFilterConfig(out, FilterName, mergeFunc(envoyTransformation))
./projects/gloo/pkg/plugins/transformation/plugin.go:   return pluginutils.ModifyRoutePerFilterConfig(out, FilterName, mergeFunc(envoyTransformation))
./projects/gloo/pkg/plugins/transformation/plugin.go:   return pluginutils.ModifyWeightedClusterPerFilterConfig(out, FilterName, mergeFunc(envoyTransformation))
./projects/gloo/pkg/plugins/cors/plugin.go:     return pluginutils.SetVhostPerFilterConfig(out, wellknown.CORS, corsPolicy)
./projects/gloo/pkg/plugins/cors/plugin.go:     return pluginutils.SetRoutePerFilterConfig(out, wellknown.CORS, corsPolicy)
./projects/gloo/pkg/plugins/local_ratelimit/utils.go:   return pluginutils.ModifyVhostPerFilterConfig(out, HTTPFilterName, modIfNoExisting(filter))
./projects/gloo/pkg/plugins/local_ratelimit/utils.go:   return pluginutils.ModifyRoutePerFilterConfig(out, HTTPFilterName, modIfNoExisting(filter))
./projects/gloo/pkg/plugins/aws/plugin.go:      err := pluginutils.MarkPerFilterConfig(params.Ctx, params.Snapshot, in, out, FilterName,
./projects/gloo/pkg/plugins/aws/plugin.go:      return pluginutils.ModifyPerFilterConfig(params.Ctx, params.Snapshot, in, out, transformation.FilterName,

Next concern is whether any filters are being configured with names which do not match canonical names, since we know all of our TPFC have canonical. Checking this should be exhaustive by checking calls to NewStagedFilter and NewFilterWithTypedConfig for Network and HTTP filters, respectively. gloo:

grep -rEA1 "(NewStagedFilter|NewFilterWithTypedConfig)[(]" ./
./projects/gloo/pkg/translator/utils.go:func NewFilterWithTypedConfig(name string, config proto.Message) (*envoy_config_listener_v3.Filter, error) {
./projects/gloo/pkg/translator/utils.go-
--
./projects/gloo/pkg/translator/network_filters.go:      hcmFilter, err := NewFilterWithTypedConfig(wellknown.HTTPConnectionManager, httpConnectionManager)
./projects/gloo/pkg/translator/network_filters.go-      if err != nil {
--
./projects/gloo/pkg/translator/network_filters.go:      newStagedFilter, err := plugins.NewStagedFilter(
./projects/gloo/pkg/translator/network_filters.go-              wellknown.Router,
--
./projects/gloo/pkg/plugins/ratelimit/plugin.go:        stagedRateLimitFilter, err := plugins.NewStagedFilter(
./projects/gloo/pkg/plugins/ratelimit/plugin.go-                wellknown.HTTPRateLimit,
--
./projects/gloo/pkg/plugins/buffer/plugin.go:   bufferFilter, err := plugins.NewStagedFilter(wellknown.Buffer, bufferConfig, pluginStage)
./projects/gloo/pkg/plugins/buffer/plugin.go-   if err != nil {
--
./projects/gloo/pkg/plugins/dynamic_forward_proxy/plugin.go:    c, err := plugins.NewStagedFilter(FilterName, dfp, pluginStage)
./projects/gloo/pkg/plugins/dynamic_forward_proxy/plugin.go-    if err != nil {
--
./projects/gloo/pkg/plugins/extauth/config_generator.go:                stagedFilter, err := plugins.NewStagedFilter(wellknown.HTTPExternalAuthorization, extAuthCfg, stage)
./projects/gloo/pkg/plugins/extauth/config_generator.go-                if err != nil {
--
./projects/gloo/pkg/plugins/grpc/plugin.go:             shf, err := plugins.NewStagedFilter(wellknown.GRPCJSONTranscoder, filterConfig, pluginStage)
./projects/gloo/pkg/plugins/grpc/plugin.go-             if err != nil {
--
./projects/gloo/pkg/plugins/grpcjson/plugin.go: grpcJsonFilter, err := plugins.NewStagedFilter(wellknown.GRPCJSONTranscoder, envoyGrpcJsonConf, pluginStage)
./projects/gloo/pkg/plugins/grpcjson/plugin.go- if err != nil {
--
./projects/gloo/pkg/plugins/grpcjson/plugin.go:         grpcJsonFilter, err := plugins.NewStagedFilter(wellknown.GRPCJSONTranscoder, envoyGrpcJsonConf, pluginStage)
./projects/gloo/pkg/plugins/grpcjson/plugin.go-         if err != nil {
--
./projects/gloo/pkg/plugins/grpcjson/plugin.go:         grpcJsonFilter, err := plugins.NewStagedFilter(wellknown.GRPCJSONTranscoder, envoyGrpcJsonConf, pluginStage)
./projects/gloo/pkg/plugins/grpcjson/plugin.go-         if err != nil {
--
./projects/gloo/pkg/plugins/grpcweb/plugin.go:  return []plugins.StagedHttpFilter{plugins.MustNewStagedFilter(wellknown.GRPCWeb, &envoygrpcweb.GrpcWeb{}, pluginStage)}, nil
./projects/gloo/pkg/plugins/grpcweb/plugin.go-}
--
./projects/gloo/pkg/plugins/gzip/plugin.go:     gzipFilter, err := plugins.NewStagedFilter(CompressorFilterName, envoyGzipConfig, pluginStage)
./projects/gloo/pkg/plugins/gzip/plugin.go-     if err != nil {
--
./projects/gloo/pkg/plugins/healthcheck/plugin.go:      healthCheckFilter, err := plugins.NewStagedFilter(wellknown.HealthCheck, hc, pluginStage)
./projects/gloo/pkg/plugins/healthcheck/plugin.go-      if err != nil {
--
./projects/gloo/pkg/plugins/csrf/plugin.go:     csrfFilter, err := plugins.NewStagedFilter(FilterName, envoyCsrfConfig, pluginStage)
./projects/gloo/pkg/plugins/csrf/plugin.go-     if err != nil {
--
./projects/gloo/pkg/plugins/faultinjection/plugin.go:   return []plugins.StagedHttpFilter{plugins.MustNewStagedFilter(wellknown.Fault, &envoyhttpfault.HTTPFault{}, pluginStage)}, nil
./projects/gloo/pkg/plugins/faultinjection/plugin.go-}
--
./projects/gloo/pkg/plugins/transformation/plugin_test.go:                              expectedFilter := plugins.MustNewStagedFilter(
./projects/gloo/pkg/plugins/transformation/plugin_test.go-                                      FilterName,
--
./projects/gloo/pkg/plugins/transformation/plugin.go:           earlyFilter, err := plugins.NewStagedFilter(FilterName, earlyStageConfig, earlyPluginStage)
./projects/gloo/pkg/plugins/transformation/plugin.go-           if err != nil {
--
./projects/gloo/pkg/plugins/transformation/plugin.go:           plugins.MustNewStagedFilter(FilterName,
./projects/gloo/pkg/plugins/transformation/plugin.go-                   &envoytransformation.FilterTransformations{
--
./projects/gloo/pkg/plugins/staged_filters.go:  return NewStagedFilter(name, config, stage)
./projects/gloo/pkg/plugins/staged_filters.go-}
--
./projects/gloo/pkg/plugins/staged_filters.go:func MustNewStagedFilter(name string, config proto.Message, stage FilterStage) StagedHttpFilter {
./projects/gloo/pkg/plugins/staged_filters.go:  s, _ := NewStagedFilter(name, config, stage)
./projects/gloo/pkg/plugins/staged_filters.go-  return s
--
./projects/gloo/pkg/plugins/staged_filters.go:func NewStagedFilter(name string, config proto.Message, stage FilterStage) (StagedHttpFilter, error) {
./projects/gloo/pkg/plugins/staged_filters.go-
--
./projects/gloo/pkg/plugins/cors/plugin.go:     return []plugins.StagedHttpFilter{plugins.MustNewStagedFilter(wellknown.CORS, &envoy_config_cors_v3.Cors{}, pluginStage)}, nil
./projects/gloo/pkg/plugins/cors/plugin.go-}
--
./projects/gloo/pkg/plugins/local_ratelimit/plugin.go:  stagedRateLimitFilter, err := plugins.NewStagedFilter(
./projects/gloo/pkg/plugins/local_ratelimit/plugin.go-          HTTPFilterName,
--
./projects/gloo/pkg/plugins/tcp/plugin.go:      tcpFilter, err := translatorutil.NewFilterWithTypedConfig(wellknown.TCPProxy, cfg)
./projects/gloo/pkg/plugins/tcp/plugin.go-      if err != nil {
--
./projects/gloo/pkg/plugins/aws/plugin.go:      f, err := plugins.NewStagedFilter(FilterName, filterConfig, pluginStage)
./projects/gloo/pkg/plugins/aws/plugin.go-      if err != nil {
--
./projects/gloo/pkg/plugins/aws/plugin.go:              tf, err := plugins.NewStagedFilter(transformation.FilterName, awsStageConfig, transformPluginStage)
./projects/gloo/pkg/plugins/aws/plugin.go-              if err != nil {

solo-projects:

grep -rEA1 "(NewStagedFilter|NewFilterWithTypedConfig)[(]" ./
./projects/gloo/pkg/plugins/graphql/plugin.go:  stagedFilter, err := plugins.NewStagedFilter(FilterName, emptyConf, FilterStage)
./projects/gloo/pkg/plugins/graphql/plugin.go-  if err != nil {
--
./projects/gloo/pkg/plugins/tap/plugin.go:      stagedFilter, err := plugins.NewStagedFilter(FilterName, envoyTapConfig, filterStage)
./projects/gloo/pkg/plugins/tap/plugin.go-      if err != nil {
--
./projects/gloo/pkg/plugins/proxylatency/plugin.go:             stagedFilter, err := plugins.NewStagedFilter(FilterName, pl, FilterStage)
./projects/gloo/pkg/plugins/proxylatency/plugin.go-             if err != nil {
--
./projects/gloo/pkg/plugins/extproc/plugin.go:  stagedFilter, err := plugins.NewStagedFilter(FilterName, extProcFilter, *convertedStage)
./projects/gloo/pkg/plugins/extproc/plugin.go-  if err != nil {
jbohanon commented 5 months ago

health check uneject behavior solution: call out to end users/field eng

This has actually been default-on behavior since 1.26, so I don't think it makes sense to call out this change. In this, we are basically changing from default-true runtime guard to a default-true config option. https://www.envoyproxy.io/docs/envoy/latest/version_history/v1.26/v1.26.0

ben-taussig-solo commented 5 months ago

Thanks for verifying the downgrade to canonical name issue so thoroughly. With that, I expect to be comfortable approving these PRs once I am able to look further into the header_mutation change

jbohanon commented 5 months ago

Thanks for verifying the downgrade to canonical name issue so thoroughly. With that, I expect to be comfortable approving these PRs once I am able to look further into the header_mutation change

It looks to me that this only affects the header_mutation HTTP filter, which we do not use. AFAICT we only configure header mutations on the route, which are applied during execution of the router filter.

our plugin and the related envoy codepaths: https://github.com/solo-io/gloo/blob/main/projects/gloo/pkg/plugins/headers/plugin.go https://github.com/envoyproxy/envoy/blob/main/source/common/router/config_impl.cc

PR which added this changelog: https://github.com/envoyproxy/envoy/pull/30220/files

ben-taussig-solo commented 5 months ago

It looks to me that this only affects the header_mutation HTTP filter, which we do not use. AFAICT we only configure header mutations on the route, which are applied during execution of the router filter.

Makes sense -- I was unsure of whether we used this filter while reviewing the changelogs

ben-taussig-solo commented 5 months ago

After another review of the changelogs:

That's the only thing I could turn up, so I'm ready to approve these PRs

jbohanon commented 5 months ago

Released in: Gloo Edge OSS v1.17.0-beta15 Will be released in: Gloo Edge EE v1.17.0-beta1

Documentation for breaking changes should be added to upgrade guide

jbohanon commented 5 months ago

Documentation work to be tracked with https://github.com/solo-io/gloo/issues/9271