fix writing secrets to vault KV(v1 and v2)

Writing to a Vault KV v1 and v2 were both broken.

V1 had a nil pointer dereference panic in Fetch where it tried to access
variables on the replied Secret which is always nil with V1.

V2 didn't get the sent in data structure correct as it didn't have the
additional map[string]interface{"data":...} wrapping around the values
that is required by V2.

Fixes #1252

dependency/vault_read_test.go

@@ -232,21 +232,17 @@ func TestVaultReadQuery_Fetch_KVv2(t *testing.T) {
 
 	// Write an initial value to the secret path
 	err := vault.CreateSecret("data/foo/bar", map[string]interface{}{
-		"data": map[string]interface{}{
-			"ttl": "100ms", // explicitly make this a short duration for testing
-			"zip": "zap",
-		},
+		"ttl": "100ms", // explicitly make this a short duration for testing
+		"zip": "zap",
 	})
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	// Write a new value to increment the version
 	err = vault.CreateSecret("data/foo/bar", map[string]interface{}{
-		"data": map[string]interface{}{
-			"ttl": "100ms", // explicitly make this a short duration for testing
-			"zip": "zop",
-		},
+		"ttl": "100ms", // explicitly make this a short duration for testing
+		"zip": "zop",
 	})
 	if err != nil {
 		t.Fatal(err)
@@ -437,6 +433,7 @@ func TestVaultReadQuery_Fetch_PKI_Anonymous(t *testing.T) {
 	})
 	_, err = anonClient.vault.client.Auth().Token().LookupSelf()
 	if err == nil || !strings.Contains(err.Error(), "missing client token") {
+		// check environment for VAULT_TOKEN
 		t.Fatalf("expected a missing client token error but found: %v", err)
 	}
 

dependency/vault_write.go

@@ -108,12 +108,14 @@ func (d *VaultWriteQuery) Fetch(clients *ClientSet, opts *QueryOptions) (interfa
 		return nil, nil, errors.Wrap(err, d.String())
 	}
 
-	// Print any warnings
-	printVaultWarnings(d, vaultSecret.Warnings)
-
-	// Create the cloned secret which will be exposed to the template.
-	d.vaultSecret = vaultSecret
-	d.secret = transformSecret(vaultSecret)
+	// vaultSecret == nil when writing to KVv1 engines
+	if vaultSecret != nil {
+		// Print any warnings
+		printVaultWarnings(d, vaultSecret.Warnings)
+		// Create the cloned secret which will be exposed to the template.
+		d.vaultSecret = vaultSecret
+		d.secret = transformSecret(vaultSecret)
+	}
 
 	return respWithMetadata(d.secret)
 }
@@ -168,14 +170,20 @@ func (d *VaultWriteQuery) writeSecret(clients *ClientSet, opts *QueryOptions) (*
 		RawQuery: opts.String(),
 	})
 
-	vaultSecret, err := clients.Vault().Logical().Write(d.path, d.data)
+	data := d.data
+
+	_, isv2, _ := isKVv2(clients.Vault(), d.path)
+	if isv2 {
+		data = map[string]interface{}{"data": d.data}
+	}
+
+	vaultSecret, err := clients.Vault().Logical().Write(d.path, data)
 	if err != nil {
 		return nil, errors.Wrap(err, d.String())
 	}
-	if vaultSecret == nil {
-		if _, isv2, _ := isKVv2(clients.Vault(), d.path); isv2 {
-			return nil, fmt.Errorf("no secret exists at %s", d.path)
-		}
+	// vaultSecret is always nil when KVv1 engine (isv2==false)
+	if isv2 && vaultSecret == nil {
+		return nil, fmt.Errorf("no secret exists at %s", d.path)
 	}
 
 	return vaultSecret, nil

dependency/vault_write_test.go

@@ -96,6 +96,9 @@ func TestVaultWriteSecretKV_Fetch(t *testing.T) {
 		}
 
 		rq, err := NewVaultReadQuery(path)
+		if err != nil {
+			t.Fatal(err)
+		}
 		act, err := rq.readSecret(clients, nil)
 		if err != nil {
 			t.Fatal(err)
@@ -126,6 +129,9 @@ func TestVaultWriteSecretKV_Fetch(t *testing.T) {
 		}
 
 		rq, err := NewVaultReadQuery(path)
+		if err != nil {
+			t.Fatal(err)
+		}
 		act, err := rq.readSecret(clients, nil)
 		if err != nil {
 			t.Fatal(err)