Single CloudWatch logging role for API Gateways (#89)

Create single CloudWatch log role for API Gateways API Gateway's CloudWatch logging role is account- and region-specific. This moves that role's creation to a single place rather than in both the cirrus and stac-server module as that leads to constant state drift.
Element84 · Jan 31, 2025 · 3ae1b4f · 3ae1b4f
1 parent 1bddd7f
commit 3ae1b4f
Show file tree

Hide file tree

Showing 9 changed files with 89 additions and 116 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
   - Pushing messages to the Cirrus `publish` SNS topic
   - Creating objects in the Cirrus `payload` S3 bucket
 - Fixed Cirrus workflow state machine permissions to allow creating state transition events
+- Fixed constant state drift caused by multiple `aws_api_gateway_account` resources (one in `stac-server`, one in `cirrus`)
 
 ### Removed
 

diff --git a/modules/base_infra/api_gateway_account/api_gateway_account.tf b/modules/base_infra/api_gateway_account/api_gateway_account.tf
@@ -0,0 +1,57 @@
+locals {
+  name_prefix = "fd-${var.project_name}-${var.environment}"
+}
+
+
+resource "aws_iam_role" "shared_api_gw_logging_role" {
+  name_prefix = "${local.name_prefix}-${data.aws_region.current.name}-apigw-"
+
+  assume_role_policy = <<-JSON_POLICY_STRING
+  {
+    "Version": "2012-10-17",
+    "Statement": [
+      {
+        "Action": "sts:AssumeRole",
+        "Principal": {
+          "Service": "apigateway.amazonaws.com"
+        },
+        "Effect": "Allow"
+      }
+    ]
+  }
+  JSON_POLICY_STRING
+}
+
+resource "aws_iam_policy" "shared_api_gw_logging_role" {
+  name_prefix = "${local.name_prefix}-${data.aws_region.current.name}-apigw-log-policy-"
+
+  policy = <<-JSON_POLICY_STRING
+  {
+    "Version": "2012-10-17",
+    "Statement": [
+      {
+        "Action": [
+          "logs:CreateLogGroup",
+          "logs:CreateLogStream",
+          "logs:DescribeLogGroups",
+          "logs:DescribeLogStreams",
+          "logs:PutLogEvents",
+          "logs:GetLogEvents",
+          "logs:FilterLogEvents"
+        ],
+        "Resource": "*",
+        "Effect": "Allow"
+      }
+    ]
+  }
+  JSON_POLICY_STRING
+}
+
+resource "aws_iam_role_policy_attachment" "shared_api_gw_logging_role" {
+  role       = aws_iam_role.shared_api_gw_logging_role.name
+  policy_arn = aws_iam_policy.shared_api_gw_logging_role.arn
+}
+
+resource "aws_api_gateway_account" "shared_api_gw_logging_role" {
+  cloudwatch_role_arn = aws_iam_role.shared_api_gw_logging_role.arn
+}
diff --git a/modules/base_infra/api_gateway_account/data.tf b/modules/base_infra/api_gateway_account/data.tf
@@ -0,0 +1 @@
+data "aws_region" "current" {}
diff --git a/modules/base_infra/api_gateway_account/inputs.tf b/modules/base_infra/api_gateway_account/inputs.tf
@@ -0,0 +1,9 @@
+variable "project_name" {
+  description = "Project Name"
+  type        = string
+}
+
+variable "environment" {
+  description = "Project environment"
+  type        = string
+}
diff --git a/modules/base_infra/api_gateway_account/providers.tf b/modules/base_infra/api_gateway_account/providers.tf
@@ -0,0 +1,9 @@
+terraform {
+  required_version = ">= 1.6.6, < 1.8.0"
+  required_providers {
+    aws = {
+      source  = "hashicorp/aws"
+      version = "~> 5.22"
+    }
+  }
+}
diff --git a/modules/cirrus/builtin-functions/api.tf b/modules/cirrus/builtin-functions/api.tf
@@ -290,10 +290,6 @@ aws apigateway update-stage --rest-api-id ${aws_api_gateway_deployment.cirrus_ap
 
 EOF
   }
-
-  depends_on = [
-    aws_api_gateway_account.cirrus_api_gateway_cw_role
-  ]
 }
 
 resource "aws_lambda_permission" "cirrus_api_gateway_lambda_permission_root_resource" {
@@ -314,60 +310,6 @@ resource "aws_lambda_permission" "cirrus_api_gateway_lambda_permission_proxy_res
   source_arn = "arn:aws:execute-api:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:${aws_api_gateway_rest_api.cirrus_api_gateway.id}/*/*${aws_api_gateway_resource.cirrus_api_gateway_proxy_resource.path}"
 }
 
