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
241 stars 167 forks source link

Issues with advanced_configuration section on mongodbatlas_cluster #210

Closed michelzanini closed 4 years ago

michelzanini commented 4 years ago

Hi,

There are a few issues with the advanced_configuration section of mongodbatlas_cluster resource.

First, if I create a new cluster with this configuration:

advanced_configuration = {
    minimum_enabled_tls_protocol = "TLS1_2"
}

I would expect all settings to have default values expect the one I have defined. That means when calling /clusters/{name}/processArgs I should get this as a result:

{
    "failIndexKeyTooLong": true,
    "javascriptEnabled": true,
    "minimumEnabledTlsProtocol": "TLS1_2",
    "noTableScan": false,
    "oplogSizeMB": null,
    "sampleRefreshIntervalBIConnector": null,
    "sampleSizeBIConnector": null
}

But this is what I get instead:

{
    "failIndexKeyTooLong": false,
    "javascriptEnabled": false,
    "minimumEnabledTlsProtocol": "TLS1_2",
    "noTableScan": false,
    "oplogSizeMB": null,
    "sampleRefreshIntervalBIConnector": 0,
    "sampleSizeBIConnector": 0
}

The most important part here is that failIndexKeyTooLong and javascriptEnabled are true by default but they are false, even my configuration have not stated that I wanted to change those properties. I believe they should stay with default values. Also you can see that sampleRefreshIntervalBIConnector and sampleSizeBIConnector have values as 0 instead of null.

The next issue is the fact that even if I go and add the other flags such as:

advanced_configuration = {
    fail_index_key_too_long      = true
    javascript_enabled           = true
    minimum_enabled_tls_protocol = "TLS1_2"
}

Every time I run terraform plan the cluster resource always comes back with 1 change required. This is what appears on the change diff:


advanced_configuration       = {
         "fail_index_key_too_long"              = "true"
         "javascript_enabled"                   = "true"
         "minimum_enabled_tls_protocol"         = "TLS1_2"
       - "no_table_scan"                        = "false" -> null
       - "oplog_size_mb"                        = "" -> null
       - "sample_refresh_interval_bi_connector" = "0" -> null
       - "sample_size_bi_connector"             = "0" -> null
}

As you can see the properties that were omitted are always coming back as a difference. I think should not be the case. The only real option here is to have to define ALL variables such as:

advanced_configuration = {
    fail_index_key_too_long              = true
    javascript_enabled                   = true
    minimum_enabled_tls_protocol         = "TLS1_2"
    no_table_scan                        = false
    oplog_size_mb                        = ""
    sample_refresh_interval_bi_connector = "0"
    sample_size_bi_connector             = "0"
  }

But is not what the documentation says... besides, what would happen and a new property is created? The issues with ""/ 0 versus null are also worth looking into fixing...

Thanks.

michelzanini commented 4 years ago

I am using version 0.5.1 of the provider, Terraform version 0.12.16

themantissa commented 4 years ago

@michelzanini thank you for this one as well - confirmed as a bug, it should not modify any other options but does.

Workaround for now, as found, was to not send individual options but the entire block.


For developers, from my own tests:

State file attributes from general cluster creation:
            "advanced_configuration": {
              "fail_index_key_too_long": "true",
              "javascript_enabled": "true",
              "minimum_enabled_tls_protocol": "TLS1_1",
              "no_table_scan": "false",
              "oplog_size_mb": "",
              "sample_refresh_interval_bi_connector": "",
              "sample_size_bi_connector": ""
            },

Add single advanced_config option:
 advanced_configuration = {
    minimum_enabled_tls_protocol = "TLS1_2"
 }

attributes in diff w/ terraform plan

   advanced_configuration       = {
          - "fail_index_key_too_long"              = "true" -> null
          - "javascript_enabled"                   = "true" -> null
          ~ "minimum_enabled_tls_protocol"         = "TLS1_1" -> "TLS1_2"
          - "no_table_scan"                        = "false" -> null
          - "oplog_size_mb"                        = "" -> null
          - "sample_refresh_interval_bi_connector" = "" -> null
          - "sample_size_bi_connector"             = "" -> null
        }

Terraform state file after terraform apply:
           "advanced_configuration": {
              **"fail_index_key_too_long": "false",**
              **"javascript_enabled": "false",**
              "minimum_enabled_tls_protocol": "TLS1_2",
              **"no_table_scan": "false",**
              **"oplog_size_mb": "",**
              **"sample_refresh_interval_bi_connector": "0",**
              **"sample_size_bi_connector": "0"**
themantissa commented 4 years ago

@michelzanini we have an improved PR w/ the fix if you happen to have time to check it out. Thank you so much for all your input on this one as we ensure we get it right!