Merge pull request #480 from hashicorp/f-fix-consul

Consul param should be "key"

builtin/providers/consul/resource_consul_keys.go

@@ -31,7 +31,7 @@ func resourceConsulKeys() *schema.Resource {
 				Optional: true,
 			},
 
-			"keys": &schema.Schema{
+			"key": &schema.Schema{
 				Type:     schema.TypeSet,
 				Optional: true,
 				Elem: &schema.Resource{
@@ -60,6 +60,7 @@ func resourceConsulKeys() *schema.Resource {
 						"delete": &schema.Schema{
 							Type:     schema.TypeBool,
 							Optional: true,
+							Default:  false,
 						},
 					},
 				},
@@ -113,19 +114,15 @@ func resourceConsulKeysCreate(d *schema.ResourceData, meta interface{}) error {
 	vars := make(map[string]string)
 
 	// Extract the keys
-	keys := d.Get("keys").(*schema.Set).List()
+	keys := d.Get("key").(*schema.Set).List()
 	for _, raw := range keys {
 		key, path, sub, err := parse_key(raw)
 		if err != nil {
 			return err
 		}
 
-		if valueRaw, ok := sub["value"]; ok {
-			value, ok := valueRaw.(string)
-			if !ok {
-				return fmt.Errorf("Failed to get value for key '%s'", key)
-			}
-
+		value := sub["value"].(string)
+		if value != "" {
 			log.Printf("[DEBUG] Setting key '%s' to '%v' in %s", path, value, dc)
 			pair := consulapi.KVPair{Key: path, Value: []byte(value)}
 			if _, err := kv.Put(&pair, &wOpts); err != nil {
@@ -142,14 +139,13 @@ func resourceConsulKeysCreate(d *schema.ResourceData, meta interface{}) error {
 			}
 			value := attribute_value(sub, key, pair)
 			vars[key] = value
-			sub["value"] = value
 		}
 	}
 
 	// Update the resource
 	d.SetId("consul")
 	d.Set("datacenter", dc)
-	d.Set("keys", keys)
+	d.Set("key", keys)
 	d.Set("var", vars)
 	return nil
 }
@@ -178,7 +174,7 @@ func resourceConsulKeysRead(d *schema.ResourceData, meta interface{}) error {
 	vars := make(map[string]string)
 
 	// Extract the keys
-	keys := d.Get("keys").(*schema.Set).List()
+	keys := d.Get("key").(*schema.Set).List()
 	for _, raw := range keys {
 		key, path, sub, err := parse_key(raw)
 		if err != nil {
@@ -197,7 +193,7 @@ func resourceConsulKeysRead(d *schema.ResourceData, meta interface{}) error {
 	}
 
 	// Update the resource
-	d.Set("keys", keys)
+	d.Set("key", keys)
 	d.Set("var", vars)
 	return nil
 }
@@ -223,7 +219,7 @@ func resourceConsulKeysDelete(d *schema.ResourceData, meta interface{}) error {
 	wOpts := consulapi.WriteOptions{Datacenter: dc, Token: token}
 
 	// Extract the keys
-	keys := d.Get("keys").(*schema.Set).List()
+	keys := d.Get("key").(*schema.Set).List()
 	for _, raw := range keys {
 		_, path, sub, err := parse_key(raw)
 		if err != nil {

builtin/providers/consul/resource_consul_keys_test.go

@@ -30,7 +30,7 @@ func TestAccConsulKeys(t *testing.T) {
 
 func testAccCheckConsulKeysDestroy(s *terraform.State) error {
 	kv := testAccProvider.Meta().(*consulapi.Client).KV()
-	opts := &consulapi.QueryOptions{Datacenter: "nyc1"}
+	opts := &consulapi.QueryOptions{Datacenter: "nyc3"}
 	pair, _, err := kv.Get("test/set", opts)
 	if err != nil {
 		return err
@@ -44,7 +44,7 @@ func testAccCheckConsulKeysDestroy(s *terraform.State) error {
 func testAccCheckConsulKeysExists() resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		kv := testAccProvider.Meta().(*consulapi.Client).KV()
-		opts := &consulapi.QueryOptions{Datacenter: "nyc1"}
+		opts := &consulapi.QueryOptions{Datacenter: "nyc3"}
 		pair, _, err := kv.Get("test/set", opts)
 		if err != nil {
 			return err
@@ -78,7 +78,7 @@ func testAccCheckConsulKeysValue(n, attr, val string) resource.TestCheckFunc {
 
 const testAccConsulKeysConfig = `
 resource "consul_keys" "app" {
-	datacenter = "nyc1"
+	datacenter = "nyc3"
 	key {
 		name = "time"
 		path = "global/time"

builtin/providers/consul/resource_provider_test.go

@@ -3,6 +3,7 @@ package consul
 import (
 	"testing"
 
+	"github.com/armon/consul-api"
 	"github.com/hashicorp/terraform/config"
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/terraform"
@@ -16,6 +17,13 @@ func init() {
 	testAccProviders = map[string]terraform.ResourceProvider{
 		"consul": testAccProvider,
 	}
+
+	// Use the demo address for the acceptance tests
+	testAccProvider.ConfigureFunc = func(d *schema.ResourceData) (interface{}, error) {
+		conf := consulapi.DefaultConfig()
+		conf.Address = "demo.consul.io:80"
+		return consulapi.NewClient(conf)
+	}
 }
 
 func TestResourceProvider(t *testing.T) {
@@ -33,7 +41,7 @@ func TestResourceProvider_Configure(t *testing.T) {
 
 	raw := map[string]interface{}{
 		"address":    "demo.consul.io:80",
-		"datacenter": "nyc1",
+		"datacenter": "nyc3",
 	}
 
 	rawConfig, err := config.NewRawConfig(raw)

helper/schema/resource_data.go

@@ -43,10 +43,11 @@ type getSource byte
 const (
 	getSourceState getSource = 1 << iota
 	getSourceConfig
-	getSourceDiff
 	getSourceSet
-	getSourceExact
-	getSourceMax = getSourceSet
+	getSourceExact               // Only get from the _exact_ level
+	getSourceDiff                // Apply the diff on top our level
+	getSourceLevelMask getSource = getSourceState | getSourceConfig | getSourceSet
+	getSourceMax       getSource = getSourceSet
 )
 
 // getResult is the internal structure that is generated when a Get
@@ -82,7 +83,7 @@ func (d *ResourceData) Get(key string) interface{} {
 // set and the new value is. This is common, for example, for boolean
 // fields which have a zero value of false.
 func (d *ResourceData) GetChange(key string) (interface{}, interface{}) {
-	o, n := d.getChange(key, getSourceConfig, getSourceDiff)
+	o, n := d.getChange(key, getSourceConfig, getSourceConfig|getSourceDiff)
 	return o.Value, n.Value
 }
 
@@ -93,7 +94,7 @@ func (d *ResourceData) GetChange(key string) (interface{}, interface{}) {
 // The first result will not necessarilly be nil if the value doesn't exist.
 // The second result should be checked to determine this information.
 func (d *ResourceData) GetOk(key string) (interface{}, bool) {
-	r := d.getRaw(key, getSourceSet)
+	r := d.getRaw(key, getSourceSet|getSourceDiff)
 	return r.Value, r.Exists
 }
 
@@ -289,12 +290,21 @@ func (d *ResourceData) getSet(
 	// entire set must come from set, diff, state, etc. So we go backwards
 	// and once we get a result, we take it. Or, we never get a result.
 	var raw getResult
-	for listSource := source; listSource > 0; listSource >>= 1 {
-		if source&getSourceExact != 0 && listSource != source {
+	sourceLevel := source & getSourceLevelMask
+	sourceFlags := source & ^getSourceLevelMask
+	for listSource := sourceLevel; listSource > 0; listSource >>= 1 {
+		// If we're already asking for an exact source and it doesn't
+		// match, then leave since the original source was the match.
+		if sourceFlags&getSourceExact != 0 && listSource != sourceLevel {
 			break
 		}
 
-		raw = d.getList(k, nil, schema, listSource|getSourceExact)
+		// The source we get from is the level we're on, plus the flags
+		// we had, plus the exact flag.
+		getSource := listSource
+		getSource |= sourceFlags
+		getSource |= getSourceExact
+		raw = d.getList(k, nil, schema, getSource)
 		if raw.Exists {
 			break
 		}
@@ -384,11 +394,13 @@ func (d *ResourceData) getMap(
 	resultSet := false
 	prefix := k + "."
 
-	exact := source&getSourceExact != 0
-	source &^= getSourceExact
+	flags := source & ^getSourceLevelMask
+	level := source & getSourceLevelMask
+	exact := flags&getSourceExact != 0
+	diff := flags&getSourceDiff != 0
 
-	if !exact || source == getSourceState {
-		if d.state != nil && source >= getSourceState {
+	if !exact || level == getSourceState {
+		if d.state != nil && level >= getSourceState {
 			for k, _ := range d.state.Attributes {
 				if !strings.HasPrefix(k, prefix) {
 					continue
@@ -401,7 +413,7 @@ func (d *ResourceData) getMap(
 		}
 	}
 
-	if d.config != nil && source == getSourceConfig {
+	if d.config != nil && level == getSourceConfig {
 		// For config, we always set the result to exactly what was requested
 		if mraw, ok := d.config.Get(k); ok {
 			result = make(map[string]interface{})
@@ -433,27 +445,25 @@ func (d *ResourceData) getMap(
 		}
 	}
 
-	if !exact || source == getSourceDiff {
-		if d.diff != nil && source >= getSourceDiff {
-			for k, v := range d.diff.Attributes {
-				if !strings.HasPrefix(k, prefix) {
-					continue
-				}
-				resultSet = true
+	if d.diff != nil && diff {
+		for k, v := range d.diff.Attributes {
+			if !strings.HasPrefix(k, prefix) {
+				continue
+			}
+			resultSet = true
 
-				single := k[len(prefix):]
+			single := k[len(prefix):]
 
-				if v.NewRemoved {
-					delete(result, single)
-				} else {
-					result[single] = d.getPrimitive(k, nil, elemSchema, source).Value
-				}
+			if v.NewRemoved {
+				delete(result, single)
+			} else {
+				result[single] = d.getPrimitive(k, nil, elemSchema, source).Value
 			}
 		}
 	}
 
-	if !exact || source == getSourceSet {
-		if d.setMap != nil && source >= getSourceSet {
+	if !exact || level == getSourceSet {
+		if d.setMap != nil && level >= getSourceSet {
 			cleared := false
 			for k, _ := range d.setMap {
 				if !strings.HasPrefix(k, prefix) {
@@ -577,8 +587,10 @@ func (d *ResourceData) getPrimitive(
 	var result string
 	var resultProcessed interface{}
 	var resultComputed, resultSet bool
-	exact := source&getSourceExact != 0
-	source &^= getSourceExact
+	flags := source & ^getSourceLevelMask
+	source = source & getSourceLevelMask
+	exact := flags&getSourceExact != 0
+	diff := flags&getSourceDiff != 0
 
 	if !exact || source == getSourceState {
 		if d.state != nil && source >= getSourceState {
@@ -604,28 +616,26 @@ func (d *ResourceData) getPrimitive(
 		resultComputed = d.config.IsComputed(k)
 	}
 
-	if !exact || source == getSourceDiff {
-		if d.diff != nil && source >= getSourceDiff {
-			attrD, ok := d.diff.Attributes[k]
-			if ok {
-				if !attrD.NewComputed {
-					result = attrD.New
-					if attrD.NewExtra != nil {
-						// If NewExtra != nil, then we have processed data as the New,
-						// so we store that but decode the unprocessed data into result
-						resultProcessed = result
-
-						err := mapstructure.WeakDecode(attrD.NewExtra, &result)
-						if err != nil {
-							panic(err)
-						}
-					}
+	if d.diff != nil && diff {
+		attrD, ok := d.diff.Attributes[k]
+		if ok {
+			if !attrD.NewComputed {
+				result = attrD.New
+				if attrD.NewExtra != nil {
+					// If NewExtra != nil, then we have processed data as the New,
+					// so we store that but decode the unprocessed data into result
+					resultProcessed = result
 
-					resultSet = true
-				} else {
-					result = ""
-					resultSet = false
+					err := mapstructure.WeakDecode(attrD.NewExtra, &result)
+					if err != nil {
+						panic(err)
+					}
 				}
+
+				resultSet = true
+			} else {
+				result = ""
+				resultSet = false
 			}
 		}
 	}
@@ -1101,14 +1111,14 @@ func (d *ResourceData) stateSingle(
 func (d *ResourceData) stateSource(prefix string) getSource {
 	// If we're not doing a partial apply, then get the set level
 	if !d.partial {
-		return getSourceSet
+		return getSourceSet | getSourceDiff
 	}
 
 	// Otherwise, only return getSourceSet if its in the partial map.
 	// Otherwise we use state level only.
 	for k, _ := range d.partialMap {
 		if strings.HasPrefix(prefix, k) {
-			return getSourceSet
+			return getSourceSet | getSourceDiff
 		}
 	}
 

helper/schema/resource_data_test.go

@@ -527,6 +527,58 @@ func TestResourceDataGet(t *testing.T) {
 
 			Value: []interface{}{80},
 		},
+
+		{
+			Schema: map[string]*Schema{
+				"data": &Schema{
+					Type:     TypeSet,
+					Optional: true,
+					Elem: &Resource{
+						Schema: map[string]*Schema{
+							"index": &Schema{
+								Type:     TypeInt,
+								Required: true,
+							},
+
+							"value": &Schema{
+								Type:     TypeString,
+								Required: true,
+							},
+						},
+					},
+					Set: func(a interface{}) int {
+						m := a.(map[string]interface{})
+						return m["index"].(int)
+					},
+				},
+			},
+
+			State: &terraform.InstanceState{
+				Attributes: map[string]string{
+					"data.#":       "1",
+					"data.0.index": "10",
+					"data.0.value": "50",
+				},
+			},
+
+			Diff: &terraform.InstanceDiff{
+				Attributes: map[string]*terraform.ResourceAttrDiff{
+					"data.0.value": &terraform.ResourceAttrDiff{
+						Old: "50",
+						New: "80",
+					},
+				},
+			},
+
+			Key: "data",
+
+			Value: []interface{}{
+				map[string]interface{}{
+					"index": 10,
+					"value": "80",
+				},
+			},
+		},
 	}
 
 	for i, tc := range cases {

helper/schema/schema.go

@@ -580,6 +580,11 @@ func (m schemaMap) diffMap(
 		return fmt.Errorf("%s: %s", k, err)
 	}
 
+	// If the new map is nil and we're computed, then ignore it.
+	if n == nil && schema.Computed {
+		return nil
+	}
+
 	// Now we compare, preferring values from the config map
 	for k, v := range configMap {
 		old := stateMap[k]

helper/schema/schema_test.go

@@ -342,6 +342,31 @@ func TestSchemaMap_Diff(t *testing.T) {
 			Err: false,
 		},
 
+		/*
+		 * Bool
+		 */
+		{
+			Schema: map[string]*Schema{
+				"delete": &Schema{
+					Type:     TypeBool,
+					Optional: true,
+					Default:  false,
+				},
+			},
+
+			State: &terraform.InstanceState{
+				Attributes: map[string]string{
+					"delete": "false",
+				},
+			},
+
+			Config: nil,
+
+			Diff: nil,
+
+			Err: false,
+		},
+
 		/*
 		 * List decode
 		 */
@@ -1166,6 +1191,27 @@ func TestSchemaMap_Diff(t *testing.T) {
 			Err: false,
 		},
 
+		{
+			Schema: map[string]*Schema{
+				"vars": &Schema{
+					Type:     TypeMap,
+					Computed: true,
+				},
+			},
+
+			State: &terraform.InstanceState{
+				Attributes: map[string]string{
+					"vars.foo": "bar",
+				},
+			},
+
+			Config: nil,
+
+			Diff: nil,
+
+			Err: false,
+		},
+
 		{
 			Schema: map[string]*Schema{
 				"config_vars": &Schema{