From 26dac1f6694a8cd9e3d6a064a4b9373e5ec93265 Mon Sep 17 00:00:00 2001 From: Oliver Slater Date: Fri, 9 Oct 2026 02:02:51 +0100 Subject: [PATCH 1/3] feat(cloudwatch): enhance metric alarm module with configurable period and add tests --- .../modules/cloudwatch-metric-alarm/README.md | 14 +- .../modules/cloudwatch-metric-alarm/locals.tf | 25 +++- .../modules/cloudwatch-metric-alarm/main.tf | 43 +++--- .../tests/cloudwatch_metric_alarm.tftest.hcl | 134 ++++++++++++++++++ 4 files changed, 190 insertions(+), 26 deletions(-) create mode 100644 infrastructure/modules/cloudwatch-metric-alarm/tests/cloudwatch_metric_alarm.tftest.hcl diff --git a/infrastructure/modules/cloudwatch-metric-alarm/README.md b/infrastructure/modules/cloudwatch-metric-alarm/README.md index c653cabc..e6afa46c 100644 --- a/infrastructure/modules/cloudwatch-metric-alarm/README.md +++ b/infrastructure/modules/cloudwatch-metric-alarm/README.md @@ -7,7 +7,7 @@ NHS Screening wrapper around the [terraform-aws-modules/CloudWatch/aws](https:// | Control | How it is enforced | | --- | --- | | Naming | Alarm names derived from context.id + alarm suffix | -| Period | Hardcoded to 60 seconds for consistency | +| Period | Defaults to 60 seconds; configurable per alarm | | Statistic | Defaults to Sum; configurable per alarm | | SNS actions | Optional list of topic ARNs for notifications | | Missing data | Defaults to `notBreaching` (safe default) | @@ -33,6 +33,7 @@ module "app_errors_alarm" { evaluation_periods = 2 threshold = 10 statistic = "Sum" + period = 300 } alarm_actions = [module.sns.topic_arn] @@ -53,6 +54,7 @@ module "ecs_task_alarms" { comparison_operator = "GreaterThanThreshold" evaluation_periods = 3 threshold = 80 + period = 60 dimensions = { ServiceName = "my-service" ClusterName = "my-cluster" @@ -64,6 +66,8 @@ module "ecs_task_alarms" { } ``` +`period` is optional and defaults to 60 seconds for each alarm. Set it to 3600 seconds for hourly metrics such as AWS License Manager usage. `actions_enabled` also defaults to `true`; set it to `false` when you want alarms to track state without sending configured alarm actions. SNS alarm, recovery, and insufficient-data destinations are configured separately through the corresponding action inputs. + ### Referencing log metric filter output ```hcl @@ -97,8 +101,8 @@ module "error_alarm" { ## Conventions -- Alarm names use format `{context.id}-alarm` (single) or `{context.id}-multi-alarm` (multi-dimension). -- Period is always 60 seconds for simplicity. +- Alarm names use format `{context.id}-alarm` (single) or `{context.id}-malarm` (multi-dimension). +- Period defaults to 60 seconds and may be overridden independently for each alarm input. - Missing data defaults to `notBreaching` (safe for production). - SNS actions (alarm, ok, insufficient data) are all optional. - At least one of `metric_alarm` or `metric_alarms_by_multiple_dimensions` must be configured (enforced by check block). @@ -121,8 +125,8 @@ No providers. | Name | Source | Version | | ---- | ------ | ------- | -| [metric\_alarm](#module\_metric\_alarm) | terraform-aws-modules/cloudwatch/aws//modules/metric-alarm | 5.7.2 | -| [metric\_alarms\_by\_multiple\_dimensions](#module\_metric\_alarms\_by\_multiple\_dimensions) | terraform-aws-modules/cloudwatch/aws//modules/metric-alarms-by-multiple-dimensions | 5.7.2 | +| [metric\_alarm](#module\_metric\_alarm) | terraform-aws-modules/cloudwatch/aws//modules/metric-alarm | 5.7.3 | +| [metric\_alarms\_by\_multiple\_dimensions](#module\_metric\_alarms\_by\_multiple\_dimensions) | terraform-aws-modules/cloudwatch/aws//modules/metric-alarms-by-multiple-dimensions | 5.7.3 | | [this](#module\_this) | ../tags | n/a | ## Resources diff --git a/infrastructure/modules/cloudwatch-metric-alarm/locals.tf b/infrastructure/modules/cloudwatch-metric-alarm/locals.tf index 245bf71e..2100aec4 100644 --- a/infrastructure/modules/cloudwatch-metric-alarm/locals.tf +++ b/infrastructure/modules/cloudwatch-metric-alarm/locals.tf @@ -1,4 +1,27 @@ locals { single_alarm_name = format("%s-alarm", module.this.id) - multi_dimension_alarm_name = format("%s-multi-alarm", module.this.id) + multi_dimension_alarm_name = format("%s-malarm", module.this.id) + + single_metric_alarm = var.metric_alarm != null ? var.metric_alarm : { + metric_name = "disabled" + namespace = "disabled" + comparison_operator = "GreaterThanThreshold" + evaluation_periods = 1 + threshold = 0 + statistic = "Sum" + period = 60 + actions_enabled = false + } + + multi_dimension_metric_alarm = var.metric_alarms_by_multiple_dimensions != null ? var.metric_alarms_by_multiple_dimensions : { + metric_name = "disabled" + namespace = "disabled" + comparison_operator = "GreaterThanThreshold" + evaluation_periods = 1 + threshold = 0 + statistic = "Sum" + period = 60 + actions_enabled = false + dimensions = {} + } } diff --git a/infrastructure/modules/cloudwatch-metric-alarm/main.tf b/infrastructure/modules/cloudwatch-metric-alarm/main.tf index 20d43dee..0566dd60 100644 --- a/infrastructure/modules/cloudwatch-metric-alarm/main.tf +++ b/infrastructure/modules/cloudwatch-metric-alarm/main.tf @@ -14,20 +14,20 @@ module "metric_alarm" { source = "terraform-aws-modules/cloudwatch/aws//modules/metric-alarm" - version = "5.7.2" + version = "5.7.3" create_metric_alarm = module.this.enabled && var.metric_alarm != null alarm_name = local.single_alarm_name - comparison_operator = var.metric_alarm.comparison_operator - evaluation_periods = var.metric_alarm.evaluation_periods - threshold = var.metric_alarm.threshold - statistic = var.metric_alarm.statistic - period = var.metric_alarm.period - actions_enabled = var.metric_alarm.actions_enabled + comparison_operator = local.single_metric_alarm.comparison_operator + evaluation_periods = local.single_metric_alarm.evaluation_periods + threshold = local.single_metric_alarm.threshold + statistic = local.single_metric_alarm.statistic + period = local.single_metric_alarm.period + actions_enabled = local.single_metric_alarm.actions_enabled - metric_name = var.metric_alarm.metric_name - namespace = var.metric_alarm.namespace + metric_name = local.single_metric_alarm.metric_name + namespace = local.single_metric_alarm.namespace alarm_actions = var.alarm_actions ok_actions = var.ok_actions @@ -39,21 +39,24 @@ module "metric_alarm" { module "metric_alarms_by_multiple_dimensions" { source = "terraform-aws-modules/cloudwatch/aws//modules/metric-alarms-by-multiple-dimensions" - version = "5.7.2" + version = "5.7.3" create_metric_alarm = module.this.enabled && var.metric_alarms_by_multiple_dimensions != null - alarm_name = local.multi_dimension_alarm_name - comparison_operator = var.metric_alarms_by_multiple_dimensions.comparison_operator - evaluation_periods = var.metric_alarms_by_multiple_dimensions.evaluation_periods - threshold = var.metric_alarms_by_multiple_dimensions.threshold - statistic = var.metric_alarms_by_multiple_dimensions.statistic - period = var.metric_alarms_by_multiple_dimensions.period - actions_enabled = var.metric_alarms_by_multiple_dimensions.actions_enabled - dimensions = var.metric_alarms_by_multiple_dimensions.dimensions + alarm_name = local.multi_dimension_alarm_name + alarm_name_delimiter = "-" + comparison_operator = local.multi_dimension_metric_alarm.comparison_operator + evaluation_periods = local.multi_dimension_metric_alarm.evaluation_periods + threshold = local.multi_dimension_metric_alarm.threshold + statistic = local.multi_dimension_metric_alarm.statistic + period = local.multi_dimension_metric_alarm.period + actions_enabled = local.multi_dimension_metric_alarm.actions_enabled + dimensions = { + default = local.multi_dimension_metric_alarm.dimensions + } - metric_name = var.metric_alarms_by_multiple_dimensions.metric_name - namespace = var.metric_alarms_by_multiple_dimensions.namespace + metric_name = local.multi_dimension_metric_alarm.metric_name + namespace = local.multi_dimension_metric_alarm.namespace alarm_actions = var.alarm_actions ok_actions = var.ok_actions diff --git a/infrastructure/modules/cloudwatch-metric-alarm/tests/cloudwatch_metric_alarm.tftest.hcl b/infrastructure/modules/cloudwatch-metric-alarm/tests/cloudwatch_metric_alarm.tftest.hcl new file mode 100644 index 00000000..d9604c35 --- /dev/null +++ b/infrastructure/modules/cloudwatch-metric-alarm/tests/cloudwatch_metric_alarm.tftest.hcl @@ -0,0 +1,134 @@ +mock_provider "aws" { + mock_data "aws_caller_identity" { + defaults = { + account_id = "111111111111" + arn = "arn:aws:iam::111111111111:role/mock" + user_id = "AIDAMOCK" + } + } + + mock_data "aws_iam_session_context" { + defaults = { + issuer_arn = "arn:aws:iam::111111111111:role/mock" + } + } + + mock_data "aws_iam_policy_document" { + defaults = { + json = "{\"Version\":\"2012-10-17\",\"Statement\":[]}" + } + } +} + +variables { + service = "bcss" + project = "bcss" + environment = "test" + stack = "account" + name = "oracle-licence" + label_order = ["service", "environment", "stack", "workspace", "name", "attributes"] +} + +run "single_metric_alarm_only" { + command = apply + + variables { + metric_alarm = { + metric_name = "ExampleMetric" + namespace = "Example/Tests" + comparison_operator = "GreaterThanThreshold" + evaluation_periods = 1 + threshold = 1 + } + } + + assert { + condition = output.cloudwatch_metric_alarm_id != null + error_message = "A configured single metric alarm must return its alarm ID." + } + + assert { + condition = output.cloudwatch_metric_alarms_by_multiple_dimensions_ids == {} + error_message = "The unused multi-dimension alarm output must stay empty when only a single alarm is configured." + } + + assert { + condition = local.single_metric_alarm.period == 60 && local.single_metric_alarm.actions_enabled + error_message = "A single metric alarm must default to a 60-second period with actions enabled." + } +} + +run "multi_dimension_alarm_only" { + command = apply + + variables { + metric_alarms_by_multiple_dimensions = { + metric_name = "ExampleMetric" + namespace = "Example/Tests" + comparison_operator = "GreaterThanThreshold" + evaluation_periods = 1 + threshold = 1 + dimensions = { Family = "integration" } + } + } + + assert { + condition = length(output.cloudwatch_metric_alarms_by_multiple_dimensions_ids) == 1 + error_message = "A configured multi-dimension alarm must return one alarm ID." + } + + assert { + condition = output.cloudwatch_metric_alarm_id == null + error_message = "The unused single-alarm output must stay null when only a multi-dimension alarm is configured." + } + + assert { + condition = local.multi_dimension_metric_alarm.period == 60 && local.multi_dimension_metric_alarm.dimensions["Family"] == "integration" + error_message = "A multi-dimension alarm must preserve its dimensions and default to a 60-second period." + } +} + +run "both_alarm_types_allow_custom_periods_and_disabled_actions" { + command = plan + + variables { + metric_alarm = { + metric_name = "ShortWindowMetric" + namespace = "Example/Tests" + comparison_operator = "GreaterThanThreshold" + evaluation_periods = 1 + threshold = 1 + period = 300 + actions_enabled = false + } + + metric_alarms_by_multiple_dimensions = { + metric_name = "HourlyMetric" + namespace = "Example/Tests" + comparison_operator = "GreaterThanOrEqualToThreshold" + evaluation_periods = 1 + threshold = 80 + period = 3600 + actions_enabled = false + dimensions = { Family = "integration" } + } + } + + assert { + condition = ( + var.metric_alarm != null && + var.metric_alarms_by_multiple_dimensions != null && + local.single_metric_alarm.period == 300 && + !local.single_metric_alarm.actions_enabled && + local.multi_dimension_metric_alarm.period == 3600 && + !local.multi_dimension_metric_alarm.actions_enabled + ) + error_message = "Both alarm inputs must support independent periods and disabled actions." + } +} + +run "at_least_one_alarm_configuration_is_required" { + command = plan + + expect_failures = [check.at_least_one_alarm_configured] +} From df949e663e6e898475d6ab87c4436c9fc7a192aa Mon Sep 17 00:00:00 2001 From: Oliver Slater Date: Fri, 9 Oct 2026 09:28:30 +0100 Subject: [PATCH 2/3] fix(cloudwatch): refactor metric alarm locals to use coalesce for default values --- infrastructure/modules/cloudwatch-metric-alarm/locals.tf | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/infrastructure/modules/cloudwatch-metric-alarm/locals.tf b/infrastructure/modules/cloudwatch-metric-alarm/locals.tf index 2100aec4..e23e770d 100644 --- a/infrastructure/modules/cloudwatch-metric-alarm/locals.tf +++ b/infrastructure/modules/cloudwatch-metric-alarm/locals.tf @@ -2,7 +2,7 @@ locals { single_alarm_name = format("%s-alarm", module.this.id) multi_dimension_alarm_name = format("%s-malarm", module.this.id) - single_metric_alarm = var.metric_alarm != null ? var.metric_alarm : { + single_metric_alarm = coalesce(var.metric_alarm, { metric_name = "disabled" namespace = "disabled" comparison_operator = "GreaterThanThreshold" @@ -11,9 +11,9 @@ locals { statistic = "Sum" period = 60 actions_enabled = false - } + }) - multi_dimension_metric_alarm = var.metric_alarms_by_multiple_dimensions != null ? var.metric_alarms_by_multiple_dimensions : { + multi_dimension_metric_alarm = coalesce(var.metric_alarms_by_multiple_dimensions, { metric_name = "disabled" namespace = "disabled" comparison_operator = "GreaterThanThreshold" @@ -23,5 +23,5 @@ locals { period = 60 actions_enabled = false dimensions = {} - } + }) } From 68ac9cd1f719aed2c3e63b808ffa8698d861d774 Mon Sep 17 00:00:00 2001 From: Oliver Slater Date: Fri, 9 Oct 2026 09:41:19 +0100 Subject: [PATCH 3/3] feat(cloudwatch): update treat_missing_data variable to support 'ignore' and change default value to 'missing' --- .../modules/cloudwatch-metric-alarm/README.md | 2 +- .../modules/cloudwatch-metric-alarm/main.tf | 2 +- .../tests/cloudwatch_metric_alarm.tftest.hcl | 42 +++++++++++++++++++ .../cloudwatch-metric-alarm/variables.tf | 8 ++-- 4 files changed, 48 insertions(+), 6 deletions(-) diff --git a/infrastructure/modules/cloudwatch-metric-alarm/README.md b/infrastructure/modules/cloudwatch-metric-alarm/README.md index e6afa46c..951e18d9 100644 --- a/infrastructure/modules/cloudwatch-metric-alarm/README.md +++ b/infrastructure/modules/cloudwatch-metric-alarm/README.md @@ -172,7 +172,7 @@ No resources. | [tags](#input\_tags) | Additional tags (e.g. `{'BusinessUnit': 'XYZ'}`).
Neither the tag keys nor the tag values will be modified by this module. | `map(string)` | `{}` | no | | [terraform\_source](#input\_terraform\_source) | Source location to record in the Terraform\_source tag. Defaults to the caller module path when not set. | `string` | `null` | no | | [tool](#input\_tool) | The tool used to deploy the resource | `string` | `"Terraform"` | no | -| [treat\_missing\_data](#input\_treat\_missing\_data) | How to handle missing data points: 'notBreaching', 'breaching', 'missing', 'ignoreMetricTime'. | `string` | `"notBreaching"` | no | +| [treat\_missing\_data](#input\_treat\_missing\_data) | How to handle missing data points: 'notBreaching', 'breaching', 'missing', or 'ignore'. | `string` | `"missing"` | no | | [workspace](#input\_workspace) | ID element. The Terraform workspace, to help ensure generated IDs are unique across workspaces | `string` | `null` | no | ## Outputs diff --git a/infrastructure/modules/cloudwatch-metric-alarm/main.tf b/infrastructure/modules/cloudwatch-metric-alarm/main.tf index 0566dd60..a666e645 100644 --- a/infrastructure/modules/cloudwatch-metric-alarm/main.tf +++ b/infrastructure/modules/cloudwatch-metric-alarm/main.tf @@ -6,7 +6,7 @@ # screening platform baseline controls: # # * Naming: derived from context.id + alarm suffix -# * Period: hardcoded to 60 seconds (enforced) +# * Period: defaults to 60 seconds (configurable per alarm) # * Statistic: defaults to Sum (configurable per-alarm) # * Actions: SNS topic ARNs optional for notifications # * Enabled flag: create = module.this.enabled diff --git a/infrastructure/modules/cloudwatch-metric-alarm/tests/cloudwatch_metric_alarm.tftest.hcl b/infrastructure/modules/cloudwatch-metric-alarm/tests/cloudwatch_metric_alarm.tftest.hcl index d9604c35..ca6f2297 100644 --- a/infrastructure/modules/cloudwatch-metric-alarm/tests/cloudwatch_metric_alarm.tftest.hcl +++ b/infrastructure/modules/cloudwatch-metric-alarm/tests/cloudwatch_metric_alarm.tftest.hcl @@ -56,6 +56,11 @@ run "single_metric_alarm_only" { condition = local.single_metric_alarm.period == 60 && local.single_metric_alarm.actions_enabled error_message = "A single metric alarm must default to a 60-second period with actions enabled." } + + assert { + condition = var.treat_missing_data == "missing" + error_message = "Missing data must default to missing when no value is supplied." + } } run "multi_dimension_alarm_only" { @@ -127,6 +132,43 @@ run "both_alarm_types_allow_custom_periods_and_disabled_actions" { } } +run "ignore_missing_data_value_is_accepted" { + command = plan + + variables { + metric_alarm = { + metric_name = "ExampleMetric" + namespace = "Example/Tests" + comparison_operator = "GreaterThanThreshold" + evaluation_periods = 1 + threshold = 1 + } + treat_missing_data = "ignore" + } + + assert { + condition = var.treat_missing_data == "ignore" + error_message = "The CloudWatch-supported ignore value must be accepted for missing data." + } +} + +run "invalid_missing_data_value_is_rejected" { + command = plan + + variables { + metric_alarm = { + metric_name = "ExampleMetric" + namespace = "Example/Tests" + comparison_operator = "GreaterThanThreshold" + evaluation_periods = 1 + threshold = 1 + } + treat_missing_data = "ignoreMetricTime" + } + + expect_failures = [var.treat_missing_data] +} + run "at_least_one_alarm_configuration_is_required" { command = plan diff --git a/infrastructure/modules/cloudwatch-metric-alarm/variables.tf b/infrastructure/modules/cloudwatch-metric-alarm/variables.tf index 3bb2285d..6f1f3a83 100644 --- a/infrastructure/modules/cloudwatch-metric-alarm/variables.tf +++ b/infrastructure/modules/cloudwatch-metric-alarm/variables.tf @@ -77,11 +77,11 @@ variable "insufficient_data_actions" { variable "treat_missing_data" { type = string - default = "notBreaching" - description = "How to handle missing data points: 'notBreaching', 'breaching', 'missing', 'ignoreMetricTime'." + default = "missing" + description = "How to handle missing data points: 'notBreaching', 'breaching', 'missing', or 'ignore'." validation { - condition = contains(["notBreaching", "breaching", "missing", "ignoreMetricTime"], var.treat_missing_data) - error_message = "treat_missing_data must be one of: notBreaching, breaching, missing, ignoreMetricTime." + condition = contains(["notBreaching", "breaching", "missing", "ignore"], var.treat_missing_data) + error_message = "treat_missing_data must be one of: notBreaching, breaching, missing, ignore." } }