aws/aws-cdk

(pipelines): Various pipeline-related constructs not visited by aspect

stekern opened this issue · 5 comments

What is the problem?

I noticed this issue when trying to tag all resources that are created when using thepipelines.CodePipeline construct. Some constructs (e.g., <...>/UpdatePipeline/SelfMutate/CodePipelineActionRole/Resource) were created and available in my cloud assembly, but never visited by my aspect (and thus not tagged).

I've observed the following behavior:

  • various pipeline-related constructs are not visited by an aspect when using an existing CodePipeline to create a CDK Pipeline and not manually calling buildPipeline.
  • All pipeline-related constructs seem to be visited when not using an existing CodePipeline and/or manually calling buildPipeline.

Reproduction Steps

  1. Synthesize the application defined by the code below.
  2. Verify that constructs such as Pipeline/CodePipeline/UpdatePipeline/SelfMutate/CodePipelineActionRole/Resource are not logged to standard out (i.e., not visited by the aspect), but exist in the generated Cloud assembly cdk.out/Pipeline.template.json.
  3. Manually call buildPipeline and/or remove the use of an existing CodePipeline, synthesize the application and verify that more constructs are visited by the aspect now than previously.
#!/usr/bin/env node

import * as cdk from "@aws-cdk/core"
import * as codepipeline from "@aws-cdk/aws-codepipeline"
import * as pipelines from "@aws-cdk/pipelines"

const app = new cdk.App()

export class MyStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props)
    new cdk.CfnOutput(this, "Output", {
      value: "Hello World",
    })
  }
}

class MyStage extends cdk.Stage {
  constructor(scope: cdk.Construct, id: string, props?: cdk.StageProps) {
    super(scope, id, props)
    new MyStack(this, "stack")
  }
}

export class PipelineStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props)
    const codePipeline = new codepipeline.Pipeline(this, "CodePipeline")
    const cdkPipeline = new pipelines.CodePipeline(this, "CdkPipeline", {
      synth: new pipelines.ShellStep("Synth", {
        input: pipelines.CodePipelineSource.connection(
          "my-org/my-app",
          "main",
          {
            connectionArn:
              "arn:aws:codestar-connections:us-east-1:222222222222:connection/7d2469ff-514a-4e4f-9003-5ca4a43cdc41",
          },
        ),
        commands: ["npm ci", "npm run build", "npx cdk synth"],
      }),
      codePipeline: codePipeline,
    })

    cdkPipeline.addStage(new MyStage(this, "example"))
    // Note: If the following line is uncommented, all expected construct paths are logged during synthesis.
    // cdkPipeline.buildPipeline()
  }
}

new PipelineStack(app, "Pipeline")

cdk.Aspects.of(app).add({
  visit(construct: cdk.IConstruct) {
    console.log(construct.node.path)
  },
})

What did you expect to happen?

  • I would expect the aspect to visit all constructs inside the defined application.
  • I would expect the usage of an existing CodePipeline to not affect this.
  • I would expect to be able to not be required to manually call buildPipeline in order to have the aspect visit all constructs.

What actually happened?

Only a subset of the expected constructs were visited by the aspect.

CDK CLI Version

1.139.0

Framework Version

No response

Node.js Version

14.17.6

OS

MacOS

Language

Typescript

Language Version

4.5.4

Other information

No response

Seeing similar behavior in CDK v2.8.0 as well.

In this specific case the consequences aren't very severe (a handful of missing tags), but it may be an indicator that there is a more general issue wrt. synthesizing and the usage of aspects.

In general I'd expect there to be a guarantee that any construct in a given scope will be visited by an aspect. If there is a bug that breaks this guarantee, it may have more severe implications for other use-cases (e.g., if you're using aspects for compliance checking, security rules, etc.).

This is unfortunately true. For example, constructs generated during aspect visiting time are not guaranteed to be visited again. It is not a bug, it's a limitation.

In the cdk lifecycle, only constructs that are added in the "construction" phase are guaranteed to be visited. By calling buildPipeline() you force the pipeline construct creation forward.

It is not safe to use Aspects as the sole defense for compliance or security checking; not just because of this behavior, but more generically because the CDK employs a general-purpose programming language where anyone could write any code to do anything (also overwrite the output of the synth directory with arbitrary contents after your validation has run).

To get any guarantees at all, you must do security analysis on the artifacts produced by a CDK application (CloudFormation templates etc), after the user code is definitely done running.

You can use Aspects for early warning/convenience, not for security.

⚠️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.

@rix0rrr I fully agree that one should not use Aspects as a single line of defense for security and compliance checking. I brought it up as a more "severe" example compared to the scenario of missing a handful of tags (and also because I've been looking at https://github.com/cdklabs/cdk-nag lately which uses Aspects specifically for compliance and security checks).

I do, however, think it could be helpful to highlight this behavior/limitation in the official pipelines documentation, or in the documentation of Aspects itself, especially if there are other official APIs that work similar to buildPipeline.