From 93f2cdeffb38a54369ee13905978c54a6e77a158 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Antonio=20Reyes?= Date: Fri, 12 Apr 2024 12:52:47 -0400 Subject: [PATCH] return remediation measures for team membership deletion --- .../resource_pagerduty_team_membership.go | 124 +++++------- ...resource_pagerduty_team_membership_test.go | 176 ++++++++++++++++-- 2 files changed, 210 insertions(+), 90 deletions(-) diff --git a/pagerduty/resource_pagerduty_team_membership.go b/pagerduty/resource_pagerduty_team_membership.go index 1585e6e65..882741179 100644 --- a/pagerduty/resource_pagerduty_team_membership.go +++ b/pagerduty/resource_pagerduty_team_membership.go @@ -4,6 +4,7 @@ import ( "fmt" "log" "net/http" + "net/url" "strings" "time" @@ -201,23 +202,19 @@ func resourcePagerDutyTeamMembershipDelete(d *schema.ResourceData, meta interfac return err } - log.Printf("[DEBUG] Removing user: %s from team: %s", userID, teamID) - - // Extract Escalation Policies associated to the team for which the userID is - // a rule target. - epsAssociatedToTeamAndUser, err := extractEPsAssociatedToTeamAndUser(client, teamID, userID) - if err != nil { - return err - } - - epsDissociatedFromTeam, err := dissociateEPsFromTeam(client, teamID, epsAssociatedToTeamAndUser) - if err != nil { - return err - } - - // Retrying to give other resources (such as escalation policies) to delete + var isFoundErrRemovingUserFromTeam bool retryErr := retry.Retry(2*time.Minute, func() *retry.RetryError { if _, err := client.Teams.RemoveUser(teamID, userID); err != nil { + if isErrCode(err, 400) && strings.Contains(err.Error(), "User cannot be removed as they belong to an escalation policy on this team") { + if !isFoundErrRemovingUserFromTeam { + // Giving some time for the escalation policies to be removed during destroy operations. + time.Sleep(2 * time.Second) + isFoundErrRemovingUserFromTeam = true + return retry.RetryableError(err) + } + + return retry.NonRetryableError(err) + } if isErrCode(err, 400) { return retry.RetryableError(err) } @@ -226,6 +223,43 @@ func resourcePagerDutyTeamMembershipDelete(d *schema.ResourceData, meta interfac } return nil }) + if retryErr != nil && isFoundErrRemovingUserFromTeam { + // Extract Escalation Policies associated to the team for which the userID is + // a rule target. + epsAssociatedToUser, err := extractEPsAssociatedToTeamAndUser(client, teamID, userID) + if err != nil { + return fmt.Errorf("%v; %w", retryErr, err) + } + + if len(epsAssociatedToUser) > 0 { + pdURLData, err := url.Parse(epsAssociatedToUser[0]) + if err != nil { + return fmt.Errorf("%v; %w", retryErr, err) + } + + accountSubdomain := strings.Split(pdURLData.Hostname(), ".")[0] + var formatEPsList = func(eps []string) string { + var formated []string + for _, ep := range eps { + formated = append(formated, fmt.Sprintf("\t* %s", ep)) + } + return strings.Join(formated, "\n") + } + + return fmt.Errorf(`User %[1]q can't be removed from Team %[2]q as they belong to an Escalation Policy on this team. +Please take only one of the following remediation measures in order to unblock the Team Membership removal: + 1. Remove the user from the following Escalation Policies: +%[4]s + 2. Remove the Escalation Policies from the Team https://%[3]s.pagerduty.com/teams/%[2]s + +After completing one of the above given remediation options come back to continue with the destruction of Team Membership.`, + userID, + teamID, + accountSubdomain, + formatEPsList(epsAssociatedToUser), + ) + } + } if retryErr != nil { time.Sleep(2 * time.Second) return retryErr @@ -233,18 +267,13 @@ func resourcePagerDutyTeamMembershipDelete(d *schema.ResourceData, meta interfac d.SetId("") - err = associateEPsBackToTeam(client, teamID, epsDissociatedFromTeam) - if err != nil { - return err - } - return nil } func buildEPsIdsList(l []*pagerduty.EscalationPolicy) []string { eps := []string{} for _, o := range l { - eps = append(eps, o.ID) + eps = append(eps, o.HTMLURL) } return unique(eps) } @@ -288,59 +317,6 @@ func extractEPsAssociatedToTeamAndUser(c *pagerduty.Client, teamID, userID strin return epsAssociatedToTeamAndUser, nil } -func dissociateEPsFromTeam(c *pagerduty.Client, teamID string, eps []string) ([]string, error) { - epsDissociatedFromTeam := []string{} - for _, ep := range eps { - retryErr := retry.Retry(2*time.Minute, func() *retry.RetryError { - _, err := c.Teams.RemoveEscalationPolicy(teamID, ep) - if err != nil && !isErrCode(err, 404) { - time.Sleep(2 * time.Second) - return retry.RetryableError(err) - } - if err != nil && isErrCode(err, 404) { - return retry.NonRetryableError(err) - } - return nil - }) - if retryErr != nil { - if !isErrCode(retryErr, 404) { - return nil, fmt.Errorf("%w; Error while trying to dissociate Team %q from Escalation Policy %q", retryErr, teamID, ep) - } else { - // Skip Escaltion Policies not found. This happens when a destroy - // operation is requested and Escalation Policy is destroyed first. - continue - } - } - epsDissociatedFromTeam = append(epsDissociatedFromTeam, ep) - log.Printf("[DEBUG] EscalationPolicy %s removed from team %s", ep, teamID) - } - return epsDissociatedFromTeam, nil -} - -func associateEPsBackToTeam(c *pagerduty.Client, teamID string, eps []string) error { - for _, ep := range eps { - retryErr := retry.Retry(2*time.Minute, func() *retry.RetryError { - _, err := c.Teams.AddEscalationPolicy(teamID, ep) - if err != nil && !isErrCode(err, 404) { - time.Sleep(2 * time.Second) - return retry.RetryableError(err) - } - return nil - }) - if retryErr != nil { - if !isErrCode(retryErr, 404) { - return fmt.Errorf("%w; Error while trying to associate back team %q to Escalation Policy %q. Resource succesfully deleted, but some team association couldn't be completed, so you need to run \"terraform plan -refresh-only\" and again \"terraform apply/destroy\" in order to remediate the drift", retryErr, teamID, ep) - } else { - // Skip Escaltion Policies not found. This happens when a destroy - // operation is requested and Escalation Policy is destroyed first. - continue - } - } - log.Printf("[DEBUG] EscalationPolicy %s added to team %s", ep, teamID) - } - return nil -} - func isTeamMember(user *pagerduty.User, teamID string) bool { var found bool diff --git a/pagerduty/resource_pagerduty_team_membership_test.go b/pagerduty/resource_pagerduty_team_membership_test.go index 9b62d6e3c..295ac35bc 100644 --- a/pagerduty/resource_pagerduty_team_membership_test.go +++ b/pagerduty/resource_pagerduty_team_membership_test.go @@ -2,6 +2,7 @@ package pagerduty import ( "fmt" + "regexp" "testing" "github.com/hashicorp/terraform-plugin-testing/helper/acctest" @@ -81,7 +82,8 @@ func TestAccPagerDutyTeamMembership_WithRoleConsistentlyAssigned(t *testing.T) { } func TestAccPagerDutyTeamMembership_DestroyWithEscalationPolicyDependant(t *testing.T) { - user := fmt.Sprintf("tf-%s", acctest.RandString(5)) + userFoo := fmt.Sprintf("tf-%s", acctest.RandString(5)) + userBar := fmt.Sprintf("tf-%s", acctest.RandString(5)) team := fmt.Sprintf("tf-%s", acctest.RandString(5)) role := "manager" escalationPolicy := fmt.Sprintf("tf-%s", acctest.RandString(5)) @@ -92,13 +94,27 @@ func TestAccPagerDutyTeamMembership_DestroyWithEscalationPolicyDependant(t *test CheckDestroy: testAccCheckPagerDutyTeamMembershipDestroy, Steps: []resource.TestStep{ { - Config: testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependant(user, team, role, escalationPolicy), + Config: testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependant(userFoo, userBar, team, role, escalationPolicy), Check: resource.ComposeTestCheckFunc( testAccCheckPagerDutyTeamMembershipExists("pagerduty_team_membership.foo"), ), }, { - Config: testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependantUpdated(user, team, role, escalationPolicy), + // This test case is expected to fail because userFoo is a member of the + // escalation policy foo + Config: testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependantUpdated(userFoo, userBar, team, role, escalationPolicy), + ExpectError: regexp.MustCompile("User \".*\" can't be removed from Team \".*\" as they belong to an Escalation Policy on this team"), + }, + { + // This test case is expected to pass because userFoo is being removed + // from escalation policy as remediation measure to unblock the team + // membership removal + Config: testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependantAfterRemediation(userFoo, userBar, team, role, escalationPolicy), + }, + { + // This test case is expected to pass because userFoo is no longer a + // member of the escalation policy + Config: testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependantUpdated(userFoo, userBar, team, role, escalationPolicy), Check: resource.ComposeTestCheckFunc( testAccCheckPagerDutyTeamMembershipNoExists("pagerduty_team_membership.foo"), ), @@ -127,6 +143,20 @@ func TestAccPagerDutyTeamMembership_DestroyWithEscalationPolicyDependantAndMulti ), }, { + // This test case is expected to fail because userOne is a member of the + // teamOne which is associated with escalation policyOne + Config: testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependantAndMultipleTeamsUpdated(userOne, teamOne, teamTwo, role, escalationPolicyOne, escalationPolicyTwo), + ExpectError: regexp.MustCompile("User \".*\" can't be removed from Team \".*\" as they belong to an Escalation Policy on this team"), + }, + { + // This test case is expected to pass because teamOne is being removed + // from escalation policy policyOne as remediation measure to unblock the + // team membership removal + Config: testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependantAndMultipleTeamsAfterRemediation(userOne, teamOne, teamTwo, role, escalationPolicyOne, escalationPolicyTwo), + }, + { + // This test case is expected to pass because teamOne is no longer a + // associated to the escalation policyOne Config: testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependantAndMultipleTeamsUpdated(userOne, teamOne, teamTwo, role, escalationPolicyOne, escalationPolicyTwo), Check: resource.ComposeTestCheckFunc( testAccCheckPagerDutyTeamMembershipNoExists("pagerduty_team_membership.one"), @@ -265,26 +295,31 @@ resource "pagerduty_team_membership" "foo" { `, user, team, role) } -func testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependant(user, team, role, escalationPolicy string) string { +func testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependant(userFoo, userBar, team, role, escalationPolicy string) string { return fmt.Sprintf(` resource "pagerduty_user" "foo" { - name = "%[1]v" - email = "%[1]v@foo.test" + name = "%[1]s" + email = "%[1]s@foo.test" +} + +resource "pagerduty_user" "bar" { + name = "%[2]s" + email = "%[2]s@foo.test" } resource "pagerduty_team" "foo" { - name = "%[2]v" + name = "%[3]s" description = "foo" } resource "pagerduty_team_membership" "foo" { user_id = pagerduty_user.foo.id team_id = pagerduty_team.foo.id - role = "%[3]v" + role = "%[4]s" } resource "pagerduty_escalation_policy" "foo" { - name = "%s" + name = "%[5]s" num_loops = 2 teams = [pagerduty_team.foo.id] @@ -294,25 +329,34 @@ resource "pagerduty_escalation_policy" "foo" { type = "user_reference" id = pagerduty_user.foo.id } + target { + type = "user_reference" + id = pagerduty_user.bar.id + } } } -`, user, team, role, escalationPolicy) +`, userFoo, userBar, team, role, escalationPolicy) } -func testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependantUpdated(user, team, role, escalationPolicy string) string { +func testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependantUpdated(userFoo, userBar, team, role, escalationPolicy string) string { return fmt.Sprintf(` resource "pagerduty_user" "foo" { - name = "%[1]v" - email = "%[1]v@foo.test" + name = "%[1]s" + email = "%[1]s@foo.test" +} + +resource "pagerduty_user" "bar" { + name = "%[2]s" + email = "%[2]s@foo.test" } resource "pagerduty_team" "foo" { - name = "%[2]v" + name = "%[3]s" description = "foo" } resource "pagerduty_escalation_policy" "foo" { - name = "%[4]s" + name = "%[5]s" num_loops = 2 teams = [pagerduty_team.foo.id] @@ -322,9 +366,51 @@ resource "pagerduty_escalation_policy" "foo" { type = "user_reference" id = pagerduty_user.foo.id } + target { + type = "user_reference" + id = pagerduty_user.bar.id + } } } -`, user, team, role, escalationPolicy) +`, userFoo, userBar, team, role, escalationPolicy) +} + +func testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependantAfterRemediation(userFoo, userBar, team, role, escalationPolicy string) string { + return fmt.Sprintf(` +resource "pagerduty_user" "foo" { + name = "%[1]s" + email = "%[1]s@foo.test" +} + +resource "pagerduty_user" "bar" { + name = "%[2]s" + email = "%[2]s@foo.test" +} + +resource "pagerduty_team" "foo" { + name = "%[3]s" + description = "foo" +} + +resource "pagerduty_team_membership" "foo" { + user_id = pagerduty_user.foo.id + team_id = pagerduty_team.foo.id + role = "%[4]s" +} + +resource "pagerduty_escalation_policy" "foo" { + name = "%[5]s" + num_loops = 2 + teams = [pagerduty_team.foo.id] + + rule { + escalation_delay_in_minutes = 10 + target { + type = "user_reference" + id = pagerduty_user.bar.id + } + } +}`, userFoo, userBar, team, role, escalationPolicy) } func testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependantAndMultipleTeams(user, teamOne, teamTwo, role, escalationPolicyOne, escalationPolicyTwo string) string { @@ -386,6 +472,64 @@ resource "pagerduty_escalation_policy" "two" { `, user, teamOne, teamTwo, role, escalationPolicyOne, escalationPolicyTwo) } +func testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependantAndMultipleTeamsAfterRemediation(user, teamOne, teamTwo, role, escalationPolicyOne, escalationPolicyTwo string) string { + return fmt.Sprintf(` +resource "pagerduty_user" "one" { + name = "%[1]v" + email = "%[1]v@foo.test" +} + +resource "pagerduty_team" "one" { + name = "%[2]v" + description = "team_one" +} + +resource "pagerduty_team" "two" { + name = "%[3]v" + description = "team_two" +} + +resource "pagerduty_team_membership" "one" { + user_id = pagerduty_user.one.id + team_id = pagerduty_team.one.id + role = "%[4]v" +} + +resource "pagerduty_team_membership" "two" { + user_id = pagerduty_user.one.id + team_id = pagerduty_team.two.id + role = "%[4]v" +} + +resource "pagerduty_escalation_policy" "one" { + name = "%s" + num_loops = 2 + + rule { + escalation_delay_in_minutes = 10 + target { + type = "user_reference" + id = pagerduty_user.one.id + } + } +} + +resource "pagerduty_escalation_policy" "two" { + name = "%s" + num_loops = 2 + teams = [pagerduty_team.two.id] + + rule { + escalation_delay_in_minutes = 10 + target { + type = "user_reference" + id = pagerduty_user.one.id + } + } +} +`, user, teamOne, teamTwo, role, escalationPolicyOne, escalationPolicyTwo) +} + func testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependantAndMultipleTeamsUpdated(user, teamOne, teamTwo, role, escalationPolicyOne, escalationPolicyTwo string) string { return fmt.Sprintf(` resource "pagerduty_user" "one" {