Skip to content

Commit

Permalink
Merge pull request #301 from stmcallister/rule-position-zero
Browse files Browse the repository at this point in the history 
Fixed Bug in Event Rule Positioning
  • Loading branch information
Scott McAllister authored Feb 10, 2021
2 parents a95364f + a56b5cf commit 0c770f5
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 26 deletions.
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require (
cloud.google.com/go v0.71.0 // indirect
github.com/google/go-querystring v1.0.0 // indirect
github.com/hashicorp/terraform-plugin-sdk v1.7.0
github.com/heimweh/go-pagerduty v0.0.0-20210208230541-602e6af0197d
github.com/heimweh/go-pagerduty v0.0.0-20210209211114-6eef07a31388
golang.org/x/tools v0.0.0-20201110124207-079ba7bd75cd // indirect
google.golang.org/api v0.35.0 // indirect
google.golang.org/genproto v0.0.0-20201109203340-2640f1f9cdfb // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,8 @@ github.com/heimweh/go-pagerduty v0.0.0-20210204180239-b6fd4689b860 h1:3Su9c7I3Fq
github.com/heimweh/go-pagerduty v0.0.0-20210204180239-b6fd4689b860/go.mod h1:6+bccpjQ/PM8uQY9m8avM4MJea+3vo3ta9r8kGQ4XFY=
github.com/heimweh/go-pagerduty v0.0.0-20210208230541-602e6af0197d h1:Cu1SpAcafU1JNnMbX7rfMSZ1x0FdK8oEB7p85YE68h0=
github.com/heimweh/go-pagerduty v0.0.0-20210208230541-602e6af0197d/go.mod h1:6+bccpjQ/PM8uQY9m8avM4MJea+3vo3ta9r8kGQ4XFY=
github.com/heimweh/go-pagerduty v0.0.0-20210209211114-6eef07a31388 h1:g9ukOOud162IdWcssRS2aqoKjSRISm4LLPsQL3QFr9w=
github.com/heimweh/go-pagerduty v0.0.0-20210209211114-6eef07a31388/go.mod h1:6+bccpjQ/PM8uQY9m8avM4MJea+3vo3ta9r8kGQ4XFY=
github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
github.com/jmespath/go-jmespath v0.0.0-20160202185014-0b12d6b521d8/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k=
Expand Down
17 changes: 10 additions & 7 deletions pagerduty/resource_pagerduty_ruleset_rule.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,9 +317,10 @@ func buildRulesetRuleStruct(d *schema.ResourceData) *pagerduty.RulesetRule {
if attr, ok := d.GetOk("time_frame"); ok {
rule.TimeFrame = expandTimeFrame(attr.([]interface{}))
}
if attr, ok := d.GetOk("position"); ok {
rule.Position = attr.(int)
}

pos := d.Get("position").(int)
rule.Position = &pos

if attr, ok := d.GetOk("disabled"); ok {
rule.Disabled = attr.(bool)
}
Expand Down Expand Up @@ -733,7 +734,9 @@ func resourcePagerDutyRulesetRuleCreate(d *schema.ResourceData, meta interface{}
return resource.RetryableError(err)
} else if rule != nil {
d.SetId(rule.ID)
if rule.Position != d.Get("position").(int) {
// Verifying the position that was defined in terraform is the same position set in PagerDuty
pos := d.Get("position").(int)
if *rule.Position != pos {
if err := resourcePagerDutyRulesetRuleUpdate(d, meta); err != nil {
return resource.NonRetryableError(err)
}
Expand Down Expand Up @@ -790,9 +793,9 @@ func resourcePagerDutyRulesetRuleUpdate(d *schema.ResourceData, meta interface{}
retryErr := resource.Retry(30*time.Second, func() *resource.RetryError {
if updatedRule, _, err := client.Rulesets.UpdateRule(rulesetID, d.Id(), rule); err != nil {
return resource.RetryableError(err)
} else if updatedRule.Position != rule.Position {
log.Printf("[INFO] PagerDuty ruleset rule %s position %d needs to be %d", updatedRule.ID, updatedRule.Position, rule.Position)
return resource.RetryableError(fmt.Errorf("Error updating ruleset rule %s position %d needs to be %d", updatedRule.ID, updatedRule.Position, rule.Position))
} else if rule.Position != nil && *updatedRule.Position != *rule.Position {
log.Printf("[INFO] PagerDuty ruleset rule %s position %d needs to be %d", updatedRule.ID, *updatedRule.Position, *rule.Position)
return resource.RetryableError(fmt.Errorf("Error updating ruleset rule %s position %d needs to be %d", updatedRule.ID, *updatedRule.Position, *rule.Position))
}
return nil
})
Expand Down
31 changes: 28 additions & 3 deletions pagerduty/resource_pagerduty_ruleset_rule_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,21 +76,24 @@ func TestAccPagerDutyRulesetRule_MultipleRules(t *testing.T) {
team := fmt.Sprintf("tf-%s", acctest.RandString(5))
rule1 := fmt.Sprintf("tf-%s", acctest.RandString(5))
rule2 := fmt.Sprintf("tf-%s", acctest.RandString(5))
rule3 := fmt.Sprintf("tf-%s", acctest.RandString(5))

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckPagerDutyRulesetRuleDestroy,
Steps: []resource.TestStep{
{
Config: testAccCheckPagerDutyRulesetRuleConfigMultipleRules(team, ruleset, rule1, rule2),
Config: testAccCheckPagerDutyRulesetRuleConfigMultipleRules(team, ruleset, rule1, rule2, rule3),
Check: resource.ComposeTestCheckFunc(
testAccCheckPagerDutyRulesetRuleExists("pagerduty_ruleset_rule.foo"),
testAccCheckPagerDutyRulesetRuleExists("pagerduty_ruleset_rule.bar"),
resource.TestCheckResourceAttr(
"pagerduty_ruleset_rule.foo", "position", "0"),
resource.TestCheckResourceAttr(
"pagerduty_ruleset_rule.bar", "position", "1"),
resource.TestCheckResourceAttr(
"pagerduty_ruleset_rule.baz", "position", "2"),
resource.TestCheckResourceAttr(
"pagerduty_ruleset_rule.foo", "disabled", "false"),
resource.TestCheckResourceAttr(
Expand All @@ -107,6 +110,8 @@ func TestAccPagerDutyRulesetRule_MultipleRules(t *testing.T) {
"pagerduty_ruleset_rule.foo", "actions.0.annotate.0.value", rule1),
resource.TestCheckResourceAttr(
"pagerduty_ruleset_rule.bar", "actions.0.annotate.0.value", rule2),
resource.TestCheckResourceAttr(
"pagerduty_ruleset_rule.baz", "actions.0.annotate.0.value", rule3),
),
},
},
Expand Down Expand Up @@ -283,7 +288,7 @@ resource "pagerduty_ruleset_rule" "foo" {
`, team, ruleset, rule)
}

func testAccCheckPagerDutyRulesetRuleConfigMultipleRules(team, ruleset, rule1, rule2 string) string {
func testAccCheckPagerDutyRulesetRuleConfigMultipleRules(team, ruleset, rule1, rule2, rule3 string) string {
return fmt.Sprintf(`
resource "pagerduty_team" "foo" {
name = "%s"
Expand Down Expand Up @@ -355,5 +360,25 @@ resource "pagerduty_ruleset_rule" "bar" {
}
}
}
`, team, ruleset, rule1, rule2)
resource "pagerduty_ruleset_rule" "baz" {
ruleset = pagerduty_ruleset.foo.id
position = 2
disabled = true
conditions {
operator = "and"
subconditions {
operator = "contains"
parameter {
value = "slow database connection"
path = "summary"
}
}
}
actions {
annotate {
value = "%s"
}
}
}
`, team, ruleset, rule1, rule2, rule3)
}
18 changes: 11 additions & 7 deletions pagerduty/resource_pagerduty_service_event_rule.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,9 +308,10 @@ func buildServiceEventRuleStruct(d *schema.ResourceData) *pagerduty.ServiceEvent
if attr, ok := d.GetOk("variable"); ok {
rule.Variables = expandRuleVariables(attr.([]interface{}))
}
if attr, ok := d.GetOk("position"); ok {
rule.Position = attr.(int)
}

pos := d.Get("position").(int)
rule.Position = &pos

if attr, ok := d.GetOk("disabled"); ok {
rule.Disabled = attr.(bool)
}
Expand All @@ -329,7 +330,9 @@ func resourcePagerDutyServiceEventRuleCreate(d *schema.ResourceData, meta interf
return resource.RetryableError(err)
} else if rule != nil {
d.SetId(rule.ID)
if rule.Position != d.Get("position").(int) {
// Verifying the position that was defined in terraform is the same position set in PagerDuty
pos := d.Get("position").(int)
if *rule.Position != pos {
if err := resourcePagerDutyServiceEventRuleUpdate(d, meta); err != nil {
return resource.NonRetryableError(err)
}
Expand Down Expand Up @@ -386,10 +389,11 @@ func resourcePagerDutyServiceEventRuleUpdate(d *schema.ResourceData, meta interf
retryErr := resource.Retry(30*time.Second, func() *resource.RetryError {
if updatedRule, _, err := client.Services.UpdateEventRule(serviceID, d.Id(), rule); err != nil {
return resource.RetryableError(err)
} else if updatedRule.Position != rule.Position {
log.Printf("[INFO] Service Event Rule %s position %d needs to be %d", updatedRule.ID, updatedRule.Position, rule.Position)
return resource.RetryableError(fmt.Errorf("Error updating service event rule %s position %d needs to be %d", updatedRule.ID, updatedRule.Position, rule.Position))
} else if rule.Position != nil && *updatedRule.Position != *rule.Position {
log.Printf("[INFO] Service Event Rule %s position %v needs to be %v", updatedRule.ID, *updatedRule.Position, *rule.Position)
return resource.RetryableError(fmt.Errorf("Error updating service event rule %s position %d needs to be %d", updatedRule.ID, *updatedRule.Position, *rule.Position))
}

return nil
})
if retryErr != nil {
Expand Down
32 changes: 28 additions & 4 deletions pagerduty/resource_pagerduty_service_event_rule_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,21 +78,24 @@ func TestAccPagerDutyServiceEventRule_MultipleRules(t *testing.T) {
service := fmt.Sprintf("tf-%s", acctest.RandString(5))
rule1 := fmt.Sprintf("tf-%s", acctest.RandString(5))
rule2 := fmt.Sprintf("tf-%s", acctest.RandString(5))
rule3 := fmt.Sprintf("tf-%s", acctest.RandString(5))

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckPagerDutyServiceEventRuleDestroy,
Steps: []resource.TestStep{
{
Config: testAccCheckPagerDutyServiceEventRuleConfigMultipleRules(username, email, escalationPolicy, service, rule1, rule2),
Config: testAccCheckPagerDutyServiceEventRuleConfigMultipleRules(username, email, escalationPolicy, service, rule1, rule2, rule3),
Check: resource.ComposeTestCheckFunc(
testAccCheckPagerDutyServiceEventRuleExists("pagerduty_service_event_rule.foo"),
testAccCheckPagerDutyServiceEventRuleExists("pagerduty_service_event_rule.bar"),
resource.TestCheckResourceAttr(
"pagerduty_service_event_rule.foo", "position", "0"),
resource.TestCheckResourceAttr(
"pagerduty_service_event_rule.bar", "position", "1"),
resource.TestCheckResourceAttr(
"pagerduty_service_event_rule.baz", "position", "2"),
resource.TestCheckResourceAttr(
"pagerduty_service_event_rule.foo", "disabled", "true"),
resource.TestCheckResourceAttr(
Expand All @@ -109,6 +112,8 @@ func TestAccPagerDutyServiceEventRule_MultipleRules(t *testing.T) {
"pagerduty_service_event_rule.foo", "actions.0.annotate.0.value", rule1),
resource.TestCheckResourceAttr(
"pagerduty_service_event_rule.bar", "actions.0.annotate.0.value", rule2),
resource.TestCheckResourceAttr(
"pagerduty_service_event_rule.baz", "actions.0.annotate.0.value", rule3),
),
},
},
Expand Down Expand Up @@ -290,7 +295,7 @@ resource "pagerduty_service_event_rule" "foo" {
`, username, email, escalationPolicy, service, ruleUpdated)
}

func testAccCheckPagerDutyServiceEventRuleConfigMultipleRules(username, email, escalationPolicy, service, rule1, rule2 string) string {
func testAccCheckPagerDutyServiceEventRuleConfigMultipleRules(username, email, escalationPolicy, service, rule1, rule2, rule3 string) string {
return fmt.Sprintf(`
resource "pagerduty_user" "foo" {
name = "%s"
Expand Down Expand Up @@ -325,7 +330,6 @@ resource "pagerduty_service" "foo" {
resource "pagerduty_service_event_rule" "foo" {
service = pagerduty_service.foo.id
position = 0
disabled = true
conditions {
operator = "and"
Expand Down Expand Up @@ -369,5 +373,25 @@ resource "pagerduty_service_event_rule" "bar" {
}
}
}
`, username, email, escalationPolicy, service, rule1, rule2)
resource "pagerduty_service_event_rule" "baz" {
service = pagerduty_service.foo.id
position = 2
disabled = true
conditions {
operator = "and"
subconditions {
operator = "contains"
parameter {
value = "slow ping"
path = "summary"
}
}
}
actions {
annotate {
value = "%s"
}
}
}
`, username, email, escalationPolicy, service, rule1, rule2, rule3)
}
2 changes: 1 addition & 1 deletion vendor/github.com/heimweh/go-pagerduty/pagerduty/ruleset.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions vendor/github.com/heimweh/go-pagerduty/pagerduty/service.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion vendor/modules.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ github.com/hashicorp/terraform-svchost/auth
github.com/hashicorp/terraform-svchost/disco
# github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d
github.com/hashicorp/yamux
# github.com/heimweh/go-pagerduty v0.0.0-20210208230541-602e6af0197d
# github.com/heimweh/go-pagerduty v0.0.0-20210209211114-6eef07a31388
## explicit
github.com/heimweh/go-pagerduty/pagerduty
# github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af
Expand Down

0 comments on commit 0c770f5

Please sign in to comment.