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

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

@@ -1,4 +1,5 @@
 import * as cdk from 'aws-cdk-lib/core';
+import * as cxapi from 'aws-cdk-lib/cx-api';
 import { Construct } from 'constructs';
 import { IApplication } from './application';
 import { CheckedStageStackAssociator } from './aspects/stack-associator';
@@ -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,
+    });
   }
 
   /**
@@ -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;
   }
 

packages/aws-cdk-lib/aws-autoscaling/lib/auto-scaling-group.ts

@@ -17,7 +17,6 @@ import * as iam from '../../aws-iam';
 import * as sns from '../../aws-sns';
 import {
   Annotations,
-  AspectPriority,
   Aspects,
   Aws,
   CfnAutoScalingRollingUpdate, CfnCreationPolicy, CfnUpdatePolicy,
@@ -26,6 +25,7 @@ import {
   Tokenization, UnscopedValidationError, ValidationError, withResolved,
 } from '../../core';
 import { addConstructMetadata, MethodMetadata } from '../../core/lib/metadata-resource';
+import { mutatingAspectPrio32333 } from '../../core/lib/private/aspect-prio';
 import { AUTOSCALING_GENERATE_LAUNCH_TEMPLATE } from '../../cx-api';
 
 /**
@@ -1608,7 +1608,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: mutatingAspectPrio32333(this),
+      });
     }
 
     this.node.addValidation({ validate: () => this.validateTargetGroup() });

packages/aws-cdk-lib/aws-backup/lib/selection.ts

@@ -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 } from '../../core';
 import { addConstructMetadata } from '../../core/lib/metadata-resource';
+import { mutatingAspectPrio32333 } from '../../core/lib/private/aspect-prio';
 
 /**
  * Options for a BackupSelection
@@ -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: mutatingAspectPrio32333(resource.construct),
+      });
       // 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`

packages/aws-cdk-lib/aws-ec2/lib/instance.ts

@@ -15,9 +15,10 @@ import { UserData } from './user-data';
 import { BlockDevice } from './volume';
 import { IVpc, Subnet, SubnetSelection } from './vpc';
 import * as iam from '../../aws-iam';
-import { Annotations, AspectPriority, Aspects, Duration, FeatureFlags, Fn, IResource, Lazy, Resource, Stack, Tags, Token } from '../../core';
+import { Annotations, Aspects, Duration, FeatureFlags, Fn, IResource, Lazy, Resource, Stack, Tags, Token } from '../../core';
 import { md5hash } from '../../core/lib/helpers-internal';
 import { addConstructMetadata, MethodMetadata } from '../../core/lib/metadata-resource';
+import { mutatingAspectPrio32333 } from '../../core/lib/private/aspect-prio';
 import * as cxapi from '../../cx-api';
 
 /**
@@ -671,7 +672,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: mutatingAspectPrio32333(this),
+      });
     }
   }
 

packages/aws-cdk-lib/aws-ecs/lib/cluster.ts

@@ -24,10 +24,10 @@ import {
   IAspect,
   Token,
   Names,
-  AspectPriority,
   FeatureFlags, Annotations,
 } from '../../core';
 import { addConstructMetadata, MethodMetadata } from '../../core/lib/metadata-resource';
+import { mutatingAspectPrio32333 } from '../../core/lib/private/aspect-prio';
 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');
@@ -331,7 +331,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: mutatingAspectPrio32333(this),
+    });
   }
 
   /**

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

@@ -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 { Aspects, CfnResource } from '../../core';
+import { mutatingAspectPrio32333 } from '../../core/lib/private/aspect-prio';
 
 /**
  * Modify the Permissions Boundaries of Users and Roles in a construct tree
@@ -40,7 +41,9 @@ export class PermissionsBoundary {
           node.addPropertyOverride('PermissionsBoundary', boundaryPolicy.managedPolicyArn);
         }
       },
-    }, { priority: AspectPriority.MUTATING });
+    }, {
+      priority: mutatingAspectPrio32333(this.scope),
+    });
   }
 
   /**
@@ -56,6 +59,8 @@ export class PermissionsBoundary {
           node.addPropertyDeletionOverride('PermissionsBoundary');
         }
       },
-    }, { priority: AspectPriority.MUTATING });
+    }, {
+      priority: mutatingAspectPrio32333(this.scope),
+    });
   }
 }

packages/aws-cdk-lib/aws-iam/lib/role.ts

@@ -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 } from '../../core';
 import { getCustomizeRolesConfig, getPrecreatedRoleConfig, CUSTOMIZE_ROLES_CONTEXT_KEY, CustomizeRoleConfig } from '../../core/lib/helpers-internal';
 import { addConstructMetadata, MethodMetadata } from '../../core/lib/metadata-resource';
+import { mutatingAspectPrio32333 } from '../../core/lib/private/aspect-prio';
 
 const MAX_INLINE_SIZE = 10000;
 const MAX_MANAGEDPOL_SIZE = 6000;
@@ -496,7 +497,9 @@ export class Role extends Resource implements IRole {
             this.splitLargePolicy();
           }
         },
-      }, { priority: AspectPriority.MUTATING });
+      }, {
+        priority: mutatingAspectPrio32333(this),
+      });
     }
 
     this.policyFragment = new ArnPrincipal(this.roleArn).policyFragment;

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

@@ -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;
@@ -166,3 +166,44 @@ test('unapply inherited boundary from a user: order 2', () => {
     PermissionsBoundary: Match.absent(),
   });
 });
+
+test.each([
+  [undefined, false, 'OVERRIDDEN'],
+  [AspectPriority.MUTATING, false, 'OVERRIDDEN'],
+  [AspectPriority.MUTATING, true, 'OVERRIDDEN'],
+  // custom DEFAULT, builtin MUTATING: custom wins and override is not applied
+  [undefined, true, 'BASE'],
+])('overriding works if base PB is applied using Aspect with prio %p (feature flag %p)', (basePrio, featureFlag, winner) => {
+  // 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: winner,
+  });
+});

packages/aws-cdk-lib/aws-servicecatalog/lib/portfolio.ts

@@ -15,6 +15,7 @@ import { IBucket } from '../../aws-s3';
 import * as sns from '../../aws-sns';
 import * as cdk from '../../core';
 import { addConstructMetadata } from '../../core/lib/metadata-resource';
+import { mutatingAspectPrio32333 } from '../../core/lib/private/aspect-prio';
 
 /**
  * Options for portfolio share.
@@ -369,7 +370,9 @@ export class Portfolio extends PortfolioBase {
           (c as Portfolio).addBucketPermissionsToSharedAccounts();
         }
       },
-    }, { priority: cdk.AspectPriority.MUTATING });
+    }, {
+      priority: mutatingAspectPrio32333(this),
+    });
   }
 
   protected generateUniqueHash(value: string): string {

packages/aws-cdk-lib/core/lib/private/aspect-prio.ts

@@ -0,0 +1,15 @@
+import { IConstruct } from 'constructs';
+import * as cxapi from '../../../cx-api';
+import { AspectPriority } from '../aspect';
+import { FeatureFlags } from '../feature-flags';
+
+/**
+ * Return the aspect priority of Aspects changed in https://github.com/aws/aws-cdk/pull/32333
+ *
+ * We retroactively made those controllable using a feature flag.
+ *
+ * Aspects newly added since this change should unconditionally have a priority of `MUTATING`.
+ */
+export function mutatingAspectPrio32333(scope: IConstruct) {
+  return FeatureFlags.of(scope).isEnabled(cxapi.ASPECT_PRIORITIES_MUTATING) ? AspectPriority.MUTATING : undefined;
+}

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

