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

(aws-eks) Dummy VPC injected during lookup pass cause dependent constructs to eventually fail #19425

Closed
codeJack opened this issue Mar 16, 2022 · 4 comments
Labels
bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.

Comments

@codeJack
Copy link

codeJack commented Mar 16, 2022

What is the problem?

This is a follow-up for issue: #10341

Whenever VPC lookup is in the picture the CDK appears to be doing two passes on the code:

  1. on the first pass the VPC lookups happen to have dummy values (vpc-12345 with subnets s-12345, s-67890, p-12345, and p-67890) that, whenever not compatible with the subnet selection scheme, cause dependent constructs to eventually fail
  2. on the second pass cdk.context.json has been properly materialized and the target selection logic can be successfully applied as the code will be finally dealing with real-life values

Reproduction Steps

The code below fails to synth as the dummy values from the first pass will not satisfy the SubnetSelection scheme.

As opposed to the example listed here I'm using a custom subnet_group_name_tag and SubnetSelection but this is just an implementation detail.

vpc = Vpc.from_lookup(
    scope=self,
    id="VPCLookup",
    vpc_id="<some vpc ID>",
    subnet_group_name_tag="Subnet", # Custom TAG - not on the dummy values
)

# Will throw "Vpc must contain private subnets to configure private endpoint access"
self.cluster = Cluster(
    scope=self,
    id="EKSCluster",
    cluster_name=cluster_name,
    version=KubernetesVersion.V1_21,
    vpc=vpc,
    vpc_subnets=[SubnetSelection(subnet_group_name="some-custom-group-name")],
    default_capacity=0,
    endpoint_access=EndpointAccess.PRIVATE,
    place_cluster_handler_in_vpc=True,
)

Workaround

As mentioned here you need to figure out whether you are on the first or the second pass and handle both cases gracefully with regards to the Constructs consuming the subnet selection.

vpc = Vpc.from_lookup(
    scope=self,
    id="VPCLookup",
    vpc_id="<some vpc ID>",
    subnet_group_name_tag="Subnet", # Custom TAG - not on the dummy values
)

# Workaround
control_plane_subnets = []
if vpc.vpc_id == "vpc-12345":
   # This is the first pass, let's use VPC's dummy private subnets
    control_plane_subnets = vpc.private_subnets
else:
   # This is the second pass I can know apply the final selection logic
    control_plane_subnets = vpc.select_subnets(
        subnet_group_name="some-custom-group-name"
    ).subnets

self.cluster = Cluster(
    scope=self,
    id="EKSCluster",
    cluster_name=cluster_name,
    version=KubernetesVersion.V1_21,
    vpc=vpc,
    vpc_subnets=control_plane_subnets,
    default_capacity=0,
    endpoint_access=EndpointAccess.PRIVATE,
    place_cluster_handler_in_vpc=True,
)

Alternatively, temporarily commenting out the failing code to generate the cdk.context.json should also be viable as this is some kind of "chicken and egg problem". It does NOT work for me since I'm packaging the CDK code as a re-usable asset to be run against a complex and dynamic, multi-account architecture, with no prior knowledge about which account it will be deployed against.

What did you expect to happen?

Ideally, lookups would occur as a preliminary "decoupled" stage and there would be no need to temporarily inject dummy objects into the picture.

What actually happened?

A dummy VPC object (with invalid CIDR 1.2.3.4/5 etc etc) is temporarily injected during the first pass waiting for cdk.context.json to be materialized.

CDK CLI Version

2.8.0 (build 8a5eb49)

Framework Version

aws-cdk-lib==2.8.0

Node.js Version

v16.10.0

OS

MacOS 11.6.3

Language

Python

Language Version

Python(3.9.10)

Other information

No response

@codeJack codeJack added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 16, 2022
@codeJack
Copy link
Author

codeJack commented Mar 16, 2022

As a side note, the same behavior applies to SSM Parameters that, during the first pass, would also assume a dummy value

some_role_arn: str = StringParameter.value_from_lookup(
    scope=self, parameter_name="/some/role/arn"
)

# fails as, "some_role_arn" will be set to "dummy-value-for-/some/role/arn" during the first pass
# which is NOT a valid ARN
Role.from_role_arn(
    scope=self,
    id="SomeRoleARN",
    role_arn=some_role_arn, 
)

This needs to be explicitly handled into the code degrading user experience, code maintainability, readability etc etc

some_role_arn: str = StringParameter.value_from_lookup(
    scope=self, parameter_name="/some/role/arn"
)

if some_role_arn.startswith("dummy-value-for-"):
    some_role_arn = "arn:aws:iam::000000000000:role/dummy-role-arn"

Role.from_role_arn(
    scope=self,
    id="SomeRoleARN",
    role_arn=some_role_arn, 
)

@gshpychka
Copy link
Contributor

Duplicate of #8699

@ryparker
Copy link
Contributor

ryparker commented Mar 16, 2022

Hey @codeJack 👋🏻

Thanks for the detailed report. I'm going to close this however as it does seem to be a duplicate as gshpychka@ mentioned. I encourage you to comment any additional relevant information to the main issue. Thanks 😄

@github-actions
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

No branches or pull requests

3 participants