Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mergify merged 16 commits into from
Dec 11, 2025
Merged

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

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
f5410c5
add destination
Dec 3, 2025
4710d26
Merge branch 'main' into vended-log/unique-construct-id
Dec 3, 2025
d53bc3a
destination type added to naming
Dec 3, 2025
f4ca026
new vended logs integ test
Dec 4, 2025
37c8ce7
fix tests
Dec 5, 2025
0a2c32a
adjust tests
Dec 5, 2025
22379b0
Merge branch 'main' into vended-log/unique-construct-id
ShadowCat567 Dec 8, 2025
6ad2e5c
bucket encryption and firehose encryption
Dec 8, 2025
f635ca7
fix iam no wildcard cfnguard rule
Dec 8, 2025
77a716f
encryption
Dec 8, 2025
c112511
possible fix for cloudwatch
Dec 8, 2025
a197315
exclude logs policy
Dec 8, 2025
3f1f28d
Merge branch 'main' into vended-log/unique-construct-id
ShadowCat567 Dec 8, 2025
e20299b
feedback
Dec 9, 2025
b90cdd9
Merge branch 'main' into vended-log/unique-construct-id
Dec 10, 2025
db6a0ad
Merge branch 'main' into vended-log/unique-construct-id
mergify[bot] Dec 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 47 additions & 27 deletions packages/@aws-cdk/mixins-preview/lib/services/aws-logs/logs-delivery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@ export interface ILogsDeliveryConfig {
*/
export interface ILogsDelivery {
/**
* Binds the destination to a delivery source and creates a delivery connection between the source and destination.
* Binds the log delivery to a source resource and creates a delivery connection between the source and destination.
* @param scope - The construct scope
* @param deliverySource - The delivery source reference
* @param logType - The type of logs that the delivery source will produce
* @param sourceResourceArn - The Arn of the source resource
* @returns The delivery reference
*/
bind(scope: IConstruct, deliverySource: logs.IDeliverySourceRef, sourceResourceArn: string): ILogsDeliveryConfig;
bind(scope: IConstruct, logType: string, sourceResourceArn: string): ILogsDeliveryConfig;
}

/**
Expand Down Expand Up @@ -84,28 +84,29 @@ export class S3LogsDelivery implements ILogsDelivery {
}

/**
* Binds the S3 destination to a delivery source and creates a delivery connection between them.
* Binds S3 Bucket to a source resource for the purposes of log delivery and creates a delivery source, a delivery destination, and a connection between them.
*/
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));
Copy link
Contributor

@mrgrain mrgrain Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add this to the deliveryId function instead, less repeat.


const bucketPolicy = this.getOrCreateBucketPolicy(container);
this.grantLogsDelivery(bucketPolicy);

const deliverySource = getOrCreateDeliverySource(logType, scope, sourceResourceArn);
const deliverySourceRef = deliverySource.deliverySourceRef;

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),
Copy link
Contributor

@mrgrain mrgrain Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add this to the deliveryDestName function instead, less repeat.

deliveryDestinationType: 'S3',
});

const delivery = new logs.CfnDelivery(container, 'Delivery', {
deliveryDestinationArn: deliveryDestination.attrArn,
deliverySourceName: deliverySource.deliverySourceRef.deliverySourceName,
deliverySourceName: deliverySourceRef.deliverySourceName,
});

delivery.node.addDependency(bucketPolicy);
deliveryDestination.node.addDependency(bucketPolicy);
deliverySource.node.addDependency(bucketPolicy);

delivery.node.addDependency(deliverySource);
delivery.node.addDependency(deliveryDestination);
Expand Down Expand Up @@ -197,18 +198,20 @@ export class FirehoseLogsDelivery implements ILogsDelivery {
}

