実録！Terraform セルフリファクタリング

かずえ

2019.10.30

この記事は公開されてから1年以上経過しています。情報が古い可能性がありますので、ご注意ください。

こんにちは！大阪オフィスのかずえです。今回は、自分で書いたTerraformコードがイマイチだったのでセルフリファクタリングしてみた過程を共有したいと思います。
ぜひ「こうすればもっと良くなるよ」といったご意見ください！

やりたいこと

VPCを二つ（VPCAとVPCB）作成する

各VPCはパブリックサブネットを二つ持つ
(後ほどTransit GatewayでVPC間を接続予定)

初回構築時のコード

data aws_availability_zones az {}

variable vpc_cidr {
  type = list(string)
  default = [
    "10.0.0.0/16",
    "172.16.0.0/16"
  ]
}

variable vpc_name {
  type = list(string)
  default = [
    "A",
    "B"
  ]
}

resource aws_vpc vpc {
  count                = length(var.vpc_cidr)
  cidr_block           = var.vpc_cidr[count.index]
  enable_dns_support   = true
  enable_dns_hostnames = true

  tags = {
    Name = "${var.basename}-${var.env}-vpc-${element(var.vpc_name, count.index)}"
  }
}

resource aws_internet_gateway igw {
  count  = length(var.vpc_cidr)
  vpc_id = aws_vpc.vpc[count.index].id
  tags = {
    Name = "${var.basename}-${var.env}-vpc-${element(var.vpc_name, count.index)}-igw"
  }
}
resource aws_subnet A-public {
  count                   = 2
  vpc_id                  = aws_vpc.vpc[index(var.vpc_name, "A")].id
  cidr_block              = cidrsubnet(aws_vpc.vpc[0].cidr_block, 8, count.index)
  availability_zone       = data.aws_availability_zones.az.names[count.index]
  map_public_ip_on_launch = false

  tags = {
    Name = "${var.basename}-${var.env}-vpc-A-public-subnet-${count.index + 1}"
  }
}
resource aws_subnet B-public {
  count                   = 2
  vpc_id                  = aws_vpc.vpc[index(var.vpc_name, "B")].id
  cidr_block              = cidrsubnet(aws_vpc.vpc[1].cidr_block, 8, count.index)
  availability_zone       = data.aws_availability_zones.az.names[count.index]
  map_public_ip_on_launch = false

  tags = {
    Name = "${var.basename}-${var.env}-vpc-B-public-subnet-${count.index + 1}"
  }
}
resource aws_route_table public {
  count  = length(var.vpc_cidr)
  vpc_id = aws_vpc.vpc[count.index].id

  tags = {
    Name = "${var.basename}-${var.env}-vpc-${var.vpc_name[count.index]}-public-subnet-rtb"
  }
}
resource aws_route public {
  count                  = length(var.vpc_cidr)
  route_table_id         = aws_route_table.public[count.index].id
  gateway_id             = aws_internet_gateway.igw[count.index].id
  destination_cidr_block = "0.0.0.0/0"
}
resource aws_route_table_association A-public {
  count          = 2
  subnet_id      = aws_subnet.A-public[count.index].id
  route_table_id = aws_route_table.public[index(var.vpc_name, "A")].id
}
resource aws_route_table_association B-public {
  count          = 2
  subnet_id      = aws_subnet.B-public[count.index].id
  route_table_id = aws_route_table.public[index(var.vpc_name, "B")].id
}

resource aws_network_acl acl {
  count  = length(var.vpc_cidr)
  vpc_id = aws_vpc.vpc[count.index].id

  subnet_ids = setunion(
    count.index == index(var.vpc_name, "A") ? aws_subnet.A-public.*.id : aws_subnet.B-public.*.id
  )

  ingress {
    protocol   = "-1"
    rule_no    = 100
    action     = "allow"
    cidr_block = "0.0.0.0/0"
    from_port  = 0
    to_port    = 0
  }

  egress {
    protocol   = "-1"
    rule_no    = 100
    action     = "allow"
    cidr_block = "0.0.0.0/0"
    from_port  = 0
    to_port    = 0
  }

  tags = {
    Name = "${var.basename}-${var.env}-vpc-${var.vpc_name[count.index]}-acl"
  }
}

