fix: corrects description for atlas_cidr_block parameter #216
Conversation
Corrects description for atlas_cidr_block parameter in both Argument and Attributes reference sections
ghost
added
|
@lmego thank you for the submission. Can you check out our https://github.com/terraform-providers/terraform-provider-mongodbatlas/blob/master/CONTRIBUTING.md file and ensure you've checked any applicable items, especially the contributor agreement? Thanks! |
|
@themantissa thank you for the link. I checked the document and signed the contributor agreement. Please do let me know if there is anything else I am missing. |
| @@ -179,7 +179,7 @@ resource "mongodbatlas_cluster" "test" { | |||
| * `aws_account_id` - (Optional | **AWS Required**) Account ID of the owner of the peer VPC. | |||
| * `route_table_cidr_block` - (Optional | **AWS Required**) Peer VPC CIDR block or subnet. | |||
| * `vpc_id` - (Optional | **AWS Required**) Unique identifier of the peer VPC. | |||
| * `atlas_cidr_block` - (Optional | **AZURE Required**) Unique identifier for an Azure AD directory. | |||
| * `atlas_cidr_block` - (Required) CIDR block that Atlas uses for your clusters. Atlas uses the specified CIDR block for all other Network Peering connections created in the project. The Atlas CIDR block must be at least a /24 and at most a /21 in one of the following [private networks](https://tools.ietf.org/html/rfc1918.html#section-3). | |||
There was a problem hiding this comment.
Ah, I see what's going on here and glad you pointed this out. So the only place this should be used is for Azure - but I think even that may be a mistake (note the example for Azure here has it but not the returned value or the parameter list - https://docs.atlas.mongodb.com/reference/api/vpc-create-peering-connection/). So I'll check into that. Basically, this isn't described correctly but it may not even be needed.
It also shouldn't be in the GCP example above, from what I can see in the tests - https://github.com/terraform-providers/terraform-provider-mongodbatlas/blob/master/mongodbatlas/resource_mongodbatlas_network_peering_test.go. I'll fix that, so glad you brought up this issue.
There was a problem hiding this comment.
There was a problem hiding this comment.
@themantissa you are indeed correct. It seems that there might be something off with mongo atlas api documentation or with the API itself. I noticed that when I try to initiate the peering directly through the atlas web console, we can input the "Atlas VPC CIDR" for all providers (i.e. AWS, GCP, Azure) unless there is an active cluster already. I am attaching some pictures here.
Granted, through the same console, I have modified that Atlas CIDR block before having a active cluster, and then proceeded to initiate a peering connection. However, the resulting Atlas_VPC_CIDR block always ends up with the same initial default value.
I'm not sure how exactly to proceed with this issue tho. Maybe I should contact the Mongo Atlas API team so they can clarify if the variable itself is or should ever be used.? Then once we are clear on that, we can review if there is an issue on the terraform provider itself and then the documentation?. What are your thoughts?
There was a problem hiding this comment.
This project is support MongoDB and is who I work for so I've already put in a ticket to the team to find out if the documentation is wrong. Just waiting for clarification that my assumptions here are correct. But thank you for being willing to help!
To your 2nd point - the API and UI differ enough that I would caution against using one to understand the other. Containers must be created before peering can happen so if you want to setup peering via API (which is what Terraform uses) before a cluster, a container must be created first. If a cluster is created and then peering setup after via API the container is created automatically when the cluster is (and we are working on an improvement to make this clearer right now). It sorta is reflected in the UI you share above, the reason you can't modify the CIDR block if a cluster is already created is because the container is already created. And a container is basically each cloud providers idea of a VPC.
So the short of it is - this variable only applies when one is creating a container (which is not what the peering doc is about, but we include it in the example to help) unless the doc is missing something. If you'd like we can close this PR and once I have the information to verify this isn't needed then I'll correct it. Thank you for pointing it out though.
There was a problem hiding this comment.
@themantissa great. Thanks for reaching out to the team directly and for the clarification on my UI/API similarities assumption.
I agree with your suggestion. We can close this PR and fix after you validate that the variable isn't needed. I would definitely appreciate it if you can keep me in the loop.
There was a problem hiding this comment.
Sounds great and I'll let you know the outcome in the issue you filed!
Corrects description for atlas_cidr_block parameter in both Argument and Attributes reference sections. Pertains to issue #215