Provider Review

[tracypholmes]

Hello! My name is Tracy Holmes and I'm from the Terraform Ecosystem Team. @mbfrahry and I have been reviewing your provider. We've seen some great stuff around your provider, and we're excited to bring it into our ecosystem. There is feedback listed below in order of importance. Use this issue as a "checklist", and if you see a specific issue on one of the resources, please take that feedback and apply it to all resources that show that same issue.

• resource_mongodbatlas_ip_whitelist: In Update: resourceIPWhitelistUpdate, you are just calling Create method. So, you should be able to use Update: resourceIPWhitelistCreate instead.
• resource_mongodbatlas_ip_whitelist: get rid of lines 64 - 67 as you are contradicting your schema. You are already taking care of this through ConflictsWith.
• The id needs to be the amalgamation of every attribute needed to retrieve the resource. Then when you call Read or Update, you'll parse that id and use those values to read or delete the resource. Values tend to be retrieved from the config file over the statefile. This is especially true when you are doing a d.Get and values may have been changed in the configuration file between runs that would affect how a resource is retrieved/modified. This is prevalent throughout.
• On line 22, you had to do this because your Read function is hinky. Instead, you can do this State: schema.ImportStatePassthrough,
• You need to return the Read function instead of nil in the Create function
• You can get rid of this line as you can simply return nil.
• data_source_mongodbatlas_project_test: The name you are passing to the test needs to be randomized. While testing Terraform, if the name isn't randomized and it continues to exist, then it will always fail.
• We should group the Required attributes together at the top of the resource or documentation first, then follow with the Optional attributes. This is so that a user knows exactly what attributes to pass to get up and running without having to look through all the documentation.
• Anything related to status or state can be removed. TF only has ready and waiting so these attrs are not needed. Here's an example.
• Any type of ForceNew: false can be deleted as it is implied. Here is an example.
• Dates: can be removed. Dates aren't needed inside Terraform as it's not necessary.
• Argument reference: the note regarding changing name can be removed as it is understood to be a part a Terraform.
• In the cluster documentation for argument reference, the paused attribute can be removed as Terraform is not the right tool to manage states.
• Groups and projects: it's stated in multiple places these are synonymous terms. Is there any reason these can't be combined or condensed into one term?
• In data_source_mongodbatlas_container identifier & id appear to be the same thing. Could you select one or the other for consistency and ease of use? If these are both needed, could you provide clarification?
• Could you clarify what this value is?
• What is main.tf for? Would it be better served in the examples folder?
• mongodbatlas_cluster: Is there a way to clean this up or combine the multiple notes? For example, the groups and projects note could be combined with the group argument.
• Import Resources: You can't import resources by default which makes this statement unneeded. Was there a specific reason you put this message in? If not could you please remove it?
• This validation should line up with what you state in your documentation.
• All resources need to have tests. For example, vpc_peering_connection does not have a corresponding test.
• It appears this resource has no corresponding documentation. Could you add that?
• This NOTE should be removed from the README as it is already noted in the mongodbatlas_container resource. The same goes for Importing Resources.
• Please remove the symlink portion under Building the Provider as it is not needed. Now, when you download the provider, it does it for you.
• CHANGELOG should have a version included. Please make sure to add 1.0.0 to the file before release.
• In your Scripts folder, please remove errcheck.sh and gotest.sh, and add websitefmtcheck.sh similar to this:
• Please use this [travis.yml file](https://github.com/terraform-providers/terraform-provider-aws/blob/master/.travis.yml) as an example to keep in line with our build processes.

[akshaykarle]

Thanks @tracypholmes and @mbfrahry for all feedback. Will start working on this soon.

[cgriggs01]

Hey team,

I just wanted to check in on the status of this provider. We have many Terraform users that are looking forward to having this released under the native Terraform ecosystem.

Best,
Chris

[hasheddan]

@akshaykarle I would be happy to provide some assistance here!