From aa3c90cd20ca57dc5a6d2be8dce0ea37078882ac Mon Sep 17 00:00:00 2001 From: Alexander Hellbom Date: Wed, 9 Aug 2017 23:59:43 +0200 Subject: [PATCH 1/4] Add overflow support and fix rvs bug --- pagerduty/resource_pagerduty_schedule.go | 73 ++++++++++++++++++++---- pagerduty/util.go | 9 +++ 2 files changed, 72 insertions(+), 10 deletions(-) diff --git a/pagerduty/resource_pagerduty_schedule.go b/pagerduty/resource_pagerduty_schedule.go index 813d07c13..404452001 100644 --- a/pagerduty/resource_pagerduty_schedule.go +++ b/pagerduty/resource_pagerduty_schedule.go @@ -21,15 +21,23 @@ func resourcePagerDutySchedule() *schema.Resource { Type: schema.TypeString, Optional: true, }, + "time_zone": { Type: schema.TypeString, Required: true, }, + + "overflow": { + Type: schema.TypeBool, + Optional: true, + }, + "description": { Type: schema.TypeString, Optional: true, Default: "Managed by Terraform", }, + "layer": { Type: schema.TypeList, Required: true, @@ -40,27 +48,33 @@ func resourcePagerDutySchedule() *schema.Resource { Type: schema.TypeString, Computed: true, }, + "name": { Type: schema.TypeString, Optional: true, Computed: true, }, + "start": { Type: schema.TypeString, Required: true, }, + "end": { Type: schema.TypeString, Optional: true, }, + "rotation_virtual_start": { Type: schema.TypeString, Required: true, }, + "rotation_turn_length_seconds": { Type: schema.TypeInt, Required: true, }, + "users": { Type: schema.TypeList, Required: true, @@ -68,6 +82,7 @@ func resourcePagerDutySchedule() *schema.Resource { Type: schema.TypeString, }, }, + "restriction": { Optional: true, Type: schema.TypeList, @@ -77,14 +92,17 @@ func resourcePagerDutySchedule() *schema.Resource { Type: schema.TypeString, Required: true, }, + "start_time_of_day": { Type: schema.TypeString, Required: true, }, + "start_day_of_week": { Type: schema.TypeInt, Optional: true, }, + "duration_seconds": { Type: schema.TypeInt, Required: true, @@ -99,28 +117,42 @@ func resourcePagerDutySchedule() *schema.Resource { } } -func buildScheduleStruct(d *schema.ResourceData) *pagerduty.Schedule { +func buildScheduleStruct(d *schema.ResourceData) (*pagerduty.Schedule, error) { + layers, err := expandScheduleLayers(d.Get("layer")) + if err != nil { + return nil, err + } + schedule := &pagerduty.Schedule{ Name: d.Get("name").(string), TimeZone: d.Get("time_zone").(string), - ScheduleLayers: expandScheduleLayers(d.Get("layer")), + ScheduleLayers: layers, } if attr, ok := d.GetOk("description"); ok { schedule.Description = attr.(string) } - return schedule + return schedule, nil } func resourcePagerDutyScheduleCreate(d *schema.ResourceData, meta interface{}) error { client := meta.(*pagerduty.Client) - schedule := buildScheduleStruct(d) + schedule, err := buildScheduleStruct(d) + if err != nil { + return err + } + + o := &pagerduty.CreateScheduleOptions{} + + if v, ok := d.GetOk("overflow"); ok { + o.Overflow = v.(bool) + } log.Printf("[INFO] Creating PagerDuty schedule: %s", schedule.Name) - schedule, _, err := client.Schedules.Create(schedule) + schedule, _, err = client.Schedules.Create(schedule, o) if err != nil { return err } @@ -154,11 +186,20 @@ func resourcePagerDutyScheduleRead(d *schema.ResourceData, meta interface{}) err func resourcePagerDutyScheduleUpdate(d *schema.ResourceData, meta interface{}) error { client := meta.(*pagerduty.Client) - schedule := buildScheduleStruct(d) + schedule, err := buildScheduleStruct(d) + if err != nil { + return err + } + + o := &pagerduty.UpdateScheduleOptions{} + + if v, ok := d.GetOk("overflow"); ok { + o.Overflow = v.(bool) + } log.Printf("[INFO] Updating PagerDuty schedule: %s", d.Id()) - if _, _, err := client.Schedules.Update(d.Id(), schedule); err != nil { + if _, _, err := client.Schedules.Update(d.Id(), schedule, o); err != nil { return err } @@ -179,17 +220,29 @@ func resourcePagerDutyScheduleDelete(d *schema.ResourceData, meta interface{}) e return nil } -func expandScheduleLayers(v interface{}) []*pagerduty.ScheduleLayer { +func expandScheduleLayers(v interface{}) ([]*pagerduty.ScheduleLayer, error) { var scheduleLayers []*pagerduty.ScheduleLayer for _, sl := range v.([]interface{}) { rsl := sl.(map[string]interface{}) + + // This is a temporary fix to prevent getting back the wrong rotation_virtual_start time. + // The background here is that if a user specifies a rotation_virtual_start time to be: + // "2017-09-01T10:00:00+02:00" the API returns back "2017-09-01T12:00:00+02:00". + // With this fix in place, we get the correct rotation_virtual_start time, thus + // eliminating the diff issues we've been seeing in the past. + // This has been confirmed working by PagerDuty support. + rvs, err := timeToUTC(rsl["rotation_virtual_start"].(string)) + if err != nil { + return nil, err + } + scheduleLayer := &pagerduty.ScheduleLayer{ ID: rsl["id"].(string), Name: rsl["name"].(string), Start: rsl["start"].(string), End: rsl["end"].(string), - RotationVirtualStart: rsl["rotation_virtual_start"].(string), + RotationVirtualStart: rvs, RotationTurnLengthSeconds: rsl["rotation_turn_length_seconds"].(int), } @@ -219,7 +272,7 @@ func expandScheduleLayers(v interface{}) []*pagerduty.ScheduleLayer { scheduleLayers = append(scheduleLayers, scheduleLayer) } - return scheduleLayers + return scheduleLayers, nil } func flattenScheduleLayers(v []*pagerduty.ScheduleLayer) []map[string]interface{} { diff --git a/pagerduty/util.go b/pagerduty/util.go index 7ba4ce197..bcb1fafe9 100644 --- a/pagerduty/util.go +++ b/pagerduty/util.go @@ -7,6 +7,15 @@ import ( "github.com/hashicorp/terraform/helper/schema" ) +func timeToUTC(v string) (string, error) { + t, err := time.Parse(time.RFC3339, v) + if err != nil { + return "", err + } + + return t.UTC().String(), nil +} + // validateRFC3339 validates that a date string has the correct RFC3339 layout func validateRFC3339(v interface{}, k string) (we []string, errors []error) { value := v.(string) From ce4717abf098766a7c8559098d9a2580925b05c9 Mon Sep 17 00:00:00 2001 From: Alexander Hellbom Date: Wed, 9 Aug 2017 23:59:57 +0200 Subject: [PATCH 2/4] Update tests --- pagerduty/resource_pagerduty_schedule_test.go | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/pagerduty/resource_pagerduty_schedule_test.go b/pagerduty/resource_pagerduty_schedule_test.go index de8887ce1..c71f3efab 100644 --- a/pagerduty/resource_pagerduty_schedule_test.go +++ b/pagerduty/resource_pagerduty_schedule_test.go @@ -99,6 +99,36 @@ func TestAccPagerDutySchedule_Basic(t *testing.T) { }) } +func TestAccPagerDutyScheduleOverflow_Basic(t *testing.T) { + username := fmt.Sprintf("tf-%s", acctest.RandString(5)) + email := fmt.Sprintf("%s@foo.com", username) + schedule := fmt.Sprintf("tf-%s", acctest.RandString(5)) + scheduleUpdated := fmt.Sprintf("tf-%s", acctest.RandString(5)) + location := "America/New_York" + start := timeNowInLoc(location).Add(30 * time.Hour).Round(1 * time.Hour).Format(time.RFC3339) + rotationVirtualStart := timeNowInLoc(location).Add(30 * time.Hour).Round(1 * time.Hour).Format(time.RFC3339) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckPagerDutyScheduleDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCheckPagerDutyScheduleOverflowConfig(username, email, schedule, location, start, rotationVirtualStart), + Check: resource.ComposeTestCheckFunc( + testAccCheckPagerDutyScheduleExists("pagerduty_schedule.foo"), + ), + }, + { + Config: testAccCheckPagerDutyScheduleOverflowConfigUpdated(username, email, scheduleUpdated, location, start, rotationVirtualStart), + Check: resource.ComposeTestCheckFunc( + testAccCheckPagerDutyScheduleExists("pagerduty_schedule.foo"), + ), + }, + }, + }) +} + func TestAccPagerDutySchedule_BasicWeek(t *testing.T) { username := fmt.Sprintf("tf-%s", acctest.RandString(5)) email := fmt.Sprintf("%s@foo.com", username) @@ -346,6 +376,64 @@ resource "pagerduty_schedule" "foo" { `, username, email, schedule, location, start, rotationVirtualStart) } +func testAccCheckPagerDutyScheduleOverflowConfig(username, email, schedule, location, start, rotationVirtualStart string) string { + return fmt.Sprintf(` +resource "pagerduty_user" "foo" { + name = "%s" + email = "%s" +} + +resource "pagerduty_schedule" "foo" { + name = "%s" + overflow = true + time_zone = "%s" + + layer { + name = "foo" + start = "%s" + rotation_virtual_start = "%s" + rotation_turn_length_seconds = 86400 + users = ["${pagerduty_user.foo.id}"] + + restriction { + type = "daily_restriction" + start_time_of_day = "08:00:00" + duration_seconds = 32101 + } + } +} +`, username, email, schedule, location, start, rotationVirtualStart) +} + +func testAccCheckPagerDutyScheduleOverflowConfigUpdated(username, email, schedule, location, start, rotationVirtualStart string) string { + return fmt.Sprintf(` +resource "pagerduty_user" "foo" { + name = "%s" + email = "%s" +} + +resource "pagerduty_schedule" "foo" { + name = "%s" + overflow = false + time_zone = "%s" + + layer { + name = "foo" + start = "%s" + rotation_virtual_start = "%s" + rotation_turn_length_seconds = 86400 + users = ["${pagerduty_user.foo.id}"] + + restriction { + type = "daily_restriction" + start_time_of_day = "08:00:00" + duration_seconds = 32101 + } + } +} +`, username, email, schedule, location, start, rotationVirtualStart) +} + func testAccCheckPagerDutyScheduleConfigWeek(username, email, schedule, location, start, rotationVirtualStart string) string { return fmt.Sprintf(` resource "pagerduty_user" "foo" { From d59f0d3f4213d221dc3eae04b6e17c9adceda0a2 Mon Sep 17 00:00:00 2001 From: Alexander Hellbom Date: Thu, 10 Aug 2017 00:00:07 +0200 Subject: [PATCH 3/4] Update go-pagerduty client --- .../go-pagerduty/pagerduty/schedule.go | 59 +++---------------- .../heimweh/go-pagerduty/pagerduty/user.go | 10 ++-- vendor/vendor.json | 6 +- 3 files changed, 17 insertions(+), 58 deletions(-) diff --git a/vendor/github.com/heimweh/go-pagerduty/pagerduty/schedule.go b/vendor/github.com/heimweh/go-pagerduty/pagerduty/schedule.go index 1bc37c1b6..21ddcdbb2 100644 --- a/vendor/github.com/heimweh/go-pagerduty/pagerduty/schedule.go +++ b/vendor/github.com/heimweh/go-pagerduty/pagerduty/schedule.go @@ -2,7 +2,6 @@ package pagerduty import ( "fmt" - "time" ) // ScheduleService handles the communication with schedule @@ -127,6 +126,11 @@ type GetScheduleOptions struct { Until string `url:"until,omitempty"` } +// CreateScheduleOptions represents options when creating a schedule. +type CreateScheduleOptions struct { + Overflow bool `url:"overflow,omitempty"` +} + // UpdateScheduleOptions represents options when updating a schedule. type UpdateScheduleOptions struct { Overflow bool `url:"overflow,omitempty"` @@ -146,17 +150,11 @@ func (s *ScheduleService) List(o *ListSchedulesOptions) (*ListSchedulesResponse, } // Create creates a new schedule. -func (s *ScheduleService) Create(schedule *Schedule) (*Schedule, *Response, error) { +func (s *ScheduleService) Create(schedule *Schedule, o *CreateScheduleOptions) (*Schedule, *Response, error) { u := "/schedules" v := new(Schedule) - for _, layer := range schedule.ScheduleLayers { - if err := normalizeTime(layer); err != nil { - return nil, nil, err - } - } - - resp, err := s.client.newRequestDo("POST", u, nil, &Schedule{Schedule: schedule}, &v) + resp, err := s.client.newRequestDo("POST", u, o, &Schedule{Schedule: schedule}, &v) if err != nil { return nil, nil, err } @@ -184,17 +182,11 @@ func (s *ScheduleService) Get(id string, o *GetScheduleOptions) (*Schedule, *Res } // Update updates an existing schedule. -func (s *ScheduleService) Update(id string, schedule *Schedule) (*Schedule, *Response, error) { +func (s *ScheduleService) Update(id string, schedule *Schedule, o *UpdateScheduleOptions) (*Schedule, *Response, error) { u := fmt.Sprintf("/schedules/%s", id) v := new(Schedule) - for _, layer := range schedule.ScheduleLayers { - if err := normalizeTime(layer); err != nil { - return nil, nil, err - } - } - - resp, err := s.client.newRequestDo("PUT", u, nil, &Schedule{Schedule: schedule}, &v) + resp, err := s.client.newRequestDo("PUT", u, o, &Schedule{Schedule: schedule}, &v) if err != nil { return nil, nil, err } @@ -246,36 +238,3 @@ func (s *ScheduleService) DeleteOverride(id string, overrideID string) (*Respons u := fmt.Sprintf("/schedules/%s/overrides/%s", id, overrideID) return s.client.newRequestDo("DELETE", u, nil, nil, nil) } - -func normalizeTime(l *ScheduleLayer) error { - s, err := timeToUTC(l.Start) - if err != nil { - return err - } - l.Start = s - - rvs, err := timeToUTC(l.RotationVirtualStart) - if err != nil { - return err - } - l.RotationVirtualStart = rvs - - if l.End != "" { - e, err := timeToUTC(l.End) - if err != nil { - return err - } - l.End = e - } - - return nil -} - -func timeToUTC(v string) (string, error) { - t, err := time.Parse(time.RFC3339, v) - if err != nil { - return "", err - } - - return t.UTC().String(), nil -} diff --git a/vendor/github.com/heimweh/go-pagerduty/pagerduty/user.go b/vendor/github.com/heimweh/go-pagerduty/pagerduty/user.go index 2b909df0e..464d3c50a 100644 --- a/vendor/github.com/heimweh/go-pagerduty/pagerduty/user.go +++ b/vendor/github.com/heimweh/go-pagerduty/pagerduty/user.go @@ -73,11 +73,11 @@ type PushContactMethodSound struct { // ListContactMethodsResponse represents type ListContactMethodsResponse struct { - Limit int `json:"limit,omitempty"` - More bool `json:"more,omitempty"` - Offset int `json:"offset,omitempty"` - Total int `json:"total,omitempty"` - Users []*User `json:"users,omitempty"` + Limit int `json:"limit,omitempty"` + More bool `json:"more,omitempty"` + Offset int `json:"offset,omitempty"` + Total int `json:"total,omitempty"` + ContactMethods []*ContactMethod `json:"contact_methods,omitempty"` } // ListUsersOptions represents options when listing users. diff --git a/vendor/vendor.json b/vendor/vendor.json index 6431bfdac..8eb766cd4 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -496,10 +496,10 @@ "revisionTime": "2016-07-20T23:31:40Z" }, { - "checksumSHA1": "wdQsbb7IEdj0riw+WRkysTD5Ri8=", + "checksumSHA1": "dUAQZ2MR++DUDweCwlkwvAJRW2c=", "path": "github.com/heimweh/go-pagerduty/pagerduty", - "revision": "2ce083be4d01d28a721911e83a38a780c7633bcc", - "revisionTime": "2017-07-07T18:34:57Z" + "revision": "274147fdf0518851dae256b9a80044a94ab814a3", + "revisionTime": "2017-08-09T21:49:27Z" }, { "checksumSHA1": "0ZrwvB6KoGPj2PoDNSEJwxQ6Mog=", From 753bc7296129a01a1063fdc429e581e85c9b341e Mon Sep 17 00:00:00 2001 From: Alexander Hellbom Date: Thu, 10 Aug 2017 00:00:16 +0200 Subject: [PATCH 4/4] Update documentation --- website/docs/r/schedule.html.markdown | 3 +++ 1 file changed, 3 insertions(+) diff --git a/website/docs/r/schedule.html.markdown b/website/docs/r/schedule.html.markdown index 9bb1f95be..6112d7356 100644 --- a/website/docs/r/schedule.html.markdown +++ b/website/docs/r/schedule.html.markdown @@ -48,6 +48,9 @@ The following arguments are supported: * `time_zone` - (Required) The time zone of the schedule (e.g Europe/Berlin). * `description` - (Optional) The description of the schedule * `layer` - (Required) A schedule layer block. Schedule layers documented below. +* `overflow` - (Optional) Any on-call schedule entries that pass the date range bounds will be truncated at the bounds, unless the parameter `overflow` is passed. For instance, if your schedule is a rotation that changes daily at midnight UTC, and your date range is from `2011-06-01T10:00:00Z` to `2011-06-01T14:00:00Z`: +If you don't pass the overflow=true parameter, you will get one schedule entry returned with a start of `2011-06-01T10:00:00Z` and end of `2011-06-01T14:00:00Z`. +If you do pass the `overflow` parameter, you will get one schedule entry returned with a start of `2011-06-01T00:00:00Z` and end of `2011-06-02T00:00:00Z`. Schedule layers (`layer`) supports the following: