Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More gracefully reject colonCompoundIDs that aren't compound IDs #762

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ func fetchPagerDutyAutomationActionsActionServiceAssociation(d *schema.ResourceD
return err
}

actionID, serviceID := resourcePagerDutyParseColonCompoundID(d.Id())
actionID, serviceID, err := resourcePagerDutyParseColonCompoundID(d.Id())
if err != nil {
return err
}

return resource.Retry(30*time.Second, func() *resource.RetryError {
resp, _, err := client.AutomationActionsAction.GetAssociationToService(actionID, serviceID)
if err != nil {
Expand Down Expand Up @@ -111,7 +115,11 @@ func resourcePagerDutyAutomationActionsActionServiceAssociationDelete(d *schema.
return err
}

actionID, serviceID := resourcePagerDutyParseColonCompoundID(d.Id())
actionID, serviceID, err := resourcePagerDutyParseColonCompoundID(d.Id())
if err != nil {
return err
}

log.Printf("[INFO] Deleting PagerDuty AutomationActionsActionServiceAssociation %s", d.Id())

retryErr := resource.Retry(2*time.Minute, func() *resource.RetryError {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,11 @@ func testAccCheckPagerDutyAutomationActionsActionServiceAssociationDestroy(s *te
if r.Type != "pagerduty_automation_actions_action_service_association" {
continue
}
actionID, serviceID := resourcePagerDutyParseColonCompoundID(r.Primary.ID)
actionID, serviceID, err := resourcePagerDutyParseColonCompoundID(r.Primary.ID)
if err != nil {
return err
}

if _, _, err := client.AutomationActionsAction.GetAssociationToService(actionID, serviceID); err == nil {
return fmt.Errorf("Automation Actions Runner still exists")
}
Expand All @@ -69,7 +73,11 @@ func testAccCheckPagerDutyAutomationActionsActionServiceAssociationExists(n stri
}

client, _ := testAccProvider.Meta().(*Config).Client()
actionID, serviceID := resourcePagerDutyParseColonCompoundID(rs.Primary.ID)
actionID, serviceID, err := resourcePagerDutyParseColonCompoundID(rs.Primary.ID)
if err != nil {
return err
}

found, _, err := client.AutomationActionsAction.GetAssociationToService(actionID, serviceID)
if err != nil {
return err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,11 @@ func fetchPagerDutyAutomationActionsActionTeamAssociation(d *schema.ResourceData
return err
}

actionID, teamID := resourcePagerDutyParseColonCompoundID(d.Id())
actionID, teamID, err := resourcePagerDutyParseColonCompoundID(d.Id())
if err != nil {
return err
}

return resource.Retry(30*time.Second, func() *resource.RetryError {
resp, _, err := client.AutomationActionsAction.GetAssociationToTeam(actionID, teamID)
if err != nil {
Expand Down Expand Up @@ -115,7 +119,11 @@ func resourcePagerDutyAutomationActionsActionTeamAssociationDelete(d *schema.Res
return err
}

actionID, teamID := resourcePagerDutyParseColonCompoundID(d.Id())
actionID, teamID, err := resourcePagerDutyParseColonCompoundID(d.Id())
if err != nil {
return err
}

log.Printf("[INFO] Deleting PagerDuty AutomationActionsActionTeamAssociation %s", d.Id())

retryErr := resource.Retry(2*time.Minute, func() *resource.RetryError {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,11 @@ func testAccCheckPagerDutyAutomationActionsActionTeamAssociationDestroy(s *terra
if r.Type != "pagerduty_automation_actions_action_team_association" {
continue
}
actionID, teamID := resourcePagerDutyParseColonCompoundID(r.Primary.ID)
actionID, teamID, err := resourcePagerDutyParseColonCompoundID(r.Primary.ID)
if err != nil {
return err
}

if _, _, err := client.AutomationActionsAction.GetAssociationToTeam(actionID, teamID); err == nil {
return fmt.Errorf("Automation Actions Runner still exists")
}
Expand All @@ -66,7 +70,11 @@ func testAccCheckPagerDutyAutomationActionsActionTeamAssociationExists(n string)
}

client, _ := testAccProvider.Meta().(*Config).Client()
actionID, teamID := resourcePagerDutyParseColonCompoundID(rs.Primary.ID)
actionID, teamID, err := resourcePagerDutyParseColonCompoundID(rs.Primary.ID)
if err != nil {
return err
}

found, _, err := client.AutomationActionsAction.GetAssociationToTeam(actionID, teamID)
if err != nil {
return err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ func fetchPagerDutyAutomationActionsRunnerTeamAssociation(d *schema.ResourceData
return err
}

runnerID, teamID := resourcePagerDutyParseColonCompoundID(d.Id())
runnerID, teamID, err := resourcePagerDutyParseColonCompoundID(d.Id())
if err != nil {
return err
}

return resource.Retry(30*time.Second, func() *resource.RetryError {
resp, _, err := client.AutomationActionsRunner.GetAssociationToTeam(runnerID, teamID)
if err != nil {
Expand Down Expand Up @@ -111,7 +115,11 @@ func resourcePagerDutyAutomationActionsRunnerTeamAssociationDelete(d *schema.Res
return err
}

runnerID, teamID := resourcePagerDutyParseColonCompoundID(d.Id())
runnerID, teamID, err := resourcePagerDutyParseColonCompoundID(d.Id())
if err != nil {
return err
}

log.Printf("[INFO] Deleting PagerDuty AutomationActionsRunnerTeamAssociation %s", d.Id())

retryErr := resource.Retry(2*time.Minute, func() *resource.RetryError {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,11 @@ func testAccCheckPagerDutyAutomationActionsRunnerTeamAssociationDestroy(s *terra
if r.Type != "pagerduty_automation_actions_runner_team_association" {
continue
}
runnerID, teamID := resourcePagerDutyParseColonCompoundID(r.Primary.ID)
runnerID, teamID, err := resourcePagerDutyParseColonCompoundID(r.Primary.ID)
if err != nil {
return err
}

if _, _, err := client.AutomationActionsRunner.GetAssociationToTeam(runnerID, teamID); err == nil {
return fmt.Errorf("Automation Actions Runner Team association still exists")
}
Expand All @@ -66,7 +70,11 @@ func testAccCheckPagerDutyAutomationActionsRunnerTeamAssociationExists(n string)
}

client, _ := testAccProvider.Meta().(*Config).Client()
runnerID, teamID := resourcePagerDutyParseColonCompoundID(rs.Primary.ID)
runnerID, teamID, err := resourcePagerDutyParseColonCompoundID(rs.Primary.ID)
if err != nil {
return err
}

found, _, err := client.AutomationActionsRunner.GetAssociationToTeam(runnerID, teamID)
if err != nil {
return err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,10 @@ func resourcePagerDutyEventOrchestrationIntegrationDelete(ctx context.Context, d
}

func resourcePagerDutyEventOrchestrationIntegrationImport(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
oid, id := resourcePagerDutyParseColonCompoundID(d.Id())
oid, id, err := resourcePagerDutyParseColonCompoundID(d.Id())
if err != nil {
return []*schema.ResourceData{}, err
}

if oid == "" || id == "" {
return []*schema.ResourceData{}, fmt.Errorf("Error importing pagerduty_event_orchestration_integration. Expected import ID format: <orchestration_id>:<integration_id>")
Expand Down
11 changes: 9 additions & 2 deletions pagerduty/resource_pagerduty_team_membership.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ func fetchPagerDutyTeamMembership(d *schema.ResourceData, meta interface{}, errC
return err
}

userID, teamID := resourcePagerDutyParseColonCompoundID(d.Id())
userID, teamID, err := resourcePagerDutyParseColonCompoundID(d.Id())
if err != nil {
return err
}

log.Printf("[DEBUG] Reading user: %s from team: %s", userID, teamID)
return resource.Retry(2*time.Minute, func() *resource.RetryError {
resp, _, err := client.Teams.GetMembers(teamID, &pagerduty.GetMembersOptions{})
Expand Down Expand Up @@ -191,7 +195,10 @@ func resourcePagerDutyTeamMembershipDelete(d *schema.ResourceData, meta interfac
return err
}

userID, teamID := resourcePagerDutyParseColonCompoundID(d.Id())
userID, teamID, err := resourcePagerDutyParseColonCompoundID(d.Id())
if err != nil {
return err
}

log.Printf("[DEBUG] Removing user: %s from team: %s", userID, teamID)

Expand Down
9 changes: 7 additions & 2 deletions pagerduty/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,12 @@ func unique(s []string) []string {
return result
}

func resourcePagerDutyParseColonCompoundID(id string) (string, string) {
func resourcePagerDutyParseColonCompoundID(id string) (string, string, error) {
parts := strings.Split(id, ":")
return parts[0], parts[1]

if len(parts) < 2 {
return "", "", fmt.Errorf("%s: expected colon compound ID to have at least two components", id)
}

return parts[0], parts[1], nil
}
41 changes: 41 additions & 0 deletions pagerduty/util_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package pagerduty

import (
"strings"
"testing"
)

func TestResourcePagerDutyParseColonCompoundID(t *testing.T) {
resourceIDComponents := []string{"PABC12A", "PABC12B"}
validCompoundResourceID := strings.Join(resourceIDComponents, ":")

first, second, err := resourcePagerDutyParseColonCompoundID(validCompoundResourceID)

if err != nil {
t.Fatalf("%s: expected success while parsing invalid compound resource id: got %s", validCompoundResourceID, err)
}

for i, component := range []string{first, second} {
expectedResourceIDComponent := resourceIDComponents[i]

if expectedResourceIDComponent != component {
t.Errorf(
"%s: expected component %d of a valid compound resource ID to be %s: got %s",
validCompoundResourceID,
i+1,
expectedResourceIDComponent,
component,
)
}
}
}

func TestResourcePagerDutyParseColonCompoundIDFailsForInvalidCompoundIDs(t *testing.T) {
invalidCompoundResourceID := "PABC12APABC12B"

_, _, err := resourcePagerDutyParseColonCompoundID(invalidCompoundResourceID)

if err == nil {
t.Fatalf("%s: expected errors while parsing invalid compound resource id: got success", invalidCompoundResourceID)
}
}
Loading