chore: apply identifier conventions to grants

pkg/datasources/grants.go

@@ -396,35 +396,59 @@ func buildOptsForGrantsTo(grantsTo map[string]any) (*sdk.ShowGrantOptions, error
 	opts := new(sdk.ShowGrantOptions)
 
 	if application := grantsTo["application"].(string); application != "" {
+		applicationId, err := sdk.ParseAccountObjectIdentifier(application)
+		if err != nil {
+			return nil, err
+		}
 		opts.To = &sdk.ShowGrantsTo{
-			Application: sdk.NewAccountObjectIdentifier(application),
+			Application: applicationId,
 		}
 	}
 	if applicationRole := grantsTo["application_role"].(string); applicationRole != "" {
+		applicationRoleId, err := sdk.ParseDatabaseObjectIdentifier(applicationRole)
+		if err != nil {
+			return nil, err
+		}
 		opts.To = &sdk.ShowGrantsTo{
-			ApplicationRole: sdk.NewDatabaseObjectIdentifierFromFullyQualifiedName(applicationRole),
+			ApplicationRole: applicationRoleId,
 		}
 	}
 	if accountRole := grantsTo["account_role"].(string); accountRole != "" {
+		accountRoleId, err := sdk.ParseAccountObjectIdentifier(accountRole)
+		if err != nil {
+			return nil, err
+		}
 		opts.To = &sdk.ShowGrantsTo{
-			Role: sdk.NewAccountObjectIdentifier(accountRole),
+			Role: accountRoleId,
 		}
 	}
 	if databaseRole := grantsTo["database_role"].(string); databaseRole != "" {
+		databaseRoleId, err := sdk.ParseDatabaseObjectIdentifier(databaseRole)
+		if err != nil {
+			return nil, err
+		}
 		opts.To = &sdk.ShowGrantsTo{
-			DatabaseRole: sdk.NewDatabaseObjectIdentifierFromFullyQualifiedName(databaseRole),
+			DatabaseRole: databaseRoleId,
 		}
 	}
 	if user := grantsTo["user"].(string); user != "" {
+		userId, err := sdk.ParseAccountObjectIdentifier(user)
+		if err != nil {
+			return nil, err
+		}
 		opts.To = &sdk.ShowGrantsTo{
-			User: sdk.NewAccountObjectIdentifier(user),
+			User: userId,
 		}
 	}
 	if share := grantsTo["share"]; share != nil && len(share.([]any)) > 0 {
 		shareMap := share.([]any)[0].(map[string]any)
+		sharedId, err := sdk.ParseAccountObjectIdentifier(shareMap["share_name"].(string))
+		if err != nil {
+			return nil, err
+		}
 		opts.To = &sdk.ShowGrantsTo{
 			Share: &sdk.ShowGrantsToShare{
-				Name: sdk.NewAccountObjectIdentifier(shareMap["share_name"].(string)),
+				Name: sharedId,
 			},
 		}
 		// TODO [SNOW-1284382]: Uncomment after SHOW GRANTS TO SHARE <share_name> IN APPLICATION PACKAGE <app_package_name> syntax starts working.
@@ -439,23 +463,39 @@ func buildOptsForGrantsOf(grantsOf map[string]any) (*sdk.ShowGrantOptions, error
 	opts := new(sdk.ShowGrantOptions)
 
 	if accountRole := grantsOf["account_role"].(string); accountRole != "" {
+		accountRoleId, err := sdk.ParseAccountObjectIdentifier(accountRole)
+		if err != nil {
+			return nil, err
+		}
 		opts.Of = &sdk.ShowGrantsOf{
-			Role: sdk.NewAccountObjectIdentifier(accountRole),
+			Role: accountRoleId,
 		}
 	}
 	if databaseRole := grantsOf["database_role"].(string); databaseRole != "" {
+		databaseRoleId, err := sdk.ParseDatabaseObjectIdentifier(databaseRole)
+		if err != nil {
+			return nil, err
+		}
 		opts.Of = &sdk.ShowGrantsOf{
-			DatabaseRole: sdk.NewDatabaseObjectIdentifierFromFullyQualifiedName(databaseRole),
+			DatabaseRole: databaseRoleId,
 		}
 	}
 	if applicationRole := grantsOf["application_role"].(string); applicationRole != "" {
+		applicationRoleId, err := sdk.ParseDatabaseObjectIdentifier(applicationRole)
+		if err != nil {
+			return nil, err
+		}
 		opts.Of = &sdk.ShowGrantsOf{
-			ApplicationRole: sdk.NewDatabaseObjectIdentifierFromFullyQualifiedName(applicationRole),
+			ApplicationRole: applicationRoleId,
 		}
 	}
 	if share := grantsOf["share"].(string); share != "" {
+		shareId, err := sdk.ParseAccountObjectIdentifier(share)
+		if err != nil {
+			return nil, err
+		}
 		opts.Of = &sdk.ShowGrantsOf{
-			Share: sdk.NewAccountObjectIdentifier(share),
+			Share: shareId,
 		}
 	}
 	return opts, nil
@@ -466,13 +506,21 @@ func buildOptsForFutureGrantsIn(futureGrantsIn map[string]any) (*sdk.ShowGrantOp
 	opts.Future = sdk.Bool(true)
 
 	if db := futureGrantsIn["database"].(string); db != "" {
+		databaseId, err := sdk.ParseAccountObjectIdentifier(db)
+		if err != nil {
+			return nil, err
+		}
 		opts.In = &sdk.ShowGrantsIn{
-			Database: sdk.Pointer(sdk.NewAccountObjectIdentifier(db)),
+			Database: sdk.Pointer(databaseId),
 		}
 	}
 	if sc := futureGrantsIn["schema"].(string); sc != "" {
+		schemaId, err := sdk.ParseDatabaseObjectIdentifier(sc)
+		if err != nil {
+			return nil, err
+		}
 		opts.In = &sdk.ShowGrantsIn{
-			Schema: sdk.Pointer(sdk.NewDatabaseObjectIdentifierFromFullyQualifiedName(sc)),
+			Schema: sdk.Pointer(schemaId),
 		}
 	}
 	return opts, nil
@@ -483,13 +531,21 @@ func buildOptsForFutureGrantsTo(futureGrantsTo map[string]any) (*sdk.ShowGrantOp
 	opts.Future = sdk.Bool(true)
 
 	if accountRole := futureGrantsTo["account_role"].(string); accountRole != "" {
+		accountRoleId, err := sdk.ParseAccountObjectIdentifier(accountRole)
+		if err != nil {
+			return nil, err
+		}
 		opts.To = &sdk.ShowGrantsTo{
-			Role: sdk.NewAccountObjectIdentifier(accountRole),
+			Role: accountRoleId,
 		}
 	}
 	if databaseRole := futureGrantsTo["database_role"].(string); databaseRole != "" {
+		databaseRoleId, err := sdk.ParseDatabaseObjectIdentifier(databaseRole)
+		if err != nil {
+			return nil, err
+		}
 		opts.To = &sdk.ShowGrantsTo{
-			DatabaseRole: sdk.NewDatabaseObjectIdentifierFromFullyQualifiedName(databaseRole),
+			DatabaseRole: databaseRoleId,
 		}
 	}
 	return opts, nil

pkg/helpers/resource_identifier.go

@@ -2,6 +2,8 @@ package helpers
 
 import (
 	"strings"
+
+	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
 )
 
 const ResourceIdDelimiter = '|'
@@ -13,6 +15,15 @@ func ParseResourceIdentifier(identifier string) []string {
 	return strings.Split(identifier, string(ResourceIdDelimiter))
 }
 
-func EncodeResourceIdentifier(parts ...string) string {
-	return strings.Join(parts, string(ResourceIdDelimiter))
+func EncodeResourceIdentifier[T sdk.AccountObjectIdentifier | sdk.DatabaseObjectIdentifier | sdk.SchemaObjectIdentifier | sdk.SchemaObjectIdentifierWithArguments | sdk.TableColumnIdentifier | sdk.ExternalObjectIdentifier | sdk.AccountIdentifier | string](parts ...T) string {
+	result := make([]string, len(parts))
+	for i, part := range parts {
+		switch typedPart := any(part).(type) {
+		case sdk.AccountObjectIdentifier, sdk.DatabaseObjectIdentifier, sdk.SchemaObjectIdentifier, sdk.SchemaObjectIdentifierWithArguments, sdk.TableColumnIdentifier, sdk.ExternalObjectIdentifier, sdk.AccountIdentifier:
+			result[i] = typedPart.(sdk.ObjectIdentifier).FullyQualifiedName()
+		case string:
+			result[i] = typedPart
+		}
+	}
+	return strings.Join(result, string(ResourceIdDelimiter))
 }

pkg/resources/account_role.go

@@ -4,11 +4,12 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"
+	"github.com/hashicorp/go-cty/cty"
 
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider"
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/schemas"
 
-	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
@@ -21,6 +22,7 @@ var accountRoleSchema = map[string]*schema.Schema{
 		Required: true,
 		// TODO(SNOW-999049): Uncomment once better identifier validation will be implemented
 		// ValidateDiagFunc: IsValidIdentifier[sdk.AccountObjectIdentifier](),
+		DiffSuppressFunc: suppressIdentifierQuoting,
 	},
 	"comment": {
 		Type:     schema.TypeString,
@@ -48,27 +50,41 @@ func AccountRole() *schema.Resource {
 		Description:   "The resource is used for role management, where roles can be assigned privileges and, in turn, granted to users and other roles. When granted to roles they can create hierarchies of privilege structures. For more details, refer to the [official documentation](https://docs.snowflake.com/en/user-guide/security-access-control-overview).",
 
 		CustomizeDiff: customdiff.All(
-			ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "name", "comment"),
-			ComputedIfAnyAttributeChanged(FullyQualifiedNameAttributeName, "name"),
+			ComputedIfAnyAttributeChanged(ShowOutputAttributeName, "comment"),
+			ComputedIfAnyAttributeChangedWithSuppressDiff(ShowOutputAttributeName, suppressIdentifierQuoting, "name"),
+			ComputedIfAnyAttributeChangedWithSuppressDiff(FullyQualifiedNameAttributeName, suppressIdentifierQuoting, "name"),
 		),
 
 		Importer: &schema.ResourceImporter{
 			StateContext: schema.ImportStatePassthroughContext,
 		},
+
+		SchemaVersion: 1,
+		StateUpgraders: []schema.StateUpgrader{
+			{
+				Version: 0,
+				// setting type to cty.EmptyObject is a bit hacky here but following https://developer.hashicorp.com/terraform/plugin/framework/migrating/resources/state-upgrade#sdkv2-1 would require lots of repetitive code; this should work with cty.EmptyObject
+				Type:    cty.EmptyObject,
+				Upgrade: migratePipeSeparatedObjectIdentifierResourceIdToFullyQualifiedName,
+			},
+		},
 	}
 }
 
 func CreateAccountRole(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
 	client := meta.(*provider.Context).Client
 
-	id := sdk.NewAccountObjectIdentifier(d.Get("name").(string))
+	id, err := sdk.ParseAccountObjectIdentifier(d.Get("name").(string))
+	if err != nil {
+		return diag.FromErr(err)
+	}
 	req := sdk.NewCreateRoleRequest(id)
 
 	if v, ok := d.GetOk("comment"); ok {
 		req.WithComment(v.(string))
 	}
 
-	err := client.Roles.Create(ctx, req)
+	err = client.Roles.Create(ctx, req)
 	if err != nil {
 		return diag.Diagnostics{
 			diag.Diagnostic{
@@ -79,14 +95,17 @@ func CreateAccountRole(ctx context.Context, d *schema.ResourceData, meta any) di
 		}
 	}
 
-	d.SetId(helpers.EncodeSnowflakeID(id))
+	d.SetId(helpers.EncodeResourceIdentifier(id))
 
 	return ReadAccountRole(ctx, d, meta)
 }
 
 func ReadAccountRole(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
 	client := meta.(*provider.Context).Client
-	id := helpers.DecodeSnowflakeID(d.Id()).(sdk.AccountObjectIdentifier)
+	id, err := sdk.ParseAccountObjectIdentifier(d.Id())
+	if err != nil {
+		return diag.FromErr(err)
+	}
 
 	accountRole, err := client.Roles.ShowByID(ctx, id)
 	if err != nil {
@@ -142,10 +161,16 @@ func ReadAccountRole(ctx context.Context, d *schema.ResourceData, meta any) diag
 
 func UpdateAccountRole(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
 	client := meta.(*provider.Context).Client
-	id := helpers.DecodeSnowflakeID(d.Id()).(sdk.AccountObjectIdentifier)
+	id, err := sdk.ParseAccountObjectIdentifier(d.Id())
+	if err != nil {
+		return diag.FromErr(err)
+	}
 
 	if d.HasChange("name") {
-		newId := sdk.NewAccountObjectIdentifier(d.Get("name").(string))
+		newId, err := sdk.ParseAccountObjectIdentifier(d.Get("name").(string))
+		if err != nil {
+			return diag.FromErr(err)
+		}
 
 		if err := client.Roles.Alter(ctx, sdk.NewAlterRoleRequest(id).WithRenameTo(newId)); err != nil {
 			return diag.Diagnostics{
@@ -158,7 +183,7 @@ func UpdateAccountRole(ctx context.Context, d *schema.ResourceData, meta any) di
 		}
 
 		id = newId
-		d.SetId(helpers.EncodeSnowflakeID(newId))
+		d.SetId(helpers.EncodeResourceIdentifier(newId))
 	}
 
 	if d.HasChange("comment") {
@@ -191,7 +216,10 @@ func UpdateAccountRole(ctx context.Context, d *schema.ResourceData, meta any) di
 
 func DeleteAccountRole(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
 	client := meta.(*provider.Context).Client
-	id := helpers.DecodeSnowflakeID(d.Id()).(sdk.AccountObjectIdentifier)
+	id, err := sdk.ParseAccountObjectIdentifier(d.Id())
+	if err != nil {
+		return diag.FromErr(err)
+	}
 
 	if err := client.Roles.Drop(ctx, sdk.NewDropRoleRequest(id).WithIfExists(true)); err != nil {
 		return diag.Diagnostics{

pkg/resources/account_role_acceptance_test.go

@@ -3,6 +3,7 @@
 import (
 	"context"
 	"fmt"
+	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"
 	"testing"
 
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/importchecks"
@@ -58,8 +59,8 @@
 				ResourceName: "snowflake_account_role.role",
 				ImportState:  true,
 				ImportStateCheck: importchecks.ComposeAggregateImportStateCheck(
-					importchecks.TestCheckResourceAttrInstanceState(id.Name(), "name", id.Name()),
-					importchecks.TestCheckResourceAttrInstanceState(id.Name(), "comment", ""),
+					importchecks.TestCheckResourceAttrInstanceState(helpers.EncodeResourceIdentifier(id), "name", id.Name()),
+					importchecks.TestCheckResourceAttrInstanceState(helpers.EncodeResourceIdentifier(id), "comment", ""),
 				),
 			},
 			// set optionals
@@ -93,8 +94,8 @@
 				ResourceName: "snowflake_account_role.role",
 				ImportState:  true,
 				ImportStateCheck: importchecks.ComposeAggregateImportStateCheck(
-					importchecks.TestCheckResourceAttrInstanceState(id.Name(), "name", id.Name()),
-					importchecks.TestCheckResourceAttrInstanceState(id.Name(), "comment", comment),
+					importchecks.TestCheckResourceAttrInstanceState(helpers.EncodeResourceIdentifier(id), "name", id.Name()),
+					importchecks.TestCheckResourceAttrInstanceState(helpers.EncodeResourceIdentifier(id), "comment", comment),
 				),
 			},
 			// unset
@@ -169,9 +170,9 @@
 				ResourceName: "snowflake_account_role.role",
 				ImportState:  true,
 				ImportStateCheck: importchecks.ComposeAggregateImportStateCheck(
-					importchecks.TestCheckResourceAttrInstanceState(id.Name(), "name", id.Name()),
-					importchecks.TestCheckResourceAttrInstanceState(id.Name(), "fully_qualified_name", id.FullyQualifiedName()),
-					importchecks.TestCheckResourceAttrInstanceState(id.Name(), "comment", comment),
+					importchecks.TestCheckResourceAttrInstanceState(helpers.EncodeResourceIdentifier(id), "name", id.Name()),
+					importchecks.TestCheckResourceAttrInstanceState(helpers.EncodeResourceIdentifier(id), "fully_qualified_name", id.FullyQualifiedName()),
+					importchecks.TestCheckResourceAttrInstanceState(helpers.EncodeResourceIdentifier(id), "comment", comment),
 				),
 			},
 			// rename + comment change
@@ -208,3 +209,36 @@
 `
 	return fmt.Sprintf(s, name, comment)
 }
+
+func TestAcc_AccountRole_migrateFromV0941_ensureSmoothUpgradeWithNewResourceId(t *testing.T) {
+	id := acc.TestClient().Ids.RandomAccountObjectIdentifier()
+	comment := random.Comment()
+
+	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",
+					},
+				},
+				Config: accountRoleBasicConfig(id.Name(), comment),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr("snowflake_account_role.role", "id", id.Name()),
+				),
+			},
+			{
+				ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+				Config:                   accountRoleBasicConfig(id.Name(), comment),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr("snowflake_account_role.role", "id", id.FullyQualifiedName()),
+				),
+			},
+		},
+	})
+}