From ea3d0952e4b8073addf7d5a9a35683a749c3deff Mon Sep 17 00:00:00 2001 From: Lorenzo Davis <> Date: Wed, 3 Jul 2024 16:45:50 +0400 Subject: [PATCH 1/3] Working reproduction of EP Management problem --- ...resource_pagerduty_team_membership_test.go | 220 ++++++++++++++++++ 1 file changed, 220 insertions(+) diff --git a/pagerduty/resource_pagerduty_team_membership_test.go b/pagerduty/resource_pagerduty_team_membership_test.go index cc6b26d27..39a61f64a 100644 --- a/pagerduty/resource_pagerduty_team_membership_test.go +++ b/pagerduty/resource_pagerduty_team_membership_test.go @@ -297,3 +297,223 @@ resource "pagerduty_escalation_policy" "foo" { } `, user, team, role, escalationPolicy) } + +func TestAccPagerDutyTeamMembership_basic(t *testing.T) { // from gpt and modified + user := fmt.Sprintf("tf-%s", acctest.RandString(5)) + team1 := fmt.Sprintf("tf-%s", acctest.RandString(5)) + team2 := fmt.Sprintf("tf-%s", acctest.RandString(5)) + role := "manager" + escalationPolicy1 := fmt.Sprintf("tf-%s", acctest.RandString(5)) + escalationPolicy2 := fmt.Sprintf("tf-%s", acctest.RandString(5)) + + fmt.Print("starting tests\n") + fmt.Printf("starting tests\n\n") + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckPagerDutyTeamMembershipDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependant_UnrelatedEPs(user, team1, team2, role, escalationPolicy1, escalationPolicy2), + Check: resource.ComposeTestCheckFunc( + printdebug("checkpoint1-1\n\n"), + testAccCheckPagerDutyTeamMembershipExists("pagerduty_team_membership.foo"), + printdebug("checkpoint1-2\n\n"), + + testAccCheckPagerDutyTeamMembershipExists("pagerduty_team_membership.bar"), + printdebug("checkpoint1-3\n\n"), + + resource.TestCheckResourceAttr("pagerduty_team_membership.bar", "role", "manager"), + printdebug("checkpoint1-4\n\n"), + ), + }, + { + // ResourceName: "pagerduty_team_membership.foo", + // ImportState: true, + // RefreshState: true, + Config: testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependant_UnrelatedEPsUpdated(user, team1, team2, role, escalationPolicy1, escalationPolicy2), + Check: resource.ComposeTestCheckFunc( + printdebug("checkpoint2-1\n\n"), + + testAccCheckPagerDutyTeamMembershipDestroyState("pagerduty_team_membership.foo"), + printdebug("checkpoint2-2\n\n"), + + testAccCheckPagerDutyTeamExists("pagerduty_team.foobar"), + printdebug("checkpoint2-3\n\n"), + + testAccCheckPagerDutyEscalationPolicyExists("pagerduty_escalation_policy.foo"), + printdebug("checkpoint2-4\n\n"), + + testAccCheckPagerDutyEscalationPolicyExists("pagerduty_escalation_policy.bar"), + printdebug("checkpoint2-5\n\n"), + + testAccCheckPagerDutyEscalationPolicyTeamsFieldMatches("pagerduty_escalation_policy.foo", "foo"), + printdebug("checkpoint2-6\n\n"), + + testAccCheckPagerDutyEscalationPolicyTeamsFieldMatches("pagerduty_escalation_policy.bar", "bar"), + printdebug("checkpoint2-7\n\n"), + ), + }, + }, + }) +} + +func printdebug(msg string) resource.TestCheckFunc { + return func(s *terraform.State) error { + fmt.Printf(msg) + return nil + } +} + +func testAccCheckPagerDutyEscalationPolicyTeamsFieldMatches(n, expectedTeam string) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("Not found: %s", n) + } + + teams := rs.Primary.Attributes["teams.#"] + if teams != "1" { + return fmt.Errorf("Expected teams to have 1 element, got: %s", teams) + } + + team := rs.Primary.Attributes["teams.0"] + if team != expectedTeam { + return fmt.Errorf("Expected team to be %s, got: %s", expectedTeam, team) + } + + return fmt.Errorf("dummy error") + // return nil + } +} + +func testAccCheckPagerDutyTeamMembershipDestroyState(n string) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("Not found: %s", n) + } + + if rs.Primary.ID != "" { + return fmt.Errorf("Resource still exists: %s", n) + } + return fmt.Errorf("dummy error: %s", "wow") + // return nil + } +} + +func testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependant_UnrelatedEPs(user, team1, team2, role, escalationPolicy1, escalationPolicy2 string) string { + return fmt.Sprintf(` +resource "pagerduty_user" "foo" { + name = "%[1]v" + email = "%[1]v@foo.test" +} + +resource "pagerduty_team" "foo" { + name = "%[2]v" + description = "foo" +} + +resource "pagerduty_team" "bar" { + name = "%[5]v" + description = "bar" +} + +resource "pagerduty_team_membership" "foo" { + user_id = data.pagerduty_user.foo.id + team_id = pagerduty_team.foo.id + role = "%[3]v" +} + +resource "pagerduty_team_membership" "bar" { + user_id = data.pagerduty_user.foo.id + team_id = pagerduty_team.bar.id + role = "%[3]v" +} + +resource "pagerduty_escalation_policy" "foo" { + name = "%[4]v" + num_loops = 2 + teams = [pagerduty_team.foo.id] + description = "foo" + + rule { + escalation_delay_in_minutes = 10 + target { + type = "user_reference" + id = data.pagerduty_user.foo.id + } + } +} + +resource "pagerduty_escalation_policy" "bar" { + name = "%[6]v" + num_loops = 2 + teams = [pagerduty_team.bar.id] + description = "bar" + + rule { + escalation_delay_in_minutes = 10 + target { + type = "user_reference" + id = data.pagerduty_user.foo.id + } + } +} +`, user, team1, role, escalationPolicy1, team2, escalationPolicy2) +} + +func testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependant_UnrelatedEPsUpdated(user, team1, team2, role, escalationPolicy1, escalationPolicy2 string) string { + return fmt.Sprintf(` +resource "pagerduty_user" "foo" { + name = "%[1]v" + email = "%[1]v@foo.test" +} + +resource "pagerduty_team" "foo" { + name = "%[2]v" + description = "foo" +} + +resource "pagerduty_team" "bar" { + name = "%[5]v" + description = "bar" +} + +// now the user should be on a single team +resource "pagerduty_team_membership" "bar" { + user_id = data.pagerduty_user.foo.id + team_id = pagerduty_team.bar.id + role = "%[3]v" +} + +resource "pagerduty_escalation_policy" "foo" { + name = "%[4]v" + num_loops = 2 + teams = [pagerduty_team.foo.id] + + rule { + escalation_delay_in_minutes = 10 + target { + type = "user_reference" + id = data.pagerduty_user.foo.id + } + } +} + +resource "pagerduty_escalation_policy" "bar" { + name = "%[6]v" + num_loops = 2 + description = "bar" + teams = [pagerduty_team.bar.id] + + rule { + escalation_delay_in_minutes = 10 + target { + type = "user_reference" + id = data.pagerduty_user.foo.id + } + } +} +`, user, team1, role, escalationPolicy1, team2, escalationPolicy2) +} From 4ec40c06eb2549c7e2f530d95adadfcec002072e Mon Sep 17 00:00:00 2001 From: Lorenzo Davis <> Date: Fri, 5 Jul 2024 10:02:15 +0400 Subject: [PATCH 2/3] Get updated test working, incl fixing test utility funcs --- .../resource_pagerduty_team_membership.go | 59 +++++++++++++++++- ...resource_pagerduty_team_membership_test.go | 61 ++----------------- .../resource_pagerduty_team_test.go | 12 ++-- 3 files changed, 71 insertions(+), 61 deletions(-) diff --git a/pagerduty/resource_pagerduty_team_membership.go b/pagerduty/resource_pagerduty_team_membership.go index 70456cd4e..2213b7f18 100644 --- a/pagerduty/resource_pagerduty_team_membership.go +++ b/pagerduty/resource_pagerduty_team_membership.go @@ -190,6 +190,44 @@ func resourcePagerDutyTeamMembershipUpdate(d *schema.ResourceData, meta interfac return fetchPagerDutyTeamMembershipWithRetries(d, meta, genError, 0, d.Get("role").(string)) } +func getAllEPsForTeam(client *pagerduty.Client, teamID string) ([]string, error) { + const maxLimit = 100 + var escalationPolicyIDs []string + opts := &pagerduty.ListEscalationPoliciesOptions{ + TeamIDs: []string{teamID}, + Limit: maxLimit, + Offset: 0, + } + + for { + listResp, _, err := client.EscalationPolicies.List(opts) + if err != nil { + return nil, err + } + + for _, ep := range listResp.EscalationPolicies { + escalationPolicyIDs = append(escalationPolicyIDs, ep.ID) + } + + if !listResp.More { + break + } + opts.Offset += opts.Limit + } + return escalationPolicyIDs, nil +} + +func filterEPsAssociatedToTeam(client *pagerduty.Client, teamID string, epsAssociatedToUser []string) ([]string, error) { + // take epsAssociatedToUser and return EP IDs asscociated with teamID + epIdsAssociatedWithTeam, err := getAllEPsForTeam(client, teamID) + if err != nil { + return nil, err + } + + epsInBoth := intersect(epIdsAssociatedWithTeam, epsAssociatedToUser) + return epsInBoth, nil +} + func resourcePagerDutyTeamMembershipDelete(d *schema.ResourceData, meta interface{}) error { client, err := meta.(*Config).Client() if err != nil { @@ -209,7 +247,12 @@ func resourcePagerDutyTeamMembershipDelete(d *schema.ResourceData, meta interfac return err } - epsDissociatedFromTeam, err := dissociateEPsFromTeam(client, teamID, epsAssociatedToUser) + epsAssociatedWithUserAndTeam, err := filterEPsAssociatedToTeam(client, teamID, epsAssociatedToUser) + if err != nil { + return err + } + + epsDissociatedFromTeam, err := dissociateEPsFromTeam(client, teamID, epsAssociatedWithUserAndTeam) if err != nil { return err } @@ -336,3 +379,17 @@ func isTeamMember(user *pagerduty.User, teamID string) bool { return found } + +func intersect(slice1, slice2 []string) []string { + m := make(map[string]struct{}) + for _, item := range slice1 { + m[item] = struct{}{} + } + var intersection []string + for _, item := range slice2 { + if _, found := m[item]; found { + intersection = append(intersection, item) + } + } + return intersection +} diff --git a/pagerduty/resource_pagerduty_team_membership_test.go b/pagerduty/resource_pagerduty_team_membership_test.go index 39a61f64a..c0b9ece50 100644 --- a/pagerduty/resource_pagerduty_team_membership_test.go +++ b/pagerduty/resource_pagerduty_team_membership_test.go @@ -316,55 +316,29 @@ func TestAccPagerDutyTeamMembership_basic(t *testing.T) { // from gpt and modifi { Config: testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependant_UnrelatedEPs(user, team1, team2, role, escalationPolicy1, escalationPolicy2), Check: resource.ComposeTestCheckFunc( - printdebug("checkpoint1-1\n\n"), testAccCheckPagerDutyTeamMembershipExists("pagerduty_team_membership.foo"), - printdebug("checkpoint1-2\n\n"), - testAccCheckPagerDutyTeamMembershipExists("pagerduty_team_membership.bar"), - printdebug("checkpoint1-3\n\n"), - resource.TestCheckResourceAttr("pagerduty_team_membership.bar", "role", "manager"), - printdebug("checkpoint1-4\n\n"), ), }, { - // ResourceName: "pagerduty_team_membership.foo", - // ImportState: true, - // RefreshState: true, Config: testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependant_UnrelatedEPsUpdated(user, team1, team2, role, escalationPolicy1, escalationPolicy2), Check: resource.ComposeTestCheckFunc( - printdebug("checkpoint2-1\n\n"), - - testAccCheckPagerDutyTeamMembershipDestroyState("pagerduty_team_membership.foo"), - printdebug("checkpoint2-2\n\n"), - - testAccCheckPagerDutyTeamExists("pagerduty_team.foobar"), - printdebug("checkpoint2-3\n\n"), - + testAccCheckPagerDutyTeamMembershipNoExists("pagerduty_team_membership.foo"), + testAccCheckPagerDutyTeamExists("pagerduty_team.foo"), + testAccCheckPagerDutyTeamExists("pagerduty_team.bar"), testAccCheckPagerDutyEscalationPolicyExists("pagerduty_escalation_policy.foo"), - printdebug("checkpoint2-4\n\n"), - testAccCheckPagerDutyEscalationPolicyExists("pagerduty_escalation_policy.bar"), - printdebug("checkpoint2-5\n\n"), - testAccCheckPagerDutyEscalationPolicyTeamsFieldMatches("pagerduty_escalation_policy.foo", "foo"), - printdebug("checkpoint2-6\n\n"), - testAccCheckPagerDutyEscalationPolicyTeamsFieldMatches("pagerduty_escalation_policy.bar", "bar"), - printdebug("checkpoint2-7\n\n"), ), }, }, }) } -func printdebug(msg string) resource.TestCheckFunc { - return func(s *terraform.State) error { - fmt.Printf(msg) - return nil - } -} - +// update this tomorrow +// the check should involve a call to the API and inspecting the state on the server func testAccCheckPagerDutyEscalationPolicyTeamsFieldMatches(n, expectedTeam string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -387,21 +361,6 @@ func testAccCheckPagerDutyEscalationPolicyTeamsFieldMatches(n, expectedTeam stri } } -func testAccCheckPagerDutyTeamMembershipDestroyState(n string) resource.TestCheckFunc { - return func(s *terraform.State) error { - rs, ok := s.RootModule().Resources[n] - if !ok { - return fmt.Errorf("Not found: %s", n) - } - - if rs.Primary.ID != "" { - return fmt.Errorf("Resource still exists: %s", n) - } - return fmt.Errorf("dummy error: %s", "wow") - // return nil - } -} - func testAccCheckPagerDutyTeamMembershipDestroyWithEscalationPolicyDependant_UnrelatedEPs(user, team1, team2, role, escalationPolicy1, escalationPolicy2 string) string { return fmt.Sprintf(` resource "pagerduty_user" "foo" { @@ -435,8 +394,6 @@ resource "pagerduty_escalation_policy" "foo" { name = "%[4]v" num_loops = 2 teams = [pagerduty_team.foo.id] - description = "foo" - rule { escalation_delay_in_minutes = 10 target { @@ -450,8 +407,6 @@ resource "pagerduty_escalation_policy" "bar" { name = "%[6]v" num_loops = 2 teams = [pagerduty_team.bar.id] - description = "bar" - rule { escalation_delay_in_minutes = 10 target { @@ -460,6 +415,7 @@ resource "pagerduty_escalation_policy" "bar" { } } } + `, user, team1, role, escalationPolicy1, team2, escalationPolicy2) } @@ -479,8 +435,6 @@ resource "pagerduty_team" "bar" { name = "%[5]v" description = "bar" } - -// now the user should be on a single team resource "pagerduty_team_membership" "bar" { user_id = data.pagerduty_user.foo.id team_id = pagerduty_team.bar.id @@ -491,7 +445,6 @@ resource "pagerduty_escalation_policy" "foo" { name = "%[4]v" num_loops = 2 teams = [pagerduty_team.foo.id] - rule { escalation_delay_in_minutes = 10 target { @@ -504,9 +457,7 @@ resource "pagerduty_escalation_policy" "foo" { resource "pagerduty_escalation_policy" "bar" { name = "%[6]v" num_loops = 2 - description = "bar" teams = [pagerduty_team.bar.id] - rule { escalation_delay_in_minutes = 10 target { diff --git a/pagerdutyplugin/resource_pagerduty_team_test.go b/pagerdutyplugin/resource_pagerduty_team_test.go index fd35cdfb1..7f76812db 100644 --- a/pagerdutyplugin/resource_pagerduty_team_test.go +++ b/pagerdutyplugin/resource_pagerduty_team_test.go @@ -176,12 +176,14 @@ func testAccCheckPagerDutyTeamDestroy(s *terraform.State) error { return nil } -func testAccCheckPagerDutyTeamExists(_ string) resource.TestCheckFunc { +func testAccCheckPagerDutyTeamExists(n string) resource.TestCheckFunc { return func(s *terraform.State) error { - for _, r := range s.RootModule().Resources { - ctx := context.Background() - if _, err := testAccProvider.client.GetTeamWithContext(ctx, r.Primary.ID); err != nil { - return fmt.Errorf("Received an error retrieving team %s ID: %s", err, r.Primary.ID) + client, _ := testAccProvider.Meta().(*Config).Client() + for name, r := range s.RootModule().Resources { + if name == n { + if _, _, err := client.Teams.Get(r.Primary.ID); err != nil { + return fmt.Errorf("Received an error retrieving team %s ID: %s", err, r.Primary.ID) + } } } return nil From 50f96babafd6eb9b736a57c22075dd1c775eb2bd Mon Sep 17 00:00:00 2001 From: Lorenzo Davis <> Date: Fri, 5 Jul 2024 11:44:53 +0400 Subject: [PATCH 3/3] update `testAccCheckPagerDutyEscalationPolicyTeamsFieldMatches` for correctness --- ...resource_pagerduty_team_membership_test.go | 53 +++++++++++-------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/pagerduty/resource_pagerduty_team_membership_test.go b/pagerduty/resource_pagerduty_team_membership_test.go index c0b9ece50..1c3c3f2c2 100644 --- a/pagerduty/resource_pagerduty_team_membership_test.go +++ b/pagerduty/resource_pagerduty_team_membership_test.go @@ -298,7 +298,7 @@ resource "pagerduty_escalation_policy" "foo" { `, user, team, role, escalationPolicy) } -func TestAccPagerDutyTeamMembership_basic(t *testing.T) { // from gpt and modified +func TestAccPagerDutyTeamMembership_basic(t *testing.T) { user := fmt.Sprintf("tf-%s", acctest.RandString(5)) team1 := fmt.Sprintf("tf-%s", acctest.RandString(5)) team2 := fmt.Sprintf("tf-%s", acctest.RandString(5)) @@ -306,8 +306,6 @@ func TestAccPagerDutyTeamMembership_basic(t *testing.T) { // from gpt and modifi escalationPolicy1 := fmt.Sprintf("tf-%s", acctest.RandString(5)) escalationPolicy2 := fmt.Sprintf("tf-%s", acctest.RandString(5)) - fmt.Print("starting tests\n") - fmt.Printf("starting tests\n\n") resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -329,16 +327,14 @@ func TestAccPagerDutyTeamMembership_basic(t *testing.T) { // from gpt and modifi testAccCheckPagerDutyTeamExists("pagerduty_team.bar"), testAccCheckPagerDutyEscalationPolicyExists("pagerduty_escalation_policy.foo"), testAccCheckPagerDutyEscalationPolicyExists("pagerduty_escalation_policy.bar"), - testAccCheckPagerDutyEscalationPolicyTeamsFieldMatches("pagerduty_escalation_policy.foo", "foo"), - testAccCheckPagerDutyEscalationPolicyTeamsFieldMatches("pagerduty_escalation_policy.bar", "bar"), + testAccCheckPagerDutyEscalationPolicyTeamsFieldMatches("pagerduty_escalation_policy.foo", "pagerduty_team.foo"), + testAccCheckPagerDutyEscalationPolicyTeamsFieldMatches("pagerduty_escalation_policy.bar", "pagerduty_team.bar"), ), }, }, }) } -// update this tomorrow -// the check should involve a call to the API and inspecting the state on the server func testAccCheckPagerDutyEscalationPolicyTeamsFieldMatches(n, expectedTeam string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -346,18 +342,22 @@ func testAccCheckPagerDutyEscalationPolicyTeamsFieldMatches(n, expectedTeam stri return fmt.Errorf("Not found: %s", n) } - teams := rs.Primary.Attributes["teams.#"] - if teams != "1" { - return fmt.Errorf("Expected teams to have 1 element, got: %s", teams) + client, _ := testAccProvider.Meta().(*Config).Client() + expectedTeamID, err := getTeamID(s, expectedTeam) + if err != nil { + return err } - team := rs.Primary.Attributes["teams.0"] - if team != expectedTeam { - return fmt.Errorf("Expected team to be %s, got: %s", expectedTeam, team) + // get the object from remote state -- tests for prescence + remoteResource, _, err := client.EscalationPolicies.Get(rs.Primary.ID, &pagerduty.GetEscalationPolicyOptions{}) + if err != nil { + return fmt.Errorf("%s\n", err) } - return fmt.Errorf("dummy error") - // return nil + if attachedTeam := remoteResource.Teams[0].ID; attachedTeam != expectedTeamID { + return fmt.Errorf("Mismatch detected. Expected %s, and got %s\n", expectedTeamID, attachedTeam) + } + return nil } } @@ -379,13 +379,13 @@ resource "pagerduty_team" "bar" { } resource "pagerduty_team_membership" "foo" { - user_id = data.pagerduty_user.foo.id + user_id = pagerduty_user.foo.id team_id = pagerduty_team.foo.id role = "%[3]v" } resource "pagerduty_team_membership" "bar" { - user_id = data.pagerduty_user.foo.id + user_id = pagerduty_user.foo.id team_id = pagerduty_team.bar.id role = "%[3]v" } @@ -398,7 +398,7 @@ resource "pagerduty_escalation_policy" "foo" { escalation_delay_in_minutes = 10 target { type = "user_reference" - id = data.pagerduty_user.foo.id + id = pagerduty_user.foo.id } } } @@ -411,7 +411,7 @@ resource "pagerduty_escalation_policy" "bar" { escalation_delay_in_minutes = 10 target { type = "user_reference" - id = data.pagerduty_user.foo.id + id = pagerduty_user.foo.id } } } @@ -436,7 +436,7 @@ resource "pagerduty_team" "bar" { description = "bar" } resource "pagerduty_team_membership" "bar" { - user_id = data.pagerduty_user.foo.id + user_id = pagerduty_user.foo.id team_id = pagerduty_team.bar.id role = "%[3]v" } @@ -449,7 +449,7 @@ resource "pagerduty_escalation_policy" "foo" { escalation_delay_in_minutes = 10 target { type = "user_reference" - id = data.pagerduty_user.foo.id + id = pagerduty_user.foo.id } } } @@ -462,9 +462,18 @@ resource "pagerduty_escalation_policy" "bar" { escalation_delay_in_minutes = 10 target { type = "user_reference" - id = data.pagerduty_user.foo.id + id = pagerduty_user.foo.id } } } `, user, team1, role, escalationPolicy1, team2, escalationPolicy2) } + +func getTeamID(s *terraform.State, teamName string) (string, error) { + for name, r := range s.RootModule().Resources { + if name == teamName { + return r.Primary.ID, nil + } + } + return "", fmt.Errorf("Could not find team %s\n", teamName) +}