mongodb / terraform-provider-mongodbatlas

Terraform MongoDB Atlas Provider: Deploy, update, and manage MongoDB Atlas infrastructure as code through HashiCorp Terraform
https://registry.terraform.io/providers/mongodb/mongodbatlas
Mozilla Public License 2.0
244 stars 172 forks source link

accepter_region_name not required for AWS on read/import/update #53

Closed edahlseng closed 4 years ago

edahlseng commented 5 years ago

According the documentation, the accepter_region_name is required for resource mongodbatlas_network_peering. Nevertheless, when we first imported our resources (migrating from the original third-party provider), we didn't encounter an error when we left out accepter_region_name.

Checking mongodbatlas/resource-mongodbatlas_network_container.go, it appears that this constraint is only enforced upon create. While this is a bit of an edge case, it might be worth enforcing this constraint across all operations for consistency.

themantissa commented 5 years ago

@edahlseng thank you. We'll add this as a non-blocking bug. Thank you.

mpaluchowski commented 4 years ago

This is causing problems for us now, because we also originally created mongodbatlas_network_peering resources without the accepter_region_name. Last week we added a new cluster, where we had to add accepter_region_name to the resource. Because we're using a shared module for all our clusters, Terraform now wants to update the previously created network peerings, adding the region name. This operation fails with:

Error: error updating MongoDB Network Peering Connection (<id>): PATCH https://cloud.mongodb.com/api/atlas/v1.0/groups/<id>/peers/<id>: 409 (request "Conflict") The peer with ID <id>in group <id> cannot be updated in its current state.
themantissa commented 4 years ago

Acknowledged and will note in backlog priority.

gmlp commented 4 years ago

Hi @edahlseng, I can't reproduce this issue. can you try to reproduce this issue with the latest provider version, we have made some changes to the code.

themantissa commented 4 years ago

@mpaluchowski and @edahlseng - we are having issues reproducing this. Can you check with the latest version and let us know if you are still hitting it?

mpaluchowski commented 4 years ago

I can check it out next week and let you know.

themantissa commented 4 years ago

Thank you @mpaluchowski - very much appreciate your frequent help and reporting on these!

mpaluchowski commented 4 years ago

@themantissa I just checked and we still have the same issue. For the clusters created early, before accepter_region_name became mandatory, terraform plan still wants to add this value:

  ~ resource "mongodbatlas_network_peering" "default" {
      + accepter_region_name   = "eu-west-3"
      ...
    }

and when I run terraform apply with this plan, I get:

Error: error updating MongoDB Network Peering Connection (<id>): PATCH https://cloud.mongodb.com/api/atlas/v1.0/groups/<id>/peers/<id>: 409 (request "Conflict") The peer with ID <id> in group <id> cannot be updated in its current state.

I have to keep this entry in our peering resource block:

  lifecycle {
    ignore_changes = [accepter_region_name]
  }
themantissa commented 4 years ago

Thank you @mpaluchowski !

PacoDw commented 4 years ago

Hello @mpaluchowski and @edahlseng ! Thank you so much for your review, I made changes on the same branch fixing the bug and added a test case to validate it, so could you test it to check if everything is okay plz?

Let me know if you have another concern or comment 👍

themantissa commented 4 years ago

@mpaluchowski or @edahlseng any feedback on the proposed fix? Looks good from our side but before releasing would be great to have a verification.

mpaluchowski commented 4 years ago

Hmm, how do I test an unreleased version? Do I need to build it myself first?

themantissa commented 4 years ago

Correct @mpaluchowski , you'd need to build from the branch the fix is in and ensure you have the correct golang version (for that it should be either 1.13 or 1.14 - I believe 1.14 will be best but @PacoDw can confirm - we are about to update the README w/ the next release). It's a bit of work but helps us verify it fixes the specific issue. If not we can release the assumed fix in 0.6.2 and if it does not correct it then try to schedule in time to re-address.

PacoDw commented 4 years ago

Hello, @mpaluchowski you need to switch to the branch fixes-53-accepter_region_name-not-required-for-aws and make a git pull to get all the recent changes, and ensure that you are using the go version > 1.13 if you have v1.14 it's excellent, make a go build later move the binary to your project folder, make terraform init and run your Terraform configuration.

mpaluchowski commented 4 years ago

Sorry I didn't make it. Too much to do lately and I currently don't even have Go installed. I'll wait for you to release the next version and then see if the error is gone.