Cleanup

MIGRATION_GUIDE.md

@@ -11,13 +11,30 @@ across different versions.
 
 ### snowflake_tag_association resource changes
 #### *(behavior change)* new id format
-In order to provide more functionality for tagging objects, we have changed the resource id from `"TAG_DATABASE"."TAG_SCHEMA"."TAG_NAME"` to `"TAG_DATABASE"."TAG_SCHEMA"."TAG_NAME"|TAG_VALUE|OBJECT_TYPE`. This allows to group tags associations per tag ID, tag value and object type like the following:
+In order to provide more functionality for tagging objects, we have changed the resource id from `"TAG_DATABASE"."TAG_SCHEMA"."TAG_NAME"` to `"TAG_DATABASE"."TAG_SCHEMA"."TAG_NAME"|TAG_VALUE|OBJECT_TYPE`. This allows to group tags associations per tag ID, tag value and object type in one resource.
 ```
+resource "snowflake_tag_association" "gold_warehouses" {
+  object_identifiers = [snowflake_warehouse.w1.fully_qualified_name, snowflake_warehouse.w2.fully_qualified_name]
+  object_type = "WAREHOUSE"
+  tag_id      = snowflake_tag.tier.fully_qualified_name
+  tag_value   = "gold"
+}
+resource "snowflake_tag_association" "silver_warehouses" {
+  object_identifiers = [snowflake_warehouse.w3.fully_qualified_name]
+  object_type = "WAREHOUSE"
+  tag_id      = snowflake_tag.tier.fully_qualified_name
+  tag_value   = "silver"
+}
+resource "snowflake_tag_association" "silver_databases" {
+  object_identifiers = [snowflake_database.d1.fully_qualified_name]
+  object_type = "DATABASE"
+  tag_id      = snowflake_tag.tier.fully_qualified_name
+  tag_value   = "silver"
+}
 ```
 
 The state is migrated automatically. There is no need to adjust configuration files, unless you use resource id like `snowflake_tag_association.example.id`.
 
-
 #### *(behavior change)* changed fields
 Behavior of some fields was changed:
 - `object_identifier` was renamed to `object_identifiers` and it is now a set of fully qualified names. Change your configurations from

pkg/acceptance/check_destroy.go

@@ -523,7 +523,7 @@ func CheckTagValueEmpty(t *testing.T) func(*terraform.State) error {
 						return fmt.Errorf("invalid object id: %w", err)
 					}
 				}
