hashicorp / terraform-provider-aws

The AWS Provider enables Terraform to manage AWS resources.
https://registry.terraform.io/providers/hashicorp/aws
Mozilla Public License 2.0
9.87k stars 9.21k forks source link

Revisiting AWS ASG Tagging #874

Closed Zordrak closed 5 years ago

Zordrak commented 7 years ago

Taken from HangOps #terraform, Then moved from https://github.com/hashicorp/terraform/issues/15298 ...

@radeksimko @stack72 I would love your input on whether we can reopen the development referenced in https://github.com/hashicorp/terraform/pull/13574.

The solution provided doesn't appear to actually help resolve the problem it was created to resolve. It is meant to provide a way to interpolate the tags passed to the ASG, but because there seems no combination of interpolation functions that can help construct a list of maps from any other source of data other that a manually defined list of maps, its no more useful than just creating the tag blocks individually supplying interpolated values.

Pretty much everyone I've seen that needs a solution for this is trying to apply the same map of tags to ASGs as to other things and their source data is a map.

I appreciate the difficulties in managing the data structure validation, but I think I might have a better alternative than the provided solution which would at least in the short term provide the solution we need.

Given that the third parameter is simply a boolean for propagation, and therefore can have only two values, why not use the same approach as with versioned and nonversioned objects in S3 lifecycle rules?

propagate_tags nonpropogate_tags

Both could be supplied as a map, and while mutually exclusive with the other tag options, you could provide either one or both. Terraform then only needs to internally constructed two lists of blocks, one where each kv pair recieves propagate = true and one which receives propagate = false. Concatenate that list of maps and boom... profit.

The only other option I can see is to create an interpolation function that takes a map, and generates the output we need for the tags list.. but I cant work out how that would work as a generic function. For inputs you'd need a map, a key key, a value key, and then an optional additional map to merge with every map in the generated list

maplist(inputmap, keykey, valkey [, mergemap]) => list(merge(map(keykey, keys(inputmap), valkey, values(inputmap)),mergemap), ...)

I can't imagine that solution having any other use or being particularly easy to implement

Zordrak commented 7 years ago

This is my life now:

default_tags = {
  "Project"     = "<project>"
  "Environment" = "<environment>"
}

asg_default_tags = [
  {
    "key"                 = "Project"
    "value"               = "<project>"
    "propagate_on_launch" = "true"
  },
  {
    "key"                 = "Environment"
    "value"               = "<environment>"
    "propagate_on_launch" = "true"
  }
]

variable "default_tags" {
  type        = "map"
  description = "Default tags for all taggable resources"
  default     = {}
}

variable "asg_default_tags" {
  type         = "list"
  description = "Default tags translated into ASG-capable input"
  default      = []
}

# It is 100% acknowledged that this is a horrific awful and horrible
# hack and indefensible duplication. I get it, I really do.
# But until we have some kind of solution for this:
# https://github.com/terraform-providers/terraform-provider-aws/issues/874
# this is the only way I can think of to provide a single source of
# interpolated tag insertion that works for AWS Autoscaling Groups.
# Please don't hurt me.
resource "aws_autoscaling_group" "<asg_identifier>" {

  <...>

  tags = [
    "${concat(
      var.asg_default_tags,
      list(
        map(
          "key", "Name",
          "value", format(
            "%s-%s-%s/%s",
            var.project,
            var.environment,
            var.component,
            var.name
          ),
          "propagate_at_launch", "true"
        ),
        map(
          "key", "Component",
          "value", var.component,
          "propagate_at_launch", "true"
        ),
        map(
          "key", "Module",
          "value", var.module,
          "propagate_at_launch", "true"
        )
      )
    )}"
  ]
radeksimko commented 7 years ago

Hi @Zordrak

The PR you're referring too was trying to work around an issue with the language/HCL, rather being specific to Auto Scaling Groups. As such we believe it'd be better to try and fix the underlying issue allowing folks to use reasonable constructs as mentioned rather than implement workarounds in the schema for each resource.

Unfortunately we're unable to share any timelines for these improvements - however the Terraform Core team are looking into this and this pull request signifies the first step in that direction.

Thanks!

haines commented 6 years ago

Here is a hacky workaround (until 0.12 comes out) for anyone else who is struggling with this:

data "external" "tags" {
  count = "${length(keys(var.tags))}"

  program = ["cat", "-"]

  query = {
    key                 = "${element(keys(var.tags), count.index)}"
    value               = "${element(values(var.tags), count.index)}"
    propagate_at_launch = "true"
  }
}

resource "aws_autoscaling_group" "hack" {
  # ...
  tags = ["${data.external.tags.*.result}"]
}
austinbutler commented 6 years ago

What should this look like after 0.12?

red8888 commented 5 years ago

Here is a hacky workaround (until 0.12 comes out) for anyone else who is struggling with this:

data "external" "tags" {
  count = "${length(keys(var.tags))}"

  program = ["cat", "-"]

  query = {
    key                 = "${element(keys(var.tags), count.index)}"
    value               = "${element(values(var.tags), count.index)}"
    propagate_at_launch = "true"
  }
}

resource "aws_autoscaling_group" "hack" {
  # ...
  tags = ["${data.external.tags.*.result}"]
}

Sir you are a gentleman and a scholar this totally works, but can you explain what this is doing? Is program = ["cat", "-"] dangerous? Is terraform actually executing that command for every iteration?

Your calling an external command because tf has no generic loop construct right?

haines commented 5 years ago

Yep, this was the only way I could think of to generate the correct format for the tags since we can't create a list of maps using a loop or interpolation functions.

The external data module serializes the query map to JSON and passes it to the program on stdin. It then reads stdout and deserializes that from JSON to the result map.

cat - just echoes whatever is sent to stdin back to stdout, so the result is equal to the query. It should be totally safe, but isn't terribly efficient (you're right that it executes once per tag).

I've found that it's more than fast enough, though; if I understand correctly it executes in parallel with other data sources, and generally those will be slower because they are making HTTP requests to external services. Also, I've only ever had a handful of tags, so the number of iterations has been quite low.

red8888 commented 5 years ago

I asked an SO question about this, looks like the best solution for this is the null_data_source now:

https://stackoverflow.com/questions/53698758/how-do-i-apply-a-map-of-tags-to-aws-autoscaling-group/53699778#53699778

tracypholmes commented 5 years ago

Thank you for using Terraform and for opening up this question! This will be closed as it appears this question was provided a workaround or two, as well as a reference to 0.12.

Issues on GitHub are intended to be related to bugs or feature requests with the provider codebase. Please use https://discuss.hashicorp.com/c/terraform-providers for community discussions, and questions around Terraform.

If you believe that your issue was miscategorized as a question or closed in error, please create a new issue using one of the following provided templates: bug report or feature request. Please make sure to provide us with the appropriate information (as well as test with 0.12) so we can best determine how to assist with the given issue.

ghost commented 5 years ago

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!