diff --git a/framework/provider/resource_monitor_resource.go b/framework/provider/resource_monitor_resource.go index 921387a376..588c509e19 100644 --- a/framework/provider/resource_monitor_resource.go +++ b/framework/provider/resource_monitor_resource.go @@ -736,7 +736,7 @@ func (r *ResourceMonitorResource) update(ctx context.Context, plan *resourceMoni for _, e := range elements { notifiedUsers = append(notifiedUsers, sdk.NotifiedUser{Name: e.ValueString()}) } - opts.NotifyUsers = &sdk.NotifyUsers{ + opts.Set.NotifyUsers = &sdk.NotifyUsers{ Users: notifiedUsers, } } diff --git a/pkg/resources/resource_monitor.go b/pkg/resources/resource_monitor.go index 59e416c69e..e4ad6a196f 100644 --- a/pkg/resources/resource_monitor.go +++ b/pkg/resources/resource_monitor.go @@ -317,24 +317,12 @@ func UpdateResourceMonitor(d *schema.ResourceData, meta interface{}) error { ctx := context.Background() var runSetStatement bool - opts := sdk.AlterResourceMonitorOptions{Set: &sdk.ResourceMonitorSet{}} - - if d.HasChange("notify_users") { - runSetStatement = true - - userNames := expandStringList(d.Get("notify_users").(*schema.Set).List()) - users := []sdk.NotifiedUser{} - for _, name := range userNames { - users = append(users, sdk.NotifiedUser{Name: name}) - } - opts.NotifyUsers = &sdk.NotifyUsers{ - Users: users, - } - } + opts := sdk.AlterResourceMonitorOptions{} + set := sdk.ResourceMonitorSet{} if d.HasChange("credit_quota") { runSetStatement = true - opts.Set.CreditQuota = sdk.Pointer(d.Get("credit_quota").(int)) + set.CreditQuota = sdk.Pointer(d.Get("credit_quota").(int)) } if d.HasChange("frequency") || d.HasChange("start_timestamp") { @@ -343,13 +331,30 @@ func UpdateResourceMonitor(d *schema.ResourceData, meta interface{}) error { if err != nil { return err } - opts.Set.Frequency = frequency - opts.Set.StartTimestamp = sdk.Pointer(d.Get("start_timestamp").(string)) + set.Frequency = frequency + set.StartTimestamp = sdk.Pointer(d.Get("start_timestamp").(string)) } if d.HasChange("end_timestamp") { runSetStatement = true - opts.Set.EndTimestamp = sdk.Pointer(d.Get("end_timestamp").(string)) + set.EndTimestamp = sdk.Pointer(d.Get("end_timestamp").(string)) + } + + if d.HasChange("notify_users") { + runSetStatement = true + + userNames := expandStringList(d.Get("notify_users").(*schema.Set).List()) + users := []sdk.NotifiedUser{} + for _, name := range userNames { + users = append(users, sdk.NotifiedUser{Name: name}) + } + set.NotifyUsers = &sdk.NotifyUsers{ + Users: users, + } + } + + if set != (sdk.ResourceMonitorSet{}) { + opts.Set = &set } // If ANY of the triggers changed, we collect all triggers and set them diff --git a/pkg/resources/resource_monitor_acceptance_test.go b/pkg/resources/resource_monitor_acceptance_test.go index ec340de422..ff121977ca 100644 --- a/pkg/resources/resource_monitor_acceptance_test.go +++ b/pkg/resources/resource_monitor_acceptance_test.go @@ -36,7 +36,7 @@ func TestAcc_ResourceMonitor(t *testing.T) { }, // CHANGE PROPERTIES { - Config: resourceMonitorConfig2(name), + Config: resourceMonitorConfig2(name, 75), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "name", name), resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "credit_quota", "150"), @@ -46,6 +46,18 @@ func TestAcc_ResourceMonitor(t *testing.T) { resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "suspend_immediate_trigger", "95"), ), }, + // CHANGE JUST suspend_trigger; proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2316 + { + Config: resourceMonitorConfig2(name, 60), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "name", name), + resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "credit_quota", "150"), + resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "set_for_account", "true"), + resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "notify_triggers.0", "50"), + resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "suspend_trigger", "60"), + resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "suspend_immediate_trigger", "95"), + ), + }, // IMPORT { ResourceName: "snowflake_resource_monitor.test", @@ -198,7 +210,7 @@ resource "snowflake_resource_monitor" "test" { `, accName) } -func resourceMonitorConfig2(accName string) string { +func resourceMonitorConfig2(accName string, suspendTrigger int) string { return fmt.Sprintf(` resource "snowflake_warehouse" "warehouse" { name = "test" @@ -212,10 +224,10 @@ resource "snowflake_resource_monitor" "test" { set_for_account = true notify_triggers = [50] warehouses = [] - suspend_trigger = 75 + suspend_trigger = %d suspend_immediate_trigger = 95 } -`, accName) +`, accName, suspendTrigger) } // TestAcc_ResourceMonitor_issue2167 proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2167 issue. diff --git a/pkg/resources/warehouse.go b/pkg/resources/warehouse.go index 2c7e053899..7116155b66 100644 --- a/pkg/resources/warehouse.go +++ b/pkg/resources/warehouse.go @@ -5,6 +5,7 @@ import ( "database/sql" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/logging" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" snowflakevalidation "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/validation" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -264,6 +265,12 @@ func ReadWarehouse(d *schema.ResourceData, meta interface{}) error { if err = d.Set("enable_query_acceleration", w.EnableQueryAcceleration); err != nil { return err } + + err = readWarehouseObjectProperties(d, id, client, ctx) + if err != nil { + return err + } + if w.EnableQueryAcceleration { if err = d.Set("query_acceleration_max_scale_factor", w.QueryAccelerationMaxScaleFactor); err != nil { return err @@ -273,6 +280,37 @@ func ReadWarehouse(d *schema.ResourceData, meta interface{}) error { return nil } +func readWarehouseObjectProperties(d *schema.ResourceData, warehouseId sdk.AccountObjectIdentifier, client *sdk.Client, ctx context.Context) error { + statementTimeoutInSecondsParameter, err := client.Parameters.ShowObjectParameter(ctx, "STATEMENT_TIMEOUT_IN_SECONDS", sdk.Object{ObjectType: sdk.ObjectTypeWarehouse, Name: warehouseId}) + if err != nil { + return err + } + logging.DebugLogger.Printf("[DEBUG] STATEMENT_TIMEOUT_IN_SECONDS parameter was fetched: %v", statementTimeoutInSecondsParameter) + if err = d.Set("statement_timeout_in_seconds", sdk.ToInt(statementTimeoutInSecondsParameter.Value)); err != nil { + return err + } + + statementQueuedTimeoutInSecondsParameter, err := client.Parameters.ShowObjectParameter(ctx, "STATEMENT_QUEUED_TIMEOUT_IN_SECONDS", sdk.Object{ObjectType: sdk.ObjectTypeWarehouse, Name: warehouseId}) + if err != nil { + return err + } + logging.DebugLogger.Printf("[DEBUG] STATEMENT_QUEUED_TIMEOUT_IN_SECONDS parameter was fetched: %v", statementQueuedTimeoutInSecondsParameter) + if err = d.Set("statement_queued_timeout_in_seconds", sdk.ToInt(statementQueuedTimeoutInSecondsParameter.Value)); err != nil { + return err + } + + maxConcurrencyLevelParameter, err := client.Parameters.ShowObjectParameter(ctx, "MAX_CONCURRENCY_LEVEL", sdk.Object{ObjectType: sdk.ObjectTypeWarehouse, Name: warehouseId}) + if err != nil { + return err + } + logging.DebugLogger.Printf("[DEBUG] MAX_CONCURRENCY_LEVEL parameter was fetched: %v", maxConcurrencyLevelParameter) + if err = d.Set("max_concurrency_level", sdk.ToInt(maxConcurrencyLevelParameter.Value)); err != nil { + return err + } + + return nil +} + // UpdateWarehouse implements schema.UpdateFunc. func UpdateWarehouse(d *schema.ResourceData, meta interface{}) error { db := meta.(*sql.DB) diff --git a/pkg/resources/warehouse_acceptance_test.go b/pkg/resources/warehouse_acceptance_test.go index 48eb3e5ead..70ecc481cb 100644 --- a/pkg/resources/warehouse_acceptance_test.go +++ b/pkg/resources/warehouse_acceptance_test.go @@ -1,14 +1,18 @@ package resources_test import ( + "context" "fmt" "os" "strings" "testing" acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/plancheck" + "github.com/stretchr/testify/require" ) func TestAcc_Warehouse(t *testing.T) { @@ -31,6 +35,7 @@ func TestAcc_Warehouse(t *testing.T) { resource.TestCheckResourceAttr("snowflake_warehouse.w", "comment", "test comment"), resource.TestCheckResourceAttr("snowflake_warehouse.w", "auto_suspend", "60"), resource.TestCheckResourceAttrSet("snowflake_warehouse.w", "warehouse_size"), + resource.TestCheckResourceAttr("snowflake_warehouse.w", "max_concurrency_level", "8"), ), }, // RENAME @@ -45,14 +50,45 @@ func TestAcc_Warehouse(t *testing.T) { }, // CHANGE PROPERTIES { - Config: wConfig2(prefix2, "X-LARGE"), + Config: wConfig2(prefix2, "X-LARGE", 20), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("snowflake_warehouse.w", "name", prefix2), resource.TestCheckResourceAttr("snowflake_warehouse.w", "comment", "test comment 2"), resource.TestCheckResourceAttr("snowflake_warehouse.w", "auto_suspend", "60"), resource.TestCheckResourceAttr("snowflake_warehouse.w", "warehouse_size", "XLARGE"), + resource.TestCheckResourceAttr("snowflake_warehouse.w", "max_concurrency_level", "20"), ), }, + // CHANGE JUST max_concurrency_level + { + Config: wConfig2(prefix2, "XLARGE", 16), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{plancheck.ExpectNonEmptyPlan()}, + }, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_warehouse.w", "name", prefix2), + resource.TestCheckResourceAttr("snowflake_warehouse.w", "comment", "test comment 2"), + resource.TestCheckResourceAttr("snowflake_warehouse.w", "auto_suspend", "60"), + resource.TestCheckResourceAttr("snowflake_warehouse.w", "warehouse_size", "XLARGE"), + resource.TestCheckResourceAttr("snowflake_warehouse.w", "max_concurrency_level", "16"), + ), + }, + // CHANGE max_concurrency_level EXTERNALLY proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2318 + { + PreConfig: func() { alterWarehouseMaxConcurrencyLevelExternally(t, prefix2, 10) }, + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{plancheck.ExpectNonEmptyPlan()}, + }, + Config: wConfig2(prefix2, "XLARGE", 16), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_warehouse.w", "name", prefix2), + resource.TestCheckResourceAttr("snowflake_warehouse.w", "comment", "test comment 2"), + resource.TestCheckResourceAttr("snowflake_warehouse.w", "auto_suspend", "60"), + resource.TestCheckResourceAttr("snowflake_warehouse.w", "warehouse_size", "XLARGE"), + resource.TestCheckResourceAttr("snowflake_warehouse.w", "max_concurrency_level", "16"), + ), + }, + // IMPORT { ResourceName: "snowflake_warehouse.w", @@ -112,7 +148,7 @@ resource "snowflake_warehouse" "w" { return fmt.Sprintf(s, prefix) } -func wConfig2(prefix string, size string) string { +func wConfig2(prefix string, size string, maxConcurrencyLevel int) string { s := ` resource "snowflake_warehouse" "w" { name = "%s" @@ -126,9 +162,10 @@ resource "snowflake_warehouse" "w" { auto_resume = true initially_suspended = true wait_for_provisioning = false + max_concurrency_level = %d } ` - return fmt.Sprintf(s, prefix, size) + return fmt.Sprintf(s, prefix, size, maxConcurrencyLevel) } func wConfigPattern(prefix string) string { @@ -142,3 +179,14 @@ resource "snowflake_warehouse" "w2" { ` return fmt.Sprintf(s, prefix, prefix) } + +func alterWarehouseMaxConcurrencyLevelExternally(t *testing.T, warehouseId string, level int) { + t.Helper() + + client, err := sdk.NewDefaultClient() + require.NoError(t, err) + ctx := context.Background() + + err = client.Warehouses.Alter(ctx, sdk.NewAccountObjectIdentifier(warehouseId), &sdk.AlterWarehouseOptions{Set: &sdk.WarehouseSet{MaxConcurrencyLevel: sdk.Int(level)}}) + require.NoError(t, err) +} diff --git a/pkg/sdk/resource_monitors.go b/pkg/sdk/resource_monitors.go index 1363f38b1e..16098547c8 100644 --- a/pkg/sdk/resource_monitors.go +++ b/pkg/sdk/resource_monitors.go @@ -288,7 +288,6 @@ type AlterResourceMonitorOptions struct { IfExists *bool `ddl:"keyword" sql:"IF EXISTS"` name AccountObjectIdentifier `ddl:"identifier"` Set *ResourceMonitorSet `ddl:"keyword" sql:"SET"` - NotifyUsers *NotifyUsers `ddl:"parameter,equals" sql:"NOTIFY_USERS"` Triggers []TriggerDefinition `ddl:"keyword,no_comma" sql:"TRIGGERS"` } @@ -300,11 +299,14 @@ func (opts *AlterResourceMonitorOptions) validate() error { if !ValidObjectIdentifier(opts.name) { errs = append(errs, ErrInvalidObjectIdentifier) } - if everyValueNil(opts.Set, opts.NotifyUsers, opts.Triggers) { - errs = append(errs, errAtLeastOneOf("AlterResourceMonitorOptions", "Set", "NotifyUsers", "Triggers")) + if everyValueNil(opts.Set, opts.Triggers) { + errs = append(errs, errAtLeastOneOf("AlterResourceMonitorOptions", "Set", "Triggers")) } - if valueSet(opts.Set) { - if (opts.Set.Frequency != nil && opts.Set.StartTimestamp == nil) || (opts.Set.Frequency == nil && opts.Set.StartTimestamp != nil) { + if set := opts.Set; valueSet(set) { + if everyValueNil(set.CreditQuota, set.Frequency, set.StartTimestamp, set.EndTimestamp, set.NotifyUsers) { + errs = append(errs, errAtLeastOneOf("ResourceMonitorSet", "CreditQuota", "Frequency", "StartTimestamp", "EndTimestamp", "NotifyUsers")) + } + if (set.Frequency != nil && set.StartTimestamp == nil) || (set.Frequency == nil && set.StartTimestamp != nil) { errs = append(errs, errors.New("must specify frequency and start time together")) } } @@ -330,10 +332,11 @@ func (v *resourceMonitors) Alter(ctx context.Context, id AccountObjectIdentifier type ResourceMonitorSet struct { // at least one - CreditQuota *int `ddl:"parameter,equals" sql:"CREDIT_QUOTA"` - Frequency *Frequency `ddl:"parameter,equals" sql:"FREQUENCY"` - StartTimestamp *string `ddl:"parameter,equals,single_quotes" sql:"START_TIMESTAMP"` - EndTimestamp *string `ddl:"parameter,equals,single_quotes" sql:"END_TIMESTAMP"` + CreditQuota *int `ddl:"parameter,equals" sql:"CREDIT_QUOTA"` + Frequency *Frequency `ddl:"parameter,equals" sql:"FREQUENCY"` + StartTimestamp *string `ddl:"parameter,equals,single_quotes" sql:"START_TIMESTAMP"` + EndTimestamp *string `ddl:"parameter,equals,single_quotes" sql:"END_TIMESTAMP"` + NotifyUsers *NotifyUsers `ddl:"parameter,equals" sql:"NOTIFY_USERS"` } // dropResourceMonitorOptions is based on https://docs.snowflake.com/en/sql-reference/sql/drop-resource-monitor. diff --git a/pkg/sdk/resource_monitors_test.go b/pkg/sdk/resource_monitors_test.go index 532820bac1..b44d0ec85b 100644 --- a/pkg/sdk/resource_monitors_test.go +++ b/pkg/sdk/resource_monitors_test.go @@ -62,7 +62,15 @@ func TestResourceMonitorAlter(t *testing.T) { opts := &AlterResourceMonitorOptions{ name: id, } - assertOptsInvalidJoinedErrors(t, opts, errAtLeastOneOf("AlterResourceMonitorOptions", "Set", "NotifyUsers", "Triggers")) + assertOptsInvalidJoinedErrors(t, opts, errAtLeastOneOf("AlterResourceMonitorOptions", "Set", "Triggers")) + }) + + t.Run("validation: no option for set provided", func(t *testing.T) { + opts := &AlterResourceMonitorOptions{ + name: id, + Set: &ResourceMonitorSet{}, + } + assertOptsInvalidJoinedErrors(t, opts, errAtLeastOneOf("ResourceMonitorSet", "CreditQuota", "Frequency", "StartTimestamp", "EndTimestamp", "NotifyUsers")) }) t.Run("with a single set", func(t *testing.T) { @@ -76,6 +84,21 @@ func TestResourceMonitorAlter(t *testing.T) { assertOptsValidAndSQLEquals(t, opts, "ALTER RESOURCE MONITOR %s SET CREDIT_QUOTA = %d", id.FullyQualifiedName(), *newCreditQuota) }) + t.Run("set notify users", func(t *testing.T) { + opts := &AlterResourceMonitorOptions{ + name: id, + Set: &ResourceMonitorSet{ + NotifyUsers: &NotifyUsers{ + Users: []NotifiedUser{ + {Name: "user1"}, + {Name: "user2"}, + }, + }, + }, + } + assertOptsValidAndSQLEquals(t, opts, "ALTER RESOURCE MONITOR %s SET NOTIFY_USERS = (\"user1\", \"user2\")", id.FullyQualifiedName()) + }) + t.Run("with a multitple set", func(t *testing.T) { newCreditQuota := Int(50) newFrequency := FrequencyYearly diff --git a/pkg/sdk/testint/resource_monitors_integration_test.go b/pkg/sdk/testint/resource_monitors_integration_test.go index 0a359dc9cc..106a118f85 100644 --- a/pkg/sdk/testint/resource_monitors_integration_test.go +++ b/pkg/sdk/testint/resource_monitors_integration_test.go @@ -211,6 +211,30 @@ func TestInt_ResourceMonitorAlter(t *testing.T) { assert.Equal(t, creditQuota, int(resourceMonitor.CreditQuota)) }) + t.Run("when changing notify users", func(t *testing.T) { + resourceMonitor, resourceMonitorCleanup := createResourceMonitor(t, client) + t.Cleanup(resourceMonitorCleanup) + alterOptions := &sdk.AlterResourceMonitorOptions{ + Set: &sdk.ResourceMonitorSet{ + NotifyUsers: &sdk.NotifyUsers{ + Users: []sdk.NotifiedUser{{Name: "ARTUR_SAWICKI"}}, + }, + }, + } + err := client.ResourceMonitors.Alter(ctx, resourceMonitor.ID(), alterOptions) + require.NoError(t, err) + resourceMonitors, err := client.ResourceMonitors.Show(ctx, &sdk.ShowResourceMonitorOptions{ + Like: &sdk.Like{ + Pattern: sdk.String(resourceMonitor.Name), + }, + }) + require.NoError(t, err) + assert.Equal(t, 1, len(resourceMonitors)) + resourceMonitor = &resourceMonitors[0] + assert.Len(t, resourceMonitor.NotifyUsers, 1) + assert.Equal(t, "ARTUR_SAWICKI", resourceMonitor.NotifyUsers[0]) + }) + t.Run("when changing scheduling info", func(t *testing.T) { resourceMonitor, resourceMonitorCleanup := createResourceMonitor(t, client) t.Cleanup(resourceMonitorCleanup) @@ -256,11 +280,11 @@ func TestInt_ResourceMonitorAlter(t *testing.T) { alterOptions := &sdk.AlterResourceMonitorOptions{ Set: &sdk.ResourceMonitorSet{ CreditQuota: &creditQuota, + NotifyUsers: &sdk.NotifyUsers{ + Users: []sdk.NotifiedUser{{Name: "ARTUR_SAWICKI"}}, + }, }, Triggers: newTriggers, - NotifyUsers: &sdk.NotifyUsers{ - Users: []sdk.NotifiedUser{{Name: "ARTUR_SAWICKI"}}, - }, } err := client.ResourceMonitors.Alter(ctx, resourceMonitor.ID(), alterOptions) require.NoError(t, err)