Skip to content
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

feat(aspects): priority-ordered aspect invocation #32097

Merged
merged 76 commits into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
76 commits
Select commit Hold shift + click to select a range
9b9ce2f
WIP - initial Aspect work; added AspectApplication class, initial uni…
sumupitchayan Nov 11, 2024
6e6cb16
WIP - initial pass rewriting invokeAspects function, yet to be tested
sumupitchayan Nov 11, 2024
76c923b
fix build errors
sumupitchayan Nov 11, 2024
ec1d7b0
Merge remote-tracking branch 'origin/main' into sumughan/priority-ord…
sumupitchayan Nov 11, 2024
c88f355
add docstrings to AspectApplication class
sumupitchayan Nov 11, 2024
af5522b
WIP - add unit test for in-place mutation Aspect getting applied
sumupitchayan Nov 11, 2024
e8388eb
WIP - add unit test for single mutating aspect that adds a node
sumupitchayan Nov 11, 2024
3a7d05a
WIP - add 2 mutating aspects test (currently failing)
sumupitchayan Nov 11, 2024
70a437d
WIP - fix 2 mutating Aspect test
sumupitchayan Nov 12, 2024
12d54f9
WIP - remove duplicated aspects array in Aspects class
sumupitchayan Nov 12, 2024
6123866
WIP - return copied list in list() function
sumupitchayan Nov 12, 2024
5e016f6
WIP - wrap priority in AspectOptions for future extensibility
sumupitchayan Nov 12, 2024
549fbbd
WIP/refactor - make priority setter public, remove function
sumupitchayan Nov 12, 2024
7e91acb
WIP - add AspectPriority class with static default variables
sumupitchayan Nov 12, 2024
8f208b1
docstrings for AspectPriority static variables
sumupitchayan Nov 12, 2024
40f2a82
WIP - add check to prevent crossing stage boundary, fixing stage tests
sumupitchayan Nov 12, 2024
0786786
delete commented out old aspects function
sumupitchayan Nov 13, 2024
477deaa
WIP - add test using Tags Aspect
sumupitchayan Nov 13, 2024
9706de9
WIP - add warning for nested aspects, fixes unit test
sumupitchayan Nov 13, 2024
2341245
linter errors
sumupitchayan Nov 14, 2024
99910e9
WIP - fix orderedPriority array in invokeAspects to order numbers, no…
sumupitchayan Nov 18, 2024
09f8643
WIP - use PriorityQueue in invokeAspects algo; still failing 1 unit test
sumupitchayan Nov 19, 2024
f4816be
WIP - temporarily comment out assertion in nested Aspect warning test
sumupitchayan Nov 19, 2024
8141a0e
Merge branch 'main' into sumughan/priority-ordered-aspects
sumupitchayan Nov 19, 2024
a1c9642
WIP - fix example-construct-library/test/integ.example-resource.ts
sumupitchayan Nov 19, 2024
f905bb8
Merge branch 'sumughan/priority-ordered-aspects' of https://github.co…
sumupitchayan Nov 19, 2024
5f7fd33
WIP - remove console statements, try to fix broken integ tests in alpha
sumupitchayan Nov 20, 2024
dad0462
Revert "WIP - fix example-construct-library/test/integ.example-resour…
sumupitchayan Nov 20, 2024
477d003
WIP - return to stabilization loop solution.
sumupitchayan Nov 21, 2024
3ad2afe
WIP - add check that aspect priority cannot be less than priority of …
sumupitchayan Nov 21, 2024
2fee57c
Start of property test suite for aspects
rix0rrr Nov 22, 2024
13f0327
Some code improvements around the proptests
rix0rrr Nov 22, 2024
6b39626
Introduce helpers to reason about priorities
rix0rrr Nov 22, 2024
27d14d5
Fix problems with mutable state sharing
rix0rrr Nov 22, 2024
2ab5c8c
Fix state sharing of visit log as well
rix0rrr Nov 22, 2024
eaa4675
Update some more docs
rix0rrr Nov 22, 2024
83bcd50
Refactor prio code a bit
rix0rrr Nov 22, 2024
7d8f829
Add another test skeleton
rix0rrr Nov 22, 2024
9a6a8c1
WIP - add Feature Flag for aspectStabilization
sumupitchayan Nov 25, 2024
3c08fa1
WIP - fix imports in synthesis
sumupitchayan Nov 25, 2024
4254865
WIP - update README
sumupitchayan Nov 25, 2024
063acd2
Fix import order
sumupitchayan Nov 25, 2024
c650bd0
WIP - add aspect stabilization to README
sumupitchayan Nov 25, 2024
ee9a93c
Fix typos
rix0rrr Nov 25, 2024
dafc3aa
Merge branch 'sumughan/priority-ordered-aspects' of github.com:aws/aw…
rix0rrr Nov 25, 2024
e2cb8d2
Record additions of constructs and aspects
rix0rrr Nov 25, 2024
018cc41
WIP - fix duplicate invocation bug
sumupitchayan Nov 25, 2024
320d664
return false if feature flag/stabilize aspects option is undefined
sumupitchayan Nov 25, 2024
1df18e3
WIP - add back error for lower prio aspect being invoked after higher…
sumupitchayan Nov 25, 2024
aaef83f
WIP - add boolean param to afterSynth, use fc.boolean in prop tests f…
sumupitchayan Nov 25, 2024
184bd70
linter
sumupitchayan Nov 25, 2024
67398c3
Merge branch 'sumughan/priority-ordered-aspects' of https://github.co…
sumupitchayan Nov 25, 2024
6d0dd20
Rico review: add more explanation to aspectStabilization stage synth …
sumupitchayan Nov 25, 2024
81d458e
Rico doc/error comments
sumupitchayan Nov 25, 2024
ff1df34
linter: remove IAspect import from synthesis.ts
sumupitchayan Nov 25, 2024
d3409da
WIP - add 2 prop tests (currently commenting out AddingAspect)
sumupitchayan Nov 25, 2024
e4ef96c
WIP - fix test error expected string msg
sumupitchayan Nov 26, 2024
169a768
WIP - last prop test (not succeding yet tho)
sumupitchayan Nov 26, 2024
1e325dd
Don't use JSON.stringify
rix0rrr Nov 26, 2024
8161bfc
Don't keep references to constructs, always use paths
rix0rrr Nov 26, 2024
88dd0a9
Change toString to be more readable
rix0rrr Nov 26, 2024
b4f80bb
WIP - add infinite recursion unit test
sumupitchayan Nov 26, 2024
8694931
WIP - finish prop tests
sumupitchayan Nov 27, 2024
fd47e1d
Merge branch 'main' into sumughan/priority-ordered-aspects
sumupitchayan Nov 27, 2024
654c429
WIP - add feature flag version/default for future versions
sumupitchayan Nov 29, 2024
cee54b8
WIP - Rico comments on README
sumupitchayan Nov 29, 2024
42f6de2
WIP - Rico comment, remove Aspects with Third-Party Constructs sectio…
sumupitchayan Nov 29, 2024
ff7263b
WIP - Rico comment: improve error message for Aspect with lower prior…
sumupitchayan Nov 29, 2024
76c8bc4
WIP - remove unnecessary getAllConstructInScope function
sumupitchayan Nov 29, 2024
0f0b59c
Update packages/aws-cdk-lib/core/test/aspect.prop.test.ts
sumupitchayan Nov 29, 2024
0dda439
WIP - rename list to applied, also fix Warn unit test (since feature …
sumupitchayan Nov 29, 2024
f8e7d95
Merge branch 'sumughan/priority-ordered-aspects' of https://github.co…
sumupitchayan Nov 29, 2024
23c0f49
WIP - fix feature flag test
sumupitchayan Nov 29, 2024
3e62ef7
Merge branch 'main' into sumughan/priority-ordered-aspects
sumupitchayan Nov 29, 2024
c156482
WIP - fix features test (use 'V2Next' placeholder in feature flag list
sumupitchayan Nov 29, 2024
ed93f22
Merge branch 'sumughan/priority-ordered-aspects' of https://github.co…
sumupitchayan Nov 29, 2024
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
142 changes: 142 additions & 0 deletions packages/aws-cdk-lib/core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1629,6 +1629,148 @@ scenarios are (non-exhaustive list):
valid)
- Warn if the user is using a deprecated API

## Aspects

[Aspects](https://docs.aws.amazon.com/cdk/v2/guide/aspects.html) is a feature in CDK that allows you to apply operations or transformations across all
constructs in a construct tree. Common use cases include tagging resources, enforcing encryption on S3 Buckets, or applying specific security or
compliance rules to all resources in a stack.

Conceptually, there are two types of Aspects:

- **Read-only aspects** scan the construct tree but do not make changes to the tree. Common use cases of read-only aspects include performing validations
(for example, enforcing that all S3 Buckets have versioning enabled) and logging (for example, collecting information about all deployed resources for
audits or compliance).
- **Mutating aspects** either (1.) add new nodes or (2.) mutate existing nodes of the tree in-place. One commonly used mutating Aspect is adding Tags to
resources. An example of an Aspect that adds a node is one that automatically adds a security group to every EC2 instance in the construct tree if
no default is specified.

Here is a simple example of creating and applying an Aspect on a Stack to enable versioning on all S3 Buckets:

```ts
import { IAspect, IConstruct, Tags, Stack } from 'aws-cdk-lib';

class EnableBucketVersioning implements IAspect {
visit(node: IConstruct) {
if (node instanceof CfnBucket) {
node.versioningConfiguration = {
status: 'Enabled'
};
}
}
}

const app = new App();
const stack = new MyStack(app, 'MyStack');

// Apply the aspect to enable versioning on all S3 Buckets
Aspects.of(stack).add(new EnableBucketVersioning());
```

### Aspect Stabilization

The modern behavior is that Aspects automatically run on newly added nodes to the construct tree. This is controlled by the
flag `@aws-cdk/core:aspectStabilization`, which is default for new projects (since version 2.172.0).

The old behavior of Aspects (without stabilization) was that Aspect invocation runs once on the entire construct
tree. This meant that nested Aspects (Aspects that create new Aspects) are not invoked and nodes created by Aspects at a higher level of the construct tree are not visited.

To enable the stabilization behavior for older versions, use this feature by putting the following into your `cdk.context.json`:

```json
{
"@aws-cdk/core:aspectStabilization": true
}
```

### Aspect Priorities

Users can specify the order in which Aspects are applied on a construct by using the optional priority parameter when applying an Aspect. Priority
sumupitchayan marked this conversation as resolved.
Show resolved Hide resolved
values must be non-negative integers, where a higher number means the Aspect will be applied later, and a lower number means it will be applied sooner.

By default, newly created nodes always inherit aspects. Priorities are mainly for ordering between mutating aspects on the construct tree.

CDK provides standard priority values for mutating and readonly aspects to help ensure consistency across different construct libraries:

```ts
/**
* Default Priority values for Aspects.
*/
export class AspectPriority {
/**
* Suggested priority for Aspects that mutate the construct tree.
*/
static readonly MUTATING: number = 200;

/**
* Suggested priority for Aspects that only read the construct tree.
*/
static readonly READONLY: number = 1000;

/**
* Default priority for Aspects that are applied without a priority.
*/
static readonly DEFAULT: number = 500;
}
```

If no priority is provided, the default value will be 500. This ensures that aspects without a specified priority run after mutating aspects but before
any readonly aspects.

Correctly applying Aspects with priority values ensures that mutating aspects (such as adding tags or resources) run before validation aspects. This allows users to avoid misconfigurations and ensure that the final
construct tree is fully validated before being synthesized.

### Applying Aspects with Priority

```ts
import { Aspects, Stack, IAspect, Tags } from 'aws-cdk-lib';
import { Bucket } from 'aws-cdk-lib/aws-s3';

class MyAspect implements IAspect {
visit(node: IConstruct) {
// Modifies a resource in some way
}
}

class ValidationAspect implements IAspect {
visit(node: IConstruct) {
// Perform some readonly validation on the cosntruct tree
}
}

const stack = new Stack();

Aspects.of(stack).add(new MyAspect(), { priority: AspectPriority.MUTATING } ); // Run first (mutating aspects)
Aspects.of(stack).add(new ValidationAspect(), { priority: AspectPriority.READONLY } ); // Run later (readonly aspects)
```

### Inspecting applied aspects and changing priorities

We also give customers the ability to view all of their applied aspects and override the priority on these aspects.
The `AspectApplication` class represents an Aspect that is applied to a node of the construct tree with a priority.

Users can access AspectApplications on a node by calling `applied` from the Aspects class as follows:

```ts
const app = new App();
const stack = new MyStack(app, 'MyStack');

Aspects.of(stack).add(new MyAspect());

let aspectApplications: AspectApplication[] = Aspects.of(root).applied;

for (const aspectApplication of aspectApplications) {
// The aspect we are applying
console.log(aspectApplication.aspect);
// The construct we are applying the aspect to
console.log(aspectApplication.construct);
// The priority it was applied with
console.log(aspectApplication.priority);

// Change the priority
aspectApplication.priority = 700;
}
```

### Acknowledging Warnings

If you would like to run with `--strict` mode enabled (warnings will throw
Expand Down
108 changes: 100 additions & 8 deletions packages/aws-cdk-lib/core/lib/aspect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,39 @@ export interface IAspect {
visit(node: IConstruct): void;
}

/**
* Default Priority values for Aspects.
*/
export class AspectPriority {
/**
* Suggested priority for Aspects that mutate the construct tree.
*/
static readonly MUTATING: number = 200;

/**
* Suggested priority for Aspects that only read the construct tree.
*/
static readonly READONLY: number = 1000;

/**
* Default priority for Aspects that are applied without a priority.
*/
static readonly DEFAULT: number = 500;
}

/**
* Options when Applying an Aspect.
*/
export interface AspectOptions {
/**
* The priority value to apply on an Aspect.
* Priority must be a non-negative integer.
*
* @default - AspectPriority.DEFAULT
*/
readonly priority?: number;
}

/**
* Aspects can be applied to CDK tree scopes and can operate on the tree before
* synthesis.
Expand All @@ -24,7 +57,7 @@ export class Aspects {
public static of(scope: IConstruct): Aspects {
let aspects = (scope as any)[ASPECTS_SYMBOL];
if (!aspects) {
aspects = new Aspects();
aspects = new Aspects(scope);

Object.defineProperty(scope, ASPECTS_SYMBOL, {
value: aspects,
Expand All @@ -35,24 +68,83 @@ export class Aspects {
return aspects;
}

private readonly _aspects: IAspect[];
private readonly _scope: IConstruct;
private readonly _appliedAspects: AspectApplication[];
sumupitchayan marked this conversation as resolved.
Show resolved Hide resolved

private constructor() {
this._aspects = [];
private constructor(scope: IConstruct) {
this._appliedAspects = [];
this._scope = scope;
}

/**
* Adds an aspect to apply this scope before synthesis.
* @param aspect The aspect to add.
* @param options Options to apply on this aspect.
*/
public add(aspect: IAspect) {
this._aspects.push(aspect);
public add(aspect: IAspect, options?: AspectOptions) {
this._appliedAspects.push(new AspectApplication(this._scope, aspect, options?.priority ?? AspectPriority.DEFAULT));
}

/**
* The list of aspects which were directly applied on this scope.
*/
public get all(): IAspect[] {
return [...this._aspects];
return this._appliedAspects.map(application => application.aspect);
}

/**
* The list of aspects with priority which were directly applied on this scope.
*
* Also returns inherited Aspects of this node.
*/
public get applied(): AspectApplication[] {
return [...this._appliedAspects];
}
}

/**
* Object respresenting an Aspect application. Stores the Aspect, the pointer to the construct it was applied
* to, and the priority value of that Aspect.
*/
export class AspectApplication {
/**
* The construct that the Aspect was applied to.
*/
public readonly construct: IConstruct;

/**
* The Aspect that was applied.
*/
public readonly aspect: IAspect;

/**
* The priority value of this Aspect. Must be non-negative integer.
*/
private _priority: number;

/**
* Initializes AspectApplication object
*/
public constructor(construct: IConstruct, aspect: IAspect, priority: number) {
this.construct = construct;
this.aspect = aspect;
this._priority = priority;
}

/**
* Gets the priority value.
*/
public get priority(): number {
return this._priority;
}
}

/**
* Sets the priority value.
*/
public set priority(priority: number) {
if (priority < 0) {
throw new Error('Priority must be a non-negative number');
}
this._priority = priority;
}
}
Loading