Resolve TF state for PKI Multi-Issuer workflows (hashicorp#1973)

Author: vinay-gopalan

go.mod

@@ -20,6 +20,7 @@ require (
 	github.com/denisenkom/go-mssqldb v0.12.0
 	github.com/docker/distribution v2.8.1+incompatible // indirect
 	github.com/go-sql-driver/mysql v1.6.0
+	github.com/google/uuid v1.3.0
 	github.com/googleapis/enterprise-certificate-proxy v0.2.0 // indirect
 	github.com/gosimple/slug v1.11.0
 	github.com/hashicorp/errwrap v1.1.0

util/util.go

@@ -51,6 +51,10 @@ func ToStringArray(input []interface{}) []string {
 	return output
 }
 
+func Is500(err error) bool {
+	return ErrorContainsHTTPCode(err, http.StatusInternalServerError)
+}
+
 func Is404(err error) bool {
 	return ErrorContainsHTTPCode(err, http.StatusNotFound)
 }
@@ -64,6 +68,10 @@ func ErrorContainsHTTPCode(err error, codes ...int) bool {
 	return false
 }
 
+func ErrorContainsString(err error, s string) bool {
+	return strings.Contains(err.Error(), s)
+}
+
 // CalculateConflictsWith returns a slice of field names that conflict with
 // a single field (self).
 func CalculateConflictsWith(self string, group []string) []string {

vault/resource_pki_secret_backend_intermediate_cert_request.go

@@ -5,9 +5,11 @@ package vault
 
 import (
 	"context"
+	"fmt"
 	"log"
 	"strings"
 
+	"github.com/google/uuid"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
@@ -231,7 +233,8 @@ func pkiSecretBackendIntermediateCertRequestCreate(ctx context.Context, d *schem
 
 	backend := d.Get(consts.FieldBackend).(string)
 	intermediateType := d.Get(consts.FieldType).(string)
-	path := pkiSecretBackendIntermediateGeneratePath(backend, intermediateType)
+
+	path := pkiSecretBackendIntermediateGeneratePath(backend, intermediateType, provider.IsAPISupported(meta, provider.VaultVersion111))
 
 	intermediateCertAPIFields := []string{
 		consts.FieldCommonName,
@@ -263,14 +266,18 @@ func pkiSecretBackendIntermediateCertRequestCreate(ctx context.Context, d *schem
 	// add multi-issuer write API fields if supported
 	isIssuerAPISupported := provider.IsAPISupported(meta, provider.VaultVersion111)
 
+	// Fields only used when we are generating a key
 	if !(intermediateType == keyTypeKMS || intermediateType == consts.FieldExisting) {
-		// if kms or existing type,
 		intermediateCertAPIFields = append(intermediateCertAPIFields, consts.FieldKeyType, consts.FieldKeyBits)
-		if isIssuerAPISupported {
+	}
+
+	if isIssuerAPISupported {
+		// Note: CSR generation does not persist an issuer, just a key, so consts.FieldIssuerName is not supported
+		if intermediateType == consts.FieldExisting {
 			intermediateCertAPIFields = append(intermediateCertAPIFields, consts.FieldKeyRef)
+		} else {
+			intermediateCertAPIFields = append(intermediateCertAPIFields, consts.FieldKeyName)
 		}
-	} else if isIssuerAPISupported {
-		intermediateCertAPIFields = append(intermediateCertAPIFields, consts.FieldKeyName)
 	}
 
 	data := map[string]interface{}{}
@@ -329,7 +336,15 @@ func pkiSecretBackendIntermediateCertRequestCreate(ctx context.Context, d *schem
 
 	}
 
-	d.SetId(path)
+	id := path
+	if provider.IsAPISupported(meta, provider.VaultVersion111) {
+		// multiple CSRs can be generated
+		// ensure unique IDs
+		uniqueSuffix := uuid.New()
+		id = fmt.Sprintf("%s/%s", path, uniqueSuffix)
+	}
+
+	d.SetId(id)
 	return pkiSecretBackendIntermediateCertRequestRead(ctx, d, meta)
 }
 
@@ -341,6 +356,9 @@ func pkiSecretBackendIntermediateCertRequestDelete(ctx context.Context, d *schem
 	return nil
 }
 
-func pkiSecretBackendIntermediateGeneratePath(backend string, intermediateType string) string {
+func pkiSecretBackendIntermediateGeneratePath(backend, intermediateType string, isMultiIssuerSupported bool) string {
+	if isMultiIssuerSupported {
+		return strings.Trim(backend, "/") + "/issuers/generate/intermediate/" + strings.Trim(intermediateType, "/")
+	}
 	return strings.Trim(backend, "/") + "/intermediate/generate/" + strings.Trim(intermediateType, "/")
 }

vault/resource_pki_secret_backend_intermediate_cert_request_test.go

@@ -80,14 +80,27 @@ func TestPkiSecretBackendIntermediateCertificate_multiIssuer(t *testing.T) {
 	resourceName := "vault_pki_secret_backend_intermediate_cert_request.test"
 	keyName := acctest.RandomWithPrefix("test-pki-key")
 
-	checks := []resource.TestCheckFunc{
+	// used to test existing key flow
+	store := &testPKIKeyStore{}
+	keyResourceName := "vault_pki_secret_backend_key.test"
+	updatedKeyName := acctest.RandomWithPrefix("test-pki-key-updated")
+
+	commonChecks := []resource.TestCheckFunc{
 		resource.TestCheckResourceAttr(resourceName, consts.FieldBackend, path),
-		resource.TestCheckResourceAttr(resourceName, consts.FieldType, "internal"),
+		resource.TestCheckResourceAttrSet(resourceName, consts.FieldKeyID),
 		resource.TestCheckResourceAttr(resourceName, consts.FieldCommonName, "test Intermediate CA"),
+	}
+
+	internalChecks := append(commonChecks,
+		resource.TestCheckResourceAttr(resourceName, consts.FieldType, "internal"),
+		// keyName is only set on internal if it is passed by user
 		resource.TestCheckResourceAttr(resourceName, consts.FieldKeyName, keyName),
-		resource.TestCheckResourceAttrSet(resourceName, consts.FieldKeyID),
+	)
+
+	existingChecks := append(commonChecks,
+		resource.TestCheckResourceAttr(resourceName, consts.FieldType, "existing"),
 		resource.TestCheckResourceAttrSet(resourceName, consts.FieldKeyRef),
-	}
+	)
 
 	resource.Test(t, resource.TestCase{
 		ProviderFactories: providerFactories,
@@ -97,11 +110,27 @@ func TestPkiSecretBackendIntermediateCertificate_multiIssuer(t *testing.T) {
 		},
 		CheckDestroy: testCheckMountDestroyed("vault_mount", consts.MountTypePKI, consts.FieldPath),
 		Steps: []resource.TestStep{
-			// @TODO add a test step with a key_ref
 			{
-				Config: testPkiSecretBackendIntermediateCertRequestConfig_multiIssuer(path, keyName),
+				Config: testPkiSecretBackendIntermediateCertRequestConfig_multiIssuerInternal(path, keyName),
 				Check: resource.ComposeTestCheckFunc(
-					append(checks)...,
+					append(internalChecks)...,
+				),
+			},
+			{
+				// Create and capture key ID
+				Config: testAccPKISecretBackendKey_basic(path, updatedKeyName, "rsa", "2048"),
+				Check: resource.ComposeTestCheckFunc(
+					testCapturePKIKeyID(keyResourceName, store),
+				),
+			},
+			{
+				Config: testPkiSecretBackendIntermediateCertRequestConfig_multiIssuerExisting(path, updatedKeyName),
+				Check: resource.ComposeTestCheckFunc(
+					append(existingChecks,
+						// confirm that root cert key ID is same as the key
+						// created in step 2; thereby confirming key_ref is passed
+						testPKIKeyUpdate(resourceName, store, true),
+					)...,
 				),
 			},
 		},
@@ -128,7 +157,26 @@ resource "vault_pki_secret_backend_intermediate_cert_request" "test" {
 `, path, addConstraints)
 }
 
-func testPkiSecretBackendIntermediateCertRequestConfig_multiIssuer(path, keyName string) string {
+func testPkiSecretBackendIntermediateCertRequestConfig_multiIssuerInternal(path, keyName string) string {
+	return fmt.Sprintf(`
+resource "vault_mount" "test" {
+  path                      = "%s"
+  type                      = "pki"
+  description               = "test"
+  default_lease_ttl_seconds = 86400
+  max_lease_ttl_seconds     = 86400
+}
+
+resource "vault_pki_secret_backend_intermediate_cert_request" "test" {
+  backend     = vault_mount.test.path
+  type        = "internal"
+  common_name = "test Intermediate CA"
+  key_name    = "%s"
+}
+`, path, keyName)
+}
+
+func testPkiSecretBackendIntermediateCertRequestConfig_multiIssuerExisting(path, keyName string) string {
 	return fmt.Sprintf(`
 resource "vault_mount" "test" {
   path                      = "%s"
@@ -141,17 +189,16 @@ resource "vault_mount" "test" {
 resource "vault_pki_secret_backend_key" "test" {
   backend  = vault_mount.test.path
   type     = "exported"
-  key_name = "test"
+  key_name = "%s"
   key_type = "rsa"
-  key_bits = "4096"
+  key_bits = "2048"
 }
 
 resource "vault_pki_secret_backend_intermediate_cert_request" "test" {
   backend     = vault_mount.test.path
-  type        = "internal"
+  type        = "existing"
   common_name = "test Intermediate CA"
-  key_ref     = vault_pki_secret_backend_key.test.id
-  key_name    = "%s"
+  key_ref     = vault_pki_secret_backend_key.test.key_id
 }
 `, path, keyName)
 }

vault/resource_pki_secret_backend_issuer.go

@@ -205,6 +205,11 @@ func pkiSecretBackendIssuerRead(ctx context.Context, d *schema.ResourceData, met
 
 	log.Printf("[DEBUG] Reading %s from Vault", path)
 	resp, err := client.Logical().ReadWithContext(ctx, path)
+	if resp == nil {
+		d.SetId("")
+		return nil
+	}
+
 	if err != nil {
 		return diag.Errorf("error reading from Vault: %s", err)
 	}
@@ -232,9 +237,11 @@ func pkiSecretBackendIssuerRead(ctx context.Context, d *schema.ResourceData, met
 	}
 
 	for _, k := range fields {
-		if err := d.Set(k, resp.Data[k]); err != nil {
-			return diag.Errorf("error setting state key %q for issuer, err=%s",
-				k, err)
+		if v, ok := resp.Data[k]; ok {
+			if err := d.Set(k, v); err != nil {
+				return diag.Errorf("error setting state key %q for issuer, err=%s",
+					k, err)
+			}
 		}
 	}
 

vault/resource_pki_secret_backend_key_test.go

@@ -86,14 +86,16 @@ func TestAccPKISecretBackendKey_basic(t *testing.T) {
 
 func testAccPKISecretBackendKey_basic(path, keyName, keyType, keyBits string) string {
 	return fmt.Sprintf(`
-resource "vault_mount" "pki" {
-	path        = "%s"
-	type        = "pki"
-    description = "PKI secret engine mount"
+resource "vault_mount" "test" {
+  path                      = "%s"
+  type                      = "pki"
+  description               = "test"
+  default_lease_ttl_seconds = "86400"
+  max_lease_ttl_seconds     = "86400"
 }
 
 resource "vault_pki_secret_backend_key" "test" {
-  backend  = vault_mount.pki.path
+  backend  = vault_mount.test.path
   type     = "exported"
   key_name = "%s"
   key_type = "%s"