fix(eventbridge): update role naming and path conventions to default to context ID

infrastructure/modules/eventbridge/README.md

@@ -23,8 +23,8 @@ multiple namespaces.
     in as variables. This causes conflicts when a stack is deployed into
     multiple workspaces in the same AWS account.
 
-    This module prefixes many of these keys with the bus name. The bus name by
-    default includes the workspace id.
+    This module prefixes many of these keys with the context ID, which by
+    default includes the workspace.
 
         Not every upstream resource name is prefixed; check planned names for
         collisions when deploying several workspaces into one account and region.
@@ -38,7 +38,8 @@ credentials contained therein from the terraform state.
 | --- | --- |
 | Encryption at rest | A customer-managed KMS key is required for the bus, and for each configured archive, schedule, and pipe. |
 | SNS KMS access | SNS target policies require specific KMS key ARNs; wildcard keys are rejected. |
-| Naming | The bus defaults to the context ID; connection, destination, pipe, schedule-group, and log-delivery names are scoped to the context. |
+| Naming | The bus and IAM role default to the context ID; connection, destination, pipe, schedule-group, schedule, and log-delivery names are scoped to the context. |
+| IAM path | The role and its policies default to the `/<service>/<project>/<environment>/` path, matching the `iam` and `ecs-service` modules. |
 | Tagging | Bus resources and IAM roles receive context tags. |
 | Creation gate | `module.this.enabled` controls creation of the upstream module. |
 | Secrets protection | Connections are disallowed, as the wrapped module doesn't handle them safely |
@@ -102,10 +103,59 @@ credentials contained therein from the terraform state.
         }
     }
 
+With the context above (`bcss-production-jobs`), this creates the schedule group
+`bcss-production-jobs-nightly-group` and the schedule
+`bcss-production-jobs-nightly-job-schedule`, with the IAM role
+`/bcss/production/bcss-production-jobs`.
+
+### Scheduler on the default bus (no bus created)
+
+Use the default bus when rules must match AWS service events, which are only
+delivered there. Universal targets (`arn:aws:scheduler:::aws-sdk:<service>:<action>`)
+call AWS APIs directly; grant the actions through `policy_json`.
+
+    module "operating_hours" {
+        source = "git::https://github.com/NHSDigital/screening-terraform-modules-aws.git//infrastructure/modules/eventbridge?ref=<approved-release>"
+
+        context = module.this.context
+        name    = "hours"
+
+        create_bus                 = false
+        bus_name                   = "default"
+        create_log_delivery_source = false
+        create_log_delivery        = false
+        # Only used for a module-created bus.
+        kms_key_identifier = module.scheduler_kms.key_arn
+
+        append_schedule_group_postfix = false
+        append_schedule_postfix       = false
+        schedule_groups               = { oracle = {} }
+        schedules = {
+            oracle-stop = {
+                arn                 = "arn:aws:scheduler:::aws-sdk:ec2:stopInstances"
+                input               = jsonencode({ InstanceIds = [module.oracle_ec2.ec2_instance_id] })
+                schedule_expression = "cron(0 19 ? * MON-FRI *)"
+                timezone            = "Europe/London"
+                group_name          = "oracle"
+                kms_key_arn         = module.scheduler_kms.key_arn
+            }
+        }
+
+        attach_policy_json = true
+        policy_json        = data.aws_iam_policy_document.operating_hours.json
+    }
+
+With a context ID of `bcss-test-application-integration-3`, this creates the group
+`bcss-test-application-integration-3-hours-oracle`, the schedule
+`bcss-test-application-integration-3-hours-oracle-stop`, and the role
+`/bcss/bcss/test/bcss-test-application-integration-3-hours`.
+
 ## Conventions
 
 - Pass a pinned release ref and supply the required customer-managed KMS keys.
 - Keys of `schedule_groups` are logical identifiers; schedule `group_name` references a key, not the prefixed AWS name.
