Skip to content

Commit

Permalink
Add fallthrough listener filer chain for BlackHoleCluster (istio#18541)
Browse files Browse the repository at this point in the history 
* Add fallthrough listener filer chain for BlackHoleCluster

* Fix telemetry reported

Fixes: istio#17271 istio#17759

* Tests for IsAllowAnyOutbound

* Use t.Run

* Added mixer plugin tests

* Added listener tests
  • Loading branch information
Neeraj Poddar authored and Steven Dake committed Dec 1, 2019
1 parent 9edc747 commit 697d3ba
Show file tree
Hide file tree
Showing 9 changed files with 274 additions and 31 deletions.
2 changes: 1 addition & 1 deletion pilot/pkg/networking/core/v1alpha3/httproute.go
View file
Expand Up @@ -182,7 +182,7 @@ func (configgen *ConfigGeneratorImpl) buildSidecarOutboundHTTPRouteConfig(env *m

if features.EnableFallthroughRoute.Get() && !useSniffing {
// This needs to be the last virtual host, as routes are evaluated in order.
if isAllowAnyOutbound(node) {
if util.IsAllowAnyOutbound(node) {
virtualHosts = append(virtualHosts, &route.VirtualHost{
Name: util.PassthroughRouteName,
Domains: []string{"*"},
Expand Down
36 changes: 12 additions & 24 deletions pilot/pkg/networking/core/v1alpha3/listener.go
View file
Expand Up @@ -30,7 +30,6 @@ import (
accesslogconfig "github.com/envoyproxy/go-control-plane/envoy/config/accesslog/v2"
accesslog "github.com/envoyproxy/go-control-plane/envoy/config/filter/accesslog/v2"
http_conn "github.com/envoyproxy/go-control-plane/envoy/config/filter/network/http_connection_manager/v2"
tcp_proxy "github.com/envoyproxy/go-control-plane/envoy/config/filter/network/tcp_proxy/v2"
envoy_type "github.com/envoyproxy/go-control-plane/envoy/type"
"github.com/envoyproxy/go-control-plane/pkg/wellknown"
"github.com/golang/protobuf/ptypes"
Expand Down Expand Up @@ -1451,7 +1450,7 @@ func (configgen *ConfigGeneratorImpl) buildSidecarOutboundListenerForPortOrUDS(n
// Lets build the new listener with the filter chains. In the end, we will
// merge the filter chains with any existing listener on the same port/bind point
l := buildListener(listenerOpts)
appendListenerFallthroughRoute(l, &listenerOpts, pluginParams.Node, currentListenerEntry)
appendListenerFallthroughRoute(l, &listenerOpts, pluginParams.Node, pluginParams.Env, currentListenerEntry)
l.TrafficDirection = core.TrafficDirection_OUTBOUND

mutable := &plugin.MutableObjects{
Expand Down Expand Up @@ -1583,7 +1582,7 @@ func (configgen *ConfigGeneratorImpl) onVirtualOutboundListener(env *model.Envir
push *model.PushContext,
ipTablesListener *xdsapi.Listener) *xdsapi.Listener {

hostname := host.Name(util.BlackHoleCluster)
svc := util.FallThroughFilterChainBlackHoleService
mesh := env.Mesh
redirectPort := &model.Port{
Port: int(mesh.ProxyListenPort),
Expand All @@ -1600,8 +1599,8 @@ func (configgen *ConfigGeneratorImpl) onVirtualOutboundListener(env *model.Envir
// contains just the final passthrough/blackhole
fallbackFilter := ipTablesListener.FilterChains[len(ipTablesListener.FilterChains)-1].Filters[0]

if isAllowAnyOutbound(node) {
hostname = util.PassthroughCluster
if util.IsAllowAnyOutbound(node) {
svc = util.FallThroughFilterChainPassthroughService
}

pluginParams := &plugin.InputParams{
Expand All @@ -1612,10 +1611,7 @@ func (configgen *ConfigGeneratorImpl) onVirtualOutboundListener(env *model.Envir
Push: push,
Bind: "",
Port: redirectPort,
Service: &model.Service{
Hostname: hostname,
Ports: model.PortList{redirectPort},
},
Service: svc,
}

mutable := &plugin.MutableObjects{
Expand Down Expand Up @@ -1750,6 +1746,7 @@ type filterChainOpts struct {
match *listener.FilterChainMatch
listenerFilters []*listener.ListenerFilter
networkFilters []*listener.Filter
isFallThrough bool
}

// buildListenerOpts are the options required to build a Listener
Expand Down Expand Up @@ -2031,9 +2028,9 @@ func buildListener(opts buildListenerOpts) *xdsapi.Listener {
// appendListenerFallthroughRoute adds a filter that will match all traffic and direct to the
// PassthroughCluster. This should be appended as the final filter or it will mask the others.
// This allows external https traffic, even when port the port (usually 443) is in use by another service.
func appendListenerFallthroughRoute(l *xdsapi.Listener, opts *buildListenerOpts, node *model.Proxy, currentListenerEntry *outboundListenerEntry) {
// If traffic policy is REGISTRY_ONLY, the traffic will already be blocked, so no action is needed.
if features.EnableFallthroughRoute.Get() && isAllowAnyOutbound(node) {
func appendListenerFallthroughRoute(l *xdsapi.Listener, opts *buildListenerOpts,
node *model.Proxy, env *model.Environment, currentListenerEntry *outboundListenerEntry) {
if features.EnableFallthroughRoute.Get() {

wildcardMatch := &listener.FilterChainMatch{}
for _, fc := range l.FilterChains {
Expand All @@ -2055,21 +2052,11 @@ func appendListenerFallthroughRoute(l *xdsapi.Listener, opts *buildListenerOpts,
}
}

tcpFilter := &listener.Filter{
Name: wellknown.TCPProxy,
}
tcpProxy := &tcp_proxy.TcpProxy{
StatPrefix: util.PassthroughCluster,
ClusterSpecifier: &tcp_proxy.TcpProxy_Cluster{Cluster: util.PassthroughCluster},
}
if util.IsXDSMarshalingToAnyEnabled(node) {
tcpFilter.ConfigType = &listener.Filter_TypedConfig{TypedConfig: util.MessageToAny(tcpProxy)}
} else {
tcpFilter.ConfigType = &listener.Filter_Config{Config: util.MessageToStruct(tcpProxy)}
}
tcpFilter := newTCPProxyOutboundListenerFilter(env, node)

opts.filterChainOpts = append(opts.filterChainOpts, &filterChainOpts{
networkFilters: []*listener.Filter{tcpFilter},
isFallThrough: true,
})
l.FilterChains = append(l.FilterChains, &listener.FilterChain{FilterChainMatch: wildcardMatch})

Expand Down Expand Up @@ -2318,6 +2305,7 @@ func getPluginFilterChain(opts buildListenerOpts) []plugin.FilterChain {
} else {
filterChain[id].ListenerProtocol = plugin.ListenerProtocolHTTP
}
filterChain[id].IsFallThrough = opts.filterChainOpts[id].isFallThrough
}

return filterChain
Expand Down
6 changes: 1 addition & 5 deletions pilot/pkg/networking/core/v1alpha3/listener_builder.go
View file
Expand Up @@ -578,7 +578,7 @@ func newTCPProxyOutboundListenerFilter(env *model.Environment, node *model.Proxy
StatPrefix: util.BlackHoleCluster,
ClusterSpecifier: &tcp_proxy.TcpProxy_Cluster{Cluster: util.BlackHoleCluster},
}
if isAllowAnyOutbound(node) {
if util.IsAllowAnyOutbound(node) {
// We need a passthrough filter to fill in the filter stack for orig_dst listener
tcpProxy = &tcp_proxy.TcpProxy{
StatPrefix: util.PassthroughCluster,
Expand All @@ -598,7 +598,3 @@ func newTCPProxyOutboundListenerFilter(env *model.Environment, node *model.Proxy
}
return &filter
}

func isAllowAnyOutbound(node *model.Proxy) bool {
return node.SidecarScope.OutboundTrafficPolicy != nil && node.SidecarScope.OutboundTrafficPolicy.Mode == networking.OutboundTrafficPolicy_ALLOW_ANY
}
75 changes: 75 additions & 0 deletions pilot/pkg/networking/core/v1alpha3/listener_test.go
View file
Expand Up @@ -32,11 +32,14 @@ import (
"github.com/golang/protobuf/proto"
"github.com/golang/protobuf/ptypes"

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

"istio.io/istio/pilot/pkg/features"
"istio.io/istio/pilot/pkg/model"
"istio.io/istio/pilot/pkg/networking/core/v1alpha3/fakes"
"istio.io/istio/pilot/pkg/networking/plugin"
"istio.io/istio/pilot/pkg/networking/util"
"istio.io/istio/pilot/pkg/serviceregistry"
"istio.io/istio/pkg/config/host"
"istio.io/istio/pkg/config/labels"
Expand Down Expand Up @@ -1709,3 +1712,75 @@ func buildListenerEnvWithVirtualServices(services []*model.Service, virtualServi

return env
}

func TestAppendListenerFallthroughRoute(t *testing.T) {
env := &model.Environment{
Mesh: &meshconfig.MeshConfig{},
}
tests := []struct {
name string
listener *xdsapi.Listener
listenerOpts *buildListenerOpts
node *model.Proxy
hostname string
}{
{
name: "Registry_Only",
listener: &xdsapi.Listener{},
listenerOpts: &buildListenerOpts{},
node: &model.Proxy{
ID: "foo.bar",
Metadata: &model.NodeMetadata{},
SidecarScope: &model.SidecarScope{
OutboundTrafficPolicy: &networking.OutboundTrafficPolicy{
Mode: networking.OutboundTrafficPolicy_REGISTRY_ONLY,
},
},
},
hostname: util.BlackHoleCluster,
},
{
name: "Allow_Any",
listener: &xdsapi.Listener{},
listenerOpts: &buildListenerOpts{},
node: &model.Proxy{
ID: "foo.bar",
Metadata: &model.NodeMetadata{},
SidecarScope: &model.SidecarScope{
OutboundTrafficPolicy: &networking.OutboundTrafficPolicy{
Mode: networking.OutboundTrafficPolicy_ALLOW_ANY,
},
},
},
hostname: util.PassthroughCluster,
},
}
for idx := range tests {
t.Run(tests[idx].name, func(t *testing.T) {
appendListenerFallthroughRoute(tests[idx].listener, tests[idx].listenerOpts,
tests[idx].node, env, nil)
if len(tests[idx].listenerOpts.filterChainOpts) != 1 {
t.Errorf("Expected exactly 1 filter chain options")
}
if !tests[idx].listenerOpts.filterChainOpts[0].isFallThrough {
t.Errorf("Expected fall through to be set")
}
if len(tests[idx].listenerOpts.filterChainOpts[0].networkFilters) != 1 {
t.Errorf("Expected exactly 1 network filter in the chain")
}
filter := tests[idx].listenerOpts.filterChainOpts[0].networkFilters[0]
var tcpProxy tcp_proxy.TcpProxy
cfg := filter.GetTypedConfig()
ptypes.UnmarshalAny(cfg, &tcpProxy)
if tcpProxy.StatPrefix != tests[idx].hostname {
t.Errorf("Expected stat prefix %s but got %s\n", tests[idx].hostname, tcpProxy.StatPrefix)
}
if tcpProxy.GetCluster() != tests[idx].hostname {
t.Errorf("Expected cluster %s but got %s\n", tests[idx].hostname, tcpProxy.GetCluster())
}
if len(tests[idx].listener.FilterChains) != 1 {
t.Errorf("Expected exactly 1 filter chain on the tests[idx].listener")
}
})
}
}
12 changes: 11 additions & 1 deletion pilot/pkg/networking/plugin/mixer/mixer.go
View file
Expand Up @@ -143,7 +143,17 @@ func (mixerplugin) OnOutboundListener(in *plugin.InputParams, mutable *plugin.Mu
}
} else {
for cnum := range mutable.FilterChains {
mutable.FilterChains[cnum].TCP = append(mutable.FilterChains[cnum].TCP, tcpFilter)
if mutable.FilterChains[cnum].IsFallThrough {
svc := util.FallThroughFilterChainBlackHoleService
if util.IsAllowAnyOutbound(in.Node) {
svc = util.FallThroughFilterChainPassthroughService
}
attrs := createOutboundListenerAttributes(in)
fallThroughFilter := buildOutboundTCPFilter(in.Env.Mesh, attrs, in.Node, svc)
mutable.FilterChains[cnum].TCP = append(mutable.FilterChains[cnum].TCP, fallThroughFilter)
} else {
mutable.FilterChains[cnum].TCP = append(mutable.FilterChains[cnum].TCP, tcpFilter)
}
}
}
return nil
Expand Down
94 changes: 94 additions & 0 deletions pilot/pkg/networking/plugin/mixer/mixer_test.go
View file
Expand Up @@ -19,11 +19,15 @@ import (
"testing"
"time"

xdsapi "github.com/envoyproxy/go-control-plane/envoy/api/v2"
//listener "github.com/envoyproxy/go-control-plane/envoy/api/v2/listener"
"github.com/golang/protobuf/ptypes"

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

"istio.io/istio/pilot/pkg/model"
"istio.io/istio/pilot/pkg/networking/plugin"
mccpb "istio.io/istio/pilot/pkg/networking/plugin/mixer/client"
"istio.io/istio/pkg/config/mesh"
)
Expand Down Expand Up @@ -129,3 +133,93 @@ func Test_proxyVersionToString(t *testing.T) {
})
}
}

func TestOnOutboundListener(t *testing.T) {
mp := mixerplugin{}
inputParams := &plugin.InputParams{
ListenerProtocol: plugin.ListenerProtocolTCP,
Env: &model.Environment{
Mesh: &meshconfig.MeshConfig{
MixerReportServer: "mixer.istio-system",
},
},
Node: &model.Proxy{
ID: "foo.bar",
Metadata: &model.NodeMetadata{},
},
}
tests := []struct {
name string
sidecarScope *model.SidecarScope
mutableObjects *plugin.MutableObjects
hostname string
}{
{
name: "Registry_Only",
sidecarScope: &model.SidecarScope{
OutboundTrafficPolicy: &networking.OutboundTrafficPolicy{
Mode: networking.OutboundTrafficPolicy_REGISTRY_ONLY,
},
},
mutableObjects: &plugin.MutableObjects{
Listener: &xdsapi.Listener{},
FilterChains: []plugin.FilterChain{
{
IsFallThrough: false,
},
{
IsFallThrough: true,
},
},
},
hostname: "BlackHoleCluster",
},
{
name: "Allow_Any",
sidecarScope: &model.SidecarScope{
OutboundTrafficPolicy: &networking.OutboundTrafficPolicy{
Mode: networking.OutboundTrafficPolicy_ALLOW_ANY,
},
},
mutableObjects: &plugin.MutableObjects{
Listener: &xdsapi.Listener{},
FilterChains: []plugin.FilterChain{
{
IsFallThrough: false,
},
{
IsFallThrough: true,
},
},
},
hostname: "PassthroughCluster",
},
}
for idx := range tests {
t.Run(tests[idx].name, func(t *testing.T) {
inputParams.Node.SidecarScope = tests[idx].sidecarScope
mp.OnOutboundListener(inputParams, tests[idx].mutableObjects)
for i := 0; i < len(tests[idx].mutableObjects.FilterChains); i++ {
if len(tests[idx].mutableObjects.FilterChains[i].TCP) != 1 {
t.Errorf("Expected 1 TCP filter")
}
var tcpClientConfig mccpb.TcpClientConfig
cfg := tests[idx].mutableObjects.FilterChains[i].TCP[0].GetTypedConfig()
ptypes.UnmarshalAny(cfg, &tcpClientConfig)
if tests[idx].mutableObjects.FilterChains[i].IsFallThrough {
hostAttr := tcpClientConfig.MixerAttributes.Attributes["destination.service.host"]
if !reflect.DeepEqual(hostAttr, attrStringValue(tests[idx].hostname)) {
t.Errorf("Expected host %s but got %+v\n",
tests[idx].hostname, hostAttr)
}

nameAttr := tcpClientConfig.MixerAttributes.Attributes["destination.service.name"]
if !reflect.DeepEqual(nameAttr, attrStringValue(tests[idx].hostname)) {
t.Errorf("Expected name %s but got %+v\n",
tests[idx].hostname, nameAttr)
}
}
}
})
}
}
2 changes: 2 additions & 0 deletions pilot/pkg/networking/plugin/plugin.go
View file
Expand Up @@ -141,6 +141,8 @@ type FilterChain struct {
HTTP []*http_conn.HttpFilter
// TCP is the set of network (TCP) filters for this filter chain.
TCP []*listener.Filter
// IsFallthrough indicates if the filter chain is fallthrough.
IsFallThrough bool
}

// MutableObjects is a set of objects passed to On*Listener callbacks. Fields may be nil or empty.
Expand Down

0 comments on commit 697d3ba

Please sign in to comment.