bazaarvoice / cloudformation-ruby-dsl

Ruby DSL for creating Cloudformation templates
Apache License 2.0
210 stars 76 forks source link

build_nested_map Depth Limit and Concatenation #72

Closed iAnomaly closed 8 years ago

iAnomaly commented 8 years ago

Hi @jonaf,

I have a some questions about your intended use of _build_nestedmap before I propose a change:

logical_id     region        zone  env          physical_id
GlobalSubnetA  eu-west-1     a     prod         subnet-abc123
GlobalSubnetB  eu-west-1     b     prod         subnet-def456
GlobalSubnetC  eu-west-1     c     prod         subnet-ghi789
  subnets = Table.load 'maps/subnet-ids.txt'
  mapping 'AZsByRegion',
    subnets.get_multimap({}, :region, :env, :region, :zone)
    "AZsByRegion": {
      "eu-west-1": {
        "prod": [
          "eu-west-1a",
          "eu-west-1b",
          "eu-west-1c"
        ]
      },

Thoughts?

Thank you, Cameron

jonaf commented 8 years ago

You're right; CloudFormation has a limit on the depth of maps. I don't think we need to add this enforcement to the Ruby DSL, though; won't CloudFormation reject the template? It should even fail validation. However, if CloudFormation makes a change in the future to permit greater map depth, we would have to update again to reflect that. I don't think it's harmful to leave as-is.

I think it may make sense to add syntactic sugar to maps such that all the "extra" values get concatenated, but it should be a new method entirely. Besides, you may have a use-case for multimaps besides generating Mappings.

You could do something like subnets.get_multimap_flattened({}, 3, :region, :env, :region, :zone) where the method starts mapping up until the input limit (3), at which point it switches over to concatenation for the remaining values. It is a little odd to do this, though, especially because the method name is contradictory (a flattened multimap makes no sense).

It would be nice if we had some composition, instead. flatten_at_depth(3, subnets.get_multimap({}, :region, :env, :region, :zone)) for example, could descend to the third level of the multimap, and then flatten everything under it into one value. I could see this being useful as a general utility for other purposes as well. What do you think about some form of composition to do this, which does not enforce any constraint, but is flexible enough to provide syntactic sugar to do what you want. Later, if CloudFormation permits greater depth, you can simply change from 3->4 in your call and deal with the map as such.

temujin9 commented 8 years ago

Good ideas, no follow-up in several months. Closing this for now, but we will gladly consider a pull request in the form @jonaf suggested, if you find you still need it.