Mw/lambda envoy extension parse region (#4107) (#16069)

* updated builtin extension to parse region directly from ARN
- added a unit test
- added some comments/light refactoring

* updated golden files with proper ARNs
- ARNs need to be right format now that they are being processed

* updated tests and integration tests
- removed 'region' from all EnvoyExtension arguments
- added properly formatted ARN which includes the same region found in the removed "Region" field: 'us-east-1'

agent/proxycfg/testing_terminating_gateway.go

@@ -954,9 +954,8 @@ func TestConfigSnapshotTerminatingGatewayWithLambdaService(t testing.T, extraUpd
 				{
 					Name: structs.BuiltinAWSLambdaExtension,
 					Arguments: map[string]interface{}{
-						"ARN":                "lambda-arn",
+						"ARN":                "arn:aws:lambda:us-east-1:111111111111:function:lambda-1234",
 						"PayloadPassthrough": true,
-						"Region":             "us-east-1",
 					},
 				},
 			},

agent/xds/builtin_extension_oss_test.go

@@ -48,10 +48,9 @@ func TestBuiltinExtensionsFromSnapshot(t *testing.T) {
 				{
 					Name: api.BuiltinAWSLambdaExtension,
 					Arguments: map[string]interface{}{
-						"ARN":                "lambda-arn",
+						"ARN":                "arn:aws:lambda:us-east-1:111111111111:function:lambda-1234",
 						"PayloadPassthrough": payloadPassthrough,
 						"InvocationMode":     invocationMode,
-						"Region":             "us-east-1",
 					},
 				},
 			},

agent/xds/builtinextensions/lambda/lambda.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 
+	arn_sdk "github.com/aws/aws-sdk-go/aws/arn"
 	envoy_cluster_v3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
 	envoy_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
 	envoy_endpoint_v3 "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3"
@@ -25,7 +26,6 @@ import (
 type lambda struct {
 	ARN                string
 	PayloadPassthrough bool
-	Region             string
 	Kind               api.ServiceKind
 	InvocationMode     string
 }
@@ -49,10 +49,6 @@ func MakeLambdaExtension(ext xdscommon.ExtensionConfiguration) (builtinextension
 		resultErr = multierror.Append(resultErr, fmt.Errorf("ARN is required"))
 	}
 
-	if plugin.Region == "" {
-		resultErr = multierror.Append(resultErr, fmt.Errorf("Region is required"))
-	}
-
 	plugin.Kind = ext.OutgoingProxyKind()
 
 	return plugin, resultErr
@@ -66,10 +62,14 @@ func toEnvoyInvocationMode(s string) envoy_lambda_v3.Config_InvocationMode {
 	return m
 }
 
+// CanApply returns true if the kind of the provided ExtensionConfiguration matches
+// the kind of the lambda configuration
 func (p lambda) CanApply(config xdscommon.ExtensionConfiguration) bool {
 	return config.Kind == p.Kind
 }
 
+// PatchRoute modifies the routing configuration for a service of kind TerminatingGateway. If the kind is
+// not TerminatingGateway, then it can not be modified.
 func (p lambda) PatchRoute(route *envoy_route_v3.RouteConfiguration) (*envoy_route_v3.RouteConfiguration, bool, error) {
 	if p.Kind != api.ServiceKindTerminatingGateway {
 		return route, false, nil
@@ -84,7 +84,8 @@ func (p lambda) PatchRoute(route *envoy_route_v3.RouteConfiguration) (*envoy_rou
 			}
 
 			// When auto_host_rewrite is set it conflicts with strip_any_host_port
-			// on the http_connection_manager filter.
+			// on the http_connection_manager filter, which is required to be true to support
+			// lambda functions. See the patch filter method for more details.
 			action.Route.HostRewriteSpecifier = nil
 		}
 	}
@@ -92,6 +93,7 @@ func (p lambda) PatchRoute(route *envoy_route_v3.RouteConfiguration) (*envoy_rou
 	return route, true, nil
 }
 
+// PatchCluster patches the provided envoy cluster with data required to support an AWS lambda function
 func (p lambda) PatchCluster(c *envoy_cluster_v3.Cluster) (*envoy_cluster_v3.Cluster, bool, error) {
 	transportSocket, err := makeUpstreamTLSTransportSocket(&envoy_tls_v3.UpstreamTlsContext{
 		Sni: "*.amazonaws.com",
@@ -101,6 +103,12 @@ func (p lambda) PatchCluster(c *envoy_cluster_v3.Cluster) (*envoy_cluster_v3.Clu
 		return c, false, fmt.Errorf("failed to make transport socket: %w", err)
 	}
 
+	// Use the aws SDK to parse the ARN so that we can later extract the region
+	parsedARN, err := arn_sdk.Parse(p.ARN)
+	if err != nil {
+		return c, false, err
+	}
+
 	cluster := &envoy_cluster_v3.Cluster{
 		Name:                 c.Name,
 		ConnectTimeout:       c.ConnectTimeout,
@@ -127,7 +135,7 @@ func (p lambda) PatchCluster(c *envoy_cluster_v3.Cluster) (*envoy_cluster_v3.Clu
 									Address: &envoy_core_v3.Address{
 										Address: &envoy_core_v3.Address_SocketAddress{
 											SocketAddress: &envoy_core_v3.SocketAddress{
-												Address: fmt.Sprintf("lambda.%s.amazonaws.com", p.Region),
+												Address: fmt.Sprintf("lambda.%s.amazonaws.com", parsedARN.Region),
 												PortSpecifier: &envoy_core_v3.SocketAddress_PortValue{
 													PortValue: 443,
 												},
@@ -146,6 +154,8 @@ func (p lambda) PatchCluster(c *envoy_cluster_v3.Cluster) (*envoy_cluster_v3.Clu
 	return cluster, true, nil
 }
 
+// PatchFilter patches the provided envoy filter with an inserted lambda filter being careful not to
+// overwrite the http filters.
 func (p lambda) PatchFilter(filter *envoy_listener_v3.Filter) (*envoy_listener_v3.Filter, bool, error) {
 	if filter.Name != "envoy.filters.network.http_connection_manager" {
 		return filter, false, nil
@@ -158,6 +168,7 @@ func (p lambda) PatchFilter(filter *envoy_listener_v3.Filter) (*envoy_listener_v
 	if config == nil {
 		return filter, false, errors.New("error unmarshalling filter")
 	}
+
 	lambdaHttpFilter, err := makeEnvoyHTTPFilter(
 		"envoy.filters.http.aws_lambda",
 		&envoy_lambda_v3.Config{
@@ -170,15 +181,12 @@ func (p lambda) PatchFilter(filter *envoy_listener_v3.Filter) (*envoy_listener_v
 		return filter, false, err
 	}
 
-	var (
-		changedFilters = make([]*envoy_http_v3.HttpFilter, 0, len(config.HttpFilters)+1)
-		changed        bool
-	)
-
 	// We need to be careful about overwriting http filters completely because
 	// http filters validates intentions with the RBAC filter. This inserts the
 	// lambda filter before `envoy.filters.http.router` while keeping everything
 	// else intact.
+	changedFilters := make([]*envoy_http_v3.HttpFilter, 0, len(config.HttpFilters)+1)
+	var changed bool
 	for _, httpFilter := range config.HttpFilters {
 		if httpFilter.Name == "envoy.filters.http.router" {
 			changedFilters = append(changedFilters, lambdaHttpFilter)
@@ -190,6 +198,9 @@ func (p lambda) PatchFilter(filter *envoy_listener_v3.Filter) (*envoy_listener_v
 		config.HttpFilters = changedFilters
 	}
 
+	// StripPortMode must be set to true since all requests have to be signed using the AWS v4 signature and
+	// if the port is included in the request, it will be used in the signature calculation causing AWS to reject the
+	// Lambda HTTP request.
 	config.StripPortMode = &envoy_http_v3.HttpConnectionManager_StripAnyHostPort{
 		StripAnyHostPort: true,
 	}

agent/xds/builtinextensions/lambda/lambda_test.go

@@ -1,9 +1,16 @@
 package lambda
 
 import (
+	"fmt"
 	"testing"
 
+	envoy_cluster_v3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
+	envoy_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
+	envoy_endpoint_v3 "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3"
+	envoy_tls_v3 "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	pstruct "google.golang.org/protobuf/types/known/structpb"
 
 	"github.com/hashicorp/consul/agent/xds/xdscommon"
 	"github.com/hashicorp/consul/api"
@@ -32,10 +39,6 @@ func TestMakeLambdaExtension(t *testing.T) {
 			region: "blah",
 			ok:     false,
 		},
-		"missing region": {
-			arn: "arn",
-			ok:  false,
-		},
 		"including payload passthrough": {
 			arn:                "arn",
 			region:             "blah",
@@ -43,7 +46,6 @@ func TestMakeLambdaExtension(t *testing.T) {
 			expected: lambda{
 				ARN:                "arn",
 				PayloadPassthrough: true,
-				Region:             "blah",
 				Kind:               kind,
 			},
 			ok: true,
@@ -66,7 +68,6 @@ func TestMakeLambdaExtension(t *testing.T) {
 					Name: extensionName,
 					Arguments: map[string]interface{}{
 						"ARN":                tc.arn,
-						"Region":             tc.region,
 						"PayloadPassthrough": tc.payloadPassthrough,
 					},
 				},
@@ -83,3 +84,103 @@ func TestMakeLambdaExtension(t *testing.T) {
 		})
 	}
 }
+
+func TestPatchCluster(t *testing.T) {
+	cases := []struct {
+		name           string
+		lambda         lambda
+		input          *envoy_cluster_v3.Cluster
+		expectedRegion string
+		isErrExpected  bool
+	}{
+		{
+			name: "nominal",
+			input: &envoy_cluster_v3.Cluster{
+				Name: "test-cluster",
+			},
+			lambda: lambda{
+				ARN:                "arn:aws:lambda:us-east-1:111111111111:function:lambda-1234",
+				PayloadPassthrough: true,
+				Kind:               "some-name",
+				InvocationMode:     "Asynchronous",
+			},
+			expectedRegion: "us-east-1",
+		},
+		{
+			name: "error invalid arn",
+			input: &envoy_cluster_v3.Cluster{
+				Name: "test-cluster",
+			},
+			lambda: lambda{
+				ARN:                "?!@%^SA",
+				PayloadPassthrough: true,
+				Kind:               "some-name",
+				InvocationMode:     "Asynchronous",
+			},
+			isErrExpected: true,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			transportSocket, err := makeUpstreamTLSTransportSocket(&envoy_tls_v3.UpstreamTlsContext{
+				Sni: "*.amazonaws.com",
+			})
+			require.NoError(t, err)
+
+			expectedCluster := &envoy_cluster_v3.Cluster{
+				Name:                 tc.input.Name,
+				ConnectTimeout:       tc.input.ConnectTimeout,
+				ClusterDiscoveryType: &envoy_cluster_v3.Cluster_Type{Type: envoy_cluster_v3.Cluster_LOGICAL_DNS},
+				DnsLookupFamily:      envoy_cluster_v3.Cluster_V4_ONLY,
+				LbPolicy:             envoy_cluster_v3.Cluster_ROUND_ROBIN,
+				Metadata: &envoy_core_v3.Metadata{
+					FilterMetadata: map[string]*pstruct.Struct{
+						"com.amazonaws.lambda": {
+							Fields: map[string]*pstruct.Value{
+								"egress_gateway": {Kind: &pstruct.Value_BoolValue{BoolValue: true}},
+							},
+						},
+					},
+				},
+				LoadAssignment: &envoy_endpoint_v3.ClusterLoadAssignment{
+					ClusterName: tc.input.Name,
+					Endpoints: []*envoy_endpoint_v3.LocalityLbEndpoints{
+						{
+							LbEndpoints: []*envoy_endpoint_v3.LbEndpoint{
+								{
+									HostIdentifier: &envoy_endpoint_v3.LbEndpoint_Endpoint{
+										Endpoint: &envoy_endpoint_v3.Endpoint{
+											Address: &envoy_core_v3.Address{
+												Address: &envoy_core_v3.Address_SocketAddress{
+													SocketAddress: &envoy_core_v3.SocketAddress{
+														Address: fmt.Sprintf("lambda.%s.amazonaws.com", tc.expectedRegion),
+														PortSpecifier: &envoy_core_v3.SocketAddress_PortValue{
+															PortValue: 443,
+														},
+													},
+												},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+				TransportSocket: transportSocket,
+			}
+
+			// Test patching the cluster
+			patchedCluster, patchSuccess, err := tc.lambda.PatchCluster(tc.input)
+			if tc.isErrExpected {
+				assert.Error(t, err)
+				assert.False(t, patchSuccess)
+			} else {
+				assert.NoError(t, err)
+				assert.True(t, patchSuccess)
+				assert.Equal(t, expectedCluster, patchedCluster)
+			}
+		})
+	}
+}

agent/xds/testdata/builtin_extension/listeners/lambda-connect-proxy-opposite-meta.latest.golden

@@ -44,7 +44,7 @@
                     "name": "envoy.filters.http.aws_lambda",
                     "typedConfig": {
                       "@type": "type.googleapis.com/envoy.extensions.filters.http.aws_lambda.v3.Config",
-                      "arn": "lambda-arn",
+                      "arn": "arn:aws:lambda:us-east-1:111111111111:function:lambda-1234",
                       "invocationMode": "ASYNCHRONOUS"
                     }
                   },

agent/xds/testdata/builtin_extension/listeners/lambda-connect-proxy.latest.golden

@@ -44,7 +44,7 @@
                     "name": "envoy.filters.http.aws_lambda",
                     "typedConfig": {
                       "@type": "type.googleapis.com/envoy.extensions.filters.http.aws_lambda.v3.Config",
-                      "arn": "lambda-arn",
+                      "arn": "arn:aws:lambda:us-east-1:111111111111:function:lambda-1234",
                       "payloadPassthrough": true
                     }
                   },

agent/xds/testdata/builtin_extension/listeners/lambda-terminating-gateway-with-service-resolvers.latest.golden

@@ -154,7 +154,7 @@
                     "name": "envoy.filters.http.aws_lambda",
                     "typedConfig": {
                       "@type": "type.googleapis.com/envoy.extensions.filters.http.aws_lambda.v3.Config",
-                      "arn": "lambda-arn",
+                      "arn": "arn:aws:lambda:us-east-1:111111111111:function:lambda-1234",
                       "payloadPassthrough": true
                     }
                   },
@@ -245,7 +245,7 @@
                     "name": "envoy.filters.http.aws_lambda",
                     "typedConfig": {
                       "@type": "type.googleapis.com/envoy.extensions.filters.http.aws_lambda.v3.Config",
-                      "arn": "lambda-arn",
+                      "arn": "arn:aws:lambda:us-east-1:111111111111:function:lambda-1234",
                       "payloadPassthrough": true
                     }
                   },
@@ -390,7 +390,7 @@
                     "name": "envoy.filters.http.aws_lambda",
                     "typedConfig": {
                       "@type": "type.googleapis.com/envoy.extensions.filters.http.aws_lambda.v3.Config",
-                      "arn": "lambda-arn",
+                      "arn": "arn:aws:lambda:us-east-1:111111111111:function:lambda-1234",
                       "payloadPassthrough": true
                     }
                   },

agent/xds/testdata/builtin_extension/listeners/lambda-terminating-gateway.latest.golden

@@ -208,7 +208,7 @@
                     "name": "envoy.filters.http.aws_lambda",
                     "typedConfig": {
                       "@type": "type.googleapis.com/envoy.extensions.filters.http.aws_lambda.v3.Config",
-                      "arn": "lambda-arn",
+                      "arn": "arn:aws:lambda:us-east-1:111111111111:function:lambda-1234",
                       "payloadPassthrough": true
                     }
                   },

agent/xds/xdscommon/xdscommon_oss_test.go

@@ -46,9 +46,8 @@ func TestGetExtensionConfigurations_TerminatingGateway(t *testing.T) {
 				EnvoyExtension: api.EnvoyExtension{
 					Name: structs.BuiltinAWSLambdaExtension,
 					Arguments: map[string]interface{}{
-						"ARN":                "lambda-arn",
+						"ARN":                "arn:aws:lambda:us-east-1:111111111111:function:lambda-1234",
 						"PayloadPassthrough": true,
-						"Region":             "us-east-1",
 					},
 				},
 				ServiceName: webService,
@@ -109,9 +108,8 @@ func TestGetExtensionConfigurations_ConnectProxy(t *testing.T) {
 		{
 			Name: structs.BuiltinAWSLambdaExtension,
 			Arguments: map[string]interface{}{
-				"ARN":                "lambda-arn",
+				"ARN":                "arn:aws:lambda:us-east-1:111111111111:function:lambda-1234",
 				"PayloadPassthrough": true,
-				"Region":             "us-east-1",
 			},
 		},
 		{
@@ -152,9 +150,8 @@ func TestGetExtensionConfigurations_ConnectProxy(t *testing.T) {
 						EnvoyExtension: api.EnvoyExtension{
 							Name: structs.BuiltinAWSLambdaExtension,
 							Arguments: map[string]interface{}{
-								"ARN":                "lambda-arn",
+								"ARN":                "arn:aws:lambda:us-east-1:111111111111:function:lambda-1234",
 								"PayloadPassthrough": true,
-								"Region":             "us-east-1",
 							},
 						},
 						ServiceName: dbService,
@@ -201,9 +198,8 @@ func TestGetExtensionConfigurations_ConnectProxy(t *testing.T) {
 						EnvoyExtension: api.EnvoyExtension{
 							Name: structs.BuiltinAWSLambdaExtension,
 							Arguments: map[string]interface{}{
-								"ARN":                "lambda-arn",
+								"ARN":                "arn:aws:lambda:us-east-1:111111111111:function:lambda-1234",
 								"PayloadPassthrough": true,
-								"Region":             "us-east-1",
 							},
 						},
 						ServiceName: dbService,
@@ -251,9 +247,8 @@ func TestGetExtensionConfigurations_ConnectProxy(t *testing.T) {
 						EnvoyExtension: api.EnvoyExtension{
 							Name: structs.BuiltinAWSLambdaExtension,
 							Arguments: map[string]interface{}{
-								"ARN":                "lambda-arn",
+								"ARN":                "arn:aws:lambda:us-east-1:111111111111:function:lambda-1234",
 								"PayloadPassthrough": true,
-								"Region":             "us-east-1",
 							},
 						},
 						ServiceName: webService,