cloudtools / troposphere

troposphere - Python library to create AWS CloudFormation descriptions
BSD 2-Clause "Simplified" License
4.92k stars 1.45k forks source link

AWS::ECS::TaskSet.LoadBalancer shadows AWS::ECS::Service.LoadBalancer #2206

Closed sterwill closed 7 months ago

sterwill commented 8 months ago

When I upgraded troposphere recently, troposphere.ecs.LoadBalancer lost its LoadBalancerName attribute, which is necessary for classic ELB support. The CloudFormation docs for AWS::ECS::Service LoadBalancer still list LoadBalancerName, so it looked like a troposphere issue. The comment on the class gave me a clue:

class LoadBalancer(AWSProperty):
    """
    `LoadBalancer <http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ecs-taskset-loadbalancer.html>`__
    """

It looks like the AWS::ECS::TaskSet.LoadBalancer is getting used to generate this type, although I would expect AWS::ECS::Service.LoadBalancer to map to that class instead (and maybe TaskSetLoadBalancer handles the other one?). I pulled the CloudFormation metadata archive and the service descriptions seem to confirm it.

I didn't submit a PR because I'm not sure how to handle a change like this in troposphere, since it has some back-compat implications.

markpeek commented 8 months ago

Thank you for the issue. I see this conflict when the code is generated:

Potential property conflict: ecs LoadBalancer AWS::ECS::Service.LoadBalancer: ['ContainerName', 'ContainerPort', 'LoadBalancerName', 'TargetGroupArn'] AWS::ECS::TaskSet.LoadBalancer: ['ContainerName', 'ContainerPort', 'TargetGroupArn']

I'll have to decide whether to delete TaskSet.LoadBalancer to prefer Service.LoadBalancer or perhaps rename the TaskSet one.

markpeek commented 7 months ago

This has been released in 4.5.3.