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: renames in update operations #2702

Merged
merged 8 commits into from
Apr 18, 2024
7 changes: 7 additions & 0 deletions MIGRATION_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@ 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.88.0 ➞ v0.89.0
#### *(behavior change)* ForceNew removed
The `ForceNew` field was removed in favor of in-place Update for `name` parameter in:
- `snowflake_file_format`
- `snowflake_masking_policy`
So from now, these objects won't be re-created when the `name` changes, but instead only the name will be updated with `ALTER .. RENAME TO` statements.

## v0.87.0 ➞ v0.88.0
### snowflake_procedure resource changes
#### *(behavior change)* Execute as validation added
Expand Down
6 changes: 5 additions & 1 deletion pkg/resources/file_format.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"fmt"
"strings"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
Expand Down Expand Up @@ -93,7 +95,6 @@ var fileFormatSchema = map[string]*schema.Schema{
Type: schema.TypeString,
Required: true,
Description: "Specifies the identifier for the file format; must be unique for the database and schema in which the file format is created.",
ForceNew: true,
},
"database": {
Type: schema.TypeString,
Expand Down Expand Up @@ -767,6 +768,7 @@ func UpdateFileFormat(d *schema.ResourceData, meta interface{}) error {

if d.HasChange("name") {
newId := sdk.NewSchemaObjectIdentifier(id.DatabaseName(), id.SchemaName(), d.Get("name").(string))

err := client.FileFormats.Alter(ctx, id, &sdk.AlterFileFormatOptions{
Rename: &sdk.AlterFileFormatRenameOptions{
NewName: newId,
Expand All @@ -775,6 +777,8 @@ func UpdateFileFormat(d *schema.ResourceData, meta interface{}) error {
if err != nil {
return fmt.Errorf("error renaming file format: %w", err)
}

d.SetId(helpers.EncodeSnowflakeID(id))
id = newId
}

Expand Down
54 changes: 54 additions & 0 deletions pkg/resources/file_format_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"fmt"
"testing"

"github.com/hashicorp/terraform-plugin-testing/plancheck"

acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
Expand Down Expand Up @@ -446,6 +448,46 @@ func TestAcc_FileFormat_issue1947(t *testing.T) {
})
}

func TestAcc_FileFormat_Rename(t *testing.T) {
name := acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)
newName := acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)
comment := acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)
newComment := acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)
resourceName := "snowflake_file_format.test"

resource.Test(t, resource.TestCase{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
PreCheck: func() { acc.TestAccPreCheck(t) },
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
tfversion.RequireAbove(tfversion.Version1_5_0),
},
sfc-gh-asawicki marked this conversation as resolved.
Show resolved Hide resolved
Steps: []resource.TestStep{
{
Config: fileFormatConfigWithComment(name, acc.TestDatabaseName, acc.TestSchemaName, comment),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "name", name),
resource.TestCheckResourceAttr(resourceName, "comment", comment),
),
},
{
Config: fileFormatConfigWithComment(newName, acc.TestDatabaseName, acc.TestSchemaName, newComment),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "name", newName),
resource.TestCheckResourceAttr(resourceName, "comment", newComment),
),
ConfigPlanChecks: resource.ConfigPlanChecks{
PreApply: []plancheck.PlanCheck{
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
},
PostApplyPostRefresh: []plancheck.PlanCheck{
plancheck.ExpectEmptyPlan(),
},
},
},
},
})
}

func fileFormatConfigCSV(n string, databaseName string, schemaName string, fieldDelimiter string, fieldOptionallyEnclosedBy string, comment string) string {
return fmt.Sprintf(`
resource "snowflake_file_format" "test" {
Expand Down Expand Up @@ -579,6 +621,18 @@ resource "snowflake_file_format" "test" {
`, n, databaseName, schemaName, formatType)
}

func fileFormatConfigWithComment(n string, databaseName string, schemaName string, comment string) string {
return fmt.Sprintf(`
resource "snowflake_file_format" "test" {
name = "%v"
database = "%s"
schema = "%s"
format_type = "XML"
comment = "%s"
}
`, n, databaseName, schemaName, comment)
}

func fileFormatConfigFullDefaultsWithAdditionalParam(n string, formatType string, databaseName string, schemaName string) string {
return fmt.Sprintf(`
resource "snowflake_file_format" "test" {
Expand Down
16 changes: 10 additions & 6 deletions pkg/resources/function.go
Original file line number Diff line number Diff line change
Expand Up @@ -657,15 +657,17 @@ func UpdateContextFunction(ctx context.Context, d *schema.ResourceData, meta int

id := sdk.NewSchemaObjectIdentifierFromFullyQualifiedName(d.Id())
if d.HasChange("name") {
name := d.Get("name")
if err := client.Functions.Alter(ctx, sdk.NewAlterFunctionRequest(id.WithoutArguments(), id.Arguments()).WithRenameTo(sdk.Pointer(sdk.NewSchemaObjectIdentifier(id.DatabaseName(), id.SchemaName(), name.(string))))); err != nil {
return diag.FromErr(err)
}
id = sdk.NewSchemaObjectIdentifierWithArguments(id.DatabaseName(), id.SchemaName(), name.(string), id.Arguments())
if err := d.Set("name", name); err != nil {
name := d.Get("name").(string)
newId := sdk.NewSchemaObjectIdentifierWithArguments(id.DatabaseName(), id.SchemaName(), name, id.Arguments())

if err := client.Functions.Alter(ctx, sdk.NewAlterFunctionRequest(id.WithoutArguments(), id.Arguments()).WithRenameTo(sdk.Pointer(newId.WithoutArguments()))); err != nil {
return diag.FromErr(err)
}

d.SetId(newId.FullyQualifiedName())
id = newId
}

if d.HasChange("is_secure") {
secure := d.Get("is_secure")
if secure.(bool) {
Expand All @@ -678,6 +680,7 @@ func UpdateContextFunction(ctx context.Context, d *schema.ResourceData, meta int
}
}
}

if d.HasChange("comment") {
comment := d.Get("comment")
if comment != "" {
Expand All @@ -690,6 +693,7 @@ func UpdateContextFunction(ctx context.Context, d *schema.ResourceData, meta int
}
}
}

return ReadContextFunction(ctx, d, meta)
}

Expand Down
53 changes: 49 additions & 4 deletions pkg/resources/function_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"strings"
"testing"

"github.com/hashicorp/terraform-plugin-testing/plancheck"

acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
Expand Down Expand Up @@ -187,6 +189,7 @@ func TestAcc_Function_complex(t *testing.T) {
// proves issue https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2490
func TestAcc_Function_migrateFromVersion085(t *testing.T) {
name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
comment := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
resourceName := "snowflake_function.f"

resource.Test(t, resource.TestCase{
Expand All @@ -208,7 +211,7 @@ func TestAcc_Function_migrateFromVersion085(t *testing.T) {
Source: "Snowflake-Labs/snowflake",
},
},
Config: functionConfig(acc.TestDatabaseName, acc.TestSchemaName, name),
Config: functionConfig(acc.TestDatabaseName, acc.TestSchemaName, name, comment),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "id", fmt.Sprintf("%s|%s|%s|VARCHAR", acc.TestDatabaseName, acc.TestSchemaName, name)),
resource.TestCheckResourceAttr(resourceName, "name", name),
Expand All @@ -218,7 +221,7 @@ func TestAcc_Function_migrateFromVersion085(t *testing.T) {
},
{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
Config: functionConfig(acc.TestDatabaseName, acc.TestSchemaName, name),
Config: functionConfig(acc.TestDatabaseName, acc.TestSchemaName, name, comment),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "id", sdk.NewSchemaObjectIdentifierWithArguments(acc.TestDatabaseName, acc.TestSchemaName, name, []sdk.DataType{sdk.DataTypeVARCHAR}).FullyQualifiedName()),
resource.TestCheckResourceAttr(resourceName, "name", name),
Expand All @@ -230,12 +233,54 @@ func TestAcc_Function_migrateFromVersion085(t *testing.T) {
})
}

func functionConfig(database string, schema string, name string) string {
func TestAcc_Function_Rename(t *testing.T) {
name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
newName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
comment := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
newComment := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
resourceName := "snowflake_function.f"

resource.Test(t, resource.TestCase{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
PreCheck: func() { acc.TestAccPreCheck(t) },
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
tfversion.RequireAbove(tfversion.Version1_5_0),
},
CheckDestroy: testAccCheckFunctionDestroy,
sfc-gh-asawicki marked this conversation as resolved.
Show resolved Hide resolved
Steps: []resource.TestStep{
{
Config: functionConfig(acc.TestDatabaseName, acc.TestSchemaName, name, comment),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "name", name),
resource.TestCheckResourceAttr(resourceName, "comment", comment),
),
},
{
Config: functionConfig(acc.TestDatabaseName, acc.TestSchemaName, newName, newComment),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "name", newName),
resource.TestCheckResourceAttr(resourceName, "comment", newComment),
),
ConfigPlanChecks: resource.ConfigPlanChecks{
PreApply: []plancheck.PlanCheck{
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
},
PostApplyPostRefresh: []plancheck.PlanCheck{
plancheck.ExpectEmptyPlan(),
},
},
},
},
})
}

func functionConfig(database string, schema string, name string, comment string) string {
return fmt.Sprintf(`
resource "snowflake_function" "f" {
database = "%[1]s"
schema = "%[2]s"
name = "%[3]s"
comment = "%[4]s"
sfc-gh-asawicki marked this conversation as resolved.
Show resolved Hide resolved
return_type = "VARCHAR"
return_behavior = "IMMUTABLE"
statement = "SELECT PARAM"
Expand All @@ -245,7 +290,7 @@ resource "snowflake_function" "f" {
type = "VARCHAR"
}
}
`, database, schema, name)
`, database, schema, name, comment)
}

