diff --git a/pkg/resources/secondary_database.go b/pkg/resources/secondary_database.go index 9363a3e26d..33ffcd4fec 100644 --- a/pkg/resources/secondary_database.go +++ b/pkg/resources/secondary_database.go @@ -21,9 +21,10 @@ var secondaryDatabaseSchema = map[string]*schema.Schema{ Description: "Specifies the identifier for the database; must be unique for your account. As a best practice for [Database Replication and Failover](https://docs.snowflake.com/en/user-guide/db-replication-intro), it is recommended to give each secondary database the same name as its primary database. This practice supports referencing fully-qualified objects (i.e. '..') by other objects in the same database, such as querying a fully-qualified table name in a view. If a secondary database has a different name from the primary database, then these object references would break in the secondary database.", }, "as_replica_of": { - Type: schema.TypeString, - Required: true, - ForceNew: true, + Type: schema.TypeString, + Required: true, + ForceNew: true, + // TODO(SNOW-1495079): Add validation when ExternalObjectIdentifier will be available in IsValidIdentifier Description: "A fully qualified path to a database to create a replica from. A fully qualified path follows the format of `\"\".\"\".\"\"`.", }, "is_transient": { diff --git a/pkg/resources/shared_database.go b/pkg/resources/shared_database.go index e5093201f6..b3d3f8e72b 100644 --- a/pkg/resources/shared_database.go +++ b/pkg/resources/shared_database.go @@ -21,9 +21,10 @@ var sharedDatabaseSchema = map[string]*schema.Schema{ Description: "Specifies the identifier for the database; must be unique for your account.", }, "from_share": { - Type: schema.TypeString, - Required: true, - ForceNew: true, + Type: schema.TypeString, + Required: true, + ForceNew: true, + // TODO(SNOW-1495079): Add validation when ExternalObjectIdentifier will be available in IsValidIdentifier Description: "A fully qualified path to a share from which the database will be created. A fully qualified path follows the format of `\"\".\"\".\"\"`.", }, "comment": { diff --git a/pkg/resources/storage_integration.go b/pkg/resources/storage_integration.go index 8fd4fe378d..c8a1be8cee 100644 --- a/pkg/resources/storage_integration.go +++ b/pkg/resources/storage_integration.go @@ -151,6 +151,7 @@ func CreateStorageIntegration(d *schema.ResourceData, meta any) error { Path: loc, } } + req.WithStorageBlockedLocations(storageBlockedLocations) } storageProvider := d.Get("storage_provider").(string) @@ -324,7 +325,7 @@ func UpdateStorageIntegration(d *schema.ResourceData, meta any) error { } } else { runSetStatement = true - stringStorageBlockedLocations := expandStringList(d.Get("storage_allowed_locations").([]any)) + stringStorageBlockedLocations := expandStringList(d.Get("storage_blocked_locations").([]any)) storageBlockedLocations := make([]sdk.StorageLocation, len(stringStorageBlockedLocations)) for i, loc := range stringStorageBlockedLocations { storageBlockedLocations[i] = sdk.StorageLocation{ diff --git a/pkg/resources/storage_integration_acceptance_test.go b/pkg/resources/storage_integration_acceptance_test.go index 38fe546e3f..7ae4079594 100644 --- a/pkg/resources/storage_integration_acceptance_test.go +++ b/pkg/resources/storage_integration_acceptance_test.go @@ -260,18 +260,18 @@ func TestAcc_StorageIntegration_GCP_Update(t *testing.T) { variables := config.Variables{ "name": config.StringVariable(name), "allowed_locations": config.SetVariable( - config.StringVariable("gcs://foo/"), + config.StringVariable("gcs://allowed_foo/"), ), } if set { variables["comment"] = config.StringVariable("some comment") variables["allowed_locations"] = config.SetVariable( - config.StringVariable("gcs://foo/"), - config.StringVariable("gcs://bar/"), + config.StringVariable("gcs://allowed_foo/"), + config.StringVariable("gcs://allowed_bar/"), ) variables["blocked_locations"] = config.SetVariable( - config.StringVariable("gcs://foo/"), - config.StringVariable("gcs://bar/"), + config.StringVariable("gcs://blocked_foo/"), + config.StringVariable("gcs://blocked_bar/"), ) } return variables @@ -292,7 +292,7 @@ func TestAcc_StorageIntegration_GCP_Update(t *testing.T) { resource.TestCheckResourceAttr("snowflake_storage_integration.test", "name", name), resource.TestCheckResourceAttr("snowflake_storage_integration.test", "enabled", "false"), resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_allowed_locations.#", "1"), - resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_allowed_locations.0", "gcs://foo/"), + resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_allowed_locations.0", "gcs://allowed_foo/"), resource.TestCheckNoResourceAttr("snowflake_storage_integration.test", "storage_blocked_locations"), resource.TestCheckResourceAttr("snowflake_storage_integration.test", "comment", ""), ), @@ -305,11 +305,11 @@ func TestAcc_StorageIntegration_GCP_Update(t *testing.T) { resource.TestCheckResourceAttr("snowflake_storage_integration.test", "enabled", "true"), resource.TestCheckResourceAttr("snowflake_storage_integration.test", "comment", "some comment"), resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_allowed_locations.#", "2"), - resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_allowed_locations.0", "gcs://bar/"), - resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_allowed_locations.1", "gcs://foo/"), + resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_allowed_locations.0", "gcs://allowed_bar/"), + resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_allowed_locations.1", "gcs://allowed_foo/"), resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_blocked_locations.#", "2"), - resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_blocked_locations.0", "gcs://bar/"), - resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_blocked_locations.1", "gcs://foo/"), + resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_blocked_locations.0", "gcs://blocked_bar/"), + resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_blocked_locations.1", "gcs://blocked_foo/"), ), }, { @@ -319,7 +319,7 @@ func TestAcc_StorageIntegration_GCP_Update(t *testing.T) { resource.TestCheckResourceAttr("snowflake_storage_integration.test", "name", name), resource.TestCheckResourceAttr("snowflake_storage_integration.test", "enabled", "false"), resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_allowed_locations.#", "1"), - resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_allowed_locations.0", "gcs://foo/"), + resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_allowed_locations.0", "gcs://allowed_foo/"), resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_blocked_locations.#", "0"), resource.TestCheckResourceAttr("snowflake_storage_integration.test", "comment", ""), ), @@ -327,3 +327,43 @@ func TestAcc_StorageIntegration_GCP_Update(t *testing.T) { }, }) } + +func TestAcc_StorageIntegration_BlockedLocations_issue2985(t *testing.T) { + name := acc.TestClient().Ids.Alpha() + configVariables := config.Variables{ + "name": config.StringVariable(name), + "allowed_locations": config.SetVariable( + config.StringVariable("gcs://allowed_foo/"), + ), + "comment": config.StringVariable("some comment"), + "blocked_locations": config.SetVariable( + config.StringVariable("gcs://blocked_foo/"), + config.StringVariable("gcs://blocked_bar/"), + ), + } + + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: acc.CheckDestroy(t, resources.StorageIntegration), + Steps: []resource.TestStep{ + { + ConfigVariables: configVariables, + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_StorageIntegration/GCP_Update/set"), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_storage_integration.test", "name", name), + resource.TestCheckResourceAttr("snowflake_storage_integration.test", "enabled", "true"), + resource.TestCheckResourceAttr("snowflake_storage_integration.test", "comment", "some comment"), + resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_allowed_locations.#", "1"), + resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_allowed_locations.0", "gcs://allowed_foo/"), + resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_blocked_locations.#", "2"), + resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_blocked_locations.0", "gcs://blocked_bar/"), + resource.TestCheckResourceAttr("snowflake_storage_integration.test", "storage_blocked_locations.1", "gcs://blocked_foo/"), + ), + }, + }, + }) +} diff --git a/pkg/resources/table.go b/pkg/resources/table.go index 7b4731c68f..44c6070ebb 100644 --- a/pkg/resources/table.go +++ b/pkg/resources/table.go @@ -91,9 +91,10 @@ var tableSchema = map[string]*schema.Schema{ // ConflictsWith: []string{".constant", ".sequence"}, - can't use, nor ExactlyOneOf due to column type being TypeList }, "sequence": { - Type: schema.TypeString, - Optional: true, - Description: "The default sequence to use for the column", + Type: schema.TypeString, + Optional: true, + Description: "The default sequence to use for the column", + DiffSuppressFunc: suppressIdentifierQuoting, // ConflictsWith: []string{".constant", ".expression"}, - can't use, nor ExactlyOneOf due to column type being TypeList }, }, diff --git a/pkg/resources/table_acceptance_test.go b/pkg/resources/table_acceptance_test.go index 5fb73dce55..49d8bc461b 100644 --- a/pkg/resources/table_acceptance_test.go +++ b/pkg/resources/table_acceptance_test.go @@ -2020,3 +2020,73 @@ func TestAcc_Table_migrateFromVersion_0_94_1(t *testing.T) { }, }) } + +func TestAcc_Table_SuppressQuotingOnDefaultSequence_issue2644(t *testing.T) { + databaseName := acc.TestClient().Ids.Alpha() + schemaName := acc.TestClient().Ids.Alpha() + name := acc.TestClient().Ids.Alpha() + resourceName := "snowflake_table.test_table" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "snowflake": { + VersionConstraint: "=0.94.1", + Source: "Snowflake-Labs/snowflake", + }, + }, + ExpectNonEmptyPlan: true, + Config: tableConfigWithSequence(name, databaseName, schemaName), + }, + { + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + Config: tableConfigWithSequence(name, databaseName, schemaName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "column.0.default.0.sequence", sdk.NewSchemaObjectIdentifier(databaseName, schemaName, name).FullyQualifiedName()), + ), + }, + }, + }) +} + +func tableConfigWithSequence(name string, databaseName string, schemaName string) string { + return fmt.Sprintf(` +resource "snowflake_database" "test_database" { + name = "%[2]s" +} + +resource "snowflake_schema" "test_schema" { + depends_on = [snowflake_database.test_database] + name = "%[3]s" + database = "%[2]s" +} + +resource "snowflake_sequence" "test_sequence" { + depends_on = [snowflake_schema.test_schema] + name = "%[1]s" + database = "%[2]s" + schema = "%[3]s" +} + +resource "snowflake_table" "test_table" { + depends_on = [snowflake_sequence.test_sequence] + name = "%[1]s" + database = "%[2]s" + schema = "%[3]s" + data_retention_time_in_days = 1 + comment = "Terraform acceptance test" + column { + name = "column1" + type = "NUMBER" + default { + sequence = "%[2]s.%[3]s.%[1]s" + } + } +} +`, name, databaseName, schemaName) +} diff --git a/pkg/resources/validators.go b/pkg/resources/validators.go index 52eaf50b6a..e43003b90c 100644 --- a/pkg/resources/validators.go +++ b/pkg/resources/validators.go @@ -51,7 +51,8 @@ func IsValidIdentifier[T sdk.AccountObjectIdentifier | sdk.DatabaseObjectIdentif } } - // TODO(SNOW-1163071): Right now we have to skip validation for AccountObjectIdentifier to handle a case where identifier contains dots + // TODO(SNOW-1495079): Right now we have to skip validation for AccountObjectIdentifier to handle a case where identifier contains dots + // TODO(SNOW-1495079): with sdk.AccountObjectIdentifier{} (or a new type of identifier) we should be able to validate individual part of the identifier field (e.g. "database" or "schema" field) if _, ok := any(sdk.AccountObjectIdentifier{}).(T); ok { return nil }