fix(logs): grants don't work on imported LogGroups

In CloudFormation, `{ Fn::GetAtt: [MyLogGroup, Arn] }` always returned
the ARN with a `:*` appended, presumably so you could stick the returned
value directly into an IAM policy and get the result you wanted (doing
something to Log Groups usually entails doing something to the Log
*Streams* inside them).

The CDK construct did not do anything special, leading to imports done
without a `:*` at the end having incorrect permissions.

This change makes the behavior between imported and constructed Log
Groups consistent.

Fixes #7096.

packages/@aws-cdk/aws-logs/README.md

@@ -216,3 +216,9 @@ const pattern = FilterPattern.spaceDelimited('time', 'component', '...', 'result
     .whereString('component', '=', 'HttpServer')
     .whereNumber('result_code', '!=', 200);
 ```
+
+### Notes
+
+Be aware that Log Group ARNs will always have the string `:*` appended to
+them, to match the behavior of [the CloudFormation `AWS::Logs::LogGroup`
+resource](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-logs-loggroup.html#aws-resource-logs-loggroup-return-values).

packages/@aws-cdk/aws-logs/lib/log-group.ts

@@ -9,7 +9,8 @@ import { ILogSubscriptionDestination, SubscriptionFilter } from './subscription-
 
 export interface ILogGroup extends IResource {
   /**
-   * The ARN of this log group
+   * The ARN of this log group, with ':*' appended
+   *
    * @attribute
    */
   readonly logGroupArn: string;
@@ -76,7 +77,7 @@ export interface ILogGroup extends IResource {
  */
 abstract class LogGroupBase extends Resource implements ILogGroup {
   /**
-   * The ARN of this log group
+   * The ARN of this log group, with ':*' appended
    */
   public abstract readonly logGroupArn: string;
 
@@ -308,9 +309,11 @@ export class LogGroup extends LogGroupBase {
    * Import an existing LogGroup given its ARN
    */
   public static fromLogGroupArn(scope: Construct, id: string, logGroupArn: string): ILogGroup {
+    const baseLogGroupArn = logGroupArn.replace(/:\*$/, '');
+
     class Import extends LogGroupBase {
-      public readonly logGroupArn = logGroupArn;
-      public readonly logGroupName = Stack.of(scope).parseArn(logGroupArn, ':').resourceName!;
+      public readonly logGroupArn = `${baseLogGroupArn}:*`;
+      public readonly logGroupName = Stack.of(scope).parseArn(baseLogGroupArn, ':').resourceName!;
     }
 
     return new Import(scope, id);
@@ -320,13 +323,15 @@ export class LogGroup extends LogGroupBase {
    * Import an existing LogGroup given its name
    */
   public static fromLogGroupName(scope: Construct, id: string, logGroupName: string): ILogGroup {
+    const baseLogGroupName = logGroupName.replace(/:\*$/, '');
+
     class Import extends LogGroupBase {
-      public readonly logGroupName = logGroupName;
+      public readonly logGroupName = baseLogGroupName;
       public readonly logGroupArn = Stack.of(scope).formatArn({
         service: 'logs',
         resource: 'log-group',
         sep: ':',
-        resourceName: logGroupName,
+        resourceName: baseLogGroupName + ':*',
       });
     }
 

packages/@aws-cdk/aws-logs/test/test.loggroup.ts

@@ -140,7 +140,7 @@ export = {
 
     // THEN
     test.deepEqual(imported.logGroupName, 'my-log-group');
-    test.deepEqual(imported.logGroupArn, 'arn:aws:logs:us-east-1:123456789012:log-group:my-log-group');
+    test.deepEqual(imported.logGroupArn, 'arn:aws:logs:us-east-1:123456789012:log-group:my-log-group:*');
     expect(stack2).to(haveResource('AWS::Logs::LogStream', {
       LogGroupName: 'my-log-group',
     }));
@@ -157,14 +157,88 @@ export = {
 
     // THEN
     test.deepEqual(imported.logGroupName, 'my-log-group');
-    test.ok(/^arn:.+:logs:.+:.+:log-group:my-log-group$/.test(imported.logGroupArn),
+    test.ok(/^arn:.+:logs:.+:.+:log-group:my-log-group:\*$/.test(imported.logGroupArn),
       `LogGroup ARN ${imported.logGroupArn} does not match the expected pattern`);
     expect(stack).to(haveResource('AWS::Logs::LogStream', {
       LogGroupName: 'my-log-group',
     }));
     test.done();
   },
 
+  'loggroups imported by name have stream wildcard appended to grant ARN': dataDrivenTests([
+    // Regardless of whether the user put :* there already because of this bug, we
+    // don't want to append it twice.
+    [''],
+    [':*'],
+  ], (test: Test, suffix: string) => {
+    // GIVEN
+    const stack = new Stack();
+    const user = new iam.User(stack, 'Role');
+    const imported = LogGroup.fromLogGroupName(stack, 'lg', `my-log-group${suffix}`);
+
+    // WHEN
+    imported.grantWrite(user);
+
+    // THEN
+    expect(stack).to(haveResource('AWS::IAM::Policy', {
+      PolicyDocument: {
+        Version: '2012-10-17',
+        Statement: [
+          {
+            Action: ['logs:CreateLogStream', 'logs:PutLogEvents'],
+            Effect: 'Allow',
+            Resource: {
+              'Fn::Join': [ '', [
+                'arn:',
+                { Ref: 'AWS::Partition' },
+                ':logs:',
+                { Ref: 'AWS::Region' },
+                ':',
+                { Ref: 'AWS::AccountId' },
+                ':log-group:my-log-group:*',
+              ]],
+            },
+          },
+        ],
+      },
+    }));
+    test.equal(imported.logGroupName, 'my-log-group');
+
+    test.done();
+  }),
+
+  'loggroups imported by ARN have stream wildcard appended to grant ARN': dataDrivenTests([
+    // Regardless of whether the user put :* there already because of this bug, we
+    // don't want to append it twice.
+    [''],
+    [':*'],
+  ], (test: Test, suffix: string) => {
+    // GIVEN
+    const stack = new Stack();
+    const user = new iam.User(stack, 'Role');
+    const imported = LogGroup.fromLogGroupArn(stack, 'lg', `arn:aws:logs:us-west-1:123456789012:log-group:my-log-group${suffix}`);
+
+    // WHEN
+    imported.grantWrite(user);
+
+    // THEN
+    expect(stack).to(haveResource('AWS::IAM::Policy', {
+      PolicyDocument: {
+        Version: '2012-10-17',
+        Statement: [
+          {
+            Action: ['logs:CreateLogStream', 'logs:PutLogEvents'],
+            Effect: 'Allow',
+            Resource: 'arn:aws:logs:us-west-1:123456789012:log-group:my-log-group:*',
+          },
+        ],
+      },
+    }));
+    test.equal(imported.logGroupName, 'my-log-group');
+
+    test.done();
+  }),
+
   'extractMetric'(test: Test) {
     // GIVEN
     const stack = new Stack();
@@ -242,3 +316,14 @@ export = {
     test.done();
   },
 };
+
+function dataDrivenTests(cases: any[][], body: (test: Test, ...args: any[]) => void) {
+  const ret: any = {};
+  for (let i = 0; i < cases.length; i++) {
+    const args = cases[i]; // Need to capture inside loop for safe use inside closure.
+    ret[`case ${i + 1}`] = function(test: Test) {
+      return body.apply(this, [test, ...args]);
+    };
+  }
+  return ret;
+}