diff --git a/builtin/providers/consul/resource_consul_keys.go b/builtin/providers/consul/resource_consul_keys.go index a2d965785..fe1db4ec1 100644 --- a/builtin/providers/consul/resource_consul_keys.go +++ b/builtin/providers/consul/resource_consul_keys.go @@ -2,7 +2,6 @@ package consul import ( "fmt" - "log" "strconv" consulapi "github.com/hashicorp/consul/api" @@ -12,7 +11,7 @@ import ( func resourceConsulKeys() *schema.Resource { return &schema.Resource{ Create: resourceConsulKeysCreate, - Update: resourceConsulKeysCreate, + Update: resourceConsulKeysUpdate, Read: resourceConsulKeysRead, Delete: resourceConsulKeysDelete, @@ -84,37 +83,22 @@ func resourceConsulKeysCreate(d *schema.ResourceData, meta interface{}) error { return err } - // Setup the operations using the datacenter - qOpts := consulapi.QueryOptions{Datacenter: dc, Token: token} - wOpts := consulapi.WriteOptions{Datacenter: dc, Token: token} + keyClient := newKeyClient(kv, dc, token) - // Store the computed vars - vars := make(map[string]string) - - // Extract the keys keys := d.Get("key").(*schema.Set).List() for _, raw := range keys { - key, path, sub, err := parseKey(raw) + _, path, sub, err := parseKey(raw) if err != nil { return err } 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 { - return fmt.Errorf("Failed to set Consul key '%s': %v", path, err) - } - vars[key] = value - } else { - log.Printf("[DEBUG] Getting key '%s' in %s", path, dc) - pair, _, err := kv.Get(path, &qOpts) - if err != nil { - return fmt.Errorf("Failed to get Consul key '%s': %v", path, err) - } - value := attributeValue(sub, key, pair) - vars[key] = value + if value == "" { + continue + } + + if err := keyClient.Put(path, value); err != nil { + return err } } @@ -123,15 +107,91 @@ func resourceConsulKeysCreate(d *schema.ResourceData, meta interface{}) error { // with some value to indicate the resource has been created. d.SetId("consul") - // Set the vars we collected above - if err := d.Set("var", vars); err != nil { + return resourceConsulKeysRead(d, meta) +} + +func resourceConsulKeysUpdate(d *schema.ResourceData, meta interface{}) error { + client := meta.(*consulapi.Client) + kv := client.KV() + token := d.Get("token").(string) + dc, err := getDC(d, client) + if err != nil { return err } + + keyClient := newKeyClient(kv, dc, token) + + if d.HasChange("key") { + o, n := d.GetChange("key") + if o == nil { + o = new(schema.Set) + } + if n == nil { + n = new(schema.Set) + } + + os := o.(*schema.Set) + ns := n.(*schema.Set) + + remove := os.Difference(ns).List() + add := ns.Difference(os).List() + + // We'll keep track of what keys we add so that if a key is + // in both the "remove" and "add" sets -- which will happen if + // its value is changed in-place -- we will avoid writing the + // value and then immediately removing it. + addedPaths := make(map[string]bool) + + // We add before we remove because then it's possible to change + // a key name (which will result in both an add and a remove) + // without very temporarily having *neither* value in the store. + // Instead, both will briefly be present, which should be less + // disruptive in most cases. + for _, raw := range add { + _, path, sub, err := parseKey(raw) + if err != nil { + return err + } + + value := sub["value"].(string) + if value == "" { + continue + } + + if err := keyClient.Put(path, value); err != nil { + return err + } + addedPaths[path] = true + } + + for _, raw := range remove { + _, path, sub, err := parseKey(raw) + if err != nil { + return err + } + + // Don't delete something we've just added. + // (See explanation at the declaration of this variable above.) + if addedPaths[path] { + continue + } + + shouldDelete, ok := sub["delete"].(bool) + if !ok || !shouldDelete { + continue + } + + if err := keyClient.Delete(path); err != nil { + return err + } + } + } + // Store the datacenter on this resource, which can be helpful for reference // in case it was read from the provider d.Set("datacenter", dc) - return nil + return resourceConsulKeysRead(d, meta) } func resourceConsulKeysRead(d *schema.ResourceData, meta interface{}) error { @@ -143,13 +203,10 @@ func resourceConsulKeysRead(d *schema.ResourceData, meta interface{}) error { return err } - // Setup the operations using the datacenter - qOpts := consulapi.QueryOptions{Datacenter: dc, Token: token} + keyClient := newKeyClient(kv, dc, token) - // Store the computed vars vars := make(map[string]string) - // Extract the keys keys := d.Get("key").(*schema.Set).List() for _, raw := range keys { key, path, sub, err := parseKey(raw) @@ -157,20 +214,33 @@ func resourceConsulKeysRead(d *schema.ResourceData, meta interface{}) error { return err } - log.Printf("[DEBUG] Refreshing value of key '%s' in %s", path, dc) - pair, _, err := kv.Get(path, &qOpts) + value, err := keyClient.Get(path) if err != nil { - return fmt.Errorf("Failed to get value for path '%s' from Consul: %v", path, err) + return err } - value := attributeValue(sub, key, pair) + value = attributeValue(sub, value) vars[key] = value + + // If there is already a "value" attribute present for this key + // then it was created as a "write" block. We need to update the + // given value within the block itself so that Terraform can detect + // when the Consul-stored value has drifted from what was most + // recently written by Terraform. + // We don't do this for "read" blocks; that causes confusing diffs + // because "value" should not be set for read-only key blocks. + if oldValue := sub["value"]; oldValue != "" { + sub["value"] = value + } } - // Update the resource if err := d.Set("var", vars); err != nil { return err } + if err := d.Set("key", keys); err != nil { + return err + } + // Store the datacenter on this resource, which can be helpful for reference // in case it was read from the provider d.Set("datacenter", dc) @@ -187,10 +257,9 @@ func resourceConsulKeysDelete(d *schema.ResourceData, meta interface{}) error { return err } - // Setup the operations using the datacenter - wOpts := consulapi.WriteOptions{Datacenter: dc, Token: token} + keyClient := newKeyClient(kv, dc, token) - // Extract the keys + // Clean up any keys that we're explicitly managing keys := d.Get("key").(*schema.Set).List() for _, raw := range keys { _, path, sub, err := parseKey(raw) @@ -198,15 +267,14 @@ func resourceConsulKeysDelete(d *schema.ResourceData, meta interface{}) error { return err } - // Ignore if the key is non-managed + // Skip if the key is non-managed shouldDelete, ok := sub["delete"].(bool) if !ok || !shouldDelete { continue } - log.Printf("[DEBUG] Deleting key '%s' in %s", path, dc) - if _, err := kv.Delete(path, &wOpts); err != nil { - return fmt.Errorf("Failed to delete Consul key '%s': %v", path, err) + if err := keyClient.Delete(path); err != nil { + return err } } @@ -236,10 +304,10 @@ func parseKey(raw interface{}) (string, string, map[string]interface{}, error) { // attributeValue determines the value for a key, potentially // using a default value if provided. -func attributeValue(sub map[string]interface{}, key string, pair *consulapi.KVPair) string { +func attributeValue(sub map[string]interface{}, readValue string) string { // Use the value if given - if pair != nil { - return string(pair.Value) + if readValue != "" { + return readValue } // Use a default if given diff --git a/builtin/providers/consul/resource_consul_keys_test.go b/builtin/providers/consul/resource_consul_keys_test.go index a820ff331..f6045658e 100644 --- a/builtin/providers/consul/resource_consul_keys_test.go +++ b/builtin/providers/consul/resource_consul_keys_test.go @@ -21,6 +21,7 @@ func TestAccConsulKeys_basic(t *testing.T) { testAccCheckConsulKeysExists(), testAccCheckConsulKeysValue("consul_keys.app", "enabled", "true"), testAccCheckConsulKeysValue("consul_keys.app", "set", "acceptance"), + testAccCheckConsulKeysValue("consul_keys.app", "remove_one", "hello"), ), }, resource.TestStep{ @@ -29,6 +30,7 @@ func TestAccConsulKeys_basic(t *testing.T) { testAccCheckConsulKeysExists(), testAccCheckConsulKeysValue("consul_keys.app", "enabled", "true"), testAccCheckConsulKeysValue("consul_keys.app", "set", "acceptanceUpdated"), + testAccCheckConsulKeysRemoved("consul_keys.app", "remove_one"), ), }, }, @@ -83,6 +85,20 @@ func testAccCheckConsulKeysValue(n, attr, val string) resource.TestCheckFunc { } } +func testAccCheckConsulKeysRemoved(n, attr string) resource.TestCheckFunc { + return func(s *terraform.State) error { + rn, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("Resource not found") + } + _, ok = rn.Primary.Attributes["var."+attr] + if ok { + return fmt.Errorf("Attribute '%s' still present: %#v", attr, rn.Primary.Attributes) + } + return nil + } +} + const testAccConsulKeysConfig = ` resource "consul_keys" "app" { datacenter = "dc1" @@ -97,6 +113,12 @@ resource "consul_keys" "app" { value = "acceptance" delete = true } + key { + name = "remove_one" + path = "test/remove_one" + value = "hello" + delete = true + } } ` diff --git a/website/source/docs/providers/consul/r/keys.html.markdown b/website/source/docs/providers/consul/r/keys.html.markdown index f0f854a3e..c1c806961 100644 --- a/website/source/docs/providers/consul/r/keys.html.markdown +++ b/website/source/docs/providers/consul/r/keys.html.markdown @@ -71,7 +71,8 @@ The `key` block supports the following: This allows a key to be written to. * `delete` - (Optional) If true, then the key will be deleted when - the resource is destroyed. Otherwise, it will be left in Consul. + either its configuration block is removed from the configuration or + the entire resource is destroyed. Otherwise, it will be left in Consul. Defaults to false. ## Attributes Reference @@ -81,4 +82,3 @@ The following attributes are exported: * `datacenter` - The datacenter the keys are being read/written to. * `var.` - For each name given, the corresponding attribute has the value of the key. -