Skip to content

Commit

Permalink
fix: 0.60 misc bug fixes / linting (#1643)
Browse files Browse the repository at this point in the history
* fix resource monitor

* fix role grants id parsing

* update docs

* fix unit test
  • Loading branch information
sfc-gh-swinkler authored Mar 23, 2023
1 parent 78782b1 commit 53da853
Show file tree
Hide file tree
Showing 6 changed files with 100 additions and 8 deletions.
2 changes: 2 additions & 0 deletions docs/resources/resource_monitor.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ resource "snowflake_resource_monitor" "monitor" {
- `set_for_account` (Boolean) Specifies whether the resource monitor should be applied globally to your Snowflake account (defaults to false).
- `start_timestamp` (String) The date and time when the resource monitor starts monitoring credit usage for the assigned warehouses.
- `suspend_immediate_trigger` (Number) The number that represents the percentage threshold at which to immediately suspend all warehouses.
- `suspend_immediate_triggers` (Set of Number, Deprecated) A list of percentage thresholds at which to suspend all warehouses.
- `suspend_trigger` (Number) The number that represents the percentage threshold at which to suspend all warehouses.
- `suspend_triggers` (Set of Number, Deprecated) A list of percentage thresholds at which to suspend all warehouses.
- `warehouses` (Set of String) A list of warehouses to apply the resource monitor to.

### Read-Only
Expand Down
76 changes: 70 additions & 6 deletions pkg/resources/resource_monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,26 @@ var resourceMonitorSchema = map[string]*schema.Schema{
Optional: true,
Description: "The number that represents the percentage threshold at which to suspend all warehouses.",
},
"suspend_triggers": {
Type: schema.TypeSet,
Elem: &schema.Schema{Type: schema.TypeInt},
Optional: true,
Description: "A list of percentage thresholds at which to suspend all warehouses.",
Deprecated: "Use suspend_trigger instead",
},
"suspend_immediate_trigger": {
Type: schema.TypeInt,
Optional: true,
Description: "The number that represents the percentage threshold at which to immediately suspend all warehouses.",
},
"suspend_immediate_triggers": {
Type: schema.TypeSet,
Elem: &schema.Schema{Type: schema.TypeInt},
Optional: true,
Description: "A list of percentage thresholds at which to suspend all warehouses.",
Deprecated: "Use suspend_immediate_trigger instead",
},

"notify_triggers": {
Type: schema.TypeSet,
Elem: &schema.Schema{Type: schema.TypeInt},
Expand Down Expand Up @@ -121,7 +136,7 @@ func CreateResourceMonitor(d *schema.ResourceData, meta interface{}) error {
}

cb := snowflake.NewResourceMonitorBuilder(name).Create()
// Set optionals
// Set optionals.
if v, ok := d.GetOk("notify_users"); ok {
cb.SetStringList("notify_users", expandStringList(v.(*schema.Set).List()))
}
Expand All @@ -137,13 +152,26 @@ func CreateResourceMonitor(d *schema.ResourceData, meta interface{}) error {
if v, ok := d.GetOk("end_timestamp"); ok {
cb.SetString("end_timestamp", v.(string))
}
// Set triggers
if v, ok := d.GetOk("suspend_trigger"); ok {
cb.SuspendAt(v.(int))
}
// Support deprecated suspend_triggers.
if v, ok := d.GetOk("suspend_triggers"); ok {
siTrigs := expandIntList(v.(*schema.Set).List())
for _, t := range siTrigs {
cb.SuspendImmediatelyAt(t)
}
}
if v, ok := d.GetOk("suspend_immediate_trigger"); ok {
cb.SuspendImmediatelyAt(v.(int))
}
// Support deprecated suspend_immediate_triggers.
if v, ok := d.GetOk("suspend_immediate_triggers"); ok {
sTrigs := expandIntList(v.(*schema.Set).List())
for _, t := range sTrigs {
cb.SuspendAt(t)
}
}
nTrigs := expandIntList(d.Get("notify_triggers").(*schema.Set).List())
for _, t := range nTrigs {
cb.NotifyAt(t)
Expand Down Expand Up @@ -228,15 +256,29 @@ func ReadResourceMonitor(d *schema.ResourceData, meta interface{}) error {
if err != nil {
return err
}
if err := d.Set("suspend_trigger", sTrig); err != nil {
return err

if len(sTrig) > 0 {
if err := d.Set("suspend_trigger", sTrig[0]); err != nil {
return err
}
} else {
if err := d.Set("suspend_trigger", nil); err != nil {
return err
}
}
siTrig, err := extractTriggerInts(rm.SuspendImmediatelyAt)
if err != nil {
return err
}
if err := d.Set("suspend_immediate_trigger", siTrig); err != nil {
return err

if len(siTrig) > 0 {
if err := d.Set("suspend_immediate_trigger", siTrig[0]); err != nil {
return err
}
} else {
if err := d.Set("suspend_immediate_trigger", nil); err != nil {
return err
}
}
nTrigs, err := extractTriggerInts(rm.NotifyAt)
if err != nil {
Expand Down Expand Up @@ -333,10 +375,32 @@ func UpdateResourceMonitor(d *schema.ResourceData, meta interface{}) error {
runSetStatement = true
ub.SuspendAt(d.Get("suspend_trigger").(int))
}
if d.HasChange("suspend_triggers") {
runSetStatement = true
ub.SuspendAt(d.Get("suspend_triggers").(int))
}
// Support deprecated suspend_triggers.
if d.HasChange("suspend_triggers") {
runSetStatement = true
siTrigs := expandIntList(d.Get("suspend_triggers").(*schema.Set).List())
for _, t := range siTrigs {
ub.SuspendAt(t)
}
}
if d.HasChange("suspend_immediate_trigger") {
runSetStatement = true
ub.SuspendImmediatelyAt(d.Get("suspend_immediate_trigger").(int))
}

// Support deprecated suspend_immediate_trigger.
if d.HasChange("suspend_immediate_triggers") {
runSetStatement = true
siTrigs := expandIntList(d.Get("suspend_immediate_triggers").(*schema.Set).List())
for _, t := range siTrigs {
ub.SuspendImmediatelyAt(t)
}
}

nTrigs := expandIntList(d.Get("notify_triggers").(*schema.Set).List())
for _, t := range nTrigs {
runSetStatement = true
Expand Down
8 changes: 8 additions & 0 deletions pkg/resources/resource_monitor_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ func TestAcc_ResourceMonitor(t *testing.T) {
resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "credit_quota", "100"),
resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "set_for_account", "false"),
resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "notify_triggers.0", "40"),
resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "suspend_trigger", "80"),
resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "suspend_immediate_trigger", "90"),
),
},
// CHANGE PROPERTIES
Expand All @@ -36,6 +38,8 @@ func TestAcc_ResourceMonitor(t *testing.T) {
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", "75"),
resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "suspend_immediate_trigger", "95"),
),
},
// IMPORT
Expand All @@ -61,6 +65,8 @@ resource "snowflake_resource_monitor" "test" {
credit_quota = 100
set_for_account = false
notify_triggers = [40]
suspend_trigger = 80
suspend_immediate_trigger = 90
warehouses = [snowflake_warehouse.warehouse.id]
}
`, accName)
Expand All @@ -80,6 +86,8 @@ resource "snowflake_resource_monitor" "test" {
set_for_account = true
notify_triggers = [50]
warehouses = []
suspend_trigger = 75
suspend_immediate_trigger = 95
}
`, accName)
}
Expand Down
8 changes: 6 additions & 2 deletions pkg/resources/role_grants.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,11 +327,15 @@ func (v *RoleGrantsID) String() string {
}

