fix: identifier issues (#2998)

Apply fixes for:
-
#2985
-
#2644

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. '<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": {

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 `\"<organization_name>\".\"<account_name>\".\"<share_name>\"`.",
 	},
 	"comment": {

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{

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,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/"),
+				),
+			},
+		},
+	})
+}

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
 							},
 						},

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)
+}

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
 		}