Re-ordering tag merging

[sc250024]

Description

Re-orders the tag merging to be consistent with Terraform's map merging strategy, and provides the user greater control over specific resource tagging when tags are inherited from upstream.

Motivation and Context

Short version

The merging strategy of merging maps for a given object should always follow the pattern:

tags = "${merge(map(default_map, least_specific_map, ..., ..., most_specific_map)}"

This is also documented on the Terraform Interpolation page:

Returns the union of 2 or more maps. The maps are consumed in the order provided, and duplicate keys overwrite previous entries.

This PR changes the tag merging strategy to honor that.

Long version

I use CloudPosse's terraform-null-label module to enforce standard naming within my entire Terraform infrastructure. Using a module block like the following:

module "vpc_label" {
  delimiter = "-"
  name      = "vpc"
  namespace = "eu-west-1"
  source    = "git::https://github.com/cloudposse/terraform-null-label.git?ref=master"
  stage     = "prod"
  tags      = "${map("LastModified", "${timestamp()}")}"
}

I can ensure that not only are tags created in a standard way, but that more complicated tagging schemes (like for auto-scaling groups) are already created for me. However, I discovered a conflict with the way that this VPC module handles merging of tags for resources.

The problem is that the terraform-null-label module above automatically creates a "Name" tag that I cannot drop. For that labeling module, the output ${module.vpc_label.tags} will return a map of tags it creates. The problem is with the way that this VPC module is currently written, if I set ...

module "vpc" {
  ...
  source  = "terraform-aws-modules/vpc/aws"
  tags    = "${module.vpc_label.tags}"
  version = "1.37.0"
}

...and then run a terraform apply, all of my Name tags look like the following:

  ~ module.vpc.aws_subnet.private[2]
      tags.LastModified:     "2018-06-30T20:50:31Z" => "2018-06-30T21:26:01Z"
      tags.Name:             "pb-euw1-prod-vpc-private-eu-west-1c" => "pb-euw1-prod-vpc"

  ~ module.vpc.aws_subnet.public[0]
      tags.LastModified:     "2018-06-30T20:50:31Z" => "2018-06-30T21:26:01Z"
      tags.Name:             "pb-euw1-prod-vpc-public-eu-west-1a" => "pb-euw1-prod-vpc"

  ~ module.vpc.aws_subnet.redshift[0]
      tags.LastModified:     "2018-06-30T20:50:31Z" => "2018-06-30T21:26:01Z"
      tags.Name:             "pb-euw1-prod-vpc-redshift-eu-west-1a" => "pb-euw1-prod-vpc"

Even if I specifically try and reset the Name tag using the <resource>_tags inputs available like so:

private_subnet_tags {
  "Name" = "some-random-name"
}

The tag still is not honored for the private_subnet resources, in this case. In general, the merging strategy of merging maps for a given object should always follow the pattern:

tags = "${merge(map(default_map, least_specific_map, ..., ..., most_specific_map)}"

How Has This Been Tested?

Ran terraform apply on the following modification of the Simple VPC example code:

provider "aws" {
  region = "eu-west-1"
}

module "vpc" {
  source = "../../"

  name = "simple-example"

  cidr = "10.0.0.0/16"

  azs             = ["eu-west-1a", "eu-west-1b", "eu-west-1c"]
  private_subnets = ["10.0.1.0/24", "10.0.2.0/24", "10.0.3.0/24"]
  public_subnets  = ["10.0.101.0/24", "10.0.102.0/24", "10.0.103.0/24"]

  enable_nat_gateway = true
  single_nat_gateway = true

  public_subnet_tags = {
    Name = "overriden-name-public"
  }

  tags = {
    Owner       = "user"
    Environment = "dev"
    Name        = "default-name-public"
  }

  vpc_tags = {
    Name = "vpc-name"
  }
}

The output shows that the Name tag specified in the public_subnet_tags block is honored even though there is already a Name tag in the tags block:

  + module.vpc.aws_subnet.public[0]
      id:                               <computed>
      assign_ipv6_address_on_creation:  "false"
      availability_zone:                "eu-west-1a"
      cidr_block:                       "10.0.101.0/24"
      ipv6_cidr_block:                  <computed>
      ipv6_cidr_block_association_id:   <computed>
      map_public_ip_on_launch:          "true"
      tags.%:                           "3"
      tags.Environment:                 "dev"
      tags.Name:                        "overriden-name-public"
      tags.Owner:                       "user"
      vpc_id:                           "${aws_vpc.this.id}"

  + module.vpc.aws_subnet.public[1]
      id:                               <computed>
      assign_ipv6_address_on_creation:  "false"
      availability_zone:                "eu-west-1b"
      cidr_block:                       "10.0.102.0/24"
      ipv6_cidr_block:                  <computed>
      ipv6_cidr_block_association_id:   <computed>
      map_public_ip_on_launch:          "true"
      tags.%:                           "3"
      tags.Environment:                 "dev"
      tags.Name:                        "overriden-name-public"
      tags.Owner:                       "user"
      vpc_id:                           "${aws_vpc.this.id}"

  + module.vpc.aws_subnet.public[2]
      id:                               <computed>
      assign_ipv6_address_on_creation:  "false"
      availability_zone:                "eu-west-1c"
      cidr_block:                       "10.0.103.0/24"
      ipv6_cidr_block:                  <computed>
      ipv6_cidr_block_association_id:   <computed>
      map_public_ip_on_launch:          "true"
      tags.%:                           "3"
      tags.Environment:                 "dev"
      tags.Name:                        "overriden-name-public"
      tags.Owner:                       "user"
      vpc_id:                           "${aws_vpc.this.id}"

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.

[sc250024]

@antonbabenko Thoughts? This one's fairly straightforward. Mostly looking to hear the reasons for keeping it the way it is.

Thank you!

[sc250024]

@antonbabenko I just fixed the merge conflicts. Seems like this one should be pretty straightforward?

[antonbabenko]

Yes, this seems rather straightforward, but still required some verification. Thank you for fixing this (and a nudge) :)

[antonbabenko]

v1.44.0 has been released

[sc250024]

Thank you @antonbabenko !

[github-actions]

I'm going to lock this pull request 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 related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.