Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicate Keys in Maps Get De-Duplicated During Count #18573

Closed
afrazkhan opened this issue Jul 31, 2018 · 8 comments
Closed

Duplicate Keys in Maps Get De-Duplicated During Count #18573

afrazkhan opened this issue Jul 31, 2018 · 8 comments

Comments

@afrazkhan
Copy link

Terraform Version

Terraform v0.11.7

Terraform Configuration Files

Given the following as part of a larger module called dns:

...
resource "digitalocean_record" "this_a_record" {
  count  = "${length(var.a_records)}"

  domain = "${var.domain}"
  type   = "A"
  name   = "${element(keys(var.a_records), count.index)}"
  value  = "${lookup(var.a_records, element(keys(var.a_records), count.index))}"
}
...

Being used like this:

module "example_com_dns" {
  source = "./modules/dns"

  domain  = "example.com"
  main_ip = "1.2.3.4"

  a_records = {
    "@" = "1.2.3.4"
    "@" = "5.6.7.8"
    "@" = "9.10.11.12"
    "foo" = "13.14.15.16"
    "bar" = "17.18.19.20"
}

Actual Behavior

The foo and bar records are created as expected; A records pointing to 13.14.15.16 and 17.18.19.20 respectively, but @ is de-duplicated, and only a single @ record is created. This would be true for any duplicate keys, i.e. if there were 2 foo keys in there, then only the last one would survive.

Either the keys() function is de-deplicating keys in the map, or applying one create function over the other. This might seem sensical, but I'll explain below why (when) you wouldn't want this behaviour.

Expected Behavior

Multiple records are created for each key, irrespective of whether or not it's a duplicate. This can be achieved by running the code in the module outside of a loop instead, i.e. multiple blocks like this:

resource "digitalocean_record" "at3" {
  domain = "example.com"
  type   = "A"
  name   = "@"
  value  = "1.2.3.4"
}

will result in multiple @ records with the correct values.

You would want this behaviour in situations where the same key can have multiple values, such as with multiple @ records.

@apparentlymart
Copy link
Contributor

Hi @afrazkhan! Sorry this doesn't work as expected.

The problem is actually right at the source:

  a_records = {
    "@" = "1.2.3.4"
    "@" = "5.6.7.8"
    "@" = "9.10.11.12"
    "foo" = "13.14.15.16"
    "bar" = "17.18.19.20"
  }

The map type is defined as a lookup table from unique keys to values, and so the immediate result of this constructor is a map with only one @. In the current version of Terraform a single value for "@" is selected arbitrarily. In the next major version, as a result of various improvements to the language implementation, this expression would actually produce an error on the second attempt to define a value for the key "@".

I think a more appropriate data type for your use-case could be a map of lists of lists:

  a_records = {
    ["@", "1.2.3.4"],
    ["@", "5.6.7.8"],
    ["@", "9.10.11.12"],
    ["foo", "13.14.15.16"],
    ["bar", "17.18.19.20"],
  }

The Terraform language in 0.11 unfortunately has some limitations that make it inconvenient to work with nested structures like this, but here's how it might look inside your module:

resource "digitalocean_record" "this_a_record" {
  count  = "${length(var.a_records)}"

  domain = "${var.domain}"
  type   = "A"
  name   = "${element(element(var.a_records, count.index), 0)}"
  value  = "${element(element(var.a_records, count.index), 1)}"
}

The next major release will include various improvements to make it easier to create this sort of module. Funnily enough, as a side-project to test out these new features while we're still under development I've been implementing various DNS-recordset modules (which are not compatible with Terraform 0.11) that show what this might look like in 0.12. I've not done one for DigitalOcean yet, but the Dyn one as a similar structure as would be needed for DigitalOcean. (These modules are not an official HashiCorp project, and I can't make any real statements about their quality at this time while they are targeting a version of Terraform that hasn't been release yet; I link this just as an example of how this problem might be solved after the next release.)

@afrazkhan
Copy link
Author

Yup, I've been reading about 0.12 for a while, and am looking forward to it.

I don't think that map is valid though? There's no assignment from a key to a value.

