Skip to content

Commit

Permalink
fix: Fix user resource import (#3181)
Browse files Browse the repository at this point in the history
Handle user import correctly.

Password is empty after the `snowflake_user` import; we can't read it
from the config or from Snowflake.
During the next terraform plan+apply it's updated to the "same" value.
It results in an error on Snowflake side: `New password rejected by
current password policy. Reason: 'PRIOR_USE'.`
The error will be ignored on the provider side (after all, it means that
the password in state is the same as on Snowflake side). Still,
plan+apply is needed after importing user.
  • Loading branch information
sfc-gh-asawicki authored Nov 8, 2024
1 parent 55591da commit 34bbbc1
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 7 deletions.
12 changes: 12 additions & 0 deletions MIGRATION_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,18 @@ Note: Because [bcr 2024_07](https://docs.snowflake.com/en/release-notes/bcr-bund

Connected issues: [#3125](https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/3125)

### *(bugfix)* Handle user import correctly

#### Context before the change

Password is empty after the `snowflake_user` import; we can't read it from the config or from Snowflake.
During the next terraform plan+apply it's updated to the "same" value.
It results in an error on Snowflake side: `New password rejected by current password policy. Reason: 'PRIOR_USE'.`

#### After the change

The error will be ignored on the provider side (after all, it means that the password in state is the same as on Snowflake side). Still, plan+apply is needed after importing user.

## v0.96.0 ➞ v0.97.0

### *(new feature)* snowflake_stream_on_table, snowflake_stream_on_external_table resource
Expand Down
2 changes: 2 additions & 0 deletions docs/resources/legacy_service_user.md
Original file line number Diff line number Diff line change
Expand Up @@ -1011,3 +1011,5 @@ Import is supported using the following syntax:
```shell
terraform import snowflake_legacy_service_user.example '"<user_name>"'
```

Note: terraform plan+apply may be needed after successful import to fill out all the missing fields (like `password`) in state.
2 changes: 2 additions & 0 deletions docs/resources/user.md
Original file line number Diff line number Diff line change
Expand Up @@ -1021,3 +1021,5 @@ Import is supported using the following syntax:
```shell
terraform import snowflake_user.example '"<user_name>"'
```

Note: terraform plan+apply may be needed after successful import to fill out all the missing fields (like `password`) in state.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ func (u *UserResourceAssert) HasEmptyPassword() *UserResourceAssert {
return u
}

func (u *UserResourceAssert) HasNotEmptyPassword() *UserResourceAssert {
u.AddAssertion(assert.ValuePresent("password"))
return u
}

func (u *UserResourceAssert) HasMustChangePassword(expected bool) *UserResourceAssert {
u.AddAssertion(assert.ValueSet("must_change_password", strconv.FormatBool(expected)))
return u
Expand Down
50 changes: 43 additions & 7 deletions pkg/resources/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -512,17 +512,17 @@ func GetReadUserFunc(userType sdk.UserType, withExternalChangesMarking bool) sch
// can't read disable_mfa
d.Set("user_type", u.Type),

func() error {
func(rd *schema.ResourceData, ud *sdk.UserDetails) error {
var errs error
if userType == sdk.UserTypePerson {
errs = errors.Join(
setFromStringPropertyIfNotEmpty(d, "first_name", userDetails.FirstName),
setFromStringPropertyIfNotEmpty(d, "middle_name", userDetails.MiddleName),
setFromStringPropertyIfNotEmpty(d, "last_name", userDetails.LastName),
setFromStringPropertyIfNotEmpty(rd, "first_name", ud.FirstName),
setFromStringPropertyIfNotEmpty(rd, "middle_name", ud.MiddleName),
setFromStringPropertyIfNotEmpty(rd, "last_name", ud.LastName),
)
}
return errs
}(),
}(d, userDetails),

d.Set(FullyQualifiedNameAttributeName, id.FullyQualifiedName()),
handleUserParameterRead(d, userParameters),
Expand Down Expand Up @@ -607,7 +607,6 @@ func GetUpdateUserFunc(userType sdk.UserType) func(ctx context.Context, d *schem
switch userType {
case sdk.UserTypePerson:
userTypeSpecificFieldsErrs = errors.Join(
stringAttributeUpdate(d, "password", &setObjectProperties.Password, &unsetObjectProperties.Password),
stringAttributeUpdate(d, "first_name", &setObjectProperties.FirstName, &unsetObjectProperties.FirstName),
stringAttributeUpdate(d, "middle_name", &setObjectProperties.MiddleName, &unsetObjectProperties.MiddleName),
stringAttributeUpdate(d, "last_name", &setObjectProperties.LastName, &unsetObjectProperties.LastName),
Expand All @@ -617,7 +616,6 @@ func GetUpdateUserFunc(userType sdk.UserType) func(ctx context.Context, d *schem
)
case sdk.UserTypeLegacyService:
userTypeSpecificFieldsErrs = errors.Join(
stringAttributeUpdate(d, "password", &setObjectProperties.Password, &unsetObjectProperties.Password),
booleanStringAttributeUpdate(d, "must_change_password", &setObjectProperties.MustChangePassword, &unsetObjectProperties.MustChangePassword),
)
}
Expand All @@ -640,6 +638,10 @@ func GetUpdateUserFunc(userType sdk.UserType) func(ctx context.Context, d *schem
}
}

if err := handlePasswordUpdate(ctx, id, userType, d, client); err != nil {
return diag.FromErr(err)
}

set := &sdk.UserSet{
SessionParameters: &sdk.SessionParameters{},
ObjectParameters: &sdk.UserObjectParameters{},
Expand Down Expand Up @@ -677,6 +679,40 @@ func GetUpdateUserFunc(userType sdk.UserType) func(ctx context.Context, d *schem
}
}

// handlePasswordUpdate is a current workaround to handle user's password after import.
// Password is empty after the import, we can't read it from the config or from Snowflake.
// During the next terraform plan+apply it's updated to the "same" value.
// It results in an error on Snowflake side: New password rejected by current password policy. Reason: 'PRIOR_USE'.
// Current workaround is to ignore such an error. We will revisit it after migration to plugin framework.
func handlePasswordUpdate(ctx context.Context, id sdk.AccountObjectIdentifier, userType sdk.UserType, d *schema.ResourceData, client *sdk.Client) error {
if userType == sdk.UserTypePerson || userType == sdk.UserTypeLegacyService {
setPassword := sdk.UserAlterObjectProperties{}
unsetPassword := sdk.UserObjectPropertiesUnset{}
if err := stringAttributeUpdate(d, "password", &setPassword.Password, &unsetPassword.Password); err != nil {
return err
}
if (setPassword != sdk.UserAlterObjectProperties{}) {
err := client.Users.Alter(ctx, id, &sdk.AlterUserOptions{Set: &sdk.UserSet{ObjectProperties: &setPassword}})
if err != nil {
if strings.Contains(err.Error(), "Error: 003002 (28P01)") || strings.Contains(err.Error(), "Reason: 'PRIOR_USE'") {
logging.DebugLogger.Printf("[DEBUG] Update to the same password is prohibited but it means we have a valid password in the current state. Continue.")
} else {
d.Partial(true)
return err
}
}
}
if (unsetPassword != sdk.UserObjectPropertiesUnset{}) {
err := client.Users.Alter(ctx, id, &sdk.AlterUserOptions{Unset: &sdk.UserUnset{ObjectProperties: &unsetPassword}})
if err != nil {
d.Partial(true)
return err
}
}
}
return nil
}

func DeleteUser(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
client := meta.(*provider.Context).Client
id := helpers.DecodeSnowflakeID(d.Id()).(sdk.AccountObjectIdentifier)
Expand Down
62 changes: 62 additions & 0 deletions pkg/resources/user_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1753,3 +1753,65 @@ func TestAcc_User_handleChangesToShowUsers_bcr202408_migration_bcr202407_disable
},
})
}

func TestAcc_User_importPassword(t *testing.T) {
_ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance)
acc.TestAccPreCheck(t)

userId := acc.TestClient().Ids.RandomAccountObjectIdentifier()
pass := random.Password()
firstName := random.AlphaN(6)

_, userCleanup := acc.TestClient().User.CreateUserWithOptions(t, userId, &sdk.CreateUserOptions{ObjectProperties: &sdk.UserObjectProperties{
Password: sdk.String(pass),
FirstName: sdk.String(firstName),
}})
t.Cleanup(userCleanup)

userModel := model.User("w", userId.Name()).WithPassword(pass).WithFirstName(firstName)

resource.Test(t, resource.TestCase{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
tfversion.RequireAbove(tfversion.Version1_5_0),
},
PreCheck: func() { acc.TestAccPreCheck(t) },
CheckDestroy: acc.CheckDestroy(t, resources.User),
Steps: []resource.TestStep{
// IMPORT
{
Config: config.FromModel(t, userModel),
ResourceName: userModel.ResourceReference(),
ImportState: true,
ImportStateId: userId.Name(),
ImportStateCheck: assert.AssertThatImport(t,
resourceassert.ImportedUserResource(t, userId.Name()).
HasNoPassword().
HasFirstNameString(firstName),
),
ImportStatePersist: true,
},
{
Config: config.FromModel(t, userModel),
Check: assert.AssertThat(t,
resourceassert.UserResource(t, userModel.ResourceReference()).
HasNotEmptyPassword().
HasFirstNameString(firstName),
),
},
{
ConfigPlanChecks: resource.ConfigPlanChecks{
PreApply: []plancheck.PlanCheck{
plancheck.ExpectEmptyPlan(),
},
},
Config: config.FromModel(t, userModel),
Check: assert.AssertThat(t,
resourceassert.UserResource(t, userModel.ResourceReference()).
HasNotEmptyPassword().
HasFirstNameString(firstName),
),
},
},
})
}
2 changes: 2 additions & 0 deletions templates/resources/legacy_service_user.md.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,5 @@ Import is supported using the following syntax:

{{ codefile "shell" (printf "examples/resources/%s/import.sh" .Name)}}
{{- end }}

Note: terraform plan+apply may be needed after successful import to fill out all the missing fields (like `password`) in state.
2 changes: 2 additions & 0 deletions templates/resources/user.md.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,5 @@ Import is supported using the following syntax:

{{ codefile "shell" (printf "examples/resources/%s/import.sh" .Name)}}
{{- end }}

Note: terraform plan+apply may be needed after successful import to fill out all the missing fields (like `password`) in state.

0 comments on commit 34bbbc1

Please sign in to comment.