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

(aws-cloudtrail): sendToCloudWatchLogs should default to true when cloudWatchLogGroup or cloudWatchLogsRetention is provided. #16092

Closed javydekoning closed 2 years ago

javydekoning commented 3 years ago

Sending events to CloudWatch is not enabled when cloudWatchLogGroup or cloudWatchLogsRetention property is passed in. You need explicitly set sendToCloudWatchLogs to true. If cloudWatchLogGroup or cloudWatchLogsRetention is passed in, this should be implicit.

Reproduction Steps

    const trail = new ct.Trail(this, 'BaseLineCloudWatchTrail', {
      cloudWatchLogGroup: this.cloudTrailLogGroup,
      managementEvents: ct.ReadWriteType.WRITE_ONLY,
      isMultiRegionTrail: false,
    });

What did you expect to happen?

I would expect events to turn up in the provided cloudWatchLogGroup. But need to manually turn on sendToCloudWatchLogs: true to make this happen.

Environment

N/A

Other

https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-cloudtrail/lib/cloudtrail.ts#L238 could be changed to:

    if (props.sendToCloudWatchLogs || props.cloudWatchLogGroup || props.cloudWatchLogsRetention) {

alternatively we could do:

if (props.cloudWatchLogGroup || props.cloudWatchLogsRetention) {
  if (props.sendToCloudWatchLogs == undefined) {
    props.sendToCloudWatchLogs = true
  }
}

This is :bug: Bug Report

javydekoning commented 3 years ago

Looks like there is already a PR open for this: https://github.com/aws/aws-cdk/pull/14366/commits/e707ec048ae93e51171f6c00f7736454e302e186

rix0rrr commented 3 years ago

I like it! The commit you referenced doesn't seem to belong to a PR anymore, but we would totally accept this change.

github-actions[bot] commented 2 years ago

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.