皆さんならどのようにリファクタリングされますでしょうか。それではリファクタリング、開始します。

変更1 : listからmapに変える

countの0がVPCA、1がVPCBと暗黙的に決まっているのがイマイチだなと感じました。

aws_route_table.public[index(var.vpc_name, "A")]といった処理もわかりにくいですよね。最初はaws_route_table.public[0]と書いていました。ですが、できるだけマジックナンバーを無くしたいと思ってindex() を使う方法にしたのです。が、わかりにくい。

v0.12の新文法 for_eachを(mapを代入して)使うと、1resouce項で複数リソース作成した場合のlistのインデックス部分（例:aws_route_table.public[0]の0の部分）が、mapのindex値に変わります。わかりやすくなりそうなので、変数をlistからmapに変えて、countをfor_eachに変えてみました。

data aws_availability_zones az {}

variable vpc {
  type = map(string)
  default = {
    A = "10.0.0.0/16",
    B = "172.16.0.0/16"
  }
}

resource aws_vpc vpc {
  for_each             = var.vpc
  cidr_block           = each.value
  enable_dns_support   = true
  enable_dns_hostnames = true

  tags = {
    Name = "${var.basename}-${var.env}-vpc-${each.key}"
  }
}

resource aws_internet_gateway igw {
  for_each = var.vpc
  vpc_id   = aws_vpc.vpc[each.key].id
  tags = {
    Name = "${var.basename}-${var.env}-vpc-${each.key}-igw"
  }
}

resource aws_subnet A-public {
  count                   = 2
  vpc_id                  = aws_vpc.vpc["A"].id
  cidr_block              = cidrsubnet(aws_vpc.vpc["A"].cidr_block, 8, count.index)
  availability_zone       = data.aws_availability_zones.az.names[count.index]
  map_public_ip_on_launch = false

  tags = {
    Name = "${var.basename}-${var.env}-vpc-A-public-subnet-${count.index + 1}"
  }
}
resource aws_subnet B-public {
  count                   = 2
  vpc_id                  = aws_vpc.vpc["B"].id
  cidr_block              = cidrsubnet(aws_vpc.vpc["B"].cidr_block, 8, count.index)
  availability_zone       = data.aws_availability_zones.az.names[count.index]
  map_public_ip_on_launch = false

  tags = {
    Name = "${var.basename}-${var.env}-vpc-B-public-subnet-${count.index + 1}"
  }
}
resource aws_route_table public {
  for_each = var.vpc
  vpc_id   = aws_vpc.vpc[each.key].id

  tags = {
    Name = "${var.basename}-${var.env}-vpc-${each.key}-public-subnet-rtb"
  }
}
resource aws_route public {
  for_each               = var.vpc
  route_table_id         = aws_route_table.public[each.key].id
  gateway_id             = aws_internet_gateway.igw[each.key].id
  destination_cidr_block = "0.0.0.0/0"
}
resource aws_route_table_association A-public {
  count          = 2
  subnet_id      = aws_subnet.A-public[count.index].id
  route_table_id = aws_route_table.public["A"].id
}
resource aws_route_table_association B-public {
  count          = 2
  subnet_id      = aws_subnet.B-public[count.index].id
  route_table_id = aws_route_table.public["B"].id
}