  a_records = {
    ["@", "1.2.3.4"],
    ["@", "5.6.7.8"],
    ["@", "9.10.11.12"],
    ["foo", "13.14.15.16"],
    ["bar", "17.18.19.20"],
  }

That's not a valid map, is it? Running it gives me expected: IDENT | STRING | ASSIGN | LBRACE got: LBRACK.

I thought maybe you meant a list of a lists:

  new_a_records = [
    ["@", "1.2.3.4"],
    ["@", "5.6.7.8"],
    ["@", "9.10.11.12"],
    ["foo", "13.14.15.16"],
    ["bar", "17.18.19.20"],
  ]

But that gives me element: argument 1 should be type list, got type string

@apparentlymart
Copy link
Contributor

Sorry @afrazkhan... I did indeed forget to update the braces {} to be brackets [] when I wrote that example.

I'm not sure what's going on with that other error, though... 🤔 this is probably one of those limitations I was referring to. It might work better to use the native index syntax for the innermost element access in those expressions:

resource "digitalocean_record" "this_a_record" {
  count  = "${length(var.a_records)}"

  domain = "${var.domain}"
  type   = "A"
  name   = "${element(var.a_records[count.index], 0)}"
  value  = "${element(var.a_records[count.index], 1)}"
}

(After 0.12 it'll be possible to write the much-more-natural var.a_records[count.index][0], but sadly the 0.11 language doesn't support chaining index brackets like that. 😖 )

@afrazkhan
Copy link
Author

I think it is down to a limitation, yes. I was trying something similar before, and concluded that Terraform just doesn't want to do anything involving iteration on multi-dimensional lists, on anything other than the first level. Same for the second attempt above, by the way.

Well, since I'd have to pretty much rewrite everything to do this the better way when 0.12 comes out soon, I think I'll just leave my explicitly defined statements in the main file for now, and rewrite it properly with 0.12.

Thanks for clarifying.

@afrazkhan
Copy link
Author

Actually this seems to point to a different issue. The example you gave does work if it's all the same file, but doesn't as part of a module. i.e. if you try to call it like this:

main.yml

module "example_com_dns" {
  source = "./modules/dns"

  domain  = "example.com"
  main_ip = "${lookup(var.example_com_dns, "at01")}"

  "a_records" = [
    ["@"   , "5.6.7.8"],
    ["@"   , "7.6.7.8"]
  ]
}

dns module

resource "digitalocean_record" "this_a_record" {
  depends_on = ["digitalocean_domain.this_domain"]
  count  = "${length(var.a_records)}"

  domain = "${var.domain}"
  type   = "A"
  name   = "${element(var.a_records[count.index],0)}"
  value  = "${element(var.a_records[count.index],1)}"
}

That fails. But if I define a_records inside the dns module, or even as a default value in it's variables.tf file, it works.

@afrazkhan afrazkhan reopened this Aug 1, 2018
@apparentlymart
Copy link
Contributor

Ahh yes, I forgot that in 0.11 Terraform doesn't fully support passing nested datastructures into (via input variables) and out of (via output values) child modules - this'll be the next installment in the preview series of blog posts. 😖

That is also something 0.12 will address, but if you want something to use in the interim the usual approach has been to pass values in as strings and then "parse" them inside the module, like this:

module "example_com_dns" {
  source = "./modules/dns"

  domain  = "example.com"
  main_ip = "${lookup(var.example_com_dns, "at01")}"

  a_records = [
    "@ 5.6.7.8",
    "@ 7.6.7.8",
  ]
}
resource "digitalocean_record" "this_a_record" {
  depends_on = ["digitalocean_domain.this_domain"]
  count  = "${length(var.a_records)}"

  domain = "${var.domain}"
  type   = "A"
  name   = "${element(split(" ", var.a_records[count.index]),0)}"
  value  = "${element(split(" ", var.a_records[count.index]),1)}"
}

This other limitation is already covered by #2114 (it's talking about nested maps, but it's really any nested type except strings), so I'm going to close this out for now. It seems like you collided with a number of different limitations all at once here, which I'm sorry about but also glad that they're all things we were already aware of an planning to fix in the next release.

@afrazkhan
Copy link
Author

Wooooo, hacky ;) Thanks for all the clarification, hopefully this helps someone else too.

Really looking forward to 0.12 now.

@ghost
Copy link

ghost commented Apr 2, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants