feat(eventbridge): BCSS-24132 - Add terraform-aws-modules/terraform-aws-eventbridge wrapper by dave4420 · Pull Request #119 · NHSDigital/screening-terraform-modules-aws

dave4420 · 2026-09-10T10:15:26Z

Copilot

Copilot review overview

🟡 Changes recommended

Unresolved correctness and security issues include null resource names, wildcard KMS permissions, and unprotected credential inputs.

Review effort: Lite
Findings: 3 · 1 · 3

Open (7)

Prevent null log-delivery names · New
Mark connections variable as sensitive · New
Require specific SNS KMS key ARNs · New
Propagate baseline tags to IAM roles · New
Update module catalogue metadata · New
Add required README guidance and examples · New
Add required EventBridge module header · New

What changed in this PR

Adds an NHS Terraform wrapper for AWS EventBridge and EventBridge Scheduler with shared naming, tagging, KMS configuration, outputs, documentation, and dependency management.

Changes:

Adds EventBridge wrapper configuration, inputs, locals, outputs, and context.
Adds Terraform/provider constraints and lock data.
Updates module documentation, catalogue, and Dependabot configuration.

File	Summary
`README.md`	Updates the available-modules catalogue.
`infrastructure/modules/eventbridge/versions.tf`	Defines Terraform and provider constraints.
`infrastructure/modules/eventbridge/variables.tf`	Defines wrapper inputs and validations.
`infrastructure/modules/eventbridge/README.md`	Documents the EventBridge wrapper.
`infrastructure/modules/eventbridge/outputs.tf`	Exposes EventBridge outputs.
`infrastructure/modules/eventbridge/main.tf`	Configures the upstream EventBridge module.
`infrastructure/modules/eventbridge/locals.tf`	Implements naming and configuration transformations.
`infrastructure/modules/eventbridge/context.tf`	Provides shared context integration.
`infrastructure/modules/eventbridge/.terraform.lock.hcl`	Locks AWS provider dependencies.
`.github/dependabot.yaml`	Enables Dependabot coverage.

Files not reviewed (1)

infrastructure/modules/eventbridge/.terraform.lock.hcl: Generated file

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…is required

…to explicitly be made unique

Copilot

Copilot review overview

🟡 Changes recommended

API destinations and schema discovery are unusable under the wrapper’s constraints, and schedule processing can fail on optional group names.

Review effort: Balanced
Findings: 2 · 3 · 2

Open (7)

Schema discovery is incompatible with required customer-managed KMS keys · New
API destinations lack a resolvable EventBridge connection · New
Schedule group lookup fails for missing or unknown group names · New
Archive validation crashes when KMS key identifier is omitted · New
Pipe validation crashes when KMS key identifier is omitted · New
Unsectioned variables reduce reviewability
Use the pinned registry module source and version

Resolved since last review (5)

Invalid can usage breaks Terraform validation
Mark connections variable as sensitive
Document Scheduler state as an optional string
Document role_arn as an optional string
Allowing empty key identifiers bypasses required encryption validation

Files not reviewed (1)

infrastructure/modules/eventbridge/.terraform.lock.hcl: Generated file

Copilot

Copilot review overview

🟡 Changes recommended

Disabled-module evaluation, conditional validation, and public input handling contain blocking correctness issues.

Review effort: Balanced
Findings: 4 · 5

Open (9)

Guard disabled module ID to allow planning · New
Remove name_prefix before setting the context-derived name · New
Gate KMS ARN validation on normal role creation · New
Conditionally require the KMS key identifier · New
Use fenced HCL blocks for all usage examples · New
Document connections as unsupported by this wrapper · New
Link the module header to cross-variable validations · New
Unsectioned variables reduce reviewability
Use the pinned registry module source and version

Resolved since last review (5)

