resilien
/
terraform
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

core: Use .% instead of .# for maps in state 

The flatmapped representation of state prior to this commit encoded maps
and lists (and therefore by extension, sets) with a key corresponding to
the number of elements, or the unknown variable indicator under a .# key
and then individual items. For example, the list ["a", "b", "c"] would
have been encoded as:

    listname.# = 3
    listname.0 = "a"
    listname.1 = "b"
    listname.2 = "c"

And the map {"key1": "value1", "key2", "value2"} would have been encoded
as:

    mapname.# = 2
    mapname.key1 = "value1"
    mapname.key2 = "value2"

Sets use the hash code as the key - for example a set with a (fictional)
hashcode calculation may look like:

    setname.# = 2
    setname.12312512 = "value1"
    setname.56345233 = "value2"

Prior to the work done to extend the type system, this was sufficient
since the internal representation of these was effectively the same.
However, following the separation of maps and lists into distinct
first-class types, this encoding presents a problem: given a state file,
it is impossible to tell the encoding of an empty list and an empty map
apart. This presents problems for the type checker during interpolation,
as many interpolation functions will operate on only one of these two
structures.

This commit therefore changes the representation in state of maps to use
a "%" as the key for the number of elements. Consequently the map above
will now be encoded as:

    mapname.% = 2
    mapname.key1 = "value1"
    mapname.key2 = "value2"

This has the effect of an empty list (or set) now being encoded as:

    listname.# = 0

And an empty map now being encoded as:

    mapname.% = 0

Therefore we can eliminate some nasty guessing logic from the resource
variable supplier for interpolation, at the cost of having to migrate
state up front (to follow in a subsequent commit).

In order to reduce the number of potential situations in which resources
would be "forced new", we continue to accept "#" as the count key when
reading maps via helper/schema. There is no situation under which we can
allow "#" as an actual map key in any case, as it would not be
distinguishable from a list or set in state.
This commit is contained in:
James Nugent 2016-06-05 09:34:43 +01:00
parent cb9ef298f3
commit 074545e536
12 changed files with 88 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
helper/schema/field_reader_diff.go
View File