func ParseRoleGrantsID(s string) (*RoleGrantsID, error) {
if IsOldGrantID(s) {
if IsOldGrantID(s) || (len(strings.Split(s, "|")) == 1 && !strings.Contains(s, "❄️")) {
idParts := strings.Split(s, "|")
var roles []string
if len(idParts) == 6 {
roles = helpers.SplitStringToSlice(idParts[4], ",")
}
return &RoleGrantsID{
ObjectName: idParts[0],
Roles: helpers.SplitStringToSlice(idParts[4], ","),
Roles: roles,
Users: []string{},
IsOldID: true,
}, nil
Expand Down
9 changes: 9 additions & 0 deletions pkg/resources/role_grants_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,3 +166,12 @@ func TestParseRoleGrantsOldID(t *testing.T) {
r.Equal("role1", grantID.Roles[0])
r.Equal("role2", grantID.Roles[1])
}

func TestParseRoleGrantsReallyOldID(t *testing.T) {
r := require.New(t)

grantID, err := resources.ParseRoleGrantsID("test-role")
r.NoError(err)
r.Equal("test-role", grantID.ObjectName)
r.Equal(0, len(grantID.Roles))
}
5 changes: 5 additions & 0 deletions pkg/snowflake/resource_monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,11 @@ func (rcb *ResourceMonitorAlterBuilder) Statement() string {
sb.WriteString(fmt.Sprintf(" %s=%s", strings.ToUpper(k), formatStringList(v)))
}

// If the only change is the trigges, then we do not need to add the SET keyword
if strings.HasSuffix(sb.String(), "SET") {
sb.Reset()
sb.WriteString(fmt.Sprintf(`ALTER %v "%v"`, rcb.entityType, rcb.name))
}
if len(rcb.triggers) > 0 {
sb.WriteString(" TRIGGERS")
}
Expand Down

0 comments on commit 53da853

Please sign in to comment.