resource aws_network_acl acl {
  for_each = var.vpc
  vpc_id   = aws_vpc.vpc[each.key].id

  subnet_ids = setunion(
    each.key == "A" ? aws_subnet.A-public.*.id : aws_subnet.B-public.*.id
  )

  ingress {
    protocol   = "-1"
    rule_no    = 100
    action     = "allow"
    cidr_block = "0.0.0.0/0"
    from_port  = 0
    to_port    = 0
  }

  egress {
    protocol   = "-1"
    rule_no    = 100
    action     = "allow"
    cidr_block = "0.0.0.0/0"
    from_port  = 0
    to_port    = 0
  }

  tags = {
    Name = "${var.basename}-${var.env}-vpc-${each.key}-acl"
  }
}

aws_route_table.public["A"].id などと書けるようになったので、少しわかりやすくなったのではないでしょうか。

変更2 : VPC毎に書いている箇所をまとめたい

大体のリソースはVPCAとVPCBでまとめて作成できているのですが、サブネットなど1VPCあたり複数個作成するものはリソースを分けて記述しています。どうせならこういったコードも駆逐してすべてVPCAとVPCBまとめて作成できるようにしたい。ということでやってみました。

data aws_availability_zones az {}

variable vpc {
  type = map(string)
  default = {
    A = "10.0.0.0/16",
    B = "172.16.0.0/16"
  }
}

locals {
  vpc_name = keys(var.vpc)
}

resource aws_vpc vpc {
  for_each             = var.vpc
  cidr_block           = each.value
  enable_dns_support   = true
  enable_dns_hostnames = true

  tags = {
    Name = "${var.basename}-${var.env}-vpc-${each.key}"
  }
}

resource aws_internet_gateway igw {
  for_each = var.vpc
  vpc_id   = aws_vpc.vpc[each.key].id
  tags = {
    Name = "${var.basename}-${var.env}-vpc-${each.key}-igw"
  }
}
resource aws_subnet public {
  for_each                = { for index, name in concat(local.vpc_name, local.vpc_name) : index => name }
  vpc_id                  = aws_vpc.vpc[each.value].id
  cidr_block              = cidrsubnet(aws_vpc.vpc[each.value].cidr_block, 8, floor(each.key / 2))
  availability_zone       = data.aws_availability_zones.az.names[floor(each.key / 2)]
  map_public_ip_on_launch = false

  tags = {
    Name = "${var.basename}-${var.env}-vpc-${each.value}-public-subnet-${floor(each.key / 2) + 1}"
  }
}
resource aws_route_table public {
  for_each = var.vpc
  vpc_id   = aws_vpc.vpc[each.key].id

  tags = {
    Name = "${var.basename}-${var.env}-vpc-${each.key}-public-subnet-rtb"
  }
}
resource aws_route public {
  for_each               = var.vpc
  route_table_id         = aws_route_table.public[each.key].id
  gateway_id             = aws_internet_gateway.igw[each.key].id
  destination_cidr_block = "0.0.0.0/0"
}
resource aws_route_table_association public {
  for_each       = { for index, name in concat(local.vpc_name, local.vpc_name) : index => name }
  subnet_id      = aws_subnet.public[each.key].id
  route_table_id = aws_route_table.public[each.value].id
}

resource aws_network_acl acl {
  for_each = var.vpc
  vpc_id   = aws_vpc.vpc[each.key].id

  subnet_ids = [for subnet in aws_subnet.public : subnet.id if subnet.vpc_id == aws_vpc.vpc[each.key].id]

  ingress {
    protocol   = "-1"
    rule_no    = 100
    action     = "allow"
    cidr_block = "0.0.0.0/0"
    from_port  = 0
    to_port    = 0
  }

  egress {
    protocol   = "-1"
    rule_no    = 100
    action     = "allow"
    cidr_block = "0.0.0.0/0"
    from_port  = 0
    to_port    = 0
  }

  tags = {
    Name = "${var.basename}-${var.env}-vpc-${each.key}-acl"
  }
}

できました。が、可読性最悪。。これはだめでしょう。

変更3 : モジュール化する

どうにかわかりやすく、かつまとめて記述する方法はないものかと試行錯誤していたのですが、「あれ、これモジュールを利用するのが一番まっとうな方法なのでは？」と気づいてしまいました。

