diff --git a/docs/resources/resource_monitor.md b/docs/resources/resource_monitor.md index 107dd5b5be..7c2942505e 100644 --- a/docs/resources/resource_monitor.md +++ b/docs/resources/resource_monitor.md @@ -43,11 +43,11 @@ resource "snowflake_resource_monitor" "monitor" { - `credit_quota` (Number) The number of credits allocated to the resource monitor per frequency interval. When total usage for all warehouses assigned to the monitor reaches this number for the current frequency interval, the resource monitor is considered to be at 100% of quota. - `end_timestamp` (String) The date and time when the resource monitor suspends the assigned warehouses. - `frequency` (String) The frequency interval at which the credit usage resets to 0. Valid values are (case-insensitive): `MONTHLY` | `DAILY` | `WEEKLY` | `YEARLY` | `NEVER`. If you set a `frequency` for a resource monitor, you must also set `start_timestamp`. If you specify `NEVER` for the frequency, the credit usage for the warehouse does not reset. After removing this field from the config, the previously set value will be preserved on the Snowflake side, not the default value. That's due to Snowflake limitation and the lack of unset functionality for this parameter. -- `notify_triggers` (Set of Number) +- `notify_triggers` (Set of Number) Specifies a list of percentages of the credit quota. After reaching any of the values the users passed in the notify_users field will be notified (to receive the notification they should have notifications enabled). Values over 100 are supported. - `notify_users` (Set of String) Specifies the list of users (their identifiers) to receive email notifications on resource monitors. - `start_timestamp` (String) The date and time when the resource monitor starts monitoring credit usage for the assigned warehouses. If you set a `start_timestamp` for a resource monitor, you must also set `frequency`. After removing this field from the config, the previously set value will be preserved on the Snowflake side, not the default value. That's due to Snowflake limitation and the lack of unset functionality for this parameter. -- `suspend_immediate_trigger` (Number) -- `suspend_trigger` (Number) +- `suspend_immediate_trigger` (Number) Represents a numeric value specified as a percentage of the credit quota. Values over 100 are supported. After reaching this value, all assigned warehouses immediately cancel any currently running queries or statements. In addition, this action sends a notification to all users who have enabled notifications for themselves. +- `suspend_trigger` (Number) Represents a numeric value specified as a percentage of the credit quota. Values over 100 are supported. After reaching this value, all assigned warehouses while allowing currently running queries to complete will be suspended. No new queries can be executed by the warehouses until the credit quota for the resource monitor is increased. In addition, this action sends a notification to all users who have enabled notifications for themselves. ### Read-Only diff --git a/pkg/acceptance/bettertestspoc/assert/objectassert/resource_monitor_snowflake_ext.go b/pkg/acceptance/bettertestspoc/assert/objectassert/resource_monitor_snowflake_ext.go new file mode 100644 index 0000000000..020374305c --- /dev/null +++ b/pkg/acceptance/bettertestspoc/assert/objectassert/resource_monitor_snowflake_ext.go @@ -0,0 +1,30 @@ +package objectassert + +import ( + "fmt" + "testing" + + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" +) + +func (r *ResourceMonitorAssert) HasNonEmptyStartTime() *ResourceMonitorAssert { + r.AddAssertion(func(t *testing.T, o *sdk.ResourceMonitor) error { + t.Helper() + if o.StartTime == "" { + return fmt.Errorf("expected start time to be non empty") + } + return nil + }) + return r +} + +func (r *ResourceMonitorAssert) HasNonEmptyEndTime() *ResourceMonitorAssert { + r.AddAssertion(func(t *testing.T, o *sdk.ResourceMonitor) error { + t.Helper() + if o.StartTime == "" { + return fmt.Errorf("expected end time to be non empty") + } + return nil + }) + return r +} diff --git a/pkg/resources/resource_monitor.go b/pkg/resources/resource_monitor.go index 330f155a5f..a9aabe284e 100644 --- a/pkg/resources/resource_monitor.go +++ b/pkg/resources/resource_monitor.go @@ -62,19 +62,25 @@ var resourceMonitorSchema = map[string]*schema.Schema{ Description: "The date and time when the resource monitor suspends the assigned warehouses.", }, "notify_triggers": { - Type: schema.TypeSet, - Optional: true, + Type: schema.TypeSet, + Optional: true, + Description: "Specifies a list of percentages of the credit quota. After reaching any of the values the users passed in the notify_users field will be notified (to receive the notification they should have notifications enabled). Values over 100 are supported.", Elem: &schema.Schema{ - Type: schema.TypeInt, + Type: schema.TypeInt, + ValidateDiagFunc: validation.ToDiagFunc(validation.IntAtLeast(1)), }, }, "suspend_trigger": { - Type: schema.TypeInt, - Optional: true, + Type: schema.TypeInt, + Optional: true, + Description: "Represents a numeric value specified as a percentage of the credit quota. Values over 100 are supported. After reaching this value, all assigned warehouses while allowing currently running queries to complete will be suspended. No new queries can be executed by the warehouses until the credit quota for the resource monitor is increased. In addition, this action sends a notification to all users who have enabled notifications for themselves.", + ValidateDiagFunc: validation.ToDiagFunc(validation.IntAtLeast(1)), }, "suspend_immediate_trigger": { - Type: schema.TypeInt, - Optional: true, + Type: schema.TypeInt, + Optional: true, + Description: "Represents a numeric value specified as a percentage of the credit quota. Values over 100 are supported. After reaching this value, all assigned warehouses immediately cancel any currently running queries or statements. In addition, this action sends a notification to all users who have enabled notifications for themselves.", + ValidateDiagFunc: validation.ToDiagFunc(validation.IntAtLeast(1)), }, ShowOutputAttributeName: { Type: schema.TypeList, diff --git a/pkg/resources/resource_monitor_acceptance_test.go b/pkg/resources/resource_monitor_acceptance_test.go index a058dc0164..51f3c47a32 100644 --- a/pkg/resources/resource_monitor_acceptance_test.go +++ b/pkg/resources/resource_monitor_acceptance_test.go @@ -600,7 +600,7 @@ func TestAcc_ResourceMonitor_Issue1500_AlteringWithOnlyTriggers(t *testing.T) { }, }, Config: config.FromModel(t, configModelWithUpdatedTriggers), - // For some reason, not returning error (SQL compilation error should be returned in this case; most likely update was processed incorrectly) + // For some reason, not returning error (SQL compilation error should be returned in this case; most likely update was handled incorrectly, or it was handled similarly as in the current version) }, // Remove all triggers (not allowed in Snowflake) { diff --git a/pkg/schemas/resource_monitor_gen.go b/pkg/schemas/resource_monitor_gen.go index 83a325fba2..6176a3cc20 100644 --- a/pkg/schemas/resource_monitor_gen.go +++ b/pkg/schemas/resource_monitor_gen.go @@ -41,10 +41,6 @@ var ShowResourceMonitorSchema = map[string]*schema.Schema{ Type: schema.TypeString, Computed: true, }, - //"notify_at": { - // Type: schema.TypeInvalid, - // Computed: true, - //}, "suspend_at": { Type: schema.TypeInt, Computed: true, @@ -65,10 +61,6 @@ var ShowResourceMonitorSchema = map[string]*schema.Schema{ Type: schema.TypeString, Computed: true, }, - //"notify_users": { - // Type: schema.TypeInvalid, - // Computed: true, - //}, } var _ = ShowResourceMonitorSchema @@ -85,13 +77,11 @@ func ResourceMonitorToSchema(resourceMonitor *sdk.ResourceMonitor) map[string]an resourceMonitorSchema["frequency"] = string(resourceMonitor.Frequency) resourceMonitorSchema["start_time"] = resourceMonitor.StartTime resourceMonitorSchema["end_time"] = resourceMonitor.EndTime - //resourceMonitorSchema["notify_at"] = resourceMonitor.NotifyAt resourceMonitorSchema["suspend_at"] = resourceMonitor.SuspendAt resourceMonitorSchema["suspend_immediate_at"] = resourceMonitor.SuspendImmediateAt resourceMonitorSchema["created_on"] = resourceMonitor.CreatedOn.String() resourceMonitorSchema["owner"] = resourceMonitor.Owner resourceMonitorSchema["comment"] = resourceMonitor.Comment - //resourceMonitorSchema["notify_users"] = resourceMonitor.NotifyUsers return resourceMonitorSchema } diff --git a/pkg/sdk/resource_monitors.go b/pkg/sdk/resource_monitors.go index 095c8a7f8d..d2ac62314c 100644 --- a/pkg/sdk/resource_monitors.go +++ b/pkg/sdk/resource_monitors.go @@ -166,9 +166,10 @@ func extractTriggerInts(s sql.NullString) ([]int, error) { ints := strings.Split(s.String, ",") out := make([]int, 0, len(ints)) for _, i := range ints { - myInt, err := strconv.Atoi(i[:len(i)-1]) + numberToParse := i[:len(i)-1] + myInt, err := strconv.Atoi(numberToParse) if err != nil { - return out, fmt.Errorf("failed to convert %v to integer err = %w", i, err) + return out, fmt.Errorf("failed to convert %v to integer err = %w", numberToParse, err) } out = append(out, myInt) } @@ -223,7 +224,6 @@ func (v *resourceMonitors) Create(ctx context.Context, id AccountObjectIdentifie if opts == nil { opts = &CreateResourceMonitorOptions{} } - // TODO: Check conventions for SDK opts.name = id return validateAndExec(v.client, ctx, opts) } diff --git a/pkg/sdk/resource_monitors_test.go b/pkg/sdk/resource_monitors_test.go index 2c18aa6d32..68148ee0d1 100644 --- a/pkg/sdk/resource_monitors_test.go +++ b/pkg/sdk/resource_monitors_test.go @@ -2,8 +2,11 @@ package sdk import ( "database/sql" + "strconv" "testing" "time" + + "github.com/stretchr/testify/require" ) func TestResourceMonitorCreate(t *testing.T) { @@ -178,25 +181,29 @@ func TestResourceMonitorShow(t *testing.T) { }) } -// TODO: Make new tests func TestExtractTriggerInts(t *testing.T) { - // TODO rewrite to use testify/assert - resp := sql.NullString{String: "51%,63%", Valid: true} - out, err := extractTriggerInts(resp) - if err != nil { - t.Error(err) - } - if l := len(out); l != 2 { - t.Errorf("Expected 2 values, got %d", l) - } - - first := 51 - if out[0] != first { - t.Errorf("Expected first value to be 51, got %d", out[0]) + testCases := []struct { + Input sql.NullString + Expected []int + Error string + }{ + {Input: sql.NullString{String: "51%,63%,123%", Valid: true}, Expected: []int{51, 63, 123}}, + {Input: sql.NullString{String: "51%,63%", Valid: true}, Expected: []int{51, 63}}, + {Input: sql.NullString{String: "51%", Valid: true}, Expected: []int{51}}, + {Input: sql.NullString{String: "", Valid: false}, Expected: []int{}}, + {Input: sql.NullString{String: "51,63", Valid: true}, Expected: []int{5, 6}}, + {Input: sql.NullString{String: "1", Valid: true}, Error: "failed to convert to integer err = strconv.Atoi"}, } - second := 63 - if out[1] != second { - t.Errorf("Expected second value to be 63, got %d", out[1]) + for _, tc := range testCases { + t.Run("extract trigger ints: "+tc.Input.String+":"+strconv.FormatBool(tc.Input.Valid), func(t *testing.T) { + result, err := extractTriggerInts(tc.Input) + if tc.Error != "" { + require.ErrorContains(t, err, tc.Error) + } else { + require.NoError(t, err) + require.Equal(t, tc.Expected, result) + } + }) } } diff --git a/pkg/sdk/testint/resource_monitors_integration_test.go b/pkg/sdk/testint/resource_monitors_integration_test.go index 075b41f6c8..a63625d559 100644 --- a/pkg/sdk/testint/resource_monitors_integration_test.go +++ b/pkg/sdk/testint/resource_monitors_integration_test.go @@ -4,6 +4,9 @@ import ( "testing" "time" + assertions "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert/objectassert" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/collections" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" @@ -96,32 +99,17 @@ func TestInt_ResourceMonitorCreate(t *testing.T) { t.Cleanup(testClientHelper().ResourceMonitor.DropResourceMonitorFunc(t, id)) - resourceMonitors, err := client.ResourceMonitors.Show(ctx, &sdk.ShowResourceMonitorOptions{ - Like: &sdk.Like{ - Pattern: sdk.String(name), - }, - }) - require.NoError(t, err) - - assert.Equal(t, 1, len(resourceMonitors)) - resourceMonitor := resourceMonitors[0] - require.NoError(t, err) - assert.Equal(t, name, resourceMonitor.Name) - assert.NotEmpty(t, resourceMonitor.CreatedOn) - assert.Equal(t, frequency, resourceMonitor.Frequency) - assert.Equal(t, creditQuota, int(resourceMonitor.CreditQuota)) - assert.NotEmpty(t, resourceMonitor.StartTime) - assert.NotEmpty(t, resourceMonitor.EndTime) - assert.Equal(t, creditQuota, int(resourceMonitor.CreditQuota)) - var allThresholds []int - allThresholds = append(allThresholds, *resourceMonitor.SuspendAt) - allThresholds = append(allThresholds, *resourceMonitor.SuspendImmediateAt) - allThresholds = append(allThresholds, resourceMonitor.NotifyAt...) - var thresholds []int - for _, trigger := range triggers { - thresholds = append(thresholds, trigger.Threshold) - } - assert.Equal(t, thresholds, allThresholds) + assertions.AssertThat(t, + objectassert.ResourceMonitor(t, id). + HasName(name). + HasFrequency(frequency). + HasCreditQuota(float64(creditQuota)). + HasNonEmptyStartTime(). + HasNonEmptyEndTime(). + HasNotifyAt([]int{100}). + HasSuspendAt(30). + HasSuspendImmediateAt(50), + ) }) t.Run("validate: only one suspend trigger", func(t *testing.T) { @@ -169,27 +157,21 @@ func TestInt_ResourceMonitorCreate(t *testing.T) { name := id.Name() err := client.ResourceMonitors.Create(ctx, id, nil) - t.Cleanup(testClientHelper().ResourceMonitor.DropResourceMonitorFunc(t, id)) - require.NoError(t, err) - resourceMonitors, err := client.ResourceMonitors.Show(ctx, &sdk.ShowResourceMonitorOptions{ - Like: &sdk.Like{ - Pattern: sdk.String(name), - }, - }) + t.Cleanup(testClientHelper().ResourceMonitor.DropResourceMonitorFunc(t, id)) - assert.Equal(t, 1, len(resourceMonitors)) - resourceMonitor := resourceMonitors[0] - require.NoError(t, err) - assert.Equal(t, name, resourceMonitor.Name) - assert.NotEmpty(t, resourceMonitor.StartTime) - assert.Empty(t, resourceMonitor.EndTime) - assert.Empty(t, resourceMonitor.CreditQuota) - assert.Equal(t, sdk.FrequencyMonthly, resourceMonitor.Frequency) - assert.Empty(t, resourceMonitor.NotifyUsers) - assert.Empty(t, resourceMonitor.NotifyAt) - assert.Empty(t, resourceMonitor.SuspendAt) - assert.Empty(t, resourceMonitor.SuspendImmediateAt) + assertions.AssertThat(t, + objectassert.ResourceMonitor(t, id). + HasName(name). + HasFrequency(sdk.FrequencyMonthly). + HasNonEmptyStartTime(). + HasCreditQuota(0). + HasEndTime(""). + HasNotifyUsers([]string{}). + HasNotifyAt([]int{}). + HasSuspendAt(0). + HasSuspendImmediateAt(0), + ) }) } @@ -258,7 +240,7 @@ func TestInt_ResourceMonitorAlter(t *testing.T) { resourceMonitor, err = client.ResourceMonitors.ShowByID(ctx, resourceMonitor.ID()) require.NoError(t, err) - assert.Nil(t, resourceMonitor.CreditQuota) + assert.Equal(t, 0, resourceMonitor.CreditQuota) }) t.Run("when changing notify users", func(t *testing.T) { @@ -312,8 +294,8 @@ func TestInt_ResourceMonitorAlter(t *testing.T) { require.NoError(t, err) assert.Equal(t, frequency, resourceMonitor.Frequency) - assert.Equal(t, startTimeStamp, resourceMonitor.StartTime) - assert.Equal(t, endTimeStamp, resourceMonitor.EndTime) + assert.NotEmpty(t, resourceMonitor.StartTime) + assert.NotEmpty(t, resourceMonitor.EndTime) err = client.ResourceMonitors.Alter(ctx, resourceMonitor.ID(), &sdk.AlterResourceMonitorOptions{ Unset: &sdk.ResourceMonitorUnset{ @@ -325,7 +307,8 @@ func TestInt_ResourceMonitorAlter(t *testing.T) { resourceMonitor, err = client.ResourceMonitors.ShowByID(ctx, resourceMonitor.ID()) require.NoError(t, err) - assert.Nil(t, resourceMonitor.EndTime) + assert.NotEmpty(t, resourceMonitor.StartTime) + assert.Empty(t, resourceMonitor.EndTime) }) t.Run("all options together", func(t *testing.T) { @@ -340,21 +323,19 @@ func TestInt_ResourceMonitorAlter(t *testing.T) { Set: &sdk.ResourceMonitorSet{ CreditQuota: &creditQuota, NotifyUsers: &sdk.NotifyUsers{ - Users: []sdk.NotifiedUser{{Name: sdk.NewAccountObjectIdentifier("TERRAFORM_SVC_ACCOUNT")}}, + Users: []sdk.NotifiedUser{{Name: sdk.NewAccountObjectIdentifier("JAN_CIESLAK")}}, }, }, Triggers: newTriggers, }) require.NoError(t, err) - resourceMonitor, err = client.ResourceMonitors.ShowByID(ctx, resourceMonitor.ID()) - require.NoError(t, err) - assert.NotNil(t, resourceMonitor.CreditQuota) - assert.Equal(t, creditQuota, int(resourceMonitor.CreditQuota)) - assert.Len(t, resourceMonitor.NotifyUsers, 1) - assert.Equal(t, "TERRAFORM_SVC_ACCOUNT", resourceMonitor.NotifyUsers[0]) - assert.Len(t, resourceMonitor.NotifyAt, 1) - assert.Equal(t, 30, resourceMonitor.NotifyAt[0]) + assertions.AssertThat(t, + objectassert.ResourceMonitor(t, resourceMonitor.ID()). + HasCreditQuota(float64(creditQuota)). + HasNotifyUsers([]string{"JAN_CIESLAK"}). + HasNotifyAt([]int{30}), + ) }) }