Import process for cloudflare_record modifies the 'name' attribute

[james-robson]

Terraform Version

0.11.8

Affected Resource(s)

• cloudflare_record

Terraform Configuration Files

resource "cloudflare_record" "example" {
  domain = "example.com"
  name   = "foo.example.com"
  value  = "${aws_cloudfront_distribution.example.domain_name}"
  type   = "CNAME"
  ttl    = 1,
  proxied = true
}

General description

When importing a cloudflare record the import seems to work OK. However if you run a plan immediately after the import the plan shows that the record should be destroyed and re-created because the name has changed. This appears to be because the name in question contains the zone name (see the example below) and the zone name is trimmed. I'm not sure if there is a specific reason for removing the zone name from the record name but that is what causes the issue.

Expected Behavior

Importing a cloudflare record into Terraform creates a new entry in the state file. Running plan immediately after shows no changes.

Actual Behavior

Importing the record works OK, however if you run a plan the name has been modified (the suffix has been removed) which causes a new record to be created. The plan output looks as follows:

Terraform will perform the following actions:

-/+ cloudflare_record.mobile_web (new resource required)
      id:          "abcd1234" => <computed> (forces new resource)
      created_on:  "2018-10-24T08:18:47.38658Z" => <computed>
      domain:      "example.com" => "example.com"
      hostname:    "foo.example.com" => <computed>
      metadata.%:  "3" => <computed>
      modified_on: "2018-10-24T08:18:47.38658Z" => <computed>
      name:        "foo" => "foo.example.com" (forces new resource)
      proxiable:   "true" => <computed>
      proxied:     "true" => "true"
      ttl:         "1" => "1"
      type:        "CNAME" => "CNAME"
      value:       "abcd1234.cloudfront.net" => "abcd1234.cloudfront.net"
      zone_id:     "abcd1234abcd1234abcd1234" => <computed>


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

Steps to Reproduce

1. Create a Terraform resource as shown above with the name foo.example.com
2. Import said resource withterraform import cloudflare_record.example example.com/1234abcd
3. Note the import succeeds. Check the Terraform state file and you will see the name is simply foo instead of foo.example.com

Important Factoids

This is happening because of lines 512 and 513 in resource_cloudflare_record.go which appears to trim the zone name from the record.Name value:

	log.Printf("[INFO] Found record: %s", record.Name)
	name := strings.TrimSuffix(record.Name, "."+zoneName)

I'd like to know if there is a reason for this trim as it looks intentional. If so we would have to change our naming convention for all domain records in cloudflare.

[jacobbednarz]

Thanks for the report @james-robson! I'm a touch confused here as I think there might be something else at play because we currently just shy of 1k DNS entries using this format via an import and we don't have the issue as above (we run the bleeding edge provider). I see your id value is also changing which is forcing a new resource to be created. Do you still get the same result if that stays static.

One option here is that we update the DiffSuppressFunc for name to take the zone name into account when comparing so this doesn't trigger a new update but I'd want to exhaust other checks first to be sure.

[james-robson]

Hi @jacobbednarz, thanks for getting back to me. I'm unsure how I would preserve the ID, I had assumed that it was updating because the resource was changing (and therefore getting a new ID).

From the Terraform import docs:

Import will find the existing resource from ID and import it into your Terraform state at the given ADDRESS.

I'll have a play around with it and see if there is anything I can do to change that.

[james-robson]

Follow up - I removed the existing entry from Terraform with terraform state rm cloudflare_record.example and then re-imported, the output was success and included the expected ID. However, re-running the plan still provides the same output as above.

[jacobbednarz]

Alright, I managed to setup a reproducible case for this one and I have some answers. The TL;DR is that the name field, doesn't really expect you to have the domain value included. Hence the TrimSuffix call upon importing. The reason for the diff showing up is that your state file has "foo" but your resource has "foo.example.com" so it thinks it needs to change the value however the ForceNew won't allow an in place update, so it creates a new resource.

My comment above regarding the ID changing was a bit of a red herring however it was caused by the name field having a change and subsequently wanting to generate a new computed ID. Not entirely off track but still an effect instead of a cause here.

We have a couple of options we could do here.

• Document the name field better that it doesn't intend for you to put the domain value in there. I don't think this is particular controversial and matches the UI behavior.
• Add a DiffSuppressFunc that performs the TrimSuffix functionality before comparing the values.

I don't think these are mutually exclusive to complete however it does feel like your usage of the field is unintended and we could prevent this with some better documentation. In the same breath, it wouldn't hurt to add better safety rails in the provider if someone does decide to put in the full domain.

[patryk]

Actually, API accepts both bare label and FQDN as valid name value. But on output it always returns FQDN. I'd say limiting name field only to bare label will be challenging when the value is taken from third party variable, as it would require to do string trimming magic, which is always fun in HCL.

[jacobbednarz]

@patryk so your vote is for adding the DiffSuppressFunc as a fix here?

[patryk]

Yeah, I suppose the least invasive way is what we need here. I'd go with DiffSuppressFunc.

[jacobbednarz]

🆒 #151 should get us what we're after here.

@james-robson do you mind taking that PR for a test and make sure it addresses your issue too?

[patryk]

Merged already, but please @james-robson check it out whether it fixes your issue.

[james-robson]

Thanks for the quick turnaround @jacobbednarz and @patryk, I'll test this out ASAP and let you know the outcome.

[james-robson]

@jacobbednarz, @patryk - I managed to get the plugin working by building from master and checking it in to my terraform.d directory. The import worked fine and showed no changes on the plan so in terms of this issue that is fixed.

One thing that did cause issues for me (and the reason it took me a while to get back to you) was that I initially ran into another issue where the plan output failed saying the terraform-cloudflare-provider file was missing.

It turned out to be the same issue as this other provider:

hashicorp/terraform-provider-helm#111

I had to re-build the provider with CGO_ENABLED=0 go build -o terraform-provider-cloudflare in order to be able to use it properly on my Linux build agent.

I'm not sure whether this is something that should be in the README or the makefile, what are your thoughts?

[jacobbednarz]

This is something that has cropped up in a few other go projects I work on and it's commonly due to relying on Alpine linux as it is now using musl libc. Using CGO_ENABLED=0 does have some caveats as it prevents cross compiling dependencies so it's something that we shouldn't just drop in. There are also a few differences in the way the provider is packaged using the Makefile.

I'm +1 for opening a PR documenting this but on the fence about having it as a default without further investigation.

[james-robson]

@jacobbednarz you are absolutely right, as I'm on a Mac I used a golang:alpine Docker container to build the Linux binary.

That's good information, thanks for getting back to me. I'll try and get a PR up for the README note today.