Add Document Data Extraction Module by laurencegoolsby · Pull Request #989 · navapbc/template-infra

laurencegoolsby · 2026-01-06T22:11:54Z

Fixes #986

Changes

Add a new document-data-extraction module that provides AWS Bedrock Data Automation (BDA) resources for document data extraction workflows.

DDE Module

Add BDA project and profile resources
Add custom blueprint support from JSON schemas
Add AWS-managed blueprint support
Add IAM access policies with bedrock permissions
Add cross-region inference profile support
Add standard and custom output configuration options

Storage Module

Uses KMS encryption (storage module provides the encryption key)

Template Files

Update service and env-config files to use new module interface
Add aws_managed_blueprints parameter

Important Notes

BDA uses its own internal service role - This module does not create a custom IAM role for BDA
S3 bucket encryption - S3 buckets used with BDA should use the KMS encryption key provided by the storage module
Lambda permissions - Lambda functions invoking BDA must have S3 permissions for both input and output buckets directly attached to their execution role

Testing

Tested in navapbc/platform-test#237

- Add enable_document_data_extraction boolean - Add environment variables and bucket policies to service - Add additional outputs for BDA project/blueprint

- Move infra/modules/document-data-extraction/resources/bedrock-data-automation/* to infra/modules/document-data-extraction/resources - Rename infra/app-flask/service/blueprints to document-data-extraction-blueprints - Remove bda_ prefix from infra/modules/document-data-extraction/resources/variables.tf and infra/app-flask/service/document_data_extraction.tf - Update infra/app-flask/app-config/env-config/document_data_extraction.tf name and path

- Create KMS key - Remove KMS configuration parameters from module interface - Add Bedrock Data Automation access_policy_arn output for service integration - Update blueprints_map to use map(string) tags instead of complex objects - Remove enabled_blueprint logic in favor of reading blueprints directory - Update README

…ts.tf, infra/modules/document-data-extraction/resources/variables.tf

- Remove Bedrock Data Automation KMS key alias - Remove unused Document Data Extraction outputs - Format code via make format

doshitan

We also need the infra/{{app_name}}/* changes for configuring/using the module.

infra/modules/document-data-extraction/resources/README.md

- Rename override_config_state to override_configuration in README - Add usage examples with service configuration and blueprint schema

laurencegoolsby · 2026-01-08T14:39:52Z

We also need a infra/{{app_name}}/* changes for configuring/using the module.

@doshitan - Added usage examples in infra/{{app_name}}/service/

doshitan · 2026-01-08T19:01:59Z

We also need a infra/{{app_name}}/* changes for configuring/using the module.

@doshitan - Added usage examples in infra/{{app_name}}/service/

@laurencegoolsby looks like we are still missing most of the infra/{{app_name}}/* changes? We need all the config and supporting plumbing for using the module.

laurencegoolsby · 2026-01-09T00:25:40Z

We also need a infra/{{app_name}}/* changes for configuring/using the module.

@doshitan - Added usage examples in infra/{{app_name}}/service/

@laurencegoolsby looks like we are still missing most of the infra/{{app_name}}/* changes? We need all the config and supporting plumbing for using the module.

@doshitan - Added/updated files in infra/{{app_name}}/*

doshitan

Please update the PR description to mention the associated ticket, matching the PR template for the project (and so also probably removing the "Summary" section/fold that into the "Changes" to match the PR template): #986

Please link to the test PR, not the branch the in the PR description.

Avoid the past tense description of changes in the PR description. For background context about how a decision was arrived at that's fine, but for what the current changes do the descriptive text should be more in the imperative mood, like the title.

We'll also need the changes in navapbc/platform-test#238

infra/{{app_name}}/app-config/dev.tf

infra/{{app_name}}/service/document_data_extraction.tf

infra/{{app_name}}/app-config/main.tf

infra/modules/document-data-extraction/resources/README.md

laurencegoolsby · 2026-01-09T19:48:01Z

Please update the PR description to mention the associated ticket, matching the PR template for the project (and so also probably removing the "Summary" section/fold that into the "Changes" to match the PR template): #986

Please link to the test PR, not the branch the in the PR description.

Avoid the past tense description of changes in the PR description. For background context about how a decision was arrived at that's fine, but for what the current changes do the descriptive text should be more in the imperative mood, like the title.

We'll also need the changes in navapbc/platform-test#238

@doshitan Added changes from navapbc/platform-test#238

- Change environment variables to DDE_INPUT_LOCATION and DDE_OUTPUT_LOCATION - Add BDA region configuration with us-east-1 default - Create region-specific AWS providers for BDA deployment - Move profile ARN generation to module output - Add comment explaining DOCUMENT vs IMAGE blueprint types

infra/{{app_name}}/app-config/env-config/document_data_extraction.tf

infra/modules/document-data-extraction/resources/outputs.tf

infra/{{app_name}}/app-config/env-config/document_data_extraction.tf

infra/{{app_name}}/service/document_data_extraction.tf

- Add support for AWS-managed encryption (Bedrock Data Automation requires AWS-managed encryption) - Add support for AWS-managed Bedrock Data Automation blueprints

doshitan

Any changes to the storage module should be a separate PR for cleaner review please.

And what exactly are we seeing that leads us to believe BDA can't use S3 buckets with customer managed keys?

Also removed was configuration for custom KMS key encryption for BDA itself. Why is that?

infra/{{app_name}}/app-config/env-config/document_data_extraction.tf

infra/modules/document-data-extraction/resources/access_control.tf

laurencegoolsby · 2026-01-26T21:32:46Z

Any changes to the storage module should be a separate PR for cleaner review please.

And what exactly are we seeing that leads us to believe BDA can't use S3 buckets with customer managed keys?

Also removed was configuration for custom KMS key encryption for BDA itself. Why is that?

@doshitan

Re: Storage module changes

BDA requires storage module changes for both encryption approaches:

CMK: Storage module needs kms_service_principals variable (default []) for bedrock.amazonaws.com access and KMS key ARN output
AWS-managed: Storage module needs use_aws_managed_encryption variable to skip CMK creation

Current storage module always creates CMKs with a basic key policy but does not expose for use by BDA.

Options:

Include minimal storage module changes in this PR
Create BDA buckets directly (not using storage module) to unblock this PR
Wait for separate storage module PR

Re: BDA encryption_configuration

Removed - caused deployment errors, parameter unsupported in current BDA API.

- Consolidate blueprints_path and aws_managed_blueprints into single blueprints list - Support mixed list of file paths and ARNs in blueprints variable - Add glob pattern expansion in service layer for blueprint files - Remove InvokeModel and InvokeModelWithResponseStream permissions (not needed for BDA) - Update README to reflect new blueprints list interface

laurencegoolsby · 2026-01-27T03:15:35Z

@doshitan Addressed feedback:

Consolidated blueprints_path and aws_managed_blueprints into single blueprints list
Removed InvokeModel/InvokeModelWithResponseStream permissions (not needed for BDA)
Updated README

Re: Storage module - BDA requires storage module changes regardless of encryption approach. Awaiting your feedback on how to proceed.

doshitan · 2026-01-27T22:02:25Z

BDA requires storage module changes for both encryption approaches:

And whatever they are, please put them in a separate PR. This PR can depend on it.

CMK: Storage module needs kms_service_principals variable (default []) for bedrock.amazonaws.com access and KMS key ARN output

This seems like the more useful functionality to have. The storage module could export the KMS ARN and the DDE module could attach whatever additional policies needed to the key/bucket. Not sure if just the caller of BDA needs to be able to provide a grant for the KMS key and/or BDA needs declared in the key policy directly.

But either option is fine for now.

Re: BDA encryption_configuration
Removed - caused deployment errors, parameter unsupported in current BDA API.

It was working previously in navapbc/platform-test#237? What deployment? What exactly is the error message?

doshitan

See #989 (comment)

laurencegoolsby · 2026-01-29T15:57:56Z

BDA requires storage module changes for both encryption approaches:

And whatever they are, please put them in a separate PR. This PR can depend on it.

CMK: Storage module needs kms_service_principals variable (default []) for bedrock.amazonaws.com access and KMS key ARN output

This seems like the more useful functionality to have. The storage module could export the KMS ARN and the DDE module could attach whatever additional policies needed to the key/bucket. Not sure if just the caller of BDA needs to be able to provide a grant for the KMS key and/or BDA needs declared in the key policy directly.

But either option is fine for now.

Re: BDA encryption_configuration
Removed - caused deployment errors, parameter unsupported in current BDA API.

It was working previously in navapbc/platform-test#237? What deployment? What exactly is the error message?

@doshitan

PR #237 invoked BDA directly from ECS; DocumentAI infra structure uses Lambda (triggered by EventBridge) to invoke BDA.

TLDR;

ECS worked sans issues
Lambda execution role lacks KMS permissions resulting in an "AccessDeniedException: Invalid KMS key. Check KMS key and associated permissions." error.

I'll create a separate PR for the storage module changes (kms_service_principals, KMS ARN export, optional AWS-managed keys)

… modules - Add providers.tf to infra/modules/storage - Add providers.tf to infra/modules/document-data-extraction/resources - Resolves Terraform warnings about undefined provider references when passing providers between modules

….com/navapbc/template-infra into lgoolsby/add-bedrock-data-automation

doshitan

Lint and test failures, please fix. Looks like this code is not completely up-to-date.

Remove obsolete use_aws_managed_encryption variable and related logic

doshitan

The module README.md and PR description are out of date. But after those are updated to reflect current code. Looks good!

infra/modules/document-data-extraction/resources/README.md

doshitan · 2026-02-20T22:05:27Z

infra/modules/document-data-extraction/resources/README.md

+
+- **BDA uses its own internal service role** - This module does not create a custom IAM role for BDA. Bedrock Data Automation uses an AWS-managed internal service role for S3 access.
+- **S3 bucket encryption** - S3 buckets used with BDA should use AWS-managed encryption (AES256), not customer-managed KMS keys.
+- **Lambda permissions** - Any Lambda function invoking BDA must have S3 permissions for both input and output buckets directly attached to its execution role.


Not sure why this is if BDA is actually accessing the buckets?

@doshitan Lambda needs S3 permissions as the DocumentAI Lambda uploading the input document to S3. A separate Lambda retrieving the output from S3, while another Lambda invokes BDA.

Updated Lambda permission verbiage accordingly.

Ah, but this is true of anything wanting to use DDE, not just an AWS Lambda right?

And a service wanting to use BDA could theoretically not have write access to the input S3 bucket, but as long as they can construct the S3 URI to pass in as a parameter to BDA they could still utilize the module right? Unlikely set up, but this module doesn't really make any assumptions about the caller like this? (BDA itself just needs to be able to r/w the input/ouput locations)

infra/modules/document-data-extraction/resources/README.md

- Remove references to S3 AWS-managed encryption - Fix indentation in usage section

laurencegoolsby · 2026-02-23T16:52:52Z

The module README.md and PR description are out of date. But after those are updated to reflect current code. Looks good!

Updated README.md and PR description.

laurencegoolsby added 7 commits January 6, 2026 16:27

Update document data extraction module

8b7c0f5

- Add enable_document_data_extraction boolean - Add environment variables and bucket policies to service - Add additional outputs for BDA project/blueprint

Fix override_configuration variable naming

2459d71

Update infra/app-flask/service/main.tf, infra/app-flask/service/outpu…

57f7673

…ts.tf, infra/modules/document-data-extraction/resources/variables.tf

Update per PR feedback.

ddf24c6

- Remove Bedrock Data Automation KMS key alias - Remove unused Document Data Extraction outputs - Format code via make format

Rename bucket_policy_arns to data_access_policy_arns

07e4928

laurencegoolsby requested a review from a team as a code owner January 6, 2026 22:11

laurencegoolsby requested a review from doshitan January 7, 2026 15:29

doshitan reviewed Jan 7, 2026

View reviewed changes

infra/modules/document-data-extraction/resources/README.md Outdated Show resolved Hide resolved

Address PR feedback

84b07a3

- Rename override_config_state to override_configuration in README - Add usage examples with service configuration and blueprint schema

Add missing document data extraction infrastructure configuration

50a2bbf

doshitan reviewed Jan 9, 2026

View reviewed changes

laurencegoolsby added 3 commits January 9, 2026 15:28

Rename dde_profile_arn to bda_profile_arn

6f46a1d

Fix DDE S3 environment variables to use S3 URI format

153c233

doshitan reviewed Jan 12, 2026

View reviewed changes

laurencegoolsby added 3 commits January 22, 2026 17:23

Address PR feedback - add comments, GovCloud TODO, update bucket naming

d919bd1

Update DDE and storage modules

436c674

- Add support for AWS-managed encryption (Bedrock Data Automation requires AWS-managed encryption) - Add support for AWS-managed Bedrock Data Automation blueprints

Add InvokeDataAutomationAsync permission and profile resource access

a8c76aa

laurencegoolsby mentioned this pull request Jan 23, 2026

Add DocumentAI reference implementation using DDE module navapbc/platform-test#252

Open

doshitan reviewed Jan 23, 2026

View reviewed changes

infra/{{app_name}}/app-config/env-config/document_data_extraction.tf Outdated Show resolved Hide resolved

infra/modules/document-data-extraction/resources/access_control.tf Outdated Show resolved Hide resolved

laurencegoolsby requested a review from doshitan January 27, 2026 03:15

doshitan reviewed Jan 28, 2026

View reviewed changes

doshitan mentioned this pull request Feb 19, 2026

BDA module test navapbc/platform-test#237

Closed

Merge branch 'main' into lgoolsby/add-bedrock-data-automation

1c2397c

laurencegoolsby requested a review from a team as a code owner February 20, 2026 02:39

laurencegoolsby added 2 commits February 19, 2026 21:46

Merge branch 'lgoolsby/add-bedrock-data-automation' of https://github…

627ce48

….com/navapbc/template-infra into lgoolsby/add-bedrock-data-automation

doshitan reviewed Feb 20, 2026

View reviewed changes

laurencegoolsby added 2 commits February 20, 2026 13:18

Update storage module encryption.tf and main.tf to match main

6164c7e

Remove obsolete use_aws_managed_encryption variable and related logic

Update document_data_extraction.tf to match platform-test

2d8671a

laurencegoolsby requested a review from doshitan February 20, 2026 19:16

doshitan approved these changes Feb 20, 2026

View reviewed changes

doshitan removed the request for review from a team February 20, 2026 22:11

Update README.md

f41b2ff

- Remove references to S3 AWS-managed encryption - Fix indentation in usage section

Update README

b63cc32

laurencegoolsby merged commit 8f4515d into main Feb 23, 2026
9 checks passed

laurencegoolsby deleted the lgoolsby/add-bedrock-data-automation branch February 23, 2026 18:01

Conversation

laurencegoolsby commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

DDE Module

Storage Module

Template Files

Important Notes

Testing

Uh oh!

doshitan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

laurencegoolsby commented Jan 8, 2026

Uh oh!

doshitan commented Jan 8, 2026

Uh oh!

laurencegoolsby commented Jan 9, 2026

Uh oh!

doshitan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

laurencegoolsby commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

doshitan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

laurencegoolsby commented Jan 26, 2026

Re: Storage module changes

Re: BDA encryption_configuration

Uh oh!

laurencegoolsby commented Jan 27, 2026

Uh oh!

doshitan commented Jan 27, 2026

Uh oh!

doshitan left a comment

Choose a reason for hiding this comment

Uh oh!

laurencegoolsby commented Jan 29, 2026

Uh oh!

doshitan left a comment

Choose a reason for hiding this comment

Uh oh!

doshitan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

doshitan Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

laurencegoolsby Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

doshitan Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

laurencegoolsby commented Feb 23, 2026

Uh oh!

Uh oh!

Reviewers

laurencegoolsby commented Jan 6, 2026 •

edited

Loading

doshitan left a comment •

edited

Loading

doshitan left a comment •

edited

Loading

laurencegoolsby commented Jan 9, 2026 •

edited

Loading