chore(mixins-preview): delivery destinations for vended logs #36087
Conversation
github-actions
bot
added
the
beginning-contributor
aws-cdk-automation
added
the
pr/needs-further-review
| interface IDeliveryDestination extends IResource { | ||
| readonly destinationArn: string; | ||
| } |
There was a problem hiding this comment.
Instead of this, we can use the existing IDeliveryDestinationRef
| interface IDeliveryDestination extends IResource { | |
| readonly destinationArn: string; | |
| } | |
| interface IDeliveryDestination extends IResource { | |
| readonly destinationArn: string; | |
| } |
| readonly destinationArn: string; | ||
| } | ||
|
|
||
| abstract class DeliveryDestinationBase extends Resource implements IDeliveryDestination { |
There was a problem hiding this comment.
| abstract class DeliveryDestinationBase extends Resource implements IDeliveryDestination { | |
| abstract class DeliveryDestinationBase extends Resource implements IDeliveryDestinationRef { |
|
|
||
| export class S3DeliveryDestination extends DeliveryDestinationBase { | ||
| public readonly destinationArn: string; | ||
| constructor(scope: Construct, id: string, props: DeliveryDestinationProps & Required<Pick<DeliveryDestinationProps, 's3Bucket'>>) { |
There was a problem hiding this comment.
This doesn't compile in jsii. Really we need 3 separate Props objects.
| const bucketPolicy = new CfnBucketPolicy(scope, 'S3Policy', { | ||
| bucket: props.s3Bucket.bucketName, | ||
| policyDocument: bucketPolicyDoc.toJSON(), | ||
| }); |
There was a problem hiding this comment.
Here is a fun side quest: We need a helper function, that for a given Stack and IBucketRef goes through the whole stack and tries to find an existing CfnBucketPolicy for that IBucketRef.
If we find one, we use that. Otherwise we create a new one.
| const destinationNamePrefix = 's3-delivery-destination-'; | ||
| const deliveryDestination = new CfnDeliveryDestination(scope, 'S3DeliveryDestination', { | ||
| destinationResourceArn: props.s3Bucket.bucketArn, | ||
| name: `${destinationNamePrefix}${Names.uniqueResourceName(props.s3Bucket, { maxLength: 60 - destinationNamePrefix.length })}`, |
| policyDocument: bucketPolicyDoc.toJSON(), | ||
| }); | ||
|
|
||
| const destinationNamePrefix = 's3-delivery-destination-'; |
There was a problem hiding this comment.
let's find a shorter prefix. Maybe something with cdk
| constructor(scope: Construct, id: string, props: DeliveryDestinationProps & Required<Pick<DeliveryDestinationProps, 'logGroup'>>) { | ||
| super(scope, id); | ||
|
|
||
| const logGroupPolicy = new CfnResourcePolicy(scope, 'CloudwatchDeliveryPolicy', { |
There was a problem hiding this comment.
For this one I'd like us to create a new singleton resource policy for delivery destination. We will getOrCreate that policy and than add statements to it.
This is so that when a user has multiple CloudwatchDeliveryDestinations in their stack, we only ever add a single policy. Because they are limited to max 10 per account.
| } | ||
| } | ||
|
|
||
| export class CloudwatchDeliveryDestination extends DeliveryDestinationBase { |
There was a problem hiding this comment.
This one I think would be this though if we follow the service name plus stuff pattern.
| export class CloudwatchDeliveryDestination extends DeliveryDestinationBase { | |
| export class LogsDeliveryDestination extends DeliveryDestinationBase { |
| bucketPolicyDoc.addStatements(v1BucketPolicy); | ||
| } | ||
|
|
||
| const bucketPolicy = new CfnBucketPolicy(scope, 'S3Policy', { |
There was a problem hiding this comment.
you are allowed to use a L2 BucketPolicy here if it helps
mrgrain
left a comment
mrgrain
left a comment
There was a problem hiding this comment.
Also needs unit tests ;)
3 participants
Reason for this change
Adds ground work for vended logs mixin.
Description of changes
Sets up mini-constructs to be used by the mixin that assists in setting up vended logs.
Max length of name for delivery destination is 60 characters (see: https://docs.aws.amazon.com/AmazonCloudWatchLogs/latest/APIReference/API_PutDeliveryDestination.html).
Describe any new or updated permissions being added
Description of how you validated changes
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license