Adding an aspect to Bucket with S3 notifications leads to an error

[Dreamescaper]

Describe the bug

In our setup we use an aspect, which adds common tags to most of the resources (like "Name" tag).
Unfortunately, this aspect stopped working after update to 2.181.0 .

Regression Issue

• Select this option if this issue appears to be a regression.

Last Known Working CDK Version

2.180.0

Expected Behavior

Tags should be applied, CDK should be executed successfully.

Current Behavior

'Error: Cannot invoke Aspect Tag with priority 200 on node CdkReproStack/test-bucket/Notifications: an Aspect Object with a lower priority (500) was already invoked on this node.'

Reproduction Steps

    public class CdkReproStack : Stack
    {
        internal CdkReproStack(Construct scope, string id, IStackProps props = null) : base(scope, id, props)
        {
            var bucket = new Bucket(this, "test-bucket", new BucketProps
            {
                BucketName = "test-bucket"
            });

            var function = new Function(this, "test-function", new FunctionProps
            {
                FunctionName = "test-function",
                Handler = "handler",
                Runtime = Runtime.NODEJS_22_X,
                Code = Code.FromInline("function(){ }")
            });

            function.AddEventSource(new S3EventSourceV2(bucket, new S3EventSourceProps
            {
                Events = [EventType.OBJECT_CREATED]
            }));

            Aspects.Of(this).Add(new TagNameAspect(), new AspectOptions { Priority = 10 });
        }
    }

    class TagNameAspect : Amazon.JSII.Runtime.Deputy.DeputyBase, IAspect
    {
        public void Visit(IConstruct node)
        {
            var name = node switch
            {
                Bucket => "test-bucket-name",
                _ => null
            };

            // Auto-generated names start with $, we skip them.
            if (name != null)
            {
                Tags.Of(node).Add("Name", name.ToLowerInvariant(), new TagProps { Priority = 10 });
            }
        }
    }

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.1004.0

Framework Version

v2.186.0

Node.js Version

v22.5.1

OS

AmazonLinux

Language

.NET

Language Version

net8.0

Other information

I have tried adding @aws-cdk/core:aspectStabilization feature flag, but it changes nothing.

[Dreamescaper]

Attaching a zip file with repro project:

CdkRepro.zip

[pahud]

probably related to #33460

Investigating.

[Dreamescaper]

I assume it is related to that PR, but I'm not sure why should it break anything.
The aspect, which I'm adding, has a priority of 10, which is less than both MUTATING (200) and DEFAULT (500).

[pahud]

Current vs Expected Behavior

Key Findings

1. Feature Flag Change: The issue is caused by the @aws-cdk/core:aspectStabilization feature, which was introduced in CDK 2.172.0 but appears to have become default behavior in CDK 2.181.0.
2. Priority Enforcement: With this feature enabled, CDK enforces a strict priority ordering for aspects. The error occurs because:

• When you add an S3 event notification to the bucket using S3EventSourceV2, a child construct called "Notifications" gets created inside the bucket

• During this process, an internal aspect with numerical priority value 500 is automatically applied to this Notifications node (lower execution priority)

• Later in your code, you add your TagNameAspect with a priority value of 10/200 (higher execution priority, because lower number)

• During synthesis, with the aspectStabilization feature enabled, the CDK framework detects this conflict:

  • An aspect with numerical priority 500 was already applied
  • Now an aspect with numerical priority 200/10 is trying to run
  • This violates the rule that aspects should be applied in ascending numerical order
  • The error is thrown because the system doesn't allow higher execution priority aspects (lower numerical values) to run after lower execution priority aspects (higher numerical values)

3. Code Sequence: In the reproduction code, the event source is added to the bucket before the aspect is added to the stack, creating this priority conflict.

Solution Options

1. Set lower priority on your aspect: Change your TagNameAspect to use a priority lower than 500(higher numeric value):

This sets the TagNameAspect to have a numerically higher value of 600, which means it has a lower execution priority than the S3 notifications aspect with priority 500.

Aspects.Of(this).Add(new TagNameAspect(), new AspectOptions { Priority = 600 });

2. Disable the feature flag: Add this to your CDK context:

{
  "@aws-cdk/core:aspectStabilization": false
}

This is confusing due to the terminology, but I need to be precise:

Lower numbers = Run earlier = Higher execution priority
Higher numbers = Run later = Lower execution priority

Let me know if it works for you.

[pahud]

Let me try to explain the Aspect priority using this diagram:

[Dreamescaper]

Set lower priority on your aspect: Change your TagNameAspect to use a priority lower than 500(higher numeric value):

Per my understanding, it wouldn't work.
I add Tags in my aspect, which are aspect themselves (with priority 200). Therefore, I need to execute my aspect before Tags aspects, i.e. have higher priority than 200.

