fix: Handle old reference for table_id in table constraint resource (#…

…2558)

- Fix the migration guide
- Fix the docs
- Fallback to `|`-delimited id for users' convenience

References: #2535

MIGRATION_GUIDE.md

@@ -41,6 +41,11 @@ It is noted as a behavior change but in some way it is not; with the previous im
 
 We will consider adding `NOT NULL` back because it can be set by `ALTER COLUMN columnX SET NOT NULL`, but first we want to revisit the whole resource design.
 
+#### *(behavior change)* table_id reference
+The docs were inconsistent. Example prior to 0.86.0 version showed using the `table.id` as the `table_id` reference. The description of the `table_id` parameter never allowed such a value (`table.id` is a `|`-delimited identifier representation and only the `.`-separated values were listed in the docs: https://registry.terraform.io/providers/Snowflake-Labs/snowflake/0.85.0/docs/resources/table_constraint#required. The misuse of `table.id` parameter will result in error after migrating to 0.86.0. To make the config work, please remove and reimport the constraint resource from the state as described in [resource migration doc](./docs/technical-documentation/resource_migration.md).
+
+After discussions in [#2535](https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2535) we decided to provide a temporary workaround in 0.87.0 version, so that the manual migration is not necessary. It allows skipping the migration and jumping straight to 0.87.0 version. However, the temporary workaround will be gone in one of the future versions. Please adjust to the newly suggested reference with the new resources you create.
+
 ### snowflake_external_function resource changes
 
 #### *(behavior change)* return_null_allowed default is now true

Makefile

@@ -64,7 +64,7 @@ test: test-client ## run unit and integration tests
 	go test -v -cover -timeout=30m ./...
 
 test-acceptance: ## run acceptance tests
-	TF_ACC=1 go test -run "^TestAcc_" -v -cover -timeout=45m ./...
+	TF_ACC=1 go test -run "^TestAcc_" -v -cover -timeout=60m ./...
 
 test-integration: ## run SDK integration tests
 	go test -run "^TestInt_" -v -cover -timeout=30m ./...

docs/resources/table_constraint.md

@@ -105,7 +105,7 @@ resource "snowflake_table_constraint" "unique" {
 
 - `columns` (List of String) Columns to use in constraint key
 - `name` (String) Name of constraint
-- `table_id` (String) Identifier for table to create constraint on. Format must follow: "\"<db_name>\".\"<schema_name>\".\"<table_name>\"" or "<db_name>.<schema_name>.<table_name>" (snowflake_table.my_table.id)
+- `table_id` (String) Identifier for table to create constraint on. Format must follow: "\"&lt;db_name&gt;\".\"&lt;schema_name&gt;\".\"&lt;table_name&gt;\"" or "&lt;db_name&gt;.&lt;schema_name&gt;.&lt;table_name&gt;" (snowflake_table.my_table.id)
 - `type` (String) Type of constraint, one of 'UNIQUE', 'PRIMARY KEY', or 'FOREIGN KEY'
 
 ### Optional

pkg/resources/table_constraint.go

@@ -41,7 +41,7 @@ var tableConstraintSchema = map[string]*schema.Schema{
 		Type:        schema.TypeString,
 		Required:    true,
 		ForceNew:    true,
-		Description: `Identifier for table to create constraint on. Format must follow: "\"<db_name>\".\"<schema_name>\".\"<table_name>\"" or "<db_name>.<schema_name>.<table_name>" (snowflake_table.my_table.id)`,
+		Description: `Identifier for table to create constraint on. Format must follow: "\"&lt;db_name&gt;\".\"&lt;schema_name&gt;\".\"&lt;table_name&gt;\"" or "&lt;db_name&gt;.&lt;schema_name&gt;.&lt;table_name&gt;" (snowflake_table.my_table.id)`,
 	},
 	"columns": {
 		Type:     schema.TypeList,
@@ -208,7 +208,15 @@ func (v *tableConstraintID) parse(s string) {
 }
 
 func getTableIdentifier(s string) (*sdk.SchemaObjectIdentifier, error) {
-	objectIdentifier, err := helpers.DecodeSnowflakeParameterID(s)
+	var objectIdentifier sdk.ObjectIdentifier
+	var err error
+	// TODO [SNOW-999049]: Fallback for old implementations using table.id instead of table.qualified_name - probably will be removed later.
+	if strings.Contains(s, "|") {
+		objectIdentifier = helpers.DecodeSnowflakeID(s)
+	} else {
+		objectIdentifier, err = helpers.DecodeSnowflakeParameterID(s)
+	}
+
 	if err != nil {
 		return nil, fmt.Errorf("table id is incorrect: %s, err: %w", objectIdentifier, err)
 	}

pkg/resources/table_constraint_acceptance_test.go

@@ -2,6 +2,8 @@ package resources_test
 
 import (
 	"fmt"
+	"regexp"
+	"strings"
 	"testing"
 
 	acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
@@ -131,3 +133,126 @@ resource "snowflake_table_constraint" "unique" {
 
 `, n, databaseName, schemaName, n)
 }
+
+// proves issue https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2535
+func TestAcc_Table_issue2535_newConstraint(t *testing.T) {
+	accName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+
+	resource.Test(t, resource.TestCase{
+		PreCheck: func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: nil,
+		Steps: []resource.TestStep{
+			{
+				ExternalProviders: map[string]resource.ExternalProvider{
+					"snowflake": {
+						VersionConstraint: "=0.86.0",
+						Source:            "Snowflake-Labs/snowflake",
+					},
+				},
+				Config:      tableConstraintUniqueConfigUsingTableId(accName, acc.TestDatabaseName, acc.TestSchemaName),
+				ExpectError: regexp.MustCompile(`.*table id is incorrect.*`),
+			},
+			{
+				ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+				Config:                   tableConstraintUniqueConfigUsingTableId(accName, acc.TestDatabaseName, acc.TestSchemaName),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("snowflake_table_constraint.unique", "type", "UNIQUE"),
+				),
+			},
+		},
+	})
+}
+
+// proves issue https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2535
+func TestAcc_Table_issue2535_existingTable(t *testing.T) {
+	accName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+
+	resource.Test(t, resource.TestCase{
+		PreCheck: func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: nil,
+		Steps: []resource.TestStep{
+			// reference done by table.id in 0.85.0
+			{
+				ExternalProviders: map[string]resource.ExternalProvider{
+					"snowflake": {
+						VersionConstraint: "=0.85.0",
+						Source:            "Snowflake-Labs/snowflake",
+					},
+				},
+				Config: tableConstraintUniqueConfigUsingTableId(accName, acc.TestDatabaseName, acc.TestSchemaName),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("snowflake_table_constraint.unique", "type", "UNIQUE"),
+				),
+			},
+			// switched to qualified_name in 0.86.0
+			{
+				ExternalProviders: map[string]resource.ExternalProvider{
+					"snowflake": {
+						VersionConstraint: "=0.86.0",
+						Source:            "Snowflake-Labs/snowflake",
+					},
+				},
+				Config:      tableConstraintUniqueConfigUsingFullyQualifiedName(accName, acc.TestDatabaseName, acc.TestSchemaName),
+				ExpectError: regexp.MustCompile(`.*table id is incorrect.*`),
+			},
+			// fixed in the current version
+			{
+				ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+				Config:                   tableConstraintUniqueConfigUsingFullyQualifiedName(accName, acc.TestDatabaseName, acc.TestSchemaName),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("snowflake_table_constraint.unique", "type", "UNIQUE"),
+				),
+			},
+		},
+	})
+}
+
+func tableConstraintUniqueConfigUsingFullyQualifiedName(n string, databaseName string, schemaName string) string {
+	return fmt.Sprintf(`
+resource "snowflake_table" "t" {
+	name     = "%s"
+	database = "%s"
+	schema   = "%s"
+
+	column {
+		name = "col1"
+		type = "NUMBER(38,0)"
+	}
+}
+
+resource "snowflake_table_constraint" "unique" {
+	name     = "%s"
+	type     = "UNIQUE"
+	table_id = snowflake_table.t.qualified_name
+	columns  = ["col1"]
+}
+`, n, databaseName, schemaName, n)
+}
+
+func tableConstraintUniqueConfigUsingTableId(n string, databaseName string, schemaName string) string {
+	return fmt.Sprintf(`
+resource "snowflake_table" "t" {
+	name     = "%s"
+	database = "%s"
+	schema   = "%s"
+
+	column {
+		name = "col1"
+		type = "NUMBER(38,0)"
+	}
+}
+
+resource "snowflake_table_constraint" "unique" {
+	name     = "%s"
+	type     = "UNIQUE"
+	table_id = snowflake_table.t.id
+	columns  = ["col1"]
+}
+`, n, databaseName, schemaName, n)
+}