From 4d5d3ca4cfa3727240b05da9f5010b4fa908f695 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Wed, 13 Dec 2023 14:29:06 +0000 Subject: [PATCH] fix: for 1624 resource monitor timestamps are always considered to have changed (#2214) * fix 1624 timestamp retrieval formating, logic changes to determine change status * removed redundant code in UpdateResourceMonitor, added acceptance test * fix conflicts --- pkg/resources/resource_monitor.go | 12 +- .../resource_monitor_acceptance_test.go | 122 ++++++++++++++++++ pkg/sdk/parsers.go | 17 +++ pkg/sdk/resource_monitors.go | 13 +- pkg/sdk/testint/parsers.go | 13 -- .../resource_monitors_integration_test.go | 8 +- 6 files changed, 157 insertions(+), 28 deletions(-) create mode 100644 pkg/sdk/parsers.go delete mode 100644 pkg/sdk/testint/parsers.go diff --git a/pkg/resources/resource_monitor.go b/pkg/resources/resource_monitor.go index 4d778e2c6a..0f8c51137d 100644 --- a/pkg/resources/resource_monitor.go +++ b/pkg/resources/resource_monitor.go @@ -335,17 +335,13 @@ func UpdateResourceMonitor(d *schema.ResourceData, meta interface{}) error { opts.Set.CreditQuota = sdk.Pointer(d.Get("credit_quota").(int)) } - if d.HasChange("frequency") { + if d.HasChange("frequency") || d.HasChange("start_timestamp") { runSetStatement = true frequency, err := sdk.FrequencyFromString(d.Get("frequency").(string)) if err != nil { return err } opts.Set.Frequency = frequency - } - - if d.HasChange("start_timestamp") { - runSetStatement = true opts.Set.StartTimestamp = sdk.Pointer(d.Get("start_timestamp").(string)) } @@ -355,10 +351,8 @@ func UpdateResourceMonitor(d *schema.ResourceData, meta interface{}) error { } // If ANY of the triggers changed, we collect all triggers and set them - if d.HasChange("suspend_trigger") || - d.HasChange("suspend_triggers") || - d.HasChange("suspend_immediate_trigger") || - d.HasChange("suspend_immediate_triggers") || + if d.HasChange("suspend_trigger") || d.HasChange("suspend_triggers") || + d.HasChange("suspend_immediate_trigger") || d.HasChange("suspend_immediate_triggers") || d.HasChange("notify_triggers") { runSetStatement = true triggers := collectResourceMonitorTriggers(d) diff --git a/pkg/resources/resource_monitor_acceptance_test.go b/pkg/resources/resource_monitor_acceptance_test.go index 17a5f7b052..c6e667897c 100644 --- a/pkg/resources/resource_monitor_acceptance_test.go +++ b/pkg/resources/resource_monitor_acceptance_test.go @@ -54,6 +54,128 @@ func TestAcc_ResourceMonitor(t *testing.T) { }) } +func TestAcc_ResourceMonitorChangeStartEndTimestamp(t *testing.T) { + name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) + + resource.ParallelTest(t, resource.TestCase{ + Providers: acc.TestAccProviders(), + PreCheck: func() { acc.TestAccPreCheck(t) }, + CheckDestroy: nil, + Steps: []resource.TestStep{ + { + Config: resourceMonitorConfigInitialTimestamp(name), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "name", name), + resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "frequency", "WEEKLY"), + resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "start_timestamp", "2050-01-01 12:00"), + resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "end_timestamp", "2055-01-01 12:00"), + ), + }, + { + Config: resourceMonitorConfigUpdatedTimestamp(name), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "name", name), + resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "frequency", "WEEKLY"), + resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "start_timestamp", "2055-01-01 12:00"), + resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "end_timestamp", "2056-01-01 12:00"), + ), + }, + // IMPORT + { + ResourceName: "snowflake_resource_monitor.test", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func resourceMonitorConfigUpdatedTimestamp(accName string) string { + return fmt.Sprintf(` +resource "snowflake_warehouse" "warehouse" { + name = "test" + comment = "foo" + warehouse_size = "XSMALL" +} + +resource "snowflake_resource_monitor" "test" { + name = "%v" + frequency = "WEEKLY" + start_timestamp = "2055-01-01 12:00" + end_timestamp = "2056-01-01 12:00" + +} +`, accName) +} + +// fix 2 added empy notifiy user +// Config for changed timestamp frequency validation test +func resourceMonitorConfigInitialTimestamp(accName string) string { + return fmt.Sprintf(` +resource "snowflake_warehouse" "warehouse" { + name = "test" + comment = "foo" + warehouse_size = "XSMALL" +} + +resource "snowflake_resource_monitor" "test" { + name = "%v" + frequency = "WEEKLY" + start_timestamp = "2050-01-01 12:00" + end_timestamp = "2055-01-01 12:00" + +} +`, accName) +} + +func TestAcc_ResourceMonitorUpdateNotifyUsers(t *testing.T) { + userEnv := os.Getenv("RESOURCE_MONITOR_NOTIFY_USERS_TEST") + if userEnv == "" { + t.Skip("Skipping ResourceMonitorUpdateNotifyUsers") + } + users := strings.Split(userEnv, ",") + name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) + config, err := resourceMonitorNotifyUsersConfig(name, users) + if err != nil { + t.Error(err) + } + checks := []resource.TestCheckFunc{ + resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "name", name), + resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "set_for_account", "false"), + } + for _, s := range users { + checks = append(checks, resource.TestCheckTypeSetElemAttr("snowflake_resource_monitor.test", "notify_users.*", s)) + } + empty := []string{} + emptyUsersConfig, err := resourceMonitorNotifyUsersConfig(name, empty) + if err != nil { + t.Error(err) + } + resource.ParallelTest(t, resource.TestCase{ + Providers: acc.TestAccProviders(), + PreCheck: func() { acc.TestAccPreCheck(t) }, + CheckDestroy: nil, + Steps: []resource.TestStep{ + { + Config: emptyUsersConfig, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "name", name), + resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "set_for_account", "false"), + ), + }, + { + Config: config, + Check: resource.ComposeTestCheckFunc(checks...), + }, + { + ResourceName: "snowflake_resource_monitor.test", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func resourceMonitorConfig(accName string) string { return fmt.Sprintf(` resource "snowflake_warehouse" "warehouse" { diff --git a/pkg/sdk/parsers.go b/pkg/sdk/parsers.go new file mode 100644 index 0000000000..02fc4798da --- /dev/null +++ b/pkg/sdk/parsers.go @@ -0,0 +1,17 @@ +package sdk + +import ( + "time" +) + +// fix timestamp merge +func ParseTimestampWithOffset(s string, dateTimeFormat string) (string, error) { + t, err := time.Parse(time.RFC3339, s) + if err != nil { + return err.Error(), err + } + _, offset := t.Zone() + adjustedTime := t.Add(-time.Duration(offset) * time.Second) + adjustedTimeFormat := adjustedTime.Format(dateTimeFormat) + return adjustedTimeFormat, nil +} diff --git a/pkg/sdk/resource_monitors.go b/pkg/sdk/resource_monitors.go index 6a865f9d07..29f887129d 100644 --- a/pkg/sdk/resource_monitors.go +++ b/pkg/sdk/resource_monitors.go @@ -97,10 +97,19 @@ func (row *resourceMonitorRow) convert() (*ResourceMonitor, error) { resourceMonitor.Frequency = *frequency } if row.StartTime.Valid { - resourceMonitor.StartTime = row.StartTime.String + convertedStartTime, err := ParseTimestampWithOffset(row.StartTime.String, "2006-01-02 15:04") + if err != nil { + return nil, err + } + resourceMonitor.StartTime = convertedStartTime } + if row.EndTime.Valid { - resourceMonitor.EndTime = row.EndTime.String + convertedEndTime, err := ParseTimestampWithOffset(row.EndTime.String, "2006-01-02 15:04") + if err != nil { + return nil, err + } + resourceMonitor.EndTime = convertedEndTime } suspendTriggers, err := extractTriggerInts(row.SuspendAt) if err != nil { diff --git a/pkg/sdk/testint/parsers.go b/pkg/sdk/testint/parsers.go deleted file mode 100644 index 99114fb599..0000000000 --- a/pkg/sdk/testint/parsers.go +++ /dev/null @@ -1,13 +0,0 @@ -package testint - -import "time" - -func parseTimestampWithOffset(s string) (*time.Time, error) { - t, err := time.Parse("2006-01-02T15:04:05-07:00", s) - if err != nil { - return nil, err - } - _, offset := t.Zone() - adjustedTime := t.Add(-time.Duration(offset) * time.Second) - return &adjustedTime, nil -} diff --git a/pkg/sdk/testint/resource_monitors_integration_test.go b/pkg/sdk/testint/resource_monitors_integration_test.go index 904a198c8d..d1124198a7 100644 --- a/pkg/sdk/testint/resource_monitors_integration_test.go +++ b/pkg/sdk/testint/resource_monitors_integration_test.go @@ -235,12 +235,12 @@ func TestInt_ResourceMonitorAlter(t *testing.T) { assert.Equal(t, 1, len(resourceMonitors)) resourceMonitor = &resourceMonitors[0] assert.Equal(t, *frequency, resourceMonitor.Frequency) - startTime, err := parseTimestampWithOffset(resourceMonitor.StartTime) + startTime := resourceMonitor.StartTime require.NoError(t, err) - endTime, err := parseTimestampWithOffset(resourceMonitor.EndTime) + endTime := resourceMonitor.EndTime require.NoError(t, err) - assert.Equal(t, startTimeStamp, startTime.Format("2006-01-01 15:04")) - assert.Equal(t, endTimeStamp, endTime.Format("2006-01-01 15:04")) + assert.Equal(t, startTimeStamp, startTime) + assert.Equal(t, endTimeStamp, endTime) }) }