Skip to content

fix(core): implicit Aspect applications do not override custom Aspect applications #34132

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.

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
merged 12 commits into from
Apr 14, 2025
Merged
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as cdk from 'aws-cdk-lib/core';
import * as cxapi from '../../cx-api';
import { Construct } from 'constructs';
import { IApplication } from './application';
import { CheckedStageStackAssociator } from './aspects/stack-associator';
Expand Down Expand Up @@ -50,7 +51,9 @@ export class ApplicationAssociator extends Construct {
this.associateCrossAccountStacks = targetBindResult.associateCrossAccountStacks;
cdk.Aspects.of(scope).add(new CheckedStageStackAssociator(this, {
associateCrossAccountStacks: this.associateCrossAccountStacks,
}), { priority: cdk.AspectPriority.MUTATING });
}), {
priority: cdk.FeatureFlags.of(this).isEnabled(cxapi.ASPECT_PRIORITIES_MUTATING) ? cdk.AspectPriority.MUTATING : undefined,
});
}

/**
Expand All @@ -61,7 +64,9 @@ export class ApplicationAssociator extends Construct {
this.associatedStages.add(stage);
cdk.Aspects.of(stage).add(new CheckedStageStackAssociator(this, {
associateCrossAccountStacks: this.associateCrossAccountStacks,
}), { priority: cdk.AspectPriority.MUTATING });
}), {
priority: cdk.FeatureFlags.of(this).isEnabled(cxapi.ASPECT_PRIORITIES_MUTATING) ? cdk.AspectPriority.MUTATING : undefined,
});
return stage;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
Tokenization, UnscopedValidationError, ValidationError, withResolved,
} from '../../core';
import { addConstructMetadata, MethodMetadata } from '../../core/lib/metadata-resource';
import * as cxapi from '../../cx-api';
import { AUTOSCALING_GENERATE_LAUNCH_TEMPLATE } from '../../cx-api';

/**
Expand Down Expand Up @@ -1608,7 +1609,9 @@ export class AutoScalingGroup extends AutoScalingGroupBase implements
this.spotPrice = props.spotPrice;

if (props.requireImdsv2) {
Aspects.of(this).add(new AutoScalingGroupRequireImdsv2Aspect(), { priority: AspectPriority.MUTATING });
Aspects.of(this).add(new AutoScalingGroupRequireImdsv2Aspect(), {
priority: FeatureFlags.of(this).isEnabled(cxapi.ASPECT_PRIORITIES_MUTATING) ? AspectPriority.MUTATING : undefined,
});
}

this.node.addValidation({ validate: () => this.validateTargetGroup() });
Expand Down
7 changes: 5 additions & 2 deletions packages/aws-cdk-lib/aws-backup/lib/selection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ import { BackupableResourcesCollector } from './backupable-resources-collector';
import { IBackupPlan } from './plan';
import { BackupResource, TagOperation } from './resource';
import * as iam from '../../aws-iam';
import { Lazy, Resource, Aspects, AspectPriority } from '../../core';
import { Lazy, Resource, Aspects, AspectPriority, FeatureFlags } from '../../core';
import { addConstructMetadata } from '../../core/lib/metadata-resource';
import * as cxapi from '../../cx-api';

/**
* Options for a BackupSelection
Expand Down Expand Up @@ -143,7 +144,9 @@ export class BackupSelection extends Resource implements iam.IGrantable {
}

if (resource.construct) {
Aspects.of(resource.construct).add(this.backupableResourcesCollector, { priority: AspectPriority.MUTATING });
Aspects.of(resource.construct).add(this.backupableResourcesCollector, {
priority: FeatureFlags.of(this).isEnabled(cxapi.ASPECT_PRIORITIES_MUTATING) ? AspectPriority.MUTATING : undefined,
});
// Cannot push `this.backupableResourcesCollector.resources` to
// `this.resources` here because it has not been evaluated yet.
// Will be concatenated to `this.resources` in a `Lazy.list`
Expand Down
4 changes: 3 additions & 1 deletion packages/aws-cdk-lib/aws-ec2/lib/instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,9 @@ export class Instance extends Resource implements IInstance {
}));

if (props.requireImdsv2) {
Aspects.of(this).add(new InstanceRequireImdsv2Aspect(), { priority: AspectPriority.MUTATING });
Aspects.of(this).add(new InstanceRequireImdsv2Aspect(), {
priority: FeatureFlags.of(this).isEnabled(cxapi.ASPECT_PRIORITIES_MUTATING) ? AspectPriority.MUTATING : undefined,
});
}
}

Expand Down
5 changes: 4 additions & 1 deletion packages/aws-cdk-lib/aws-ecs/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
FeatureFlags, Annotations,
} from '../../core';
import { addConstructMetadata, MethodMetadata } from '../../core/lib/metadata-resource';
import * as cxapi from '../../cx-api';
import { Disable_ECS_IMDS_Blocking, Enable_IMDS_Blocking_Deprecated_Feature } from '../../cx-api';

const CLUSTER_SYMBOL = Symbol.for('@aws-cdk/aws-ecs/lib/cluster.Cluster');
Expand Down Expand Up @@ -331,7 +332,9 @@ export class Cluster extends Resource implements ICluster {
// since it's harmless, but we'd prefer not to add unexpected new
// resources to the stack which could surprise users working with
// brown-field CDK apps and stacks.
Aspects.of(this).add(new MaybeCreateCapacityProviderAssociations(this, id), { priority: AspectPriority.MUTATING });
Aspects.of(this).add(new MaybeCreateCapacityProviderAssociations(this, id), {
priority: FeatureFlags.of(this).isEnabled(cxapi.ASPECT_PRIORITIES_MUTATING) ? AspectPriority.MUTATING : undefined,
});
}

/**
Expand Down
11 changes: 8 additions & 3 deletions packages/aws-cdk-lib/aws-iam/lib/permissions-boundary.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { IConstruct } from 'constructs';
import { CfnRole, CfnUser } from './iam.generated';
import { IManagedPolicy } from './managed-policy';
import { AspectPriority, Aspects, CfnResource } from '../../core';
import { AspectPriority, Aspects, CfnResource, FeatureFlags } from '../../core';
import * as cxapi from '../../cx-api';

/**
* Modify the Permissions Boundaries of Users and Roles in a construct tree
Expand Down Expand Up @@ -40,7 +41,9 @@ export class PermissionsBoundary {
node.addPropertyOverride('PermissionsBoundary', boundaryPolicy.managedPolicyArn);
}
},
}, { priority: AspectPriority.MUTATING });
}, {
priority: FeatureFlags.of(this.scope).isEnabled(cxapi.ASPECT_PRIORITIES_MUTATING) ? AspectPriority.MUTATING : undefined,
});
}

/**
Expand All @@ -56,6 +59,8 @@ export class PermissionsBoundary {
node.addPropertyDeletionOverride('PermissionsBoundary');
}
},
}, { priority: AspectPriority.MUTATING });
}, {
priority: FeatureFlags.of(this.scope).isEnabled(cxapi.ASPECT_PRIORITIES_MUTATING) ? AspectPriority.MUTATING : undefined,
});
}
}
7 changes: 5 additions & 2 deletions packages/aws-cdk-lib/aws-iam/lib/role.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ import { ImportedRole } from './private/imported-role';
import { MutatingPolicyDocumentAdapter } from './private/policydoc-adapter';
import { PrecreatedRole } from './private/precreated-role';
import { AttachedPolicies, UniqueStringSet } from './private/util';
import { ArnFormat, Duration, Resource, Stack, Token, TokenComparison, Aspects, Annotations, RemovalPolicy, AspectPriority } from '../../core';
import { ArnFormat, Duration, Resource, Stack, Token, TokenComparison, Aspects, Annotations, RemovalPolicy, AspectPriority, FeatureFlags } from '../../core';
import { getCustomizeRolesConfig, getPrecreatedRoleConfig, CUSTOMIZE_ROLES_CONTEXT_KEY, CustomizeRoleConfig } from '../../core/lib/helpers-internal';
import { addConstructMetadata, MethodMetadata } from '../../core/lib/metadata-resource';
import * as cxapi from '../../cx-api';

const MAX_INLINE_SIZE = 10000;
const MAX_MANAGEDPOL_SIZE = 6000;
Expand Down Expand Up @@ -496,7 +497,9 @@ export class Role extends Resource implements IRole {
this.splitLargePolicy();
}
},
}, { priority: AspectPriority.MUTATING });
}, {
priority: FeatureFlags.of(this).isEnabled(cxapi.ASPECT_PRIORITIES_MUTATING) ? AspectPriority.MUTATING : undefined,
});
}

this.policyFragment = new ArnPrincipal(this.roleArn).policyFragment;
Expand Down
42 changes: 41 additions & 1 deletion packages/aws-cdk-lib/aws-iam/test/permissions-boundary.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as path from 'path';
import { Match, Template } from '../../assertions';
import { App, CfnResource, CustomResourceProvider, CustomResourceProviderRuntime, Stack } from '../../core';
import { App, AspectPriority, Aspects, CfnResource, CustomResourceProvider, CustomResourceProviderRuntime, Stack } from '../../core';
import * as iam from '../lib';

let app: App;
Expand Down Expand Up @@ -166,3 +166,43 @@ test('unapply inherited boundary from a user: order 2', () => {
PermissionsBoundary: Match.absent(),
});
});

test.each([
[undefined, false],
/* [undefined, true], -- this case doesn't work, that expected */
[AspectPriority.MUTATING, false],
[AspectPriority.MUTATING, true],
])('overriding works if base PB is applied using Aspect with prio %p (feature flag %p)', (basePrio, featureFlag) => {
// When a custom aspect is used to apply a permissions boundary, and the built-in APIs to override it,
// the override still works.

if (featureFlag !== undefined) {
app = new App({ context: { '@aws-cdk/core:aspectPrioritiesMutating': featureFlag } });
stack = new Stack(app, 'Stack');
}

// GIVEN
Aspects.of(stack).add({
visit(node) {
if (node instanceof CfnResource && node.cfnResourceType === 'AWS::IAM::Role') {
node.addPropertyOverride('PermissionsBoundary', 'BASE');
}
},
}, {
priority: basePrio,
});

const role = new iam.Role(stack, 'Role', {
assumedBy: new iam.AnyPrincipal(),
});

// WHEN
iam.PermissionsBoundary.of(role).apply({
managedPolicyArn: 'OVERRIDDEN',
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::IAM::Role', {
PermissionsBoundary: 'OVERRIDDEN',
});
});
5 changes: 4 additions & 1 deletion packages/aws-cdk-lib/aws-servicecatalog/lib/portfolio.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
CloudFormationRuleConstraintOptions, CommonConstraintOptions,
StackSetsConstraintOptions, TagUpdateConstraintOptions,
} from './constraints';
import * as cxapi from '../../cx-api';
import { AssociationManager } from './private/association-manager';
import { hashValues } from './private/util';
import { InputValidator } from './private/validation';
Expand Down Expand Up @@ -369,7 +370,9 @@ export class Portfolio extends PortfolioBase {
(c as Portfolio).addBucketPermissionsToSharedAccounts();
}
},
}, { priority: cdk.AspectPriority.MUTATING });
}, {
priority: cdk.FeatureFlags.of(this).isEnabled(cxapi.ASPECT_PRIORITIES_MUTATING) ? cdk.AspectPriority.MUTATING : undefined,
});
}

