aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.51k stars 3.85k forks source link

elasticloadbalancingv2: elvb2.HealthCheck requires ecs.Protocol instead of elbv2.Protocol #22689

Open cmoore1776 opened 1 year ago

cmoore1776 commented 1 year ago

Describe the bug

In CDK 2.48.0+, typeguard in Python is requiring that the Protocol parameter of elasticloadbalancingv2.HealthCheck is of type ecs.Protocol, not elasticloadbalancingv2.Protocol as it was in CDK 2.47.0 and older.

Expected Behavior

elasticloadbalancingv2.HealthCheck should require elasticloadbalancingv2.Protocol, not ecs.Protocol

The following Python code should be valid:

elbv2.ApplicationTargetGroup(
  scope= self,
  id= "webserverTargetGroup",
  port= 80,
  target_type= elbv2.TargetType.IP,
  vpc= myVpc,
  health_check= elbv2.HealthCheck(
    protocol= elbv2.Protocol.HTTP,
    healthy_threshold_count= 2,
    unhealthy_threshold_count= 4
  )
)

Current Behavior

The above code results in the following error during synthesis:

TypeError: type of argument protocol must be one of (aws_cdk.aws_ecs.Protocol, NoneType); got aws_cdk.aws_elasticloadbalancingv2.Protocol instead

Reproduction Steps

See "Expected Behavior" above

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.48.0 and 2.49.0

Framework Version

2.48.0 and 2.49.0

Node.js Version

v18.12.0

OS

macOS 12.6

Language

Python

Language Version

Python 3.10.8

Other information

No response

hectorvs commented 1 year ago

I've hit this issue too, I ended up commenting out the attribute and adding it via escape hatch like so

        health_check = elbv2.HealthCheck(
            interval=Duration.seconds(11),
            path=healthcheck_path,
            port=healthcheck_port,
            # protocol=elbv2.Protocol.HTTPS, # known bug https://github.com/aws/aws-cdk/issues/22689
            timeout=Duration.seconds(10),
            healthy_http_codes="200,202",
        )

        target_group = elbv2.ApplicationTargetGroup(
            self,
            id="TargetGroup",
            target_type=elbv2.TargetType.IP,
            target_group_name=tg_name,
            health_check=health_check,
            protocol=elbv2.ApplicationProtocol.HTTPS,
            port=app_port,
            vpc=vpc,
            deregistration_delay=Duration.seconds(10),
        )

        # escape hatch
        target_group.node.default_child.add_override(
            "Properties.HealthCheckProtocol", "HTTPS"
        )
corymhall commented 1 year ago

@shamelesscookie it looks like I can't reproduce the issue with the code that you provided. I know there an issue because we've received a couple other issues reporting the same type of thing. Can you provide more of your CDK app so we can reproduce your case? I'm assuming there is some ECS code in there.

cmoore1776 commented 1 year ago

@corymhall Here's a complete repro case.

Findings: If, anywhere in your stack, you instantiate ecs.FargateTaskDefintion and then invoke add_port_mapping, all usage of elbv2.HealthCheck in that stack, whether related or not, must occur prior to any ecs references, or the stack will fail to synthesize.

#!/usr/bin/env python3

from constructs import Construct
from aws_cdk import App, Stack, Environment

import aws_cdk.aws_ec2 as ec2
import aws_cdk.aws_elasticloadbalancingv2 as elbv2
import aws_cdk.aws_ecs as ecs

app = App()
AccountId = "<redacted>"

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

    self.TestVpc= ec2.Vpc.from_lookup(scope= self, id= "TestVpc",
      vpc_name= "TestVpc"
    )

    self.TestTargetGroup = elbv2.ApplicationTargetGroup(scope= self, id= "TestTargetGroup",
      vpc= self.TestVpc,
      health_check= elbv2.HealthCheck(
        protocol= elbv2.Protocol.HTTP
      )
    )

    # 👆 👇 Reverse the order of the TargetGroup and Cluster to induce/resolve the issue 
    # (even though the two resources are unrelated, except for sharing a VPC)

    self.TestEcsCluster= ecs.Cluster(scope= self, id= "TestEcsCluster",
      vpc= self.TestVpc
    )

    self.TestTaskDefinition= ecs.FargateTaskDefinition(scope= self, id= "TestTaskDefinition")

    NginxContainer = self.TestTaskDefinition.add_container(
      id= "nginx-container",
      image= ecs.ContainerImage.from_registry("nginx")
    )

    # If you comment out the add_port_mappings statement below,
    # the stack will always synthesize, regardless of order above

    NginxContainer.add_port_mappings(ecs.PortMapping(
      container_port= 80,
      protocol= ecs.Protocol.TCP
    ))

TestStack(
  app,
  construct_id= "test-stack",
  env= Environment(account= AccountId, region= "us-east-1")  
)

app.synth()
RomainMuller commented 1 year ago

@shamelesscookie you're correct... there is an order-of-initialization issue here caused by the use of some forward-declarations that trip the type resolver around homonymous types (the forward refs. are emitted un-qualified).