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

rix0rrr · 2025-04-14T06:44:39Z

Some CDK methods apply mutating Aspects on behalf of users. Since #32333, these Aspects have a priority of MUTATING to classify their behavior.

If a user-applied Aspect (priority DEFAULT) now configures the same property as an implicitly added Aspect:

Before that change, the relative execution order depended on the location of the Aspects in the construct tree.
After that change, the user Aspect always "wins" (executes last) because its priority is higher.

In this change, we roll back to the behavior from pre-2.172.0, and introduce a feature flag which gives the Aspects a priority only if the feature flag is enabled. This introduces the feature flag:

{
  "context": {
    "@aws-cdk/core:aspectPrioritiesMutating": true
  }
}

Which sets the priority of Aspects added on your behalf a priority of MUTATING (200) (instead of the default DEFAULT, 500).

If you have given your own Aspect a priority of MUTATING already to make sure it can get overridden by another Aspect of priority MUTATING, this current change will not affect you (either with or without feature flag).
If you have come to rely on the new default priority being low already, you can set the above feature flag to re-enable the new behavior.

Did not touch the following Aspects:

In integ-tests-alpha: overriding logical IDs in assertions stacks does not affect production infrastructure.
Tags: tags are exclusively manipulated through the official APIs, so there no conflict between custom and implicit Aspects.
CDK Pipelines: there cannot be a conflict because the customer can't create a default pipeline before the implicit Aspect.

This PR also introduces some slight rendering and documentation changes to the feature flags to improve clarity of the purpose of certain fields and the produced report.

… applications Some CDK methods apply mutatinhg Aspects on behalf of users. Since #32333, these Aspects have a priority of `MUTATING` to classify their behavior. If a user-applied Aspect (priority `DEFAULT`) now configures the same property as an implicitly added Aspect: * Before that change, the relative execution order depended on the location of the Aspects in the construct tree. * After that change, the user Aspect always "wins" (executes last) because its priority is higher. In this change, we introduce a feature flag which gives the Aspects introduced in #32333 that users are likely to override a priority only if the feature flag is enabled. This change does not affect users have given their own Aspects a `MUTATING` priority as well since 2.172.0. Did not touch the following Aspects: - In `integ-tests-alpha`: overriding logical IDs in assertions stacks does not affect production infrastructure. - Tags: tags are exclusively manipulated through the official APIs, so there no conflict between custom and implicit Aspects. - CDK Pipelines: there cannot be a conflict because the customer can't create a default pipeline before the implicit Aspect.

aws-cdk-automation

(This review is outdated)

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

mergify · 2025-04-14T09:13:44Z

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).

iliapolo

What about:

aws-cdk/packages/aws-cdk-lib/core/lib/removal-policies.ts

Line 140 in c5365a0

priority: props.priority ?? AspectPriority.MUTATING,

aws-cdk/packages/aws-cdk-lib/custom-resources/lib/custom-resource-config/custom-resource-config.ts

Line 46 in c5365a0

    
           Aspects.of(this.scope).add(new CustomResourceRemovalPolicy(removalPolicy), { priority: AspectPriority.MUTATING });

aws-cdk/packages/aws-cdk-lib/custom-resources/lib/custom-resource-config/custom-resource-config.ts

Line 55 in c5365a0

    
           Aspects.of(this.scope).add(new CustomResourceLambdaRuntime(lambdaRuntime), { priority: AspectPriority.MUTATING });

Also, you mention:

Tags: tags are exclusively manipulated through the official APIs, so there no conflict between custom and implicit Aspects

Can you elaborate? is it not possible that both Tags and a user defined aspect look at the same .tags property of construct?

packages/aws-cdk-lib/aws-iam/test/permissions-boundary.test.ts

packages/@aws-cdk/aws-servicecatalogappregistry-alpha/lib/application-associator.ts

iliapolo · 2025-04-14T08:52:43Z

packages/aws-cdk-lib/cx-api/lib/features.ts

@@ -1464,6 +1469,39 @@ export const FLAGS: Record<string, FlagInfo> = {
  },

  //////////////////////////////////////////////////////////////////////
