feat(ops): wrap day-2 account-scope update-stack-set in a script - #110
feat(ops): wrap day-2 account-scope update-stack-set in a script#110mintun-myo-2020 wants to merge 1 commit into
Conversation
Add scripts/map-tagger.sh, which takes plain account IDs and builds the documented update-stack-set command — the escaped ScopedAccountIds JSON and the full UsePreviousValue parameter list. Full-replacement semantics matching docs/INSTRUCTIONS.md; validates account IDs (12 digits or ALL). README day-2 section now leads with the script, keeping the raw CLI command as a fallback.
ngjinshan
left a comment
There was a problem hiding this comment.
Reviewed the script against the template source (src/js/deploy/template-main.js), the Lambda's scope semantics, and the #108 upgrade behavior. The problem is real and the shell is carefully written (set -euo pipefail, correct empty-array guard, describe-stack-set preflight, /dev/tty guard, byte-identical escaped output). Requesting changes for two issues that recreate the failure class this script exists to prevent.
1. Hardcoded 8-parameter list recreates the "silently reset a parameter" bug class (blocking)
The script already fetches CURRENT_KEYS from describe-stack-set, then ignores it and hardcodes the 8 known parameters. The next template parameter added (exactly as CentralAlertAccountId was in #108) will be silently reset to its default by this script — the same failure mode the PR description cites as motivation.
Since the keys are already in hand, build the list dynamically: every key in CURRENT_KEYS except ScopedAccountIds gets UsePreviousValue=true. The special-cased CentralAlertAccountId preserve-branch then falls out for free (only the "not set yet, ask to add" branch remains), and the script never goes stale.
2. Raw fallback command still silently resets CentralAlertAccountId (blocking)
The script handles CentralAlertAccountId, but the raw one-liner kept in README + INSTRUCTIONS (×3 locations) as the fallback does not. On any post-#108 stack with central alerting configured, running the documented fallback resets CentralAlertAccountId to '' and silently disables central alarm delivery — the CT6-class silent-alerting failure again. The fallback commands should gain ParameterKey=CentralAlertAccountId,UsePreviousValue=true (with a note that pre-#108 stacks must omit it), or at minimum a warning.
3. <MPE_ID> notation inconsistency
Everywhere else in the docs, <MPE_ID> is the bare ID — stack names are written map-auto-tagger-mig<MPE_ID>. The script requires the mig-prefixed value (^mig[a-zA-Z0-9]+$), but the new doc snippets say ./scripts/map-tagger.sh <MPE_ID> <REGION>. A user following the docs' own convention passes the bare ID and gets a validation error. Either write mig<MPE_ID> in the snippets or accept both in the script (prepend mig when missing).
4. ALL can be mixed with account IDs
./scripts/map-tagger.sh mig… us-east-1 ALL 111111111111 is accepted and produces ["ALL","111111111111"]. The Lambda's if 'ALL' in scoped: makes the extra IDs silently meaningless — and mixing suggests the user misunderstood the full-replacement semantics. Reject ALL combined with anything else.
Minor
- CloudShell gap: docs say "run in CloudShell", but CloudShell won't have the repo cloned. One line on how to get the script there would help.
- CI coverage:
lint_shell_injection.pyonly guards generated scripts. Ashellcheck scripts/map-tagger.shstep inlint.ymlwould be cheap, and the quoting is the load-bearing part of this script. - Naming:
map-tagger.shnames the whole project, not the action —update-scope.sh/set-account-scope.shwould age better if more day-2 helpers arrive. - Testing checklist: several checked items (CloudWatch logs reviewed, IAM least-privilege) don't apply to this change — can you confirm the script was actually run against a real StackSet at least once? The quoting equivalence deserves one live verification.
Add scripts/map-tagger.sh, which takes plain account IDs and builds the documented update-stack-set command — the escaped ScopedAccountIds JSON and the full UsePreviousValue parameter list. Full-replacement semantics matching docs/INSTRUCTIONS.md; validates account IDs (12 digits or ALL). README day-2 section now leads with the script, keeping the raw CLI command as a fallback.
Description
Re-scoping a multi-account deployment currently means running a long CloudShell one-liner by hand: eight
UsePreviousValueclauses plus a triple-escapedParameterValue="[\"111...\",\"222...\"]". One wrong backslash silently resets a parameter to its template default.scripts/map-tagger.shtakes plain account IDs and generates the exact documented command, with input validation. It adds no AWS resources, IAM, or template changes: it only assembles an existing CLI call.Type of change
Testing
Checklist
cfn-lintpasses — checked by CI).github/scripts/resource_groups/for E2E coverageCHANGELOG.mdupdatedCI Notes