Add an error_on_missing_keys option to consul_keys datasource (#375)

Closes #14


Co-authored-by: Rémi Lapeyre <[email protected]>

consul/config.go

@@ -28,7 +28,8 @@ type Config struct {
 	CAPath        string `mapstructure:"ca_path"`
 	InsecureHttps bool   `mapstructure:"insecure_https"`
 	Namespace     string `mapstructure:"namespace"`
-	client        *consulapi.Client
+
+	client *consulapi.Client
 }
 
 // Client returns a new client for accessing consul.

consul/data_source_consul_key_prefix.go

@@ -97,7 +97,7 @@ func dataSourceConsulKeyPrefixRead(d *schema.ResourceData, meta interface{}) err
 		}
 
 		fullPath := pathPrefix + path
-		value, _, err := keyClient.Get(fullPath)
+		_, value, _, err := keyClient.Get(fullPath)
 		if err != nil {
 			return err
 		}

consul/data_source_consul_keys.go

@@ -4,66 +4,85 @@
 package consul
 
 import (
+	"fmt"
+
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
 )
 
 func dataSourceConsulKeys() *schema.Resource {
 	return &schema.Resource{
 		Read: dataSourceConsulKeysRead,
 
+		Description: "The `consul_keys` datasource reads values from the Consul key/value store. This is a powerful way to dynamically set values in templates.",
+
 		Schema: map[string]*schema.Schema{
 			"datacenter": {
-				Type:     schema.TypeString,
-				Optional: true,
-				Computed: true,
+				Type:        schema.TypeString,
+				Description: "The datacenter to use. This overrides the agent's default datacenter and the datacenter in the provider setup.",
+				Optional:    true,
+				Computed:    true,
+			},
+
+			"error_on_missing_keys": {
+				Type:        schema.TypeBool,
+				Optional:    true,
+				Description: "Whether to return an error when a key is absent from the KV store and no default is configured. This defaults to `false`.",
 			},
 
 			"token": {
-				Type:       schema.TypeString,
-				Optional:   true,
-				Sensitive:  true,
-				Deprecated: tokenDeprecationMessage,
+				Type:        schema.TypeString,
+				Description: "The ACL token to use. This overrides the token that the agent provides by default.",
+				Deprecated:  tokenDeprecationMessage,
+				Optional:    true,
+				Sensitive:   true,
 			},
 
 			"key": {
-				Type:     schema.TypeSet,
-				Optional: true,
+				Type:        schema.TypeSet,
+				Description: "Specifies a key in Consul to be read. Supported values documented below. Multiple blocks supported.",
+				Optional:    true,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"name": {
-							Type:     schema.TypeString,
-							Required: true,
+							Type:        schema.TypeString,
+							Description: "This is the name of the key. This value of the key is exposed as `var.<name>`. This is not the path of the key in Consul.",
+							Required:    true,
 						},
 
 						"path": {
-							Type:     schema.TypeString,
-							Required: true,
+							Type:        schema.TypeString,
+							Description: "This is the path in Consul that should be read or written to.",
+							Required:    true,
 						},
 
 						"default": {
-							Type:     schema.TypeString,
-							Optional: true,
+							Type:        schema.TypeString,
+							Description: "This is the default value to set for `var.<name>` if the key does not exist in Consul. Defaults to an empty string.",
+							Optional:    true,
 						},
 					},
 				},
 			},
 
 			"var": {
-				Type:     schema.TypeMap,
-				Computed: true,
+				Type:        schema.TypeMap,
+				Description: "For each name given, the corresponding attribute has the value of the key.",
+				Computed:    true,
 				Elem: &schema.Schema{
 					Type: schema.TypeString,
 				},
 			},
 
 			"namespace": {
-				Type:     schema.TypeString,
-				Optional: true,
+				Type:        schema.TypeString,
+				Description: "The namespace to lookup the keys.",
+				Optional:    true,
 			},
 
 			"partition": {
-				Type:     schema.TypeString,
-				Optional: true,
+				Type:        schema.TypeString,
+				Description: "The partition to lookup the keys.",
+				Optional:    true,
 			},
 		},
 	}
