feat(ec2): flow logs from TransitGateway and TransitGatewayAttachment

[badmintoncryer]

I have enabled the configuration of flow logs for TransitGateway and TransitGatewayAttachment.

Create flow logs from TransitGateway:

declare const tgw: ec2.CfnTransitGateway;

new ec2.FlowLog(this, 'TransitGatewayFlowLog', {
  resourceType: ec2.FlowLogResourceType.fromTransitGatewayId(tgw.ref)
})

Create flowlogs from TransitGatewayAttachment:

declare const tgwAttachment: ec2.CfnTransitGatewayAttachment;

new ec2.FlowLog(this, 'TransitGatewayAttachmentFlowLog', {
  resourceType: ec2.FlowLogResourceType.fromTransitGatewayAttachmentId(tgwAttachment.ref)
})

Since trafficType cannot be set for flow logs related to TransitGateway resources, I have also added error handling for this.

if (props.resourceType.resourceType === 'TransitGateway' || props.resourceType.resourceType === 'TransitGatewayAttachment') {
      if (props.trafficType) {
        throw new Error('trafficType is not supported for Transit Gateway and Transit Gateway Attachment');
      }
      ...
    }

Closes #27222.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

[go-to-k]

Validation of maxAggregationInterval might be a good idea in this PR.
Also, it might be good to comment the description for the validation in the doc of the maxAggregationInterval parameter (and add unit tests for it).

--max-aggregation-interval has a default value of 60, and is the only accepted value for transit gateway resource types. An error is returned if you try to pass any other value. This limit applies only to transit gateway resource types.

https://docs.aws.amazon.com/vpc/latest/tgw/working-with-flow-logs.html

This parameter must be 60 seconds for transit gateway resource types.

https://docs.aws.amazon.com/ja_jp/AWSCloudFormation/latest/UserGuide/aws-resource-ec2-flowlog.html#cfn-ec2-flowlog-maxaggregationinterval

[go-to-k]

And this PR title feat(ec2): flow logs from TransitGateway and TransitGatewayAttachment would be better.

[badmintoncryer]

@go-to-k
Thank you for your review! I have incorporated your suggested modifications.

I've implemented validation of maxAggregationInterval.
The default value for this parameter is 10 minutes, which should result in an error when creating a Transit Gateway flow log. However, in practice, CFN allows deployment when maxAggregationInterval is set to undefined. It's likely that, internally, the default value is adjusted to 1 minute for Transit Gateway targets.

Therefore, when the target is TransitGateway, we allow maxAggregationInterval to be either ONE_MINUTE or undefined, and only consider it an error if TEN_MINUTES is explicitly specified."

[go-to-k]

Thanks, I've made a few more adjustment comments.

[go-to-k]

Looks good.

[paulhcsun]

Nice work @badmintoncryer, looks good overall, just a few minor comments to address.

[paulhcsun]

Can you create constants or enums for the resourceType?

[badmintoncryer]

@paulhcsun
I agree with creating an enum!
However, in the existing code, a resourceType defined as a string is already being used. Along with defining the enum, is it okay to also modify the FlowLogResourceType's ResourceType variable and the methods fromSubnet(), fromVpc(), and fromNetworkInterfaceId()?

export enum ResourceType {
  TRANSIT_GATEWAY = 'transitGateway',
  ...
}

export abstract class FlowLogResourceType {
  public static fromSubnet(subnet: ISubnet): FlowLogResourceType {
    return {
      resourceType: ResourceType.SUBNET,
      resourceId: subnet.subnetId,
    };
  }
...
  /**
   * The type of resource to attach a flow log to.
   */
  public abstract resourceType: ResourceType;
...
}

[paulhcsun]

Ah I see. I believe it should be safe to make that change for FlowLogResourceType's ResourceType as I see something similar being done with the FlowLogDestinationType enum in FlowLogDestination but let me double check that it won't create any backwards compatibility issues and get back to you.

I'm okay with approving this PR as other parts of the code are defining resourceType with a string. Updating resourceType to an enum can be addressed in a separate PR if you're interested, otherwise I can make that change once I've confirmed it's safe.

/**
 * The available destination types for Flow Logs
 */
export enum FlowLogDestinationType {
  /**
   * Send flow logs to CloudWatch Logs Group
   */
  CLOUD_WATCH_LOGS = 'cloud-watch-logs',
  ...
}


export abstract class FlowLogDestination {
  /**
   * Use CloudWatch logs as the destination
   */
  public static toCloudWatchLogs(logGroup?: logs.ILogGroup, iamRole?: iam.IRole): FlowLogDestination {
    return new CloudWatchLogsDestination({
      logDestinationType: FlowLogDestinationType.CLOUD_WATCH_LOGS,
      ...
    });
  }
...
}

[badmintoncryer]

@paulhcsun I see! I may be create PR for that later.

[kaizencc]

@badmintoncryer I don't think doing so would be safe so I would hold off on that for now

[badmintoncryer]

@kaizencc Understood. Thank you for the additional information.

Pull request has been modified.

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 3a14a7e
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[paulhcsun]

Great work @badmintoncryer!

And thanks for the review @go-to-k!

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).