From a56b5cf261e25f99c07aa1f56873c8c5d7417dca Mon Sep 17 00:00:00 2001 From: Scott McAllister Date: Wed, 10 Feb 2021 13:50:18 -0800 Subject: [PATCH] fixed bug in event rule positioning --- go.mod | 2 +- go.sum | 2 ++ pagerduty/resource_pagerduty_ruleset_rule.go | 17 ++++++---- .../resource_pagerduty_ruleset_rule_test.go | 31 ++++++++++++++++-- .../resource_pagerduty_service_event_rule.go | 18 +++++++---- ...ource_pagerduty_service_event_rule_test.go | 32 ++++++++++++++++--- .../heimweh/go-pagerduty/pagerduty/ruleset.go | 2 +- .../heimweh/go-pagerduty/pagerduty/service.go | 4 +-- vendor/modules.txt | 2 +- 9 files changed, 84 insertions(+), 26 deletions(-) diff --git a/go.mod b/go.mod index c2b397728..b97cd6946 100644 --- a/go.mod +++ b/go.mod @@ -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 diff --git a/go.sum b/go.sum index be8284be7..e4e42e25e 100644 --- a/go.sum +++ b/go.sum @@ -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= diff --git a/pagerduty/resource_pagerduty_ruleset_rule.go b/pagerduty/resource_pagerduty_ruleset_rule.go index bf23d21c3..884ef666c 100644 --- a/pagerduty/resource_pagerduty_ruleset_rule.go +++ b/pagerduty/resource_pagerduty_ruleset_rule.go @@ -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) } @@ -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) } @@ -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 }) diff --git a/pagerduty/resource_pagerduty_ruleset_rule_test.go b/pagerduty/resource_pagerduty_ruleset_rule_test.go index 082ce2098..38b034735 100644 --- a/pagerduty/resource_pagerduty_ruleset_rule_test.go +++ b/pagerduty/resource_pagerduty_ruleset_rule_test.go @@ -76,6 +76,7 @@ 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) }, @@ -83,7 +84,7 @@ func TestAccPagerDutyRulesetRule_MultipleRules(t *testing.T) { 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"), @@ -91,6 +92,8 @@ func TestAccPagerDutyRulesetRule_MultipleRules(t *testing.T) { "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( @@ -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), ), }, }, @@ -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" @@ -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) } diff --git a/pagerduty/resource_pagerduty_service_event_rule.go b/pagerduty/resource_pagerduty_service_event_rule.go index 0ff7e9ed4..32f745a33 100644 --- a/pagerduty/resource_pagerduty_service_event_rule.go +++ b/pagerduty/resource_pagerduty_service_event_rule.go @@ -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) } @@ -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) } @@ -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 { diff --git a/pagerduty/resource_pagerduty_service_event_rule_test.go b/pagerduty/resource_pagerduty_service_event_rule_test.go index 014d40fb2..c5d076b6b 100644 --- a/pagerduty/resource_pagerduty_service_event_rule_test.go +++ b/pagerduty/resource_pagerduty_service_event_rule_test.go @@ -78,6 +78,7 @@ 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) }, @@ -85,7 +86,7 @@ func TestAccPagerDutyServiceEventRule_MultipleRules(t *testing.T) { 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"), @@ -93,6 +94,8 @@ func TestAccPagerDutyServiceEventRule_MultipleRules(t *testing.T) { "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( @@ -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), ), }, }, @@ -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" @@ -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" @@ -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) } diff --git a/vendor/github.com/heimweh/go-pagerduty/pagerduty/ruleset.go b/vendor/github.com/heimweh/go-pagerduty/pagerduty/ruleset.go index c1e50cf68..b0ca95b4e 100644 --- a/vendor/github.com/heimweh/go-pagerduty/pagerduty/ruleset.go +++ b/vendor/github.com/heimweh/go-pagerduty/pagerduty/ruleset.go @@ -42,7 +42,7 @@ type ListRulesetsResponse struct { // RulesetRule represents a Ruleset rule type RulesetRule struct { ID string `json:"id,omitempty"` - Position int `json:"position,omitempty"` + Position *int `json:"position,omitempty"` Disabled bool `json:"disabled"` Conditions *RuleConditions `json:"conditions,omitempty"` Actions *RuleActions `json:"actions,omitempty"` diff --git a/vendor/github.com/heimweh/go-pagerduty/pagerduty/service.go b/vendor/github.com/heimweh/go-pagerduty/pagerduty/service.go index 6f183d2c0..51b8ccad3 100644 --- a/vendor/github.com/heimweh/go-pagerduty/pagerduty/service.go +++ b/vendor/github.com/heimweh/go-pagerduty/pagerduty/service.go @@ -89,11 +89,11 @@ type Service struct { type ServiceEventRule struct { ID string `json:"id,omitempty"` Self string `json:"self,omitempty"` - Disabled bool `json:"disabled,omitempty"` + Disabled bool `json:"disabled"` Conditions *RuleConditions `json:"conditions,omitempty"` TimeFrame *RuleTimeFrame `json:"time_frame,omitempty"` Variables []*RuleVariable `json:"variables,omitempty"` - Position int `json:"position,omitempty"` + Position *int `json:"position,omitempty"` Actions *RuleActions `json:"actions,omitempty"` Service *ServiceReference `json:"service_id,omitempty"` } diff --git a/vendor/modules.txt b/vendor/modules.txt index 5bffa1ea4..594d95432 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -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