@ -76,7 +76,7 @@ func (r *DiffFieldReader) readMap(
if !strings.HasPrefix(k, prefix) {
continue
}
if strings.HasPrefix(k, prefix+"#") {
if strings.HasPrefix(k, prefix+"%") {
// Ignore the count field
continue
}

4
helper/schema/field_reader_diff_test.go
View File

@ -22,7 +22,7 @@ func TestDiffFieldReader_MapHandling(t *testing.T) {
Schema: schema,
Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{
"tags.#": &terraform.ResourceAttrDiff{
"tags.%": &terraform.ResourceAttrDiff{
Old: "1",
New: "2",
},
@ -35,7 +35,7 @@ func TestDiffFieldReader_MapHandling(t *testing.T) {
Source: &MapFieldReader{
Schema: schema,
Map: BasicMapReader(map[string]string{
"tags.#": "1",
"tags.%": "1",
"tags.foo": "bar",
}),
},

2
helper/schema/field_reader_map.go
View File

@ -53,7 +53,7 @@ func (r *MapFieldReader) readMap(k string) (FieldReadResult, error) {
resultSet = true
key := k[len(prefix):]
if key != "#" {
if key != "%" && key != "#" {
result[key] = v
}
}

4
helper/schema/field_reader_map_test.go
View File

@ -28,7 +28,7 @@ func TestMapFieldReader(t *testing.T) {
"listInt.0": "21",
"listInt.1": "42",
"map.#": "2",
"map.%": "2",
"map.foo": "bar",
"map.bar": "baz",
@ -56,7 +56,7 @@ func TestMapFieldReader_extra(t *testing.T) {
Map: BasicMapReader(map[string]string{
"mapDel": "",
"mapEmpty.#": "0",
"mapEmpty.%": "0",
}),
}

2
helper/schema/field_writer_map.go
View File

@ -165,7 +165,7 @@ func (w *MapFieldWriter) setMap(
}
// Set the count
w.result[k+".#"] = strconv.Itoa(len(vs))
w.result[k+".%"] = strconv.Itoa(len(vs))
return nil
}

2
helper/schema/field_writer_map_test.go
View File

@ -161,7 +161,7 @@ func TestMapFieldWriter(t *testing.T) {
map[string]interface{}{"foo": "bar"},
false,
map[string]string{
"map.#": "1",
"map.%": "1",
"map.foo": "bar",
},
},

20
helper/schema/resource_data_test.go
View File

@ -1987,10 +1987,10 @@ func TestResourceDataState(t *testing.T) {
State: &terraform.InstanceState{
Attributes: map[string]string{
"config_vars.#": "2",
"config_vars.0.#": "2",
"config_vars.0.%": "2",
"config_vars.0.foo": "bar",
"config_vars.0.bar": "bar",
"config_vars.1.#": "1",
"config_vars.1.%": "1",
"config_vars.1.bar": "baz",
},
},
@ -2017,9 +2017,9 @@ func TestResourceDataState(t *testing.T) {
Result: &terraform.InstanceState{
Attributes: map[string]string{
"config_vars.#": "2",
"config_vars.0.#": "1",
"config_vars.0.%": "1",
"config_vars.0.foo": "bar",
"config_vars.1.#": "1",
"config_vars.1.%": "1",
"config_vars.1.baz": "bang",
},
},
@ -2444,10 +2444,10 @@ func TestResourceDataState(t *testing.T) {
Attributes: map[string]string{
// TODO: broken, shouldn't bar be removed?
"config_vars.#": "2",
"config_vars.0.#": "2",
"config_vars.0.%": "2",
"config_vars.0.foo": "bar",
"config_vars.0.bar": "bar",
"config_vars.1.#": "1",
"config_vars.1.%": "1",
"config_vars.1.bar": "baz",
},
},
@ -2551,7 +2551,7 @@ func TestResourceDataState(t *testing.T) {
Result: &terraform.InstanceState{
Attributes: map[string]string{
"tags.#": "1",
"tags.%": "1",
"tags.Name": "foo",
},
},
@ -2584,7 +2584,7 @@ func TestResourceDataState(t *testing.T) {
Result: &terraform.InstanceState{
Attributes: map[string]string{
"tags.#": "0",
"tags.%": "0",
},
},
},
@ -2690,7 +2690,7 @@ func TestResourceDataState(t *testing.T) {
Attributes: map[string]string{
"ports.#": "1",
"ports.10.index": "10",
"ports.10.uuids.#": "1",
"ports.10.uuids.%": "1",
"ports.10.uuids.80": "value",
},
},
@ -2831,7 +2831,7 @@ func TestResourceDataState(t *testing.T) {
Attributes: map[string]string{
"ports.#": "1",
"ports.0.index": "10",
"ports.0.uuids.#": "1",
"ports.0.uuids.%": "1",
"ports.0.uuids.80": "value",
},
},

2
helper/schema/resource_test.go
View File

@ -162,7 +162,7 @@ func TestResourceApply_destroyCreate(t *testing.T) {
Attributes: map[string]string{
"id": "foo",
"foo": "42",
"tags.#": "1",
"tags.%": "1",
"tags.Name": "foo",
},
}

6
helper/schema/schema.go
View File

@ -760,8 +760,8 @@ func (m schemaMap) diffMap(
stateExists := o != nil
// Delete any count values, since we don't use those
delete(configMap, "#")
delete(stateMap, "#")
delete(configMap, "%")
delete(stateMap, "%")
// Check if the number of elements has changed.
oldLen, newLen := len(stateMap), len(configMap)
@ -795,7 +795,7 @@ func (m schemaMap) diffMap(
oldStr = ""
}
diff.Attributes[k+".#"] = countSchema.finalizeDiff(
diff.Attributes[k+".%"] = countSchema.finalizeDiff(
&terraform.ResourceAttrDiff{
Old: oldStr,
New: newStr,

12
helper/schema/schema_test.go
View File

@ -1297,7 +1297,7 @@ func TestSchemaMap_Diff(t *testing.T) {
Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{
"config_vars.#": &terraform.ResourceAttrDiff{
"config_vars.%": &terraform.ResourceAttrDiff{
Old: "0",
New: "1",
},
@ -1473,7 +1473,7 @@ func TestSchemaMap_Diff(t *testing.T) {
Old: "1",
New: "0",
},
"config_vars.0.#": &terraform.ResourceAttrDiff{
"config_vars.0.%": &terraform.ResourceAttrDiff{
Old: "2",
New: "0",
},
@ -1763,7 +1763,7 @@ func TestSchemaMap_Diff(t *testing.T) {
Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{
"vars.#": &terraform.ResourceAttrDiff{
"vars.%": &terraform.ResourceAttrDiff{
Old: "",
NewComputed: true,
},
@ -1783,7 +1783,7 @@ func TestSchemaMap_Diff(t *testing.T) {
State: &terraform.InstanceState{
Attributes: map[string]string{
"vars.#": "0",
"vars.%": "0",
},
},
@ -1799,7 +1799,7 @@ func TestSchemaMap_Diff(t *testing.T) {
Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{
"vars.#": &terraform.ResourceAttrDiff{
"vars.%": &terraform.ResourceAttrDiff{
Old: "",
NewComputed: true,
},
@ -2046,7 +2046,7 @@ func TestSchemaMap_Diff(t *testing.T) {
Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{
"vars.#": &terraform.ResourceAttrDiff{
"vars.%": &terraform.ResourceAttrDiff{
Old: "0",
New: "1",
},

89
terraform/interpolate.go
View File

@ -353,6 +353,10 @@ func (i *Interpolater) computeResourceVariable(
unknownVariable := unknownVariable()
// These variables must be declared early because of the use of GOTO
var isList bool
var isMap bool
// Get the information about this resource variable, and verify
// that it exists and such.
module, _, err := i.resourceVariableInfo(scope, v)
@ -388,7 +392,9 @@ func (i *Interpolater) computeResourceVariable(
}
// computed list or map attribute
if _, ok := r.Primary.Attributes[v.Field+".#"]; ok {
_, isList = r.Primary.Attributes[v.Field+".#"]
_, isMap = r.Primary.Attributes[v.Field+".%"]
if isList || isMap {
variable, err := i.interpolateComplexTypeAttribute(v.Field, r.Primary.Attributes)
return &variable, err
}
@ -566,49 +572,70 @@ func (i *Interpolater) interpolateComplexTypeAttribute(
resourceID string,
attributes map[string]string) (ast.Variable, error) {
attr := attributes[resourceID+".#"]
log.Printf("[DEBUG] Interpolating computed complex type attribute %s (%s)",
resourceID, attr)
// We can now distinguish between lists and maps in state by the count field:
// - lists (and by extension, sets) use the traditional .# notation
// - maps use the newer .% notation
// Consequently here we can decide how to deal with the keys appropriately
// based on whether the type is a map of list.
if lengthAttr, isList := attributes[resourceID+".#"]; isList {
log.Printf("[DEBUG] Interpolating computed list element attribute %s (%s)",
resourceID, lengthAttr)
// In Terraform's internal dotted representation of list-like attributes, the
// ".#" count field is marked as unknown to indicate "this whole list is
// unknown". We must honor that meaning here so computed references can be
// treated properly during the plan phase.
if attr == config.UnknownVariableValue {
return unknownVariable(), nil
}
// At this stage we don't know whether the item is a list or a map, so we
// examine the keys to see whether they are all numeric.
var numericKeys []string
var allKeys []string
numberedListKey := regexp.MustCompile("^" + resourceID + "\\.[0-9]+$")
otherListKey := regexp.MustCompile("^" + resourceID + "\\.([^#]+)$")
for id, _ := range attributes {
if numberedListKey.MatchString(id) {
numericKeys = append(numericKeys, id)
// In Terraform's internal dotted representation of list-like attributes, the
// ".#" count field is marked as unknown to indicate "this whole list is
// unknown". We must honor that meaning here so computed references can be
// treated properly during the plan phase.
if lengthAttr == config.UnknownVariableValue {
return unknownVariable(), nil
}
var keys []string
listElementKey := regexp.MustCompile("^" + resourceID + "\\.[0-9]+$")
for id, _ := range attributes {
if listElementKey.MatchString(id) {
keys = append(keys, id)
}
}
if submatches := otherListKey.FindAllStringSubmatch(id, -1); len(submatches) > 0 {
allKeys = append(allKeys, submatches[0][1])
}
}
if len(numericKeys) == len(allKeys) {
// This is a list
var members []string
for _, key := range numericKeys {
for _, key := range keys {
members = append(members, attributes[key])
}
// This behaviour still seems very broken to me... it retains BC but is
// probably going to cause problems in future
sort.Strings(members)
return hil.InterfaceToVariable(members)
} else {
// This is a map
}
if lengthAttr, isMap := attributes[resourceID+".%"]; isMap {
log.Printf("[DEBUG] Interpolating computed map element attribute %s (%s)",
resourceID, lengthAttr)
// In Terraform's internal dotted representation of map attributes, the
// ".%" count field is marked as unknown to indicate "this whole list is
// unknown". We must honor that meaning here so computed references can be
// treated properly during the plan phase.
if lengthAttr == config.UnknownVariableValue {
return unknownVariable(), nil
}
var keys []string
mapElementKey := regexp.MustCompile("^" + resourceID + "\\.([^%]+)$")
for id, _ := range attributes {
if submatches := mapElementKey.FindAllStringSubmatch(id, -1); len(submatches) > 0 {
keys = append(keys, submatches[0][1])
}
}
members := make(map[string]interface{})
for _, key := range allKeys {
for _, key := range keys {
members[key] = attributes[resourceID+"."+key]
}
return hil.InterfaceToVariable(members)
}
return ast.Variable{}, fmt.Errorf("No complex type %s found", resourceID)
}
func (i *Interpolater) resourceVariableInfo(

4
terraform/interpolate_test.go
View File

@ -294,7 +294,7 @@ func TestInterpolator_resourceMultiAttributes(t *testing.T) {
"name_servers.3": "ns-601.awsdns-11.net",
"listeners.#": "1",
"listeners.0": "red",
"tags.#": "1",
"tags.%": "1",
"tags.Name": "reindeer",
"nothing.#": "0",
},
@ -529,7 +529,7 @@ func testInterpolate(
}
if !reflect.DeepEqual(actual, expected) {
spew.Config.DisableMethods = true
t.Fatalf("%q: actual: %#v\nexpected: %#v\n\n%s\n\n%s\n\n", n, actual, expected,
t.Fatalf("%q:\n\n actual: %#v\nexpected: %#v\n\n%s\n\n%s\n\n", n, actual, expected,
spew.Sdump(actual), spew.Sdump(expected))
}
}