Skip to content

Commit fcca058

Browse files
njlynchnjlynch
committed
fix(lambda): grantInvoke fails on second invocation
If `grantInvoke()` is called twice for the same principal, the second call fails due to attempting to create two `CfnPermission` nodes with the same id. This (simple) fix skips the second creation if the node already exists. A more robust check would be to check the existing `CfnPermission`, and compare every field, skipping creation if the two are identical and throwing an error otherwise, as well as handling that in the upstream `grantInvoke` call. I opted for the simpler solution for now, but willing to take arguments for something more complex. I also nested the existing grantInvoke tests for future readability. fixes #8553
1 parent 0ca09a7 commit fcca058

File tree

2 files changed

+142
-107
lines changed

2 files changed

+142
-107
lines changed

‎packages/@aws-cdk/aws-lambda/lib/function-base.ts

+11-8
Original file line numberDiff line numberDiff line change
@@ -209,14 +209,17 @@ export abstract class FunctionBase extends Resource implements IFunction {
209209
const action = permission.action || 'lambda:InvokeFunction';
210210
const scope = permission.scope || this;
211211

212-
new CfnPermission(scope, id, {
213-
action,
214-
principal,
215-
functionName: this.functionArn,
216-
eventSourceToken: permission.eventSourceToken,
217-
sourceAccount: permission.sourceAccount,
218-
sourceArn: permission.sourceArn,
219-
});
212+
// Skip duplicate permissions nodes. A more robust check would verify if the existing node matches the new one exactly.
213+
if (!scope.node.tryFindChild(id)) {
214+
new CfnPermission(scope, id, {
215+
action,
216+
principal,
217+
functionName: this.functionArn,
218+
eventSourceToken: permission.eventSourceToken,
219+
sourceAccount: permission.sourceAccount,
220+
sourceArn: permission.sourceArn,
221+
});
222+
}
220223
}
221224

