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

Allows Fn::GetAtt with Lambda DLQ-onError #5139

Merged
merged 2 commits into from
Jan 28, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/providers/aws/guide/functions.md
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ provider:
functions: functions:
hello: hello:
handler: handler.hello handler: handler.hello
onError: arn:aws:sns:us-east-1:XXXXXX:test # Ref and Fn::ImportValue are supported as well onError: arn:aws:sns:us-east-1:XXXXXX:test # Ref, Fn::GetAtt and Fn::ImportValue are supported as well
``` ```


### DLQ with SQS ### DLQ with SQS
Expand Down
2 changes: 1 addition & 1 deletion docs/providers/aws/guide/serverless.yml.md
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ functions:
runtime: nodejs6.10 # Runtime for this specific function. Overrides the default which is set on the provider level runtime: nodejs6.10 # Runtime for this specific function. Overrides the default which is set on the provider level
timeout: 10 # Timeout for this specific function. Overrides the default set above. timeout: 10 # Timeout for this specific function. Overrides the default set above.
role: arn:aws:iam::XXXXXX:role/role # IAM role which will be used for this function role: arn:aws:iam::XXXXXX:role/role # IAM role which will be used for this function
onError: arn:aws:sns:us-east-1:XXXXXX:sns-topic # Optional SNS topic arn (Ref and Fn::ImportValue are supported as well) which will be used for the DeadLetterConfig onError: arn:aws:sns:us-east-1:XXXXXX:sns-topic # Optional SNS topic / SQS arn (Ref, Fn::GetAtt and Fn::ImportValue are supported as well) which will be used for the DeadLetterConfig
awsKmsKeyArn: arn:aws:kms:us-east-1:XXXXXX:key/some-hash # Optional KMS key arn which will be used for encryption (overwrites the one defined on the service level) awsKmsKeyArn: arn:aws:kms:us-east-1:XXXXXX:key/some-hash # Optional KMS key arn which will be used for encryption (overwrites the one defined on the service level)
environment: # Function level environment variables environment: # Function level environment variables
functionEnvVar: 12345678 functionEnvVar: 12345678
Expand Down
8 changes: 4 additions & 4 deletions lib/plugins/aws/package/compile/functions/index.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -184,14 +184,14 @@ class AwsCompileFunctions {
const errorMessage = 'onError config must be a SNS topic arn or SQS queue arn'; const errorMessage = 'onError config must be a SNS topic arn or SQS queue arn';
return BbPromise.reject(new this.serverless.classes.Error(errorMessage)); return BbPromise.reject(new this.serverless.classes.Error(errorMessage));
} }
} else if (this.isArnRefOrImportValue(arn)) { } else if (this.isArnRefGetAttOrImportValue(arn)) {
newFunction.Properties.DeadLetterConfig = { newFunction.Properties.DeadLetterConfig = {
TargetArn: arn, TargetArn: arn,
}; };
} else { } else {
const errorMessage = [ const errorMessage = [
'onError config must be provided as an arn string,', 'onError config must be provided as an arn string,',
' Ref or Fn::ImportValue', ' Ref, Fn::GetAtt or Fn::ImportValue',
].join(''); ].join('');
return BbPromise.reject(new this.serverless.classes.Error(errorMessage)); return BbPromise.reject(new this.serverless.classes.Error(errorMessage));
} }
Expand Down Expand Up @@ -407,9 +407,9 @@ class AwsCompileFunctions {
} }


// helper functions // helper functions
isArnRefOrImportValue(arn) { isArnRefGetAttOrImportValue(arn) {
return typeof arn === 'object' && return typeof arn === 'object' &&
_.some(_.keys(arn), (k) => _.includes(['Ref', 'Fn::ImportValue'], k)); _.some(_.keys(arn), (k) => _.includes(['Ref', 'Fn::GetAtt', 'Fn::ImportValue'], k));
} }


cfLambdaFunctionTemplate() { cfLambdaFunctionTemplate() {
Expand Down
57 changes: 53 additions & 4 deletions lib/plugins/aws/package/compile/functions/index.test.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -61,14 +61,17 @@ describe('AwsCompileFunctions', () => {
expect(awsCompileFunctions.provider).to.be.instanceof(AwsProvider)); expect(awsCompileFunctions.provider).to.be.instanceof(AwsProvider));
}); });


describe('#isArnRefOrImportValue()', () => { describe('#isArnRefGetAttOrImportValue()', () => {
it('should accept a Ref', () => it('should accept a Ref', () =>
expect(awsCompileFunctions.isArnRefOrImportValue({ Ref: 'DLQ' })).to.equal(true)); expect(awsCompileFunctions.isArnRefGetAttOrImportValue({ Ref: 'DLQ' })).to.equal(true));
it('should accept a Fn::GetAtt', () =>
expect(awsCompileFunctions.isArnRefGetAttOrImportValue({ 'Fn::GetAtt': ['DLQ', 'Arn'] }))
.to.equal(true));
it('should accept a Fn::ImportValue', () => it('should accept a Fn::ImportValue', () =>
expect(awsCompileFunctions.isArnRefOrImportValue({ 'Fn::ImportValue': 'DLQ' })) expect(awsCompileFunctions.isArnRefGetAttOrImportValue({ 'Fn::ImportValue': 'DLQ' }))
.to.equal(true)); .to.equal(true));
it('should reject other objects', () => it('should reject other objects', () =>
expect(awsCompileFunctions.isArnRefOrImportValue({ Blah: 'vtha' })).to.equal(false)); expect(awsCompileFunctions.isArnRefGetAttOrImportValue({ Blah: 'vtha' })).to.equal(false));
}); });


describe('#compileFunctions()', () => { describe('#compileFunctions()', () => {
Expand Down Expand Up @@ -896,6 +899,52 @@ describe('AwsCompileFunctions', () => {
expect(functionResource).to.deep.equal(compiledFunction); expect(functionResource).to.deep.equal(compiledFunction);
}); });
}); });

it('should create necessary resources if a Fn::GetAtt is provided', () => {
awsCompileFunctions.serverless.service.functions = {
func: {
handler: 'func.function.handler',
name: 'new-service-dev-func',
onError: {
'Fn::GetAtt': ['DLQ', 'Arn'],
},
},
};

const compiledFunction = {
Type: 'AWS::Lambda::Function',
DependsOn: [
'FuncLogGroup',
'IamRoleLambdaExecution',
],
Properties: {
Code: {
S3Key: `${s3Folder}/${s3FileName}`,
S3Bucket: { Ref: 'ServerlessDeploymentBucket' },
},
FunctionName: 'new-service-dev-func',
Handler: 'func.function.handler',
MemorySize: 1024,
Role: { 'Fn::GetAtt': ['IamRoleLambdaExecution', 'Arn'] },
Runtime: 'nodejs4.3',
Timeout: 6,
DeadLetterConfig: {
TargetArn: {
'Fn::GetAtt': ['DLQ', 'Arn'],
},
},
},
};

return expect(awsCompileFunctions.compileFunctions()).to.be.fulfilled
.then(() => {
const compiledCfTemplate = awsCompileFunctions.serverless.service.provider
.compiledCloudFormationTemplate;

const functionResource = compiledCfTemplate.Resources.FuncLambdaFunction;
expect(functionResource).to.deep.equal(compiledFunction);
});
});
}); });


describe('when IamRoleLambdaExecution is not used', () => { describe('when IamRoleLambdaExecution is not used', () => {
Expand Down