brabster / crucible

AWS CloudFormation templates built with Clojure
Eclipse Public License 1.0
72 stars 18 forks source link

Add ecs-parameters to AWS::Events::Rule #200

Closed keerts closed 4 years ago

shooit commented 4 years ago

Looks good to me!

keerts commented 4 years ago

I have found one problem with this code that I cannot resolve. Apparently AWS is not very consistent with the name of the AwsVpcConfiguration property. In ecs/service there is an override for the json key: (defmethod ->key :aws-vpc-configuration [_] "AwsvpcConfiguration"). That override also overrides it for Events::Rule, which is not correct.

This is the workaround that I currently use, but it's ugly:

    {"AwsVpcConfiguration"
      {::aws-vpc-configuration/security-groups [(xref :sg-private)]

Any thoughts on this?

shooit commented 4 years ago

I've got nothing quick that would work other than hard coding something in encoding/convert-key to handle this case.

Leaving that key as a string means spec won't check it unless you define a more complicated spec for the resource than the usual s/keys

At a higher level, if the usage of ->key was changed to take fully qualified keywords then this type of problem could be avoided. Although that comes with the complication of having to define the conversion for each fully qualified key with the same name that needs the same conversion. There might be a clever (i.e. macro-y) way around that though.