+  [ASPECT_PRIORITIES_MUTATING]: {
+    type: FlagType.ApiDefault,
+    summary: 'When set to true, Aspects added by the construct library on your behalf will be given a priority of MUTATING.',


All aspects will be given MUTATING, or just ones that actually mutate?

I don't think we add any non-mutating aspects.

right now

Well this only affects pre-2.172.0 aspects anyway. Aspects added in the future don't have backwards compatibility concerns.

packages/aws-cdk-lib/cx-api/build-tools/flag-report.ts

packages/aws-cdk-lib/custom-resources/lib/custom-resource-config/custom-resource-config.ts

packages/aws-cdk-lib/core/lib/stage.ts

mergify · 2025-04-14T09:28:44Z

Co-authored-by: Eli Polonsky <[email protected]>

rix0rrr · 2025-04-14T10:07:12Z

Re: RemovalPolicies and Tags.

is it not possible that both Tags and a user defined aspect look at the same .tags property of construct?

The reason I didn't do those initially was "blast radius reduction". It is of course possible that a user defined aspect touches those values, but there are well-established and well-known APIs for manipulating them. Since we are protecting solely against the case where a custom aspect picks a default value, and relies on a built-in API to override that default... I figured it is just just unlikely that people would do that, plus they are so widely-used that I considered changing them (again) a little scary.

On the other hand, given that we changed their behavior months ago and we haven't heard anyone complain about it, it should be equally safe to change that behavior back for consistency's sake. Guess I'll do that.

…s/pb-fix

aws-cdk-automation · 2025-04-14T13:24:38Z

AWS CodeBuild CI Report

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

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

mergify · 2025-04-14T13:25:04Z

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).

github-actions · 2025-04-14T13:25:21Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

rix0rrr added 2 commits March 27, 2025 14:17

Clear up flag documentation & report

e7c3c2f

rix0rrr requested a review from a team as a code owner April 14, 2025 06:44

aws-cdk-automation requested a review from a team April 14, 2025 06:44

github-actions bot added the p2 label Apr 14, 2025

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

aws-cdk-automation previously requested changes Apr 14, 2025

View reviewed changes

Merge remote-tracking branch 'origin/main' into huijbers/pb-fix

9fbbe09

rix0rrr added the pr-linter/exempt-integ-test The PR linter will not require integ test changes label Apr 14, 2025

Fix import

d49779b

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

otaviomacedo approved these changes Apr 14, 2025

View reviewed changes

Merge branch 'main' into huijbers/pb-fix

11355ee

iliapolo reviewed Apr 14, 2025

View reviewed changes

iliapolo added the pr/do-not-merge This PR should not be merged at this time. label Apr 14, 2025

Update packages/aws-cdk-lib/aws-iam/test/permissions-boundary.test.ts

dd6f291

Co-authored-by: Eli Polonsky <[email protected]>

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

rix0rrr added 3 commits April 14, 2025 12:11

Comments

f7fded5

Merge branch 'huijbers/pb-fix' of github.com:aws/aws-cdk into huijber…

14c8b52

…s/pb-fix

Unified function

8741a17

iliapolo approved these changes Apr 14, 2025

View reviewed changes

rix0rrr added 2 commits April 14, 2025 13:22

Clarify scope of flag

684976c

Fix build

edf322e

rix0rrr removed the pr/do-not-merge This PR should not be merged at this time. label Apr 14, 2025

Fix unit tests

b59f93f

mergify bot merged commit 9a76fdc into main Apr 14, 2025
13 of 14 checks passed

mergify bot deleted the huijbers/pb-fix branch April 14, 2025 13:25

github-actions bot locked as resolved and limited conversation to collaborators Apr 14, 2025

rix0rrr self-assigned this Apr 17, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

rix0rrr commented Apr 14, 2025 •

edited

Loading

Uh oh!

aws-cdk-automation left a comment •

edited

Loading

Uh oh!

mergify bot commented Apr 14, 2025

Uh oh!

iliapolo left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

iliapolo Apr 14, 2025

Uh oh!

rix0rrr Apr 14, 2025

Uh oh!

iliapolo Apr 14, 2025

Uh oh!

rix0rrr Apr 14, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mergify bot commented Apr 14, 2025

Uh oh!

rix0rrr commented Apr 14, 2025 •

edited

Loading

Uh oh!

aws-cdk-automation commented Apr 14, 2025

Uh oh!

mergify bot commented Apr 14, 2025

Uh oh!

Uh oh!

github-actions bot commented Apr 14, 2025

Uh oh!

Uh oh!

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

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

Uh oh!

Conversation

rix0rrr commented Apr 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aws-cdk-automation left a comment • 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!

mergify bot commented Apr 14, 2025

Uh oh!

iliapolo left a comment • 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!

Uh oh!

Uh oh!

iliapolo Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

rix0rrr Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

iliapolo Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

rix0rrr Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mergify bot commented Apr 14, 2025

Uh oh!

rix0rrr commented Apr 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aws-cdk-automation commented Apr 14, 2025

AWS CodeBuild CI Report

Uh oh!

mergify bot commented Apr 14, 2025

Uh oh!

Uh oh!

github-actions bot commented Apr 14, 2025

Uh oh!

Uh oh!

rix0rrr commented Apr 14, 2025 •

edited

Loading

aws-cdk-automation left a comment •

edited

Loading

iliapolo left a comment •

edited

Loading

rix0rrr commented Apr 14, 2025 •

edited

Loading