Issue1252: vault write broken

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

@@ -71,6 +71,77 @@ func TestNewVaultWriteQuery(t *testing.T) {
 	}
 }
 
+func TestVaultWriteSecretKV_Fetch(t *testing.T) {
+	t.Parallel()
+
+	// 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
+
+		path := secretsPath + "/foo"
+		exp := map[string]interface{}{
+			"bar": "zed",
+		}
+
+		wq, err := NewVaultWriteQuery(path, exp)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		_, _, err = wq.Fetch(clients, &QueryOptions{})
+		if err != nil {
+			fmt.Println(err)
+		}
+
+		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)
+	})
+
+	// 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
+
+		path := secretsPath + "/data/foo"
+		exp := map[string]interface{}{
+			"bar": "zed",
+		}
+
+		wq, err := NewVaultWriteQuery(path, exp)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		_, _, err = wq.Fetch(clients, &QueryOptions{})
+		if err != nil {
+			fmt.Println(err)
+		}
+
+		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["data"])
+	})
+}
+
 func TestVaultWriteQuery_Fetch(t *testing.T) {
 	t.Parallel()