-
Notifications
You must be signed in to change notification settings - Fork 208
feat: Adds new mongodbatlas_cloud_user_team_assignment
resource
#3502
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
"github.com/mongodb/terraform-provider-mongodbatlas/internal/testutil/mig" | ||
) | ||
|
||
func TestMigCloudUserTeamAssignmentRS_basic(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we add test fro migration journeys to this resource?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we do not have a clear migration journey at the moment we will do it in a separate pr
…e_assignments in `organization` and `team` DS for consistency in naming.
Only Alert Config tests are failing (same as in master) |
APIx bot: a message has been sent to Docs Slack channel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new mongodbatlas_cloud_user_team_assignment
resource to the MongoDB Atlas Terraform provider, enabling management of user-to-team assignments within an organization. The implementation also includes a field name correction for project role assignments across multiple existing resources.
- Implements the new cloud user team assignment resource with complete CRUD operations and import functionality
- Corrects field naming from
project_roles_assignments
toproject_role_assignments
across existing data sources for consistency - Adds comprehensive test coverage including unit tests, acceptance tests, and migration tests
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
internal/service/clouduserteamassignment/* | Complete implementation of the new resource including schema, model conversion, CRUD operations, and tests |
internal/service/team/data_source_team_test.go | Updates test to use corrected field name project_role_assignments |
internal/service/organization/resource_organization_test.go | Updates tests to use corrected field name project_role_assignments |
internal/common/dsschema/users_schema.go | Corrects schema field name from project_roles_assignments to project_role_assignments |
internal/common/conversion/flatten_expand.go | Updates flattening logic to use corrected field name |
internal/provider/provider.go | Registers the new resource with the provider |
.github/workflows/acceptance-tests-runner.yml | Adds CI configuration for the new resource tests |
.changelog/3502.txt | Documents the new resource addition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
MarkdownDescription: "Unique 24-hexadecimal digit string that identifies the MongoDB Cloud user.", | ||
}, | ||
"username": schema.StringAttribute{ | ||
Computed: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a bit surprised we are requiring user_id
and not username
.
I think it is easier for a customer to use username
, but maybe I missed the discussion in the technical design?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API requires user_id
attribute. If we put usernames
as computed instead of user_id
, we would have to implement extra logic to retrieve user_id
in order to call the API.
In short, the reason is that we wanted to be aligned with the API.
Additionally, we do support import with username
.
Further discussion will be done in today's sync
}, | ||
}, | ||
}, | ||
"team_ids": schema.SetAttribute{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we including this? Is there no other way for the user to get this info?
Computed: true, | ||
MarkdownDescription: "String enum that indicates whether the MongoDB Cloud user has a pending invitation to join the organization or they are already active in the organization.", | ||
}, | ||
"roles": schema.SingleNestedAttribute{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as about the team_ids
.
I think the roles for the team would make sense but not all roles?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, the reason behind this is that we want to be aligned with what the API returns.
The same for team_ids
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did find roles
and team_ids
strange within this resource as well, but I see why we have added them. Are both of the attributes also present in cloud_user_org_assignment
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, leaving some improvement suggestions
map[string]attr.Value{ | ||
"org_roles": orgRoles, | ||
"project_role_assignments": projectRoleAssignmentsSet, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
q: Any reason why we don't use the typed model TFRolesModel?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just an oversight on my part. I'll update it to use TFRolesModel
func (r *rs) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can provide an error to give users more transparency and guidance, e.g. https://github.com/mongodb/terraform-provider-mongodbatlas/blob/master/internal/service/streamprivatelinkendpoint/resource.go#L133
…ignment` (#3517) * Added DS schema generation and Read operation * Add to provider * Fix * Added tests * Changelog * Modified tests * Removed redundant condition * Removed IsUserID --------- Co-authored-by: Cristina Sánchez Sánchez <[email protected]>
Description
Adds new
mongodbatlas_cloud_user_team_assignment
resource.Additionally changed the attribute name
project_roles_assignments
to project_role_assignments inorganization
andteam
DS for consistency in naming.Follow-up PRs:
Datasource
Examples & documentation
Link to any related issue(s): CLOUDP-329984
Type of change:
Required Checklist:
Further comments