戦いの中で見つけたTerraformアンチパターン

この記事は [NTTテクノクロス Advent Calendar 2022](https://qiita.com/advent-calendar/2022/ntt-tx) の5日目の記事です。

NTTテクノクロスの水谷です。
普段は自社製品開発の現場でAWSまわりのお仕事や、社内向けにAWS研修講師をしています。
社内研修の裏側に関しては、弊社@watanyのJAWS DAYS 2022での登壇内容もぜひご覧ください。
（勝手に宣伝）

https://speakerdeck.com/watany/chi-sok-ke-neng-nashe-nei-yan-xiu-noyun-ying-nixiang-kete

## Terraformに触れるまで
みなさんの関わるプロダクトでは、AWSリソースをどのように作成・管理しているでしょうか。
- AWSらしくCloudFormationやAWS CDK
- カッコよくTerraform
- 手順書を書いてマネジメントコンソールで手動構築

等、さまざまなパターンがあると思います。
長所短所それぞれありますが、私が関わっているプロダクトではオンプレミス→AWS移行に伴いTerraformを採用しました。
IaCを取り入れるにしても、なぜCloudFormationやCDKでなかったのかについては色々理由がありますが、

- CloudFormation: 自分は結構書いてきたけど、そこそこ大規模なインフラ構成なのでキツそう[^1]
- CDK: JS(TS)書けなくはないが得意ではない&まだ枯れていなかった感があり、イマイチ[^2]
- Terraform: AWSデキルな人はほぼ0 → AWSサービス/Terraformで学習コスト変わらない？拡張性高そう

のような感触があり、Terraformにしてみよっかなぁ～と思っていたところに
「AWSだけじゃなくてさ、GCPとかAzureとかどうなのよ」
という天の声が聞こえ、IaC手段のベンダーロックインを避ける機運も発生しました。

そんなわけでTerraformに決めてリポジトリの整備を進めてきましたが、
**「ぜんぜんわからない 俺たちは雰囲気でTerraformをやっている」**
状態で進んできたので、今振り返ってみると失敗したなぁとか、イケてないなぁと思う部分が多々出てきます。
今回はそんな内容からいくつか紹介してみようと思います。割と入門編的な内容多めです。

## Anti-patterns
**おことわり: 以降で紹介する内容には多大に脚色を含みます。**

### Case 1. 命名規則大集合
EKS ClusterをTerraformリソースとして管理しています。
Clusterに割り当てるIAM Roleを事前に作成しました。
このIAM Roleには`AmazonEKSClusterPolicy`をアタッチしてやる必要があります。

https://docs.aws.amazon.com/eks/latest/userguide/service_IAM_role.html

```terraform
resource "aws_iam_role" "eks-cluster" {
  name = hoge-eks-cluster-role
 
  assume_role_policy = <<POLICY
{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Effect": "Allow",
      "Principal": {
        "Service": "eks.amazonaws.com"
      },
      "Action": "sts:AssumeRole"
    }
  ]
}
POLICY
}

resource "aws_iam_role_policy_attachment" "eks-cluster-AmazonEKSClusterPolicy" {
  policy_arn = "arn:aws:iam::aws:policy/AmazonEKSClusterPolicy"
  role       = aws_iam_role.eks-cluster.name
```

さて、これらのリソースを含めてEKS Clusterのリソースを定義しました。
（他にもたくさん必要なリソースはありますが、省略しています）

```terraform
resource "aws_eks_cluster" "hoge" {
  depends_on = [
    aws_iam_role_policy_attachment.eks-cluster-AmazonEKSClusterPolicy,
    aws_cloudwatch_log_group.eks-cluster
  ]

  name     = hoge-eks
  version  = var.eks_control_plane.cluster_version
  role_arn = aws_iam_role.eks-cluster.arn
 
  vpc_config {
    endpoint_private_access = true
 
    security_group_ids = [aws_security_group.eks-control-plane.id]
    subnet_ids         = [ for subnet in aws_subnet.public : subnet.id ]
  }
}
```

ここまででどうでしょう。そこはかとない違和感、ありませんか。
それもそのはず、1リソースの記述内に

- スネークケース
- キャメルケース
- ケバブケース

が一堂に会しています。命名規則の見本市。

統一的なコーディング規約を事前に定められず、雰囲気で書き進めてしまったことが一番の原因でした。
たとえば、AWS管理ポリシーの命名規則はキャメルケース（`AmazonEKSClusterPolicy`）なので、
個別のポリシーとTerraformリソース（`aws_iam_role_policy_attachment`）を間違いなく対応させることを意識した結果、そのまま残ってしまっているように思えます。
Terraformではリソース名の変更は実体であるクラウドリソースの再作成を伴うため、後々に与える影響も大きいです。

#### Solution: Terraform書くならスネークケース

やはりTerraformコードを書いていく前に規則を統一する必要がありましたね。
Terraform Best Practicesにはこう書いてありました。

https://www.terraform-best-practices.com/naming

> 1. Use _ (underscore) instead of - (dash) everywhere (in resource names, data source names, variable names, outputs, etc).
> 2. Prefer to use lowercase letters and numbers (even though UTF-8 is supported).

こうなりますね。

```terraform
resource "aws_iam_role" "eks_cluster" {
  ...
}

resource "aws_iam_role_policy_attachment" "eks_cluster_amazon_eks_cluster_policy" {
  ...
}
```

```terraform
resource "aws_eks_cluster" "hoge" {
  depends_on = [
    aws_iam_role_policy_attachment.eks_cluster_amazon_eks_cluster_policy,
    aws_cloudwatch_log_group.eks_cluster
  ]

  name     = hoge-eks
  version  = var.eks_control_plane.cluster_version
  role_arn = aws_iam_role.eks_cluster.arn
 
  vpc_config {
    endpoint_private_access = true
 
    security_group_ids = [aws_security_group.eks_control_plane.id]
    subnet_ids         = [ for subnet in aws_subnet.public : subnet.id ]
  }
}
```

AWSリソース名はケバブケースで書くことが多い気もするのでそのままにしました。
どうでしょうか。リソースの参照部分なども含め、読みやすくなりましたね。

他にも、Terraform Best practicesには命名やコーディング規約の指針になる情報がたくさん揃っています。
このような記述もあって、
> Do not repeat resource type in resource name (not partially, nor completely):

> Resource name should be named `this` if there is no more descriptive and general name available, or if the resource module creates a single resource of this type (eg, in AWS VPC module there is a single resource of type `aws_nat_gateway` and multiple resources of type `aws_route_table`, so `aws_nat_gateway` should be named `this` and `aws_route_table` should have more descriptive names - like `private`, `public`, `database`).

```terraform
resource "aws_eks_cluster" "eks" {
  ...
}
```

なんて書かずに、
```terraform
resource "aws_eks_cluster" "this" {
  ...
}
```

とするのが良さそう。もっと早くに知れればよかった……

### Case 2. mapの乱用がパラメータ設計の負債になりかける
map、便利ですよね。
```terraform:environments/foo/variables.tf
variable "ec2_hoge" {
  type = map(string)
  default = {
    "ami_id"         = "ami-xxxxxxxx"
    "instance_type"  = "t3.medium"
    "key_name"       = "ec2-key-hogehoge"
  }
}
```
複数の変数をmoduleに対して受け渡したい時に、mapでまとめて定義してしまえば、
```terraform:environments/foo/main.tf
module "ec2_hoge" {
  source      = "../../modules/ec2/"
  ec2         = var.ec2_hoge
}
```
こんな感じでまとめてmoduleに引き渡せて、
```terraform:modules/ec2/main.tf
resource "aws_instance" "this" {
  ami           = var.ec2.ami_id
  instance_type = var.ec2.instance_type
  key_name      = var.ec2.key_name
}
```
moduleからもいい感じに参照できますね。実にエレガント。

この時点ではメリットしか見えてこないですが、落とし穴はすぐそこにあります。
たとえば`ec2_hoge`module内で、複数IPアドレスからのインバウンドアクセスを許可するセキュリティグループを作成したくなったらどうすればよいでしょうか。
`whitelisted_ips`みたいなlistを作成してIPアドレスをまとめて持ちたくなりますね。

```terraform:environments/foo/variables.tf
variable "ec2_hoge" {
  type = map(any)
  default = {
    "ami_id"          = "ami-xxxxxxxx"
    "instance_type"   = "t3.medium"
    "key_name"        = "ec2-key-hogehoge"
    "whitelisted_ips" = [
      "227.223.xx.xx/32",
      "180.106.yy.yy/32",
      "237.129.zz.zz/32"
    ]
  }
}
```

でもこうしちゃうと、コケますよね。

```bash
$ terraform plan
╷
│ Error: Invalid default value for variable
│ 
│   on variables.tf line 3, in variable "ec2_hoge":
│    3:   default = {
│    4:     "ami_id"          = "ami-xxxxxxxx"
│    5:     "instance_type"   = "t3.medium"
│    6:     "key_name"        = "ec2-key-hogehoge"
│    7:     "whitelisted_ips" = [
│    8:       "227.223.xx.xx/32",
│    9:       "180.106.yy.yy/32",
│   10:       "237.129.zz.zz/32"
│   11:     ]
│   12:   }
│ 
│ This default value is not compatible with the variable's type constraint: element "whitelisted_ips": string required.
```
`map(any)` はどんなtypeでも受け付けてくれるけれど、すべての要素のtypeが同じでなくてはならないからです。

https://developer.hashicorp.com/terraform/language/expressions/type-constraints

> The keyword map is a shorthand for map(any), which accepts any element type as long as every element is the same type. 

となると、こうやってtype(が異なるパラメータを含むmap)ごとに別々のパラメータとして渡すことになってしまいます。統一感がないですし、カッコよさも台無しです。
```terraform:environments/foo/main.tf
module "ec2_hoge" {
  source      = "../../modules/ec2/"
  ec2             = var.ec2_hoge
  whitelisted_ips = var.whitelisted_ips
}
```
何より、こう見ると`var.ec2_hoge`の中に何が入っているのか何だかよく分からないの、結構怖くないですか？

#### Solution: すべてのパラメータをひとつのmapに詰め込もうとしない
これが現状考えられる解だと思っています。
ポータビリティの高さがmapを使ってパラメータを受け渡すメリットですが、裏を返すと何が渡っているのかが見えづらくなります。
Terraform RegistryにあるいくつかのmoduleのUsageを見ても、**「名前からは中身がなんだか分からないmap」** が現れているものはありませんでした。

https://registry.terraform.io/browse/modules

たとえば [terraform-aws-modules/terraform-aws-iam](https://registry.terraform.io/modules/terraform-aws-modules/iam/aws/latest) の場合。
```terraform:
module "iam_assumable_role_with_oidc" {
  source  = "terraform-aws-modules/iam/aws//modules/iam-assumable-role-with-oidc"

  create_role = true

  role_name = "role-with-oidc"

  tags = {
    Role = "role-with-oidc"
  }

  provider_url = "oidc.eks.eu-west-1.amazonaws.com/id/BA9E170D464AF7B92084EF72A69B9DC8"

  role_policy_arns = [
    "arn:aws:iam::aws:policy/AmazonEKS_CNI_Policy",
  ]
  number_of_role_policy_arns = 1
}
```

こう考えると、moduleを実装する場合には、**ひとつのmoduleで何でもかんでも作ってしまおうとしない**ことも、もしかすると必要なのかもしれません。（リソースを大量に詰め込んだでっかいmoduleを作ってしまっていることへの反省）

### Case 3. サービス単位でモジュール分割するのはいいけれど……
Terraformリポジトリについては、実装初期段階で以下のようなディレクトリ構成を取りました。
※イメージです

- `environments/`: 環境固有/要件により変動しうるパラメータを定義
    - スペックパターン（インスタンスタイプや台数等）に応じて選べるよう、複数をテンプレートとして準備
- `modules/`: AWSリソースを作成するためのmodule群
    - 概ねAWSサービス単位に分割
    - Terraform Registryに公開されているようなmoduleは採用せず、自前で実装

```
├── environments/
│   ├── spec_small/            # 小規模な環境用
│   ├── spec_large/            # 大規模な環境用
│   └── resources/             # templatefile用のファイルを管理(主にEKSのyml)
└── modules
    ├── acm
    ├── alb_internal_client    # VPN接続経由でGUIへ接続するためのALB
    ├── alb_internet_facing    # VPN接続を通さずにGUIへ接続するためのALB
    ├── eks_control_plane
    ├── eks_node
    ├── opensearch
    ├── rds
    ├── route53
    ├── s3
    └── vpc
```
AWSサービス単位で分割するのは分かりやすい&割とよくあるパターンで、一見悪くなさそうですね。
さて、moduleの中身がどうなっているか見ていきましょう。

`alb_internal_client` `alb_internet_facing`は、EKSで起動するサービス向けのfrontend ALBを作成します。
GUIへのアクセスにVPNを通す/通さないの要件がどちらもありうるので、選べるようにmoduleを用意しています。[^3]


```bash
$ tree modules/alb_internal_client
└── alb_internal_client
    ├── load_balancer.tf
    ├── outputs.tf
    ├── security_group.tf
    ├── target_group.tf
    └── variables.tf
```

ふむふむ。`variables.tf`と`outouts.tf`は置いておいて、
- ALB[^4]
- セキュリティグループ
- ターゲットグループ

を作成しているように見えますね。
そういえば、DNSレコードはどこで作ってるんだ……？

!!!
```bash
$ tree modules/route53
└── route53
    ├── hostzone.tf
    ├── outputs.tf
    ├── record_private.tf
    ├── record_public.tf
    └── variables.tf
```
```terraform:modules/route53/record_private.tf
resource "aws_route53_record" "alb_internal_client" {
  for_each = toset(
    [
      "service-a",
      "service-b",
      "service-c"
    ]
  )
  zone_id = aws_route53_zone.private.zone_id
  name    = ${each.key}.hoge.com
  type    = "A"

  alias {
    name                   = var.alb_internet_facing.load_balancer.dns_name
    zone_id                = var.alb_internet_facing.load_balancer.zone_id
    evaluate_target_health = false
}

resource "aws_route53_record" "rds" {
  ...
}

resource "aws_route53_record" "opensearch" {
  ...
}
```

なんと、ALBだけでなく、全サービスのDNSレコードをRoute53用のmoduleでまとめて作成していました。

#### Solution:「サービス単位の分割」に縛られすぎない
リソースを作成していけば、AWSサービス間での依存関係が必ず発生します。
たとえば、EC2インスタンスを1台作成するには、VPCとSubnet, Security Groupが用意されている必要がありますね。
これらの依存関係を図示すると、こうなります。
```
EC2 → Security Group, Subnet → VPC
```

Terraformのmoduleで見ると、以下のように分割するのが良さそう。
```
EC2 → Security Group | Subnet → VPC
```
- EC2, Security Group: EC2インスタンスのmoduleで作成・管理
- Subnet, VPC: VPC(network)のmoduleで作成・管理

綺麗に分けられ、かつ直感的ですね。

ALBとRoute53のDNSレコードについても同様に考えてみましょう。
ALB、 DNSレコード、 Hostzoneの依存関係はこんなところでしょうか。
```
DNS Record → ALB, Hostzone
```

先に見たTerraform moduleの構成から見ると、どこで切ればよいでしょうか……？
現状の構成だと、こう切れていますが、
```
DNS Record → Hostzone | ALB
```

**DNSレコードとALBがライフサイクルを同じくする**[^5]ことを考えると、

```
DNS Record → ALB | Hostzone
```

こう切ったほうが、なんとなく自然な感じがしますよね。
ということは、`aws_route53_record.alb_internal_client`の定義は、
`route53`moduleではなく、`alb`module側に寄せたほうがよさそうです。

**「AWSサービス単位でmodule分割」を大原則としつつ、リソースのライフサイクルに応じてどのmoduleに持たせるかを個別に検討する**ことで、よりスマートなmodule分割ができそうに思えます。
今回の例であればmodule間で受け渡すべきパラメータの数は大差ないですが（ALBのDNS名とゾーンID vs Route53のホストゾーンID）、全体を通して見ていくと大きな違いに繋がりそうです。数だけでなく、全体の見通しがよくなりそう。
現状は単一リポジトリですが、複数のリポジトリに分割する際にも同じ考え方で行けそうですね。

## おわりに
手探りでTerraformを書き始めてから1年ちょっとが経ち、今回挙げた以外にも多くのイケてないを乗り越えてきました。こうした経験を振り返って反省を言語化できるようになったことは、ちょっとした成長だったかなと思います。今後も戦いの中で成長していきたい。
周りにはTerraformerが居らずなかなかこの苦しみを普段は共有できないので、「こんな失敗したよ」「こう乗り越えたよ」みたいな経験談や「わかる」「それな」等々あれば、是非コメントでも聞かせてください。

## 明日は
@yamaguchi_yy449がPostgreSQL15に関する記事を書きます。お楽しみに！

[^1]: 最近はNested Stackとか結構見やすくなったので、今振り返ると割と何とかなっていたのかもしれません。
[^2]: 後々一緒にTerraformを書いていくことになるメンバも同様だったので結果オーライ
[^3]: EKS向けなELBの作成には、AWS Load Balancer Controllerも一部使っています。正直どっちかに寄せたい気持ちもあります。
[^4]: ALBのListenerもここで作っています。
[^5]: もっと正確には、ALBの裏にいるサービスとライフサイクルを同じくしているので、サービス固有のリソースをTerraform moduleで管理しているのであれば、そちらに寄せたほうがより適切。