fix(mixins-preview): improving delivery source and delivery destination creation #36314

ShadowCat567 · 2025-12-05T18:55:03Z

Reason for this change

There were several issues with the initial release of Vended logs, primarily:

if you attempted to set up vended logs for the same source resource that go to the same destination but with different log types you would have a naming conflict for the delivery destination name and it would fail, see below for example

// previously this did not work
eventBus.with(CfnEventBusLogsMixin.ERROR_LOGS.toLogGroup(logGroup));
eventBus.with(CfnEventBusLogsMixin.INFO_LOGS.toLogGroup(logGroup));

if you attempted to set up vended logs for the same source resource that go to different destination and have the same log type, your deployment would also fail -- I haven't been able to find any documentation about why this happens (this error occurs even if you ensure the DeliverySource names are unique, while testing I noticed you cannot have DeliverySources that are identical aside from their name)

// previously this did not work
eventBus.with(CfnEventBusLogsMixin.ERROR_LOGS.toLogGroup(logGroup));
eventBus.with(CfnEventBusLogsMixin.ERROR_LOGS.toS3(bucket));

our integration tests were slow because they used a Cloudfront Distribution as their log source

Description of changes

Delivery Destination Names
Delivery destination names now depend on the logType as well as the destination type, the destination resource, and the source resource to come up with a unique name. This ensures we can no longer have naming conflicts if we set up 2 vended log mixin deliveries that only differ based on the type of log that is getting send through them.

Delivery Sources
Previously a new delivery source was created for every delivery. However, it seems like there is some kind of internal implementation that blocks new delivery sources from being created that share a resourceArn and logType with an existing delivery source even if the names are unique.
Now only one DeliverySource is created per resourceArn + logType pair and log deliveries look for an existing DeliverySource they can use before creating a new one.

Unrelated to Mixins
Another PR introduced much stricter cfn-guard rules to the Security Guardian: #36110
This caused Security Guardian to start flagging some of the policies that the integ tests for the Vended Logs Mixin was using. Of the issues that were flagged, 2 of them were somewhat valid (encrypt s3 buckets and firehose data stream) and 2 of them were invalid (could not find relevant sections, even though they were there and threw error), updated the cfn guard rules for the invalid cases so they could properly validate the generated policies.

Updated the IAM_NO_WILDCARD_ACTIONS_INLINE cfn guard rule so it could properly validate IAM Roles that have this kind of structure Properties.AssumeRolePolicyDocument.Statement[*] as well as the structure it was previously looking for.
Excluded AWS:Logs::ResourcePolicy from NO_ROOT_PRINCIPALS_EXCEPT_KMS_SECRETS cfn guard rule. Cloudwatch Logs resource policies take in normal strings as their Policy Document (unlike most other policies which use JSON strings), which makes them difficult to deal with when working with cfn guard. See: https://docs.aws.amazon.com/AWSCloudFormation/latest/TemplateReference/aws-resource-logs-resourcepolicy.html

Describe any new or updated permissions being added

N/A

Description of how you validated changes

Added unit tests to ensure that delivery sources are getting created when we expect them to be and only one is getting created for each resourceArn + logType pair.
Added and updated integration tests to test previously untested scenarios and switched the integration tests from using a Cloudfront Distribution as their log source or an EventBus so they spin up and tear down faster.
Removed some of the requirements that force our Deliveries to be created sequentially, but only from the tests that create different DeliveryDestinations. When using the same DeliveryDestination for different Deliveries, they must deploy in sequence.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

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

github-actions · 2025-12-08T14:24:29Z

	Tests	Passed ✅	Skipped	Failed
Security Guardian Results	130 ran	130 passed

Test	Result
No test annotations available

View Security Guardian Results

github-actions · 2025-12-08T14:24:31Z

	Tests	Passed ✅	Skipped	Failed
Security Guardian Results with resolved templates	130 ran	130 passed

Test	Result
No test annotations available

View Security Guardian Results with resolved templates

mrgrain · 2025-12-09T18:44:03Z

packages/@aws-cdk/mixins-preview/lib/services/aws-logs/logs-delivery.ts