222225
/**

‎packages/@aws-cdk/aws-lambda/test/test.lambda.ts

+131-99
Original file line numberDiff line numberDiff line change
@@ -1020,120 +1020,152 @@ export = {
10201020
test.done();
10211021
},
10221022

1023-
'grantInvoke adds iam:InvokeFunction'(test: Test) {
1024-
// GIVEN
1025-
const stack = new cdk.Stack();
1026-
const role = new iam.Role(stack, 'Role', {
1027-
assumedBy: new iam.AccountPrincipal('1234'),
1028-
});
1029-
const fn = new lambda.Function(stack, 'Function', {
1030-
code: lambda.Code.fromInline('xxx'),
1031-
handler: 'index.handler',
1032-
runtime: lambda.Runtime.NODEJS_10_X,
1033-
});
1023+
'grantInvoke': {
10341024

1035-
// WHEN
1036-
fn.grantInvoke(role);
1025+
'adds iam:InvokeFunction'(test: Test) {
1026+
// GIVEN
1027+
const stack = new cdk.Stack();
1028+
const role = new iam.Role(stack, 'Role', {
1029+
assumedBy: new iam.AccountPrincipal('1234'),
1030+
});
1031+
const fn = new lambda.Function(stack, 'Function', {
1032+
code: lambda.Code.fromInline('xxx'),
1033+
handler: 'index.handler',
1034+
runtime: lambda.Runtime.NODEJS_10_X,
1035+
});
10371036

1038-
// THEN
1039-
expect(stack).to(haveResource('AWS::IAM::Policy', {
1040-
PolicyDocument: {
1041-
Version: '2012-10-17',
1042-
Statement: [
1043-
{
1044-
Action: 'lambda:InvokeFunction',
1045-
Effect: 'Allow',
1046-
Resource: { 'Fn::GetAtt': ['Function76856677', 'Arn'] },
1047-
},
1048-
],
1049-
},
1050-
}));
1037+
// WHEN
1038+
fn.grantInvoke(role);
10511039

1052-
test.done();
1053-
},
1040+
// THEN
1041+
expect(stack).to(haveResource('AWS::IAM::Policy', {
1042+
PolicyDocument: {
1043+
Version: '2012-10-17',
1044+
Statement: [
1045+
{
1046+
Action: 'lambda:InvokeFunction',
1047+
Effect: 'Allow',
1048+
Resource: { 'Fn::GetAtt': ['Function76856677', 'Arn'] },
1049+
},
1050+
],
1051+
},
1052+
}));
10541053

1055-
'grantInvoke with a service principal'(test: Test) {
1056-
// GIVEN
1057-
const stack = new cdk.Stack();
1058-
const fn = new lambda.Function(stack, 'Function', {
1059-
code: lambda.Code.fromInline('xxx'),
1060-
handler: 'index.handler',
1061-
runtime: lambda.Runtime.NODEJS_10_X,
1062-
});
1063-
const service = new iam.ServicePrincipal('apigateway.amazonaws.com');
1054+
test.done();
1055+
},
10641056

1065-
// WHEN
1066-
fn.grantInvoke(service);
1057+
'with a service principal'(test: Test) {
1058+
// GIVEN
1059+
const stack = new cdk.Stack();
1060+
const fn = new lambda.Function(stack, 'Function', {
1061+
code: lambda.Code.fromInline('xxx'),
1062+
handler: 'index.handler',
1063+
runtime: lambda.Runtime.NODEJS_10_X,
1064+
});
1065+
const service = new iam.ServicePrincipal('apigateway.amazonaws.com');
10671066

1068-
// THEN
1069-
expect(stack).to(haveResource('AWS::Lambda::Permission', {
1070-
Action: 'lambda:InvokeFunction',
1071-
FunctionName: {
1072-
'Fn::GetAtt': [
1073-
'Function76856677',
1074-
'Arn',
1075-
],
1076-
},
1077-
Principal: 'apigateway.amazonaws.com',
1078-
}));
1067+
// WHEN
1068+
fn.grantInvoke(service);
10791069

1080-
test.done();
1081-
},
1070+
// THEN
1071+
expect(stack).to(haveResource('AWS::Lambda::Permission', {
1072+
Action: 'lambda:InvokeFunction',
1073+
FunctionName: {
1074+
'Fn::GetAtt': [
1075+
'Function76856677',
1076+
'Arn',
1077+
],
1078+
},
1079+
Principal: 'apigateway.amazonaws.com',
1080+
}));
10821081

1083-
'grantInvoke with an account principal'(test: Test) {
1084-
// GIVEN
1085-
const stack = new cdk.Stack();
1086-
const fn = new lambda.Function(stack, 'Function', {
1087-
code: lambda.Code.fromInline('xxx'),
1088-
handler: 'index.handler',
1089-
runtime: lambda.Runtime.NODEJS_10_X,
1090-
});
1091-
const account = new iam.AccountPrincipal('123456789012');
1082+
test.done();
1083+
},
10921084

1093-
// WHEN
1094-
fn.grantInvoke(account);
1085+
'with an account principal'(test: Test) {
1086+
// GIVEN
1087+
const stack = new cdk.Stack();
1088+
const fn = new lambda.Function(stack, 'Function', {
1089+
code: lambda.Code.fromInline('xxx'),
1090+
handler: 'index.handler',
1091+
runtime: lambda.Runtime.NODEJS_10_X,
1092+
});
1093+
const account = new iam.AccountPrincipal('123456789012');
10951094

1096-
// THEN
1097-
expect(stack).to(haveResource('AWS::Lambda::Permission', {
1098-
Action: 'lambda:InvokeFunction',
1099-
FunctionName: {
1100-
'Fn::GetAtt': [
1101-
'Function76856677',
1102-
'Arn',
1103-
],
1104-
},
1105-
Principal: '123456789012',
1106-
}));
1095+
// WHEN
1096+
fn.grantInvoke(account);
11071097

1108-
test.done();
1109-
},
1098+
// THEN
1099+
expect(stack).to(haveResource('AWS::Lambda::Permission', {
1100+
Action: 'lambda:InvokeFunction',
1101+
FunctionName: {
1102+
'Fn::GetAtt': [
1103+
'Function76856677',
1104+
'Arn',
1105+
],
1106+
},
1107+
Principal: '123456789012',
1108+
}));
11101109

1111-
'grantInvoke with an arn principal'(test: Test) {
1112-
// GIVEN
1113-
const stack = new cdk.Stack();
1114-
const fn = new lambda.Function(stack, 'Function', {
1115-
code: lambda.Code.fromInline('xxx'),
1116-
handler: 'index.handler',
1117-
runtime: lambda.Runtime.NODEJS_10_X,
1118-
});
1119-
const account = new iam.ArnPrincipal('arn:aws:iam::123456789012:role/someRole');
1110+
test.done();
1111+
},
11201112

1121-
// WHEN
1122-
fn.grantInvoke(account);
1113+
'with an arn principal'(test: Test) {
1114+
// GIVEN
1115+
const stack = new cdk.Stack();
1116+
const fn = new lambda.Function(stack, 'Function', {
1117+
code: lambda.Code.fromInline('xxx'),
1118+
handler: 'index.handler',
1119+
runtime: lambda.Runtime.NODEJS_10_X,
1120+
});
1121+
const account = new iam.ArnPrincipal('arn:aws:iam::123456789012:role/someRole');
11231122

1124-
// THEN
1125-
expect(stack).to(haveResource('AWS::Lambda::Permission', {
1126-
Action: 'lambda:InvokeFunction',
1127-
FunctionName: {
1128-
'Fn::GetAtt': [
1129-
'Function76856677',
1130-
'Arn',
1131-
],
1132-
},
1133-
Principal: 'arn:aws:iam::123456789012:role/someRole',
1134-
}));
1123+
// WHEN
1124+
fn.grantInvoke(account);
11351125

1136-
test.done();
1126+
// THEN
1127+
expect(stack).to(haveResource('AWS::Lambda::Permission', {
1128+
Action: 'lambda:InvokeFunction',
1129+
FunctionName: {
1130+
'Fn::GetAtt': [
1131+
'Function76856677',
1132+
'Arn',
1133+
],
1134+
},
1135+
Principal: 'arn:aws:iam::123456789012:role/someRole',
1136+
}));
1137+
1138+
test.done();
1139+
},
1140+
1141+
'can be called twice for the same service principal'(test: Test) {
1142+
// GIVEN
1143+
const stack = new cdk.Stack();
1144+
const fn = new lambda.Function(stack, 'Function', {
1145+
code: lambda.Code.fromInline('xxx'),
1146+
handler: 'index.handler',
1147+
runtime: lambda.Runtime.NODEJS_10_X,
1148+
});
1149+
const service = new iam.ServicePrincipal('elasticloadbalancing.amazonaws.com');
1150+
1151+
// WHEN
1152+
fn.grantInvoke(service);
1153+
fn.grantInvoke(service);
1154+
1155+
// THEN
1156+
expect(stack).to(haveResource('AWS::Lambda::Permission', {
1157+
Action: 'lambda:InvokeFunction',
1158+
FunctionName: {
1159+
'Fn::GetAtt': [
1160+
'Function76856677',
1161+
'Arn',
1162+
],
1163+
},
1164+
Principal: 'elasticloadbalancing.amazonaws.com',
1165+
}));
1166+
1167+
test.done();
1168+
},
11371169
},
11381170

11391171
'Can use metricErrors on a lambda Function'(test: Test) {

0 commit comments

Comments
 (0)