Fix final test

Snowflake-Labs · Aug 22, 2024 · 3f07fef · 3f07fef
1 parent c35c252
commit 3f07fef
Show file tree

Hide file tree

Showing 16 changed files with 224 additions and 57 deletions.
diff --git a/pkg/resources/account_role.go b/pkg/resources/account_role.go
@@ -4,9 +4,8 @@ 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/helpers"
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider"
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/schemas"
 
@@ -56,17 +55,7 @@ func AccountRole() *schema.Resource {
 		),
 
 		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,
-			},
+			StateContext: ImportName[sdk.AccountObjectIdentifier],
 		},
 	}
 }
@@ -132,16 +121,6 @@ func ReadAccountRole(ctx context.Context, d *schema.ResourceData, meta any) diag
 		return diag.FromErr(err)
 	}
 
-	if err := d.Set("name", sdk.NewAccountObjectIdentifier(accountRole.Name).Name()); err != nil {
-		return diag.Diagnostics{
-			diag.Diagnostic{
-				Severity: diag.Error,
-				Summary:  "Failed to set account role name",
-				Detail:   fmt.Sprintf("Account role name: %s, err: %s", accountRole.Name, err),
-			},
-		}
-	}
-
 	if err := d.Set("comment", accountRole.Comment); err != nil {
 		return diag.Diagnostics{
 			diag.Diagnostic{

diff --git a/pkg/resources/account_role_acceptance_test.go b/pkg/resources/account_role_acceptance_test.go
@@ -3,9 +3,10 @@ package resources_test
 import (
 	"context"
 	"fmt"
-	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"
 	"testing"
 
+	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"
+
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/importchecks"
 	"github.com/hashicorp/terraform-plugin-testing/plancheck"
 	"github.com/stretchr/testify/require"
@@ -235,8 +236,16 @@ func TestAcc_AccountRole_migrateFromV0941_ensureSmoothUpgradeWithNewResourceId(t
 			{
 				ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
 				Config:                   accountRoleBasicConfig(id.Name(), comment),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction("snowflake_account_role.role", plancheck.ResourceActionNoop),
+					},
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction("snowflake_account_role.role", plancheck.ResourceActionNoop),
+					},
+				},
 				Check: resource.ComposeAggregateTestCheckFunc(
-					resource.TestCheckResourceAttr("snowflake_account_role.role", "id", id.FullyQualifiedName()),
+					resource.TestCheckResourceAttr("snowflake_account_role.role", "id", id.Name()),
 				),
 			},
 		},

diff --git a/pkg/resources/common.go b/pkg/resources/common.go
@@ -1,8 +1,11 @@
 package resources
 
 import (
+	"context"
 	"strings"
 
+	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
+
 	"github.com/hashicorp/go-cty/cty"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
@@ -45,3 +48,49 @@ func ctyValToSliceString(valueElems []cty.Value) []string {
 	}
 	return elems
 }
+
+func ImportName[T sdk.AccountObjectIdentifier | sdk.DatabaseObjectIdentifier | sdk.SchemaObjectIdentifier](ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) {
+	switch any(new(T)).(type) {
+	case sdk.AccountObjectIdentifier:
+		id, err := sdk.ParseAccountObjectIdentifier(d.Id())
+		if err != nil {
+			return nil, err
+		}
+
+		if err := d.Set("name", id.Name()); err != nil {
+			return nil, err
+		}
+	case sdk.DatabaseObjectIdentifier:
+		id, err := sdk.ParseDatabaseObjectIdentifier(d.Id())
+		if err != nil {
+			return nil, err
+		}
+
+		if err := d.Set("name", id.Name()); err != nil {
+			return nil, err
+		}
+
+		if err := d.Set("database", id.DatabaseName()); err != nil {
+			return nil, err
+		}
+	case sdk.SchemaObjectIdentifier:
+		id, err := sdk.ParseSchemaObjectIdentifier(d.Id())
+		if err != nil {
+			return nil, err
+		}
+
+		if err := d.Set("name", id.Name()); err != nil {
+			return nil, err
+		}
+
+		if err := d.Set("database", id.DatabaseName()); err != nil {
+			return nil, err
+		}
+
+		if err := d.Set("schema", id.SchemaName()); err != nil {
+			return nil, err
+		}
+	}
+
+	return []*schema.ResourceData{d}, nil
+}
diff --git a/pkg/resources/database_role.go b/pkg/resources/database_role.go
@@ -3,9 +3,9 @@ package resources
 import (
 	"context"
 	"fmt"
-	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"
 	"log"
 
+	"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"
@@ -48,7 +48,7 @@ func DatabaseRole() *schema.Resource {
 
 		Schema: databaseRoleSchema,
 		Importer: &schema.ResourceImporter{
-			StateContext: schema.ImportStatePassthroughContext,
+			StateContext: ImportName[sdk.DatabaseObjectIdentifier],
 		},
 
 		SchemaVersion: 1,
@@ -85,14 +85,6 @@ func ReadDatabaseRole(d *schema.ResourceData, meta interface{}) error {
 		return err
 	}
 
-	if err := d.Set("name", databaseRole.Name); err != nil {
-		return err
-	}
-
-	if err := d.Set("database", id.DatabaseName()); err != nil {
-		return err
-	}
-
 	if err := d.Set("comment", databaseRole.Comment); err != nil {
 		return err
 	}

diff --git a/pkg/resources/database_role_acceptance_test.go b/pkg/resources/database_role_acceptance_test.go
@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/hashicorp/terraform-plugin-testing/plancheck"
+
 	acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
 
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random"
@@ -84,6 +86,14 @@ func TestAcc_DatabaseRole_migrateFromV0941_ensureSmoothUpgradeWithNewResourceId(
 			{
 				ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
 				Config:                   databaseRoleConfig(id.Name(), id.DatabaseName(), comment),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction("snowflake_database_role.test_db_role", plancheck.ResourceActionNoop),
+					},
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction("snowflake_database_role.test_db_role", plancheck.ResourceActionNoop),
+					},
+				},
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttr("snowflake_database_role.test_db_role", "id", id.FullyQualifiedName()),
 				),
@@ -121,6 +131,14 @@ func TestAcc_DatabaseRole_IdentifierQuotingDiffSuppression(t *testing.T) {
 			{
 				ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
 				Config:                   databaseRoleConfig(quotedDatabaseRoleId, id.DatabaseName(), comment),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction("snowflake_database_role.test_db_role", plancheck.ResourceActionNoop),
+					},
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction("snowflake_database_role.test_db_role", plancheck.ResourceActionNoop),
+					},
+				},
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttr("snowflake_database_role.test_db_role", "database", id.DatabaseName()),
 					resource.TestCheckResourceAttr("snowflake_database_role.test_db_role", "name", id.Name()),

