Port v0.17 template-infra changes (storage, docs, project-config, version bump)#10811
Merged
Conversation
added 2 commits
June 8, 2026 18:05
Ports the v0.17 storage module change (upstream PR navapbc/template-infra#997) that lets a caller grant AWS service principals direct access to the bucket's KMS key. Strictly additive — the variable defaults to an empty list so existing callers (api, analytics, frontend, nofos) see no behavioral change. What changed: - variables.tf: add service_principals_with_access (list(string), default []) - encryption.tf: replace the implicit AWS-managed KMS key policy with an explicit policy doc that - grants the AWS account root full kms:* (the default policy AWS would attach anyway — functionally equivalent), - conditionally grants the listed service principals Decrypt/GenerateDataKey/DescribeKey via the S3 service. Also adds data sources for caller identity and region. - providers.tf: new file declaring the aws provider source (matches the rest of the modules updated in v0.17) - access_control.tf: cosmetic blank line between two statements - tests/storage.tftest.hcl: override the kms_key_policy data source so mock_provider returns valid JSON (default mock value isn't valid JSON and aws_kms_key validates the policy field as JSON) Validated locally: - terraform fmt clean - terraform validate green - terraform test: 8 passed, 0 failed Scope notes (other v0.17 items NOT in this PR): - New modules (document-data-extraction, notifications-sms, notifications-phone-pool) — pure feature additions, only port if backend wants them - Network vpc_endpoints restructure — sgg's network module diverges from upstream; safer as a focused follow-up - bin script cleanups, doc additions, monitoring/database/service tweaks — bundle into a separate "v0.17 housekeeping" PR - Playwright bump — frontend layer, separate concern Tracks issue #9837.
Second commit on the v0.17 upgrade branch covering the safely-additive remaining items. Storage module changes are in the previous commit (cherry-picked from #10763). New docs from upstream v0.17: - docs/infra/deletion-protection-and-temporary-environments.md - docs/infra/temporary-environments-and-out-of-band-resources.md (the third v0.17 doc, migrate-terraform-state-locking-to-s3.md, is already in sgg via #9672) project-config/aws_services.tf: - Add "bedrock" (AWS Bedrock LLM access) - Add "cloudformation" (IaC service) Deliberately NOT applied from v0.17: - Removal of "mobiletargeting" — sgg still uses Pinpoint; this is being unwound via the SES migration on PRs #10358 / #10487 - Addition of "sms-voice" — sgg isn't adopting SMS notifications in this PR Template version pin: .template-infra/*.yml bumped from v0.15.7 to v0.17.0 (base + 4 apps).
Collaborator
Author
terraform plan results (dev, all 3 impacted layers)Plans run from this branch against dev backend. Summary
Confirmed PR effectsKMS key policy (analytics + frontend): The current policy is the AWS implicit default (with auto-assigned GitHub Actions IAM policy (accounts): Exactly what's expected from adding those two services to Drift not caused by this PR (worth knowing)In accounts the plan also shows:
In analytics + frontend the plan also shows:
Two pre-existing accounts-plan errors (unrelated to this PR): NewRelic provider missing local credentials. Plan generated despite this; would block apply unless the env vars are set. Full plan outputThree follow-up comments below — one per layer. ~1800 total lines, posting separately due to GitHub's 64KB comment cap. |
Collaborator
Author
Full plan:
|
Collaborator
Author
Full plan:
|
Collaborator
Author
Full plan:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket
Closes #9837 — template-infra v0.16 → v0.17 upgrade.
What this PR ships
The safely additive parts of upstream v0.17. Two commits:
Storage module:
service_principals_with_access(upstream PR Support configuring storage module for service principal access navapbc/template-infra#997)variables.tf— newservice_principals_with_access(list(string), default[])encryption.tf— explicit KMS key policy doc; conditional service-principal grantsproviders.tf— new file declaring aws provider sourceaccess_control.tf— cosmetic blank linetests/storage.tftest.hcl—override_datafor the new policy doc so mock_provider produces valid JSONDocs, project-config, version bump
deletion-protection-and-temporary-environments.md,temporary-environments-and-out-of-band-resources.mdproject-config/aws_services.tf— addbedrock+cloudformationto the AWS services allow list.template-infra/*.yml— bump_commit: v0.15.7→v0.17.0across base + 4 app answers filesWhat's deliberately NOT in this PR (and why)
Skipped — feature additions, not template upgrades
infra/modules/document-data-extraction/infra/modules/notifications-sms/+notifications-phone-pool/sms-voiceAWS service / SMS-related VPC endpointsSkipped — sgg has diverged from upstream
Applying v0.17 deltas here would clobber sgg-specific features. Each deserves its own focused PR if/when worth doing.
infra/modules/monitoring/main.tfaws_cloudwatch_metric_alarm.service_errors+aws_cloudwatch_log_metric_filter.service_error_filter. Also different alarm thresholds.infra/modules/service/database_access.tfaws_iam_role_policy_attachment.migrator_app_db_accessresource.infra/modules/terraform-backend-s3/main.tfinfra/modules/network/resources/vpc_endpoints.tfbin/*(many)Skipped — already aligned with v0.17 or N/A
infra/accounts/outputs.tfdocs/infra/migrate-terraform-state-locking-to-s3.mdbin/migrate-terraform-state-locking-to-s3infra/{{app_name}}/...Jinja-only filestemplate-only-*Removal of
mobiletargetingfrom aws_services intentionally NOT hereUpstream v0.17 dropped
mobiletargeting(Pinpoint) since v0.16 already removed the Pinpoint module. sgg still uses Pinpoint actively — that removal is being staged via PRs #10358 (full migration, paused) and #10487 (additive SES, ready for review). Keepingmobiletargetinghere until those land.Testing
terraform fmt -recursive infra/cleanterraform validateclean oninfra/modules/storageandinfra/project-configterraform test— 8/8 pass on storage moduleterraform planagainst each consumer ofmodule.storageto confirm only the KMS key policy field changes from null → explicit docCloses
Closes #9837. Supersedes #10763 (which only had the storage subset).
Follow-up issues to file (post-merge)