fix(datasources): use AWS::Partition in generated ARNs#726
Conversation
Default data-source IAM policy and config ARNs hardcoded the 'aws' partition (as a literal Fn::Join segment), producing invalid ARNs in the aws-cn (China) and aws-us-gov (GovCloud) partitions and failing deployment. Use the AWS::Partition pseudo-parameter so the partition is resolved per-region. Affects the DynamoDB, RDS (policy + cluster config), and OpenSearch ARNs; Lambda is unaffected (uses Fn::GetAtt). Adds a regression test and updates the dataSources snapshots (only 'aws' -> AWS::Partition). Closes #352
📝 WalkthroughWalkthroughDataSource now generates AWS ARN strings using CloudFormation's ChangesPartition-aware ARN construction
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/resources/DataSource.ts (1)
370-390:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winOpenSearch endpoint regex hardcodes
.es.amazonaws.com, preventing theAWS::PartitionARN fix from working for aws-cnThe regex at lines 370-371 only matches
...es.amazonaws.com; China endpoints use...es.amazonaws.com.cn, sorx.exec(...)returns null and the code throwsInvalid AWS OpenSearch endpoint(lines 373-376) before building the ARN withAWS::Partition(line 383). Thedomain-based branch viaFn::GetAtt(lines 391-397) remains partition-agnostic.🛠️ Proposed regex adjustment
- const rx = - /^https:\/\/([a-z0-9-]+\.(\w{2}-[a-z]+-\d)\.es\.amazonaws\.com)$/; + const rx = + /^https:\/\/([a-z0-9-]+\.(\w{2}-[a-z]+-\d)\.es\.amazonaws\.com(?:\.cn)?)$/;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/resources/DataSource.ts` around lines 370 - 390, The endpoint regex (rx) in DataSource.ts is too specific to ".es.amazonaws.com" and fails for China partitions (".es.amazonaws.com.cn"); update rx to accept both variants (e.g., allow an optional ".cn" suffix or otherwise match the partition-specific host) while preserving the capture groups used later (so result[1] retains the full host and result[2] retains the region-like segment), then keep the existing ARN construction that uses result[1] and result[2] and throws the same error when exec returns null.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/resources/DataSource.ts`:
- Around line 370-390: The endpoint regex (rx) in DataSource.ts is too specific
to ".es.amazonaws.com" and fails for China partitions (".es.amazonaws.com.cn");
update rx to accept both variants (e.g., allow an optional ".cn" suffix or
otherwise match the partition-specific host) while preserving the capture groups
used later (so result[1] retains the full host and result[2] retains the
region-like segment), then keep the existing ARN construction that uses
result[1] and result[2] and throws the same error when exec returns null.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0e98a597-b8d3-46cf-9bc2-d7924e869e0c
⛔ Files ignored due to path filters (1)
src/__tests__/__snapshots__/dataSources.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
src/__tests__/dataSources.test.tssrc/resources/DataSource.ts
fix(datasources): use
AWS::Partitionin generated ARNs (China / GovCloud)Closes #352
Problem
The default IAM policy / config ARNs synthesized for data sources hardcode the
awspartition. In the
aws-cn(China:cn-north-1,cn-northwest-1) andaws-us-gov(GovCloud) partitions this produces invalid ARNs and deployment fails, e.g.:
The ARNs are built with
Fn::Joinwhere the partition segment is the literal string'aws'(so it isn't obvious from a text search forarn:aws:). This affects four sites insrc/resources/DataSource.ts:RdsHttpEndpointConfig.DbClusterIdentifierARN(Reported originally against the v1
src/index.js; the same hardcoding carried into v2.)Fix
Replace the hardcoded
'aws'partition segment with the CloudFormation pseudo-parameter{ Ref: 'AWS::Partition' }in all fourFn::JoinARN constructions. CloudFormation resolvesAWS::Partitionto the correct partition (aws/aws-cn/aws-us-gov) for the deploymentregion, so the ARNs are valid everywhere.
Lambda data sources are unaffected: their ARNs already come from
Fn::GetAtt/passthrough(
Api.generateLambdaArn), which are partition-correct.This mirrors the existing intrinsic-function style in the same arrays (which already use
{ Ref: 'AWS::Region' }and{ Ref: 'AWS::AccountId' }).Tests
dataSources.test.ts→ DynamoDB): asserts the generated roleARN contains
{ Ref: 'AWS::Partition' }and never the hardcoded["arn","aws"...].dataSourcessnapshots. The diff is only"aws"→{ "Ref": "AWS::Partition" }(10 occurrences across 6 snapshots); no other changes.Verification
npm run build— OKnpm run lint— 0 errorsnpm test— 21 suites / 348 tests pass; 216 snapshotsnpm run test:e2e— 31 suites / 90 tests passVerified the patch applies cleanly to a fresh
origin/master(978c4cb) and that thedataSourcessuite (26 tests) passes there afternpm ci && npm run build.Note
Verified via offline CloudFormation synthesis only — the sandbox can't deploy to a real
China/GovCloud account. The change is a straightforward swap to the standard partition
pseudo-parameter, which is the documented AWS-recommended approach for partition-agnostic
ARNs.
Summary by CodeRabbit
Bug Fixes
Tests