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

feat: Add public and private tags per az #860

Merged
merged 2 commits into from
Jan 13, 2023

Conversation

nitrocode
Copy link
Member

@nitrocode nitrocode commented Nov 16, 2022

Description

Add public and private tags per az

  • public_subnet_tags_per_az
  • private_subnet_tags_per_az
  public_subnet_tags_per_az = {
    "${local.region}a" = {
      "availability-zone" = "${local.region}a"
    }
  }

Motivation and Context

I want to set up kubernetes controllers that can use a specific subnet by tag. I could use name but our names have a lot of information in them. I would much rather be able to tag a subnet by AZ with certain tags like the AZ itself or the cluster I'm trying to target for this AZ.

Breaking Changes

None

How Has This Been Tested?

  • I have updated at least one of the examples/* to demonstrate and validate my change(s)
  • I have tested and validated these changes using one or more of the provided examples/* projects
  • I have executed pre-commit run -a on my pull request
cd examples/simple-vpc
terraform init
terraform plan
  # module.vpc.aws_subnet.public[0] will be created
  + resource "aws_subnet" "public" {
      + tags                                           = {
          + "Example"           = "ex-simple-vpc"
          + "GithubOrg"         = "terraform-aws-modules"
          + "GithubRepo"        = "terraform-aws-vpc"
          + "Name"              = "overridden-name-public"
          + "availability-zone" = "eu-west-1a"
        }
      + tags_all                                       = {
          + "Example"           = "ex-simple-vpc"
          + "GithubOrg"         = "terraform-aws-modules"
          + "GithubRepo"        = "terraform-aws-vpc"
          + "Name"              = "overridden-name-public"
          + "availability-zone" = "eu-west-1a"
        }
    }

  # module.vpc.aws_subnet.public[1] will be created
  + resource "aws_subnet" "public" {
      + tags                                           = {
          + "Example"           = "ex-simple-vpc"
          + "GithubOrg"         = "terraform-aws-modules"
          + "GithubRepo"        = "terraform-aws-vpc"
          + "Name"              = "overridden-name-public"
        }
      + tags_all                                       = {
          + "Example"           = "ex-simple-vpc"
          + "GithubOrg"         = "terraform-aws-modules"
          + "GithubRepo"        = "terraform-aws-vpc"
          + "Name"              = "overridden-name-public"
        }
    }

@nitrocode nitrocode changed the title Add public and private tags per az feat: Add public and private tags per az Nov 16, 2022
@nitrocode
Copy link
Member Author

cc: @bryantbiggs @antonbabenko could you please review this when you folks have a chance?

@nitrocode
Copy link
Member Author

cc: @bryantbiggs @antonbabenko friendly ping

@bryantbiggs
Copy link
Member

My personal opinion is that I don't agree with this approach. I am not sure what the intention is, but controllers can be written to query the IMDS to find out what AZ a pod/node is running within

@nitrocode
Copy link
Member Author

The goal is to be able to concentrate traffic against a specific AZ.

We can filter the subnet by tag using the aws-load-balancer-controller (for example) but if we cannot tag per subnet AZ, then we cannot filter by AZ. Being able to tag here by subnet AZ, would allow us to control traffic using these controllers.

@jessegonzalez-life360
Copy link

but controllers can be written to query the IMDS to find out what AZ a pod/node is running within

Yes they could, but a feature to allow defining target subnets based on tags exists already. Also, the effort to include that functionality in a controller would be more complex than what is proposed here.

Without the ability to tag subnets per AZ, one has some options:

  • click ops
  • using the terraform null_resource with provisioner to tag subnets as needed

With @nitrocode's submission, one can accomplish tagging subnets as needed to satisfy the single AZ target, or any combination of subnet aggregation tagging based on business needs.

@nitrocode
Copy link
Member Author

cc: @bryantbiggs (friendly ping)

Copy link
Member

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM because this addition is helpful for people who are now even using EKS but they still want to have the possibility to tag subnets differently.

@antonbabenko antonbabenko merged commit a82c9d3 into terraform-aws-modules:master Jan 13, 2023
antonbabenko pushed a commit that referenced this pull request Jan 13, 2023
## [3.19.0](v3.18.1...v3.19.0) (2023-01-13)

### Features

* Add public and private tags per az ([#860](#860)) ([a82c9d3](a82c9d3))

### Bug Fixes

* Use a version for  to avoid GitHub API rate limiting on CI workflows ([#876](#876)) ([2a0319e](2a0319e))
@antonbabenko
Copy link
Member

This PR is included in version 3.19.0 🎉

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2023
@nitrocode nitrocode deleted the tags-per-az branch February 14, 2023 02:38
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.

4 participants