variable vpc {
  type = map(string)
  default = {
    A = "10.0.0.0/16",
    B = "172.16.0.0/16"
  }
}

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

  env      = var.env
  basename = var.basename
  name     = "A"
  cidr     = var.vpc["A"]
}
module B {
  source = "./modules/vpc"

  env      = var.env
  basename = var.basename
  name     = "B"
  cidr     = var.vpc["B"]
}

./modules/vpc/vpc.tf

variable env {}
variable basename {}
variable name {}
variable cidr {}

data aws_availability_zones az {}

resource aws_vpc vpc {
  cidr_block           = var.cidr
  enable_dns_support   = true
  enable_dns_hostnames = true

  tags = {
    Name = "${var.basename}-${var.env}-vpc-${var.name}"
  }
}

resource aws_internet_gateway igw {
  vpc_id = aws_vpc.vpc.id
  tags = {
    Name = "${var.basename}-${var.env}-vpc-${var.name}-igw"
  }
}
resource aws_subnet public {
  count                   = 2
  vpc_id                  = aws_vpc.vpc.id
  cidr_block              = cidrsubnet(aws_vpc.vpc.cidr_block, 8, count.index)
  availability_zone       = data.aws_availability_zones.az.names[count.index]
  map_public_ip_on_launch = false

  tags = {
    Name = "${var.basename}-${var.env}-vpc-${var.name}-public-subnet-${count.index + 1}"
  }
}
resource aws_route_table public {
  vpc_id = aws_vpc.vpc.id

  tags = {
    Name = "${var.basename}-${var.env}-vpc-${var.name}-public-subnet-rtb"
  }
}
resource aws_route public {
  route_table_id         = aws_route_table.public.id
  gateway_id             = aws_internet_gateway.igw.id
  destination_cidr_block = "0.0.0.0/0"
}
resource aws_route_table_association public {
  count          = 2
  subnet_id      = aws_subnet.public[count.index].id
  route_table_id = aws_route_table.public.id
}

resource aws_network_acl acl {
  vpc_id = aws_vpc.vpc.id

  subnet_ids = aws_subnet.public.*.id

  ingress {
    protocol   = "-1"
    rule_no    = 100
    action     = "allow"
    cidr_block = "0.0.0.0/0"
    from_port  = 0
    to_port    = 0
  }

  egress {
    protocol   = "-1"
    rule_no    = 100
    action     = "allow"
    cidr_block = "0.0.0.0/0"
    from_port  = 0
    to_port    = 0
  }

  tags = {
    Name = "${var.basename}-${var.env}-vpc-${var.name}-acl"
  }
}

はい。わかりやすく、かつ重複処理を共通化することができています。

おまけ : モジュール内でもfor_eachすればいいんじゃないの？

こんな感じ

./modules/vpc/vpc.tf

module vpc {
　for_each = var.vpc
  source = "./modules/vpc"

  env      = var.env
  basename = var.basename
  name     = each.key
  cidr     = each.value
}

2019/10/30現時点ではこれはできません。

Moduleでのfor_each（とcount）の利用は現在準備中だそうですよ。

Module count and for_each

感想、学び

moduleの利便性を改めて実感しました。

「変更2 : VPC毎に書いている箇所をまとめたい」で書いたコードの可読性が最悪になったのは、Terraformはネストループを記述できないからだと考えています。Subnet部分などの記述はVPCごとに二つずつリソース作成 = 2x2のリソースを作成する処理です。for_eachでネストループが書けるとか、for_eachとcountを併用できると言ったことができればもう少し簡潔に書けたのではないかと思います。

そこで、moduleを活用することで

moduleを複数回利用することで二つのVPCの処理を共通化する
module内ではcount(for_eachでも良い)を使ってVPC内の複数回リソース作成する処理(Subnetなど)を共通化する

