Skip to content

Commit

Permalink
Fix identifier issues
Browse files Browse the repository at this point in the history
  • Loading branch information
@sfc-gh-jcieslak
sfc-gh-jcieslak committed Aug 15, 2024
1 parent 1b0462f commit 817da77
Show file tree
Hide file tree
Showing 6 changed files with 135 additions and 21 deletions.
7 changes: 4 additions & 3 deletions pkg/resources/secondary_database.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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. '<db>.<schema>.<object>') 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 `\"<organization_name>\".\"<account_name>\".\"<database_name>\"`.",
},
"is_transient": {
Expand Down
7 changes: 4 additions & 3 deletions pkg/resources/shared_database.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 `\"<organization_name>\".\"<account_name>\".\"<share_name>\"`.",
},
"comment": {
Expand Down
3 changes: 2 additions & 1 deletion pkg/resources/storage_integration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ func CreateStorageIntegration(d *schema.ResourceData, meta any) error {
Path: loc,
}
}
req.WithStorageBlockedLocations(storageBlockedLocations)
}

storageProvider := d.Get("storage_provider").(string)
Expand Down Expand Up @@ -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{
Expand Down
62 changes: 51 additions & 11 deletions pkg/resources/storage_integration_acceptance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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", ""),
),
Expand All @@ -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/"),
),
},
{
Expand All @@ -319,11 +319,51 @@ 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", ""),
),
},
},
})
}

func TestAcc_StorageIntegration_BlockedLocations_issue295(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/"),
),
},
},
})
}
7 changes: 4 additions & 3 deletions pkg/resources/table.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
},
Expand Down
70 changes: 70 additions & 0 deletions pkg/resources/table_acceptance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

0 comments on commit 817da77

Please sign in to comment.