@@ -81,12 +100,22 @@ func dataSourceConsulKeysRead(d *schema.ResourceData, meta interface{}) error {
 			return err
 		}
 
-		value, _, err := keyClient.Get(path)
+		exist, value, _, err := keyClient.Get(path)
 		if err != nil {
 			return err
 		}
 
+		// This returns the value if it exists or the default value if one is set.
+		// If the key does not exist and there is no default, value will be the
+		// empty string.
 		value = attributeValue(sub, value)
+
+		if !exist && value == "" && d.Get("error_on_missing_keys").(bool) {
+			// We return an error when the key does not exist, there is no default
+			// and error_on_missing_keys has been set in the config.
+			return fmt.Errorf("Key %q does not exist", path)
+		}
+
 		vars[key] = value
 	}
 

consul/data_source_consul_keys_test.go

@@ -22,6 +22,28 @@ func TestAccDataConsulKeys_basic(t *testing.T) {
 					testAccCheckConsulKeysValue("data.consul_keys.read", "read", "written"),
 				),
 			},
+			{
+				Config:      testAccDataConsulKeysNonExistantKeyConfig,
+				ExpectError: regexp.MustCompile(`Key ".*" does not exist`),
+			},
+			{
+				Config: testAccDataConsulKeysNonExistantKeyDefaultBehaviourConfig,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckConsulKeysValue("data.consul_keys.read", "read", ""),
+				),
+			},
+			{
+				Config: testAccDataConsulKeysNonExistantKeyWithDefaultConfig,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckConsulKeysValue("data.consul_keys.read", "read", "myvalue"),
+				),
+			},
+			{
+				Config: testAccDataConsulKeysExistantKeyWithDefaultAndEmptyValueConfig,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckConsulKeysValue("data.consul_keys.read", "read", "myvalue"),
+				),
+			},
 		},
 	})
 }
@@ -72,7 +94,61 @@ func TestAccDataConsulKeys_datacenter(t *testing.T) {
 	})
 }
 
