Skip to content

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Jan 24, 2025

Motivation

We got a report from a support case that trying to deploy an EC2 instance with CDK and a basic stack would fail with a KeyError:

2025-01-10 08:31:51 localstack-main  |   File "/opt/code/localstack/.venv/lib/python3.11/site-packages/localstack/services/ec2/resource_providers/aws_ec2_instance.py", line 255, in create
2025-01-10 08:31:51 localstack-main  |     model["PublicIp"] = instance["PublicIpAddress"]
2025-01-10 08:31:51 localstack-main  |                         ~~~~~~~~^^^^^^^^^^^^^^^^^^^
2025-01-10 08:31:51 localstack-main  | KeyError: 'PublicIpAddress'
The Stack:

from aws_cdk import (
    Stack,
    aws_ec2 as ec2,
    CfnOutput,
)
from constructs import Construct

class CdkStack(Stack):

    def __init__(self, scope: Construct, construct_id: str, **kwargs) -> None:
        super().__init__(scope, construct_id, **kwargs)

        # Create a VPC with outbound internet access
        vpc = ec2.Vpc(self, "Vpc", max_azs=3, vpc_name="my-vpc")

        # Create an ec2 instance
        instance = ec2.Instance(
            self,
             "Instance",
            vpc=vpc,
            instance_type=ec2.InstanceType("t3.nano"),
            instance_name="my-instance",
            machine_image=ec2.MachineImage.lookup(
                name="amazonlinux-2023"
            ),
            key_name="my-key"
        )

(Note: To run on AWS, I had to update the stack to use machine_image=ec2.MachineImage.latest_amazon_linux2(), to find the proper AMI, and also create a key pair).

This is due because the PublicIpAddress response field is optional, and is not given to be returned depending on the configuration of the instance (and if the VPC of the instance is the default one or not, see https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-instance-addressing.html#concepts-public-addresses).

This stack did not failed for me, possibly due to the different platform (macOS).

I do not know how to add a test for this, because spawning real instance is pretty difficult as it is very platform dependent, and the fact that we do not return the PublicIpAddress seems to also be dependent on the platform.

I've done a lot of testing on AWS, and with this stack, the instance does not have a Public Address, but it deploys correctly. However, trying to add the following to the stack and deploying again, or deploying from scratch:

CfnOutput(self, "TestOutput", value=instance.instance_public_ip)

See https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ec2.Instance.html#instancepublicip

Will fail in AWS with the following message: Attribute 'PublicIp' does not exist, meaning that the field is actually not attached to the "model", and thus failing.
I believe we can get close to this behavior by conditionally adding the field to the model.

If you have an idea about the testing, please feel free to suggest it.

Changes

  • only add the PublicIpAddress and PublicDnsName to the CFN model if those are present

@bentsku bentsku added aws:ec2 Amazon Elastic Compute Cloud semver: patch Non-breaking changes which can be included in patch releases labels Jan 24, 2025
@bentsku bentsku added this to the 4.1 milestone Jan 24, 2025
@bentsku bentsku self-assigned this Jan 24, 2025
Copy link

LocalStack Community integration with Pro

 2 files  ±    0   2 suites  ±0   27s ⏱️ - 1h 53m 16s
22 tests  - 3 994  20 ✅  - 3 679  2 💤  - 315  0 ❌ ±0 
24 runs   - 3 994  20 ✅  - 3 679  4 💤  - 315  0 ❌ ±0 

Results for commit 4c16b6b. ± Comparison against base commit 44992c0.

This pull request removes 3994 tests.
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

@bentsku bentsku marked this pull request as ready for review January 24, 2025 00:46
Copy link
Contributor

@macnev2013 macnev2013 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix @bentsku! 🦸🏻 LGTM 🚀

I tested this along with @bentsku using Java CDK, and here are the findings:

On AWS:

  • If the instance does not have a PublicIp, CloudFormation rolls back the stack.

On LocalStack:

  • When the PublicIp field is missing in the response, the CloudFormation output shows undefined.
  • When the PublicIp field is present but empty (i.e., ""), the CloudFormation output displays an empty string.

@bentsku bentsku merged commit 8987fe5 into master Jan 28, 2025
40 checks passed
@bentsku bentsku deleted the fix-cfn-ec2-public-address branch January 28, 2025 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws:ec2 Amazon Elastic Compute Cloud semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants