Manual backport 1.15.x of Disable remote proxy patching except AWS La…

…mbda

backport of commit ba18381

agent/envoyextensions/builtin/aws-lambda/aws_lambda.go

@@ -39,7 +39,7 @@ func Constructor(ext api.EnvoyExtension) (extensioncommon.EnvoyExtender, error)
 	if err := a.fromArguments(ext.Arguments); err != nil {
 		return nil, err
 	}
-	return &extensioncommon.BasicEnvoyExtender{
+	return &extensioncommon.UpstreamEnvoyExtender{
 		Extension: &a,
 	}, nil
 }
@@ -62,7 +62,7 @@ func (a *awsLambda) validate() error {
 // CanApply returns true if the kind of the provided ExtensionConfiguration matches
 // the kind of the lambda configuration
 func (a *awsLambda) CanApply(config *extensioncommon.RuntimeConfig) bool {
-	return config.Kind == config.OutgoingProxyKind()
+	return config.Kind == config.UpstreamOutgoingProxyKind()
 }
 
 // PatchRoute modifies the routing configuration for a service of kind TerminatingGateway. If the kind is
@@ -72,6 +72,11 @@ func (a *awsLambda) PatchRoute(r *extensioncommon.RuntimeConfig, route *envoy_ro
 		return route, false, nil
 	}
 
+	// Only patch outbound routes.
+	if extensioncommon.IsRouteToLocalAppCluster(route) {
+		return route, false, nil
+	}
+
 	for _, virtualHost := range route.VirtualHosts {
 		for _, route := range virtualHost.Routes {
 			action, ok := route.Action.(*envoy_route_v3.Route_Route)
@@ -92,6 +97,11 @@ func (a *awsLambda) PatchRoute(r *extensioncommon.RuntimeConfig, route *envoy_ro
 
 // PatchCluster patches the provided envoy cluster with data required to support an AWS lambda function
 func (a *awsLambda) PatchCluster(_ *extensioncommon.RuntimeConfig, c *envoy_cluster_v3.Cluster) (*envoy_cluster_v3.Cluster, bool, error) {
+	// Only patch outbound clusters.
+	if extensioncommon.IsLocalAppCluster(c) {
+		return c, false, nil
+	}
+
 	transportSocket, err := makeUpstreamTLSTransportSocket(&envoy_tls_v3.UpstreamTlsContext{
 		Sni: "*.amazonaws.com",
 	})
@@ -153,7 +163,12 @@ func (a *awsLambda) PatchCluster(_ *extensioncommon.RuntimeConfig, c *envoy_clus
 
 // PatchFilter patches the provided envoy filter with an inserted lambda filter being careful not to
 // overwrite the http filters.
-func (a *awsLambda) PatchFilter(_ *extensioncommon.RuntimeConfig, filter *envoy_listener_v3.Filter) (*envoy_listener_v3.Filter, bool, error) {
+func (a *awsLambda) PatchFilter(_ *extensioncommon.RuntimeConfig, filter *envoy_listener_v3.Filter, isInboundListener bool) (*envoy_listener_v3.Filter, bool, error) {
+	// Only patch outbound filters.
+	if isInboundListener {
+		return filter, false, nil
+	}
+
 	if filter.Name != "envoy.filters.network.http_connection_manager" {
 		return filter, false, nil
 	}

agent/envoyextensions/builtin/aws-lambda/aws_lambda_test.go

@@ -83,7 +83,7 @@ func TestConstructor(t *testing.T) {
 
 			if tc.ok {
 				require.NoError(t, err)
-				require.Equal(t, &extensioncommon.BasicEnvoyExtender{Extension: &tc.expected}, e)
+				require.Equal(t, &extensioncommon.UpstreamEnvoyExtender{Extension: &tc.expected}, e)
 			} else {
 				require.Error(t, err)
 			}
@@ -320,10 +320,11 @@ func TestPatchFilter(t *testing.T) {
 		return v
 	}
 	tests := map[string]struct {
-		filter       *envoy_listener_v3.Filter
-		expectFilter *envoy_listener_v3.Filter
-		expectBool   bool
-		expectErr    string
+		filter          *envoy_listener_v3.Filter
+		isInboundFilter bool
+		expectFilter    *envoy_listener_v3.Filter
+		expectBool      bool
+		expectErr       string
 	}{
 		"invalid filter name is ignored": {
 			filter:       &envoy_listener_v3.Filter{Name: "something"},
@@ -413,6 +414,36 @@ func TestPatchFilter(t *testing.T) {
 			},
 			expectBool: true,
 		},
+		"inbound filter ignored": {
+			filter: &envoy_listener_v3.Filter{
+				Name: "envoy.filters.network.http_connection_manager",
+				ConfigType: &envoy_listener_v3.Filter_TypedConfig{
+					TypedConfig: makeAny(&envoy_http_v3.HttpConnectionManager{
+						HttpFilters: []*envoy_http_v3.HttpFilter{
+							{Name: "one"},
+							{Name: "two"},
+							{Name: "envoy.filters.http.router"},
+							{Name: "three"},
+						},
+					}),
+				},
+			},
+			expectFilter: &envoy_listener_v3.Filter{
+				Name: "envoy.filters.network.http_connection_manager",
+				ConfigType: &envoy_listener_v3.Filter_TypedConfig{
+					TypedConfig: makeAny(&envoy_http_v3.HttpConnectionManager{
+						HttpFilters: []*envoy_http_v3.HttpFilter{
+							{Name: "one"},
+							{Name: "two"},
+							{Name: "envoy.filters.http.router"},
+							{Name: "three"},
+						},
+					}),
+				},
+			},
+			isInboundFilter: true,
+			expectBool:      false,
+		},
 	}
 
 	for name, tc := range tests {
@@ -422,7 +453,7 @@ func TestPatchFilter(t *testing.T) {
 				PayloadPassthrough: true,
 				InvocationMode:     "asynchronous",
 			}
-			f, ok, err := l.PatchFilter(nil, tc.filter)
+			f, ok, err := l.PatchFilter(nil, tc.filter, tc.isInboundFilter)
 			require.Equal(t, tc.expectBool, ok)
 			if tc.expectErr == "" {
 				require.NoError(t, err)

agent/envoyextensions/builtin/lua/lua.go

@@ -61,11 +61,11 @@ func (l *lua) validate() error {
 
 // CanApply determines if the extension can apply to the given extension configuration.
 func (l *lua) CanApply(config *extensioncommon.RuntimeConfig) bool {
-	return string(config.Kind) == l.ProxyType && l.matchesListenerDirection(config)
+	return string(config.Kind) == l.ProxyType
 }
 
-func (l *lua) matchesListenerDirection(config *extensioncommon.RuntimeConfig) bool {
-	return (config.IsUpstream() && l.Listener == "outbound") || (!config.IsUpstream() && l.Listener == "inbound")
+func (l *lua) matchesListenerDirection(isInboundListener bool) bool {
+	return (!isInboundListener && l.Listener == "outbound") || (isInboundListener && l.Listener == "inbound")
 }
 
 // PatchRoute does nothing.
@@ -79,7 +79,12 @@ func (l *lua) PatchCluster(_ *extensioncommon.RuntimeConfig, c *envoy_cluster_v3
 }
 
 // PatchFilter inserts a lua filter directly prior to envoy.filters.http.router.
-func (l *lua) PatchFilter(_ *extensioncommon.RuntimeConfig, filter *envoy_listener_v3.Filter) (*envoy_listener_v3.Filter, bool, error) {
+func (l *lua) PatchFilter(_ *extensioncommon.RuntimeConfig, filter *envoy_listener_v3.Filter, isInboundListener bool) (*envoy_listener_v3.Filter, bool, error) {
+	// Make sure filter matches extension config.
+	if !l.matchesListenerDirection(isInboundListener) {
+		return filter, false, nil
+	}
+
 	if filter.Name != "envoy.filters.network.http_connection_manager" {
 		return filter, false, nil
 	}

agent/envoyextensions/registered_extensions_test.go

@@ -1,9 +1,11 @@
 package envoyextensions
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/hashicorp/consul/api"
+	"github.com/hashicorp/consul/envoyextensions/extensioncommon"
 	"github.com/stretchr/testify/require"
 )
 
@@ -58,3 +60,52 @@ func TestValidateExtensions(t *testing.T) {
 		})
 	}
 }
+
+// This test is included here so that we can test all registered extensions without creating a cyclic dependency between
+// envoyextensions and extensioncommon.
+func TestUpstreamExtenderLimitations(t *testing.T) {
+	type testCase struct {
+		config *extensioncommon.RuntimeConfig
+		ok     bool
+		errMsg string
+	}
+	unauthorizedExtensionCase := func(name string) testCase {
+		return testCase{
+			config: &extensioncommon.RuntimeConfig{
+				Kind:                  api.ServiceKindConnectProxy,
+				ServiceName:           api.CompoundServiceName{Name: "api"},
+				Upstreams:             map[api.CompoundServiceName]*extensioncommon.UpstreamData{},
+				IsSourcedFromUpstream: true,
+				EnvoyExtension: api.EnvoyExtension{
+					Name: name,
+				},
+			},
+			ok:     false,
+			errMsg: fmt.Sprintf("extension %q is not permitted to be applied via upstream service config", name),
+		}
+	}
+	cases := map[string]testCase{
+		// Make sure future extensions are theoretically covered, even if not registered in the same way.
+		"unknown extension": unauthorizedExtensionCase("someotherextension"),
+	}
+	for name := range extensionConstructors {
+		// AWS Lambda is the only extension permitted to modify downstream proxy resources.
+		if name == api.BuiltinAWSLambdaExtension {
+			continue
+		}
+		cases[name] = unauthorizedExtensionCase(name)
+	}
+
+	for n, tc := range cases {
+		t.Run(n, func(t *testing.T) {
+			extender := extensioncommon.UpstreamEnvoyExtender{}
+			_, err := extender.Extend(nil, tc.config)
+			if tc.ok {
+				require.NoError(t, err)
+			} else {
+				require.Error(t, err)
+				require.ErrorContains(t, err, tc.errMsg)
+			}
+		})
+	}
+}

agent/xds/delta_envoy_extender_oss_test.go

@@ -59,17 +59,16 @@ func TestEnvoyExtenderWithSnapshot(t *testing.T) {
 		}
 	}
 
-	makeLuaServiceDefaults := func(inbound bool) *structs.ServiceConfigEntry {
+	// Apply Lua extension to the local service and ensure http is used so the extension can be applied.
+	makeLuaNsFunc := func(inbound bool) func(ns *structs.NodeService) {
 		listener := "inbound"
 		if !inbound {
 			listener = "outbound"
 		}
 
-		return &structs.ServiceConfigEntry{
-			Kind:     structs.ServiceDefaults,
-			Name:     "db",
-			Protocol: "http",
-			EnvoyExtensions: []structs.EnvoyExtension{
+		return func(ns *structs.NodeService) {
+			ns.Proxy.Config["protocol"] = "http"
+			ns.Proxy.EnvoyExtensions = []structs.EnvoyExtension{
 				{
 					Name: api.BuiltinLuaExtension,
 					Arguments: map[string]interface{}{
@@ -81,7 +80,7 @@ function envoy_on_request(request_handle)
 end`,
 					},
 				},
-			},
+			}
 		}
 	}
 
@@ -127,57 +126,47 @@ end`,
 			create: proxycfg.TestConfigSnapshotTerminatingGatewayWithLambdaServiceAndServiceResolvers,
 		},
 		{
-			name: "lua-outbound-applies-to-upstreams",
+			name: "lua-outbound-applies-to-local-upstreams",
 			create: func(t testinf.T) *proxycfg.ConfigSnapshot {
-				return proxycfg.TestConfigSnapshotDiscoveryChain(t, "default", false, nil, nil, makeLuaServiceDefaults(false))
+				// upstreams need to be http in order for lua to be applied to listeners.
+				return proxycfg.TestConfigSnapshotDiscoveryChain(t, "default", false, makeLuaNsFunc(false), nil, &structs.ServiceConfigEntry{
+					Kind:     structs.ServiceDefaults,
+					Name:     "db",
+					Protocol: "http",
+				}, &structs.ServiceConfigEntry{
+					Kind:     structs.ServiceDefaults,
+					Name:     "geo-cache",
+					Protocol: "http",
+				})
 			},
 		},
 		{
-			name: "lua-inbound-doesnt-applies-to-upstreams",
+			// We expect an inbound public listener lua filter here because the extension targets inbound.
+			// The only difference between goldens for this and lua-inbound-applies-to-inbound
+			// should be that db has HTTP filters rather than TCP.
+			name: "lua-inbound-doesnt-apply-to-local-upstreams",
 			create: func(t testinf.T) *proxycfg.ConfigSnapshot {
-				return proxycfg.TestConfigSnapshotDiscoveryChain(t, "default", false, nil, nil, makeLuaServiceDefaults(true))
+				// db is made an HTTP upstream so that the extension _could_ apply, but does not because
+				// the direction for the extension is inbound.
+				return proxycfg.TestConfigSnapshotDiscoveryChain(t, "default", false, makeLuaNsFunc(true), nil, &structs.ServiceConfigEntry{
+					Kind:     structs.ServiceDefaults,
+					Name:     "db",
+					Protocol: "http",
+				})
 			},
 		},
 		{
 			name: "lua-inbound-applies-to-inbound",
 			create: func(t testinf.T) *proxycfg.ConfigSnapshot {
-				return proxycfg.TestConfigSnapshot(t, func(ns *structs.NodeService) {
-					ns.Proxy.Config["protocol"] = "http"
-					ns.Proxy.EnvoyExtensions = []structs.EnvoyExtension{
-						{
-							Name: api.BuiltinLuaExtension,
-							Arguments: map[string]interface{}{
-								"ProxyType": "connect-proxy",
-								"Listener":  "inbound",
-								"Script": `
-function envoy_on_request(request_handle)
-  request_handle:headers():add("test", "test")
-end`,
-							},
-						},
-					}
-				}, nil)
+				return proxycfg.TestConfigSnapshotDiscoveryChain(t, "default", false, makeLuaNsFunc(true), nil)
 			},
 		},
 		{
+			// We expect _no_ lua filters here, because the extension targets outbound, but there are
+			// no upstream HTTP services. We also should not see public listener, which is HTTP, patched.
 			name: "lua-outbound-doesnt-apply-to-inbound",
 			create: func(t testinf.T) *proxycfg.ConfigSnapshot {
-				return proxycfg.TestConfigSnapshot(t, func(ns *structs.NodeService) {
-					ns.Proxy.Config["protocol"] = "http"
-					ns.Proxy.EnvoyExtensions = []structs.EnvoyExtension{
-						{
-							Name: api.BuiltinLuaExtension,
-							Arguments: map[string]interface{}{
-								"ProxyType": "connect-proxy",
-								"Listener":  "outbound",
-								"Script": `
-function envoy_on_request(request_handle)
-  request_handle:headers():add("test", "test")
-end`,
-							},
-						},
-					}
-				}, nil)
+				return proxycfg.TestConfigSnapshotDiscoveryChain(t, "default", false, makeLuaNsFunc(false), nil)
 			},
 		},
 		{