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

ec2: specifying EC2 private IP with associate public IP enabled generates invalid CFn #26187

Closed
hoppersoft opened this issue Jun 30, 2023 · 4 comments · Fixed by #26208
Closed
Labels
aws-cdk-lib Related to the aws-cdk-lib package bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@hoppersoft
Copy link
Contributor

Describe the bug

When both associatePublicIpAddress and privateIpAddress properties are supplied to the EC2 Instance construct, an invalid CloudFormation template is generated due to the presence of both PrivateIpAddess and NetworkInterfaces properties on the AWS::EC2::Instance resource.

Expected Behavior

The value of the privateIpAddress property should be added to the generated NetworkInterface, omitting the resource-level PrivateIpAddress property.

Current Behavior

The generated template leaves the PrivateIpAddress property populated but also includes a NetworkInterfaces property, resulting in a deployment error: "Network interfaces and an instance-level private IP address may not be specified on the same request"

Reproduction Steps

  1. Create an EC2 instance (using Typescript syntax below):
new Instance(stack, 'Instance', {
      vpc,
      vpcSubnets: { subnetType: SubnetType.PUBLIC },
      securityGroup,
      machineImage: new AmazonLinuxImage(),
      instanceType: InstanceType.of(InstanceClass.T3, InstanceSize.LARGE),
      privateIpAddress: privateIpAddress,
      associatePublicIpAddress: true,
    });
  1. Run cdk synth, capturing the output.
  2. Attempt to cdk deploy - this will result in the "Network interfaces and an instance-level private IP address may not be specified on the same request" error and rollback.
  3. (Extra credit) Inspect the AWS::EC2::Instance resource; you should see something like the below:
  <Resource ID>:
    Type: AWS::EC2::Instance
    Properties:
      #
      # Omitting other properties for brevity
      NetworkInterfaces:
        - AssociatePublicIpAddress: true
          DeviceIndex: "0"
          GroupSet:
            - Fn::GetAtt:
                - <Security Group ID>
                - GroupId
          SubnetId:
            Ref: <Subnet ID>
      PrivateIpAddress: 10.0.1.10

Possible Solution

The logic used to detect the use of associatePublicIpAddress should also include moving the value of privateIpAddress to the NetworkInterface object.

Additional Information/Context

No response

CDK CLI Version

2.85.0 (build 4e0d726)

Framework Version

No response

Node.js Version

18.0.0

OS

Windows, Mac

Language

Typescript

Language Version

5.1.3

Other information

No response

@hoppersoft hoppersoft added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 30, 2023
@github-actions github-actions bot added the aws-cdk-lib Related to the aws-cdk-lib package label Jun 30, 2023
@hoppersoft hoppersoft changed the title aws-cdk-lib: specifying private IP and associate public IP generates invalid CFn aws-cdk-lib: specifying EC2 private IP with associate public IP enabled generates invalid CFn Jul 1, 2023
@pahud
Copy link
Contributor

pahud commented Jul 3, 2023

Per cloudformation doc

You cannot specify this option and the network interfaces option in the same request.

I believe CDK should throw when both this option and network interfaces option are specified.

@pahud pahud added p1 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Jul 3, 2023
@pahud pahud changed the title aws-cdk-lib: specifying EC2 private IP with associate public IP enabled generates invalid CFn ec2: specifying EC2 private IP with associate public IP enabled generates invalid CFn Jul 3, 2023
@hoppersoft
Copy link
Contributor Author

hoppersoft commented Jul 3, 2023

@pahud: as mentioned in #17127, you cannot specify the network interfaces in the construct props. The solution I'm proposing aligns with the current behavior of associatePublicIpAddress where a NetworkInterface is automatically created and moves the private IP to that ENI.

@mergify mergify bot closed this as completed in #26208 Jul 6, 2023
mergify bot pushed a commit that referenced this issue Jul 6, 2023
…enabled generates invalid CFn (#26208)

When both associatePublicIpAddress and privateIpAddress properties are supplied to the EC2 Instance construct, an invalid CloudFormation template is generated due to the presence of both PrivateIpAddess and NetworkInterfaces properties on the AWS::EC2::Instance resource. The generated template leaves the PrivateIpAddress property populated but also includes a NetworkInterfaces property, resulting in a deployment error: "Network interfaces and an instance-level private IP address may not be specified on the same request." (see [the AWS::EC2::Instance docs](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-instance.html#cfn-ec2-instance-privateipaddress))

This erroneous behavior is due to the fact that a network interface is automatically created by the construct when the associatePublicIpAddress is ```true``` while leaving the PrivateIpAddress property on the resource. This PR includes a fix that modifies the behavior of the logic that creates the NetworkInterface to move the private IP to that auto-generated NI, eliminating the top-level resource property.

Closes #26187.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Jul 6, 2023

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

@hoppersoft
Copy link
Contributor Author

@mrgrain I see that this change has been merged in, and I also see that the changelog indicates that the change was incorporated into 2.87.0 (build 9fca790). However, when I test locally, I'm still seeing the invalid EC2 resource being generated. I would debug it, but since I have no mapfiles (see #20561) to walk through the code, I can't. Is there a chance an old copy of aws-cdk-lib is being shipped in 2.87.0?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws-cdk-lib Related to the aws-cdk-lib package bug This issue is a bug. effort/medium Medium work item – several days of effort p1
Projects
None yet
2 participants