aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.68k stars 3.92k forks source link

(core): Option to run Aspects after entire construct tree is generated #17140

Closed dontirun closed 3 years ago

dontirun commented 3 years ago

Description

Aspects currently run on constructs as the tree is generated. This feature would additionally allow aspects to be be run again after the entire tree is generated

Use Case

I would like to have the ability to write aspects that check relations between resources to validate if my application is following best practices prior to deployment.

For example I would like to be able to reliably check whether a VPC has an associated flow log.

Currently, this can't be reliably validated this in all scenarios like this one

    const vpc = new Vpc(this, 'rVpc')
    new FlowLog(this, 'rFlowLog', {
      resourceType: FlowLogResourceType.fromVpc(vpc)
    })

While not the most optimal way to create a flow log, in this example the flow log does not in the construct tree when validating the VPC therefore it would lead to a false negative of there being a flow log related to the VPC

Proposed Solution

  1. Add a revisit method or option to run Aspects after the construct tree is generated
  2. Disallow modifications to constructs on the revisit method

Other information

While this could be checked with tests. Aspects can more easily be packaged andapplied across applications and can be used to prevent stack deployment rather than retroactively check

Acknowledge

rix0rrr commented 3 years ago

Aspects currently run on constructs as the tree is generated.

Are they? As far as I'm aware the are invoked just before the tree is synthesized, well after it has been put together:

https://github.com/aws/aws-cdk/blob/master/packages/@aws-cdk/core/lib/private/synthesis.ts/#L16

Can you write some example code which shows that the aspects run too early? Potentially with some console.log()s to show the ordering of events?

dontirun commented 3 years ago

Minimal Code

import { CfnFlowLog, CfnVPC, FlowLog, FlowLogResourceType, FlowLogTrafficType, Vpc } from '@aws-cdk/aws-ec2';
import { Construct, Stack, StackProps, IAspect, IConstruct, Aspects, Annotations } from '@aws-cdk/core';

export class CdkTestStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);
    const vpc = new Vpc(this, 'Vpc', { flowLogs: { One: { trafficType: FlowLogTrafficType.ALL } } })
    vpc.addFlowLog('Two')
    new FlowLog(this, 'Three', {
      resourceType: FlowLogResourceType.fromVpc(vpc)
    })
    new CfnFlowLog(this, 'Not Showing', {
      resourceId: vpc.vpcId,
      resourceType: 'VPC',
      trafficType: FlowLogTrafficType.ALL
    })
    Aspects.of(this).add(new TreeList())
  }
}

class TreeList implements IAspect {
  public visit(node: IConstruct): void {
    if (node instanceof CfnVPC) {
      const children = Stack.of(node).node.findAll()
      for (const child of children) {
        if (child instanceof CfnFlowLog) {
          Annotations.of(child).addError(`<--- Path`);
        }
      }
    }
  }
}

Output on Synth

[Error at /YouAreAwesome/Vpc/One/FlowLog] <--- Path
[Error at /YouAreAwesome/Vpc/Two/FlowLog] <--- Path
[Error at /YouAreAwesome/Three/FlowLog] <--- Path
Found errors
dontirun commented 3 years ago

Closing this issue. I had mismatched versions of the aws-ec2 and core modules that caused this error. Once I updated everything, the issue went away

github-actions[bot] commented 3 years ago

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.