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

events: log group target #20855

Open
2 tasks done
seyeong opened this issue Jun 24, 2022 · 3 comments
Open
2 tasks done

events: log group target #20855

seyeong opened this issue Jun 24, 2022 · 3 comments
Labels
@aws-cdk/aws-events Related to CloudWatch Events effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Comments

@seyeong
Copy link
Contributor

seyeong commented Jun 24, 2022

Describe the feature

In #10598, the author added support for CloudWatch Logs log group as a target of an EventBridge event bus. The author noted that at that time, CloudFormation did not have support for CWL resource policy, they used a custom lambda to manipulate the policy. Since then, CFN added the resource support and another author added support in #17015.

Now the custom resource added in #10598 can be removed in favour of the native support.

Note that this may be a breaking change as CDK needs to manage the resource policy by importing it to the stack.

Use Case

Removing tech debt. Custom resource lambda functions require S3 bucket to provision the code.

Proposed Solution

No response

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.29.0

Environment details (OS name and version, etc.)

N/A

@seyeong seyeong added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jun 24, 2022
@github-actions github-actions bot added the @aws-cdk/aws-events Related to CloudWatch Events label Jun 24, 2022
@seyeong seyeong changed the title (events): log group target events: log group target Jun 24, 2022
@seyeong
Copy link
Contributor Author

seyeong commented Jun 27, 2022

Possible workaround

If do not like to have a custom lambda managing the policy, you can create a dummy resource which prevents the construct to create the custom resource lambda.

 ...
    this.rule = new Rule(eventBus, "Log Group Sink Rule", {
      eventBus,
      eventPattern,
    });

    this.injectEmptyResource();
    this.rule.addTarget(new CloudWatchLogGroup(this.logGroup));
  }

  /**
   * This injects a dummy resource so that CDK won't create a custom resource for managing LogGroup resource policy;
   * See https://github.com/aws/aws-cdk/issues/20855
   *
   * The resource policy is created as a native CloudFormation resource when `logGroup.grant*` is called.
   * See https://github.com/aws/aws-cdk/blob/v2.29.1/packages/@aws-cdk/aws-logs/lib/log-group.ts#L168-L184
   */
  private injectEmptyResource() {
    const resourcePolicyId = `EventsLogGroupPolicy${Names.nodeUniqueId(
      this.rule.node
    )}`;

    const dummyResource = new CfnWaitConditionHandle(
      this.logGroup,
      resourcePolicyId
    );
    dummyResource.overrideLogicalId(resourcePolicyId);
  }

@rix0rrr rix0rrr added effort/medium Medium work item – several days of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Jul 7, 2022
@rix0rrr rix0rrr removed their assignment Jul 7, 2022
@rix0rrr
Copy link
Contributor

rix0rrr commented Jul 7, 2022

Because of the breaking change, would need a feature flag

@frankpengau
Copy link
Contributor

Possibly changing the bind method to the following, with the use of the grantWrite made availble for CloudWatch Log Groups:

  /**
   * Returns a RuleTarget that can be used to log an event into a CloudWatch LogGroup
   */
  public bind(_rule: events.IRule, _id?: string): events.RuleTargetConfig {
    const logGroupStack = cdk.Stack.of(this.logGroup);

    if (this.props.event && this.props.logEvent) {
      throw new Error('Only one of "event" or "logEvent" can be specified');
    }

    this.target = this.props.event?.bind(_rule);
    if (this.target?.inputPath || this.target?.input) {
      throw new Error('CloudWatchLogGroup targets does not support input or inputPath');
    }

    _rule.node.addValidation({ validate: () => this.validateInputTemplate() });

    this.logGroup.grantWrite(new iam.ServicePrincipal('events.amazonaws.com'));

    return {
      ...bindBaseTargetConfig(this.props),
      arn: logGroupStack.formatArn({
        service: 'logs',
        resource: 'log-group',
        arnFormat: ArnFormat.COLON_RESOURCE_NAME,
        resourceName: this.logGroup.logGroupName,
      }),
      input: this.props.event ?? this.props.logEvent,
      targetResource: this.logGroup,
    };
  }

With potential unit test, something like this:

test('grant write permissions to log group', () => {
  // GIVEN
  const stack = new cdk.Stack();
  const logGroup = new logs.LogGroup(stack, 'MyLogGroup', {
    logGroupName: '/aws/events/MyLogGroup',
  });
  const rule1 = new events.Rule(stack, 'Rule', {
    schedule: events.Schedule.rate(cdk.Duration.minutes(1)),
  });

  // WHEN
  rule1.addTarget(new targets.CloudWatchLogGroup(logGroup));

  // THEN
  Template.fromStack(stack).hasResourceProperties('AWS::Logs::ResourcePolicy', {
    PolicyDocument: {
      Statement: [
        {
          Action: ['logs:CreateLogStream', 'logs:PutLogEvents'],
          Effect: 'Allow',
          Principal: { Service: 'events.amazonaws.com' },
          Resource: { 'Fn::GetAtt': ['MyLogGroup5C0DAD85', 'Arn'] },
        },
      ],
    },
  });
});

I was planning to do it myself, but I can't seem to get the integration tests to run without my computer freezing up and making intense fan revving sounds similar to a jet engine. 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-events Related to CloudWatch Events effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
4 participants