Skip to content

Commit

Permalink
fix: Fix issues 2606 2642 2652 2653 (#2654)
Browse files Browse the repository at this point in the history
- Fix unsetting comment on schema (references #2606)
- Fix changing the execute as on procedure (references #2642)
- Fix setting only min cluster count on warehouse (references #2652)
- Add missing S3_STAGE_VPCE_DNS_NAME session parameter (references
#2653)
  • Loading branch information
sfc-gh-asawicki authored Mar 27, 2024
1 parent 3c3ede6 commit 4a73721
Show file tree
Hide file tree
Showing 24 changed files with 139 additions and 55 deletions.
5 changes: 5 additions & 0 deletions MIGRATION_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ This document is meant to help you migrate your Terraform config to the new newe
describe deprecations or breaking changes and help you to change your configuration to keep the same (or similar) behavior
across different versions.

## v0.87.0 ➞ v0.88.0
### snowflake_procedure resource changes
#### *(behavior change)* Execute as validation added
From now on, the `snowflake_procedure`'s `execute_as` parameter allows only two values: OWNER and CALLER (case-insensitive). Setting other values earlier resulted in falling back to the Snowflake default (currently OWNER) and creating a permadiff.

## v0.86.0 ➞ v0.87.0
### snowflake_database resource changes
#### *(behavior change)* External object identifier changes
Expand Down
2 changes: 1 addition & 1 deletion docs/resources/procedure.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ EOT

- `arguments` (Block List) List of the arguments for the procedure (see [below for nested schema](#nestedblock--arguments))
- `comment` (String) Specifies a comment for the procedure.
- `execute_as` (String) Sets execute context - see caller's rights and owner's rights
- `execute_as` (String) Sets execution context. Allowed values are CALLER and OWNER (consult a proper section in the [docs](https://docs.snowflake.com/en/sql-reference/sql/create-procedure#id1)). For more information see [caller's rights and owner's rights](https://docs.snowflake.com/en/developer-guide/stored-procedure/stored-procedures-rights).
- `handler` (String) The handler method for Java / Python procedures.
- `imports` (List of String) Imports for Java / Python procedures. For Java this a list of jar files, for Python this is a list of Python files.
- `language` (String) Specifies the language of the stored procedure code.
Expand Down
22 changes: 16 additions & 6 deletions pkg/resources/procedure.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,14 @@ var procedureSchema = map[string]*schema.Schema{
Description: "Specifies the language of the stored procedure code.",
},
"execute_as": {
Type: schema.TypeString,
Optional: true,
Default: "OWNER",
Description: "Sets execute context - see caller's rights and owner's rights",
Type: schema.TypeString,
Optional: true,
Default: "OWNER",
DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
return strings.EqualFold(old, new)
},
ValidateFunc: validation.StringInSlice([]string{"CALLER", "OWNER"}, true),
Description: "Sets execution context. Allowed values are CALLER and OWNER (consult a proper section in the [docs](https://docs.snowflake.com/en/sql-reference/sql/create-procedure#id1)). For more information see [caller's rights and owner's rights](https://docs.snowflake.com/en/developer-guide/stored-procedure/stored-procedures-rights).",
},
"null_input_behavior": {
Type: schema.TypeString,
Expand Down Expand Up @@ -679,8 +683,14 @@ func UpdateContextProcedure(ctx context.Context, d *schema.ResourceData, meta in
}
}
if d.HasChange("execute_as") {
executeAs := d.Get("execute_as")
if err := client.Procedures.Alter(ctx, sdk.NewAlterProcedureRequest(id.WithoutArguments(), id.Arguments()).WithExecuteAs(sdk.Pointer(sdk.ExecuteAs(executeAs.(string))))); err != nil {
req := sdk.NewAlterProcedureRequest(id.WithoutArguments(), id.Arguments())
executeAs := d.Get("execute_as").(string)
if strings.ToUpper(executeAs) == "OWNER" {
req.WithExecuteAs(sdk.Pointer(sdk.ExecuteAsOwner))
} else if strings.ToUpper(executeAs) == "CALLER" {
req.WithExecuteAs(sdk.Pointer(sdk.ExecuteAsCaller))
}
if err := client.Procedures.Alter(ctx, req); err != nil {
return diag.FromErr(err)
}
}
Expand Down
24 changes: 14 additions & 10 deletions pkg/resources/procedure_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,16 @@ func testAccProcedure(t *testing.T, configDirectory string) {
resourceName := "snowflake_procedure.p"
m := func() map[string]config.Variable {
return map[string]config.Variable{
"name": config.StringVariable(name),
"database": config.StringVariable(acc.TestDatabaseName),
"schema": config.StringVariable(acc.TestSchemaName),
"comment": config.StringVariable("Terraform acceptance test"),
"name": config.StringVariable(name),
"database": config.StringVariable(acc.TestDatabaseName),
"schema": config.StringVariable(acc.TestSchemaName),
"comment": config.StringVariable("Terraform acceptance test"),
"execute_as": config.StringVariable("CALLER"),
}
}
variableSet2 := m()
variableSet2["comment"] = config.StringVariable("Terraform acceptance test - updated")
variableSet2["execute_as"] = config.StringVariable("OWNER")

ignoreDuringImport := []string{"null_input_behavior"}
if strings.Contains(configDirectory, "/sql") {
Expand All @@ -53,16 +55,16 @@ func testAccProcedure(t *testing.T, configDirectory string) {
resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName),
resource.TestCheckResourceAttr(resourceName, "comment", "Terraform acceptance test"),
resource.TestCheckResourceAttr(resourceName, "return_behavior", "VOLATILE"),
resource.TestCheckResourceAttr(resourceName, "execute_as", "CALLER"),

// computed attributes
resource.TestCheckResourceAttrSet(resourceName, "return_type"),
resource.TestCheckResourceAttrSet(resourceName, "statement"),
resource.TestCheckResourceAttrSet(resourceName, "execute_as"),
resource.TestCheckResourceAttrSet(resourceName, "secure"),
),
},

// test - change comment
// test - change comment and caller (proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2642)
{
ConfigDirectory: acc.ConfigurationDirectory(configDirectory),
ConfigVariables: variableSet2,
Expand All @@ -71,6 +73,7 @@ func testAccProcedure(t *testing.T, configDirectory string) {
resource.TestCheckResourceAttr(resourceName, "database", acc.TestDatabaseName),
resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName),
resource.TestCheckResourceAttr(resourceName, "comment", "Terraform acceptance test - updated"),
resource.TestCheckResourceAttr(resourceName, "execute_as", "OWNER"),
),
},

Expand Down Expand Up @@ -115,10 +118,11 @@ func TestAcc_Procedure_complex(t *testing.T) {
resourceName := "snowflake_procedure.p"
m := func() map[string]config.Variable {
return map[string]config.Variable{
"name": config.StringVariable(name),
"database": config.StringVariable(acc.TestDatabaseName),
"schema": config.StringVariable(acc.TestSchemaName),
"comment": config.StringVariable("Terraform acceptance test"),
"name": config.StringVariable(name),
"database": config.StringVariable(acc.TestDatabaseName),
"schema": config.StringVariable(acc.TestSchemaName),
"comment": config.StringVariable("Terraform acceptance test"),
"execute_as": config.StringVariable("CALLER"),
}
}
variableSet2 := m()
Expand Down
25 changes: 18 additions & 7 deletions pkg/resources/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,13 +197,24 @@ func UpdateSchema(d *schema.ResourceData, meta interface{}) error {

if d.HasChange("comment") {
comment := d.Get("comment")
err := client.Schemas.Alter(ctx, id, &sdk.AlterSchemaOptions{
Set: &sdk.SchemaSet{
Comment: sdk.String(comment.(string)),
},
})
if err != nil {
return fmt.Errorf("error updating schema comment on %v err = %w", d.Id(), err)
if comment != "" {
err := client.Schemas.Alter(ctx, id, &sdk.AlterSchemaOptions{
Set: &sdk.SchemaSet{
Comment: sdk.String(comment.(string)),
},
})
if err != nil {
return fmt.Errorf("error updating schema comment on %v err = %w", d.Id(), err)
}
} else {
err := client.Schemas.Alter(ctx, id, &sdk.AlterSchemaOptions{
Unset: &sdk.SchemaUnset{
Comment: sdk.Bool(true),
},
})
if err != nil {
return fmt.Errorf("error updating schema comment on %v err = %w", d.Id(), err)
}
}
}

Expand Down
16 changes: 16 additions & 0 deletions pkg/resources/schema_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,22 @@ func TestAcc_Schema(t *testing.T) {
checkBool("snowflake_schema.test", "is_managed", false),
),
},
// UPDATE COMMENT (proves issue https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2606)
{
ConfigDirectory: config.TestNameDirectory(),
ConfigVariables: map[string]config.Variable{
"name": config.StringVariable(name),
"database": config.StringVariable(acc.TestDatabaseName),
"comment": config.StringVariable(""),
},
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("snowflake_schema.test", "name", name),
resource.TestCheckResourceAttr("snowflake_schema.test", "database", acc.TestDatabaseName),
resource.TestCheckResourceAttr("snowflake_schema.test", "comment", ""),
checkBool("snowflake_schema.test", "is_transient", false),
checkBool("snowflake_schema.test", "is_managed", false),
),
},
},
})
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/resources/testdata/TestAcc_Procedure/complex/test.tf
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ resource "snowflake_procedure" "p" {
}
language = "JAVASCRIPT"
return_type = "VARCHAR"
execute_as = "CALLER"
execute_as = var.execute_as
null_input_behavior = "RETURNS NULL ON NULL INPUT"
comment = var.comment
statement = <<EOT
Expand Down
4 changes: 4 additions & 0 deletions pkg/resources/testdata/TestAcc_Procedure/complex/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,7 @@ variable "schema" {
variable "comment" {
type = string
}

variable "execute_as" {
type = string
}
2 changes: 1 addition & 1 deletion pkg/resources/testdata/TestAcc_Procedure/java/test.tf
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ resource "snowflake_procedure" "p" {
runtime_version = "11"
packages = ["com.snowflake:snowpark:1.9.0"]
handler = "Filter.filterByRole"
execute_as = "CALLER"
execute_as = var.execute_as
comment = var.comment
statement = <<EOT
import com.snowflake.snowpark_java.*;
Expand Down
4 changes: 4 additions & 0 deletions pkg/resources/testdata/TestAcc_Procedure/java/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,7 @@ variable "schema" {
variable "comment" {
type = string
}

variable "execute_as" {
type = string
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ resource "snowflake_procedure" "p" {
language = "JAVASCRIPT"
return_type = "VARCHAR"
comment = var.comment
execute_as = var.execute_as
statement = <<EOT
return "Hi"
EOT
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,7 @@ variable "schema" {
variable "comment" {
type = string
}

variable "execute_as" {
type = string
}
2 changes: 1 addition & 1 deletion pkg/resources/testdata/TestAcc_Procedure/python/test.tf
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ resource "snowflake_procedure" "p" {
runtime_version = "3.8"
packages = ["snowflake-snowpark-python"]
handler = "filter_by_role"
execute_as = "CALLER"
execute_as = var.execute_as
comment = var.comment
statement = <<EOT
from snowflake.snowpark.functions import col
Expand Down
4 changes: 4 additions & 0 deletions pkg/resources/testdata/TestAcc_Procedure/python/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,7 @@ variable "schema" {
variable "comment" {
type = string
}

variable "execute_as" {
type = string
}
2 changes: 1 addition & 1 deletion pkg/resources/testdata/TestAcc_Procedure/scala/test.tf
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ resource "snowflake_procedure" "p" {
runtime_version = "2.12"
packages = ["com.snowflake:snowpark:1.9.0"]
handler = "Filter.filterByRole"
execute_as = "CALLER"
execute_as = var.execute_as
comment = var.comment
statement = <<EOT
import com.snowflake.snowpark.functions._
Expand Down
4 changes: 4 additions & 0 deletions pkg/resources/testdata/TestAcc_Procedure/scala/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,7 @@ variable "schema" {
variable "comment" {
type = string
}

variable "execute_as" {
type = string
}
2 changes: 1 addition & 1 deletion pkg/resources/testdata/TestAcc_Procedure/sql/test.tf
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ resource "snowflake_procedure" "p" {
}
language = "SQL"
return_type = "VARCHAR"
execute_as = "CALLER"
execute_as = var.execute_as
null_input_behavior = "RETURNS NULL ON NULL INPUT"
comment = var.comment
statement = <<EOT
Expand Down
4 changes: 4 additions & 0 deletions pkg/resources/testdata/TestAcc_Procedure/sql/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,7 @@ variable "schema" {
variable "comment" {
type = string
}

variable "execute_as" {
type = string
}
36 changes: 18 additions & 18 deletions pkg/resources/warehouse_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ func TestAcc_Warehouse(t *testing.T) {
resource.TestCheckResourceAttr("snowflake_warehouse.w", "auto_suspend", "60"),
resource.TestCheckResourceAttrSet("snowflake_warehouse.w", "warehouse_size"),
resource.TestCheckResourceAttr("snowflake_warehouse.w", "max_concurrency_level", "8"),
resource.TestCheckResourceAttr("snowflake_warehouse.w", "min_cluster_count", "1"),
),
},
// RENAME
Expand All @@ -48,20 +49,21 @@ func TestAcc_Warehouse(t *testing.T) {
resource.TestCheckResourceAttrSet("snowflake_warehouse.w", "warehouse_size"),
),
},
// CHANGE PROPERTIES
// CHANGE PROPERTIES (proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2652)
{
Config: wConfig2(prefix2, "X-LARGE", 20),
Config: wConfig2(prefix2, "X-LARGE", 20, 2),
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"),
resource.TestCheckResourceAttr("snowflake_warehouse.w", "min_cluster_count", "2"),
),
},
// CHANGE JUST max_concurrency_level
{
Config: wConfig2(prefix2, "XLARGE", 16),
Config: wConfig2(prefix2, "XLARGE", 16, 2),
ConfigPlanChecks: resource.ConfigPlanChecks{
PreApply: []plancheck.PlanCheck{plancheck.ExpectNonEmptyPlan()},
},
Expand All @@ -73,13 +75,13 @@ func TestAcc_Warehouse(t *testing.T) {
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
// 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),
Config: wConfig2(prefix2, "XLARGE", 16, 2),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("snowflake_warehouse.w", "name", prefix2),
resource.TestCheckResourceAttr("snowflake_warehouse.w", "comment", "test comment 2"),
Expand Down Expand Up @@ -130,41 +132,39 @@ func TestAcc_WarehousePattern(t *testing.T) {
}

func wConfig(prefix string) string {
s := `
return fmt.Sprintf(`
resource "snowflake_warehouse" "w" {
name = "%s"
comment = "test comment"
auto_suspend = 60
max_cluster_count = 1
max_cluster_count = 4
min_cluster_count = 1
scaling_policy = "STANDARD"
auto_resume = true
initially_suspended = true
wait_for_provisioning = false
}
`
return fmt.Sprintf(s, prefix)
`, prefix)
}

func wConfig2(prefix string, size string, maxConcurrencyLevel int) string {
s := `
func wConfig2(prefix string, size string, maxConcurrencyLevel int, minClusterCount int) string {
return fmt.Sprintf(`
resource "snowflake_warehouse" "w" {
name = "%s"
name = "%[1]s"
comment = "test comment 2"
warehouse_size = "%s"
warehouse_size = "%[2]s"
auto_suspend = 60
max_cluster_count = 1
min_cluster_count = 1
max_cluster_count = 4
min_cluster_count = %[4]d
scaling_policy = "STANDARD"
auto_resume = true
initially_suspended = true
wait_for_provisioning = false
max_concurrency_level = %d
max_concurrency_level = %[3]d
}
`
return fmt.Sprintf(s, prefix, size, maxConcurrencyLevel)
`, prefix, size, maxConcurrencyLevel, minClusterCount)
}

func wConfigPattern(prefix string) string {
Expand Down
Loading

0 comments on commit 4a73721

Please sign in to comment.