と、moduleとcount(for_each)で2階層作成し、ネストループのようなことが実現できました。当たり前といえば当たり前のことではありますが、個人的には今回のリファクタリングでこの部分の理解が深まりとても良かったです。

「こうすればもっと良くに書けるよ」というご意見、お待ちしております！

実録！Terraform セルフリファクタリング

やりたいこと

初回構築時のコード

変更1 : listからmapに変える

変更2 : VPC毎に書いている箇所をまとめたい

変更3 : モジュール化する

おまけ : モジュール内でもfor_eachすればいいんじゃないの？

感想、学び

イベント

EVENT【5/14（火）】アノテーションの中途向けオンライン会社説明会を開催します

EVENT【5/23（木）】ユースケースに学ぶAWS運用のノウハウ～可視化から統制まで～

EVENT【5/16（木）】Snowflakeを触ってみよう！初めての方向けハンズオンセミナー

EVENT【4/18リモート】クラスメソッドのフリーランスエンジニア会社説明会〜AWS/システムアーキテクト/Android案件特集〜を開催します

EVENT【5/10（金）名古屋】クラスメソッドグループの会社説明会を開催します！

EVENT【4/19（金）】認証機能の開発工数削減をデモで体験！次世代認証基盤サービス『Auth0 by Okta』導入実践ウェビナー

EVENT【4/25（木）リモート】Google Cloudに携わるエンジニアのキャリア～クラスメソッド会社説明会～

EVENT【4/30（火）リモート】クラスメソッドの会社説明会を開催します

EVENT【5/9（木）】組織的なクラウド統制はじめの一歩

EVENT【4/24（水）リモート】会社員からフリーランスエンジニアへ、変化の時の体験談 [パネルトーク] フリーランストーク#10

実録！Terraform セルフリファクタリング

やりたいこと

初回構築時のコード

変更1 : listからmapに変える

変更2 : VPC毎に書いている箇所をまとめたい

変更3 : モジュール化する

おまけ : モジュール内でもfor_eachすればいいんじゃないの？

感想、学び

イベント

EVENT【5/14（火）】アノテーションの中途向けオンライン会社説明会を開催します

EVENT【5/23（木）】ユースケースに学ぶAWS運用のノウハウ～可視化から統制まで～

EVENT【5/16（木）】Snowflakeを触ってみよう！初めての方向けハンズオンセミナー

EVENT【4/18リモート】クラスメソッドのフリーランスエンジニア会社説明会 〜AWS/システムアーキテクト/Android案件特集〜 を開催します

EVENT【5/10（金） 名古屋】クラスメソッドグループの会社説明会を開催します！

EVENT【4/19（金）】認証機能の開発工数削減をデモで体験！次世代認証基盤サービス『Auth0 by Okta』導入実践ウェビナー

EVENT【4/25（木）リモート】Google Cloudに携わるエンジニアのキャリア～クラスメソッド会社説明会～

EVENT【4/30（火）リモート】クラスメソッドの会社説明会を開催します

EVENT【5/9（木）】組織的なクラウド統制 はじめの一歩

EVENT【4/24（水）リモート】会社員からフリーランスエンジニアへ、変化の時の体験談 [パネルトーク] フリーランストーク#10

関連記事

Terraform でパスと IP を指定して AWS WAF のルールグループを動的に生成する

SnowflakeのリソースをTerraform×GitHub Actionsで管理するための手順をまとめてみた

JenkinsをTerraformで構築してみた(ECS on EC2 + Amazon FSx for NetApp ONTAP)

Cloud9 上で Terraform の実行環境を作る～Amazon Linux 2023 版～

EVENT【4/18リモート】クラスメソッドのフリーランスエンジニア会社説明会〜AWS/システムアーキテクト/Android案件特集〜を開催します

EVENT【5/10（金）名古屋】クラスメソッドグループの会社説明会を開催します！

EVENT【5/9（木）】組織的なクラウド統制はじめの一歩