Skip to content

CAB-4856: Apply master-branch kno2-discharge-summary Terraform and check for drift#37

Merged
irsanchez merged 2 commits intopp_masterfrom
CAB-4856-fixloggroup
Oct 1, 2025
Merged

CAB-4856: Apply master-branch kno2-discharge-summary Terraform and check for drift#37
irsanchez merged 2 commits intopp_masterfrom
CAB-4856-fixloggroup

Conversation

@irsanchez
Copy link
Copy Markdown

@irsanchez irsanchez commented Oct 1, 2025

Link to Ticket: https://hbuco.atlassian.net/browse/CAB-4856

Summary by CodeRabbit

  • New Features
    • Added an optional configuration to customize the CloudWatch Logs subscription filter name for Lambda setups. You can now provide a specific name; if omitted, the system defaults to a function-based name, preserving existing behavior.
    • Introduced a new variable to set this name explicitly, offering better alignment with naming conventions and organizational standards while remaining fully backward compatible.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 1, 2025

Walkthrough

Adds an optional variable to override the CloudWatch Logs subscription filter name and updates the Terraform resource to compute the name via coalesce, defaulting to "${var.function_name}_subscription_filter" when no override is provided. No other behavioral changes.

Changes

Cohort / File(s) Summary
Lambda subscription filter naming override
lambda_function/variables.tf, lambda_function/lambda_function.tf
Introduced variable subscription_filter_name (string, default null). Updated aws_cloudwatch_log_subscription_filter.name from "${var.function_name}_subscription_filter" to coalesce(var.subscription_filter_name, "${var.function_name}_subscription_filter").

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my ears at names so neat,
A coalesce carrot—optional treat!
If none is found, the default’s fine,
Logs hop along in tidy line.
Burrow built with Terraform cheer,
Subscription filters crystal clear. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The pull request title references applying “master-branch kno2-discharge-summary Terraform and check for drift,” which does not correspond to the actual changes of parameterizing the CloudWatch log subscription filter name and adding an override variable; it is therefore misleading and unrelated to the modifications in the diff. Please update the title to concisely describe the key change, for example: “Parameterize CloudWatch log subscription filter name with optional override variable.”
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch CAB-4856-fixloggroup

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
lambda_function/variables.tf (1)

187-191: Add validation to guard against empty overrides.

As written, a caller could pass "", which Terraform treats as non-null, and we would attempt to create a subscription filter with an empty name—CloudWatch rejects that. Please add a validation block so only null or non-empty strings are accepted.

You can enforce this with:

 variable "subscription_filter_name" {
   type        = string
   default     = null
   description = "Optional override for the CloudWatch Logs subscription filter name"
+  validation {
+    condition     = var.subscription_filter_name == null || trim(var.subscription_filter_name) != ""
+    error_message = "subscription_filter_name must be null or a non-empty string."
+  }
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9249947 and 421933c.

📒 Files selected for processing (2)
  • lambda_function/lambda_function.tf (1 hunks)
  • lambda_function/variables.tf (2 hunks)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants