2
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

Terraformですらクソコードを生み出してしまったので、お焚き上げ供養させてください

Posted at

どーまん!せーまん!すぐにやりましょ、お焚き上げ(れっつごー)

안녕하신게라!パナソニック コネクト株式会社クラウドソリューション部の加賀です。

この記事は「クソコードお焚き上げの会 Advent Calendar 2025」に、私がTerraform(!)で生み出してしまったクソコードを供養しに来ました。

絶景かな、絶景かな

「AWS SSO1利用ユーザの許可追加に3営業日かかるってマジ?そんな馬鹿な話があるかいな!」
半信半疑で首を突っ込んだ私を待っていたのは、想像を超える絶景でした。
おお、なんと小さきスクロールバーよ。

# Aさんに開発アカウントの管理者権限を割り当てる
resource "aws_ssoadmin_account_assignment" "userA_project999_dev_ADMIN" {
  instance_arn       = "arn:aws:sso:::instance/ssoins-xxxxxxxxxxxxxxxx"
  permission_set_arn = "arn:aws:sso:::permissionSet/ssoins-xxxxxxxxxxxxxxxx/ps-xxxxxxxxxxxxxxxx" # AdministratorAccess

  principal_id   = "xxxx-xxxx-xxxx-xxxx" # AさんのユーザID
  principal_type = "USER"

  target_id   = "111111111111" # project999の開発アカウントID
  target_type = "AWS_ACCOUNT"
}

# Aさんに本番アカウントの閲覧者権限を割り当てる
resource "aws_ssoadmin_account_assignment" "userA_project999_PROD_viewer" {
  # ...以下、ユーザとアカウントと権限の組み合わせの数だけ、無限に続くコピペの嵐...
}

# ...地平線が見えるほどの絶景が続く...

伝説のコピペ運用コード、まさに「人力IaC (Infrastructure as Code)」の極致!
むしろ3営業日は早かったのでは? これはもう、コードではなく怨念です。即刻供養せねば。

そして生まれた輝かしきコード(のはずだった)

当時の管理台帳がExcelで、そこからCSV出力できるとのことだったので、これはチャンスとばかりにリファクタリングに着手。教科書のようなTerraformコードに書き直しました。

resource "aws_ssoadmin_account_assignment" "this" {
  for_each = local.assign_data

  instance_arn       = var.instance_arn
  permission_set_arn = each.value.target_arn

  principal_id   = each.value.guid
  principal_type = "USER"

  target_id   = each.value.account
  target_type = "AWS_ACCOUNT"
}

なんと美しいコードでしょう!(自画自賛)
何万行もあった汗とコピペの結晶が、わずか10行程度に凝縮され、パッと見は完璧に。管理台帳から吐き出されたCSVを読み込むだけで、自動で権限設定が可能に。よしよし。(数千もの既存設定名を変更する大量の terraform import コマンドのバッチを用意するのが地獄でしたが割愛)

申請して翌営業日には反映されるようになりましたとさ。めでたしめでたし。

3ヶ月後、悪夢再び。今度は「過去の自分」が敵になる

しかし、平和も長くは続きませんでした。
3ヶ月後、私のもとに飛び込んできたのは「CSVファイルが読み込めないエラー」の報告。

「え、ちゃんとShift-JISからUTF-8に変換した?」(地味なCSV地雷)「エラーの内容は?」

どれどれ、とコードを覗き込む私。覗き返してくる深淵の三重ループ。

locals {
  assign_data = merge([
    for pk in local.permissions : merge([
      for uk in local.users : {
        for ak in local.aws_accounts : "${pk.権限名}_${ak.AWSID}_${uk.email}" = {
          email      = uk.email
          guid       = lookup(data.aws_identitystore_user.okta, uk.email).id
          account    = ak.AWSID
          permission = pk.権限名
          target_arn = lookup(data.aws_ssoadmin_permission_set.permission_sets, pk.権限名).arn
        } if lookup(ak, pk.権限名, null) != null
      } if lookup(uk, pk.権限名, null) != null
    ]...)
  ]...)
  
  permissions  = csvdecode(file("./csv/AWSSSO_permissions.csv"))
  aws_accounts = csvdecode(file("./csv/AWSSSO_accounts.csv"))
  users        = csvdecode(file("./csv/AWSSSO_users.csv"))
}

(匙を投げる音)

三重ループを書いたの誰だよ? 3ヶ月前の俺だよ!

そう、過去の自分は他人です。そんな自分を恨むしか出来ないのはエンジニアあるあるなのでしょうか。と思いつつ、半泣きでバリデーションを追加し、デバッグの沼に沈んだ結果、衝撃の事実が判明します。
このクソコードのロジックは正しかったのです。すわ一大事、お焚き上げ記事の危機です。