-  public bind(scope: IConstruct, deliverySource: logs.IDeliverySourceRef, _sourceResourceArn: string): ILogsDeliveryConfig {
-    const container = new Construct(scope, deliveryId('S3', scope, this.bucket));
+  public bind(scope: IConstruct, logType: string, sourceResourceArn: string): ILogsDeliveryConfig {
+    const container = new Construct(scope, deliveryId('S3'.concat(logType.split('_').map(word => word.charAt(0) + word.slice(1).toLowerCase()).join('')), scope, this.bucket));


add this to the deliveryId function instead, less repeat.

mrgrain · 2025-12-09T18:44:21Z

packages/@aws-cdk/mixins-preview/lib/services/aws-logs/logs-delivery.ts

    const deliveryDestination = new logs.CfnDeliveryDestination(container, 'Dest', {
      destinationResourceArn: this.bucket.bucketRef.bucketArn,
-      name: deliveryDestName('s3', container),
+      name: deliveryDestName('s3'.concat(`-${logType.split('_').map(word => word.toLowerCase()).join('-')}`), container),


add this to the deliveryDestName function instead, less repeat.

mrgrain · 2025-12-09T18:47:13Z

packages/@aws-cdk/mixins-preview/lib/services/aws-logs/logs-delivery.ts

 }
+
+function getOrCreateDeliverySource(logType: string, resource: IConstruct, sourceArn: string) {
+  // the delivery source should always be a child of the Construct passed in by resource


Is this actually more behaving like the BucketPolicy where the Delivery Source could be anywhere in the tree and we are looking for one that matches the given resourceArn and logType?

in theory, yes, if the user already has some vended log configurations they set up without the Mixin

mrgrain · 2025-12-09T18:51:00Z

packages/@aws-cdk/mixins-preview/test/codegen/__snapshots__/logs-delivery.test.ts.snap

+  public constructor(logType: string, destinationType: string, logDelivery: logsDelivery.ILogsDelivery) {
    super();
    this.logType = logType;
+    this.destinationType = destinationType;


this should be available on the logDelivery object.

yes, this was a relic of something else I was doing, we don't need it here any more

mrgrain

see comments

aws-amplify-bot added 5 commits December 3, 2025 10:08

add destination

f5410c5

Merge branch 'main' into vended-log/unique-construct-id

4710d26

destination type added to naming

d53bc3a

new vended logs integ test

f4ca026

fix tests

37c8ce7

github-actions bot added p2 repeat-contributor [Pilot] contributed between 3-5 PRs to the CDK labels Dec 5, 2025

aws-cdk-automation requested a review from a team December 5, 2025 18:55

mergify bot added the contribution/core This is a PR that came from AWS. label Dec 5, 2025

ShadowCat567 changed the title ~~fix(mixins-preview): parallel delivery creation and improving delivery source creation~~ fix(mixins-preview): improving delivery source and delivery destination creation Dec 5, 2025

pahud mentioned this pull request Dec 5, 2025

Failed CodeBuild CI - 23 PR(s) need attention pahud/aws-cdk#17

Open

aws-amplify-bot and others added 2 commits December 5, 2025 16:39

adjust tests

0a2c32a

Merge branch 'main' into vended-log/unique-construct-id

22379b0

aws-amplify-bot and others added 6 commits December 8, 2025 11:59

bucket encryption and firehose encryption

6ad2e5c

fix iam no wildcard cfnguard rule

f635ca7

encryption

77a716f

possible fix for cloudwatch

c112511

exclude logs policy

a197315

Merge branch 'main' into vended-log/unique-construct-id

3f1f28d

ShadowCat567 marked this pull request as ready for review December 9, 2025 14:57

mrgrain reviewed Dec 9, 2025

View reviewed changes

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 9, 2025

mrgrain reviewed Dec 9, 2025

View reviewed changes

mrgrain requested changes Dec 9, 2025

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 9, 2025

feedback

e20299b

Merge branch 'main' into vended-log/unique-construct-id

b90cdd9

ShadowCat567 requested a review from mrgrain December 10, 2025 15:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(mixins-preview): improving delivery source and delivery destination creation #36314

fix(mixins-preview): improving delivery source and delivery destination creation #36314

ShadowCat567 commented Dec 5, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Dec 8, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Dec 8, 2025 •

edited

Loading

Uh oh!

mrgrain Dec 9, 2025

Uh oh!

mrgrain Dec 9, 2025

Uh oh!

mrgrain Dec 9, 2025

Uh oh!

ShadowCat567 Dec 9, 2025

Uh oh!

mrgrain Dec 9, 2025

Uh oh!

ShadowCat567 Dec 9, 2025 •

edited

Loading

Uh oh!

mrgrain left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fix(mixins-preview): improving delivery source and delivery destination creation #36314

Are you sure you want to change the base?

fix(mixins-preview): improving delivery source and delivery destination creation #36314

Conversation

ShadowCat567 commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reason for this change

Description of changes

Describe any new or updated permissions being added

Description of how you validated changes

Checklist

Uh oh!

github-actions bot commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrgrain Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

mrgrain Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

mrgrain Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

ShadowCat567 Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

mrgrain Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

ShadowCat567 Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrgrain left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ShadowCat567 commented Dec 5, 2025 •

edited

Loading

github-actions bot commented Dec 8, 2025 •

edited

Loading

github-actions bot commented Dec 8, 2025 •

edited

Loading

ShadowCat567 Dec 9, 2025 •

edited

Loading