/**
* Binds the Firehose destination to a delivery source and creates a delivery connection between them.
* Binds Firehose Delivery Stream to a source resource for the purposes of log delivery and creates a delivery source, a delivery destination, and a connection between them.
*/
public bind(scope: IConstruct, deliverySource: logs.IDeliverySourceRef, _sourceResourceArn: string): ILogsDeliveryConfig {
const container = new Construct(scope, deliveryId('Firehose', scope, this.deliveryStream));
public bind(scope: IConstruct, logType: string, sourceResourceArn: string): ILogsDeliveryConfig {
const container = new Construct(scope, deliveryId('Firehose'.concat(logType.split('_').map(word => word.charAt(0) + word.slice(1).toLowerCase()).join('')), scope, this.deliveryStream));

// Firehose uses a service-linked role to deliver logs
// This tag marks the destination stream as an allowed destination for the service-linked role
Tags.of(this.deliveryStream).add('LogDeliveryEnabled', 'true');

const deliverySource = getOrCreateDeliverySource(logType, scope, sourceResourceArn);

const deliveryDestination = new logs.CfnDeliveryDestination(container, 'Dest', {
destinationResourceArn: this.deliveryStream.deliveryStreamRef.deliveryStreamArn,
name: deliveryDestName('fh', container),
name: deliveryDestName('fh'.concat(`-${logType.split('_').map(word => word.toLowerCase()).join('-')}`), container),
deliveryDestinationType: 'FH',
});

Expand Down Expand Up @@ -243,30 +246,31 @@ export class LogGroupLogsDelivery implements ILogsDelivery {
}

/**
* Binds the log group destination to a delivery source and creates a delivery connection between them.
* Binds Log Group to a source resource for the purposes of log delivery and creates a delivery source, a delivery destination, and a connection between them.
*/
public bind(scope: IConstruct, deliverySource: logs.IDeliverySourceRef, _sourceResourceArn: string): ILogsDeliveryConfig {
const container = new Construct(scope, deliveryId('LogGroup', scope, this.logGroup));
public bind(scope: IConstruct, logType: string, sourceResourceArn: string): ILogsDeliveryConfig {
const container = new Construct(scope, deliveryId('LogGroup'.concat(logType.split('_').map(word => word.charAt(0) + word.slice(1).toLowerCase()).join('')), scope, this.logGroup));

const deliverySource = getOrCreateDeliverySource(logType, scope, sourceResourceArn);
const deliverySourceRef = deliverySource.deliverySourceRef;

const logGroupPolicy = this.getOrCreateLogsResourcePolicy(container);
this.grantLogsDelivery(logGroupPolicy);

const deliveryDestination = new logs.CfnDeliveryDestination(container, 'Dest', {
destinationResourceArn: this.logGroup.logGroupRef.logGroupArn,
name: deliveryDestName('cwl', container),
name: deliveryDestName('cwl'.concat(`-${logType.split('_').map(word => word.toLowerCase()).join('-')}`), container),
deliveryDestinationType: 'CWL',
});

const delivery = new logs.CfnDelivery(container, 'Delivery', {
deliveryDestinationArn: deliveryDestination.deliveryDestinationRef.deliveryDestinationArn,
deliverySourceName: deliverySource.deliverySourceRef.deliverySourceName,
deliverySourceName: deliverySourceRef.deliverySourceName,
});

delivery.node.addDependency(logGroupPolicy);
delivery.node.addDependency(deliverySource);
delivery.node.addDependency(deliveryDestination);

deliverySource.node.addDependency(logGroupPolicy);
deliveryDestination.node.addDependency(logGroupPolicy);

return {
Expand Down Expand Up @@ -327,16 +331,17 @@ export class XRayLogsDelivery implements ILogsDelivery {
constructor() {}

/**
* Binds the X-Ray destination to a delivery source and creates a delivery connection between them.
* Binds X-Ray Destination to a source resource for the purposes of log delivery and creates a delivery source, a delivery destination, and a connection between them.
*/
public bind(scope: IConstruct, deliverySource: logs.IDeliverySourceRef, sourceResourceArn: string): ILogsDeliveryConfig {
const container = new Construct(scope, deliveryId('XRay', scope, deliverySource));
public bind(scope: IConstruct, logType: string, sourceResourceArn: string): ILogsDeliveryConfig {
const deliverySource = getOrCreateDeliverySource(logType, scope, sourceResourceArn);
const container = new Construct(scope, deliveryId('XRay'.concat(logType.split('_').map(word => word.charAt(0) + word.slice(1).toLowerCase()).join('')), scope, deliverySource));

const xrayResourcePolicy = this.getOrCreateResourcePolicy(container);
this.grantLogsDelivery(xrayResourcePolicy, sourceResourceArn);

const deliveryDestination = new logs.CfnDeliveryDestination(container, 'Dest', {
name: deliveryDestName('xray', container),
name: deliveryDestName('xray'.concat(`-${logType.split('_').map(word => word.toLowerCase()).join('-')}`), container),
deliveryDestinationType: 'XRAY',
});

Expand All @@ -345,9 +350,7 @@ export class XRayLogsDelivery implements ILogsDelivery {
deliverySourceName: deliverySource.deliverySourceRef.deliverySourceName,
});

delivery.node.addDependency(xrayResourcePolicy);
deliveryDestination.node.addDependency(xrayResourcePolicy);
deliverySource.node.addDependency(xrayResourcePolicy);

delivery.node.addDependency(deliverySource);
delivery.node.addDependency(deliveryDestination);
Expand Down Expand Up @@ -411,3 +414,20 @@ function deliveryDestName(type: string, scope: IConstruct) {
const prefix = `cdk-${type}-dest-`;
return `${prefix}${Names.uniqueResourceName(scope, { maxLength: 60 - prefix.length })}`;
}

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

@mrgrain mrgrain Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@ShadowCat567 ShadowCat567 Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

const sourceId = `CDKSource${logType}${Names.uniqueId(resource)}`;
const sourceResource = resource.node.tryFindChild(sourceId) as logs.CfnDeliverySource;

if (!sourceResource) {
const prefix = `cdk-${logType.split('_').map(word => word.toLowerCase()).join('')}-source-`;
const newSource = new logs.CfnDeliverySource(resource, sourceId, {
name: `${prefix}${Names.uniqueResourceName(resource, { maxLength: 60 - prefix.length })}`,
logType,
resourceArn: sourceArn,
});
return newSource;
}
return sourceResource;
}
44 changes: 21 additions & 23 deletions packages/@aws-cdk/mixins-preview/scripts/spec2logs/builder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { naming, util } from '@aws-cdk/spec2cdk';
import { CDK_CORE, CONSTRUCTS } from '@aws-cdk/spec2cdk/lib/cdk/cdk';
import type { Method } from '@cdklabs/typewriter';
import { Module, ExternalModule, ClassType, Stability, Type, expr, stmt, ThingSymbol, $this, CallableProxy, NewExpression, $E } from '@cdklabs/typewriter';
import { CDK_AWS_LOGS, MIXINS_LOGS_DELIVERY, REF_INTERFACES } from './helpers';
import { MIXINS_LOGS_DELIVERY, REF_INTERFACES } from './helpers';
import type { ServiceSubmoduleProps, SelectiveImport, LocatedModule } from '@aws-cdk/spec2cdk/lib/cdk/service-submodule';
import { BaseServiceSubmodule, relativeImportPath } from '@aws-cdk/spec2cdk/lib/cdk/service-submodule';
import type { AddServiceProps, LibraryBuilderProps } from '@aws-cdk/spec2cdk/lib/cdk/library-builder';
Expand Down Expand Up @@ -74,7 +74,6 @@ export class LogsDeliveryBuilder extends LibraryBuilder<LogsDeliveryBuilderServi
CDK_CORE.import(module, 'cdk');
REF_INTERFACES.import(module, 'interfaces');
CONSTRUCTS.import(module, 'constructs');
CDK_AWS_LOGS.import(module, 'logs');
MIXINS_CORE.import(module, 'core', { fromLocation: relativeImportPath(filePath, '../core') });
MIXINS_LOGS_DELIVERY.import(module, 'logsDelivery', { fromLocation: '../aws-logs/logs-delivery' });
submodule.constructLibModule.import(module, 'service');
Expand Down Expand Up @@ -163,7 +162,7 @@ class LogsHelper extends ClassType {
const permissions = this.resource.vendedLogs!.permissionsVersion === 'V2' ? MIXINS_LOGS_DELIVERY.S3LogsDeliveryPermissionsVersion.V2 : MIXINS_LOGS_DELIVERY.S3LogsDeliveryPermissionsVersion.V1;
toS3.addBody(stmt.block(
stmt.ret(
mixin.newInstance(expr.str(this.logType), new NewExpression(MIXINS_LOGS_DELIVERY.S3LogsDelivery, paramS3,
mixin.newInstance(expr.str(this.logType), expr.str(dest), new NewExpression(MIXINS_LOGS_DELIVERY.S3LogsDelivery, paramS3,
expr.object({ permissionsVersion: permissions }))),
),
));
Expand All @@ -184,7 +183,7 @@ class LogsHelper extends ClassType {

toCWL.addBody(stmt.block(
stmt.ret(
mixin.newInstance(expr.str(this.logType), new NewExpression(MIXINS_LOGS_DELIVERY.LogGroupLogsDelivery, paramCWL)),
mixin.newInstance(expr.str(this.logType), expr.str(dest), new NewExpression(MIXINS_LOGS_DELIVERY.LogGroupLogsDelivery, paramCWL)),
),
));
break;
Expand All @@ -204,7 +203,7 @@ class LogsHelper extends ClassType {

toFH.addBody(stmt.block(
stmt.ret(
mixin.newInstance(expr.str(this.logType), new NewExpression(MIXINS_LOGS_DELIVERY.FirehoseLogsDelivery, paramFH)),
mixin.newInstance(expr.str(this.logType), expr.str(dest), new NewExpression(MIXINS_LOGS_DELIVERY.FirehoseLogsDelivery, paramFH)),
),
));
break;
Expand All @@ -219,7 +218,7 @@ class LogsHelper extends ClassType {

toXRAY.addBody(stmt.block(
stmt.ret(
mixin.newInstance(expr.str(this.logType), new NewExpression(MIXINS_LOGS_DELIVERY.XRayLogsDelivery)),
mixin.newInstance(expr.str(this.logType), expr.str(dest), new NewExpression(MIXINS_LOGS_DELIVERY.XRayLogsDelivery)),
),
));
break;
Expand Down Expand Up @@ -286,6 +285,13 @@ class LogsMixin extends ClassType {
immutable: true,
});

this.addProperty({
name: 'destinationType',
type: Type.STRING,
protected: true,
immutable: true,
});

this.addProperty({
name: 'logDelivery',
type: MIXINS_LOGS_DELIVERY.ILogsDelivery,
Expand All @@ -305,15 +311,22 @@ class LogsMixin extends ClassType {
documentation: 'Type of logs that are getting vended',
});

const destinationType = init.addParameter({
name: 'destinationType',
type: Type.STRING,
documentation: 'Resource type of the delivery destination',
});

const delivery = init.addParameter({
name: 'logDelivery',
type: MIXINS_LOGS_DELIVERY.ILogsDelivery,
documentation: 'Object in charge of setting up the delivery destination and delivery connection',
documentation: 'Object in charge of setting up the delivery source, delivery destination, and delivery connection',
});

init.addBody(
expr.sym(new ThingSymbol('super', this.scope)).call(),
stmt.assign($this.logType, logType),
stmt.assign($this.destinationType, destinationType),
stmt.assign($this.logDelivery, delivery),
);
}
Expand Down Expand Up @@ -362,28 +375,13 @@ class LogsMixin extends ClassType {
const sourceArn = expr.ident('sourceArn');
const arnBuilder = $E(expr.sym(this.resourceType.symbol!)).callMethod(`arnFor${this.resource.name}`, resource);

const prefix = `${this.resource.name}Source-`;
const newCfnDeliverySource = CDK_AWS_LOGS.CfnDeliverySource.newInstance(
resource,
expr.strConcat(expr.str('CdkSource'), CDK_CORE.uniqueId(resource)),
expr.object({
name: expr.strConcat(expr.str(prefix), CDK_CORE.uniqueResourceName(resource, expr.object({
maxLength: expr.binOp(expr.num(60 - (prefix.length + 1)), '-', $this.logType.prop('length')),
})), expr.str('-'), $this.logType),
resourceArn: sourceArn,
logType: $this.logType,
}),
);
const deliverySource = expr.ident('deliverySource');

method.addBody(
stmt
.if_(expr.not(CallableProxy.fromMethod(supports).invoke(resource)))
.then(stmt.block(stmt.ret(resource))),

stmt.constVar(sourceArn, arnBuilder),
stmt.constVar(deliverySource, newCfnDeliverySource),
$this.logDelivery.callMethod('bind', resource, deliverySource, sourceArn),
$this.logDelivery.callMethod('bind', resource, $this.logType, sourceArn),

stmt.ret(resource),
);
Expand Down
5 changes: 0 additions & 5 deletions packages/@aws-cdk/mixins-preview/scripts/spec2logs/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,5 @@ class CdkRefInterfaces extends ExternalModule {
public readonly IDeliveryStreamRef = Type.fromName(this, 'aws_kinesisfirehose.IDeliveryStreamRef');
}

class CdkAwsLogs extends ExternalModule {
public readonly CfnDeliverySource = Type.fromName(this, 'CfnDeliverySource');
}

export const MIXINS_LOGS_DELIVERY = new MixinsLogsDelivery('@aws-cdk/mixins-preview/services/aws-logs');
export const REF_INTERFACES = new CdkRefInterfaces('aws-cdk-lib/interfaces');
export const CDK_AWS_LOGS = new CdkAwsLogs('aws-cdk-lib/aws-logs');
Loading
Loading