エラーの原因は、入力データのAWSアカウントIDの桁数不足。
管理台帳上では「012345678901」のように先頭が「0」から始まるAWSアカウントIDが正しく表示されていたにも関わらず、CSV出力する際に、Microsoft Excel君がご親切にも「数値」として認識し、先頭の「0」を削ってくれていたのです。

MSさん、ありがとう、良いクスリです。💢

怒りと絶望、そして今までこの問題に遭遇しなかった確率への奇妙な感心が入り混じり、これもまたクソコードの神髄だと悟りました。まとめてお焚き上げです!

クソコードにも『仕組み』にもメスを入れる

コードは正しい、データが間違っていた、その対策を考える。ではなく、本来あるべき管理体制、運用負荷軽減を考えるのが、エンジニアの存在価値。

平たく言えば「いや、待てよ?」と、シンプルに考えればよかったのです。(3ヶ月前に)

この短いクソコードには、1つのリソース定義に、3つの管理軸である利用ユーザ・AWSアカウント・権限セットの全てを押し込めており、三重ループになるのは必定だったのです。

リファクタリングのタイミングで、コードだけでなくもっと本質的な部分にメスを入れるべきでした。つまり、実際の運用を想像してみることです。ユーザ増減の依頼の都度都度、この三重ループのterraformコードが全件回ります。数も膨大なためplan/applyに要する時間も無視できません。そもそも全件回す必然性が無かったのでは?という発想を持つべきだったのです。

そうと決めれば話は早い。
運用観点に立ち、リソース定義に線引きを行いました。更新頻度の高いユーザー増減は、そのグループに所属させる形に分け、頻度の低いAWSアカウントと権限セットはグループに対して許可します。これによりリソース定義が分離され、plan/applyの時間が短縮。
管理台帳上も「ユーザは必ずグループに所属する」という原則を追加し、そのグループに対してAWSアカウントと権限セットを適用する形式に変更。AWSアカウントIDの先頭「0」出力対策、CSVファイルの日本語を廃して文字コード変換の作業を廃止。

これらの方針変更に合わせてコードを再々リファクタリング。
(ユーザグループのコードは一部掲載不可の部分があったため省略しています)

locals {
  csv = csvdecode(file("./csv/AWSSSO_group_accounts.csv"))
  group_accounts = { for i in csv : join("_", [i.permission_set, i.group_name, i.aws_id]) => i }
}
check "aws_id_is_12_digits" {
  assert {
    condition     = alltrue([ for i in csv : length(i.aws_id) == 12 ])
    error_message = "[注意] AWSアカウントIDが12桁になっていないデータが入っています"
  }
}

resource "aws_ssoadmin_account_assignment" "this" {
  for_each = local.group_accounts

  instance_arn       = var.instance_arn
  permission_set_arn = aws_ssoadmin_permission_set.this[each.value.permission_set].arn

  principal_id   = local.idp_group[each.value.group_name].group_id # IdP側のグループIDを参照
  principal_type = "GROUP"                                         # "USER"から変わりました!

  target_id   = each.value.aws_id
  target_type = "AWS_ACCOUNT"
}

CSVファイルの内容と作成するリソースがリンクし、スッキリ!
ユーザとグループはIdP側のTerraformへと分かれ、そちらもCSVの内容とリンクしてスッキリ!
Excel側で担保しているはずの12桁じゃないAWSアカウントID入力に警告文を出し、転ばぬ先の杖。
terraform plan/applyに掛かる時間も1/10以下に短縮されました。

お焚き上げ完了

Terraformの文法は確かに強力です。しかし、その力を過信し、システム本来の責務範囲を超えて複雑なロジックを実装しようとすると、あっという間に保守不能なクソコードが生まれてしまうことを痛感しました。

今回の供養から得られた教訓は、これに尽きます。

複雑なコードが生まれそうな時、まず疑うべきは「コードの書き方」ではなく、「アプローチ」そのものである。

技術的なテクニックをこねくり回す前に、一歩引いて「もっとシンプルな解決策はないか?」「クラウドのマネージドサービスや機能で代替できないか?」「そもそも、その要件は正しいのか?」と多角的に考えること。それが、私たちをクソコードという地獄から救い出し、真のIaCの恩恵をもたらしてくれるはずです。

お役目を果たしてくれたクソコードたちに感謝し、次なる場所へお還しいたします。


お断り
記事内容は個人の見解であり、所属組織の立場や戦略・意見を代表するものではありません。記事化するにあたり、社内事情をボカし、分かりやすさを優先するため、内容を一部脚色・簡略化しています。
あくまでエンジニアとしての経験や考えを発信していますので、ご了承ください。

  1. 現「AWS IAM Identity Center」ですが、言いやすいのと意味がそのままで伝わり易いので、弊社内では旧名「AWS SSO」と呼ぶ人が多いです。

2
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
2
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?