fix: Avoids sending database user password in update request if the v…

…alue has not changed (#2005) * include warning during creation * fix: Avoids sending database user password in update request if the value has not changed * add unit test for case when password attribute is removed and should not be sent in patch request * include warning message
mongodb · Mar 12, 2024 · 29d4268 · 29d4268
1 parent b1e90e5
commit 29d4268
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 19 deletions.
diff --git 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
@@ -73,41 +73,76 @@ 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, types.StringValue(password)),
+			passwordStateValue:  types.StringNull(),
 			expectedResult:      cloudDatabaseUser,
 			expectedError:       false,
 		},
+		{
+			name:                "CloudDatabaseUser with new password in model when password value is modified",
+			tfDatabaseUserModel: *getDatabaseUserModel(rolesSet, labelsSet, scopesSet, types.StringValue(password)),
+			passwordStateValue:  types.StringValue("oldPassword"),
+			expectedResult:      cloudDatabaseUser,
+			expectedError:       false,
+		},
+		{
+			name:                "CloudDatabaseUser with no password in model when password value remains the same",
+			tfDatabaseUserModel: *getDatabaseUserModel(rolesSet, labelsSet, scopesSet, types.StringValue(password)),
+			passwordStateValue:  types.StringValue(password),
+			expectedResult:      cloudDatabaseUserWithoutPassword,
+			expectedError:       false,
+		},
+		{
+			name:                "CloudDatabaseUser with no password in model when password value is removed",
+			tfDatabaseUserModel: *getDatabaseUserModel(rolesSet, labelsSet, scopesSet, types.StringNull()),
+			passwordStateValue:  types.StringValue(password),
+			expectedResult:      cloudDatabaseUserWithoutPassword,
+			expectedError:       false,
+		},
 		{
 			name:                "Roles fail",
-			TfDatabaseUserModel: *getDatabaseUserModel(wrongRoleSet, labelsSet, scopesSet),
+			tfDatabaseUserModel: *getDatabaseUserModel(wrongRoleSet, labelsSet, scopesSet, types.StringValue(password)),
 			expectedError:       true,
 		},
 		{
 			name:                "Labels fail",
-			TfDatabaseUserModel: *getDatabaseUserModel(rolesSet, wrongLabelSet, scopesSet),
+			tfDatabaseUserModel: *getDatabaseUserModel(rolesSet, wrongLabelSet, scopesSet, types.StringValue(password)),
 			expectedError:       true,
 		},
 		{
 			name:                "Scopes fail",
-			TfDatabaseUserModel: *getDatabaseUserModel(rolesSet, labelsSet, wrongScopeSet),
+			tfDatabaseUserModel: *getDatabaseUserModel(rolesSet, labelsSet, wrongScopeSet, types.StringValue(password)),
 			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)
@@ -129,7 +164,7 @@ func TestNewTfDatabaseUserModel(t *testing.T) {
 			name:            "Success TfDatabaseUserModel",
 			sdkDatabaseUser: cloudDatabaseUser,
 			currentModel:    databaseuser.TfDatabaseUserModel{Password: types.StringValue(password)},
-			expectedResult:  getDatabaseUserModel(rolesSet, labelsSet, scopesSet),
+			expectedResult:  getDatabaseUserModel(rolesSet, labelsSet, scopesSet, types.StringValue(password)),
 			expectedError:   false,
 		},
 	}
@@ -308,7 +343,7 @@ func TestNewTFRolesModel(t *testing.T) {
 	}
 }
 
-func getDatabaseUserModel(roles, labels, scopes basetypes.SetValue) *databaseuser.TfDatabaseUserModel {
+func getDatabaseUserModel(roles, labels, scopes basetypes.SetValue, password types.String) *databaseuser.TfDatabaseUserModel {
 	encodedID := conversion.EncodeStateID(map[string]string{
 		"project_id":         projectID,
 		"username":           username,
@@ -319,7 +354,7 @@ func getDatabaseUserModel(roles, labels, scopes basetypes.SetValue) *databaseuse
 		ProjectID:        types.StringValue(projectID),
 		AuthDatabaseName: types.StringValue(authDatabaseName),
 		Username:         types.StringValue(username),
-		Password:         types.StringValue(password),
+		Password:         password,
 		X509Type:         types.StringValue(x509Type),
 		OIDCAuthType:     types.StringValue(oidCAuthType),
 		LDAPAuthType:     types.StringValue(ldapAuthType),

diff --git 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
@@ -286,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
@@ -14,6 +14,8 @@ Each user has a set of roles that provide access to the project’s databases. U
 
 -> **NOTE:** Groups and projects are synonymous terms. You may find group_id in the official documentation.
 
+~> **WARNING:** The password argument is required for creation but should be removed after creation if it will be managed externally. More details can be found in the password argument documentation.
+
 ~> **IMPORTANT:** All arguments including the password will be stored in the raw state as plain-text. [Read more about sensitive data in state.](https://www.terraform.io/docs/state/sensitive-data.html)
 
 ## Example Usages
@@ -130,7 +132,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 but may be removed from your Terraform configuration after user creation without impacting the user, password or Terraform management. 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.  If you do change management of the password to outside of Terraform be sure to remove the argument from the Terraform configuration so it is not inadvertently updated to the original password.
+* `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.