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

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

#Terraform
#Amazon VPC
#AWS
かずえ

かずえ

facebook logohatena logotwitter logo
Clock Icon2019.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"]
}

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すればいいんじゃないの？

こんな感じ

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の利便性を改めて実感しました。

「変更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階層作成し、ネストループのようなことが実現できました。当たり前といえば当たり前のことではありますが、個人的には今回のリファクタリングでこの部分の理解が深まりとても良かったです。

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

Share this article

facebook logohatena logotwitter logo

関連記事

既存のリソース上にTerraformでリソースを追加してみた

既存のリソース上にTerraformでリソースを追加してみた

User avatar
よなみね
2024.11.13
EKS Blueprints for Terraformがversion 5になって変わったこと

EKS Blueprints for Terraformがversion 5になって変わったこと

User avatar
かずえ
2024.10.31
【登壇資料】JAWS-UG名古屋で「色々なIaCツールを実際に触って比較してみる」というタイトルで登壇しました #jawsug_nagoya

【登壇資料】JAWS-UG名古屋で「色々なIaCツールを実際に触って比較してみる」というタイトルで登壇しました #jawsug_nagoya

User avatar
入井啓太
2024.10.30
[Terraform]Aurora MySQLのログ出力先ログクラスをInfrequent Accessにしてみた

[Terraform]Aurora MySQLのログ出力先ログクラスをInfrequent Accessにしてみた

User avatar
神野 雄大
2024.10.29
Classmethod's white logo
Facebook's logo
X's logo
YouTube's logo

© Classmethod, Inc. All rights reserved.