Don't panic on missing role in consul_cal_role_policy_attachment (#379)

`resourceConsulACLRolePolicyAttachmentCreate()` and `resourceConsulACLRolePolicyAttachmentDelete()`
were not checking properly if the given role ID was not found in Consul.

Some functions in the Consul SDK return an error when the object is missing,
other return a `nil` object.

In the case of `RoleRead()` I was checking for the wrong thing. I checked the
rest of the ACL resources and as far as I know it, this was the only
resource impacted.

I also update the `consul_acl_policy` data source to use the new
`PolicyReadByName()` function.

consul/data_source_consul_acl_policy.go

@@ -6,7 +6,6 @@ package consul
 import (
 	"fmt"
 
-	consulapi "github.com/hashicorp/consul/api"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
 )
 
@@ -53,25 +52,13 @@ func dataSourceConsulACLPolicyRead(d *schema.ResourceData, meta interface{}) err
 	client, qOpts, _ := getClient(d, meta)
 	name := d.Get("name").(string)
 
-	var policyEntry *consulapi.ACLPolicyListEntry
-	policyEntries, _, err := client.ACL().PolicyList(qOpts)
-	if err != nil {
-		return fmt.Errorf("could not list policies: %v", err)
-	}
-	for _, pe := range policyEntries {
-		if pe.Name == name {
-			policyEntry = pe
-			break
-		}
-	}
-	if policyEntry == nil {
-		return fmt.Errorf("could not find policy '%s'", name)
-	}
-
-	policy, _, err := client.ACL().PolicyRead(policyEntry.ID, qOpts)
+	policy, _, err := client.ACL().PolicyReadByName(name, qOpts)
 	if err != nil {
 		return fmt.Errorf("could not read policy '%s': %v", name, err)
 	}
+	if policy == nil {
+		return fmt.Errorf("could not find policy %q", name)
+	}
 
 	d.SetId(policy.ID)
 

consul/data_source_consul_acl_policy_test.go

@@ -18,7 +18,7 @@ func TestAccDataACLPolicy_basic(t *testing.T) {
 		Steps: []resource.TestStep{
 			{
 				Config:      testAccDataSourceACLPolicyConfigNotFound,
-				ExpectError: regexp.MustCompile("could not find policy 'not-found'"),
+				ExpectError: regexp.MustCompile(`could not find policy "not-found"`),
 			},
 			{
 				Config: testAccDataSourceACLPolicyConfigBasic,

consul/resource_consul_acl_auth_method_test.go

@@ -135,7 +135,6 @@ func testAuthMethodCACert(client *consulapi.Client, name, v string) func(s *terr
 		if err != nil {
 			return err
 		}
-
 		if authMethod == nil {
 			return fmt.Errorf("Auth method %q does not exists", name)
 		}

consul/resource_consul_acl_role_policy_attachment.go

@@ -5,7 +5,6 @@ package consul
 
 import (
 	"fmt"
-	"strings"
 
 	consulapi "github.com/hashicorp/consul/api"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
@@ -46,7 +45,10 @@ func resourceConsulACLRolePolicyAttachmentCreate(d *schema.ResourceData, meta in
 
 	role, _, err := client.ACL().RoleRead(roleID, qOpts)
 	if err != nil {
-		return fmt.Errorf("role '%s' not found", roleID)
+		return fmt.Errorf("failed to find role %q: %w", roleID, err)
+	}
+	if role == nil {
+		return fmt.Errorf("role %q not found", roleID)
 	}
 
 	newPolicyName := d.Get("policy").(string)
@@ -84,12 +86,12 @@ func resourceConsulACLRolePolicyAttachmentRead(d *schema.ResourceData, meta inte
 
 	role, _, err := client.ACL().RoleRead(roleID, qOpts)
 	if err != nil {
-		if strings.Contains(err.Error(), "role not found") {
-			d.SetId("")
-			return nil
-		}
 		return fmt.Errorf("failed to read token '%s': %v", id, err)
 	}
+	if role == nil {
+		d.SetId("")
+		return nil
+	}
 
 	policyFound := false
 	for _, iPolicy := range role.Policies {
@@ -124,6 +126,10 @@ func resourceConsulACLRolePolicyAttachmentDelete(d *schema.ResourceData, meta in
 	if err != nil {
 		return fmt.Errorf("role '%s' not found", roleID)
 	}
+	if role == nil {
+		// If the role does not exist there is no policy attachment to remove
+		return nil
+	}
 
 	for i, iPolicy := range role.Policies {
 		if iPolicy.Name == policyName {

consul/resource_consul_acl_role_policy_attachment_test.go

@@ -5,6 +5,7 @@ package consul
 
 import (
 	"fmt"
+	"regexp"
 	"testing"
 
 	consulapi "github.com/hashicorp/consul/api"
@@ -102,11 +103,16 @@ func TestAccConsulACLRolePolicyAttachment_basic(t *testing.T) {
 				ImportState:       true,
 				ImportStateVerify: true,
 			},
+			{
+				Config:      testResourceACLRolePolicyAttachmentConfigMissingRole,
+				ExpectError: regexp.MustCompile(`role "80ab84e7-eb3a-4dfe-8122-a7b680619100" not found`),
+			},
 		},
 	})
 }
 
-const testResourceACLRolePolicyAttachmentConfigBasic = `
+const (
+	testResourceACLRolePolicyAttachmentConfigBasic = `
 resource "consul_acl_policy" "test_policy" {
 	name = "test-attachment"
 	rules = "node \"\" { policy = \"read\" }"
@@ -127,7 +133,7 @@ resource "consul_acl_role_policy_attachment" "test" {
 }
 `
 
-const testResourceACLRolePolicyAttachmentConfigUpdate = `
+	testResourceACLRolePolicyAttachmentConfigUpdate = `
 // Using another resource to force the update of consul_acl_role
 resource "consul_acl_policy" "test2" {
 	name = "test2"
@@ -147,3 +153,24 @@ resource "consul_acl_role_policy_attachment" "test" {
     role_id = consul_acl_role.test_role.id
     policy  = consul_acl_policy.test2.name
 }`
+
+	testResourceACLRolePolicyAttachmentConfigMissingRole = `
+resource "consul_acl_policy" "test2" {
+	name = "test2"
+	rules = "node \"\" { policy = \"read\" }"
+	datacenters = [ "dc1" ]
+}
+
+resource "consul_acl_role" "test_role" {
+    name = "test"
+
+    lifecycle {
+		ignore_changes = ["policies"]
+	}
+}
+
+resource "consul_acl_role_policy_attachment" "test" {
+    role_id = "80ab84e7-eb3a-4dfe-8122-a7b680619100"
+    policy  = consul_acl_policy.test2.name
+}`
+)