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

helper/schema: diffs for sets should include the full set [GH-457] 

Prior to this, the diff only contained changed set elements. The issue
with this is that `getSet`, the internal function that reads a set from
the ResourceData, expects that each level (state, config, diff, etc.)
has the _full set_ information. This change was done to fix merging
issues.

Because of this, we need to make sure the full set is visible in the
diff.
This commit is contained in:
Mitchell Hashimoto 2014-10-21 10:49:27 -07:00
parent 85a57ab792
commit f63a5d24e9
6 changed files with 106 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
CHANGELOG.md
View File

@ -42,6 +42,8 @@ BUG FIXES:
* providers/aws: Disassociate EIP before destroying.
* providers/aws: ELB treats subnets as a set.
* providers/aws: Fix case where in a destroy/create tags weren't reapplied. [GH-464]
* providers/aws: Fix incorrect/erroneous apply cases around security group
rules. [GH-457]
* providers/consul: Fix regression where `key` param changed to `keys. [GH-475]
## 0.3.0 (October 14, 2014)

55
builtin/providers/aws/resource_aws_security_group.go
View File

@ -66,14 +66,18 @@ func resourceAwsSecurityGroup() *schema.Resource {
},
"security_groups": &schema.Schema{
Type: schema.TypeList,
Type: schema.TypeSet,
Optional: true,
Elem: &schema.Schema{Type: schema.TypeString},
Set: func(v interface{}) int {
return hashcode.String(v.(string))
},
},
"self": &schema.Schema{
Type: schema.TypeBool,
Optional: true,
Default: false,
},
},
},
@ -112,7 +116,7 @@ func resourceAwsSecurityGroupIngressHash(v interface{}) int {
}
}
if v, ok := m["security_groups"]; ok {
vs := v.([]interface{})
vs := v.(*schema.Set).List()
s := make([]string, len(vs))
for i, raw := range vs {
s[i] = raw.(string)
@ -283,15 +287,28 @@ func resourceAwsSecurityGroupRead(d *schema.ResourceData, meta interface{}) erro
sg := sgRaw.(*ec2.SecurityGroupInfo)
// Gather our ingress rules
ingressRules := make([]map[string]interface{}, len(sg.IPPerms))
for i, perm := range sg.IPPerms {
n := make(map[string]interface{})
n["from_port"] = perm.FromPort
n["protocol"] = perm.Protocol
n["to_port"] = perm.ToPort
ingressMap := make(map[string]map[string]interface{})
for _, perm := range sg.IPPerms {
k := fmt.Sprintf("%s-%d-%d", perm.Protocol, perm.FromPort, perm.ToPort)
m, ok := ingressMap[k]
if !ok {
m = make(map[string]interface{})
ingressMap[k] = m
}
m["from_port"] = perm.FromPort
m["to_port"] = perm.ToPort
m["protocol"] = perm.Protocol
if len(perm.SourceIPs) > 0 {
n["cidr_blocks"] = perm.SourceIPs
raw, ok := m["cidr_blocks"]
if !ok {
raw = make([]string, 0, len(perm.SourceIPs))
}
list := raw.([]string)
list = append(list, perm.SourceIPs...)
m["cidr_blocks"] = list
}
var groups []string
@ -301,14 +318,24 @@ func resourceAwsSecurityGroupRead(d *schema.ResourceData, meta interface{}) erro
for i, id := range groups {
if id == d.Id() {
groups[i], groups = groups[len(groups)-1], groups[:len(groups)-1]
n["self"] = true
m["self"] = true
}
}
if len(groups) > 0 {
n["security_groups"] = groups
}
ingressRules[i] = n
if len(groups) > 0 {
raw, ok := m["security_groups"]
if !ok {
raw = make([]string, 0, len(groups))
}
list := raw.([]string)
list = append(list, groups...)
m["security_groups"] = list
}
}
ingressRules := make([]map[string]interface{}, 0, len(ingressMap))
for _, m := range ingressMap {
ingressRules = append(ingressRules, m)
}
d.Set("description", sg.Description)

3
builtin/providers/aws/structure.go
View File

@ -3,6 +3,7 @@ package aws
import (
"strings"
"github.com/hashicorp/terraform/helper/schema"
"github.com/mitchellh/goamz/ec2"
"github.com/mitchellh/goamz/elb"
)
@ -48,7 +49,7 @@ func expandIPPerms(id string, configured []interface{}) []ec2.IPPerm {
var groups []string
if raw, ok := m["security_groups"]; ok {
list := raw.([]interface{})
list := raw.(*schema.Set).List()
for _, v := range list {
groups = append(groups, v.(string))
}

12
helper/schema/resource_data.go
View File

@ -2,6 +2,7 @@ package schema
import (
"fmt"
"log"
"reflect"
"strconv"
"strings"
@ -647,6 +648,17 @@ func (d *ResourceData) getPrimitive(
}
}
if strings.HasSuffix(k, "protocol") && result == "" {
log.Printf("------------------------------------------------")
log.Printf("KEY: %s", k)
log.Printf("LEVEL: %#v", source)
log.Printf("DIFF: %#v", diff)
log.Printf("EXACT: %#v", exact)
log.Printf("Config: %#v\n\n", d.config)
log.Printf("DIFF: %#v\n\n", d.diff)
log.Printf("State: %#v\n\n", d.state)
}
if !exact || source == getSourceSet {
if d.setMap != nil && source >= getSourceSet {
if v, ok := d.setMap[k]; ok {

60
helper/schema/schema.go
View File

@ -205,7 +205,7 @@ func (m schemaMap) Diff(
}
for k, schema := range m {
err := m.diff(k, schema, result, d)
err := m.diff(k, schema, result, d, false)
if err != nil {
return nil, err
}
@ -225,7 +225,7 @@ func (m schemaMap) Diff(
// Perform the diff again
for k, schema := range m {
err := m.diff(k, schema, result2, d)
err := m.diff(k, schema, result2, d, false)
if err != nil {
return nil, err
}
@ -427,7 +427,8 @@ func (m schemaMap) diff(
k string,
schema *Schema,
diff *terraform.InstanceDiff,
d *ResourceData) error {
d *ResourceData,
all bool) error {
var err error
switch schema.Type {
case TypeBool:
@ -435,13 +436,13 @@ func (m schemaMap) diff(
case TypeInt:
fallthrough
case TypeString:
err = m.diffString(k, schema, diff, d)
err = m.diffString(k, schema, diff, d, all)
case TypeList:
err = m.diffList(k, schema, diff, d)
err = m.diffList(k, schema, diff, d, all)
case TypeMap:
err = m.diffMap(k, schema, diff, d)
err = m.diffMap(k, schema, diff, d, all)
case TypeSet:
err = m.diffSet(k, schema, diff, d)
err = m.diffSet(k, schema, diff, d, all)
default:
err = fmt.Errorf("%s: unknown type %#v", k, schema.Type)
}
@ -453,7 +454,8 @@ func (m schemaMap) diffList(
k string,
schema *Schema,
diff *terraform.InstanceDiff,
d *ResourceData) error {
d *ResourceData,
all bool) error {
o, n, _, computedList := d.diffChange(k)
nSet := n != nil
@ -481,7 +483,7 @@ func (m schemaMap) diffList(
// If the new value was set, and the two are equal, then we're done.
// We have to do this check here because sets might be NOT
// reflect.DeepEqual so we need to wait until we get the []interface{}
if nSet && reflect.DeepEqual(os, vs) {
if !all && nSet && reflect.DeepEqual(os, vs) {
return nil
}
@ -502,7 +504,7 @@ func (m schemaMap) diffList(
// If the counts are not the same, then record that diff
changed := oldLen != newLen
computed := oldLen == 0 && newLen == 0 && schema.Computed
if changed || computed {
if changed || computed || all {
countSchema := &Schema{
Type: TypeInt,
Computed: schema.Computed,
@ -539,7 +541,7 @@ func (m schemaMap) diffList(
// just diff each.
for i := 0; i < maxLen; i++ {
subK := fmt.Sprintf("%s.%d", k, i)
err := m.diff(subK, &t2, diff, d)
err := m.diff(subK, &t2, diff, d, all)
if err != nil {
return err
}
@ -549,7 +551,7 @@ func (m schemaMap) diffList(
for i := 0; i < maxLen; i++ {
for k2, schema := range t.Schema {
subK := fmt.Sprintf("%s.%d.%s", k, i, k2)
err := m.diff(subK, schema, diff, d)
err := m.diff(subK, schema, diff, d, all)
if err != nil {
return err
}
@ -566,8 +568,8 @@ func (m schemaMap) diffMap(
k string,
schema *Schema,
diff *terraform.InstanceDiff,
d *ResourceData) error {
//elemSchema := &Schema{Type: TypeString}
d *ResourceData,
all bool) error {
prefix := k + "."
// First get all the values from the state
@ -590,7 +592,7 @@ func (m schemaMap) diffMap(
old := stateMap[k]
delete(stateMap, k)
if old == v {
if old == v && !all {
continue
}
@ -613,15 +615,35 @@ func (m schemaMap) diffSet(
k string,
schema *Schema,
diff *terraform.InstanceDiff,
d *ResourceData) error {
return m.diffList(k, schema, diff, d)
d *ResourceData,
all bool) error {
if !all {
// This is a bit strange, but we expect the entire set to be in the diff,
// so we first diff the set normally but with a new diff. Then, if
// there IS any change, we just set the change to the entire list.
tempD := new(terraform.InstanceDiff)
tempD.Attributes = make(map[string]*terraform.ResourceAttrDiff)
if err := m.diffList(k, schema, tempD, d, false); err != nil {
return err
}
// If we had no changes, then we're done
if tempD.Empty() {
return nil
}
}
// We have changes, so re-run the diff, but set a flag to force
// getting all diffs, even if there is no change.
return m.diffList(k, schema, diff, d, true)
}
func (m schemaMap) diffString(
k string,
schema *Schema,
diff *terraform.InstanceDiff,
d *ResourceData) error {
d *ResourceData,
all bool) error {
var originalN interface{}
var os, ns string
o, n, _, _ := d.diffChange(k)
@ -646,7 +668,7 @@ func (m schemaMap) diffString(
return fmt.Errorf("%s: %s", k, err)
}
if os == ns {
if os == ns && !all {
// They're the same value. If there old value is not blank or we
// have an ID, then return right away since we're already setup.
if os != "" || d.Id() != "" {

8
helper/schema/schema_test.go
View File

@ -865,6 +865,14 @@ func TestSchemaMap_Diff(t *testing.T) {
Old: "2",
New: "3",
},
"ports.0": &terraform.ResourceAttrDiff{
Old: "1",
New: "1",
},
"ports.1": &terraform.ResourceAttrDiff{
Old: "2",
New: "2",
},
"ports.2": &terraform.ResourceAttrDiff{
Old: "",
New: "5",