-resource "aws_api_gateway_account" "cirrus_api_gateway_cw_role" {
-  cloudwatch_role_arn = aws_iam_role.cirrus_api_gw_role.arn
-}
-
-resource "aws_iam_role" "cirrus_api_gw_role" {
-  name_prefix = "${var.cirrus_prefix}-${data.aws_region.current.name}"
-
-  assume_role_policy = <<EOF
-{
-  "Version": "2012-10-17",
-  "Statement": [
-    {
-      "Action": "sts:AssumeRole",
-      "Principal": {
-        "Service": "apigateway.amazonaws.com"
-      },
-      "Effect": "Allow"
-    }
-  ]
-}
-EOF
-}
-
-resource "aws_iam_policy" "cirrus_api_gw_policy" {
-  name_prefix = "${var.cirrus_prefix}-${data.aws_region.current.name}-apigw"
-
-  policy = <<EOF
-{
-    "Version": "2012-10-17",
-    "Statement": [
-        {
-            "Action": [
-              "logs:CreateLogGroup",
-              "logs:CreateLogStream",
-              "logs:DescribeLogGroups",
-              "logs:DescribeLogStreams",
-              "logs:PutLogEvents",
-              "logs:GetLogEvents",
-              "logs:FilterLogEvents"
-            ],
-            "Resource": "*",
-            "Effect": "Allow"
-        }
-    ]
-}
-EOF
-
-}
-
-resource "aws_iam_role_policy_attachment" "cirrus_api_gw_base_policy" {
-  role       = aws_iam_role.cirrus_api_gw_role.name
-  policy_arn = aws_iam_policy.cirrus_api_gw_policy.arn
-}
-
 resource "aws_cloudwatch_metric_alarm" "cirrus_api_lambda_errors_warning_alarm" {
   count                     = var.deploy_alarms ? 1 : 0
   alarm_name                = "WARNING: ${var.cirrus_prefix}-api Lambda Errors Warning Alarm"

diff --git a/modules/stac-server/api.tf b/modules/stac-server/api.tf
@@ -303,10 +303,6 @@ aws apigateway update-stage --rest-api-id ${aws_api_gateway_deployment.stac_serv
 
 EOF
   }
-
-  depends_on = [
-    aws_api_gateway_account.stac_server_api_gateway_cw_role
-  ]
 }
 
 resource "aws_lambda_permission" "stac_server_api_gateway_lambda_permission_root_resource" {
@@ -326,7 +322,3 @@ resource "aws_lambda_permission" "stac_server_api_gateway_lambda_permission_prox
 
   source_arn = "arn:aws:execute-api:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:${aws_api_gateway_rest_api.stac_server_api_gateway.id}/*/*${aws_api_gateway_resource.stac_server_api_gateway_proxy_resource.path}"
 }
-
-resource "aws_api_gateway_account" "stac_server_api_gateway_cw_role" {
-  cloudwatch_role_arn = aws_iam_role.stac_api_gw_role.arn
-}
diff --git a/modules/stac-server/iam.tf b/modules/stac-server/iam.tf
@@ -109,53 +109,3 @@ resource "aws_iam_role_policy_attachment" "stac_api_lambda_base_policy" {
   role       = aws_iam_role.stac_api_lambda_role.name
   policy_arn = aws_iam_policy.stac_api_lambda_policy.arn
 }
-
-resource "aws_iam_role" "stac_api_gw_role" {
-  name_prefix = "${local.name_prefix}-stac-server-${data.aws_region.current.name}"
-
-  assume_role_policy = <<EOF
-{
-  "Version": "2012-10-17",
-  "Statement": [
-    {
-      "Action": "sts:AssumeRole",
-      "Principal": {
-        "Service": "apigateway.amazonaws.com"
-      },
-      "Effect": "Allow"
-    }
-  ]
-}
-EOF
-}
-
-resource "aws_iam_policy" "stac_api_gw_policy" {
-  name_prefix = "${local.name_prefix}-stac-server-${data.aws_region.current.name}-apigw"
-
-  policy = <<EOF
-{
-    "Version": "2012-10-17",
-    "Statement": [
-        {
-            "Action": [
-              "logs:CreateLogGroup",
-              "logs:CreateLogStream",
-              "logs:DescribeLogGroups",
-              "logs:DescribeLogStreams",
-              "logs:PutLogEvents",
-              "logs:GetLogEvents",
-              "logs:FilterLogEvents"
-            ],
-            "Resource": "*",
-            "Effect": "Allow"
-        }
-    ]
-}
-EOF
-
-}
-
-resource "aws_iam_role_policy_attachment" "stac_api_gw_base_policy" {
-  role       = aws_iam_role.stac_api_gw_role.name
-  policy_arn = aws_iam_policy.stac_api_gw_policy.arn
-}
diff --git a/profiles/base/main.tf b/profiles/base/main.tf
@@ -21,6 +21,18 @@ module "filmdrop_vpc" {
   archive_log_bucket_name      = var.deploy_log_archive ? module.filmdrop_log_archive[0].s3_logs_archive_bucket : var.s3_logs_archive_bucket
 }
 
+module "api_gateway_account" {
+  # Create the single API Gateway account resource for setting log permissions.
+  # This is only needed once per account per region with API Gateway resources
+  # that need to manage logging. Having multiple within the same account and
+  # region (e.g., one for both stac-server and cirrus) would lead to constant
+  # state drift as Terraform will continuously switch which one is used.
+  source = "../../modules/base_infra/api_gateway_account"
+
+  environment  = var.environment
+  project_name = var.project_name
+}
+
 module "sns_alarm_topics" {
   source = "../../modules/base_infra/sns"