Skip to content

Add S3 table for submission events - #2308

Open
theseanything wants to merge 3 commits into
mainfrom
submission-events-s3-table
Open

theseanything wants to merge 3 commits into
mainfrom
submission-events-s3-table

Conversation

@theseanything

@theseanything theseanything commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

What problem does this pull request solve?

Trello card: https://trello.com/c/b3cNvzu5/3136-record-kpis

We want data about submission events so we can measure our KPIs. This is the first step. It creates:

  • an 'analytics' S3 table bucket
  • a forms namespace
  • a submission_events table
  • the account wide "s3tablescatalog" AWS Glue Catalog

It also connects S3 Tables to the Glue Data Catalog so that Athena can query them.

Things to consider when reviewing

  • The Glue catalog is in forms/account because each account and region can only have one, and it covers every table bucket in the account. IAM controls access (IAM_ALLOWED_PRINCIPALS), so we do not need Lake Formation grants.
  • The table does not have partitions. The AWS provider cannot set partitions yet (hashicorp/terraform-provider-aws#46494). At our data volumes, Iceberg's file statistics are enough to filter queries by time.

Reminders

This pull request changes the deployer role (modules/deployer-access) and adds the Glue catalog to forms/account. No pipeline applies these changes.

Copilot AI balanced review requested due to automatic review settings October 7, 2026 14:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The deployment role lacks the S3 Tables and Glue write permissions required to apply these resources.

Review effort: Balanced
Findings: 1 High severity

Open (1)
What changed in this PR

Adds S3 Tables infrastructure for querying form submission events through Athena.

Changes:

  • Creates an Iceberg submission_events table and supporting bucket/namespace.
  • Federates S3 Tables into Glue.
  • Enables analytics in the health deployment.
File Description
infra/​modules/​analytics/​variables.tf Defines the environment input.
infra/​modules/​analytics/​main.tf Defines analytics resource names.
infra/​modules/​analytics/​s3-tables.tf Creates the bucket, namespace, and table.
infra/​modules/​analytics/​glue-catalog.tf Configures Glue federation.
infra/​modules/​analytics/​outputs.tf Exports bucket identifiers.
infra/​deployments/​forms/​health/​analytics.tf Instantiates the analytics module.

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

Comment thread infra/deployments/forms/health/analytics.tf
Athena and Firehose can only reach S3 Tables through the "s3tablescatalog"
Glue catalog. There can be only one per account and region, and it covers
every table bucket in the account, so it belongs with the other account-wide
resources rather than in the module that creates a table bucket.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 7, 2026 14:29
@theseanything
theseanything force-pushed the submission-events-s3-table branch from fc8f0a1 to 35f4912 Compare October 7, 2026 14:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The deployment role lacks the S3 Tables write permissions required by the automated health apply.

Review effort: Balanced
Findings: 2 High severity

Open (2)

Comment thread infra/modules/analytics/s3-tables.tf
The health pipeline will create the analytics S3 table bucket, namespace and
table, but the deployer role only has read access to S3 Tables. The
permissions are scoped to this environment's analytics table bucket.

The statement is in forms_infra_1 rather than next to s3 in forms_infra_2
because forms_infra_2 is close to the 6,144 character managed policy limit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 7, 2026 14:33
@theseanything
theseanything force-pushed the submission-events-s3-table branch from 35f4912 to 2380734 Compare October 7, 2026 14:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The persistent submission-events table needs protection against accidental Terraform deletion.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (2)

Comment thread infra/modules/analytics/s3-tables.tf
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 7, 2026 14:41
@theseanything
theseanything force-pushed the submission-events-s3-table branch from 2380734 to 64957e9 Compare October 7, 2026 14:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@theseanything
theseanything requested a review from a team October 7, 2026 14:43
@whi-tw
whi-tw requested a balanced review from Copilot October 7, 2026 16:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The shared account module unintentionally provisions the Forms-specific Glue catalog in the deploy account.

1 open finding
1 resolved since last review

🧠 Review effort: Balanced

# it covers every table bucket in the account, not just the one in this module.
# Access control is IAM-based (IAM_ALLOWED_PRINCIPALS), so consumers need IAM
# permissions only, no Lake Formation grants.
resource "aws_glue_catalog" "s3tablescatalog" {

This branch has not been deployed

No deployments
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