diff --git a/infrastructure/modules/eventbridge/README.md b/infrastructure/modules/eventbridge/README.md index f2a37b5b..f858fc6a 100644 --- a/infrastructure/modules/eventbridge/README.md +++ b/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 `////` 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::`) +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=" + + 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 `-` (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,7 +278,7 @@ Wildcards and aliases are not accepted for this policy. | [policy](#input\_policy) | An additional policy document ARN to attach to IAM role | `string` | `null` | no | | [policy\_json](#input\_policy\_json) | An additional policy document as JSON to attach to IAM role | `string` | `null` | no | | [policy\_jsons](#input\_policy\_jsons) | List of additional policy documents as JSON to attach to IAM role | `list(string)` | `[]` | no | -| [policy\_path](#input\_policy\_path) | Path of IAM policy to use for EventBridge | `string` | `null` | no | +| [policy\_path](#input\_policy\_path) | Path of IAM policy to use for EventBridge. Defaults to `////` derived from context. | `string` | `null` | no | | [policy\_statements](#input\_policy\_statements) | Map of dynamic policy statements to attach to IAM role

The type should really be

map(object({
sid = optional(string)
effect = optional(string)
actions = optional(list(string))
not\_actions = optional(list(string))
resources = optional(list(string))
not\_resources = optional(list(string))
principals = optional(any)
not\_principals = optional(any)
condition = optional(any)
}))

but it causes problems in the community module when Terraform sets
omitted fields to null. | `any` | `{}` | no | | [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 | | [public\_facing](#input\_public\_facing) | Whether this resource is public facing | `bool` | `false` | no | @@ -236,8 +286,8 @@ Wildcards and aliases are not accepted for this policy. | [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 | | [role\_description](#input\_role\_description) | Description of IAM role to use for EventBridge | `string` | `null` | no | | [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 | -| [role\_name](#input\_role\_name) | Name of IAM role to use for EventBridge | `string` | `null` | no | -| [role\_path](#input\_role\_path) | Path of IAM role to use for EventBridge | `string` | `null` | no | +| [role\_name](#input\_role\_name) | Name of IAM role to use for EventBridge. Defaults to the context ID. | `string` | `null` | no | +| [role\_path](#input\_role\_path) | Path of IAM role to use for EventBridge. Defaults to `////` derived from context. | `string` | `null` | no | | [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 | | [role\_tags](#input\_role\_tags) | A map of tags to assign to IAM role | `map(string)` | `{}` | no | | [rules](#input\_rules) | A map of objects with EventBridge Rule definitions.

The type should really be

map(object({
name\_prefix = optional(string)
description = optional(string)
event\_pattern = optional(string)
schedule\_expression = optional(string)
role\_arn = optional(bool) # the underlying module uses the role created by the wrapped module if true, or null if false
enabled = optional(bool)
state = optional(string)
force\_destroy = optional(bool)
}))

but it causes problems in the community module when Terraform sets
omitted fields to null. | `map(any)` | `{}` | no | diff --git a/infrastructure/modules/eventbridge/locals.tf b/infrastructure/modules/eventbridge/locals.tf index afd037fd..3463522d 100644 --- a/infrastructure/modules/eventbridge/locals.tf +++ b/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, diff --git a/infrastructure/modules/eventbridge/main.tf b/infrastructure/modules/eventbridge/main.tf index 8324e018..813e5f51 100644 --- a/infrastructure/modules/eventbridge/main.tf +++ b/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) diff --git a/infrastructure/modules/eventbridge/tests/eventbridge.tftest.hcl b/infrastructure/modules/eventbridge/tests/eventbridge.tftest.hcl new file mode 100644 index 00000000..39fb26b6 --- /dev/null +++ b/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 --schedule." + } +} diff --git a/infrastructure/modules/eventbridge/variables.tf b/infrastructure/modules/eventbridge/variables.tf index 29606d17..bbabf9b5 100644 --- a/infrastructure/modules/eventbridge/variables.tf +++ b/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 `////` 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 `////` 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" {