chore: update password policy attachment (#2485)

Continue the change for password policy attachment (to a user): - minor resource changes - minor test changes - changed returned field types for policy_references call - added code conventions - added integration test
Snowflake-Labs · Feb 13, 2024 · 6ec9ff7 · 6ec9ff7
1 parent d565af1
commit 6ec9ff7
Show file tree

Hide file tree

Showing 10 changed files with 275 additions and 243 deletions.
diff --git a/docs/resources/user_password_policy_attachment.md b/docs/resources/user_password_policy_attachment.md
@@ -35,9 +35,7 @@ resource "snowflake_user_password_policy_attachment" "ppa" {
 
 ### Required
 
-- `password_policy_database` (String) Database name where the password policy is stored
-- `password_policy_name` (String) Non-qualified name of the password policy
-- `password_policy_schema` (String) Schema name where the password policy is stored
+- `password_policy_name` (String) Fully qualified name of the password policy
 - `user_name` (String) User name of the user you want to attach the password policy to
 
 ### Read-Only

diff --git a/pkg/resources/user_password_policy_attachment.go b/pkg/resources/user_password_policy_attachment.go
@@ -18,53 +18,35 @@ var userPasswordPolicyAttachmentSchema = map[string]*schema.Schema{
 		ForceNew:    true,
 		Description: "User name of the user you want to attach the password policy to",
 	},
-	"password_policy_database": {
-		Type:        schema.TypeString,
-		Required:    true,
-		ForceNew:    true,
-		Description: "Database name where the password policy is stored",
-	},
-	"password_policy_schema": {
-		Type:        schema.TypeString,
-		Required:    true,
-		ForceNew:    true,
-		Description: "Schema name where the password policy is stored",
-	},
 	"password_policy_name": {
-		Type:        schema.TypeString,
-		Required:    true,
-		ForceNew:    true,
-		Description: "Non-qualified name of the password policy",
+		Type:             schema.TypeString,
+		Required:         true,
+		ForceNew:         true,
+		Description:      "Fully qualified name of the password policy",
+		ValidateDiagFunc: IsValidIdentifier[sdk.SchemaObjectIdentifier](),
 	},
 }
 
 func UserPasswordPolicyAttachment() *schema.Resource {
 	return &schema.Resource{
 		Description: "Specifies the password policy to use for a certain user.",
-
-		Create: CreateUserPasswordPolicyAttachment,
-		Read:   ReadUserPasswordPolicyAttachment,
-		Delete: DeleteUserPasswordPolicyAttachment,
-
-		Schema: userPasswordPolicyAttachmentSchema,
-
+		Create:      CreateUserPasswordPolicyAttachment,
+		Read:        ReadUserPasswordPolicyAttachment,
+		Delete:      DeleteUserPasswordPolicyAttachment,
+		Schema:      userPasswordPolicyAttachmentSchema,
 		Importer: &schema.ResourceImporter{
 			StateContext: schema.ImportStatePassthroughContext,
 		},
 	}
 }
 
-func CreateUserPasswordPolicyAttachment(d *schema.ResourceData, meta interface{}) error {
+func CreateUserPasswordPolicyAttachment(d *schema.ResourceData, meta any) error {
 	db := meta.(*sql.DB)
 	client := sdk.NewClientFromDB(db)
 	ctx := context.Background()
 
 	userName := sdk.NewAccountObjectIdentifierFromFullyQualifiedName(d.Get("user_name").(string))
-	passwordPolicy := sdk.NewSchemaObjectIdentifier(
-		d.Get("password_policy_database").(string),
-		d.Get("password_policy_schema").(string),
-		d.Get("password_policy_name").(string),
-	)
+	passwordPolicy := sdk.NewSchemaObjectIdentifierFromFullyQualifiedName(d.Get("password_policy_name").(string))
 
 	err := client.Users.Alter(ctx, userName, &sdk.AlterUserOptions{
 		Set: &sdk.UserSet{
@@ -74,27 +56,25 @@ func CreateUserPasswordPolicyAttachment(d *schema.ResourceData, meta interface{}
 	if err != nil {
 		return err
 	}
-	d.SetId(fmt.Sprintf(`%s|%s`, helpers.EncodeSnowflakeID(passwordPolicy), helpers.EncodeSnowflakeID(userName)))
+
+	d.SetId(helpers.EncodeSnowflakeID(userName.FullyQualifiedName(), passwordPolicy.FullyQualifiedName()))
 
 	return ReadUserPasswordPolicyAttachment(d, meta)
 }
 
-func ReadUserPasswordPolicyAttachment(d *schema.ResourceData, meta interface{}) error {
-	parts := strings.Split(d.Id(), helpers.IDDelimiter)
-	if len(parts) != 4 {
-		// Note: this exception handling is particularly useful when importing
-		return fmt.Errorf("id should be in the format 'database|schema|password_policy|user_name', but I got '%s'", d.Id())
-	}
-	// Note: there is no alphanumeric id for an attachment, so we retrieve the password policies attached to a certain user.
-	userName := sdk.NewAccountObjectIdentifierFromFullyQualifiedName(parts[3])
+func ReadUserPasswordPolicyAttachment(d *schema.ResourceData, meta any) error {
 	db := meta.(*sql.DB)
 	client := sdk.NewClientFromDB(db)
 	ctx := context.Background()
-	policyReferences, err := client.PolicyReferences.GetForEntity(ctx, &sdk.GetForEntityPolicyReferenceRequest{
-		// Note: I cannot insert both single and double quotes in the SDK, so for now I need to do this
-		RefEntityName:   userName.FullyQualifiedName(),
-		RefEntityDomain: "user",
-	})
+
+	parts := strings.Split(d.Id(), helpers.IDDelimiter)
+	if len(parts) != 2 {
+		return fmt.Errorf("required id format 'user_name|password_policy_name', but got: '%s'", d.Id())
+	}
+
+	// Note: there is no alphanumeric id for an attachment, so we retrieve the password policies attached to a certain user.
+	userName := sdk.NewAccountObjectIdentifierFromFullyQualifiedName(parts[0])
+	policyReferences, err := client.PolicyReferences.GetForEntity(ctx, sdk.NewGetForEntityPolicyReferenceRequest(userName, sdk.PolicyEntityDomainUser))
 	if err != nil {
 		return err
 	}
@@ -109,23 +89,24 @@ func ReadUserPasswordPolicyAttachment(d *schema.ResourceData, meta interface{})
 		d.SetId("")
 		return nil
 	}
-	if err := d.Set("password_policy_database", sdk.NewAccountIdentifierFromFullyQualifiedName(policyReferences[0].PolicyDb).Name()); err != nil {
-		return err
-	}
-	if err := d.Set("password_policy_schema", sdk.NewAccountIdentifierFromFullyQualifiedName(policyReferences[0].PolicySchema).Name()); err != nil {
-		return err
-	}
-	if err := d.Set("password_policy_name", sdk.NewAccountIdentifierFromFullyQualifiedName(policyReferences[0].PolicyName).Name()); err != nil {
+
+	if err := d.Set("user_name", userName.Name()); err != nil {
 		return err
 	}
-	if err := d.Set("user_name", helpers.EncodeSnowflakeID(userName)); err != nil {
+	if err := d.Set(
+		"password_policy_name",
+		sdk.NewSchemaObjectIdentifier(
+			*policyReferences[0].PolicyDb,
+			*policyReferences[0].PolicySchema,
+			policyReferences[0].PolicyName,
+		).FullyQualifiedName()); err != nil {
 		return err
 	}
+
 	return err
 }
 
-// DeleteAccountPasswordPolicyAttachment implements schema.DeleteFunc.
-func DeleteUserPasswordPolicyAttachment(d *schema.ResourceData, meta interface{}) error {
+func DeleteUserPasswordPolicyAttachment(d *schema.ResourceData, meta any) error {
 	db := meta.(*sql.DB)
 	client := sdk.NewClientFromDB(db)
 	ctx := context.Background()
@@ -137,12 +118,11 @@ func DeleteUserPasswordPolicyAttachment(d *schema.ResourceData, meta interface{}
 			PasswordPolicy: sdk.Bool(true),
 		},
 	})
-
-	d.SetId("")
-
 	if err != nil {
 		return err
 	}
 
+	d.SetId("")
+
 	return nil
 }
diff --git a/pkg/resources/user_password_policy_attachment_acceptance_test.go b/pkg/resources/user_password_policy_attachment_acceptance_test.go
@@ -16,8 +16,10 @@ import (
 )
 
 func TestAcc_UserPasswordPolicyAttachment(t *testing.T) {
-	prefix := "tst-terraform" + strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
-	prefix2 := "tst-terraform" + strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+	userName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+	NewUserName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+	passwordPolicyName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+	newPasswordPolicyName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
 
 	resource.Test(t, resource.TestCase{
 		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
@@ -26,18 +28,21 @@ func TestAcc_UserPasswordPolicyAttachment(t *testing.T) {
 		Steps: []resource.TestStep{
 			// CREATE
 			{
-				Config: userPasswordPolicyAttachmentConfig("USER", acc.TestDatabaseName, acc.TestSchemaName, prefix),
+				Config: userPasswordPolicyAttachmentConfig(userName, acc.TestDatabaseName, acc.TestSchemaName, passwordPolicyName),
 				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttrSet("snowflake_user_password_policy_attachment.ppa", "id"),
+					resource.TestCheckResourceAttr("snowflake_user_password_policy_attachment.ppa", "user_name", userName),
+					resource.TestCheckResourceAttr("snowflake_user_password_policy_attachment.ppa", "password_policy_name", sdk.NewSchemaObjectIdentifier(acc.TestDatabaseName, acc.TestSchemaName, passwordPolicyName).FullyQualifiedName()),
+					resource.TestCheckResourceAttr("snowflake_user_password_policy_attachment.ppa", "id", fmt.Sprintf("%s|%s", sdk.NewAccountObjectIdentifier(userName).FullyQualifiedName(), sdk.NewSchemaObjectIdentifier(acc.TestDatabaseName, acc.TestSchemaName, passwordPolicyName).FullyQualifiedName())),
 				),
 				Destroy: false,
 			},
 			// UPDATE
 			{
-				Config: userPasswordPolicyAttachmentConfig(fmt.Sprintf("USER_%s", prefix), acc.TestDatabaseName, acc.TestSchemaName, prefix2),
+				Config: userPasswordPolicyAttachmentConfig(NewUserName, acc.TestDatabaseName, acc.TestSchemaName, newPasswordPolicyName),
 				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttrSet("snowflake_user_password_policy_attachment.ppa", "id"),
-					resource.TestCheckResourceAttr("snowflake_user_password_policy_attachment.ppa", "user_name", fmt.Sprintf("USER_%s", prefix)),
+					resource.TestCheckResourceAttr("snowflake_user_password_policy_attachment.ppa", "user_name", NewUserName),
+					resource.TestCheckResourceAttr("snowflake_user_password_policy_attachment.ppa", "password_policy_name", sdk.NewSchemaObjectIdentifier(acc.TestDatabaseName, acc.TestSchemaName, newPasswordPolicyName).FullyQualifiedName()),
+					resource.TestCheckResourceAttr("snowflake_user_password_policy_attachment.ppa", "id", fmt.Sprintf("%s|%s", sdk.NewAccountObjectIdentifier(NewUserName).FullyQualifiedName(), sdk.NewSchemaObjectIdentifier(acc.TestDatabaseName, acc.TestSchemaName, newPasswordPolicyName).FullyQualifiedName())),
 				),
 			},
 			// IMPORT
@@ -55,46 +60,41 @@ func testAccCheckUserPasswordPolicyAttachmentDestroy(s *terraform.State) error {
 	client := sdk.NewClientFromDB(db)
 	ctx := context.Background()
 	for _, rs := range s.RootModule().Resources {
-		// Note: I leverage the fact that the state during the test is specific to the test case, so there should only be there resources created in this test
 		if rs.Type != "snowflake_user_password_policy_attachment" {
 			continue
 		}
-		userName := sdk.NewAccountObjectIdentifierFromFullyQualifiedName(rs.Primary.Attributes["user_name"])
-		policyReferences, err := client.PolicyReferences.GetForEntity(ctx, &sdk.GetForEntityPolicyReferenceRequest{
-			RefEntityName:   userName.FullyQualifiedName(),
-			RefEntityDomain: "user",
-		})
+		policyReferences, err := client.PolicyReferences.GetForEntity(ctx, sdk.NewGetForEntityPolicyReferenceRequest(
+			sdk.NewAccountObjectIdentifierFromFullyQualifiedName(rs.Primary.Attributes["user_name"]),
+			sdk.PolicyEntityDomainUser,
+		))
 		if err != nil {
 			if strings.Contains(err.Error(), "does not exist or not authorized") {
-				// Note: this can happen if the Policy Reference or the User have been deleted as well; in this case, just ignore the error
+				// Note: this can happen if the Policy Reference or the User has been deleted as well; in this case, ignore the error
 				continue
 			}
 			return err
 		}
 		if len(policyReferences) > 0 {
-			return fmt.Errorf("User Password Policy attachment %v still exists", policyReferences[0].PolicyName)
+			return fmt.Errorf("user password policy attachment %v still exists", policyReferences[0].PolicyName)
 		}
 	}
 	return nil
 }
 
-func userPasswordPolicyAttachmentConfig(userName, databaseName, schemaName, prefix string) string {
-	s := `
+func userPasswordPolicyAttachmentConfig(userName, databaseName, schemaName, passwordPolicyName string) string {
+	return fmt.Sprintf(`
 resource "snowflake_user" "user" {
 	name = "%s"
 }
 resource "snowflake_password_policy" "pp" {
 	database   = "%s"
 	schema     = "%s"
-	name       = "pp_%v"
+	name       = "%s"
 }
 
 resource "snowflake_user_password_policy_attachment" "ppa" {
-	password_policy_database = snowflake_password_policy.pp.database
-	password_policy_schema = snowflake_password_policy.pp.schema
-	password_policy_name = snowflake_password_policy.pp.name
+	password_policy_name = "\"${snowflake_password_policy.pp.database}\".\"${snowflake_password_policy.pp.schema}\".\"${snowflake_password_policy.pp.name}\""
 	user_name = snowflake_user.user.name
 }
-`
-	return fmt.Sprintf(s, userName, databaseName, schemaName, prefix)
+`, userName, databaseName, schemaName, passwordPolicyName)
 }