Disable the feature flag:

Again, woudn't work because I add aspects inside my aspect (if I understand feature flag description correctly).

An aspect with numerical priority 500 was already applied

Per my understanding, aspectStabilization feature should prevent this, and run my aspect (with priority 10) before aspect with priority 500. Am I right?
(Btw, should it have MUTATING priority? It adds node dependencies)

[pahud]

Let me write a equivalent typescript CDK app to reproduce this.

[pahud]

Original Reproducible Code

with

% grep "@aws-cdk/core:aspectStabilization" cdk.json
"@aws-cdk/core:aspectStabilization": true

import { Stack, StackProps, Aspects, Tags, IAspect } from 'aws-cdk-lib';
import { Construct, IConstruct } from 'constructs';
import { Bucket, EventType } from 'aws-cdk-lib/aws-s3';
import { Function, Runtime, Code } from 'aws-cdk-lib/aws-lambda';
import { S3EventSourceV2 } from 'aws-cdk-lib/aws-lambda-event-sources';

export class S3AspectNotificationStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    const bucket = new Bucket(this, 'test-bucket', {
      bucketName: 'test-bucket-aspect-notification'
    });

    const lambdaFunction = new Function(this, 'test-function', {
      functionName: 'test-function-aspect-notification',
      handler: 'index.handler',
      runtime: Runtime.NODEJS_22_X,
      code: Code.fromInline('exports.handler = function() { }')
    });

    lambdaFunction.addEventSource(new S3EventSourceV2(bucket, {
      events: [EventType.OBJECT_CREATED]
    }));

    Aspects.of(this).add(new TagNameAspect(), { priority: 10 });
  }
}

class TagNameAspect implements IAspect {
  public visit(node: IConstruct): void {
    let name: string | null = null;
    
    if (node instanceof Bucket) {
      name = 'test-bucket-name';
    }
    
    // Auto-generated names start with $, we skip them.
    if (name !== null) {
      Tags.of(node).add('Name', name.toLowerCase(), { priority: 10 });
    }
  }
}

On cdk diff, I got

Error: Cannot invoke Aspect Tag with priority 200 on node S3AspectNotificationStack/test-bucket/Notifications: an Aspect Object with a lower priority (500) was already invoked on this node.

So this essentially reproduces your original issue.

Now, if we make it this way

export class S3AspectNotificationStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    const bucket = new Bucket(this, 'test-bucket', {
      bucketName: 'test-bucket-aspect-notification'
    });

    const lambdaFunction = new Function(this, 'test-function', {
      functionName: 'test-function-aspect-notification',
      handler: 'index.handler',
      runtime: Runtime.NODEJS_22_X,
      code: Code.fromInline('exports.handler = function() { }')
    });

    lambdaFunction.addEventSource(new S3EventSourceV2(bucket, {
      events: [EventType.OBJECT_CREATED]
    }));

    Aspects.of(this).add(new TagNameAspect(), { priority: 600 });
  }
}

class TagNameAspect implements IAspect {
  public visit(node: IConstruct): void {
    let name: string | null = null;
    
    if (node instanceof Bucket) {
      name = 'test-bucket-name';
    }
    
    if (name !== null) {
      Tags.of(node).add('Name', name.toLowerCase(), { priority: 600 });
    }
  }
}

It would still fail.

 Tags.of(node).add('Name', name.toLowerCase(), { priority: 600 });

This is because Tags class has a hardcoded priority of 200. The priority is tag resolution priority not aspect execution priority.
It's to determine which tag value wins when multiple tags with the same key are applied. Under the hood, it's aspect resolution priority is implicitly set to 200. And when you execute TagNameAspect, you initially have a 600 applied followed by a 200 and that violates the aspectStabilization.

Solutions

Option 1:

disable aspectStabilization, which is probably not you want as this is now default true.

% grep "@aws-cdk/core:aspectStabilization" cdk.json
"@aws-cdk/core:aspectStabilization": false

cdk diff should work.

Option 2:

As you can't use Tags in TagNameAspect as mentioned above, to achieve the same intention, we need to find an alternative way to add a tag to the bucket. You'll need to locate the cfnBucket L1 resource and directly addPropertyOverride "Tags" on it like this.

class TagNameAspect implements IAspect {
  public visit(node: IConstruct): void {
    if (node instanceof Bucket) {
      // Access the underlying CloudFormation resource
      const cfnBucket = node.node.defaultChild as CfnBucket;
      
      // Add the tag directly to the CFN resource
      cfnBucket.addPropertyOverride('Tags', [
        { Key: 'Name', Value: 'test-bucket-name'.toLowerCase() }
      ]);
    }
  }
}

This should work.