API destinations lack a resolvable EventBridge connection
Schema discovery is incompatible with required customer-managed KMS keys
Pipe validation crashes when KMS key identifier is omitted
Archive validation crashes when KMS key identifier is omitted
Schedule group lookup fails for missing or unknown group names

Files not reviewed (1)

infrastructure/modules/eventbridge/.terraform.lock.hcl: Generated file

…refix properties

…re ergonomic

Copilot

Copilot review overview

🟡 Changes recommended

The default invocation fails when evaluating the nullable enabled input, alongside unresolved API and source-version issues.

Review effort: Balanced
Findings: 1 · 3

Open (4)

Use resolved context value for enabled fallback · New
Use fenced HCL blocks for all usage examples
Unsectioned variables reduce reviewability
Use the pinned registry module source and version

Resolved since last review (6)

Conditionally require the KMS key identifier
Gate KMS ARN validation on normal role creation
Remove name_prefix before setting the context-derived name
Guard disabled module ID to allow planning
Link the module header to cross-variable validations
Document connections as unsupported by this wrapper

Files not reviewed (1)

infrastructure/modules/eventbridge/.terraform.lock.hcl: Generated file

…ditional

Copilot

Copilot review overview

🟡 Changes recommended

Disabled and bus-less configurations currently fail because of the bus-name expression and unconditional KMS input.

Review effort: Balanced
Findings: 1 · 2

Open (3)

Guard coalesce against null values when the module is disabled · New
Use fenced HCL blocks for all usage examples
Use the pinned registry module source and version

Resolved since last review (2)

Use resolved context value for enabled fallback
Unsectioned variables reduce reviewability

Files not reviewed (1)

infrastructure/modules/eventbridge/.terraform.lock.hcl: Generated file

Previously missed (1)

In code that hasn't changed since last review

Remove unsupported resources from the naming enforcement claim

infrastructure/modules/eventbridge/README.md:41

This enforcement claim contradicts the wrapper's explicit removal of connections and API destinations: neither resource has a creation input, so their names cannot be scoped here. Remove those resource types from the naming claim.

Copilot

Copilot review overview

🟡 Changes recommended

Schema discovery and schedule-group naming can currently cause apply failures.

Review effort: Balanced
Findings: 2 · 1

Open (3)

Include group postfix in rewritten schedule reference · New
Disable schema discovery for customer-key-encrypted buses · New
Use the pinned registry module source and version

Resolved since last review (2)

Guard coalesce against null values when the module is disabled
Use fenced HCL blocks for all usage examples

Files not reviewed (1)

infrastructure/modules/eventbridge/.terraform.lock.hcl: Generated file

Previously missed (1)

In code that hasn't changed since last review

Document required KMS key when creating the bus

infrastructure/modules/eventbridge/README.md:116

The Validation section documents the SNS precondition but omits the other precondition in validations.tf: kms_key_identifier must be non-empty when create_bus is enabled. Add it here so callers can discover all enforced cross-variable constraints before planning.

dave4420 · 2026-10-02T10:27:20Z

