Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(lambda-nodejs): NodejsFunction construct incompatible with lambda@edge #9562

Merged
merged 17 commits into from
Aug 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions packages/@aws-cdk/aws-cloudfront/lib/private/cache-behavior.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,10 @@ export class CacheBehavior {
smoothStreaming: this.props.smoothStreaming,
viewerProtocolPolicy: this.props.viewerProtocolPolicy ?? ViewerProtocolPolicy.ALLOW_ALL,
lambdaFunctionAssociations: this.props.edgeLambdas
? this.props.edgeLambdas.map(edgeLambda => {
if (edgeLambda.functionVersion.version === '$LATEST') {
throw new Error('$LATEST function version cannot be used for Lambda@Edge');
}
return {
lambdaFunctionArn: edgeLambda.functionVersion.functionArn,
eventType: edgeLambda.eventType.toString(),
};
})
? this.props.edgeLambdas.map(edgeLambda => ({
lambdaFunctionArn: edgeLambda.functionVersion.edgeArn,
eventType: edgeLambda.eventType.toString(),
}))
: undefined,
};
}
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-cloudfront/lib/web_distribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -896,7 +896,7 @@ export class CloudFrontWebDistribution extends cdk.Resource implements IDistribu
lambdaFunctionAssociations: input.lambdaFunctionAssociations
.map(fna => ({
eventType: fna.eventType,
lambdaFunctionArn: fna.lambdaFunction && fna.lambdaFunction.functionArn,
lambdaFunctionArn: fna.lambdaFunction && fna.lambdaFunction.edgeArn,
})),
});

Expand Down
53 changes: 53 additions & 0 deletions packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,59 @@ describe('with Lambda@Edge functions', () => {
});
}).toThrow(/\$LATEST function version cannot be used for Lambda@Edge/);
});

test('with removable env vars', () => {
const envLambdaFunction = new lambda.Function(stack, 'EnvFunction', {
runtime: lambda.Runtime.NODEJS,
code: lambda.Code.fromInline('whateverwithenv'),
handler: 'index.handler',
});
envLambdaFunction.addEnvironment('KEY', 'value', { removeInEdge: true });

new Distribution(stack, 'MyDist', {
defaultBehavior: {
origin,
edgeLambdas: [
{
functionVersion: envLambdaFunction.currentVersion,
eventType: LambdaEdgeEventType.ORIGIN_REQUEST,
},
],
},
});

expect(stack).toHaveResource('AWS::Lambda::Function', {
Environment: ABSENT,
Code: {
ZipFile: 'whateverwithenv',
},
});
});

test('with incompatible env vars', () => {
const envLambdaFunction = new lambda.Function(stack, 'EnvFunction', {
runtime: lambda.Runtime.NODEJS,
code: lambda.Code.fromInline('whateverwithenv'),
handler: 'index.handler',
environment: {
KEY: 'value',
},
});

new Distribution(stack, 'MyDist', {
defaultBehavior: {
origin,
edgeLambdas: [
{
functionVersion: envLambdaFunction.currentVersion,
eventType: LambdaEdgeEventType.ORIGIN_REQUEST,
},
],
},
});

expect(() => app.synth()).toThrow(/KEY/);
});
});

