Skip to content
This repository has been archived by the owner on Feb 5, 2020. It is now read-only.

Revert "Merge pull request #1314 from estroz/autoscaling-group-tags" #1341

Merged
merged 1 commit into from
Jul 10, 2017

Conversation

estroz
Copy link
Contributor

@estroz estroz commented Jul 10, 2017

This reverts commit b018c80, reversing changes made to d684cca.

When running terraform plan to deploy a cluster, the following error message is received:

1 error(s) occurred:

* variable tectonic_autoscaling_group_extra_tags should be type list, got map

Comments like hashicorp/terraform#7705 (comment) indicate that terraform cannot handle complicated data structures, such as lists of maps whose structures are not explicitly declared, i.e. user-created variables. Therefore the changes created by b018c80 should be reverted until lists of arbitrary maps are supported. In the meantime a workaround will be created and merged.

…tags"

This reverts commit b018c80, reversing
changes made to d684cca.
@estroz estroz requested review from ggreer, amrutac and kans July 10, 2017 19:03
@kans kans merged commit 18bf314 into coreos:master Jul 10, 2017
@estroz estroz deleted the autoscaling-group-tags branch July 10, 2017 23:07
@sym3tri
Copy link
Contributor

sym3tri commented Aug 24, 2017

@estroz what was the workaround?

@estroz
Copy link
Contributor Author

estroz commented Aug 24, 2017

@sym3tri users must manually tag their autoscaling groups. I can make a PR documenting this if you'd like.

@s-urbaniak
Copy link
Contributor

s-urbaniak commented Sep 18, 2017

This is very strange, but I believe I have the explanation. We are generating a JSON based terraform.tfvars file.

Using a list of maps is fine in HCL syntax (I am using the following for all my clusters):

tectonic_autoscaling_group_extra_tags = [
  {
    key = "expirationDate",
    value = "yyyy-mm-dd",
    propagate_at_launch = true,
  },
]

When I convert the above to JSON syntax:

{
  "tectonic_autoscaling_group_extra_tags": [
    {
      "key": "expirationDate",
      "propagate_at_launch": true,
      "value": "yyyy-mm-dd"
    },
  ],
}

Then I also get the same reported error:

1 error(s) occurred:

* variable tectonic_autoscaling_group_extra_tags should be type list, got map

This error message doesn't make any sense, until we convert the above using https://github.com/kvz/json2hcl (which simply uses the Terraform JSON parser) back to HCL:

"tectonic_autoscaling_group_extra_tags" = {
  "key" = "expirationDate"

  "propagate_at_launch" = true

  "value" = "yyyy-mm-dd"
}

One can see, that the JSON parser simply omitted the list declaration and generated a simple map. Now the error message makes total sense.

@s-urbaniak
Copy link
Contributor

Other interesting failure cases. When I declare two list entries like this:

  "tectonic_autoscaling_group_extra_tags": [
    {
      "key": "expirationDate",
      "propagate_at_launch": true,
      "value": "2017-09-20"
    },
    {
      "key": "foo",
      "propagate_at_launch": true,
      "value": "bar"
    }
  ],

Terraform generates two HCL variables of the same name:

"tectonic_autoscaling_group_extra_tags" = {
  "key" = "expirationDate"

  "propagate_at_launch" = true

  "value" = "2017-09-20"
}

"tectonic_autoscaling_group_extra_tags" = {
  "key" = "foo"

  "propagate_at_launch" = true

  "value" = "bar"
}

The terraform plan error message here obviously also makes sense:

invalid value "terraform.tfvars" for flag -var-file-default: multiple map declarations not supported for variables

@lucab
Copy link
Contributor

lucab commented Sep 18, 2017

For reference hashicorp/hcl#209.

@s-urbaniak
Copy link
Contributor

Another reference hashicorp/terraform#15549

@s-urbaniak
Copy link
Contributor

two ways out of this:

  • we generate terraform.tfvars in HCL syntax (which supports lists of maps)
  • we fix the JSON parser (that's obviously the cleaner way)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants