Allow recreation of recently deleted project and org custom roles (#1681)

* undelete-update recently soft-deleted custom roles

* remove my TODO statements

* check values on soft-delete-recreate for custom role tests

* final fixes to make sure delete works; return read() when updating to 'create'

* check for non-404 errors for custom role get

* add warnings to custom roles docs

google/resource_google_organization_iam_custom_role.go

@@ -50,9 +50,10 @@ func resourceGoogleOrganizationIamCustomRole() *schema.Resource {
 				Optional: true,
 			},
 			"deleted": {
-				Type:     schema.TypeBool,
-				Optional: true,
-				Default:  false,
+				Type:       schema.TypeBool,
+				Optional:   true,
+				Default:    false,
+				Deprecated: `deleted will be converted to a computed-only field soon - if you want to delete this role, please use destroy`,
 			},
 		},
 	}
@@ -62,25 +63,50 @@ func resourceGoogleOrganizationIamCustomRoleCreate(d *schema.ResourceData, meta
 	config := meta.(*Config)
 
 	if d.Get("deleted").(bool) {
-		return fmt.Errorf("Cannot create a custom organization role with a deleted state. `deleted` field should be false.")
+		return fmt.Errorf("cannot create a custom organization role with a deleted state. `deleted` field should be false.")
 	}
 
-	role, err := config.clientIAM.Organizations.Roles.Create("organizations/"+d.Get("org_id").(string), &iam.CreateRoleRequest{
-		RoleId: d.Get("role_id").(string),
-		Role: &iam.Role{
-			Title:               d.Get("title").(string),
-			Description:         d.Get("description").(string),
-			Stage:               d.Get("stage").(string),
-			IncludedPermissions: convertStringSet(d.Get("permissions").(*schema.Set)),
-		},
-	}).Do()
+	org := d.Get("org_id").(string)
+	roleId := fmt.Sprintf("organizations/%s/roles/%s", org, d.Get("role_id").(string))
+	orgId := fmt.Sprintf("organizations/%s", org)
 
-	if err != nil {
-		return fmt.Errorf("Error creating the custom organization role %s: %s", d.Get("title").(string), err)
+	// Look for role with given ID.
+	// If it exists in deleted state, update to match "created" role state
+	// If it exists and and is enabled, return error - we should not try to recreate.
+	r, err := config.clientIAM.Organizations.Roles.Get(roleId).Do()
+	if err == nil {
+		if r.Deleted {
+			// This role was soft-deleted; update to match new state.
+			d.SetId(r.Name)
+			if err := resourceGoogleOrganizationIamCustomRoleUpdate(d, meta); err != nil {
+				// If update failed, make sure it wasn't actually added to state.
+				d.SetId("")
+				return err
+			}
+		} else {
+			// If a role with same name exists and is enabled, just return error
+			return fmt.Errorf("Custom project role %s already exists and must be imported", roleId)
+		}
+	} else if err := handleNotFoundError(err, d, fmt.Sprintf("Custom Organization Role %q", roleId)); err == nil {
+		// If no role was found, actually create a new role.
+		role, err := config.clientIAM.Organizations.Roles.Create(orgId, &iam.CreateRoleRequest{
+			RoleId: d.Get("role_id").(string),
+			Role: &iam.Role{
+				Title:               d.Get("title").(string),
+				Description:         d.Get("description").(string),
+				Stage:               d.Get("stage").(string),
+				IncludedPermissions: convertStringSet(d.Get("permissions").(*schema.Set)),
+			},
+		}).Do()
+		if err != nil {
+			return fmt.Errorf("Error creating the custom organization role %s: %s", d.Get("title").(string), err)
+		}
+
+		d.SetId(role.Name)
+	} else {
+		return fmt.Errorf("Unable to verify whether custom org role %s already exists and must be undeleted: %v", roleId, err)
 	}
 
-	d.SetId(role.Name)
-
 	return resourceGoogleOrganizationIamCustomRoleRead(d, meta)
 }
 
@@ -113,19 +139,51 @@ func resourceGoogleOrganizationIamCustomRoleUpdate(d *schema.ResourceData, meta
 
 	d.Partial(true)
 
-	if d.HasChange("deleted") {
-		if d.Get("deleted").(bool) {
+	if d.Get("deleted").(bool) {
+		if d.HasChange("deleted") {
+			// If other fields were changed, we need to update those first and then delete.
+			// If we don't update, we will get diffs from re-apply
+			// If we delete and then try to update, we will get an error.
+			if err := resourceGoogleOrganizationIamCustomRoleUpdateNonDeletedFields(d, meta); err != nil {
+				return err
+			}
+
 			if err := resourceGoogleOrganizationIamCustomRoleDelete(d, meta); err != nil {
 				return err
 			}
+
+			d.SetPartial("deleted")
+			d.Partial(false)
+			return nil
 		} else {
-			if err := resourceGoogleOrganizationIamCustomRoleUndelete(d, meta); err != nil {
-				return err
-			}
+			return fmt.Errorf("cannot make changes to deleted custom organization role %s", d.Id())
+		}
+	}
+
+	// We want to update the role to some undeleted state.
+	// Make sure the role with given ID exists and is un-deleted before patching.
+	r, err := config.clientIAM.Organizations.Roles.Get(d.Id()).Do()
+	if err != nil {
+		return fmt.Errorf("unable to find custom project role %s to update: %v", d.Id(), err)
+	}
+	if r.Deleted {
+		if err := resourceGoogleOrganizationIamCustomRoleUndelete(d, meta); err != nil {
+			return err
 		}
 		d.SetPartial("deleted")
 	}
 
+	if err := resourceGoogleOrganizationIamCustomRoleUpdateNonDeletedFields(d, meta); err != nil {
+		return err
+	}
+	d.Partial(false)
+
+	return nil
+}
+
+func resourceGoogleOrganizationIamCustomRoleUpdateNonDeletedFields(d *schema.ResourceData, meta interface{}) error {
+	config := meta.(*Config)
+
 	if d.HasChange("title") || d.HasChange("description") || d.HasChange("stage") || d.HasChange("permissions") {
 		_, err := config.clientIAM.Organizations.Roles.Patch(d.Id(), &iam.Role{
 			Title:               d.Get("title").(string),
@@ -143,15 +201,19 @@ func resourceGoogleOrganizationIamCustomRoleUpdate(d *schema.ResourceData, meta
 		d.SetPartial("permissions")
 	}
 
-	d.Partial(false)
-
 	return nil
 }
 
 func resourceGoogleOrganizationIamCustomRoleDelete(d *schema.ResourceData, meta interface{}) error {
 	config := meta.(*Config)
 
-	_, err := config.clientIAM.Organizations.Roles.Delete(d.Id()).Do()
+	r, err := config.clientIAM.Organizations.Roles.Get(d.Id()).Do()
+	if err == nil && r != nil && r.Deleted && d.Get("deleted").(bool) {
+		// This role has already been deleted, don't try again.
+		return nil
+	}
+
+	_, err = config.clientIAM.Organizations.Roles.Delete(d.Id()).Do()
 	if err != nil {
 		return fmt.Errorf("Error deleting the custom organization role %s: %s", d.Get("title").(string), err)
 	}

google/resource_google_organization_iam_custom_role_test.go

@@ -78,6 +78,45 @@ func TestAccOrganizationIamCustomRole_undelete(t *testing.T) {
 	})
 }
 
+func TestAccOrganizationIamCustomRole_createAfterDestroy(t *testing.T) {
+	t.Parallel()
+
+	org := getTestOrgFromEnv(t)
+	roleId := "tfIamCustomRole" + acctest.RandString(10)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckGoogleOrganizationIamCustomRoleDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccCheckGoogleOrganizationIamCustomRole_basic(org, roleId),
+				Check: testAccCheckGoogleOrganizationIamCustomRole(
+					"google_organization_iam_custom_role.foo",
+					"My Custom Role",
+					"foo",
+					"GA",
+					[]string{"resourcemanager.projects.list"}),
+			},
+			// Destroy resources
+			{
+				Config:  " ",
+				Destroy: true,
+			},
+			// Re-create with no existing state
+			{
+				Config: testAccCheckGoogleOrganizationIamCustomRole_basic(org, roleId),
+				Check: testAccCheckGoogleOrganizationIamCustomRole(
+					"google_organization_iam_custom_role.foo",
+					"My Custom Role",
+					"foo",
+					"GA",
+					[]string{"resourcemanager.projects.list"}),
+			},
+		},
+	})
+}
+
 func testAccCheckGoogleOrganizationIamCustomRoleDestroy(s *terraform.State) error {
 	config := testAccProvider.Meta().(*Config)
 

google/resource_google_project_iam_custom_role.go

@@ -71,22 +71,41 @@ func resourceGoogleProjectIamCustomRoleCreate(d *schema.ResourceData, meta inter
 		return fmt.Errorf("Cannot create a custom project role with a deleted state. `deleted` field should be false.")
 	}
 
-	role, err := config.clientIAM.Projects.Roles.Create("projects/"+project, &iam.CreateRoleRequest{
-		RoleId: d.Get("role_id").(string),
-		Role: &iam.Role{
-			Title:               d.Get("title").(string),
-			Description:         d.Get("description").(string),
-			Stage:               d.Get("stage").(string),
-			IncludedPermissions: convertStringSet(d.Get("permissions").(*schema.Set)),
-		},
-	}).Do()
+	roleId := fmt.Sprintf("projects/%s/roles/%s", project, d.Get("role_id").(string))
+	r, err := config.clientIAM.Projects.Roles.Get(roleId).Do()
+	if err == nil {
+		if r.Deleted {
+			// This role was soft-deleted; update to match new state.
+			d.SetId(r.Name)
+			if err := resourceGoogleProjectIamCustomRoleUpdate(d, meta); err != nil {
+				// If update failed, make sure it wasn't actually added to state.
+				d.SetId("")
+				return err
+			}
+		} else {
+			// If a role with same name exists and is enabled, just return error
+			return fmt.Errorf("Custom project role %s already exists and must be imported", roleId)
+		}
+	} else if err := handleNotFoundError(err, d, fmt.Sprintf("Custom Project Role %q", roleId)); err == nil {
+		// If no role is found, actually create a new role.
+		role, err := config.clientIAM.Projects.Roles.Create("projects/"+project, &iam.CreateRoleRequest{
+			RoleId: d.Get("role_id").(string),
+			Role: &iam.Role{
+				Title:               d.Get("title").(string),
+				Description:         d.Get("description").(string),
+				Stage:               d.Get("stage").(string),
+				IncludedPermissions: convertStringSet(d.Get("permissions").(*schema.Set)),
+			},
+		}).Do()
+		if err != nil {
+			return fmt.Errorf("Error creating the custom project role %s: %v", roleId, err)
+		}
 
-	if err != nil {
-		return fmt.Errorf("Error creating the custom project role %s: %s", d.Get("title").(string), err)
+		d.SetId(role.Name)
+	} else {
+		return fmt.Errorf("Unable to verify whether custom project role %s already exists and must be undeleted: %v", roleId, err)
 	}
 
-	d.SetId(role.Name)
-
 	return resourceGoogleProjectIamCustomRoleRead(d, meta)
 }
 
@@ -119,19 +138,51 @@ func resourceGoogleProjectIamCustomRoleUpdate(d *schema.ResourceData, meta inter
 
 	d.Partial(true)
 
-	if d.HasChange("deleted") {
-		if d.Get("deleted").(bool) {
+	if d.Get("deleted").(bool) {
+		if d.HasChange("deleted") {
+			// If other fields were changed, we need to update those first and then delete.
+			// If we don't update, we will get diffs from re-apply
+			// If we delete and then try to update, we will get an error.
+			if err := resourceGoogleProjectIamCustomRoleUpdateNonDeletedFields(d, meta); err != nil {
+				return err
+			}
 			if err := resourceGoogleProjectIamCustomRoleDelete(d, meta); err != nil {
 				return err
 			}
+
+			d.SetPartial("deleted")
+			d.Partial(false)
+			return nil
 		} else {
-			if err := resourceGoogleProjectIamCustomRoleUndelete(d, meta); err != nil {
-				return err
-			}
+			return fmt.Errorf("cannot make changes to deleted custom project role %s", d.Id())
+		}
+	}
+
+	// We want to update the role to some undeleted state.
+	// Make sure the role with given ID exists and is un-deleted before patching.
+	r, err := config.clientIAM.Projects.Roles.Get(d.Id()).Do()
+	if err != nil {
+		return fmt.Errorf("unable to find custom project role %s to update: %v", d.Id(), err)
+	}
+	if r.Deleted {
+		// Undelete if deleted previously
+		if err := resourceGoogleProjectIamCustomRoleUndelete(d, meta); err != nil {
+			return err
 		}
 		d.SetPartial("deleted")
 	}
 
+	if err := resourceGoogleProjectIamCustomRoleUpdateNonDeletedFields(d, meta); err != nil {
+		return err
+	}
+	d.Partial(false)
+
+	return nil
+}
+
+func resourceGoogleProjectIamCustomRoleUpdateNonDeletedFields(d *schema.ResourceData, meta interface{}) error {
+	config := meta.(*Config)
+
 	if d.HasChange("title") || d.HasChange("description") || d.HasChange("stage") || d.HasChange("permissions") {
 		_, err := config.clientIAM.Projects.Roles.Patch(d.Id(), &iam.Role{
 			Title:               d.Get("title").(string),
@@ -148,9 +199,6 @@ func resourceGoogleProjectIamCustomRoleUpdate(d *schema.ResourceData, meta inter
 		d.SetPartial("stage")
 		d.SetPartial("permissions")
 	}
-
-	d.Partial(false)
-
 	return nil
 }
 

google/resource_google_project_iam_custom_role_test.go

@@ -71,6 +71,43 @@ func TestAccProjectIamCustomRole_undelete(t *testing.T) {
 	})
 }
 
+func TestAccProjectIamCustomRole_createAfterDestroy(t *testing.T) {
+	t.Parallel()
+
+	roleId := "tfIamCustomRole" + acctest.RandString(10)
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckGoogleProjectIamCustomRoleDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccCheckGoogleProjectIamCustomRole_basic(roleId),
+				Check: testAccCheckGoogleProjectIamCustomRole(
+					"google_project_iam_custom_role.foo",
+					"My Custom Role",
+					"foo",
+					"GA",
+					[]string{"iam.roles.list"}),
+			},
+			// Destroy resources
+			{
+				Config:  " ",
+				Destroy: true,
+			},
+			// Re-create with no existing state
+			{
+				Config: testAccCheckGoogleProjectIamCustomRole_basic(roleId),
+				Check: testAccCheckGoogleProjectIamCustomRole(
+					"google_project_iam_custom_role.foo",
+					"My Custom Role",
+					"foo",
+					"GA",
+					[]string{"iam.roles.list"}),
+			},
+		},
+	})
+}
+
 func testAccCheckGoogleProjectIamCustomRoleDestroy(s *terraform.State) error {
 	config := testAccProvider.Meta().(*Config)
 

website/docs/r/google_organization_iam_custom_role.html.markdown

@@ -13,6 +13,13 @@ Allows management of a customized Cloud IAM organization role. For more informat
 and
 [API](https://cloud.google.com/iam/reference/rest/v1/organizations.roles).
 
+~> **Warning:** Note that custom roles in GCP have the concept of a soft-delete. There are two issues that may arise
+ from this and how roles are propagated. 1) creating a role may involve undeleting and then updating a role with the
+ same name, possibly causing confusing behavior between undelete and update. 2) A deleted role is permanently deleted
+ after 7 days, but it can take up to 30 more days (i.e. between 7 and 37 days after deletion) before the role name is
+ made available again. This means a deleted role that has been deleted for more than 7 days cannot be changed at all
+ by Terraform, and new roles cannot share that name.
+ 
 ## Example Usage
 
 This snippet creates a customized IAM organization role.

website/docs/r/google_project_iam_custom_role.html.markdown

@@ -13,6 +13,13 @@ Allows management of a customized Cloud IAM project role. For more information s
 and
 [API](https://cloud.google.com/iam/reference/rest/v1/projects.roles).
 
+~> **Warning:** Note that custom roles in GCP have the concept of a soft-delete. There are two issues that may arise
+ from this and how roles are propagated. 1) creating a role may involve undeleting and then updating a role with the
+ same name, possibly causing confusing behavior between undelete and update. 2) A deleted role is permanently deleted
+ after 7 days, but it can take up to 30 more days (i.e. between 7 and 37 days after deletion) before the role name is
+ made available again. This means a deleted role that has been deleted for more than 7 days cannot be changed at all
+ by Terraform, and new roles cannot share that name.
+
 ## Example Usage
 
 This snippet creates a customized IAM role.
@@ -45,8 +52,6 @@ The following arguments are supported:
 
 * `description` - (Optional) A human-readable description for the role.
 
-* `deleted` - (Optional) The current deleted state of the role. Defaults to `false`.
-
 ## Import
 
 Customized IAM project role can be imported using their URI, e.g.