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: identifier issues #2998

Merged
merged 1 commit into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions pkg/resources/secondary_database.go
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
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
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
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_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/"),
),
},
},
})
}
7 changes: 4 additions & 3 deletions pkg/resources/table.go
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
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)
}
3 changes: 2 additions & 1 deletion pkg/resources/validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading