From a1498b015d8e1d3d309ff8e7a6e9d88030caadf0 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson <62034708+wilkermichael@users.noreply.github.com> Date: Thu, 26 Jan 2023 15:44:52 -0800 Subject: [PATCH] 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 | 3 +- agent/xds/builtin_extension_oss_test.go | 3 +- agent/xds/builtinextensions/lambda/lambda.go | 35 ++++-- .../builtinextensions/lambda/lambda_test.go | 113 +++++++++++++++++- ...-connect-proxy-opposite-meta.latest.golden | 2 +- .../lambda-connect-proxy.latest.golden | 2 +- ...teway-with-service-resolvers.latest.golden | 6 +- .../lambda-terminating-gateway.latest.golden | 2 +- agent/xds/xdscommon/xdscommon_oss_test.go | 15 +-- 9 files changed, 143 insertions(+), 38 deletions(-) diff --git a/agent/proxycfg/testing_terminating_gateway.go b/agent/proxycfg/testing_terminating_gateway.go index d2945a35f4..f2c342c9f2 100644 --- a/agent/proxycfg/testing_terminating_gateway.go +++ b/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", }, }, }, diff --git a/agent/xds/builtin_extension_oss_test.go b/agent/xds/builtin_extension_oss_test.go index ed71104a2a..bd484a5b01 100644 --- a/agent/xds/builtin_extension_oss_test.go +++ b/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", }, }, }, diff --git a/agent/xds/builtinextensions/lambda/lambda.go b/agent/xds/builtinextensions/lambda/lambda.go index 9dea16ac0b..3ef0468e86 100644 --- a/agent/xds/builtinextensions/lambda/lambda.go +++ b/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, } diff --git a/agent/xds/builtinextensions/lambda/lambda_test.go b/agent/xds/builtinextensions/lambda/lambda_test.go index dbdd70bfc6..26a017a495 100644 --- a/agent/xds/builtinextensions/lambda/lambda_test.go +++ b/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) + } + }) + } +} diff --git a/agent/xds/testdata/builtin_extension/listeners/lambda-connect-proxy-opposite-meta.latest.golden b/agent/xds/testdata/builtin_extension/listeners/lambda-connect-proxy-opposite-meta.latest.golden index e37af5aa16..8f2299d453 100644 --- a/agent/xds/testdata/builtin_extension/listeners/lambda-connect-proxy-opposite-meta.latest.golden +++ b/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" } }, diff --git a/agent/xds/testdata/builtin_extension/listeners/lambda-connect-proxy.latest.golden b/agent/xds/testdata/builtin_extension/listeners/lambda-connect-proxy.latest.golden index 8dd8a34a87..701333156b 100644 --- a/agent/xds/testdata/builtin_extension/listeners/lambda-connect-proxy.latest.golden +++ b/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 } }, diff --git a/agent/xds/testdata/builtin_extension/listeners/lambda-terminating-gateway-with-service-resolvers.latest.golden b/agent/xds/testdata/builtin_extension/listeners/lambda-terminating-gateway-with-service-resolvers.latest.golden index 59000983e2..8ffefdb1b2 100644 --- a/agent/xds/testdata/builtin_extension/listeners/lambda-terminating-gateway-with-service-resolvers.latest.golden +++ b/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 } }, diff --git a/agent/xds/testdata/builtin_extension/listeners/lambda-terminating-gateway.latest.golden b/agent/xds/testdata/builtin_extension/listeners/lambda-terminating-gateway.latest.golden index 732272c3cb..4ebb04edb2 100644 --- a/agent/xds/testdata/builtin_extension/listeners/lambda-terminating-gateway.latest.golden +++ b/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 } }, diff --git a/agent/xds/xdscommon/xdscommon_oss_test.go b/agent/xds/xdscommon/xdscommon_oss_test.go index 7e46e4720e..709e159c78 100644 --- a/agent/xds/xdscommon/xdscommon_oss_test.go +++ b/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,