Support both UUIDs and names in consul_acl_role.policies (#363)

The `consul_acl_role` attribute `policies` only support UUIDs like:

```terraform
resource "consul_acl_policy" "read-policy" {
  name        = "read-policy"
  rules       = "node \"\" { policy = \"read\" }"
}

resource "consul_acl_role" "read" {
  name        = "foo"

  policies = [
    consul_acl_policy.read-policy.id
  ]
}
```

This differs from the Consul API payload where the `Policies` attribute
is actually a list of objects, not a list of strings:

```json
{
  "Name": "example-role",
  "Policies": [
    {
      "ID": "783beef3-783f-f41f-7422-7087dc272765"
	}
  ]
}
```

This makes it possible to set the policies using either the ID, or the
name:

```json
{
  "Name": "example-role",
  "Policies": [
    {
      "ID": "783beef3-783f-f41f-7422-7087dc272765"
	},
	{
	  "Name: "Test"
	}
  ]
}
```

This is not supported by the `consul_acl_role` resource.

Not being able to add a policy using its name makes things harder for
users and hanging its schema to make `policies` a list of objects would
be best, but it would also break the backward compatibility of the provider
which I always try not to.

This patch makes it possible possible to use either and tries first to
fetch the corresponding policy using the ID, then using the name.

`resourceConsulACLRoleRead()` is also updated to set the correct value in
Terraform so that we don't have a perpetual diff.

consul/resource_consul_acl_role.go

@@ -7,8 +7,8 @@ import (
 	"fmt"
 
 	consulapi "github.com/hashicorp/consul/api"
+	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
-	"github.com/hashicorp/terraform-plugin-sdk/helper/validation"
 )
 
 func resourceConsulACLRole() *schema.Resource {
@@ -21,7 +21,7 @@ func resourceConsulACLRole() *schema.Resource {
 			State: schema.ImportStatePassthrough,
 		},
 
-		Description: "Starting with Consul 1.5.0, the `consul_acl_role` can be used to managed Consul ACL roles.",
+		Description: "The `consul_acl_role` can be used to manage [Consul ACL roles](https://developer.hashicorp.com/consul/docs/security/acl/acl-roles).",
 
 		Schema: map[string]*schema.Schema{
 			"name": {
@@ -38,10 +38,9 @@ func resourceConsulACLRole() *schema.Resource {
 				Type:     schema.TypeSet,
 				Optional: true,
 				Elem: &schema.Schema{
-					ValidateFunc: validation.IsUUID,
-					Type:         schema.TypeString,
+					Type: schema.TypeString,
 				},
-				Description: "The list of policies that should be applied to the role.",
+				Description: "The list of policies that should be applied to the role. Both the policy ID or its name can be used.",
 			},
 			"service_identities": {
 				Type:     schema.TypeSet,
@@ -137,12 +136,15 @@ func resourceConsulACLRole() *schema.Resource {
 }
 
 func resourceConsulACLRoleCreate(d *schema.ResourceData, meta interface{}) error {
-	client, _, wOpts := getClient(d, meta)
+	client, qOpts, wOpts := getClient(d, meta)
 	ACL := client.ACL()
-	role := getRole(d, meta)
+	role, err := getRole(d, client, qOpts)
+	if err != nil {
+		return err
+	}
 
 	name := role.Name
-	role, _, err := ACL.RoleCreate(role, wOpts)
+	role, _, err = ACL.RoleCreate(role, wOpts)
 	if err != nil {
 		return fmt.Errorf("failed to create role '%s': %s", name, err)
 	}
@@ -165,8 +167,21 @@ func resourceConsulACLRoleRead(d *schema.ResourceData, meta interface{}) error {
 	}
 
 	policies := make([]string, len(role.Policies))
+	// byName indicates which policies where set using their name by the user
+	byName := map[string]struct{}{}
+	for _, raw := range d.Get("policies").(*schema.Set).List() {
+		identifier := raw.(string)
+		policy, isID, _ := getPolicyByIdOrName(identifier, client, qOpts)
+		if policy != nil && !isID {
+			byName[identifier] = struct{}{}
+		}
+	}
 	for i, policy := range role.Policies {
-		policies[i] = policy.ID
+		if _, found := byName[policy.Name]; found {
+			policies[i] = policy.Name
+		} else {
+			policies[i] = policy.ID
+		}
 	}
 
 	serviceIdentities := make([]map[string]interface{}, len(role.ServiceIdentities))
@@ -209,13 +224,16 @@ func resourceConsulACLRoleRead(d *schema.ResourceData, meta interface{}) error {
 }
 
 func resourceConsulACLRoleUpdate(d *schema.ResourceData, meta interface{}) error {
-	client, _, wOpts := getClient(d, meta)
+	client, qOpts, wOpts := getClient(d, meta)
 	ACL := client.ACL()
-	role := getRole(d, meta)
+	role, err := getRole(d, client, qOpts)
+	if err != nil {
+		return err
+	}
 
 	role.ID = d.Id()
 
-	role, _, err := ACL.RoleUpdate(role, wOpts)
+	role, _, err = ACL.RoleUpdate(role, wOpts)
 	if err != nil {
 		return fmt.Errorf("failed to update role '%s': %s", d.Id(), err)
 	}
@@ -236,21 +254,26 @@ func resourceConsulACLRoleDelete(d *schema.ResourceData, meta interface{}) error
 	return nil
 }
 
-func getRole(d *schema.ResourceData, meta interface{}) *consulapi.ACLRole {
-	_, qOpts, _ := getClient(d, meta)
+func getRole(d *schema.ResourceData, client *consulapi.Client, qOpts *consulapi.QueryOptions) (*consulapi.ACLRole, error) {
 	roleName := d.Get("name").(string)
 	role := &consulapi.ACLRole{
 		Name:        roleName,
 		Description: d.Get("description").(string),
 		Namespace:   qOpts.Namespace,
 	}
-	policies := make([]*consulapi.ACLRolePolicyLink, 0)
+
 	for _, raw := range d.Get("policies").(*schema.Set).List() {
-		policies = append(policies, &consulapi.ACLRolePolicyLink{
-			ID: raw.(string),
-		})
+		identifier := raw.(string)
+		link, err := getACLRolePolicyLink(identifier, client, qOpts)
+		if err != nil {
+			return nil, err
+		}
+		if link == nil {
+			return nil, fmt.Errorf("failed to find policy %q", identifier)
+		}
+
+		role.Policies = append(role.Policies, link)
 	}
-	role.Policies = policies
 
 	for _, raw := range d.Get("service_identities").(*schema.Set).List() {
 		s := raw.(map[string]interface{})
@@ -298,5 +321,42 @@ func getRole(d *schema.ResourceData, meta interface{}) *consulapi.ACLRole {
 		role.TemplatedPolicies = append(role.TemplatedPolicies, templatedPolicy)
 	}
 
-	return role
+	return role, nil
+}
+
+// getPolicyByIdOrName looks for a policy in Consul first by ID, then by name if
+// it found nothing. It also returns a boolean indicating whether the identifier
+// given is the ID or the name
+func getPolicyByIdOrName(identifier string, client *consulapi.Client, qOpts *consulapi.QueryOptions) (*consulapi.ACLPolicy, bool, error) {
+	var errResult *multierror.Error
+
+	policy, _, err := client.ACL().PolicyRead(identifier, qOpts)
+	errResult = multierror.Append(errResult, err)
+	if policy != nil {
+		return policy, true, errResult.ErrorOrNil()
+	}
+
+	policy, _, err = client.ACL().PolicyReadByName(identifier, qOpts)
+	if policy != nil && err == nil {
+		// we ignore the initial error that might have happened in client.ACL().PolicyRead()
+		return policy, false, nil
+	}
+
+	errResult = multierror.Append(errResult, err)
+	return policy, false, errResult.ErrorOrNil()
+}
+
+func getACLRolePolicyLink(identifier string, client *consulapi.Client, qOpts *consulapi.QueryOptions) (*consulapi.ACLRolePolicyLink, error) {
+	policy, isID, err := getPolicyByIdOrName(identifier, client, qOpts)
+	if err != nil {
+		return nil, fmt.Errorf("failed to read policy %q: %w", identifier, err)
+	}
+	if policy == nil {
+		return nil, nil
+	}
+
+	if isID {
+		return &consulapi.ACLLink{ID: identifier}, nil
+	}
+	return &consulapi.ACLLink{Name: identifier}, nil
 }

consul/resource_consul_acl_role_test.go

@@ -5,7 +5,6 @@ package consul
 
 import (
 	"fmt"
-	"regexp"
 	"testing"
 
 	consulapi "github.com/hashicorp/consul/api"
@@ -65,16 +64,25 @@ func TestAccConsulACLRole_basic(t *testing.T) {
 					resource.TestCheckResourceAttr("consul_acl_role.test", "templated_policies.1.template_variables.#", "0"),
 				),
 			},
-			{
-				Config:      testResourceACLRoleConfigPolicyName,
-				ExpectError: regexp.MustCompile(`expected "policies.0" to be a valid UUID`),
-			},
 			{
 				Config:            testResourceACLRoleConfigUpdate,
 				ResourceName:      "consul_acl_role.test",
 				ImportState:       true,
 				ImportStateVerify: true,
 			},
+			{
+				Config: testResourceACLRoleConfigPolicyName,
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr("consul_acl_role.test", "description", ""),
+					resource.TestCheckResourceAttrSet("consul_acl_role.test", "id"),
+					resource.TestCheckResourceAttr("consul_acl_role.test", "name", "baz"),
+					resource.TestCheckResourceAttr("consul_acl_role.test", "namespace", ""),
+					resource.TestCheckResourceAttr("consul_acl_role.test", "node_identities.#", "0"),
+					resource.TestCheckResourceAttr("consul_acl_role.test", "policies.#", "1"),
+					resource.TestCheckResourceAttr("consul_acl_role.test", "policies.2198728100", "test-role"),
+					resource.TestCheckResourceAttr("consul_acl_role.test", "service_identities.#", "0"),
+				),
+			},
 		},
 	})
 }
@@ -189,9 +197,15 @@ resource "consul_acl_role" "test" {
 }`
 
 	testResourceACLRoleConfigPolicyName = `
+resource "consul_acl_policy" "test-read" {
+	name        = "test-role"
+	rules       = "node \"\" { policy = \"read\" }"
+	datacenters = [ "dc1" ]
+}
+
 resource "consul_acl_role" "test" {
 	name    = "baz"
-	policies = ["test"]
+	policies = [consul_acl_policy.test-read.name]
 }`
 
 	testResourceACLRoleNamespaceCE = `

docs/resources/acl_role.md

@@ -3,12 +3,12 @@
 page_title: "consul_acl_role Resource - terraform-provider-consul"
 subcategory: ""
 description: |-
-  Starting with Consul 1.5.0, the consul_acl_role can be used to managed Consul ACL roles.
+  The consul_acl_role can be used to manage Consul ACL roles https://developer.hashicorp.com/consul/docs/security/acl/acl-roles.
 ---
 
 # consul_acl_role (Resource)
 
-Starting with Consul 1.5.0, the `consul_acl_role` can be used to managed Consul ACL roles.
+The `consul_acl_role` can be used to manage [Consul ACL roles](https://developer.hashicorp.com/consul/docs/security/acl/acl-roles).
 
 ## Example Usage
 
@@ -46,7 +46,7 @@ resource "consul_acl_role" "read" {
 - `namespace` (String) The namespace to create the role within.
 - `node_identities` (Block List) The list of node identities that should be applied to the role. (see [below for nested schema](#nestedblock--node_identities))
 - `partition` (String) The partition the ACL role is associated with.
-- `policies` (Set of String) The list of policies that should be applied to the role.
+- `policies` (Set of String) The list of policies that should be applied to the role. Both the policy ID or its name can be used.
 - `service_identities` (Block Set) The list of service identities that should be applied to the role. (see [below for nested schema](#nestedblock--service_identities))
 - `templated_policies` (Block List) The list of templated policies that should be applied to the token. (see [below for nested schema](#nestedblock--templated_policies))
 
@@ -93,3 +93,11 @@ Optional:
 Optional:
 
 - `name` (String) The name of node, workload identity or service.
+
+## Import
+
+Import is supported using the following syntax:
+
+```shell
+terraform import consul_acl_role.read 816a195f-6cb1-2e8d-92af-3011ae706318
+```

examples/resources/consul_acl_role/import.sh

@@ -0,0 +1 @@
+terraform import consul_acl_role.read 816a195f-6cb1-2e8d-92af-3011ae706318