Skip to content

fix(deploy): --china flag never selects a China region - #2

Open
Rasputin02 wants to merge 1 commit into
aws-samples:mainfrom
Rasputin02:fix/deploy-china-region-flag
Open

fix(deploy): --china flag never selects a China region#2
Rasputin02 wants to merge 1 commit into
aws-samples:mainfrom
Rasputin02:fix/deploy-china-region-flag

Conversation

@Rasputin02

Copy link
Copy Markdown

What's wrong

deploy/deploy.sh:36 implements the --china flag as:

--china)    CHINA="true"; REGION="${REGION:-cn-north-1}"; shift ;;

REGION is unconditionally assigned at deploy/deploy.sh:21 as REGION="${AWS_DEFAULT_REGION:-us-east-1}", so by the time argument parsing runs it is never empty. ${REGION:-cn-north-1} is therefore always a no-op, and --china never actually selects a China region.

This matters because deploy/README.md:76 documents ./deploy.sh --china --init-data as the China deployment one-liner — following the README as written deploys to the wrong partition.

Concrete failure

With AWS_DEFAULT_REGION unset and a stubbed aws sts get-caller-identity:

$ ./deploy.sh --china
China region mode: us-east-1
   Models: Sonnet 3 / Haiku 3 (China Bedrock)
   Region:   us-east-1
   Image:    <acct>.dkr.ecr.us-east-1.amazonaws.com/agentic-data:latest

So --china alone silently targets us-east-1 while switching SUP_MODEL/SUB_MODEL to the China Bedrock model IDs (anthropic.claude-3-sonnet-20240229-v1:0 / haiku) and building a .amazonaws.com ECR domain instead of .amazonaws.com.cn. The result is a global-partition stack configured with model IDs that do not exist there.

What the fix does

  • Removes the ineffective expansion from the --china case and applies the region default after the parse loop.
  • Tracks whether --region was passed explicitly (REGION_EXPLICIT) so an explicit --region still wins over --china regardless of flag order. ./deploy.sh --china --region cn-northwest-1 and ./deploy.sh --region cn-northwest-1 --china both resolve to cn-northwest-1.

Two small related fixes in the same argument-handling block:

  • --help used head -14 "$0" | tail -12, a fixed line offset that printed lines 3-14 and cut off the last two documented options (--destroy and --help). Replaced with a sed range over the two # === banner rules, so the help output stays correct as options are added.
  • Added the existing --key NAME option to the usage block; it is implemented at deploy/deploy.sh:38 but was never documented.

How I verified

Ran inside a container against the checked-out tree, with a stub aws on PATH returning a dummy account ID so the script reaches the banner, and AWS_DEFAULT_REGION unset:

  • Reproduced the failure on main (output above).
  • After the fix, checked the resolved Region: and Image: lines for: --china -> cn-north-1 + .amazonaws.com.cn; --china --region cn-northwest-1 -> cn-northwest-1; --region cn-northwest-1 --china -> cn-northwest-1; --region ap-southeast-1 -> ap-southeast-1 + .amazonaws.com; no args -> us-east-1 + .amazonaws.com.
  • bash -n deploy/deploy.sh passes.
  • Confirmed --help now lists all nine options including --key, --destroy and --help.

What I could not verify

I did not run a real deployment — no AWS credentials for either partition, so ECR push, cloudformation deploy, and the actual China Bedrock model behaviour are untested end to end. Verification covers the argument parsing and the region/ECR-domain values the script derives from it. shellcheck was not available in my environment, so the change is only syntax-checked with bash -n.

The --china flag is documented as deploying to the China region, and
deploy/README.md shows "./deploy.sh --china --init-data" as the China
one-liner. However the flag set REGION="${REGION:-cn-north-1}", and
REGION is always already populated from AWS_DEFAULT_REGION or the
us-east-1 default, so the parameter expansion never took effect.

Running "./deploy.sh --china" therefore printed "China region mode:
us-east-1" and deployed to us-east-1 with the China-only Bedrock model
IDs and a non-China ECR domain.

--china now sets cn-north-1 after argument parsing, and only when
--region was not passed explicitly, so an explicit --region still wins
regardless of flag order.

Also replaces the fixed-offset "head -14 | tail -12" --help output,
which truncated --destroy and --help, with a sed range over the banner
block, and documents the existing but unlisted --key option.
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.

1 participant