From 15c1f35dfa415caab5f0672a31628a69a2f54b5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Antonio=20Reyes?= Date: Tue, 27 Feb 2024 20:39:01 -0300 Subject: [PATCH 1/4] add list of allowed TZ for validating tome_zone attribute --- pagerduty/resource_pagerduty_schedule.go | 13 +- pagerduty/resource_pagerduty_service.go | 13 +- pagerduty/resource_pagerduty_user.go | 15 +- util/util.go | 179 +++++++++++++++++++++++ 4 files changed, 192 insertions(+), 28 deletions(-) diff --git a/pagerduty/resource_pagerduty_schedule.go b/pagerduty/resource_pagerduty_schedule.go index 980c35879..11a751678 100644 --- a/pagerduty/resource_pagerduty_schedule.go +++ b/pagerduty/resource_pagerduty_schedule.go @@ -9,6 +9,7 @@ import ( "strings" "time" + "github.com/PagerDuty/terraform-provider-pagerduty/util" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" @@ -53,15 +54,9 @@ func resourcePagerDutySchedule() *schema.Resource { }, "time_zone": { - Type: schema.TypeString, - Required: true, - ValidateFunc: func(val interface{}, key string) (warns []string, errs []error) { - _, err := time.LoadLocation(val.(string)) - if err != nil { - errs = append(errs, err) - } - return - }, + Type: schema.TypeString, + Required: true, + ValidateDiagFunc: util.ValidateTZValueDiagFunc, }, "overflow": { diff --git a/pagerduty/resource_pagerduty_service.go b/pagerduty/resource_pagerduty_service.go index db7cfb0a4..586de1917 100644 --- a/pagerduty/resource_pagerduty_service.go +++ b/pagerduty/resource_pagerduty_service.go @@ -8,6 +8,7 @@ import ( "strconv" "time" + "github.com/PagerDuty/terraform-provider-pagerduty/util" "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" @@ -238,15 +239,9 @@ func resourcePagerDutyService() *schema.Resource { Optional: true, }, "time_zone": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: func(val interface{}, key string) (warns []string, errs []error) { - _, err := time.LoadLocation(val.(string)) - if err != nil { - errs = append(errs, err) - } - return - }, + Type: schema.TypeString, + Optional: true, + ValidateDiagFunc: util.ValidateTZValueDiagFunc, }, "start_time": { Type: schema.TypeString, diff --git a/pagerduty/resource_pagerduty_user.go b/pagerduty/resource_pagerduty_user.go index 21924e963..e96c51993 100644 --- a/pagerduty/resource_pagerduty_user.go +++ b/pagerduty/resource_pagerduty_user.go @@ -7,6 +7,7 @@ import ( "strings" "time" + "github.com/PagerDuty/terraform-provider-pagerduty/util" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/heimweh/go-pagerduty/pagerduty" @@ -79,16 +80,10 @@ func resourcePagerDutyUser() *schema.Resource { }, "time_zone": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ValidateFunc: func(val interface{}, key string) (warns []string, errs []error) { - _, err := time.LoadLocation(val.(string)) - if err != nil { - errs = append(errs, err) - } - return - }, + Type: schema.TypeString, + Optional: true, + Computed: true, + ValidateDiagFunc: util.ValidateTZValueDiagFunc, }, "html_url": { diff --git a/util/util.go b/util/util.go index 879c4e241..5206e08ca 100644 --- a/util/util.go +++ b/util/util.go @@ -276,3 +276,182 @@ func ResourcePagerDutyParseColonCompoundID(id string) (string, string, error) { return parts[0], parts[1], nil } + +func ValidateTZValueDiagFunc(v interface{}, p cty.Path) diag.Diagnostics { + var diags diag.Diagnostics + + value := v.(string) + valid := false + for _, tz := range validTZ { + if value == tz { + valid = true + break + } + } + + if !valid { + diags = append(diags, diag.Diagnostic{ + Severity: diag.Error, + Summary: fmt.Sprintf("%q is a not valid input. Please refer to the list of allowed Time Zone values at https://developer.pagerduty.com/docs/1afe25e9c94cb-types#time-zone", value), + AttributePath: p, + }) + } + + return diags +} + +// validTZ at the moment there not an API to fetch this values, so hardcoding +// them here +var validTZ []string = []string{ + "Africa/Algiers", + "Africa/Cairo", + "Africa/Casablanca", + "Africa/Harare", + "Africa/Johannesburg", + "Africa/Monrovia", + "Africa/Nairobi", + "America/Argentina/Buenos_Aires", + "America/Bogota", + "America/Caracas", + "America/Chicago", + "America/Chihuahua", + "America/Denver", + "America/Godthab", + "America/Guatemala", + "America/Guyana", + "America/Halifax", + "America/Indiana/Indianapolis", + "America/Juneau", + "America/La_Paz", + "America/Lima", + "America/Lima", + "America/Los_Angeles", + "America/Mazatlan", + "America/Mexico_City", + "America/Mexico_City", + "America/Monterrey", + "America/Montevideo", + "America/New_York", + "America/Phoenix", + "America/Puerto_Rico", + "America/Regina", + "America/Santiago", + "America/Sao_Paulo", + "America/St_Johns", + "America/Tijuana", + "Asia/Almaty", + "Asia/Baghdad", + "Asia/Baku", + "Asia/Bangkok", + "Asia/Bangkok", + "Asia/Chongqing", + "Asia/Colombo", + "Asia/Dhaka", + "Asia/Dhaka", + "Asia/Hong_Kong", + "Asia/Irkutsk", + "Asia/Jakarta", + "Asia/Jerusalem", + "Asia/Kabul", + "Asia/Kamchatka", + "Asia/Karachi", + "Asia/Karachi", + "Asia/Kathmandu", + "Asia/Kolkata", + "Asia/Kolkata", + "Asia/Kolkata", + "Asia/Kolkata", + "Asia/Krasnoyarsk", + "Asia/Kuala_Lumpur", + "Asia/Kuwait", + "Asia/Magadan", + "Asia/Muscat", + "Asia/Muscat", + "Asia/Novosibirsk", + "Asia/Rangoon", + "Asia/Riyadh", + "Asia/Seoul", + "Asia/Shanghai", + "Asia/Singapore", + "Asia/Srednekolymsk", + "Asia/Taipei", + "Asia/Tashkent", + "Asia/Tbilisi", + "Asia/Tehran", + "Asia/Tokyo", + "Asia/Tokyo", + "Asia/Tokyo", + "Asia/Ulaanbaatar", + "Asia/Urumqi", + "Asia/Vladivostok", + "Asia/Yakutsk", + "Asia/Yekaterinburg", + "Asia/Yerevan", + "Atlantic/Azores", + "Atlantic/Cape_Verde", + "Atlantic/South_Georgia", + "Australia/Adelaide", + "Australia/Brisbane", + "Australia/Darwin", + "Australia/Hobart", + "Australia/Melbourne", + "Australia/Melbourne", + "Australia/Perth", + "Australia/Sydney", + "Etc/GMT+12", + "Etc/UTC", + "Europe/Amsterdam", + "Europe/Athens", + "Europe/Belgrade", + "Europe/Berlin", + "Europe/Bratislava", + "Europe/Brussels", + "Europe/Bucharest", + "Europe/Budapest", + "Europe/Copenhagen", + "Europe/Dublin", + "Europe/Helsinki", + "Europe/Istanbul", + "Europe/Kaliningrad", + "Europe/Kiev", + "Europe/Lisbon", + "Europe/Ljubljana", + "Europe/London", + "Europe/London", + "Europe/Madrid", + "Europe/Minsk", + "Europe/Moscow", + "Europe/Moscow", + "Europe/Paris", + "Europe/Prague", + "Europe/Riga", + "Europe/Rome", + "Europe/Samara", + "Europe/Sarajevo", + "Europe/Skopje", + "Europe/Sofia", + "Europe/Stockholm", + "Europe/Tallinn", + "Europe/Vienna", + "Europe/Vilnius", + "Europe/Volgograd", + "Europe/Warsaw", + "Europe/Zagreb", + "Europe/Zurich", + "Europe/Zurich", + "Pacific/Apia", + "Pacific/Auckland", + "Pacific/Auckland", + "Pacific/Chatham", + "Pacific/Fakaofo", + "Pacific/Fiji", + "Pacific/Guadalcanal", + "Pacific/Guam", + "Pacific/Honolulu", + "Pacific/Majuro", + "Pacific/Midway", + "Pacific/Noumea", + "Pacific/Pago_Pago", + "Pacific/Port_Moresby", + "Pacific/Tongatapu", +} From 80b1128f688a9d4a1ca1d3f370e4dade863b80ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Antonio=20Reyes?= Date: Wed, 28 Feb 2024 15:01:28 -0300 Subject: [PATCH 2/4] replace for loop by `sort.SearchStrings` --- util/util.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/util/util.go b/util/util.go index 5206e08ca..1a6ee0828 100644 --- a/util/util.go +++ b/util/util.go @@ -7,6 +7,7 @@ import ( "math" "reflect" "regexp" + "sort" "strings" "time" "unicode" @@ -282,11 +283,9 @@ func ValidateTZValueDiagFunc(v interface{}, p cty.Path) diag.Diagnostics { value := v.(string) valid := false - for _, tz := range validTZ { - if value == tz { - valid = true - break - } + + if sort.SearchStrings(validTZ, value) < len(validTZ) { + valid = true } if !valid { From f55f9f9573cae227cdf1f47b4857c7d18fb2da6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Antonio=20Reyes?= Date: Thu, 29 Feb 2024 15:09:42 -0300 Subject: [PATCH 3/4] add validation of tz found --- util/util.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/util/util.go b/util/util.go index 1a6ee0828..4f24958ba 100644 --- a/util/util.go +++ b/util/util.go @@ -284,7 +284,8 @@ func ValidateTZValueDiagFunc(v interface{}, p cty.Path) diag.Diagnostics { value := v.(string) valid := false - if sort.SearchStrings(validTZ, value) < len(validTZ) { + foundAt := sort.SearchStrings(validTZ, value) + if foundAt < len(validTZ) && validTZ[foundAt] == value { valid = true } From e2543428d950179009cf0bce79b772cf13aaab7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Antonio=20Reyes?= Date: Thu, 29 Feb 2024 15:10:47 -0300 Subject: [PATCH 4/4] add test cases --- util/util_test.go | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 util/util_test.go diff --git a/util/util_test.go b/util/util_test.go new file mode 100644 index 000000000..770d1dd95 --- /dev/null +++ b/util/util_test.go @@ -0,0 +1,43 @@ +package util + +import ( + "fmt" + "reflect" + "testing" + + "github.com/hashicorp/go-cty/cty" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" +) + +func TestValidateTZValueDiagFunc(t *testing.T) { + notValidTZ1 := "not a valid TZ" + + cases := []struct { + given string + want diag.Diagnostics + path cty.Path + }{ + { + given: "America/Montevideo", + want: nil, + path: cty.Path{}, + }, + { + given: "America/Indiana/Indianapolis", + want: nil, + path: cty.Path{}, + }, + { + given: notValidTZ1, + want: diag.Diagnostics{diag.Diagnostic{Severity: 0, Summary: fmt.Sprintf("\"%s\" is a not valid input. Please refer to the list of allowed Time Zone values at https://developer.pagerduty.com/docs/1afe25e9c94cb-types#time-zone", notValidTZ1), Detail: "", AttributePath: cty.Path{cty.GetAttrStep{Name: "time_zone"}}}}, + path: cty.Path{cty.GetAttrStep{Name: "time_zone"}}, + }, + } + + for _, c := range cases { + got := ValidateTZValueDiagFunc(c.given, c.path) + if !reflect.DeepEqual(got, c.want) { + t.Errorf("want %v; got %v", c.want, got) + } + } +}