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

(codebuild): removing vpc from codebuild project will not actually remove the vpc configuration #22733

Open
phsiao opened this issue Nov 1, 2022 · 6 comments
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud bug This issue is a bug. effort/small Small work item – less than a day of effort needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. p2

Comments

@phsiao
Copy link

phsiao commented Nov 1, 2022

Describe the bug

If you build a CodeBuild Project first with a vpc and deploy, then remove the vpc and perform the second deploy, the related IAM permission would be removed but the vpc remains in the CodeBuild Project's configuration.

Below is a simple example that I can use to reproduce the problem

export class MyStack extends Stack {
  constructor(scope: Construct, id: string, props: StackProps = {}) {
    super(scope, id, props);

    const vpc = ec2.Vpc.fromLookup(this, "Vpc", {
      vpcId: "vpc-12345678",   // any valid vpc id in the target account
    });

    new codebuild.Project(this, "Project", {
      // vpc,
      buildSpec: codebuild.BuildSpec.fromObject({
        version: "0.2",
        env: {},
        phases: {
          build: {
            commands: 'echo "Hello World!"',
          },
        },
      }),
    });
  }
}

This is problematic especially with pipeline, because the pipeline would fail because it can't perform ec2:DescribeNetworkInterfaces on the vpc and the project fails during Queue time.

Expected Behavior

The VPC configuration be cleared from the CodeBuild Project.

Current Behavior

The VPC configuration remains in the CodeBuild Project according to console and cli.

Reproduction Steps

First deploy this

export class MyStack extends Stack {
  constructor(scope: Construct, id: string, props: StackProps = {}) {
    super(scope, id, props);

    const vpc = ec2.Vpc.fromLookup(this, "Vpc", {
      vpcId: "vpc-12345678",   // any valid vpc id in the target account
    });

    new codebuild.Project(this, "Project", {
      vpc,
      buildSpec: codebuild.BuildSpec.fromObject({
        version: "0.2",
        env: {},
        phases: {
          build: {
            commands: 'echo "Hello World!"',
          },
        },
      }),
    });
  }
}

then deploy this

export class MyStack extends Stack {
  constructor(scope: Construct, id: string, props: StackProps = {}) {
    super(scope, id, props);

    ec2.Vpc.fromLookup(this, "Vpc", {
      vpcId: "vpc-12345678",   // any valid vpc id in the target account
    });

    new codebuild.Project(this, "Project", {
      // vpc,
      buildSpec: codebuild.BuildSpec.fromObject({
        version: "0.2",
        env: {},
        phases: {
          build: {
            commands: 'echo "Hello World!"',
          },
        },
      }),
    });
  }
}

The second deploy would remove the Project role's ability to perform various ec2 actions, including ec2:DescribeNetworkInterfaces. But the vpc configuration remains in the Project (according to console as well as aws cli)

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.50.0

Framework Version

No response

Node.js Version

v18.6.0

OS

MacOS

Language

Typescript

Language Version

No response

Other information

No response

@phsiao phsiao added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 1, 2022
@github-actions github-actions bot added the @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud label Nov 1, 2022
@peterwoodworth
Copy link
Contributor

Thanks for reporting this @phsiao, I've been able to confirm this behavior.

This appears to be a bug on CloudFormation's end - the template we deploy is accurate and doesn't include any Vpc configurations when not defined, but the infrastructure isn't updated when CloudFormation updates the resource.

I've reported this internally to CloudFormation - P74606487 - I will provide updates as they become available

@peterwoodworth peterwoodworth added needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. and removed needs-triage This issue or PR still needs to be triaged. labels Nov 2, 2022
@peterwoodworth peterwoodworth changed the title (codebuild): change from with vpc to be without vpc does not remove vpc from build configuration (codebuild): removing vpc from codebuild project will not actually remove the vpc configuration Nov 2, 2022
@peterwoodworth
Copy link
Contributor

peterwoodworth commented Nov 3, 2022

OK they've let us know that to actually unset the VPC we need to pass in an empty object (see below), otherwise it will maintain the same value.

Resources:
  CodeBuildProject:
    Type: AWS::CodeBuild::Project
    Properties:
      Name: aaa
      Description: testss
      ServiceRole: somerole
        Type: CODEPIPELINE
      Environment:
        Type: LINUX_CONTAINER
        ComputeType: BUILD_GENERAL1_SMALL
        Image: aws/codebuild/ubuntu-base:14.04
      Source:
        Type: CODEPIPELINE
      TimeoutInMinutes: 10
      Tags: []
      VpcConfig: {}

Not only is this true for VpcConfig property, it's true for all CodeBuild Project properties

per CodeBuild team, while updating a CodeBuild project, not specifying a property is equivalent to not updating it and retaining the existing value for it. An empty value needs to be specified in the template

We should ensure that we account for this on all properties, not just VpcConfig. There are quite a few...

const resource = new CfnProject(this, 'Resource', {

I am marking this issue as p2, which means that we are unable to work on this immediately.

We use +1s to help prioritize our work, and are happy to revaluate this issue based on community feedback. You can reach out to the cdk.dev community on Slack to solicit support for reprioritization.

Check out our contributing guide if you're interested in contributing yourself - there's a low chance the team will be able to address this soon but we'd be happy to review a PR 🙂

Until we get this fixed, you can use an escape hatch to set the value of any property to an empty value like the following

const project = new Project(this, 'Project', {...});
(project.node.defaultChild as CfnProject).addPropertyOverride('VpcConfig', {});

@peterwoodworth peterwoodworth added p2 effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md and removed needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. labels Nov 3, 2022
@hoegertn
Copy link
Contributor

hoegertn commented Nov 3, 2022

Sorry, but imho this is still a CFN bug. The behavior of CFN is to use default values when not specifying a value and not the previous value. Otherwise this would lead to two different states if you apply the same template to an existing stack vs a new stack.

Please escalate this. I am currently looking into another strange effect on the CodeBuild project resource on CFN updates. So maybe there are some more hidden bugs out there.

@peterwoodworth
Copy link
Contributor

@hoegertn thanks for sharing your opinion, I've shared this concern with the team.

I've come across this behavior before as well, see #18077

@rix0rrr
Copy link
Contributor

rix0rrr commented Jan 17, 2023

Yeah, CodeBuild Projects implement updates using the UpdateProject API, which ignores unspecified values. This is an upstream bug.

@rix0rrr rix0rrr added needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. and removed good first issue Related to contributions. See CONTRIBUTING.md labels Jan 17, 2023
@amcubic
Copy link

amcubic commented Mar 22, 2023

In addition to not removing the VPC it also appears that the values are not updated. For example, if you change the subnets on the VPC object it does not seem to be applied. My workaround was to try and remove the VPC config and add it back though that didn't work either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud bug This issue is a bug. effort/small Small work item – less than a day of effort needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. p2
Projects
None yet
Development

No branches or pull requests

6 participants