diff --git a/pkg/resources/grant_account_role_acceptance_test.go b/pkg/resources/grant_account_role_acceptance_test.go
@@ -5,6 +5,8 @@ import (
 	"regexp"
 	"testing"
 
+	"github.com/hashicorp/terraform-plugin-testing/plancheck"
+
 	acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
 
 	"github.com/hashicorp/terraform-plugin-testing/config"
@@ -115,6 +117,14 @@ func TestAcc_GrantAccountRole_migrateFromV0941_ensureSmoothUpgradeWithNewResourc
 			{
 				ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
 				Config:                   grantAccountRoleBasicConfig(roleName.Name(), parentRoleName.Name()),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction("snowflake_grant_account_role.test", plancheck.ResourceActionNoop),
+					},
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction("snowflake_grant_account_role.test", plancheck.ResourceActionNoop),
+					},
+				},
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttr("snowflake_grant_account_role.test", "id", fmt.Sprintf(`%v|ROLE|%v`, roleName.FullyQualifiedName(), parentRoleName.FullyQualifiedName())),
 				),
@@ -171,6 +181,14 @@ func TestAcc_GrantAccountRole_IdentifierQuotingDiffSuppression(t *testing.T) {
 			{
 				ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
 				Config:                   grantAccountRoleBasicConfig(quotedRoleId, quotedParentRoleId),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction("snowflake_grant_account_role.test", plancheck.ResourceActionNoop),
+					},
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction("snowflake_grant_account_role.test", plancheck.ResourceActionNoop),
+					},
+				},
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttr("snowflake_grant_account_role.test", "role_name", roleId.Name()),
 					resource.TestCheckResourceAttr("snowflake_grant_account_role.test", "parent_role_name", parentRoleId.Name()),

