Support v2 KV mount #156

Mongey · 2018-08-12T17:41:42Z

Should address #140

Borrows a lot of code from the cli, kv get in hashicorp/vault/command, since it's not public api.

jwierzbo · 2018-09-03T10:59:49Z

@Mongey are you going to update the code in this PR?

tyrannosaurus-becks · 2018-09-24T23:40:25Z

@Mongey excited about this one. Is it still a WIP or ready for review?

Mongey · 2018-09-27T16:34:53Z

@tyrannosaurus-becks This should be ready for some 📝 now. Ended up being a bit more work than expected, but should support all v2 crud operations + data source, while preserving v1 compatibility.

tyrannosaurus-becks · 2018-09-28T20:15:29Z

@Mongey thank you! I will get to reviewing this next week.

tyrannosaurus-becks

@Mongey thanks for writing this, it looks great! Just a couple minor questions.

tyrannosaurus-becks · 2018-10-05T16:39:25Z

vault/data_source_generic_secret.go

+				Type:     schema.TypeInt,
+				Required: false,
+				Optional: true,
+				Default:  -1,


I'm curious about why the default of -1 here? Just curious. I recently learned that there's a paradigm in the Terraform providers of starting at version 0 which is the default if unset here.

This is actually to retrieve the latest version of the secret from Vault, rather than the Terraform state version. I've extracted it to a constant to (hopefully) better represent what it does.

tyrannosaurus-becks · 2018-10-05T16:44:01Z

vault/kv_helpers.go

+	return fmt.Sprintf("%s %s %s", strings.Repeat("=", equalSigns/2), header, strings.Repeat("=", equalSigns/2))
+}
+
+func kvParseVersionsFlags(versions []string) []string {


This doesn't appear to be used either.

tyrannosaurus-becks · 2018-10-05T16:44:22Z

vault/kv_helpers.go

+	}
+}
+
+func getHeaderForMap(header string, data map[string]interface{}) string {


This doesn't appear to be used.

tyrannosaurus-becks · 2018-10-05T16:57:20Z

vault/resource_generic_secret.go

+	originalPath := path // if the path belongs to a v2 endpoint, it will be modified
+	mountPath, v2, err := isKVv2(path, client)
+	if err != nil {
+		return fmt.Errorf("Error determining if it's a v2 path: %s", err)


Would it be possible to lower case this error?

tyrannosaurus-becks · 2018-10-05T16:58:04Z

vault/resource_generic_secret.go

@@ -120,8 +134,17 @@ func genericSecretResourceDelete(d *schema.ResourceData, meta interface{}) error

 	path := d.Id()

+	mountPath, v2, err := isKVv2(path, client)
+	if err != nil {
+		return fmt.Errorf("Error determining if it's a v2 path: %s", err)


This is used to represent retrieving the latest version of a secret from Vault.

tyrannosaurus-becks

@Mongey this looks fantastic. Also I note you fixed some of the tests that are currently failing on master. Thank you!

lattwood · 2018-11-06T18:37:28Z

vault/kv_helpers.go

+	if err != nil {
+		// If we get a 404 we are using an older version of vault, default to
+		// version 1
+		if resp != nil && resp.StatusCode == 404 {


This needs to check for 403 as well, for older vault versions.

Support v2 KV mount

Mongey force-pushed the cm-versioned-kv branch from 5b3eaff to 0abafbe Compare September 18, 2018 09:37

ghost added the size/XXL label Sep 18, 2018

Mongey force-pushed the cm-versioned-kv branch from 0abafbe to 4f9607c Compare September 18, 2018 09:39

ghost added size/L and removed size/XXL labels Sep 18, 2018

tyrannosaurus-becks self-assigned this Sep 24, 2018

Mongey force-pushed the cm-versioned-kv branch from 4f9607c to a9215ef Compare September 27, 2018 10:47