-const testAccDataConsulKeysConfig = `
+const (
+	testAccDataConsulKeysNonExistantKeyDefaultBehaviourConfig = `
+data "consul_keys" "read" {
+    key {
+        path = "test/set"
+        name = "read"
+    }
+}
+`
+	testAccDataConsulKeysNonExistantKeyConfig = `
+data "consul_keys" "read" {
+	error_on_missing_keys = true
+    key {
+        path = "test/set"
+        name = "read"
+    }
+}
+`
+
+	testAccDataConsulKeysNonExistantKeyWithDefaultConfig = `
+data "consul_keys" "read" {
+    key {
+        path = "test/set"
+        name = "read"
+		default = "myvalue"
+    }
+}
+
+`
+
+	testAccDataConsulKeysExistantKeyWithDefaultAndEmptyValueConfig = `
+resource "consul_keys" "write" {
+    datacenter = "dc1"
+
+    key {
+        path   = "test/set"
+        value  = ""
+		delete = true
+    }
+}
+
+data "consul_keys" "read" {
+    # Create a dependency on the resource so we're sure to
+    # have the value in place before we try to read it.
+    datacenter = consul_keys.write.datacenter
+
+    key {
+        path = "test/set"
+        name = "read"
+		default = "myvalue"
+    }
+}
+`
+
+	testAccDataConsulKeysConfig = `
 resource "consul_keys" "write" {
     datacenter = "dc1"
 
@@ -85,7 +161,7 @@ resource "consul_keys" "write" {
 data "consul_keys" "read" {
     # Create a dependency on the resource so we're sure to
     # have the value in place before we try to read it.
-    datacenter = "${consul_keys.write.datacenter}"
+    datacenter = consul_keys.write.datacenter
 
     key {
         path = "test/data_source"
@@ -94,7 +170,7 @@ data "consul_keys" "read" {
 }
 `
 
-const testAccDataConsulKeysConfigNamespaceCE = `
+	testAccDataConsulKeysConfigNamespaceCE = `
 data "consul_keys" "read" {
   namespace  = "test-data-consul-keys"
 
@@ -104,12 +180,12 @@ data "consul_keys" "read" {
   }
 }`
 
-const testAccDataConsulKeysConfigNamespaceEE = `
+	testAccDataConsulKeysConfigNamespaceEE = `
 resource "consul_keys" "write" {
   datacenter = "dc1"
 
   key {
-    path = "test/data_source"
+    path  = "test/data_source"
     value = "written"
   }
 }
@@ -119,7 +195,7 @@ resource "consul_namespace" "test" {
 }
 
 data "consul_keys" "read" {
-  namespace = consul_namespace.test.name
+  namespace  = consul_namespace.test.name
   datacenter = consul_keys.write.datacenter
 
   key {
@@ -128,14 +204,14 @@ data "consul_keys" "read" {
   }
 }`
 
-const testAccDataConsulKeysConfigDatacenter = `
+	testAccDataConsulKeysConfigDatacenter = `
 resource "consul_keys" "write" {
     datacenter = "dc2"
 
     key {
         path   = "test/dc"
         value  = "dc2"
-		delete = true
+        delete = true
     }
 }
 
@@ -155,3 +231,4 @@ data "consul_keys" "dc2" {
     }
 }
 `
+)

consul/key_client.go

@@ -29,24 +29,20 @@ func newKeyClient(d *schema.ResourceData, meta interface{}) *keyClient {
 	}
 }
 
-func (c *keyClient) Get(path string) (string, int, error) {
+func (c *keyClient) Get(path string) (bool, string, int, error) {
 	log.Printf(
 		"[DEBUG] Reading key '%s' in %s",
 		path, c.qOpts.Datacenter,
 	)
 	pair, _, err := c.client.Get(path, c.qOpts)
 	if err != nil {
-		return "", 0, fmt.Errorf("failed to read Consul key '%s': %s", path, err)
+		return false, "", 0, fmt.Errorf("failed to read Consul key '%s': %s", path, err)
 	}
-	value := ""
-	if pair != nil {
-		value = string(pair.Value)
+	if pair == nil {
+		return false, "", 0, nil
 	}
-	flags := 0
-	if pair != nil {
-		flags = int(pair.Flags)
-	}
-	return value, flags, nil
+
+	return true, string(pair.Value), int(pair.Flags), nil
 }
 
 func (c *keyClient) GetUnderPrefix(pathPrefix string) (consulapi.KVPairs, error) {

consul/resource_consul_keys.go

@@ -208,7 +208,7 @@ func resourceConsulKeysRead(d *schema.ResourceData, meta interface{}) error {
 			return err
 		}
 
-		value, flags, err := keyClient.Get(path)
+		_, value, flags, err := keyClient.Get(path)
 		if err != nil {
 			return err
 		}

consul/resource_provider.go

@@ -311,6 +311,7 @@ func providerConfigure(d *schema.ResourceData) (interface{}, error) {
 		for k, v := range authConfig["meta"].(map[string]interface{}) {
 			meta[k] = v.(string)
 		}
+
 		_, wOpts := getOptions(d, config)
 		token, _, err := client.ACL().Login(&consulapi.ACLLoginParams{
 			AuthMethod:  authMethod,
@@ -329,6 +330,7 @@ func providerConfigure(d *schema.ResourceData) (interface{}, error) {
 func getClient(d *schema.ResourceData, meta interface{}) (*consulapi.Client, *consulapi.QueryOptions, *consulapi.WriteOptions) {
 	config := meta.(*Config)
 	client := config.client
+
 	qOpts, wOpts := getOptions(d, config)
 	return client, qOpts, wOpts
 }
@@ -337,6 +339,7 @@ func getOptions(d *schema.ResourceData, meta interface{}) (*consulapi.QueryOptio
 	config := meta.(*Config)
 	client := config.client
 	var dc, token, namespace, partition string
+
 	if v, ok := d.GetOk("datacenter"); ok {
 		dc = v.(string)
 	}