From e3dcb64d4539540a27d0dcaa944e9e6c1d4e7b7f Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 28 Sep 2026 00:50:24 +0200 Subject: [PATCH] fix(iac/aws): point Lambda at its own log group via logging_config aws_lambda_function.main had no logging_config, so Lambda wrote to its own auto-created /aws/lambda/ group regardless of what aws_cloudwatch_log_group.lambda's name_prefix produced. That left var.log_retention_days applied to an empty, unreferenced group while production logs went to the auto-created group with "Never expire" retention, and made the migration-failure metric filter (migration-alarm.tf) attach to the same empty group, so it could never fire on a failed migration. An earlier revision of this fix changed the log group to an exact name = "/aws/lambda/${var.stack_name}-api" to match the fixed name Lambda writes to by default. That is wrong for this repo's actual deployed state: the fixed-name group already exists in every environment (Lambda auto-created it before this module ever managed a log group), so Terraform creating a same-named resource fails with ResourceAlreadyExistsException on the very next deploy (deploy-aws-lambda.yml auto-applies on push to main). The previous revision's `terraform import` runbook does not help either -- there is nothing to import into for a resource address that was never in this module's state. Fix (zero operator steps): keep aws_cloudwatch_log_group.lambda as name_prefix (a distinct, Terraform-owned group, not the colliding fixed name) and add logging_config { log_format = "Text", log_group = aws_cloudwatch_log_group.lambda.name } to aws_lambda_function.main, so Lambda writes to the group Terraform manages regardless of the group's actual name. The log group's name_prefix stays the literal "${var.stack_name}-api-" (not aws_lambda_function.main.function_name), since the function now depends on the log group via logging_config and a reference back the other way would cycle; dropped the depends_on that literal used to need. requires aws provider >= 5.32 for logging_config; this module's constraint (~> 5.0) already resolves to 5.100.0 per the committed lock file. The old auto-created /aws/lambda/-api group is simply abandoned by this change: its history is preserved, and it can be deleted manually later once it's no longer needed for reference. No ci-cd-permissions bootstrap re-apply and no manual import step is required. Regression test: terraform/modules/compute/aws/lambda_log_group_test.go now asserts aws_cloudwatch_log_group.lambda uses name_prefix (not an exact name, which would collide with the pre-existing group), that its name_prefix is the literal "${var.stack_name}-api-", that aws_lambda_function.main has a logging_config block referencing aws_cloudwatch_log_group.lambda.name, and that the metric filter still attaches to that same group. Verified it fails against the actual current origin/main source (git show origin/main:.../lambda/main.tf swapped in, test left in place) with the logging_config and name_prefix-literal assertions both failing, and passes with this fix restored. Closes #126 Co-Authored-By: claude-flow --- terraform/modules/compute/aws/lambda/main.tf | 28 +++++++- .../compute/aws/lambda_log_group_test.go | 70 +++++++++++++++++++ 2 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 terraform/modules/compute/aws/lambda_log_group_test.go diff --git a/terraform/modules/compute/aws/lambda/main.tf b/terraform/modules/compute/aws/lambda/main.tf index acbdbd4a..2942497c 100644 --- a/terraform/modules/compute/aws/lambda/main.tf +++ b/terraform/modules/compute/aws/lambda/main.tf @@ -108,6 +108,15 @@ resource "aws_lambda_function" "main" { reserved_concurrent_executions = var.reserved_concurrent_executions tags = var.tags + + # Points Lambda at Terraform's own log group (see aws_cloudwatch_log_group.lambda + # below) instead of letting Lambda auto-create /aws/lambda/ + # itself with "Never expire" retention on first invocation (#126). The + # reference here is the dependency; no depends_on needed. + logging_config { + log_format = "Text" + log_group = aws_cloudwatch_log_group.lambda.name + } } # ============================================== @@ -517,8 +526,25 @@ resource "aws_iam_role_policy" "org_discovery" { # CloudWatch Log Group # ============================================== +# Kept as name_prefix, deliberately not the exact /aws/lambda/ +# name: that fixed name was already auto-created by Lambda in every deployed +# environment (with "Never expire" retention) before this module ever +# managed a log group, so a same-named Terraform resource would collide with +# it (ResourceAlreadyExistsException on create -- there is nothing to import +# into, since the auto-created group was never in this module's state +# either). Terraform instead owns a distinct, prefixed group, and +# aws_lambda_function.main.logging_config (above) points Lambda at it +# explicitly -- Lambda writes wherever logging_config says, not only to the +# fixed default name. The old auto-created group is simply abandoned (its +# history is preserved; delete it manually later once it's no longer +# needed). +# +# Literal ${var.stack_name}-api (not aws_lambda_function.main.function_name) +# so this resource has no dependency on the function -- aws_lambda_function.main +# already depends on THIS resource via logging_config.log_group, and a +# reference back the other way would be a cycle. resource "aws_cloudwatch_log_group" "lambda" { - name_prefix = "/aws/lambda/${aws_lambda_function.main.function_name}-" + name_prefix = "/aws/lambda/${var.stack_name}-api-" retention_in_days = var.log_retention_days tags = var.tags diff --git a/terraform/modules/compute/aws/lambda_log_group_test.go b/terraform/modules/compute/aws/lambda_log_group_test.go new file mode 100644 index 00000000..beeaf68e --- /dev/null +++ b/terraform/modules/compute/aws/lambda_log_group_test.go @@ -0,0 +1,70 @@ +// Regression test for issue #126: aws_lambda_function.main had no +// logging_config, so Lambda wrote to its own auto-created +// /aws/lambda/ group (retention "Never expire") regardless of +// what aws_cloudwatch_log_group.lambda's name_prefix produced. That left +// var.log_retention_days applied to an empty, unreferenced group, and made +// the migration-failure metric filter (migration-alarm.tf) match nothing +// because it attached to the same empty group. +// +// Fix keeps name_prefix (an exact "name" would collide with the group Lambda +// already auto-created in every deployed environment -- ResourceAlreadyExistsException, +// with nothing to import into) and instead points Lambda at the +// Terraform-managed group explicitly via logging_config. +package aws_test + +import ( + "os" + "regexp" + "testing" +) + +// lambdaLogGroupBlockPattern isolates the aws_cloudwatch_log_group "lambda" +// resource block so assertions below can't accidentally match an unrelated +// resource elsewhere in the file. +var lambdaLogGroupBlockPattern = regexp.MustCompile(`(?s)resource\s+"aws_cloudwatch_log_group"\s+"lambda"\s*\{.*?\n\}`) + +// lambdaFunctionBlockPattern isolates aws_lambda_function.main. Matches up to +// the first top-level "}\n\n" after the opening brace, which is safe here +// because the resource's nested blocks (environment, vpc_config, +// logging_config) never contain a blank line before their own closing brace. +var lambdaFunctionBlockPattern = regexp.MustCompile(`(?s)resource\s+"aws_lambda_function"\s+"main"\s*\{.*?\n\}\n`) + +func TestLambdaLogGroupUsesExactName(t *testing.T) { + data, err := os.ReadFile("lambda/main.tf") + if err != nil { + t.Fatalf("reading lambda/main.tf: %v", err) + } + content := string(data) + + logGroupBlock := lambdaLogGroupBlockPattern.FindString(content) + if logGroupBlock == "" { + t.Fatal(`lambda/main.tf has no resource "aws_cloudwatch_log_group" "lambda" block; the log group Lambda writes to must be Terraform-managed so retention_in_days and the migration-failure metric filter apply to it`) + } + + if regexp.MustCompile(`\bname\s*=\s*"`).MatchString(logGroupBlock) { + t.Error(`aws_cloudwatch_log_group.lambda sets an exact "name". That collides with the fixed /aws/lambda/ group Lambda already auto-created in every deployed environment before this module managed a log group (ResourceAlreadyExistsException, nothing to import into). Use name_prefix and point Lambda at it via logging_config instead.`) + } + + if !regexp.MustCompile(`\bname_prefix\s*=\s*"/aws/lambda/\$\{var\.stack_name\}-api-"`).MatchString(logGroupBlock) { + t.Error(`aws_cloudwatch_log_group.lambda must set name_prefix = "/aws/lambda/${var.stack_name}-api-", a literal not derived from aws_lambda_function.main (deriving it from the function would cycle against aws_lambda_function.main.logging_config referencing this group)`) + } + + functionBlock := lambdaFunctionBlockPattern.FindString(content) + if functionBlock == "" { + t.Fatal(`lambda/main.tf has no resource "aws_lambda_function" "main" block`) + } + + if !regexp.MustCompile(`logging_config\s*\{[^}]*log_group\s*=\s*aws_cloudwatch_log_group\.lambda\.name\b`).MatchString(functionBlock) { + t.Error(`aws_lambda_function.main must have a logging_config block with log_group = aws_cloudwatch_log_group.lambda.name (issue #126), or Lambda writes to its own auto-created default group regardless of what aws_cloudwatch_log_group.lambda's name_prefix produces`) + } + + // The metric filter must attach to that same Terraform-managed group, not + // re-derive its own name (which would silently drift from the check above). + alarmData, err := os.ReadFile("lambda/migration-alarm.tf") + if err != nil { + t.Fatalf("reading lambda/migration-alarm.tf: %v", err) + } + if !regexp.MustCompile(`log_group_name\s*=\s*aws_cloudwatch_log_group\.lambda\.name\b`).MatchString(string(alarmData)) { + t.Error(`aws_cloudwatch_log_metric_filter.migration_failed must set log_group_name = aws_cloudwatch_log_group.lambda.name, so the migration-failure alarm (issue #126) attaches to the exact group Lambda writes to`) + } +}