@@ -2,6 +2,7 @@ import { IConstruct } from 'constructs';
 import { Annotations } from './annotations';
 import { Aspects, IAspect, AspectPriority } from './aspect';
 import { CfnResource } from './cfn-resource';
+import { mutatingAspectPrio32333 } from './private/aspect-prio';
 import { RemovalPolicy } from './removal-policy';
 
 /**
@@ -137,7 +138,7 @@ export class RemovalPolicies {
    */
   public apply(policy: RemovalPolicy, props: RemovalPolicyProps = {}) {
     Aspects.of(this.scope).add(new RemovalPolicyAspect(policy, props), {
-      priority: props.priority ?? AspectPriority.MUTATING,
+      priority: props.priority ?? mutatingAspectPrio32333(this.scope),
     });
   }
 

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

@@ -4,7 +4,7 @@ import { IConstruct, Construct, Node } from 'constructs';
 import { Annotations } from './annotations';
 import { App } from './app';
 import { Arn, ArnComponents, ArnFormat } from './arn';
-import { AspectPriority, Aspects } from './aspect';
+import { Aspects } from './aspect';
 import { DockerImageAssetLocation, DockerImageAssetSource, FileAssetLocation, FileAssetSource } from './assets';
 import { CfnElement } from './cfn-element';
 import { Fn } from './cfn-fn';
@@ -584,7 +584,9 @@ export class Stack extends Construct implements ITaggable {
             node.addPropertyOverride('PermissionsBoundary', permissionsBoundaryArn);
           }
         },
-      }, { priority: AspectPriority.MUTATING });
+      }, {
+        priority: mutatingAspectPrio32333(this),
+      });
     }
   }
 
@@ -1838,4 +1840,5 @@ import { deployTimeLookup } from './private/region-lookup';
 import { makeUniqueResourceName } from './private/unique-resource-name';
 import { PRIVATE_CONTEXT_DEFAULT_STACK_SYNTHESIZER } from './private/private-context';
 import { Intrinsic } from './private/intrinsic';
+import { mutatingAspectPrio32333 } from './private/aspect-prio';
 /* eslint-enable import/order */

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

@@ -68,6 +68,20 @@ export interface StageProps {
    * Options for applying a permissions boundary to all IAM Roles
    * and Users created within this Stage
    *
+   * Be aware that this feature uses Aspects, and the Aspects are applied at the
+   * Stack level with a priority of `MUTATING` (if the feature flag
+   * `@aws-cdk/core:aspectPrioritiesMutating` is set) or `DEFAULT` (if the flag
+   * is not set). This is relevant if you are both using your own Aspects to
+   * assign Permissions Boundaries, as well as specifying this property.  The
+   * Aspect added by this property will overwrite the Permissions Boundary
+   * assigned by your own Aspect if both: (a) your Aspect has a lower or equal
+   * priority to the automatic Aspect, and (b) your Aspect is applied *above*
+   * the Stack level.  If either of those conditions are not true, your own
+   * Aspect will win.
+   *
+   * We recommend assigning Permissions Boundaries only using the provided APIs,
+   * and not using custom Aspects.
+   *
    * @default - no permissions boundary is applied
    */
   readonly permissionsBoundary?: PermissionsBoundary;

packages/aws-cdk-lib/core/lib/tag-aspect.ts

@@ -1,6 +1,7 @@
 import { Construct, IConstruct } from 'constructs';
 import { Annotations } from './annotations';
-import { IAspect, Aspects, AspectPriority, AspectOptions } from './aspect';
+import { IAspect, Aspects, AspectOptions } from './aspect';
+import { mutatingAspectPrio32333 } from './private/aspect-prio';
 import { ITaggable, ITaggableV2, TagManager } from './tag-manager';
 
 /**
@@ -160,7 +161,7 @@ export class Tags {
    */
   public add(key: string, value: string, props: TagProps = {}) {
     const tag = new Tag(key, value, props);
-    const options: AspectOptions = { priority: AspectPriority.MUTATING };
+    const options: AspectOptions = { priority: mutatingAspectPrio32333(this.scope) };
     Aspects.of(this.scope).add(tag, options);
   }
 
@@ -169,7 +170,7 @@ export class Tags {
    */
   public remove(key: string, props: TagProps = {}) {
     const removeTag = new RemoveTag(key, props);
-    const options: AspectOptions = { priority: AspectPriority.MUTATING };
+    const options: AspectOptions = { priority: mutatingAspectPrio32333(this.scope) };
     Aspects.of(this.scope).add(removeTag, options);
   }
 }

packages/aws-cdk-lib/core/test/aspect.test.ts

@@ -232,7 +232,6 @@ describe('aspect', () => {
     // THEN - check that Tags Aspect is applied to stack with mutating priority
     let aspectApplications = Aspects.of(stack).applied;
     expect(aspectApplications.length).toEqual(2);
-    expect(aspectApplications[1].priority).toEqual(AspectPriority.MUTATING);
 
     // THEN - both Aspects are successfully applied, new logging bucket is added with versioning enabled
     Template.fromStack(stack).hasResourceProperties('AWS::S3::Bucket', {

packages/aws-cdk-lib/core/test/tag-aspect.test.ts

@@ -144,8 +144,9 @@ describe('tag aspect', () => {
     expect(res2.tags.renderTags()).toEqual([{ key: 'first', value: 'there is only 1' }]);
   });
 
-  test('Tags applied without priority get mutating priority value', () => {
+  test.each([false, true])('Tags applied without priority get priority value that depends on feature flag %p', (flag) => {
     const root = new Stack();
+    root.node.setContext('@aws-cdk/core:aspectPrioritiesMutating', flag);
     const res = new TaggableResource(root, 'FakeResource', {
       type: 'AWS::Fake::Thing',
     });
@@ -154,11 +155,13 @@ describe('tag aspect', () => {
     Tags.of(res).add('first', 'there is only 1');
     Tags.of(res).remove('root');
 
+    const expected = flag ? AspectPriority.MUTATING : AspectPriority.DEFAULT;
+
     const rootAspectApplications = Aspects.of(root).applied;
-    expect(rootAspectApplications[0].priority).toEqual(AspectPriority.MUTATING);
+    expect(rootAspectApplications[0].priority).toEqual(expected);
     const resAspectApplications = Aspects.of(res).applied;
-    expect(resAspectApplications[0].priority).toEqual(AspectPriority.MUTATING);
-    expect(resAspectApplications[1].priority).toEqual(AspectPriority.MUTATING);
+    expect(resAspectApplications[0].priority).toEqual(expected);
+    expect(resAspectApplications[1].priority).toEqual(expected);
   });
 
   test('add will add a tag and remove will remove a tag if it exists', () => {