consul_node resource doesn't inherit datacenter attribute from provider definition when defined

[splashx]

Terraform Version

Refer to #56

Affected Resource(s)

Refer to #56

Terraform Configuration Files

Refer to #56

Expected Behavior

Refer to #56

Actual Behavior

Refer to #56

Steps to Reproduce

Refer to #56

Important Factoids

Refer to #56

References

Refer to #56

[remilapeyre]

Hi @splashx , thanks for reporting the bug.

In #56 you say that Permission Denied is expected and you are trying to register an external service.
Do I understand correctly that:

• you want to default to the configuration of the provider when /agent/self returns Permission Denied
• the fact that this node is external is not really important and the bug would manifest itself as long as the token used to call Consul API has not the permission to call /agent/self

?

[splashx]

you want to default to the configuration of the provider when /agent/self returns Permission Denied

That is I think a reasonable acceptable behavior.

the fact that this node is external is not really important and the bug would manifest itself as long as the token used to call Consul API has not the permission to call /agent/self

Yes - well I have not tested with other node types, but this is I think a general bug.

[remilapeyre]

I thought we had to do

1. Datacenter of the resource if configured
2. Datacenter of the agent if the one in the resource is not defined
3. Datacenter of provider as last resort

Currently, it seems to me, that the behavior is 1 and 2 and never 3 but reading the documentation (https://github.com/terraform-providers/terraform-provider-consul/blame/master/website/docs/index.html.markdown#L57 and https://github.com/terraform-providers/terraform-provider-consul/blame/master/website/docs/r/keys.html.markdown#L40-L41) it should actually be:

1. Datacenter of the resource if configured
2. Datacenter of provider if set
3. Datacenter of the agent as of last resort

Unless I'm missing something, this is not what is being done in https://github.com/terraform-providers/terraform-provider-consul/blob/master/consul/resource_consul_keys.go#L330-L340

From a quick glance at the code, most of the resources are using getDC to get the datacenter when it is not defined in the resource except consul_intentions and consul_service.

The documentation seems consistent that the order should be (resource, provider then agent): https://github.com/terraform-providers/terraform-provider-consul/blame/master/website/docs/r/catalog_entry.html.markdown#L48-L49, https://github.com/terraform-providers/terraform-provider-consul/blame/master/website/docs/r/key_prefix.html.markdown#L60-L61, etc.

Only the documentation of consul_node seems consistent with its behavior.

The way getDC show some confusion around the datacenter definition like at https://github.com/terraform-providers/terraform-provider-consul/blob/master/consul/resource_consul_node.go#L55-L63 where it seems to me that this could be simplified as if dc, err = getDC(d, client); err != nil { return err } since getDC will try to read from d first.

I think changing the code to respect the documentation would be too much of a change and could surprise many people and that I could be better to change the documentation and the code so that first the datacenter defined in the resource is used, then the datacenter of the agent (current behavior) if possible then the one in the provider.

This would change most parts of the documentation from:

* `datacenter` - (Optional) The datacenter to use. This overrides the
  datacenter in the provider setup and the agent's default datacenter.

to:

* `datacenter` - (Optional) The datacenter to use. This overrides the
  agent's default datacenter and the datacenter in the provider setup.

and would not be a breaking change.

I will start working on a new PR for this change.

[splashx]

@remilapeyre

... so that first the datacenter defined in the resource is used, then the datacenter of the agent (current behavior) if possible then the one in the provider

Cool! I think that makes sense. Since we didnt have 1 defined and 3 never happened, it failed on 2 (the token did not have rights to list agents). From #56

The Permission Denied message is correct because the token doesn't have the rights to /agent/self.

🤞

[remilapeyre]

Hi @splashx, I started to work on the issue.

You can find the change here: https://github.com/remilapeyre/terraform-provider-consul/tree/datacenter-attribute-inheritance

I checked the change with the following Consul (1.4.0) configuration:

data_dir = "/tmp/consul"
server = true
ui = true
bootstrap_expect = 1

acl {
  enabled = true
  default_policy = "deny"

  tokens {
    master = "master_token"
  }
}

this anonymous policy:

key_prefix "" {
  policy = "write"
}

and the following terraform scenario:

provider "consul" {
  datacenter = "dc1"
}

resource "consul_keys" "app" {
  key {
    path  = "foo"
    value = "bar"
  }
}

Here, the result, first with the version 2.2.0 then with the version from the datacenter-attribute-inheritance branch:

➜  terraform-provider-consul git:(datacenter-attribute-inheritance) ✗ terraform init

Initializing provider plugins...
- Checking for available provider plugins on https://releases.hashicorp.com...
- Downloading plugin for provider "consul" (2.2.0)...

The following providers do not have any version constraints in configuration,
so the latest version was installed.

To prevent automatic upgrades to new major versions that may contain breaking
changes, it is recommended to add version = "..." constraints to the
corresponding provider blocks in configuration, with the constraint strings
suggested below.

* provider.consul: version = "~> 2.2"

Terraform has been successfully initialized!

You may now begin working with Terraform. Try running "terraform plan" to see
any changes that are required for your infrastructure. All Terraform commands
should now work.

If you ever set or change modules or backend configuration for Terraform,
rerun this command to reinitialize your working directory. If you forget, other
commands will detect it and remind you to do so if necessary.
➜  terraform-provider-consul git:(datacenter-attribute-inheritance) ✗ terraform version
Terraform v0.11.10
+ provider.consul v2.2.0

Your version of Terraform is out of date! The latest version
is 0.11.11. You can update by downloading from www.terraform.io/downloads.html
➜  terraform-provider-consul git:(datacenter-attribute-inheritance) ✗ terraform apply

An execution plan has been generated and is shown below.Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  + consul_keys.app
      id:                     <computed>
      datacenter:             <computed>
      key.#:                  "1"
      key.3548728206.default: ""
      key.3548728206.delete:  "false"
      key.3548728206.name:    ""
      key.3548728206.path:    "foo"
      key.3548728206.value:   "bar"
      var.%:                  <computed>


Plan: 1 to add, 0 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

consul_keys.app: Creating...
  datacenter:             "" => "<computed>"
  key.#:                  "" => "1"
  key.3548728206.default: "" => ""
  key.3548728206.delete:  "" => "false"
  key.3548728206.name:    "" => ""
  key.3548728206.path:    "" => "foo"
  key.3548728206.value:   "" => "bar"
  var.%:                  "" => "<computed>"

Error: Error applying plan:

1 error(s) occurred:

* consul_keys.app: 1 error(s) occurred:

* consul_keys.app: Failed to get datacenter from Consul agent: Unexpected response code: 403 (Permission denied)

Terraform does not automatically rollback in the face of errors.
Instead, your Terraform state file has been partially updated with
any resources that successfully completed. Please address the error
above and apply again to incrementally change your infrastructure.


➜  terraform-provider-consul git:(datacenter-attribute-inheritance) ✗ make && rm -rf .terraform && mkdir -p ~/.terraform.d
/plugins && cp ~/go/bin/terraform-provider-consul ~/.terraform.d/plugins/ && terraform init
==> Checking that code complies with gofmt requirements...
go install

Initializing provider plugins...

Terraform has been successfully initialized!

You may now begin working with Terraform. Try running "terraform plan" to see
any changes that are required for your infrastructure. All Terraform commands
should now work.

If you ever set or change modules or backend configuration for Terraform,
rerun this command to reinitialize your working directory. If you forget, other
commands will detect it and remind you to do so if necessary.
➜  terraform-provider-consul git:(datacenter-attribute-inheritance) ✗ terraform apply

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  + consul_keys.app
      id:                     <computed>
      datacenter:             <computed>
      key.#:                  "1"
      key.3548728206.default: ""
      key.3548728206.delete:  "false"
      key.3548728206.name:    ""
      key.3548728206.path:    "foo"
      key.3548728206.value:   "bar"
      var.%:                  <computed>


Plan: 1 to add, 0 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

consul_keys.app: Creating...
  datacenter:             "" => "<computed>"
  key.#:                  "" => "1"
  key.3548728206.default: "" => ""
  key.3548728206.delete:  "" => "false"
  key.3548728206.name:    "" => ""
  key.3548728206.path:    "" => "foo"
  key.3548728206.value:   "" => "bar"
  var.%:                  "" => "<computed>"
consul_keys.app: Creation complete after 0s (ID: consul)

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.

As you can see, the 2.2.0 version was not able to get the datacenter from the agent but the other got it from the provider configuration.

As I wrote in the commit, I'm not sure how to write tests for this change as it requires a specific configuration of Consul to trigger the bug.

Could you confirm the new version fixed the issue you had?

[ashald]

I just downloaded the latest provider version and have been bitten by this issue. :(

[remilapeyre]

Hi @ashald, I will merge the attached parch so this can be fixed in the next release.

[ashald]

Thanks a lot!

[remilapeyre]

Hi, the fix for this issue has been released in the 2.4.0 version so this should work as expected now :)