Full Code

export class S3AspectNotificationStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    const bucket = new Bucket(this, 'test-bucket', {
      bucketName: 'test-bucket-aspect-notification'
    });
    
    const lambdaFunction = new Function(this, 'test-function', {
      functionName: 'test-function-aspect-notification',
      handler: 'index.handler',
      runtime: Runtime.NODEJS_22_X,
      code: Code.fromInline('exports.handler = function() { }')
    });

    lambdaFunction.addEventSource(new S3EventSourceV2(bucket, {
      events: [EventType.OBJECT_CREATED]
    }));

    Aspects.of(this).add(new TagNameAspect(), { priority: 600 });
  }
}

class TagNameAspect implements IAspect {
  public visit(node: IConstruct): void {
    if (node instanceof Bucket) {
      // Access the underlying CloudFormation resource
      const cfnBucket = node.node.defaultChild as CfnBucket;
      
      // Add the tag directly to the CFN resource
      cfnBucket.addPropertyOverride('Tags', [
        { Key: 'Name', Value: 'test-bucket-name'.toLowerCase() }
      ]);
    }
  }
}

verify it:

% cdk synth S3AspectNotificationStack | grep -A5 "Type: AWS::S3::Bucket"
    Type: AWS::S3::Bucket
    Properties:
      BucketName: test-bucket-aspect-notification
      Tags:
        - Key: Name
          Value: test-bucket-name

Let me know if it works for you.

[Dreamescaper]

It looks quite ugly, especially considering that adding tags is one of the main use-cases, according to docs.
This particular solution wouldn't work, as we're adding tags to lots of resource types, not only buckets.

Anyway, my main question is - is this a bug? Should aspectStibilization be able to order aspects according to their priorities?

[pahud]

@Dreamescaper Thank you for raising your concern.

When aspectStabilization is enabled:

1. Sorting: The CDK framework gathers all aspects applicable to a node (both those added directly and those inherited from parent constructs). It then sorts these aspects based on their numerical priority value in ascending order (lower numbers first).

2. Execution: It iterates through this sorted list and executes the visit method of each aspect on the node.

3. Error Check: During execution, it keeps track of the priority of the last aspect that was successfully invoked on that specific node. Before invoking the next aspect in the sorted list, it checks if this next aspect's priority is numerically lower than the last one invoked. If it is, it throws the error because this violates the strict ascending order rule.

I think the primary concern is the error checking and throwing. It would be great if CDK can handle it internally without throwing the error. I am bringing this up to the team for inputs here.

[Dreamescaper]

My main concern is actually with Sorting part. Why doesn't it work here? My aspect has the highest priority, yet the aspect with lower priority is applied first.
Why, and is that expected?

[rix0rrr]

The reason is that the Aspect with lowest priority doesn't exist yet when sorting happens.

So what happens is:

1. TagNameAspect gets added with priority 500
2. Come execution time, there is one aspect: TagNameAspect. Invoke that.
3. As part of the execution, an AddTag aspect gets added (gets prio 200 by default)
4. There is now an uninvoked Aspect we could invoke. However, if we were to do that, we would execute them in the order 500 first, then 200 which is an inversion of the priority.

The fix would be to give your own aspect a priority of 200 (or lower) as well.

The old behavior is that we used to warn "hey this might not do what you expect" and proceed anyway. The new behavior is to make sure we strictly follow priority order, and fail if we cannot, to make sure that we don't run into cases where behavior doesn't follow contract but happens to work for some people and doesn't work for others.

I'm in doubt on whether or not to make changes to make this easier. The only "fix" I can think of is to give tagging aspects a priority of 500 or higher, so that "default" priority aspects can add them without any further configuration; I'm not sure whether that would be helpful or confusing (since they are technically mutating, but wouldn't have mutating priority anymore).

[Dreamescaper]

The fix would be to give your own aspect a priority of 200 (or lower) as well.

As you can see from repro code, my aspect has a priority of 10.
So per my understanding, it should be executed before any other aspects, right?

[rix0rrr]

I see. Yes it should. My apologies, I was too quick in reading. I'll have a closer look.

[rix0rrr]

Confusingly:

                Tags.Of(node).Add("Name", name.ToLowerInvariant(), new TagProps { Priority = 10 });

This priority has nothing to do with Aspect priority unfortunately. It is Tagging priority. I see we need to change naming here.

[Dreamescaper]

@rix0rrr

I'm talking about this line:

Aspects.Of(this).Add(new TagNameAspect(), new AspectOptions { Priority = 10 });

[rix0rrr]

Yep, I understand.

[rix0rrr]

Found it! Thanks for reporting, and sorry for misunderstanding initially: #33979

[github-actions]

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.

[github-actions]

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.