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

@@ -74,8 +74,9 @@ func TestNewVaultWriteQuery(t *testing.T) {
 func TestVaultWriteSecretKV_Fetch(t *testing.T) {
 	t.Parallel()
 
-	// trigger a nil-pointer-deref panic in wq.Fetch() with KVv1 due to
-	// writeSecret() returning nil for vaultSecret
+	// previously triggered a nil-pointer-deref panic in wq.Fetch() with KVv1
+	// due to writeSecret() returning nil for vaultSecret
+	// see GH-1252
 	t.Run("write_secret_v1", func(t *testing.T) {
 		clients, vault := testVaultServer(t, "write_secret_v1", "1")
 		secretsPath := vault.secretsPath
@@ -96,16 +97,20 @@ 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)
 		}
 		assert.Equal(t, exp, act.Data)
 	})
 
-	// trigger vault returning "no data provided" with KVv2 as the data
-	// structure passed in didn't have additional wrapping map with the "data"
-	// key as used in KVv2
+	// previously triggered vault returning "no data provided" with KVv2 as the
+	// data structure passed in didn't have additional wrapping map with the
+	// "data" key as used in KVv2
+	// see GH-1252
 	t.Run("write_secret_v2", func(t *testing.T) {
 		clients, vault := testVaultServer(t, "write_secret_v2", "2")
 		secretsPath := vault.secretsPath
@@ -126,6 +131,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)