hashicorp/terraform — pull request #541 by svanharmelen (2014-11-04 15:55:40+00:00)
Title: Partial proof of concept - Needs discussion!

The short story:
--------------------
I believe we need a better, more fine grained and intelligent, way to diff sets.

The long story:
-------------------
As we are trying to use TF to manage our firewalls (giving us a way to do desired state firewalling) we tend to have fairly big (100+) sets (of firewall rules in this case) in our TF config. The current way a set is diffed is (as you of course know) by squashing the whole set into line items that contain an index (starting from zero) to separate the items. This gives a nice clean view of your plan when first creating the set, but it becomes pretty nasty when you update such a large set.

The examples are simplified for the purpose of this discussion...
```
CREATE TWO NEW RULES:
+ firewall_x.test
rule.#: "" => "2"
rule.0.ports.#: "" => "2"
rule.0.ports.0: "" => "79"
rule.0.ports.1: "" => "81"
rule.0.protocol: "" => "tcp"
rule.0.source_cidr: "" => "172.16.100.0/24"
rule.1.ports.#: "" => "3"
rule.1.ports.0: "" => "80"
rule.1.ports.1: "" => "8080"
rule.1.ports.2: "" => "1000-2000"
rule.1.protocol: "" => "tcp"
rule.1.source_cidr: "" => "10.0.0.0/24"
```

Say you delete 1 rule from your set, if this turns out to be the rule with the smallest hash code (so rule.0.\* in your state) it will mean that your plan will show all remaining 99 rules as changed resulting a al whole lot of messy text on your screen.

```
DELETE ONLY THE FIRST RULE LOOKS LIKE THIS:
~ firewall_x.test
rule.#: "2" => "1"
rule.0.ports.#: "2" => "3"
rule.0.ports.0: "79" => "80"
rule.0.ports.1: "81" => "8080"
rule.0.ports.2: "" => "1000-2000"
rule.0.source_cidr: "172.16.100.0/24" => "10.0.0.0/24"
rule.1.ports.#: "3" => ""
rule.1.ports.0: "80" => ""
rule.1.ports.1: "8080" => ""
rule.1.ports.2: "1000-2000" => ""
rule.1.protocol: "tcp" => ""
rule.1.source_cidr: "10.0.0.0/24" => ""
```

Additionally if you happend to have a TypeMap with computed values in your set (like we have in order to save UUID's of separate parts in a rule), the content of the map will be merged from one rule to the other when rules shift their positions. This is of course a separate bug that could/should be solved on it's own, but it would be solved automatically when changing the way the items in a set are diffed.

```
SHOW CONFIG BEFORE:
firewall_x.test:
rule.# = 2
rule.0.ports.# = 2
rule.0.ports.0 = 79
rule.0.ports.1 = 81
rule.0.protocol = tcp
rule.0.source_cidr = 172.16.100.0/24
rule.0.uuids.79 = 0ba807f4-df9a-4a61-95df-bda5b332dfe7
rule.0.uuids.80 = 3fee0f1c-f759-41c3-bdeb-af0356edf981
rule.1.ports.# = 3
rule.1.ports.0 = 80
rule.1.ports.1 = 8080
rule.1.ports.2 = 1000-2000
rule.1.protocol = tcp
rule.1.source_cidr = 10.0.0.0/24
rule.1.uuids.1000-2000 = c5891eb6-394f-481a-b86c-936f7ac68cca
rule.1.uuids.80 = c69aa8e5-cb99-44a2-b441-10378aa20074
rule.1.uuids.8080 = e455cd08-9381-47e1-bf52-0035e96db786

SHOW CONFIG AFTER DELETING THE FIRST RULE:
firewall_x.test:
rule.# = 1
rule.0.ports.# = 3
rule.0.ports.0 = 80
rule.0.ports.1 = 8080
rule.0.ports.2 = 1000-2000
rule.0.protocol = tcp
rule.0.source_cidr = 10.0.0.0/24
rule.0.uuids.79 = 0ba807f4-df9a-4a61-95df-bda5b332dfe7 <= This should be here
rule.0.uuids.80 = 3fee0f1c-f759-41c3-bdeb-af0356edf981 <= This has the UUID of the previous rule
rule.0.uuids.1000-2000 = c5891eb6-394f-481a-b86c-936f7ac68cca
rule.0.uuids.8080 = e455cd08-9381-47e1-bf52-0035e96db786
```

What would make way more sense (IMHO) would be to do the diffing of set items based on the complete items. In theory not a difficult thing to do, but since everything in TF is build around/on top of this line based instead of object based diff model, it turns out that in order to do this right you actually need to refactor quite a lot of functions/code of the core of TF. So after giving it a few tries with different approaches I thought I could better open a discussion first to discuss the right way to do this. As an example I have a partial working example. This does the diffing based on complete items, but it not yet updates the state accordingly.

```
DELETING THE FIRST RULE NOW SHOWS THIS:
~ firewall_x.test
rule.#: "2" => "1"
rule.0.ports.#: "2" => "0"
rule.0.ports.0: "79" => ""
rule.0.ports.1: "81" => ""
rule.0.protocol: "tcp" => ""
rule.0.source_cidr: "172.16.100.0/24" => ""
```

So... How do you think about this? Would a change like this be acceptable/wanted? Or do we need to think about other possible solutions?

I also started with an approach to change from using the index based (like rule.0.\*, rule.1.\*) formatting to use the actual hash code (like rule.46278389.\*, rule.78986245.\*). This could maybe be a better approach, but also for this solution it means having to change quite a lot of existing logic.

So I hope to get some good discussions from this PR in order to get to a solution about how to deal with diffing sets. And also hopefully learn/understand why the choice was made to do diffing like this instead of diffing the types as types (so diff the map as a map and not as a bunch of squashed separate lines items).

Thx!