+      ? merge(
+        v,
+        {
+          group_name = local.schedule_groups[v.group_name].name


This isn't true. We are using this with https://github.com/NHSDigital/bcss/pull/590 and it works fine.

dave4420 · 2026-10-02T07:57:35Z

+  event_source_name              = var.event_source_name
+  kms_key_identifier             = var.kms_key_identifier
+  dead_letter_config             = var.dead_letter_config
+  schemas_discoverer_description = var.schemas_discoverer_description


No. The pinned upstream module defaults create_schemas_discoverer to false.

dave4420 marked this pull request as ready for review September 28, 2026 09:21

dave4420 requested review from a team and saliceti as code owners September 28, 2026 09:21

nhs-oliverslater requested a lite review from Copilot September 28, 2026 16:35

Copilot started reviewing on behalf of nhs-oliverslater September 28, 2026 16:36 View session

nhs-oliverslater reviewed Sep 28, 2026

View reviewed changes

Comment thread README.md Outdated

Copilot AI reviewed Sep 28, 2026

View reviewed changes

dave4420 added 22 commits October 1, 2026 00:12

chore(eventbridge): create directory

c4e4303

chore: add versions

f0e2d5a

chore: start main

d48fa5f

docs: start notes

a663c3b

docs: continue notes

cb2199a

docs: more notes

9a33382

docs: document name uniqueness requirements

5adbe35

docs: document encryption key inputs

a904d8e

docs: rm comment that is expanded on by notes

9415eb8

chore: add context

83326d6

feat: copy input variables from underlying module

b9b6b5a

feat: copy output variables from underlying module

b41c083

docs: add notes about where names are obtained from where uniqueness …

0451196

…is required

chore: add todos for inputs that need to be unique names

779d976

docs: todo

df64bb9

feat: allow overriding bus name

2f9bd32

docs: log_delivery_source_name defaults to bus name, so doesn’t need …

5ca23d2

…to explicitly be made unique

feat: provide default unique log delivery names

dfd375a

feat: enforce unique connection names

f5510f0

feat: enforce unique API destination names

bd8057f

feat: enforce unique schedule group names

d91c751

feat: enforce unique pipe names

7233623

docs: explain why state is a bool and not a string (enum really)

f8f0540

dave4420 requested a balanced review from Copilot October 1, 2026 12:59

Copilot started reviewing on behalf of dave4420 October 1, 2026 13:00 View session

chore: Merge branch 'main' into feature/BCSS-24132-wrap-eventbridge

eee49e0

Copilot AI reviewed Oct 1, 2026

View reviewed changes

dave4420 added 4 commits October 1, 2026 14:49

fix: disable schema discovery

77639b7

fix: rm api_destinations, as it’s unless without connections

a761cc1

fix: handle missing properties properly in local.schedules

8e2ce5f

fix: handle missing properties properly in variable validation

0ecabeb

dave4420 requested a balanced review from Copilot October 1, 2026 14:42

Copilot started reviewing on behalf of dave4420 October 1, 2026 14:42 View session

Copilot AI reviewed Oct 1, 2026

View reviewed changes

dave4420 added 6 commits October 1, 2026 16:12

fix: prevent var.bus_name from being ""

502985f

fix: validate that var.schedule_groups doesn’t contain name or name_p…

fdc138c

…refix properties

fix: make precondition on var.attach_sns_policy / var.sns_kms_arns mo…

db123fc

…re ergonomic

fix: make var.kms_key_identifier validation more ergonomic

d751244

docs: clarify what we don’t do

64415eb

chore: point maintainers to validations.tf

27de7ad

dave4420 requested a balanced review from Copilot October 1, 2026 16:41

Copilot started reviewing on behalf of dave4420 October 1, 2026 16:42 View session

Copilot AI reviewed Oct 1, 2026

View reviewed changes

Comment thread infrastructure/modules/eventbridge/locals.tf Outdated

dave4420 added 2 commits October 2, 2026 08:16

fix: var.enabled could be null, so use module.this.enabled in con…

6d26a58

…ditional

docs: add section headers to variables.tf

6a0c8d4

dave4420 requested a balanced review from Copilot October 2, 2026 07:25

Copilot started reviewing on behalf of dave4420 October 2, 2026 07:25 View session

Copilot AI reviewed Oct 2, 2026

View reviewed changes

Comment thread infrastructure/modules/eventbridge/locals.tf Outdated

fix: coalesce() errors if all arguments are null!

4ea2a45

dave4420 requested a balanced review from Copilot October 2, 2026 07:37

Copilot started reviewing on behalf of dave4420 October 2, 2026 07:37 View session

Copilot AI reviewed Oct 2, 2026

View reviewed changes

Conversation

dave4420 commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

dave4420 Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

dave4420 Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dave4420 commented Sep 10, 2026 •

edited

Loading