Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix issues 2606 2642 2652 2653 #2654

Merged
merged 8 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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 execute context - see caller's rights and owner's rights",
sfc-gh-jcieslak marked this conversation as resolved.
Show resolved Hide resolved
},
"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
Loading