Skip to content

Commit

Permalink
Support interop between v2 and v3 EnvoyFilters (#23322)
Browse files Browse the repository at this point in the history 
* Support interop between v2 and v3 EnvoyFilters

For istio/istio#19885

This allows support for EnvoyFilters using `any` to work, regardless of
Pilot's internal representation as v2 or v3 config. Previously if a user
created an EnvoyFilter with a v3 type, it would not be applied, as the
internal type Pilot is generating is v2. The same applies to
EnvoyFilters with v2 once we move to v3 internally. This change is not
like other XDS changes that can be mitigated by proxyMatch, since its an
incompatibiltiy with Pilot rather than the proxy/version.

* Extend tests, ignore unknown

* Change validation test
  • Loading branch information
@howardjohn
howardjohn authored Apr 30, 2020
1 parent aa3183f commit e0e53ee
Show file tree
Hide file tree
Showing 6 changed files with 177 additions and 9 deletions.
9 changes: 7 additions & 2 deletions pilot/pkg/model/envoyfilter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,13 @@ func convertToEnvoyFilterWrapper(local *Config) *EnvoyFilterWrapper {
Match: cp.Match,
Operation: cp.Patch.Operation,
}
// there won't be an error here because validation catches mismatched types
cpw.Value, _ = xds.BuildXDSObjectFromStruct(cp.ApplyTo, cp.Patch.Value)
var err error
cpw.Value, err = xds.BuildXDSObjectFromStruct(cp.ApplyTo, cp.Patch.Value)
// There generally won't be an error here because validation catches mismatched types
// Should only happen in tests or without validation
if err != nil {
log.Errorf("failed to build envoy filter value: %v", err)
}
if cp.Match == nil {
// create a match all object
cpw.Match = &networking.EnvoyFilter_EnvoyConfigObjectMatch{Context: networking.EnvoyFilter_ANY}
Expand Down
14 changes: 14 additions & 0 deletions pilot/pkg/networking/core/v1alpha3/envoyfilter/listener_patch.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,13 @@ func doNetworkFilterOperation(patchContext networking.EnvoyFilter_PatchContext,
var retVal *any.Any
if userFilter.GetTypedConfig() != nil {
// user has any typed struct
// The type may not match up exactly. For example, if we use v2 internally but they use v3.
// Assuming they are not using deprecated/new fields, we can safely swap out the TypeUrl
// If we did not do this, proto.Merge below will panic (which is recovered), so even though this
// is not 100% reliable its better than doing nothing
if userFilter.GetTypedConfig().TypeUrl != filter.GetTypedConfig().TypeUrl {
userFilter.ConfigType.(*xdslistener.Filter_TypedConfig).TypedConfig.TypeUrl = filter.GetTypedConfig().TypeUrl
}
if retVal, err = util.MergeAnyWithAny(filter.GetTypedConfig(), userFilter.GetTypedConfig()); err != nil {
retVal = filter.GetTypedConfig()
}
Expand Down Expand Up @@ -465,6 +472,13 @@ func doHTTPFilterOperation(patchContext networking.EnvoyFilter_PatchContext,
var retVal *any.Any
if userHTTPFilter.GetTypedConfig() != nil {
// user has any typed struct
// The type may not match up exactly. For example, if we use v2 internally but they use v3.
// Assuming they are not using deprecated/new fields, we can safely swap out the TypeUrl
// If we did not do this, proto.Merge below will panic (which is recovered), so even though this
// is not 100% reliable its better than doing nothing
if userHTTPFilter.GetTypedConfig().TypeUrl != httpFilter.GetTypedConfig().TypeUrl {
userHTTPFilter.ConfigType.(*http_conn.HttpFilter_TypedConfig).TypedConfig.TypeUrl = httpFilter.GetTypedConfig().TypeUrl
}
if retVal, err = util.MergeAnyWithAny(httpFilter.GetTypedConfig(), userHTTPFilter.GetTypedConfig()); err != nil {
retVal = httpFilter.GetTypedConfig()
}
Expand Down
70 changes: 66 additions & 4 deletions pilot/pkg/networking/core/v1alpha3/envoyfilter/listener_patch_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,16 @@ import (
fault "github.com/envoyproxy/go-control-plane/envoy/config/filter/http/fault/v2"
http_conn "github.com/envoyproxy/go-control-plane/envoy/config/filter/network/http_connection_manager/v2"
xdsutil "github.com/envoyproxy/go-control-plane/pkg/wellknown"
"github.com/gogo/protobuf/jsonpb"
"github.com/gogo/protobuf/proto"
"github.com/gogo/protobuf/types"
"github.com/golang/protobuf/ptypes"
"github.com/golang/protobuf/ptypes/wrappers"

"github.com/gogo/protobuf/jsonpb"
"github.com/gogo/protobuf/types"
"github.com/google/go-cmp/cmp"

meshconfig "istio.io/api/mesh/v1alpha1"
networking "istio.io/api/networking/v1alpha3"

istio_proto "istio.io/istio/pkg/proto"

"istio.io/istio/pilot/pkg/config/kube/crd"
Expand Down Expand Up @@ -316,6 +316,34 @@ func TestApplyListenerPatches(t *testing.T) {
Value: buildPatchStruct(`{"name": "http-filter4"}`),
},
},
// Merge v3 any with v2 any
{
ApplyTo: networking.EnvoyFilter_HTTP_FILTER,
Match: &networking.EnvoyFilter_EnvoyConfigObjectMatch{
Context: networking.EnvoyFilter_SIDECAR_INBOUND,
ObjectTypes: &networking.EnvoyFilter_EnvoyConfigObjectMatch_Listener{
Listener: &networking.EnvoyFilter_ListenerMatch{
PortNumber: 80,
FilterChain: &networking.EnvoyFilter_ListenerMatch_FilterChainMatch{
Filter: &networking.EnvoyFilter_ListenerMatch_FilterMatch{
Name: xdsutil.HTTPConnectionManager,
SubFilter: &networking.EnvoyFilter_ListenerMatch_SubFilterMatch{Name: "envoy.fault"},
},
},
},
},
},
Patch: &networking.EnvoyFilter_Patch{
Operation: networking.EnvoyFilter_Patch_MERGE,
Value: buildPatchStruct(`
{"name": "envoy.fault",
"typed_config": {
"@type": "type.googleapis.com/envoy.extensions.filters.http.fault.v3.HTTPFault",
"downstreamNodes": ["foo"]
}
}`),
},
},
{
ApplyTo: networking.EnvoyFilter_HTTP_FILTER,
Match: &networking.EnvoyFilter_EnvoyConfigObjectMatch{
Expand Down Expand Up @@ -378,11 +406,41 @@ func TestApplyListenerPatches(t *testing.T) {
Patch: &networking.EnvoyFilter_Patch{
Operation: networking.EnvoyFilter_Patch_MERGE,
Value: buildPatchStruct(`
{"name": "envoy.http_connection_manager",
{"name": "envoy.http_connection_manager",
"typed_config": {
"@type": "type.googleapis.com/envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager",
"xffNumTrustedHops": "4"
}
}`),
},
},
// Ensure we can mix v3 patches with v2 internal
// Note that alwaysSetRequestIdInResponse is only present in v3 protos. It will be silently ignored
// as we are working in v2 protos internally
{
ApplyTo: networking.EnvoyFilter_NETWORK_FILTER,
Match: &networking.EnvoyFilter_EnvoyConfigObjectMatch{
Context: networking.EnvoyFilter_SIDECAR_INBOUND,
ObjectTypes: &networking.EnvoyFilter_EnvoyConfigObjectMatch_Listener{
Listener: &networking.EnvoyFilter_ListenerMatch{
PortNumber: 80,
FilterChain: &networking.EnvoyFilter_ListenerMatch_FilterChainMatch{
Filter: &networking.EnvoyFilter_ListenerMatch_FilterMatch{
Name: xdsutil.HTTPConnectionManager,
},
},
},
},
},
Patch: &networking.EnvoyFilter_Patch{
Operation: networking.EnvoyFilter_Patch_MERGE,
Value: buildPatchStruct(`
{"name": "envoy.http_connection_manager",
"typed_config": {
"@type": "type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager",
"mergeSlashes": true,
"alwaysSetRequestIdInResponse": true
}
}`),
},
},
Expand Down Expand Up @@ -501,6 +559,7 @@ func TestApplyListenerPatches(t *testing.T) {
faultFilterInAny, _ := ptypes.MarshalAny(faultFilterIn)
faultFilterOut := &fault.HTTPFault{
UpstreamCluster: "scooby",
DownstreamNodes: []string{"foo"},
}
faultFilterOutAny, _ := ptypes.MarshalAny(faultFilterOut)
sidecarInboundIn := []*xdsapi.Listener{
Expand Down Expand Up @@ -576,6 +635,7 @@ func TestApplyListenerPatches(t *testing.T) {
ConfigType: &listener.Filter_TypedConfig{
TypedConfig: util.MessageToAny(&http_conn.HttpConnectionManager{
XffNumTrustedHops: 4,
MergeSlashes: true,
HttpFilters: []*http_conn.HttpFilter{
{Name: xdsutil.Fault,
ConfigType: &http_conn.HttpFilter_TypedConfig{TypedConfig: faultFilterOutAny},
Expand Down Expand Up @@ -773,6 +833,7 @@ func TestApplyListenerPatches(t *testing.T) {
ConfigType: &listener.Filter_TypedConfig{
TypedConfig: util.MessageToAny(&http_conn.HttpConnectionManager{
XffNumTrustedHops: 4,
MergeSlashes: true,
HttpFilters: []*http_conn.HttpFilter{
{Name: xdsutil.Fault,
ConfigType: &http_conn.HttpFilter_TypedConfig{TypedConfig: faultFilterOutAny},
Expand Down Expand Up @@ -920,6 +981,7 @@ func BenchmarkTelemetryV2Filters(b *testing.B) {
ConfigType: &listener.Filter_TypedConfig{
TypedConfig: util.MessageToAny(&http_conn.HttpConnectionManager{
XffNumTrustedHops: 4,
MergeSlashes: true,
HttpFilters: []*http_conn.HttpFilter{
{Name: "http-filter3"},
{Name: xdsutil.Router},
Expand Down
4 changes: 2 additions & 2 deletions pkg/config/validation/validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3135,15 +3135,15 @@ func TestValidateEnvoyFilter(t *testing.T) {
Operation: networking.EnvoyFilter_Patch_ADD,
Value: &types.Struct{
Fields: map[string]*types.Value{
"foo": {
"hosts": {
Kind: &types.Value_BoolValue{BoolValue: false},
},
},
},
},
},
},
}, error: `Envoy filter: unknown field "foo" in envoy_api_v2.Cluster`},
}, error: `Envoy filter: json: cannot unmarshal bool into Go value of type []json.RawMessage`},
{name: "happy config", in: &networking.EnvoyFilter{
ConfigPatches: []*networking.EnvoyFilter_EnvoyConfigObjectPatch{
{
Expand Down
86 changes: 86 additions & 0 deletions pkg/config/xds/filter_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,90 @@ import (
_ "github.com/envoyproxy/go-control-plane/envoy/config/filter/thrift/router/v2alpha1"
_ "github.com/envoyproxy/go-control-plane/envoy/config/filter/udp/udp_proxy/v2alpha"
_ "github.com/envoyproxy/go-control-plane/envoy/config/trace/v2"

// v3 protos
_ "github.com/envoyproxy/go-control-plane/envoy/config/accesslog/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/config/bootstrap/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/config/grpc_credential/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/config/metrics/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/config/overload/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/config/ratelimit/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/config/tap/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/config/trace/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/access_loggers/file/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/access_loggers/grpc/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/clusters/aggregate/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/clusters/dynamic_forward_proxy/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/clusters/redis/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/common/dynamic_forward_proxy/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/common/ratelimit/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/common/tap/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/common/fault/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/adaptive_concurrency/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/aws_lambda/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/aws_request_signing/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/buffer/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/cache/v3alpha"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/compressor/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/cors/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/csrf/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/dynamic_forward_proxy/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/dynamo/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/ext_authz/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/fault/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/grpc_http1_bridge/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/grpc_http1_reverse_bridge/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/grpc_json_transcoder/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/grpc_stats/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/grpc_web/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/gzip/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/header_to_metadata/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/health_check/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/ip_tagging/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/jwt_authn/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/lua/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/on_demand/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/original_src/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/ratelimit/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/rbac/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/router/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/squash/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/tap/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/listener/http_inspector/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/listener/original_dst/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/listener/original_src/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/listener/proxy_protocol/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/listener/tls_inspector/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/client_ssl_auth/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/direct_response/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/dubbo_proxy/router/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/dubbo_proxy/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/echo/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/ext_authz/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/kafka_broker/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/local_ratelimit/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/mongo_proxy/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/mysql_proxy/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/ratelimit/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/rbac/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/redis_proxy/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/sni_cluster/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/tcp_proxy/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/thrift_proxy/filters/ratelimit/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/thrift_proxy/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/zookeeper_proxy/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/retry/host/omit_host_metadata/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/retry/priority/previous_priorities/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/alts/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/raw_buffer/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tap/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/wasm/v3"
)
3 changes: 2 additions & 1 deletion pkg/config/xds/xds.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,6 @@ func GogoStructToMessage(pbst *types.Struct, out proto.Message) error {
return err
}

return jsonpb.Unmarshal(buf, out)
// Ignore unknown fields as they may be sending versions of the proto we are not internally using
return (&jsonpb.Unmarshaler{AllowUnknownFields: true}).Unmarshal(buf, out)
}

0 comments on commit e0e53ee

Please sign in to comment.