diff --git a/pkg/resources/grant_application_role_acceptance_test.go b/pkg/resources/grant_application_role_acceptance_test.go
@@ -5,6 +5,8 @@ import (
 	"regexp"
 	"testing"
 
+	"github.com/hashicorp/terraform-plugin-testing/plancheck"
+
 	acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
 
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/testvars"
@@ -152,6 +154,14 @@ func TestAcc_GrantApplicationRole_migrateFromV0941_ensureSmoothUpgradeWithNewRes
 			{
 				ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
 				Config:                   grantApplicationRoleBasicConfig(fmt.Sprintf(`\"%s\".\"%s\"`, appRoleId.DatabaseName(), appRoleId.Name()), parentRoleId.Name()),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction("snowflake_grant_application_role.test", plancheck.ResourceActionNoop),
+					},
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction("snowflake_grant_application_role.test", plancheck.ResourceActionNoop),
+					},
+				},
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttr("snowflake_grant_application_role.test", "id", fmt.Sprintf(`%s|ACCOUNT_ROLE|%s`, appRoleId.FullyQualifiedName(), parentRoleId.FullyQualifiedName())),
 				),
@@ -162,19 +172,15 @@ func TestAcc_GrantApplicationRole_migrateFromV0941_ensureSmoothUpgradeWithNewRes
 
 func grantApplicationRoleBasicConfig(applicationRoleName string, parentRoleName string) string {
 	return fmt.Sprintf(`
-locals {
-  application_role_identifier = "%s"
-}
-
 resource "snowflake_account_role" "test" {
   name = "%s"
 }
 
 resource "snowflake_grant_application_role" "test" {
-  application_role_name    = local.application_role_identifier
-  parent_account_role_name = "${snowflake_account_role.test.name}"
+  application_role_name    = "%s"
+  parent_account_role_name = snowflake_account_role.test.name
 }
-`, applicationRoleName, parentRoleName)
+`, parentRoleName, applicationRoleName)
 }
 
 func TestAcc_GrantApplicationRole_IdentifierQuotingDiffSuppression(t *testing.T) {
@@ -206,6 +212,14 @@ func TestAcc_GrantApplicationRole_IdentifierQuotingDiffSuppression(t *testing.T)
 			{
 				ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
 				Config:                   grantApplicationRoleBasicConfig(unquotedApplicationRoleId, quotedParentRoleId),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction("snowflake_grant_application_role.test", plancheck.ResourceActionNoop),
+					},
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction("snowflake_grant_application_role.test", plancheck.ResourceActionNoop),
+					},
+				},
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttr("snowflake_grant_application_role.test", "application_role_name", unquotedApplicationRoleId),
 					resource.TestCheckResourceAttr("snowflake_grant_application_role.test", "parent_account_role_name", parentRoleId.Name()),

diff --git a/pkg/resources/grant_database_role_acceptance_test.go b/pkg/resources/grant_database_role_acceptance_test.go
@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/hashicorp/terraform-plugin-testing/plancheck"
+
 	acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
 
 	"github.com/hashicorp/terraform-plugin-testing/config"
@@ -194,6 +196,14 @@ func TestAcc_GrantDatabaseRole_migrateFromV0941_ensureSmoothUpgradeWithNewResour
 			{
 				ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
 				Config:                   grantDatabaseRoleBasicConfigQuoted(databaseId.Name(), roleId.Name(), parentRoleId.Name()),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction("snowflake_grant_database_role.test", plancheck.ResourceActionNoop),
+					},
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction("snowflake_grant_database_role.test", plancheck.ResourceActionNoop),
+					},
+				},
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttr("snowflake_grant_database_role.test", "id", fmt.Sprintf(`%s|DATABASE ROLE|%s`, roleId.FullyQualifiedName(), parentRoleId.FullyQualifiedName())),
 				),
@@ -276,6 +286,14 @@ func TestAcc_GrantDatabaseRole_IdentifierQuotingDiffSuppression(t *testing.T) {
 			{
 				ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
 				Config:                   grantDatabaseRoleBasicConfigUnquoted(databaseId.Name(), roleId.Name(), parentRoleId.Name()),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction("snowflake_grant_database_role.test", plancheck.ResourceActionNoop),
+					},
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction("snowflake_grant_database_role.test", plancheck.ResourceActionNoop),
+					},
+				},
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttr("snowflake_grant_database_role.test", "database_role_name", fmt.Sprintf("%s.%s", roleId.DatabaseName(), roleId.Name())),
 					resource.TestCheckResourceAttr("snowflake_grant_database_role.test", "parent_database_role_name", fmt.Sprintf("%s.%s", roleId.DatabaseName(), parentRoleId.Name())),

diff --git a/pkg/resources/grant_ownership.go b/pkg/resources/grant_ownership.go
@@ -178,7 +178,7 @@ func ImportGrantOwnership() schema.StateContextFunc {
 
 		switch id.GrantOwnershipTargetRoleKind {
 		case ToAccountGrantOwnershipTargetRoleKind:
-			if err := d.Set("account_role_name", id.AccountRoleName.FullyQualifiedName()); err != nil {
+			if err := d.Set("account_role_name", id.AccountRoleName.Name()); err != nil {
 				return nil, err
 			}
 		case ToDatabaseGrantOwnershipTargetRoleKind:
@@ -199,7 +199,11 @@ func ImportGrantOwnership() schema.StateContextFunc {
 
 			onObject := make(map[string]any)
 			onObject["object_type"] = data.ObjectType.String()
-			onObject["object_name"] = data.ObjectName.FullyQualifiedName()
+			if objectName, ok := any(data.ObjectName).(sdk.AccountObjectIdentifier); ok {
+				onObject["object_name"] = objectName.Name()
+			} else {
+				onObject["object_name"] = data.ObjectName.FullyQualifiedName()
+			}
 
 			if err := d.Set("on", []any{onObject}); err != nil {
 				return nil, err
@@ -212,7 +216,7 @@ func ImportGrantOwnership() schema.StateContextFunc {
 			onAllOrFuture["object_type_plural"] = data.ObjectNamePlural.String()
 			switch data.Kind {
 			case InDatabaseBulkOperationGrantKind:
-				onAllOrFuture["in_database"] = data.Database.FullyQualifiedName()
+				onAllOrFuture["in_database"] = data.Database.Name()
 			case InSchemaBulkOperationGrantKind:
 				onAllOrFuture["in_schema"] = data.Schema.FullyQualifiedName()
 			}
@@ -502,7 +506,7 @@ func getOwnershipGrantTo(d *schema.ResourceData) (sdk.OwnershipGrantTo, error) {
 		if err != nil {
 			return ownershipGrantTo, err
 		}
-		ownershipGrantTo.AccountRoleName = sdk.Pointer(accountRoleId)
+		ownershipGrantTo.AccountRoleName = &accountRoleId
 	}
 
 	if databaseRoleName, ok := d.GetOk("database_role_name"); ok {