search

cloudtools / troposphere

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

v3.2.0 - ecs - no AuthorizationConfig AWSProperty #1997

Closed JohnPreston closed 2 years ago

JohnPreston commented 2 years ago

Hi @markpeek

Just updated to 3.2.0 which uses a generated model for ecs. Seems like the class AuthorizationConfig is no longer available.

Why would the generator not have created a AuthorizationConfig AWSProperty class for it ? Could it be because that was already generated for batch ? (It is a common property to both ECS and Batch with the same props it seems).

JohnPreston commented 2 years ago

Also @markpeek , just checking, as per https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/cfn-resource-specification.html, the specifications vary based on the region. How are we ensuring that the generated files are using a region that is always up to date ? I'd imagine us-east-1 always is but who in AWS knows then if that is true ?

markpeek commented 2 years ago

Good catch @JohnPreston. Thanks! I just pushed a fix.

The spec was showing in EFSVolumeConfiguration:

        "AuthorizationConfig": {
          "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ecs-taskdefinition-efsvolumeconfiguration.html#cfn-ecs-taskdefinition-efsvolumeconfiguration-authorizationconfig",
          "UpdateType": "Immutable",
          "Required": false,
          "PrimitiveType": "Json",
          "Type": "AuthorizationConfig"
        }

The generator prefers using PrimitiveType so AuthorizationConfig was not being used (or followed for emitting). The fix removes PrimitiveType via a jsonpatch which causes it to use Type.

To your point about batch, I isolate the resources/properties into services which corresponds to the files being emitted so that should not happen. There are cases of the same Property name being used inside the same service which can cause issues. I'm checking for the same set of keys to determine uniqueness and rely on jsonpatch to rename if necessary.

Good point about the region issue. I've just been going with us-east-1 to be consistent. I'll rely on AWS to have these specs "eventually consistent".

I'll do another release to include this later today in case any other issues come in.

markpeek commented 2 years ago

Fixed in Release 3.2.1 along with two other compatibility regressions.