fix: for 1624 resource monitor timestamps are always considered to ha…

…ve changed (#2214) * fix 1624 timestamp retrieval formating, logic changes to determine change status * removed redundant code in UpdateResourceMonitor, added acceptance test * fix conflicts
Snowflake-Labs · Dec 13, 2023 · 4d5d3ca · 4d5d3ca
1 parent 1c98a80
commit 4d5d3ca
Show file tree

Hide file tree

Showing 6 changed files with 157 additions and 28 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
diff --git 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)
 	})
 }