Skip to content

Commit

Permalink
Fix stage copy options permadiff
Browse files Browse the repository at this point in the history
References: #2679
  • Loading branch information
sfc-gh-asawicki committed Apr 18, 2024
1 parent 6301482 commit 798a399
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 19 deletions.
5 changes: 5 additions & 0 deletions MIGRATION_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
13 changes: 12 additions & 1 deletion pkg/resources/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
}
7 changes: 4 additions & 3 deletions pkg/resources/stage.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
64 changes: 49 additions & 15 deletions pkg/resources/stage_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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),
Expand All @@ -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),
}
}

Expand All @@ -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),
Expand All @@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,5 @@ resource "snowflake_stage" "test" {
credentials = var.credentials
encryption = var.encryption
file_format = var.file_format
copy_options = var.copy_options
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,7 @@ variable "encryption" {
variable "file_format" {
type = string
}

variable "copy_options" {
type = string
}

0 comments on commit 798a399

Please sign in to comment.