aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.68k stars 3.93k forks source link

events: log group target #20855

Open seyeong opened 2 years ago

seyeong commented 2 years ago

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

CDK version used

2.29.0

Environment details (OS name and version, etc.)

N/A

seyeong commented 2 years ago

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 commented 2 years ago

Because of the breaking change, would need a feature flag

frankpengau commented 5 months ago

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. 😅