Merge pull request #186 from catsby/vault_kv2

Fix panic with Vault KV 2 secrets

runner.go

@@ -22,9 +22,10 @@ import (
 	"github.com/pkg/errors"
 )
 
-// Regexp for invalid characters in keys
+// InvalidRegexp is a regexp for invalid characters in keys
 var InvalidRegexp = regexp.MustCompile(`[^a-zA-Z0-9_]`)
 
+// Runner executes a given child process with configuration
 type Runner struct {
 	// ErrCh and DoneCh are channels where errors and finish notifications occur.
 	ErrCh  chan error
@@ -195,7 +196,7 @@ func (r *Runner) Stop() {
 	r.stopChild()
 
 	if err := r.deletePid(); err != nil {
-		log.Printf("[WARN] (runner) could not remove pid at %q: %s",
+		log.Printf("[WARN] (runner) could not remove pid at %#v: %s",
 			r.config.PidFile, err)
 	}
 
@@ -418,6 +419,43 @@ func (r *Runner) appendSecrets(
 	cp := r.configPrefixMap[d.String()]
 
 	for key, value := range typed.Data {
+		// check for presence of "metadata", indicating this value came from Vault
+		// kv version 2, and we should then use the sub map instead
+		if key == "metadata" {
+			continue
+		}
+
+		// Vault Secrets KV1 and KV2 return different formats. Here we check the key
+		// value, and if we've found another key called "data" that is of type
+		// map[string]interface, we assume it's KV2 and use the key/value pair from
+		// it, otherwise we assume it's KV1
+		//
+		// value here in KV1 format is a simple string.
+		// value in KV2 format is a map:
+		// map[string]interface {}{
+		//   "key": "value",
+		// }
+		if key == "data" {
+			switch value.(type) {
+			case map[string]interface{}:
+				log.Printf("[DEBUG] Found KV2 secret")
+				mapV := value.(map[string]interface{})
+
+				// assumes this map is only 1 element long, we change the key and value
+				// to match the sub element
+				for k, v := range mapV {
+					key = k
+					value = v
+					break
+				}
+			default:
+				// Only handle the sub data map, do nothing otherwise.
+				// If the secret has been deleted but not destroyed, Vault will return a
+				// response with a nil sub data map, and will not be handled by the
+				// above block
+			}
+		}
+
 		// Ignore any keys that are empty (not sure if this is even possible in
 		// Vault, but I play defense).
 		if strings.TrimSpace(key) == "" {

runner_test.go

@@ -0,0 +1,105 @@
+package main
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/hashicorp/consul-template/config"
+	"github.com/hashicorp/consul-template/dependency"
+)
+
+func TestRunner_appendSecrets(t *testing.T) {
+	t.Parallel()
+
+	secretValue := "somevalue"
+
+	cases := map[string]struct {
+		path     string
+		data     *dependency.Secret
+		notFound bool
+	}{
+		"kv1_secret": {
+			"kv/bar",
+			&dependency.Secret{
+				Data: map[string]interface{}{
+					"key_field": secretValue,
+				},
+			},
+			false,
+		},
+		"kv2_secret": {
+			"secret/data/foo",
+			&dependency.Secret{
+				Data: map[string]interface{}{
+					"metadata": map[string]interface{}{
+						"destroyed": bool(false),
+						"version":   "1",
+					},
+					"data": map[string]interface{}{
+						"key_field": secretValue,
+					},
+				},
+			},
+			false,
+		},
+		"kv2_secret_destroyed": {
+			"secret/data/foo",
+			&dependency.Secret{
+				Data: map[string]interface{}{
+					"metadata": map[string]interface{}{
+						"destroyed": bool(true),
+						"version":   "2",
+					},
+					"data": nil,
+				},
+			},
+			true,
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(fmt.Sprintf("%s", name), func(t *testing.T) {
+			cfg := Config{
+				Secrets: &PrefixConfigs{
+					&PrefixConfig{
+						Path: config.String(tc.path),
+					},
+				},
+			}
+			c := DefaultConfig().Merge(&cfg)
+			r, err := NewRunner(c, true)
+			if err != nil {
+				t.Fatal(err)
+			}
+			vrq, err := dependency.NewVaultReadQuery(tc.path)
+			if err != nil {
+				t.Fatal(err)
+			}
+			env := make(map[string]string)
+			appendError := r.appendSecrets(env, vrq, tc.data)
+			if appendError != nil {
+				t.Fatalf("got err: %s", appendError)
+			}
+
+			if len(env) > 1 {
+				t.Fatalf("Expected only 1 value in this test")
+			}
+
+			keyName := tc.path + "_key_field"
+			keyName = strings.Replace(keyName, "/", "_", -1)
+
+			var value string
+			value, ok := env[keyName]
+			if !ok && !tc.notFound {
+				t.Fatalf("expected (%s) key, but was not found", keyName)
+			}
+			if ok && tc.notFound {
+				t.Fatalf("expected to not find key, but (%s) was found", keyName)
+			}
+			if ok && value != secretValue {
+				t.Fatalf("values didn't match, expected (%s), got (%s)", secretValue, value)
+			}
+		})
+	}
+}