diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 83ab6ce5ae..450175ce6f 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -181,6 +181,11 @@ As the guide is more general and applies to every version (and provider), we mov ## v0.84.0 ➞ v0.85.0 +### snowflake_stage resource changes + +#### *(behavior change/regression)* copy_options +Setting `copy_options` to `ON_ERROR = 'CONTINUE'` would result in a permadiff. Use `ON_ERROR = CONTINUE` (without single quotes) or bump to v0.89.0 in which the behavior was fixed. + ### snowflake_notification_integration resource changes #### *(behavior change)* notification_provider `notification_provider` becomes required and has three possible values `AZURE_STORAGE_QUEUE`, `AWS_SNS`, and `GCP_PUBSUB`. diff --git a/pkg/resources/common.go b/pkg/resources/common.go index 34b7307f26..5cfbf007a4 100644 --- a/pkg/resources/common.go +++ b/pkg/resources/common.go @@ -26,7 +26,7 @@ func normalizeQuery(str string) string { } // TODO [SNOW-999049]: address during identifiers rework -func suppressIdentifierQuoting(k, oldValue, newValue string, d *schema.ResourceData) bool { +func suppressIdentifierQuoting(_, oldValue, newValue string, _ *schema.ResourceData) bool { if oldValue == "" || newValue == "" { return false } else { @@ -41,3 +41,14 @@ func suppressIdentifierQuoting(k, oldValue, newValue string, d *schema.ResourceD return oldId.FullyQualifiedName() == newId.FullyQualifiedName() } } + +// TODO [SNOW-1325214]: address during stage resource rework +func suppressCopyOptionsQuoting(_, oldValue, newValue string, _ *schema.ResourceData) bool { + if oldValue == "" || newValue == "" { + return false + } else { + oldWithoutQuotes := strings.ReplaceAll(oldValue, "'", "") + newWithoutQuotes := strings.ReplaceAll(newValue, "'", "") + return oldWithoutQuotes == newWithoutQuotes + } +} diff --git a/pkg/resources/stage.go b/pkg/resources/stage.go index 8c41b89c95..b5046c8224 100644 --- a/pkg/resources/stage.go +++ b/pkg/resources/stage.go @@ -55,9 +55,10 @@ var stageSchema = map[string]*schema.Schema{ Description: "Specifies the file format for the stage.", }, "copy_options": { - Type: schema.TypeString, - Optional: true, - Description: "Specifies the copy options for the stage.", + Type: schema.TypeString, + Optional: true, + Description: "Specifies the copy options for the stage.", + DiffSuppressFunc: suppressCopyOptionsQuoting, }, "encryption": { Type: schema.TypeString, diff --git a/pkg/resources/stage_acceptance_test.go b/pkg/resources/stage_acceptance_test.go index 6b3a271f39..48973d5dc6 100644 --- a/pkg/resources/stage_acceptance_test.go +++ b/pkg/resources/stage_acceptance_test.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/terraform-plugin-testing/config" "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" ) @@ -54,17 +55,19 @@ func TestAcc_Stage_CreateAndAlter(t *testing.T) { name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) url := "s3://foo/" comment := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) - storageIntegration := "" + initialStorageIntegration := "" credentials := fmt.Sprintf("AWS_KEY_ID = '%s' AWS_SECRET_KEY = '%s'", awsKeyId, awsSecretKey) encryption := "TYPE = 'NONE'" + copyOptionsWithQuotes := "ON_ERROR = 'CONTINUE'" changedUrl := awsBucketUrl + "/some-path" changedStorageIntegration := "S3_STORAGE_INTEGRATION" changedEncryption := "TYPE = 'AWS_SSE_S3'" changedFileFormat := "TYPE = JSON NULL_IF = []" changedComment := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) + copyOptionsWithoutQuotes := "ON_ERROR = CONTINUE" - configVariables := func(url string, storageIntegration string, credentials string, encryption string, fileFormat string, comment string) config.Variables { + configVariables := func(url string, storageIntegration string, credentials string, encryption string, fileFormat string, comment string, copyOptions string) config.Variables { return config.Variables{ "database": config.StringVariable(databaseName), "schema": config.StringVariable(schemaName), @@ -75,6 +78,7 @@ func TestAcc_Stage_CreateAndAlter(t *testing.T) { "encryption": config.StringVariable(encryption), "file_format": config.StringVariable(fileFormat), "comment": config.StringVariable(comment), + "copy_options": config.StringVariable(copyOptions), } } @@ -89,22 +93,26 @@ func TestAcc_Stage_CreateAndAlter(t *testing.T) { Steps: []resource.TestStep{ { ConfigDirectory: config.TestNameDirectory(), - ConfigVariables: configVariables(url, storageIntegration, credentials, encryption, "", comment), + ConfigVariables: configVariables(url, initialStorageIntegration, credentials, encryption, "", comment, copyOptionsWithQuotes), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(resourceName, "database", databaseName), resource.TestCheckResourceAttr(resourceName, "schema", schemaName), resource.TestCheckResourceAttr(resourceName, "name", name), - resource.TestCheckResourceAttr(resourceName, "storage_integration", storageIntegration), + resource.TestCheckResourceAttr(resourceName, "storage_integration", initialStorageIntegration), resource.TestCheckResourceAttr(resourceName, "credentials", credentials), resource.TestCheckResourceAttr(resourceName, "encryption", encryption), resource.TestCheckResourceAttr(resourceName, "file_format", ""), + resource.TestCheckResourceAttr(resourceName, "copy_options", copyOptionsWithoutQuotes), resource.TestCheckResourceAttr(resourceName, "url", url), resource.TestCheckResourceAttr(resourceName, "comment", comment), ), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PostApplyPreRefresh: []plancheck.PlanCheck{plancheck.ExpectEmptyPlan()}, + }, }, { ConfigDirectory: config.TestNameDirectory(), - ConfigVariables: configVariables(changedUrl, changedStorageIntegration, credentials, changedEncryption, changedFileFormat, changedComment), + ConfigVariables: configVariables(changedUrl, changedStorageIntegration, credentials, changedEncryption, changedFileFormat, changedComment, copyOptionsWithoutQuotes), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(resourceName, "database", databaseName), resource.TestCheckResourceAttr(resourceName, "schema", schemaName), @@ -113,32 +121,58 @@ func TestAcc_Stage_CreateAndAlter(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "credentials", credentials), resource.TestCheckResourceAttr(resourceName, "encryption", changedEncryption), resource.TestCheckResourceAttr(resourceName, "file_format", changedFileFormat), + resource.TestCheckResourceAttr(resourceName, "copy_options", copyOptionsWithoutQuotes), resource.TestCheckResourceAttr(resourceName, "url", changedUrl), resource.TestCheckResourceAttr(resourceName, "comment", changedComment), ), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PostApplyPreRefresh: []plancheck.PlanCheck{plancheck.ExpectEmptyPlan()}, + }, + }, + { + ConfigDirectory: config.TestNameDirectory(), + ConfigVariables: configVariables(changedUrl, changedStorageIntegration, credentials, changedEncryption, changedFileFormat, changedComment, copyOptionsWithoutQuotes), + Destroy: true, + }, + { + ConfigDirectory: config.TestNameDirectory(), + ConfigVariables: configVariables(url, initialStorageIntegration, credentials, encryption, "", comment, copyOptionsWithoutQuotes), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "database", databaseName), + resource.TestCheckResourceAttr(resourceName, "schema", schemaName), + resource.TestCheckResourceAttr(resourceName, "name", name), + resource.TestCheckResourceAttr(resourceName, "storage_integration", initialStorageIntegration), + resource.TestCheckResourceAttr(resourceName, "credentials", credentials), + resource.TestCheckResourceAttr(resourceName, "encryption", encryption), + resource.TestCheckResourceAttr(resourceName, "file_format", ""), + resource.TestCheckResourceAttr(resourceName, "copy_options", copyOptionsWithoutQuotes), + resource.TestCheckResourceAttr(resourceName, "url", url), + resource.TestCheckResourceAttr(resourceName, "comment", comment), + ), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PostApplyPreRefresh: []plancheck.PlanCheck{plancheck.ExpectEmptyPlan()}, + }, }, }, }) } func stageIntegrationConfig(name string, siNameSuffix string, url string, databaseName string, schemaName string) string { - resources := ` + return fmt.Sprintf(` resource "snowflake_storage_integration" "test" { - name = "%s%s" - storage_allowed_locations = ["%s"] + name = "%[1]s%[2]s" + storage_allowed_locations = ["%[3]s"] storage_provider = "S3" storage_aws_role_arn = "arn:aws:iam::000000000001:/role/test" } resource "snowflake_stage" "test" { - name = "%s" - url = "%s" + name = "%[1]s" + url = "%[3]s" storage_integration = snowflake_storage_integration.test.name - database = "%s" - schema = "%s" + database = "%[4]s" + schema = "%[5]s" } -` - - return fmt.Sprintf(resources, name, siNameSuffix, url, name, url, databaseName, schemaName) +`, name, siNameSuffix, url, databaseName, schemaName) } diff --git a/pkg/resources/testdata/TestAcc_Stage_CreateAndAlter/test.tf b/pkg/resources/testdata/TestAcc_Stage_CreateAndAlter/test.tf index 7a0de913f2..e2d5d700f3 100644 --- a/pkg/resources/testdata/TestAcc_Stage_CreateAndAlter/test.tf +++ b/pkg/resources/testdata/TestAcc_Stage_CreateAndAlter/test.tf @@ -17,4 +17,5 @@ resource "snowflake_stage" "test" { credentials = var.credentials encryption = var.encryption file_format = var.file_format + copy_options = var.copy_options } diff --git a/pkg/resources/testdata/TestAcc_Stage_CreateAndAlter/variables.tf b/pkg/resources/testdata/TestAcc_Stage_CreateAndAlter/variables.tf index 944a8d90bf..81d90be9c5 100644 --- a/pkg/resources/testdata/TestAcc_Stage_CreateAndAlter/variables.tf +++ b/pkg/resources/testdata/TestAcc_Stage_CreateAndAlter/variables.tf @@ -33,3 +33,7 @@ variable "encryption" { variable "file_format" { type = string } + +variable "copy_options" { + type = string +} diff --git a/pkg/sdk/tags_test.go b/pkg/sdk/tags_test.go index 0fd98edbe3..b977dafbca 100644 --- a/pkg/sdk/tags_test.go +++ b/pkg/sdk/tags_test.go @@ -44,6 +44,14 @@ func TestTagCreate(t *testing.T) { assertOptsInvalidJoinedErrors(t, opts, ErrInvalidObjectIdentifier) }) + t.Run("validation: allowed values count", func(t *testing.T) { + opts := defaultOpts() + opts.AllowedValues = &AllowedValues{ + Values: []AllowedValue{}, + } + assertOptsInvalidJoinedErrors(t, opts, errIntBetween("AllowedValues", "Values", 1, 300)) + }) + t.Run("validation: both ifNotExists and orReplace present", func(t *testing.T) { opts := defaultOpts() opts.IfNotExists = Bool(true) @@ -296,6 +304,16 @@ func TestTagAlter(t *testing.T) { opts.Unset = &TagUnset{} assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("TagUnset", "MaskingPolicies", "AllowedValues", "Comment")) }) + + t.Run("validation: allowed values count", func(t *testing.T) { + opts := defaultOpts() + opts.Add = &TagAdd{ + AllowedValues: &AllowedValues{ + Values: []AllowedValue{}, + }, + } + assertOptsInvalidJoinedErrors(t, opts, errIntBetween("AllowedValues", "Values", 1, 300)) + }) } func TestTagSet(t *testing.T) { diff --git a/pkg/sdk/tags_validations.go b/pkg/sdk/tags_validations.go index b055116315..c5b136ae04 100644 --- a/pkg/sdk/tags_validations.go +++ b/pkg/sdk/tags_validations.go @@ -35,8 +35,8 @@ func (opts *createTagOptions) validate() error { } func (v *AllowedValues) validate() error { - if !validateIntInRange(len(v.Values), 1, 50) { - return errIntBetween("AllowedValues", "Values", 1, 50) + if !validateIntInRange(len(v.Values), 1, 300) { + return errIntBetween("AllowedValues", "Values", 1, 300) } return nil }