secret/pki: fix key_usage panic and unset function (#2036)

* secret/pki: fix key_usage panic and unset function * add changelog * update docs * handle empty array set in config * Update website/docs/r/pki_secret_backend_role.html.md Co-authored-by: vinay-gopalan <[email protected]> * update doc * remove commented code --------- Co-authored-by: vinay-gopalan <[email protected]>
hashicorp · Oct 5, 2023 · b672cb4 · b672cb4
1 parent 95a3230
commit b672cb4
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@ FEATURES:
 
 BUGS:
 * Fix duplicate timestamp and incorrect level messages: ([#2031](https://github.com/hashicorp/terraform-provider-vault/pull/2031))
+* Fix panic when setting `key_usage` to an array of empty string and enable it to unset the key usage constraints: ([#2036](https://github.com/hashicorp/terraform-provider-vault/pull/2036))
 
 IMPROVEMENTS:
 * Ensure sensitive values are masked in `vault_approle_auth_backend_login` plan output ([#2008](https://github.com/hashicorp/terraform-provider-vault/pull/2008))

diff --git a/vault/resource_pki_secret_backend_role.go b/vault/resource_pki_secret_backend_role.go
@@ -49,7 +49,6 @@ var pkiSecretListFields = []string{
 	consts.FieldAllowedDomains,
 	consts.FieldAllowedSerialNumbers,
 	consts.FieldExtKeyUsage,
-	consts.FieldKeyUsage,
 }
 
 var pkiSecretBooleanFields = []string{
@@ -448,17 +447,25 @@ func pkiSecretBackendRoleCreate(ctx context.Context, d *schema.ResourceData, met
 	// handle TypeList
 	for _, k := range pkiSecretListFields {
 		if v, ok := d.GetOk(k); ok {
-			ifcList := v.([]interface{})
-			list := make([]string, 0, len(ifcList))
-			for _, ifc := range ifcList {
-				list = append(list, ifc.(string))
-			}
+			list := expandStringSlice(v.([]interface{}))
 
 			if len(list) > 0 {
 				data[k] = list
 			}
 		}
 	}
+	// special handling for key_usage because an empty array or array with
+	// empty string means we do not want to specify key usage constraints
+	if v, ok := d.GetOk(consts.FieldKeyUsage); ok {
+		data[consts.FieldKeyUsage] = expandStringSlice(v.([]interface{}))
+	} else {
+		// check if we are an empty array or null (not set in config)
+		val, _ := d.GetRawConfig().AsValueMap()[consts.FieldKeyUsage]
+		if !val.IsNull() {
+			// value was set as empty array in config
+			data[consts.FieldKeyUsage] = make([]string, 0)
+		}
+	}
 
 	// handle TypeBool
 	for _, k := range pkiSecretBooleanFields {
@@ -548,13 +555,10 @@ func pkiSecretBackendRoleRead(_ context.Context, d *schema.ResourceData, meta in
 	d.Set(consts.FieldBackend, backend)
 	d.Set(consts.FieldName, name)
 
+	listFields := append(pkiSecretListFields, consts.FieldKeyUsage)
 	// handle TypeList
-	for _, k := range pkiSecretListFields {
-		ifcList := secret.Data[k].([]interface{})
-		list := make([]string, 0, len(ifcList))
-		for _, ifc := range ifcList {
-			list = append(list, ifc.(string))
-		}
+	for _, k := range listFields {
+		list := expandStringSlice(secret.Data[k].([]interface{}))
 
 		if len(list) > 0 {
 			d.Set(k, list)
@@ -630,17 +634,25 @@ func pkiSecretBackendRoleUpdate(ctx context.Context, d *schema.ResourceData, met
 	data := map[string]interface{}{}
 	for _, k := range pkiSecretListFields {
 		if v, ok := d.GetOk(k); ok {
-			ifcList := v.([]interface{})
-			list := make([]string, 0, len(ifcList))
-			for _, ifc := range ifcList {
-				list = append(list, ifc.(string))
-			}
+			list := expandStringSlice(v.([]interface{}))
 
 			if len(list) > 0 {
 				data[k] = list
 			}
 		}
 	}
+	// special handling for key_usage because an empty array or array with
+	// empty string means we do not want to specify key usage constraints
+	if v, ok := d.GetOk(consts.FieldKeyUsage); ok {
+		data[consts.FieldKeyUsage] = expandStringSlice(v.([]interface{}))
+	} else {
+		// check if we are an empty array or null (not set in config)
+		val, _ := d.GetRawConfig().AsValueMap()[consts.FieldKeyUsage]
+		if !val.IsNull() {
+			// value was set as empty array in config
+			data[consts.FieldKeyUsage] = make([]string, 0)
+		}
+	}
 
 	// handle TypeBool
 	for _, k := range pkiSecretBooleanFields {

diff --git a/vault/resource_pki_secret_backend_role_test.go b/vault/resource_pki_secret_backend_role_test.go
@@ -321,6 +321,19 @@ func TestPkiSecretBackendRole_basic(t *testing.T) {
 				ImportState:       true,
 				ImportStateVerify: true,
 			},
+			{
+				Config: testPkiSecretBackendRoleConfig_basic(name, backend, 3600, 7200, `key_usage = [""]`),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "key_usage.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "key_usage.0", ""),
+				),
+			},
+			{
+				Config: testPkiSecretBackendRoleConfig_basic(name, backend, 3600, 7200, `key_usage = []`),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "key_usage.#", "0"),
+				),
+			},
 		},
 	})
 }

diff --git a/website/docs/r/pki_secret_backend_role.html.md b/website/docs/r/pki_secret_backend_role.html.md
@@ -95,7 +95,9 @@ The following arguments are supported:
 
 * `key_bits` - (Optional) The number of bits of generated keys
 
-* `key_usage` - (Optional) Specify the allowed key usage constraint on issued certificates
+* `key_usage` - (Optional) Specify the allowed key usage constraint on issued
+  certificates. Defaults to `["DigitalSignature", "KeyAgreement", "KeyEncipherment"])`.
+  To specify no default key usage constraints, set this to an empty list `[]`.
 
 * `ext_key_usage` - (Optional) Specify the allowed extended key usage constraint on issued certificates