From 6a0b414f3a81a9ea02be87324c36c7b3162a492f Mon Sep 17 00:00:00 2001 From: Agustin Bettati Date: Mon, 11 Mar 2024 13:38:56 +0100 Subject: [PATCH] fix: Avoids sending database user password in update request if the value has not changed --- .../databaseuser/model_database_user.go | 13 ++++-- .../databaseuser/model_database_user_test.go | 42 +++++++++++++++---- .../databaseuser/resource_database_user.go | 13 +++--- website/docs/r/database_user.html.markdown | 2 +- 4 files changed, 52 insertions(+), 18 deletions(-) diff --git a/internal/service/databaseuser/model_database_user.go b/internal/service/databaseuser/model_database_user.go index 635521d5e7..05f115f57e 100644 --- a/internal/service/databaseuser/model_database_user.go +++ b/internal/service/databaseuser/model_database_user.go @@ -11,7 +11,7 @@ import ( "go.mongodb.org/atlas-sdk/v20231115007/admin" ) -func NewMongoDBDatabaseUser(ctx context.Context, dbUserModel *TfDatabaseUserModel) (*admin.CloudDatabaseUser, diag.Diagnostics) { +func NewMongoDBDatabaseUser(ctx context.Context, statePasswordValue types.String, dbUserModel *TfDatabaseUserModel) (*admin.CloudDatabaseUser, diag.Diagnostics) { var rolesModel []*TfRoleModel var labelsModel []*TfLabelModel var scopesModel []*TfScopeModel @@ -31,10 +31,9 @@ func NewMongoDBDatabaseUser(ctx context.Context, dbUserModel *TfDatabaseUserMode return nil, diags } - return &admin.CloudDatabaseUser{ + result := admin.CloudDatabaseUser{ GroupId: dbUserModel.ProjectID.ValueString(), Username: dbUserModel.Username.ValueString(), - Password: dbUserModel.Password.ValueStringPointer(), X509Type: dbUserModel.X509Type.ValueStringPointer(), AwsIAMType: dbUserModel.AWSIAMType.ValueStringPointer(), OidcAuthType: dbUserModel.OIDCAuthType.ValueStringPointer(), @@ -43,7 +42,13 @@ func NewMongoDBDatabaseUser(ctx context.Context, dbUserModel *TfDatabaseUserMode Roles: NewMongoDBAtlasRoles(rolesModel), Labels: NewMongoDBAtlasLabels(labelsModel), Scopes: NewMongoDBAtlasScopes(scopesModel), - }, nil + } + + if statePasswordValue != dbUserModel.Password { + // Password value has been modified or no previous state was present. Password is only updated if changed in the terraform configuration CLOUDP-235738 + result.Password = dbUserModel.Password.ValueStringPointer() + } + return &result, nil } func NewTfDatabaseUserModel(ctx context.Context, model *TfDatabaseUserModel, dbUser *admin.CloudDatabaseUser) (*TfDatabaseUserModel, diag.Diagnostics) { diff --git a/internal/service/databaseuser/model_database_user_test.go b/internal/service/databaseuser/model_database_user_test.go index d9299384f0..6e206f2b91 100644 --- a/internal/service/databaseuser/model_database_user_test.go +++ b/internal/service/databaseuser/model_database_user_test.go @@ -73,41 +73,69 @@ var ( Labels: &[]admin.ComponentLabel{sdkLabel}, Scopes: &[]admin.UserScope{sdkScope}, } + cloudDatabaseUserWithoutPassword = &admin.CloudDatabaseUser{ + GroupId: projectID, + DatabaseName: authDatabaseName, + Username: username, + X509Type: &x509Type, + OidcAuthType: &oidCAuthType, + LdapAuthType: &ldapAuthType, + AwsIAMType: &awsIAMType, + Roles: &[]admin.DatabaseUserRole{sdkRole}, + Labels: &[]admin.ComponentLabel{sdkLabel}, + Scopes: &[]admin.UserScope{sdkScope}, + } ) func TestNewMongoDBDatabaseUser(t *testing.T) { testCases := []struct { - TfDatabaseUserModel databaseuser.TfDatabaseUserModel + tfDatabaseUserModel databaseuser.TfDatabaseUserModel + passwordStateValue types.String expectedResult *admin.CloudDatabaseUser name string expectedError bool }{ { - name: "Success CloudDatabaseUser", - TfDatabaseUserModel: *getDatabaseUserModel(rolesSet, labelsSet, scopesSet), + name: "CloudDatabaseUser for create", + tfDatabaseUserModel: *getDatabaseUserModel(rolesSet, labelsSet, scopesSet), + passwordStateValue: types.StringNull(), expectedResult: cloudDatabaseUser, expectedError: false, }, + { + name: "CloudDatabaseUser with new password in model as value is modified", + tfDatabaseUserModel: *getDatabaseUserModel(rolesSet, labelsSet, scopesSet), + passwordStateValue: types.StringValue("oldPassword"), + expectedResult: cloudDatabaseUser, + expectedError: false, + }, + { + name: "CloudDatabaseUser with no password in model as value remains the same", + tfDatabaseUserModel: *getDatabaseUserModel(rolesSet, labelsSet, scopesSet), + passwordStateValue: types.StringValue(password), + expectedResult: cloudDatabaseUserWithoutPassword, + expectedError: false, + }, { name: "Roles fail", - TfDatabaseUserModel: *getDatabaseUserModel(wrongRoleSet, labelsSet, scopesSet), + tfDatabaseUserModel: *getDatabaseUserModel(wrongRoleSet, labelsSet, scopesSet), expectedError: true, }, { name: "Labels fail", - TfDatabaseUserModel: *getDatabaseUserModel(rolesSet, wrongLabelSet, scopesSet), + tfDatabaseUserModel: *getDatabaseUserModel(rolesSet, wrongLabelSet, scopesSet), expectedError: true, }, { name: "Scopes fail", - TfDatabaseUserModel: *getDatabaseUserModel(rolesSet, labelsSet, wrongScopeSet), + tfDatabaseUserModel: *getDatabaseUserModel(rolesSet, labelsSet, wrongScopeSet), expectedError: true, }, } for i, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - resultModel, err := databaseuser.NewMongoDBDatabaseUser(context.Background(), &testCases[i].TfDatabaseUserModel) + resultModel, err := databaseuser.NewMongoDBDatabaseUser(context.Background(), tc.passwordStateValue, &testCases[i].tfDatabaseUserModel) if (err != nil) != tc.expectedError { t.Errorf("Case %s: Received unexpected error: %v", tc.name, err) diff --git a/internal/service/databaseuser/resource_database_user.go b/internal/service/databaseuser/resource_database_user.go index 48a6fd8a8c..c59ece4807 100644 --- a/internal/service/databaseuser/resource_database_user.go +++ b/internal/service/databaseuser/resource_database_user.go @@ -212,7 +212,7 @@ func (r *databaseUserRS) Create(ctx context.Context, req resource.CreateRequest, return } - dbUserReq, d := NewMongoDBDatabaseUser(ctx, databaseUserPlan) + dbUserReq, d := NewMongoDBDatabaseUser(ctx, types.StringNull(), databaseUserPlan) resp.Diagnostics.Append(d...) if resp.Diagnostics.HasError() { return @@ -231,6 +231,8 @@ func (r *databaseUserRS) Create(ctx context.Context, req resource.CreateRequest, return } + resp.Diagnostics.AddWarning("If the password value will be managed externally it is advised to remove the attribute", "More details can be found in resource documentation under the 'password' attribute") + resp.Diagnostics.Append(resp.State.Set(ctx, &dbUserModel)...) if resp.Diagnostics.HasError() { return @@ -278,8 +280,6 @@ func (r *databaseUserRS) Read(ctx context.Context, req resource.ReadRequest, res return } - resp.Diagnostics.AddWarning("If the password value will be managed externally, it is advised to remove the attribute", "More details can be found in resource documentation under the 'password' attribute") - resp.Diagnostics.Append(resp.State.Set(ctx, &dbUserModel)...) if resp.Diagnostics.HasError() { return @@ -288,14 +288,15 @@ func (r *databaseUserRS) Read(ctx context.Context, req resource.ReadRequest, res func (r *databaseUserRS) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { var databaseUserPlan *TfDatabaseUserModel + var databaseUserState *TfDatabaseUserModel - diags := req.Plan.Get(ctx, &databaseUserPlan) - resp.Diagnostics.Append(diags...) + resp.Diagnostics.Append(req.Plan.Get(ctx, &databaseUserPlan)...) + resp.Diagnostics.Append(req.State.Get(ctx, &databaseUserState)...) if resp.Diagnostics.HasError() { return } - dbUserReq, d := NewMongoDBDatabaseUser(ctx, databaseUserPlan) + dbUserReq, d := NewMongoDBDatabaseUser(ctx, databaseUserState.Password, databaseUserPlan) resp.Diagnostics.Append(d...) if resp.Diagnostics.HasError() { return diff --git a/website/docs/r/database_user.html.markdown b/website/docs/r/database_user.html.markdown index bd99867f45..238eaf92ed 100644 --- a/website/docs/r/database_user.html.markdown +++ b/website/docs/r/database_user.html.markdown @@ -130,7 +130,7 @@ Accepted values include: * `project_id` - (Required) The unique ID for the project to create the database user. * `roles` - (Required) List of user’s roles and the databases / collections on which the roles apply. A role allows the user to perform particular actions on the specified database. A role on the admin database can include privileges that apply to the other databases as well. See [Roles](#roles) below for more details. * `username` - (Required) Username for authenticating to MongoDB. USER_ARN or ROLE_ARN if `aws_iam_type` is USER or ROLE. -* `password` - (Required) User's initial password. A value is required to create the database user, however the argument may be removed from your Terraform configuration after user creation without impacting the user, password or Terraform management. If you do change management of the password to outside of Terraform it is advised to remove the argument from the Terraform configuration so it is not inadvertently updated to the original password. IMPORTANT --- Passwords may show up in Terraform related logs and it will be stored in the Terraform state file as plain-text. Password can be changed after creation using your preferred method, e.g. via the MongoDB Atlas UI, to ensure security. +* `password` - (Required) User's initial password. A value is required to create the database user, however the argument may be removed from your Terraform configuration after user creation without impacting the user, password or Terraform management. If you do change management of the password to outside of Terraform it is advised to remove the argument from the Terraform configuration. IMPORTANT --- Passwords may show up in Terraform related logs and it will be stored in the Terraform state file as plain-text. Password can be changed after creation using your preferred method, e.g. via the MongoDB Atlas UI, to ensure security. * `x509_type` - (Optional) X.509 method by which the provided username is authenticated. If no value is given, Atlas uses the default value of NONE. The accepted types are: * `NONE` - The user does not use X.509 authentication.