From f8bb85fad8f659a2b72d5d05d7a94c97765a76f8 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy <36202692+kaizen3031593@users.noreply.github.com> Date: Mon, 14 Feb 2022 12:13:02 -0500 Subject: [PATCH] fix(synthetics): generated role has incorrect permissions for cloudwatch logs (#18946) The generated role did not have the correct permissions to create cloudwatch logs, so even if the canary successfully deployed and ran, no cloudwatch streams were generated for the resource. This seems to be a long-standing bug in the synthetics module. What was missing was the region and account id, which should be the same region/account as the created canary. Fixes #18910. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../@aws-cdk/aws-synthetics/lib/canary.ts | 12 ++- .../aws-synthetics/test/canary.test.ts | 95 +++++++++++++++++++ .../test/integ.canary.expected.json | 50 +++++++++- 3 files changed, 150 insertions(+), 7 deletions(-) diff --git a/packages/@aws-cdk/aws-synthetics/lib/canary.ts b/packages/@aws-cdk/aws-synthetics/lib/canary.ts index c078fd0718ce7..fab47a960289a 100644 --- a/packages/@aws-cdk/aws-synthetics/lib/canary.ts +++ b/packages/@aws-cdk/aws-synthetics/lib/canary.ts @@ -290,7 +290,6 @@ export class Canary extends cdk.Resource { * Returns a default role for the canary */ private createDefaultRole(prefix?: string): iam.IRole { - const { partition } = cdk.Stack.of(this); // Created role will need these policies to run the Canary. // https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-synthetics-canary.html#cfn-synthetics-canary-executionrolearn const policy = new iam.PolicyDocument({ @@ -313,7 +312,7 @@ export class Canary extends cdk.Resource { conditions: { StringEquals: { 'cloudwatch:namespace': 'CloudWatchSynthetics' } }, }), new iam.PolicyStatement({ - resources: [`arn:${partition}:logs:::*`], + resources: [this.logGroupArn()], actions: ['logs:CreateLogStream', 'logs:CreateLogGroup', 'logs:PutLogEvents'], }), ], @@ -327,6 +326,15 @@ export class Canary extends cdk.Resource { }); } + private logGroupArn() { + return cdk.Stack.of(this).formatArn({ + service: 'logs', + resource: 'log-group', + arnFormat: cdk.ArnFormat.COLON_RESOURCE_NAME, + resourceName: '/aws/lambda/cwsyn-*', + }); + } + /** * Returns the code object taken in by the canary resource. */ diff --git a/packages/@aws-cdk/aws-synthetics/test/canary.test.ts b/packages/@aws-cdk/aws-synthetics/test/canary.test.ts index 83ba173562834..873bf36d61d6d 100644 --- a/packages/@aws-cdk/aws-synthetics/test/canary.test.ts +++ b/packages/@aws-cdk/aws-synthetics/test/canary.test.ts @@ -440,3 +440,98 @@ test('can specify custom test', () => { }, }); }); + +test('Role policy generated as expected', () => { + // GIVEN + const stack = new Stack(); + + // WHEN + new synthetics.Canary(stack, 'Canary', { + test: synthetics.Test.custom({ + handler: 'index.handler', + code: synthetics.Code.fromInline('/* Synthetics handler code */'), + }), + runtime: synthetics.Runtime.SYNTHETICS_NODEJS_PUPPETEER_3_3, + }); + + // THEN + Template.fromStack(stack).hasResourceProperties('AWS::IAM::Role', { + Policies: [{ + PolicyDocument: { + Statement: [ + { + Action: 's3:ListAllMyBuckets', + Effect: 'Allow', + Resource: '*', + }, + { + Action: 's3:GetBucketLocation', + Effect: 'Allow', + Resource: { + 'Fn::GetAtt': [ + 'CanaryArtifactsBucket4A60D32B', + 'Arn', + ], + }, + }, + { + Action: 's3:PutObject', + Effect: 'Allow', + Resource: { + 'Fn::Join': [ + '', + [ + { + 'Fn::GetAtt': [ + 'CanaryArtifactsBucket4A60D32B', + 'Arn', + ], + }, + '/*', + ], + ], + }, + }, + { + Action: 'cloudwatch:PutMetricData', + Condition: { + StringEquals: { + 'cloudwatch:namespace': 'CloudWatchSynthetics', + }, + }, + Effect: 'Allow', + Resource: '*', + }, + { + Action: [ + 'logs:CreateLogStream', + 'logs:CreateLogGroup', + 'logs:PutLogEvents', + ], + Effect: 'Allow', + Resource: { + 'Fn::Join': [ + '', + [ + 'arn:', + { + Ref: 'AWS::Partition', + }, + ':logs:', + { + Ref: 'AWS::Region', + }, + ':', + { + Ref: 'AWS::AccountId', + }, + ':log-group:/aws/lambda/cwsyn-*', + ], + ], + }, + }, + ], + }, + }], + }); +}); diff --git a/packages/@aws-cdk/aws-synthetics/test/integ.canary.expected.json b/packages/@aws-cdk/aws-synthetics/test/integ.canary.expected.json index 2425fad01de67..aa88a65ad353a 100644 --- a/packages/@aws-cdk/aws-synthetics/test/integ.canary.expected.json +++ b/packages/@aws-cdk/aws-synthetics/test/integ.canary.expected.json @@ -82,7 +82,15 @@ { "Ref": "AWS::Partition" }, - ":logs:::*" + ":logs:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":log-group:/aws/lambda/cwsyn-*" ] ] } @@ -269,7 +277,15 @@ { "Ref": "AWS::Partition" }, - ":logs:::*" + ":logs:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":log-group:/aws/lambda/cwsyn-*" ] ] } @@ -490,7 +506,15 @@ { "Ref": "AWS::Partition" }, - ":logs:::*" + ":logs:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":log-group:/aws/lambda/cwsyn-*" ] ] } @@ -711,7 +735,15 @@ { "Ref": "AWS::Partition" }, - ":logs:::*" + ":logs:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":log-group:/aws/lambda/cwsyn-*" ] ] } @@ -932,7 +964,15 @@ { "Ref": "AWS::Partition" }, - ":logs:::*" + ":logs:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":log-group:/aws/lambda/cwsyn-*" ] ] }