+- Keys of `schedules` are logical identifiers; the AWS schedule name is `<context ID>-<key>` (plus `-schedule` when `append_schedule_postfix` is true) and must not exceed 64 characters. Set `append_schedule_postfix = false` when the context ID is long.
+- Set `role_name` only when another resource must reference the role ARN before apply (for example `iam:PassRole` or an SSM `AutomationAssumeRole`); include `role_path` in that ARN.
 - `log_delivery` entries default to context-prefixed names when `name` is omitted or null.
 - Additional `role_tags` are combined with context tags; context values take precedence.
 - Review the plan for resource names, policies, and target permissions before applying.
@@ -228,16 +278,16 @@ Wildcards and aliases are not accepted for this policy.
 | <a name="input_policy"></a> [policy](#input\_policy) | An additional policy document ARN to attach to IAM role | `string` | `null` | no |
 | <a name="input_policy_json"></a> [policy\_json](#input\_policy\_json) | An additional policy document as JSON to attach to IAM role | `string` | `null` | no |
 | <a name="input_policy_jsons"></a> [policy\_jsons](#input\_policy\_jsons) | List of additional policy documents as JSON to attach to IAM role | `list(string)` | `[]` | no |
-| <a name="input_policy_path"></a> [policy\_path](#input\_policy\_path) | Path of IAM policy to use for EventBridge | `string` | `null` | no |
+| <a name="input_policy_path"></a> [policy\_path](#input\_policy\_path) | Path of IAM policy to use for EventBridge. Defaults to `/<service>/<project>/<environment>/` derived from context. | `string` | `null` | no |
 | <a name="input_policy_statements"></a> [policy\_statements](#input\_policy\_statements) | Map of dynamic policy statements to attach to IAM role<br/><br/>The type should really be<br/><br/>  map(object({<br/>    sid            = optional(string)<br/>    effect         = optional(string)<br/>    actions        = optional(list(string))<br/>    not\_actions    = optional(list(string))<br/>    resources      = optional(list(string))<br/>    not\_resources  = optional(list(string))<br/>    principals     = optional(any)<br/>    not\_principals = optional(any)<br/>    condition      = optional(any)<br/>  }))<br/><br/>but it causes problems in the community module when Terraform sets<br/>omitted fields to null. | `any` | `{}` | no |
 | <a name="input_project"></a> [project](#input\_project) | ID element. A project identifier, indicating the name or role of the project the resource is for, such as `website` or `api` | `string` | `null` | no |
 | <a name="input_public_facing"></a> [public\_facing](#input\_public\_facing) | Whether this resource is public facing | `bool` | `false` | no |
 | <a name="input_regex_replace_chars"></a> [regex\_replace\_chars](#input\_regex\_replace\_chars) | Terraform regular expression (regex) string.<br/>Characters matching the regex will be removed from the ID elements.<br/>If not set, `"/[^a-zA-Z0-9-]/"` is used to remove all characters other than hyphens, letters and digits. | `string` | `null` | no |
 | <a name="input_region"></a> [region](#input\_region) | ID element \_(Rarely used, not included by default)\_.  Usually an abbreviation of the selected AWS region e.g. 'uw2', 'ew2' or 'gbl' for resources like IAM roles that have no region | `string` | `null` | no |
 | <a name="input_role_description"></a> [role\_description](#input\_role\_description) | Description of IAM role to use for EventBridge | `string` | `null` | no |
 | <a name="input_role_force_detach_policies"></a> [role\_force\_detach\_policies](#input\_role\_force\_detach\_policies) | Specifies to force detaching any policies the IAM role has before destroying it. | `bool` | `true` | no |
-| <a name="input_role_name"></a> [role\_name](#input\_role\_name) | Name of IAM role to use for EventBridge | `string` | `null` | no |
-| <a name="input_role_path"></a> [role\_path](#input\_role\_path) | Path of IAM role to use for EventBridge | `string` | `null` | no |
+| <a name="input_role_name"></a> [role\_name](#input\_role\_name) | Name of IAM role to use for EventBridge. Defaults to the context ID. | `string` | `null` | no |
+| <a name="input_role_path"></a> [role\_path](#input\_role\_path) | Path of IAM role to use for EventBridge. Defaults to `/<service>/<project>/<environment>/` derived from context. | `string` | `null` | no |
 | <a name="input_role_permissions_boundary"></a> [role\_permissions\_boundary](#input\_role\_permissions\_boundary) | The ARN of the policy that is used to set the permissions boundary for the IAM role used by EventBridge | `string` | `null` | no |
 | <a name="input_role_tags"></a> [role\_tags](#input\_role\_tags) | A map of tags to assign to IAM role | `map(string)` | `{}` | no |
 | <a name="input_rules"></a> [rules](#input\_rules) | A map of objects with EventBridge Rule definitions.<br/><br/>The type should really be<br/><br/>  map(object({<br/>    name\_prefix         = optional(string)<br/>    description         = optional(string)<br/>    event\_pattern       = optional(string)<br/>    schedule\_expression = optional(string)<br/>    role\_arn            = optional(bool) # the underlying module uses the role created by the wrapped module if true, or null if false<br/>    enabled             = optional(bool)<br/>    state               = optional(string)<br/>    force\_destroy       = optional(bool)<br/>  }))<br/><br/>but it causes problems in the community module when Terraform sets<br/>omitted fields to null. | `map(any)` | `{}` | no |

infrastructure/modules/eventbridge/locals.tf

@@ -11,6 +11,17 @@ locals {
     : null
   )
 
+  # The community module otherwise names the role after the bus, giving "default" on the default bus.
+  role_name = module.this.enabled ? coalesce(var.role_name, module.this.id) : null
+
+  # Same default as the iam and ecs-service wrappers, e.g. "/bcss/bcss/test/".
+  default_iam_path = format(
+    "/%s/",
+    join("/", compact([module.this.service, module.this.project, module.this.environment]))
+  )
+  role_path   = coalesce(var.role_path, local.default_iam_path)
+  policy_path = coalesce(var.policy_path, local.default_iam_path)
+
   # log delivery names must be unique per AWS account
   # provide a default name based on the module ID
   log_delivery = {
@@ -33,8 +44,10 @@ locals {
 
   # if a schedule gives a group_name, fix it to match the corresponding
   # name in schedule_groups
+  # schedule names follow the context naming convention, so prefix keys with
+  # the module ID; a group_name key is resolved to the prefixed group name
   schedules = {
-    for k, v in var.schedules : k => (
+    for k, v in var.schedules : "${module.this.id}-${k}" => (
       try(local.schedule_groups[v.group_name], null) != null # either property could be absent
       ? merge(
         v,

infrastructure/modules/eventbridge/main.tf

@@ -53,10 +53,10 @@ module "eventbridge" {
   schedules                      = local.schedules
   pipes                          = local.pipes
   schedule_group_timeouts        = var.schedule_group_timeouts
-  role_name                      = var.role_name
+  role_name                      = local.role_name
   role_description               = var.role_description
-  role_path                      = var.role_path
-  policy_path                    = var.policy_path
+  role_path                      = local.role_path
+  policy_path                    = local.policy_path
   role_force_detach_policies     = var.role_force_detach_policies
   role_permissions_boundary      = var.role_permissions_boundary
   role_tags                      = merge(var.role_tags, module.this.tags)

infrastructure/modules/eventbridge/tests/eventbridge.tftest.hcl

@@ -0,0 +1,109 @@
+mock_provider "aws" {
+  mock_data "aws_caller_identity" {
+    defaults = {
+      arn = "arn:aws:iam::111111111111:role/mock"
+    }
+  }
+
+  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"
+  environment = "test"
+  stack       = "application"
+  name        = "hours"
+  label_order = ["service", "environment", "stack", "workspace", "name", "attributes"]
+
+  create_bus                 = false
+  bus_name                   = "default"
+  create_log_delivery_source = false
+  create_log_delivery        = false
+  kms_key_identifier         = "arn:aws:kms:eu-west-2:111111111111:key/mock"
+  role_name                  = "bcss-test-application-default-hours"
+
+  append_schedule_group_postfix = false
+  schedule_groups = {
+    oracle = {}
+  }
+}
+
+run "schedule_names_are_context_prefixed" {
+  command = plan
+
+  variables {
+    append_schedule_postfix = false
+    schedules = {
+      oracle-start = {
+        arn                 = "arn:aws:scheduler:::aws-sdk:ec2:startInstances"
+        input               = "{\"InstanceIds\":[\"i-123\"]}"
+        schedule_expression = "cron(0 7 ? * MON-FRI *)"
+        group_name          = "oracle"
+        kms_key_arn         = "arn:aws:kms:eu-west-2:111111111111:key/mock"
+      }
+    }
+  }
+
+  assert {
+    condition     = keys(module.eventbridge.eventbridge_schedules) == ["bcss-test-application-default-hours-oracle-start"]
+    error_message = "Schedule names should be prefixed with the context ID."
+  }
+
+  assert {
+    condition     = module.eventbridge.eventbridge_schedules["bcss-test-application-default-hours-oracle-start"].group_name == "bcss-test-application-default-hours-oracle"
+    error_message = "Schedule group_name should resolve to the prefixed schedule group."
+  }
+}
+
+run "role_name_defaults_to_context_id" {
+  command = plan
+
+  variables {
+    role_name = null
+    schedules = {
+      oracle-stop = {
+        arn                 = "arn:aws:scheduler:::aws-sdk:ec2:stopInstances"
+        input               = "{\"InstanceIds\":[\"i-123\"]}"
+        schedule_expression = "cron(0 19 ? * MON-FRI *)"
+        group_name          = "oracle"
+        kms_key_arn         = "arn:aws:kms:eu-west-2:111111111111:key/mock"
+      }
+    }
+  }
+
+  assert {
+    condition     = output.eventbridge_role_name == "bcss-test-application-default-hours"
+    error_message = "The IAM role should default to the context ID, not the bus name."
+  }
+}
+
+run "schedule_postfix_is_appended_after_prefix" {
+  command = plan
+
+  variables {
+    schedules = {
+      oracle-stop = {
+        arn                 = "arn:aws:scheduler:::aws-sdk:ec2:stopInstances"
+        input               = "{\"InstanceIds\":[\"i-123\"]}"
+        schedule_expression = "cron(0 19 ? * MON-FRI *)"
+        group_name          = "oracle"
+        kms_key_arn         = "arn:aws:kms:eu-west-2:111111111111:key/mock"
+      }
+    }
+  }
+
+  assert {
+    condition     = module.eventbridge.eventbridge_schedules["bcss-test-application-default-hours-oracle-stop"].name == "bcss-test-application-default-hours-oracle-stop-schedule"
+    error_message = "With append_schedule_postfix, the name should be <context ID>-<key>-schedule."
+  }
+}

infrastructure/modules/eventbridge/variables.tf

@@ -441,7 +441,7 @@ variable "schedule_group_timeouts" {
 ################################################################
 
 variable "role_name" {
-  description = "Name of IAM role to use for EventBridge"
+  description = "Name of IAM role to use for EventBridge. Defaults to the context ID."
   type        = string
   default     = null
 }
@@ -453,15 +453,25 @@ variable "role_description" {
 }
 
 variable "role_path" {
-  description = "Path of IAM role to use for EventBridge"
+  description = "Path of IAM role to use for EventBridge. Defaults to `/<service>/<project>/<environment>/` derived from context."
   type        = string
   default     = null
+
+  validation {
+    condition     = var.role_path == null || can(regex("^/.*/$", coalesce(var.role_path, "/")))
+    error_message = "role_path must start and end with a forward slash, e.g. \"/bcss/\"."
+  }
 }
 
 variable "policy_path" {
-  description = "Path of IAM policy to use for EventBridge"
+  description = "Path of IAM policy to use for EventBridge. Defaults to `/<service>/<project>/<environment>/` derived from context."
   type        = string
   default     = null
+
+  validation {
+    condition     = var.policy_path == null || can(regex("^/.*/$", coalesce(var.policy_path, "/")))
+    error_message = "policy_path must start and end with a forward slash, e.g. \"/bcss/\"."
+  }
 }
 
 variable "role_force_detach_policies" {