protected generateUniqueHash(value: string): string {
Expand Down
4 changes: 3 additions & 1 deletion packages/aws-cdk-lib/core/lib/stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,9 @@ export class Stack extends Construct implements ITaggable {
node.addPropertyOverride('PermissionsBoundary', permissionsBoundaryArn);
}
},
}, { priority: AspectPriority.MUTATING });
}, {
priority: FeatureFlags.of(this).isEnabled(cxapi.ASPECT_PRIORITIES_MUTATING) ? AspectPriority.MUTATING : undefined,
});
}
}

Expand Down
3 changes: 3 additions & 0 deletions packages/aws-cdk-lib/core/lib/stage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ export interface StageProps {
* Options for applying a permissions boundary to all IAM Roles
* and Users created within this Stage
*
* This feature uses Aspects, and the Aspects are applied at the Stack level.
* This is relevant
*
* @default - no permissions boundary is applied
*/
readonly permissionsBoundary?: PermissionsBoundary;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import { IConstruct, MetadataEntry } from 'constructs';
import * as cloudformation from '../../../aws-cloudformation';
import * as lambda from '../../../aws-lambda';
import * as logs from '../../../aws-logs';
import { AspectPriority, Aspects, IAspect, RemovalPolicy } from '../../../core/lib';
import { AspectPriority, Aspects, FeatureFlags, IAspect, RemovalPolicy } from '../../../core/lib';
import * as cxapi from '../../../cx-api';

/* This is duplicated in @aws-cdk/custom-resource-handlers/lib/custom-resources-framework/config.ts */
export const CUSTOM_RESOURCE_PROVIDER = 'aws:cdk:is-custom-resource-handler-customResourceProvider';
Expand Down Expand Up @@ -32,7 +33,9 @@ export class CustomResourceConfig {
* This feature is currently experimental.
*/
public addLogRetentionLifetime(rentention: logs.RetentionDays) {
Aspects.of(this.scope).add(new CustomResourceLogRetention(rentention), { priority: AspectPriority.MUTATING });
Aspects.of(this.scope).add(new CustomResourceLogRetention(rentention), {
priority: FeatureFlags.of(this.scope).isEnabled(cxapi.ASPECT_PRIORITIES_MUTATING) ? AspectPriority.MUTATING : undefined,
});
}

/**
Expand Down
Loading
Loading