digitalocean / terraform-provider-digitalocean

Terraform DigitalOcean provider
https://registry.terraform.io/providers/digitalocean/digitalocean/latest/docs
Mozilla Public License 2.0
510 stars 278 forks source link

digitalocean_database_opensearch_config touches unnecessary options on change #1272

Closed brianhelba closed 3 days ago

brianhelba commented 5 days ago

Bug Report

Describe the bug

When performing change actions, digitalocean_database_opensearch_config sends HTTP requests with many unnecessary options.

This is especially impactful, since sending the particular option override_main_response_version triggers an unrelated upstream bug within DigitalOcean OpenSearch, whereby the database version is misreported, which disrupts usage of the related digitalocean_database_cluster.

Affected Resource(s)

Expected Behavior

For the change from the resource:

resource "digitalocean_database_opensearch_config" "elasticsearch" {
  cluster_id = digitalocean_database_cluster.elasticsearch.id
}

to

resource "digitalocean_database_opensearch_config" "elasticsearch" {
  cluster_id = digitalocean_database_cluster.elasticsearch.id

  action_auto_create_index_enabled = false
}

I would expect an HTTP request like:

PATCH /v2/databases/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx/config HTTP/1.1
Host: api.digitalocean.com
User-Agent: Terraform/1.9.8 godo/1.129.0

{
 "config": {
  "action_auto_create_index_enabled": false
 }
}

Actual Behavior

Instead, in response to this change, terraform apply makes an HTTP request like:

PATCH /v2/databases/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx/config HTTP/1.1
Host: api.digitalocean.com
User-Agent: Terraform/1.9.8 godo/1.129.0

{
 "config": {
  "http_max_content_length_bytes": 100000000,
  "http_max_header_size_bytes": 8192,
  "http_max_initial_line_length_bytes": 4096,
  "indices_query_bool_max_clause_count": 1024,
  "indices_memory_index_buffer_size_percentage": 10,
  "indices_memory_min_index_buffer_size_mb": 48,
  "indices_queries_cache_size_percentage": 10,
  "indices_recovery_max_mb_per_sec": 40,
  "indices_recovery_max_concurrent_file_chunks": 2,
  "ism_enabled": true,
  "ism_history_enabled": true,
  "ism_history_max_age_hours": 24,
  "ism_history_max_docs": 2500000,
  "ism_history_rollover_check_period_hours": 8,
  "ism_history_rollover_retention_period_days": 30,
  "search_max_buckets": 10000,
  "action_auto_create_index_enabled": false,
  "enable_security_audit": false,
  "action_destructive_requires_name": false,
  "override_main_response_version": false,
  "script_max_compilations_rate": "use-context",
  "cluster_routing_allocation_node_concurrent_recoveries": 2,
  "plugins_alerting_filter_by_backend_roles_enabled": false
 }
}

Terraform version

Terraform v1.9.8

Additional context

Note that all of the additional keys sent have the default value (except the changed action_auto_create_index_enabled in my example). Normally, this should be harmless but unnecessary extra data.

However, DigitialOcean ElasticSearch currently has a behavior (which seems like a bug) whereby setting the config option "override_main_response_version": false (which is already the default value!) causes the reported version of a v2 database to be "1". For a resource managed with digitalocean_database_cluster, this will forever cause Terraform to attempt (unsuccessfully) to change the version from 1 to 2, causing all future plans to show required changes for the digitalocean_database_cluster.

If only a minimal set of changes is actually passed to the HTTP PATCH request, users can better workaround this upstream bug by avoiding explicit usage of the override_main_response_version option until the behavior is fixed upstream.


Separately, note that any key with a value of 0 is omitted from the request. Maybe this is an extension of the bug with false values addressed in #1268? This seems like it's an additional bug, but it's not the focus of my bug report here.

References

brianhelba commented 5 days ago

Ping @andrewsomething.

andrewsomething commented 3 days ago

@brianhelba I've raised the issue regarding override_main_response_version internally. In the meantime, I've just cut a new release that should workaround the issue by not sending the unnecessary options in the request.

Thanks for all the feedback!

brianhelba commented 3 days ago

@andrewsomething Thanks again!

I've confirmed that #1273 fixes this bug for me.