azurerm_mongo_cluster - support for new properties customer_managed_key, data_api_mode_enabled, identity, restore, auth_config_allowed_modes and storage_type#31100
Conversation
teowa
left a comment
There was a problem hiding this comment.
Hi @neil-yechenwei , thank you for the PR. I have added some comments.
| if metadata.ResourceData.HasChange("storage_size_in_gb") { | ||
| payload.Properties.Storage = &mongoclusters.StorageProperties{ | ||
| SizeGb: pointer.To(state.StorageSizeInGb), | ||
| Type: pointer.To(mongoclusters.StorageType(state.StorageType)), |
There was a problem hiding this comment.
pointer.ToEnum is preferred when working with enums.
| Type: pointer.To(mongoclusters.StorageType(state.StorageType)), | |
| Type: pointer.ToEnum[mongoclusters.StorageType](state.StorageType), |
|
|
||
| if v := props.Storage; v != nil { | ||
| state.StorageSizeInGb = pointer.From(v.SizeGb) | ||
| state.StorageType = string(pointer.From(v.Type)) |
There was a problem hiding this comment.
| state.StorageType = string(pointer.From(v.Type)) | |
| state.StorageType = pointer.FromEnum(v.Type) |
| point_in_time_utc = timeadd(timestamp(), "-1s") | ||
| } | ||
|
|
||
| // As "point_in_time_utc" is retrieved dynamically, so it would cause diff when tf plan. So we have to ignore it here. |
There was a problem hiding this comment.
This can be solved with time_static resource, here is an example
| @@ -99,10 +113,36 @@ The following arguments are supported: | |||
|
|
|||
| * `storage_size_in_gb` - (Optional) The size of the data disk space for the MongoDB Cluster. | |||
|
|
|||
| * `storage_type` - (Optional) The type of the storage type for the MongoDB Cluster. Possible values are `PremiumSSD` and `PremiumSSDv2`. Defaults to `PremiumSSD`. Changing this forces a new resource to be created. | |||
There was a problem hiding this comment.
| * `storage_type` - (Optional) The type of the storage type for the MongoDB Cluster. Possible values are `PremiumSSD` and `PremiumSSDv2`. Defaults to `PremiumSSD`. Changing this forces a new resource to be created. | |
| * `storage_type` - (Optional) The storage type for the MongoDB Cluster. Possible values are `PremiumSSD` and `PremiumSSDv2`. Defaults to `PremiumSSD`. Changing this forces a new resource to be created. |
|
Hi @neil-yechenwei , LGTM! |
WodansSon
left a comment
There was a problem hiding this comment.
Hi @neil-yechenwei, thanks for the PR! I reviewed the code and found a number of things to address - documented everything below.
Status: Changes Requested
Next Steps:
- Please go through my comments and make those updates
- Push your changes when ready
- Reply here for re-review
Thanks! Reach out if you have any questions. 🚀
| // Update it separately if user wants it enabled | ||
| if state.CreateMode == string(mongoclusters.CreateModeDefault) { | ||
| parameter.Properties.DataApi = &mongoclusters.DataApiProperties{ | ||
| Mode: pointer.To(mongoclusters.DataApiModeDisabled), |
There was a problem hiding this comment.
Missing use of pointer.ToEnum for enum values.
| Mode: pointer.To(mongoclusters.DataApiModeDisabled), | |
| Mode: pointer.ToEnum[mongoclusters.DataApiMode](mongoclusters.DataApiModeDisabled), |
There was a problem hiding this comment.
mongoclusters.DataApiModeDisabled is not string. So we aren't able to use pointer.ToEnum.
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
We should add consistent error handling per the contributor guidelines:
| if err != nil { | |
| return err | |
| } | |
| if err != nil { | |
| fmt.Errorf("flattening identity: %+v", err) | |
| } |
Reference: reference-errors
|
|
||
| if v := props.Storage; v != nil { | ||
| state.StorageSizeInGb = pointer.From(v.SizeGb) | ||
| state.StorageType = pointer.FromEnum(v.Type) |
There was a problem hiding this comment.
Can we update the usage of pointer.ToEnum for enum values here:
| state.StorageType = pointer.FromEnum(v.Type) | |
| state.StorageType = pointer.ToEnum[mongoclusters.StorageType](v.Type) |
| return &identity.LegacySystemAndUserAssignedMap{ | ||
| Type: identityObj.Type, | ||
| IdentityIds: identityIds, | ||
| } |
There was a problem hiding this comment.
Missing usage of pointer.ToEnum for enum values.
| return &identity.LegacySystemAndUserAssignedMap{ | |
| Type: identityObj.Type, | |
| IdentityIds: identityIds, | |
| } | |
| return &identity.LegacySystemAndUserAssignedMap{ | |
| Type: pointer.ToEnum[identity.Type](identityObj.Type), | |
| IdentityIds: identityIds, | |
| } |
| * `administrator_username` - (Optional) The administrator username of the MongoDB Cluster. Changing this forces a new resource to be created. | ||
|
|
||
| * `create_mode` - (Optional) The creation mode for the MongoDB Cluster. Possibles values are `Default` and `GeoReplica`. Defaults to `Default`. Changing this forces a new resource to be created. | ||
| * `create_mode` - (Optional) The creation mode for the MongoDB Cluster. Possibles values are `Default`, `GeoReplica` and `PointInTimeRestore`. Defaults to `Default`. Changing this forces a new resource to be created. |
There was a problem hiding this comment.
Typo in documentation: Possibles values should be Possible values. This should also be corrected for the high_availability_mode, public_network_access and version fields as well, while I know that wasn't your contribution since we are in the code we should fix those up too.
| * `create_mode` - (Optional) The creation mode for the MongoDB Cluster. Possibles values are `Default`, `GeoReplica` and `PointInTimeRestore`. Defaults to `Default`. Changing this forces a new resource to be created. | |
| * `create_mode` - (Optional) The creation mode for the MongoDB Cluster. Possible values are `Default`, `GeoReplica` and `PointInTimeRestore`. Defaults to `Default`. Changing this forces a new resource to be created. |
|
|
||
| * `type` - (Required) The type of managed identity to assign. Possible value is `UserAssigned`. | ||
|
|
||
| * `identity_ids` - (Optional) - A list of one or more Resource IDs for User Assigned Managed identities to assign. Required when `type` is set to `UserAssigned`. |
There was a problem hiding this comment.
| * `identity_ids` - (Optional) - A list of one or more Resource IDs for User Assigned Managed identities to assign. Required when `type` is set to `UserAssigned`. | |
| * `identity_ids` - (Optional) - A list of one or more Resource IDs for User Assigned Managed identities to assign. | |
| -> **Note:** Required when `type` is set to `UserAssigned`. | |
| ~> **Note:** Modifying the `identity_ids` field will trigger a resource recreation. |
WodansSon
left a comment
There was a problem hiding this comment.
This is a pre-existing issue that wasn't your fault, but since we are touching this code we should fix up the Arguments Reference order as well. I don't know why this didn't show up in my previous review, sorry.
Status: Changes Requested
Next Steps:
- Please go through my comments and make those updates
- Push your changes when ready
- Reply here for re-review
Thanks! Reach out if you have any questions.
| * `tags` - (Optional) A mapping of tags to assign to the MongoDB Cluster. | ||
|
|
||
| * `version` - (Optional) The version for the MongoDB Cluster. Possibles values are `5.0`, `6.0`, `7.0` and `8.0`. |
There was a problem hiding this comment.
The tags field should always appear at the bottom of the Arguments Reference:
| * `tags` - (Optional) A mapping of tags to assign to the MongoDB Cluster. | |
| * `version` - (Optional) The version for the MongoDB Cluster. Possibles values are `5.0`, `6.0`, `7.0` and `8.0`. | |
| * `version` - (Optional) The version for the MongoDB Cluster. Possible values are `5.0`, `6.0`, `7.0` and `8.0`. | |
| * `tags` - (Optional) A mapping of tags to assign to the MongoDB Cluster. |
| Optional: true, | ||
| Computed: true, |
There was a problem hiding this comment.
Missing required O+C comment between Optional and Computed properties. Add a comment explaining why this property is Optional+Computed.
Reference: Best Practices
|
@WodansSon , thanks for the comments. I updated PR. Please take another look. Below is the latest test result. |
WodansSon
left a comment
There was a problem hiding this comment.
@neil-yechenwei, thank you for pushing those changes, this LGTM now. I have kicked off the TC tests, if those pass, I think this is good to merge. Thank you for all of your hard work getting this fixed up and ready to go! 🚀
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |



Community Note
Description
Service team reached out to us to support new features in the new API version
2025-09-01for Mongo Cluster.PR Checklist
For example: “
resource_name_here- description of change e.g. adding propertynew_property_name_here”Changes to existing Resource / Data Source
Testing
--- PASS: TestAccMongoClusterFreeTier/freeTier/basic (842.46s)
--- PASS: TestAccMongoClusterFreeTier/freeTier/update (2300.17s)
--- PASS: TestAccMongoClusterFreeTier/freeTier/import (780.58s)
--- PASS: TestAccMongoCluster_previewFeature (2518.00s)
--- PASS: TestAccMongoCluster_geoReplica (2393.81s)
--- PASS: TestAccMongoCluster_pointInTimeRestore (2570.64s)
--- PASS: TestAccMongoCluster_customerManagedKey (1055.90s)
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_mongo_cluster- support for new propertiescustomer_managed_key,data_api_mode_enabled,identity,restore,auth_config_allowed_modesandstorage_typeThis is a (please select all that apply):
Related Issue(s)
Fixes #30255
AI Assistance Disclosure
Rollback Plan
If a change needs to be reverted, we will publish an updated version of the provider.
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
Note
If this PR changes meaningfully during the course of review please update the title and description as required.