test('price class is included if provided', () => {
Expand Down
91 changes: 80 additions & 11 deletions packages/@aws-cdk/aws-cloudfront/test/web_distribution.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { expect, haveResource, haveResourceLike } from '@aws-cdk/assert';
import { ABSENT, expect, haveResource, haveResourceLike } from '@aws-cdk/assert';
import * as certificatemanager from '@aws-cdk/aws-certificatemanager';
import * as lambda from '@aws-cdk/aws-lambda';
import * as s3 from '@aws-cdk/aws-s3';
Expand Down Expand Up @@ -426,8 +426,7 @@ nodeunitShim({
const stack = new cdk.Stack();
const sourceBucket = new s3.Bucket(stack, 'Bucket');

const lambdaFunction = new lambda.SingletonFunction(stack, 'Lambda', {
Copy link
Contributor Author

@jogold jogold Aug 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to move from a SingletonFunction to Function because SingletonFunction has no addVersion() it's a FunctionBase. This test was actually incorrect because it associated a latest version with a CF distribution.

SingletonFunction now has a _checkEdgeCompatibility() but no addVersion() yet, for this PR?

uuid: 'xxxx-xxxx-xxxx-xxxx',
const lambdaFunction = new lambda.Function(stack, 'Lambda', {
code: lambda.Code.inline('foo'),
handler: 'index.handler',
runtime: lambda.Runtime.NODEJS_10_X,
Expand All @@ -444,7 +443,7 @@ nodeunitShim({
isDefaultBehavior: true,
lambdaFunctionAssociations: [{
eventType: LambdaEdgeEventType.ORIGIN_REQUEST,
lambdaFunction: lambdaFunction.latestVersion,
lambdaFunction: lambdaFunction.addVersion('1'),
}],
},
],
Expand All @@ -459,13 +458,7 @@ nodeunitShim({
{
'EventType': 'origin-request',
'LambdaFunctionARN': {
'Fn::Join': [
'',
[
{ 'Fn::GetAtt': ['SingletonLambdaxxxxxxxxxxxxxxxx69D4268A', 'Arn'] },
':$LATEST',
],
],
'Ref': 'LambdaVersion1BB7548E1',
},
},
],
Expand All @@ -476,6 +469,82 @@ nodeunitShim({
test.done();
},

'associate a lambda with removable env vars'(test: Test) {
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack');
const sourceBucket = new s3.Bucket(stack, 'Bucket');

const lambdaFunction = new lambda.Function(stack, 'Lambda', {
code: lambda.Code.inline('foo'),
handler: 'index.handler',
runtime: lambda.Runtime.NODEJS_10_X,
});
lambdaFunction.addEnvironment('KEY', 'value', { removeInEdge: true });

new CloudFrontWebDistribution(stack, 'AnAmazingWebsiteProbably', {
originConfigs: [
{
s3OriginSource: {
s3BucketSource: sourceBucket,
},
behaviors: [
{
isDefaultBehavior: true,
lambdaFunctionAssociations: [{
eventType: LambdaEdgeEventType.ORIGIN_REQUEST,
lambdaFunction: lambdaFunction.addVersion('1'),
}],
},
],
},
],
});

expect(stack).to(haveResource('AWS::Lambda::Function', {
Environment: ABSENT,
}));

test.done();
},

'throws when associating a lambda with incompatible env vars'(test: Test) {
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack');
const sourceBucket = new s3.Bucket(stack, 'Bucket');

const lambdaFunction = new lambda.Function(stack, 'Lambda', {
code: lambda.Code.inline('foo'),
handler: 'index.handler',
runtime: lambda.Runtime.NODEJS_10_X,
environment: {
KEY: 'value',
},
});

new CloudFrontWebDistribution(stack, 'AnAmazingWebsiteProbably', {
originConfigs: [
{
s3OriginSource: {
s3BucketSource: sourceBucket,
},
behaviors: [
{
isDefaultBehavior: true,
lambdaFunctionAssociations: [{
eventType: LambdaEdgeEventType.ORIGIN_REQUEST,
lambdaFunction: lambdaFunction.addVersion('1'),
}],
},
],
},
],
});

test.throws(() => app.synth(), /KEY/);

test.done();
},

'distribution has a defaultChild'(test: Test) {
const stack = new cdk.Stack();
const sourceBucket = new s3.Bucket(stack, 'Bucket');
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-lambda-nodejs/lib/function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export class NodejsFunction extends lambda.Function {

// Enable connection reuse for aws-sdk
if (props.awsSdkConnectionReuse ?? true) {
this.addEnvironment('AWS_NODEJS_CONNECTION_REUSE_ENABLED', '1');
this.addEnvironment('AWS_NODEJS_CONNECTION_REUSE_ENABLED', '1', { removeInEdge: true });
}
} finally {
// We can only restore after the code has been bound to the function
Expand Down
13 changes: 13 additions & 0 deletions packages/@aws-cdk/aws-lambda/lib/function-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,15 @@ export abstract class FunctionBase extends Resource implements IFunction {
});
}

/**
* Checks whether this function is compatible for Lambda@Edge.
*
* @internal
*/
public _checkEdgeCompatibility(): void {
nija-at marked this conversation as resolved.
Show resolved Hide resolved
return;
}

/**
* Returns the construct tree node that corresponds to the lambda function.
* For use internally for constructs, when the tree is set up in non-standard ways. Ex: SingletonFunction.
Expand Down Expand Up @@ -417,4 +426,8 @@ class LatestVersion extends FunctionBase implements IVersion {
public addAlias(aliasName: string, options: AliasOptions = {}) {
return addAlias(this, this, aliasName, options);
}

public get edgeArn(): never {
throw new Error('$LATEST function version cannot be used for Lambda@Edge');
}
}
77 changes: 59 additions & 18 deletions packages/@aws-cdk/aws-lambda/lib/function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ export class Function extends FunctionBase {
/**
* Environment variables for this function
*/
private readonly environment: { [key: string]: string };
private environment: { [key: string]: EnvironmentConfig } = {};

private readonly currentVersionOptions?: VersionOptions;
private _currentVersion?: Version;
Expand Down Expand Up @@ -558,7 +558,7 @@ export class Function extends FunctionBase {
const code = props.code.bind(this);
verifyCodeConfig(code, props.runtime);

let profilingGroupEnvironmentVariables = {};
let profilingGroupEnvironmentVariables: { [key: string]: string } = {};
if (props.profilingGroup && props.profiling !== false) {
this.validateProfilingEnvironmentVariables(props);
props.profilingGroup.grantPublish(this.role);
Expand All @@ -582,7 +582,10 @@ export class Function extends FunctionBase {
};
}

this.environment = { ...profilingGroupEnvironmentVariables, ...(props.environment || {}) };
const env = { ...profilingGroupEnvironmentVariables, ...props.environment };
for (const [key, value] of Object.entries(env)) {
this.addEnvironment(key, value);
}

this.deadLetterQueue = this.buildDeadLetterQueue(props);

Expand Down Expand Up @@ -675,9 +678,10 @@ export class Function extends FunctionBase {
* If this is a ref to a Lambda function, this operation results in a no-op.
* @param key The environment variable key.
* @param value The environment variable's value.
* @param options Environment variable options.
*/
public addEnvironment(key: string, value: string): this {
this.environment[key] = value;
public addEnvironment(key: string, value: string, options?: EnvironmentOptions): this {
this.environment[key] = { value, ...options };
return this;
}

Expand Down Expand Up @@ -764,27 +768,43 @@ export class Function extends FunctionBase {
return this._logGroup;
}

/** @internal */
public _checkEdgeCompatibility(): void {
// Check env vars
const envEntries = Object.entries(this.environment);
for (const [key, config] of envEntries) {
if (config.removeInEdge) {
delete this.environment[key];
this.node.addInfo(`Removed ${key} environment variable for Lambda@Edge compatibility`);
}
}
const envKeys = Object.keys(this.environment);
if (envKeys.length !== 0) {
throw new Error(`The function ${this.node.path} contains environment variables [${envKeys}] and is not compatible with Lambda@Edge. \
Environment variables can be marked for removal when used in Lambda@Edge by setting the \'removeInEdge\' property in the \'addEnvironment()\' API.`);
}

return;
}

private renderEnvironment() {
if (!this.environment || Object.keys(this.environment).length === 0) {
return undefined;
}

// for backwards compatibility we do not sort environment variables in case
// _currentVersion is not defined. otherwise, this would have invalidated
const variables: { [key: string]: string } = {};
// Sort environment so the hash of the function used to create
// `currentVersion` is not affected by key order (this is how lambda does
// it). For backwards compatibility we do not sort environment variables in case
// _currentVersion is not defined. Otherwise, this would have invalidated
// the template, and for example, may cause unneeded updates for nested
// stacks.
if (!this._currentVersion) {
return {
variables: this.environment,
};
}
const keys = this._currentVersion
? Object.keys(this.environment).sort()
: Object.keys(this.environment);

// sort environment so the hash of the function used to create
// `currentVersion` is not affected by key order (this is how lambda does
// it).
const variables: { [key: string]: string } = {};
for (const key of Object.keys(this.environment).sort()) {
variables[key] = this.environment[key];
for (const key of keys) {
variables[key] = this.environment[key].value;
}

return { variables };
Expand Down Expand Up @@ -905,6 +925,27 @@ export class Function extends FunctionBase {
}
}

/**
* Environment variables options
*/
export interface EnvironmentOptions {
/**
* When used in Lambda@Edge via edgeArn() API, these environment
* variables will be removed. If not set, an error will be thrown.
* @see https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/lambda-requirements-limits.html#lambda-requirements-lambda-function-configuration
*
* @default false - using the function in Lambda@Edge will throw
*/
readonly removeInEdge?: boolean
}

/**
* Configuration for an environment variable
*/
interface EnvironmentConfig extends EnvironmentOptions {
readonly value: string;
}

/**
* Given an opaque (token) ARN, returns a CloudFormation expression that extracts the function
* name from the ARN.
Expand Down
Loading