func testAccCheckFunctionDestroy(s *terraform.State) error {
Expand Down
35 changes: 17 additions & 18 deletions pkg/resources/masking_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ var maskingPolicySchema = map[string]*schema.Schema{
Type: schema.TypeString,
Required: true,
Description: "Specifies the identifier for the masking policy; must be unique for the database and schema in which the masking policy is created.",
ForceNew: true,
},
"database": {
Type: schema.TypeString,
Expand Down Expand Up @@ -250,16 +249,30 @@ func ReadMaskingPolicy(d *schema.ResourceData, meta interface{}) error {
// UpdateMaskingPolicy implements schema.UpdateFunc.
func UpdateMaskingPolicy(d *schema.ResourceData, meta interface{}) error {
client := meta.(*provider.Context).Client
objectIdentifier := helpers.DecodeSnowflakeID(d.Id()).(sdk.SchemaObjectIdentifier)
id := helpers.DecodeSnowflakeID(d.Id()).(sdk.SchemaObjectIdentifier)
ctx := context.Background()

if d.HasChange("name") {
newID := sdk.NewSchemaObjectIdentifier(id.DatabaseName(), id.SchemaName(), d.Get("name").(string))

err := client.MaskingPolicies.Alter(ctx, id, &sdk.AlterMaskingPolicyOptions{
NewName: &newID,
})
if err != nil {
return err
}

d.SetId(helpers.EncodeSnowflakeID(newID))
id = newID
}

if d.HasChange("masking_expression") {
alterOptions := &sdk.AlterMaskingPolicyOptions{}
_, n := d.GetChange("masking_expression")
alterOptions.Set = &sdk.MaskingPolicySet{
Body: sdk.String(n.(string)),
}
err := client.MaskingPolicies.Alter(ctx, objectIdentifier, alterOptions)
err := client.MaskingPolicies.Alter(ctx, id, alterOptions)
if err != nil {
return err
}
Expand All @@ -276,26 +289,12 @@ func UpdateMaskingPolicy(d *schema.ResourceData, meta interface{}) error {
Comment: sdk.Bool(true),
}
}
err := client.MaskingPolicies.Alter(ctx, objectIdentifier, alterOptions)
err := client.MaskingPolicies.Alter(ctx, id, alterOptions)
if err != nil {
return err
}
}

if d.HasChange("name") {
_, n := d.GetChange("name")
newName := n.(string)
newID := sdk.NewSchemaObjectIdentifier(objectIdentifier.DatabaseName(), objectIdentifier.SchemaName(), newName)
alterOptions := &sdk.AlterMaskingPolicyOptions{
NewName: &newID,
}
err := client.MaskingPolicies.Alter(ctx, objectIdentifier, alterOptions)
if err != nil {
return err
}
d.SetId(helpers.EncodeSnowflakeID(newID))
}

return ReadMaskingPolicy(d, meta)
}

Expand Down
19 changes: 9 additions & 10 deletions pkg/resources/masking_policy_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ import (
"strings"
"testing"

"github.com/hashicorp/terraform-plugin-testing/plancheck"

acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"

"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/hashicorp/terraform-plugin-testing/tfversion"
)

Expand Down Expand Up @@ -41,19 +42,17 @@ func TestAcc_MaskingPolicy(t *testing.T) {
resource.TestCheckResourceAttr("snowflake_masking_policy.test", "signature.0.column.0.type", "VARCHAR"),
),
},
// change comment
{
Config: maskingPolicyConfig(accName, accName, comment2, acc.TestDatabaseName, acc.TestSchemaName),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("snowflake_masking_policy.test", "name", accName),
resource.TestCheckResourceAttr("snowflake_masking_policy.test", "comment", comment2),
),
},
// rename
// rename + change comment
{
Config: maskingPolicyConfig(accName, accName2, comment2, acc.TestDatabaseName, acc.TestSchemaName),
ConfigPlanChecks: resource.ConfigPlanChecks{
PreApply: []plancheck.PlanCheck{
plancheck.ExpectResourceAction("snowflake_masking_policy.test", plancheck.ResourceActionUpdate),
},
},
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("snowflake_masking_policy.test", "name", accName2),
resource.TestCheckResourceAttr("snowflake_masking_policy.test", "comment", comment2),
),
},
// change body and unset comment
Expand Down
4 changes: 1 addition & 3 deletions pkg/resources/materialized_view.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,7 @@ func UpdateMaterializedView(d *schema.ResourceData, meta interface{}) error {
id := helpers.DecodeSnowflakeID(d.Id()).(sdk.SchemaObjectIdentifier)

if d.HasChange("name") {
newName := d.Get("name").(string)

newId := sdk.NewSchemaObjectIdentifier(id.DatabaseName(), id.SchemaName(), newName)
newId := sdk.NewSchemaObjectIdentifier(id.DatabaseName(), id.SchemaName(), d.Get("name").(string))

err := client.MaterializedViews.Alter(ctx, sdk.NewAlterMaterializedViewRequest(id).WithRenameTo(&newId))
if err != nil {
Expand Down
Loading
Loading