aws / aws/aws-cdk

(core): introduce readonly Aspects

Open
#22,426 8 comments 12 reactions 0 assignees View on GitHub
@aws-cdk/core effort/large feature-request p1
Dominant language
TypeScript
Stars
12.9k
Forks
4.6k
Avg merge
2d 3h
Merged PRs (30d)
83

Description

### Describe the feature

The current implementation of aspects can be used to both analyze and modify constructs. Tags, for example, are implemented with aspects. Compliance checks, including cdk-nag, are also often implemented through aspects, but only create annotations and don't change constructs.

Even though the current implementation is deterministic, it doesn't allow to specify the exact execution order of the aspects.
That can lead to situations where aspects don't see the final state of a construct as other aspects may change it later during the execution of the aspects.

This can even have critical security impact when aspects are used for compliance checks. Aspects may make critical changes that stay undetected by the compliance check when the compliance check aspect gets executed before the aspect making the construct modification.

As a solution, I propose to introduce the concept of readonly aspects. That would lead to two categories of aspects:

1) aspects that modify constructs
2) aspects that only analyse constructs, but need to see the final state

During synthesis all normal aspects get invoked before the final tree is synthesised so that they can conduct changes, the readonly aspects in contrast get invoked after so that they see the final state and may only add annotations.

### Use Case

The current implementation of aspects appears to be risky in situations where you need to rely on a specific execution order of the aspects. In my case I use cdk-nag with some extra rules that check tagging, but depending on the execution order cdk-nag doesn't see the final state of a construct and may produce false results.

### Proposed Solution

I think either one of the following might work:

1) add a second parameter "readonly" to Aspects.of to separate normal and readonly asepcts: Aspects.of(app, readonly: true).add(cdk-nag)). The new parameter should be optional and default to false for backward compatibility:

`public static of(scope: IConstruct, readonly?: boolean): Aspects`

2) or create a dedicated class for readonly aspects:

`ReadonlyAspects.of(app).add(...)`

I favour the first version.

### Other Information

_No response_

### Acknowledgements

- [X] I may be able to implement this feature request
- [ ] This feature might incur a breaking change

### CDK version used

2.45.0

### Environment details (OS name and version, etc.)

os independent

Contributor guide

Open the contributing guide

Research direction

Start by tracing Aspects.of and the synthesis flow described in the issue, including how normal aspects are invoked and how annotations are added. Compare the proposed readonly parameter with a dedicated ReadonlyAspects class, then define completion as readonly aspects seeing the final construct state while only adding annotations and preserving existing behavior by default.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
infrastructure
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.