hashicorp / terraform-provider-consul

Terraform Consul provider
https://www.terraform.io/docs/providers/consul/
Mozilla Public License 2.0
125 stars 112 forks source link

service-defaults LocalRequestTimeoutMs is shown in plan but not saved in state #327

Closed chrisjohnson closed 1 year ago

chrisjohnson commented 1 year ago

Terraform Version

terraform 0.14.11

Affected Resource(s)

Terraform Configuration Files

locals {
  # For each app listed, extend timeouts for requests from the default of 15s to 120s
  #    sidecar -> sidecar
  #    sidecar -> destination app
  # TODO: Currently, due to a bug in consul or the terraform provider itself,
  #       we cannot configure LocalRequestTimeoutMs on service-defaults for the
  #       applications listed here. A puppet PR is also necessary to configure
  #       local_request_timeout_ms in your app's app::consul_service
  longer-timeout-apps = [
    "service1",
    "service2",
  ]
  needs-defaults = distinct(concat(local.longer-timeout-apps))
  app-defaults = flatten([
    for namespace in local.namespaces : [
      for service in local.needs-defaults : {
        namespace = namespace
        service   = service
        config = {
          # Provide empty objects to prevent terraform from continually showing changes
          "Expose"                = {},
          "LocalRequestTimeoutMs" = contains(local.longer-timeout-apps, service) ? 120000 : null,
          "MeshGateway"           = {},
          "Protocol"              = "http",
          "TransparentProxy"      = {},
        }
      }
    ]
  ])
}

resource "consul_config_entry" "service_defaults" {
  for_each = {
    for app in local.app-defaults : "${app.namespace}-${app.service}" => app
  }
  kind        = "service-defaults"
  name        = each.value.service
  config_json = jsonencode(each.value.config)
  namespace   = each.value.namespace
}

And a screenshot of a planned resource to show that the terraform code computes as expected:

image

Expected Behavior

The produced service-default should have a LocalRequestTimeoutMs

Actual Behavior

The service-default does not reflect it:

$ consul config read -kind service-defaults  -namespace testing-spc2 -name rxcallback
{
    "Kind": "service-defaults",
    "Name": "rxcallback",
    "Partition": "default",
    "Namespace": "testing-spc2",
    "Protocol": "http",
    "TransparentProxy": {},
    "MeshGateway": {},
    "Expose": {},
    "CreateIndex": 8370875,
    "ModifyIndex": 8370875
}

The terraform state also reflects this (snippet from overall terraform state):

        {
          "index_key": "testing-spc2-rxcallback",
          "schema_version": 0,
          "attributes": {
            "config_json": "{\"Expose\":{},\"MeshGateway\":{},\"Protocol\":\"http\",\"TransparentProxy\":{}}",
            "id": "service-defaults-rxcallback",
            "kind": "service-defaults",
            "name": "rxcallback",
            "namespace": "testing-spc2",
            "partition": null
          },
          "sensitive_attributes": [],
          "private": "[redacted]",
          "dependencies": [
            "module.cluster.data.http.namespaces"
          ]
        }

Steps to Reproduce

Create a service-defaults using terraform code above

Important Factoids

Using consul config write I'm able to make the same service-defaults and consul preserves the LocalRequestTimeoutMs

$ cat rxindicator-defaults.json
{
    "Kind": "service-defaults",
    "Name": "rxindicator",
    "Namespace": "testing-inf2",
    "Protocol": "http",
    "LocalRequestTimeoutMs": 120000
}

$ consul config write rxindicator-defaults.json
Config entry written: service-defaults/rxindicator

$ consul config read -kind service-defaults  -namespace testing-inf2 -name rxindicator
{
    "Kind": "service-defaults",
    "Name": "rxindicator",
    "Partition": "default",
    "Namespace": "testing-inf2",
    "Protocol": "http",
    "TransparentProxy": {},
    "MeshGateway": {},
    "Expose": {},
    "LocalRequestTimeoutMs": 120000,
    "CreateIndex": 8468166,
    "ModifyIndex": 8468166
}
remilapeyre commented 1 year ago

Hi @chrisjohnson, the new fields in the consul_config_entry should all be available in the release 2.17 of the provider that will arrive this week.

chrisjohnson commented 1 year ago

Excellent! If I could suggest a feature then, for future releases to throw an error on unexpected fields, rather than silently producing a plan that includes the field and then discarding it. It was pretty difficult to track this issue down to being the provider rather than my config or consul itself.

remilapeyre commented 1 year ago

Hi @chrisjohnson, the LocalRequestTimeoutMs argument is now supported by the consul_config_entry resource so I will close this issue.

Please let me know if there is other attribute that you need support for :) !