-				tag, err := TestClient().Tag.GetTag(t, tagId, id, objectType)
+				tag, err := TestClient().SystemFunctions.GetTag(t, tagId, id, objectType)
 				if err != nil {
 					if strings.Contains(err.Error(), "does not exist or not authorized") {
 						// Note: this can happen if the object has been deleted as well; in this case, ignore the error

pkg/acceptance/helpers/context_client.go

@@ -53,6 +53,15 @@ func (c *ContextClient) CurrentOrganizationName(t *testing.T) string {
 	return orgName
 }
 
+func (c *ContextClient) CurrentAccountIdentifier(t *testing.T) sdk.AccountIdentifier {
+	t.Helper()
+
+	orgName := c.CurrentOrganizationName(t)
+	accountName := c.CurrentAccountName(t)
+
+	return sdk.NewAccountIdentifier(orgName, accountName)
+}
+
 func (c *ContextClient) CurrentRole(t *testing.T) sdk.AccountObjectIdentifier {
 	t.Helper()
 	ctx := context.Background()

pkg/acceptance/helpers/system_functions_client.go

@@ -0,0 +1,30 @@
+package helpers
+
+import (
+	"context"
+	"testing"
+
+	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
+)
+
+type SystemFunctionsClient struct {
+	context *TestClientContext
+}
+
+func NewSystemFunctionsClient(context *TestClientContext) *SystemFunctionsClient {
+	return &SystemFunctionsClient{
+		context: context,
+	}
+}
+
+func (c *SystemFunctionsClient) client() sdk.SystemFunctions {
+	return c.context.client.SystemFunctions
+}
+
+func (c *SystemFunctionsClient) GetTag(t *testing.T, tagId sdk.SchemaObjectIdentifier, objectId sdk.ObjectIdentifier, objectType sdk.ObjectType) (*string, error) {
+	t.Helper()
+	ctx := context.Background()
+	client := c.context.client.SystemFunctions
+
+	return client.GetTag(ctx, tagId, objectId, objectType)
+}

pkg/acceptance/helpers/tag_client.go

@@ -60,14 +60,6 @@ func (c *TagClient) Unset(t *testing.T, objectType sdk.ObjectType, id sdk.Object
 	require.NoError(t, err)
 }
 
-func (c *TagClient) GetTag(t *testing.T, tagId sdk.SchemaObjectIdentifier, objectId sdk.ObjectIdentifier, objectType sdk.ObjectType) (*string, error) {
-	t.Helper()
-	ctx := context.Background()
-	client := c.context.client.SystemFunctions
-
-	return client.GetTag(ctx, tagId, objectId, objectType)
-}
-
 func (c *TagClient) Alter(t *testing.T, req *sdk.AlterTagRequest) {
 	t.Helper()
 	ctx := context.Background()

pkg/acceptance/helpers/test_client.go

@@ -59,6 +59,7 @@ type TestClient struct {
 	StorageIntegration           *StorageIntegrationClient
 	Stream                       *StreamClient
 	Streamlit                    *StreamlitClient
+	SystemFunctions              *SystemFunctionsClient
 	Table                        *TableClient
 	Tag                          *TagClient
 	Task                         *TaskClient
@@ -131,6 +132,7 @@ func NewTestClient(c *sdk.Client, database string, schema string, warehouse stri
 		StorageIntegration:           NewStorageIntegrationClient(context, idsGenerator),
 		Stream:                       NewStreamClient(context, idsGenerator),
 		Streamlit:                    NewStreamlitClient(context, idsGenerator),
+		SystemFunctions:              NewSystemFunctionsClient(context),
 		Table:                        NewTableClient(context, idsGenerator),
 		Tag:                          NewTagClient(context, idsGenerator),
 		Task:                         NewTaskClient(context, idsGenerator),

pkg/helpers/helpers.go

@@ -290,36 +290,3 @@ func ContainsIdentifierIgnoringQuotes(ids []string, id string) bool {
 
 	return false
 }
-
-func ExpandStringList(configured []interface{}) []string {
-	vs := make([]string, 0, len(configured))
-	for _, v := range configured {
-		val, ok := v.(string)
-		if ok && val != "" {
-			vs = append(vs, val)
-		}
-	}
-	return vs
-}
-
-func ExpandObjectIdentifierSet(configured []any, objectType sdk.ObjectType) ([]sdk.ObjectIdentifier, error) {
-	vs := ExpandStringList(configured)
-	ids := make([]sdk.ObjectIdentifier, len(vs))
-	for i, idRaw := range vs {
-		var id sdk.ObjectIdentifier
-		var err error
-		if objectType == sdk.ObjectTypeAccount {
-			id, err = sdk.ParseAccountIdentifier(idRaw)
-			if err != nil {
-				return nil, fmt.Errorf("invalid account id: %w", err)
-			}
-		} else {
-			id, err = sdk.ParseObjectIdentifierString(idRaw)
-			if err != nil {
-				return nil, fmt.Errorf("invalid object id: %w", err)
-			}
-		}
-		ids[i] = id
-	}
-	return ids, nil
-}

pkg/resources/helper_expansion.go

@@ -1,9 +1,9 @@
 package resources
 
 import (
+	"fmt"
 	"slices"
 
-	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
 )
 
@@ -20,11 +20,36 @@ func expandIntList(configured []interface{}) []int {
 }
 
 func expandStringList(configured []interface{}) []string {
-	return helpers.ExpandStringList(configured)
+	vs := make([]string, 0, len(configured))
+	for _, v := range configured {
+		val, ok := v.(string)
+		if ok && val != "" {
+			vs = append(vs, val)
+		}
+	}
+	return vs
 }
 
 func ExpandObjectIdentifierSet(configured []any, objectType sdk.ObjectType) ([]sdk.ObjectIdentifier, error) {
-	return helpers.ExpandObjectIdentifierSet(configured, objectType)
+	vs := expandStringList(configured)
+	ids := make([]sdk.ObjectIdentifier, len(vs))
+	for i, idRaw := range vs {
+		var id sdk.ObjectIdentifier
+		var err error
+		if objectType == sdk.ObjectTypeAccount {
+			id, err = sdk.ParseAccountIdentifier(idRaw)
+			if err != nil {
+				return nil, fmt.Errorf("invalid account id: %w", err)
+			}
+		} else {
+			id, err = sdk.ParseObjectIdentifierString(idRaw)
+			if err != nil {
+				return nil, fmt.Errorf("invalid object id: %w", err)
+			}
+		}
+		ids[i] = id
+	}
+	return ids, nil
 }
 
 func expandStringListAllowEmpty(configured []interface{}) []string {

pkg/resources/tag_association_acceptance_test.go

@@ -313,9 +313,7 @@ func TestAcc_TagAssociationAccountIssues1910(t *testing.T) {
 	_ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance)
 
 	tagId := acc.TestClient().Ids.RandomSchemaObjectIdentifier()
-	accountName := acc.TestClient().Context.CurrentAccountName(t)
-	orgName := acc.TestClient().Context.CurrentOrganizationName(t)
-	accountId := sdk.NewAccountIdentifier(orgName, accountName)
+	accountId := acc.TestClient().Context.CurrentAccountIdentifier(t)
 	resourceName := "snowflake_tag_association.test"
 	m := func() map[string]config.Variable {
 		return map[string]config.Variable{

pkg/resources/tag_association_state_upgraders.go

@@ -25,18 +25,21 @@ func v0_98_0_TagAssociationStateUpgrader(ctx context.Context, rawState map[strin
 	for _, objectIdentifierOld := range objectIdentifiersOld {
 		obj := objectIdentifierOld.(map[string]any)
 		var id sdk.ObjectIdentifier
-		database := obj["database"].(string)
-		schema := obj["schema"].(string)
-		name := obj["name"].(string)
-		switch {
-		case schema != "":
-			id = sdk.NewSchemaObjectIdentifier(database, schema, name)
-		case database != "":
-			id = sdk.NewDatabaseObjectIdentifier(database, name)
-		default:
-			id = sdk.NewAccountObjectIdentifier(name)
+		if objectType == string(sdk.ObjectTypeAccount) {
+			id = sdk.NewAccountIdentifierFromFullyQualifiedName(obj["name"].(string))
+		} else {
+			database := obj["database"].(string)
+			schema := obj["schema"].(string)
+			name := obj["name"].(string)
+			switch {
+			case schema != "":
+				id = sdk.NewSchemaObjectIdentifier(database, schema, name)
+			case database != "":
+				id = sdk.NewDatabaseObjectIdentifier(database, name)
+			default:
+				id = sdk.NewAccountObjectIdentifier(name)
+			}
 		}
-
 		objectIdentifiers = append(objectIdentifiers, id.FullyQualifiedName())
 	}
 	rawState["object_identifiers"] = objectIdentifiers

pkg/resources/tag_association_test.go

@@ -11,6 +11,22 @@ import (
 )
 
 func TestTagIdentifierAndObjectIdentifier(t *testing.T) {
+	t.Run("account identifier", func(t *testing.T) {
+		in := map[string]any{
+			"tag_id":      "\"test_db\".\"test_schema\".\"test_tag\"",
+			"object_type": "ACCOUNT",
+			"object_identifier": []any{
+				"orgname.accountname",
+			},
+		}
+		d := schema.TestResourceDataRaw(t, resources.TagAssociation().Schema, in)
+		tid, identifiers, objectType, err := resources.TagIdentifierAndObjectIdentifier(d)
+		require.NoError(t, err)
+		assert.Equal(t, sdk.NewSchemaObjectIdentifier("test_db", "test_schema", "test_tag"), tid)
+		assert.Len(t, identifiers, 1)
+		assert.Equal(t, `"orgname"."accountname"`, identifiers[0].FullyQualifiedName())
+		assert.Equal(t, sdk.ObjectTypeAccount, objectType)
+	})
 	t.Run("account object identifier", func(t *testing.T) {
 		in := map[string]any{
 			"tag_id":      "\"test_db\".\"test_schema\".\"test_tag\"",

pkg/resources/testdata/TestAcc_TagAssociation/issue1202/main.tf

@@ -15,11 +15,6 @@ resource "snowflake_table" "test" {
 }
 
 resource "snowflake_tag_association" "test" {
-  // we need to set the object_identifier to avoid the following error:
-  // provider_test.go:17: err: resource snowflake_tag_association: object_identifier: Optional or Required must be set, not both
-  // we should consider deprecating object_identifier in favor of object_name
-  // https://github.com/Snowflake-Labs/terraform-provider-snowflake/pull/2534#discussion_r1507570740
-  // object_name = "\"${var.database}\".\"${var.schema}\".\"${var.table_name}\""
   object_identifiers = [snowflake_table.test.fully_qualified_name]
   object_type        = "TABLE"
   tag_id             = snowflake_tag.test.fully_qualified_name

pkg/resources/testdata/TestAcc_TagAssociation/issue1909/test.tf

@@ -25,18 +25,8 @@ resource "snowflake_table" "test2" {
 
 resource "snowflake_tag_association" "test" {
   object_identifiers = [var.column_fully_qualified_name, var.column2_fully_qualified_name]
-  # object_identifier {
-  #   database = var.database
-  #   schema   = var.schema
-  #   name     = "${snowflake_table.test.name}.${snowflake_table.test.column[0].name}"
-  # }
-  # object_identifier {
-  #   database = var.database
-  #   schema   = var.schema
-  #   name     = "${snowflake_table.test2.name}.${snowflake_table.test2.column[0].name}"
-  # }
-  object_type = "COLUMN"
-  tag_id      = snowflake_tag.test.fully_qualified_name
-  tag_value   = "v1"
-  depends_on  = [snowflake_table.test, snowflake_table.test2]
+  object_type        = "COLUMN"
+  tag_id             = snowflake_tag.test.fully_qualified_name
+  tag_value          = "v1"
+  depends_on         = [snowflake_table.test, snowflake_table.test2]
 }

pkg/resources/testdata/TestAcc_TagAssociation/issue1926/test.tf

@@ -7,6 +7,8 @@ resource "snowflake_table" "test" {
   name     = var.table_name
   database = var.database
   schema   = var.schema
+  // TODO(SNOW-1348114): use only one column, if possible.
+  // We need a dummy column here because a table must have at least one column, and when we rename the second one in the config, it gets dropped for a moment.
   column {
     name = "DUMMY"
     type = "VARIANT"

pkg/sdk/tags_impl.go

@@ -149,6 +149,7 @@ func (s *SetTagRequest) toOpts() *setTagOptions {
 			o.column = String(id.Name())
 		}
 	}
+	// TODO(SNOW-1818976): remove this hack
 	if o.objectType == ObjectTypeAccount {
 		id, ok := o.objectName.(AccountIdentifier)
 		if ok {
@@ -174,6 +175,7 @@ func (s *UnsetTagRequest) toOpts() *unsetTagOptions {
 			o.column = String(id.Name())
 		}
 	}
+	// TODO(SNOW-1818976): remove this hack
 	if o.objectType == ObjectTypeAccount {
 		id, ok := o.objectName.(AccountIdentifier)
 		if ok {