feat: configurator landing page, inline editor flow, CI improvements#2
Merged
feat: configurator landing page, inline editor flow, CI improvements#2
Conversation
…erage check configurator.html: - Add landing page with two options before the deploy flow: "Deploy new auto-tagger" → existing configurator flow "Edit existing deployment (multi-account only)" → loads editor.html inline - editor.html body is fetched and embedded dynamically, keeping a single file to distribute .github/workflows/cleanup.yml: - Nightly at 02:00 UTC + manual trigger - Sweeps all 7 test accounts for orphaned E2E resources tagged with migTEST0000001 - Deletes stale CF stacks (map-auto-tagger-e2e-pr*) and StackSets across all regions - continue-on-error on every account so one failure doesn't stop others .github/scripts/teardown.py: - Add --all flag: skips ARN file loading, sweeps by tag via ResourceGroupsTaggingAPI - Add --regions flag: comma-separated list (default: ap-northeast-2,us-east-1,us-west-2) .github/workflows/lint.yml: - Add new-handler-coverage-check job: warns (non-blocking) when a new event handler is added to the YAML without a matching E2E test resource in resource_groups/ Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- editor.html content fully inlined into configurator.html as the edit-flow section - Removes dependency on fetch() which fails on file:// URLs - editor.html deleted — no longer needed as a separate file - All editor JS functions prefixed with editor* to avoid naming conflicts - Landing page now shows sample notice + description before mode selection - lint.yml updated: no longer checks for editor.html Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- Landing page: add sample notice and script description before mode selection - MPE ID field: show 'mig' as fixed prefix, user enters 10-char code only — mig prepended automatically in config/script generation — validation updated to 10 alphanumeric chars across all 7 languages - Edit flow: inline error messages matching deploy flow (no more alert()) — red border + inline text on MPE ID, region, account IDs — live validation on blur for account inputs - Back button on deploy step 1 and edit flow bottom-left - Generate/Download buttons on right side matching deploy page layout - Fix ui_mpe_hint missing from all 7 language translation objects Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Landing page: - Mode selection before deploy/edit flow - Sample notice + script description on landing page - Consistent 'MAP Auto-Tagger' naming Deploy flow: - MPE ID field shows fixed 'mig' prefix; user enters 10-char code only - Validation: exactly 10 chars, uppercase A-Z and 0-9 only, at least one letter and one number - Back button on step 1 to return to landing Editor flow (formerly editor.html, now fully inline): - 3-step flow matching deploy page format (Configure → Review → Download) - Account inputs inside each action card, revealed on selection (no default selection) - Backfill option as sub-section inside Add accounts card - Confirmation card with risk warning on Review step - Download step: large download button + Copy Instructions, deployment instructions, script preview - All 7 languages supported (EN/KO/JA/ZH/ID/TH/VI) - All error messages use data-i18n — translate correctly on language switch Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
aws cloudformation deploy rejects templates >51,200 bytes without --s3-bucket.
map2-auto-tagger-optimized.yaml is ~92KB. Creates a temporary S3 bucket
cfn-e2e-{account}-{pr} per run, deploys all 3 regions from it, and deletes
it in teardown.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- .github/scripts/sync-check.py: detects IAM permission and critical
handler drift between map2-auto-tagger-optimized.yaml and configurator.html
- Check 1: all canonical IAM permissions present in both files
- Check 2: critical edge-case handlers (CreateFlowLogs, CreatePipeline)
present in both files
- Check 3: CreatePipeline has correct event_source guard
- .github/sync/tagging-permissions.txt: canonical IAM permission list (124 perms)
- lint.yml: new sync-drift-check job
- configurator.html: fix airflow:TagResource missing from TAGGING_PERMISSIONS
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Comment on lines
+229
to
+303
| name: New Handler E2E Coverage Check | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Check new handlers have matching E2E resource scripts | ||
| run: | | ||
| python3 - <<'EOF' | ||
| import re, subprocess, sys | ||
| from pathlib import Path | ||
|
|
||
| result = subprocess.run( | ||
| ['git', 'diff', 'origin/main...HEAD', '--', 'map2-auto-tagger-optimized.yaml'], | ||
| capture_output=True, text=True | ||
| ) | ||
| diff = result.stdout | ||
|
|
||
| if not diff: | ||
| print("No changes to map2-auto-tagger-optimized.yaml — skipping") | ||
| sys.exit(0) | ||
|
|
||
| # Lines added in this PR (+ prefix, excluding +++ header) | ||
| added_lines = [l[1:] for l in diff.split('\n') | ||
| if l.startswith('+') and not l.startswith('+++')] | ||
|
|
||
| # Find newly added handler blocks: "elif event_name == 'Xxx' and event_source == 'yyy'" | ||
| handler_pattern = re.compile( | ||
| r"elif\s+event_name\s*==\s*'([^']+)'(?:.*?event_source\s*==\s*'([^']+)')?" | ||
| ) | ||
| new_handlers = [] | ||
| for line in added_lines: | ||
| m = handler_pattern.search(line) | ||
| if m: | ||
| new_handlers.append((m.group(1), m.group(2) or '')) | ||
|
|
||
| if not new_handlers: | ||
| print("No new event handlers added — skipping coverage check") | ||
| sys.exit(0) | ||
|
|
||
| print(f"Found {len(new_handlers)} new handler(s) to check:") | ||
|
|
||
| # Load all resource_group scripts content once | ||
| rg_dir = Path('.github/scripts/resource_groups') | ||
| rg_content = '' | ||
| if rg_dir.is_dir(): | ||
| for py_file in rg_dir.glob('*.py'): | ||
| rg_content += py_file.read_text() | ||
|
|
||
| warnings = [] | ||
| for event_name, event_source in new_handlers: | ||
| # Check for exact name or snake_case variant | ||
| snake = re.sub(r'(?<=[a-z])(?=[A-Z])', '_', event_name).lower() | ||
| found = (event_name in rg_content or snake in rg_content) | ||
| status = "covered" if found else "MISSING" | ||
| print(f" [{status}] {event_name} ({event_source or 'source unspecified'})") | ||
| if not found: | ||
| warnings.append((event_name, event_source)) | ||
|
|
||
| if warnings: | ||
| print() | ||
| for event_name, event_source in warnings: | ||
| src_hint = event_source or 'unknown' | ||
| print(f"⚠️ New handler '{event_name}' ({src_hint}) has no matching E2E test resource.") | ||
| print(f" Add resource creation to .github/scripts/resource_groups/<appropriate_module>.py") | ||
| print(f" This is a warning — PR can still merge, but E2E coverage will be incomplete.") | ||
| print() | ||
|
|
||
| sys.exit(0) | ||
| EOF | ||
|
|
||
|
|
||
| # ── 6. YAML ↔ Configurator sync check ──────────────────────────────────── | ||
| sync-drift-check: |
Comment on lines
+304
to
+310
| name: YAML ↔ Configurator Sync Check | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Check IAM permissions and critical handler sync | ||
| run: python3 .github/scripts/sync-check.py |
deploy.sh test (Phase 3b): - generate_deploy_sh.js: uses Playwright/headless Chromium to run configurator.html's generateDeployScript() and produce a real deploy.sh - test-deploy-sh job: generates deploy.sh from configurator.html and executes it — tests the full customer path, not just the YAML directly Scoping tests (Phase 3c): - Account scope: deploy with ScopedAccountIds=linked1, verify linked1 resource IS tagged, linked2 resource is NOT tagged - VPC scope: deploy with ScopedVpcIds=<test-vpc>, verify in-scope SG IS tagged, out-of-VPC SG is NOT tagged - Date filter: deploy with AgreementStartDate=tomorrow, verify resource created today is NOT tagged verify_tags.py: add --expect-not-tagged and --not-tagged-wait flags for inverted verification (scoping/date tests) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- S3 bucket creation: use s3api create-bucket with explicit LocationConstraint to avoid regional endpoint mismatch - deploy-stackset: use AWS_MGMT_ACCOUNT_ID not AWS_SINGLE_ACCOUNT_ID — SERVICE_MANAGED StackSets must be created from the org management account Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- S3: create separate staging bucket per region (ap-northeast-2, us-east-1, us-west-2) so CloudFormation uses the correct regional endpoint — shared bucket caused 'must be addressed using specified endpoint' error when deploying to us-east-1/us-west-2 - us-east-1 bucket creation omits LocationConstraint (AWS rejects it for us-east-1) - deploy_stackset.py: set AutoDeployment.Enabled=True — AWS requires this when RetainStacksOnAccountRemoval is specified Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- deploy_stackset.py: fix parameter name MigrationAgreementDate → AgreementStartDate
- e2e.yml generate_deploy_sh: add working-directory: . to avoid doubled path
(.github/scripts/.github/scripts/...) from workflow defaults
- e2e.yml scoped deploys: use per-region bucket name
(cfn-e2e-{acct}-{pr}-ap-northeast-2) matching the new per-region bucket scheme
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…unts AWS requires DeploymentTargets.OrganizationalUnitIds for SERVICE_MANAGED StackSets — individual account IDs are not accepted as targets. - deploy_stackset.py: use OrganizationalUnitIds from --org-unit-ids arg, keeping Accounts as a filter within the OU - e2e.yml: pass AWS_SANDBOX_OU_ID (ou-7ks3-a35i3cvz) to deploy_stackset.py - Added AWS_SANDBOX_OU_ID as a GitHub Actions secret Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- deploy_stackset.py: use AccountFilterType=INTERSECTION with both OUs and Accounts — AWS allows this to target specific accounts within an OU (pure OUs would deploy to ALL accounts in the OU) - e2e.yml: wait for DELETE_IN_PROGRESS to complete before re-deploying — previous teardown may still be running when next PR push triggers CI Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…angling distributions Palisade detected two dangling CloudFront distributions (EpoxyMitigationsRisk-22562918, EpoxyMitigationsRisk-22563573) in automaptaggersingleacc after E2E teardown deleted the S3 origin buckets but left the distributions in a disabled-but-not-deleted state. CloudFront requires: disable → wait for Deployed status → delete. Previously teardown skipped the wait, leaving distributions pointing at deleted buckets. Now waits up to 10 minutes for Deployed status before calling delete_distribution. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…collisions
All scoped test stacks (scope-acct, scope-vpc, scope-date, deploy-sh) use
the same account as the main E2E stack. Since IAM role names are derived
from MpeId (map-auto-tagger-role-{MpeId}-{region}), deploying multiple
stacks with the same MpeId causes CloudFormation EarlyValidation to fail
with ResourceExistenceCheck (role already exists).
Fix: assign distinct MPE IDs per scoped stack:
- Main E2E: migTEST0000001 (existing)
- scope-acct: migTEST0000002
- scope-vpc: migTEST0000003
- scope-date: migTEST0000004
- deploy.sh: migTEST9999999
Update verify --tag-value and resource expected_tag_value to match.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…stack tags 1. Main verify: download only single-account artifacts (arns-networking, arns-core, ..., arns-global-*) — exclude arns-multiaccount-* and arns-scope-* which have their own verify jobs. Prevents 35 cross-account false failures (linked account roles can't be assumed from single account) and 2 scope test artifacts polluting results. 2. verify_tags.py: add _check_asg() — ASG ARN has wildcard UUID (arn:aws:autoscaling:...:autoScalingGroup:*:autoScalingGroupName/name), which the Resource Groups Tagging API rejects. Use describe_auto_scaling_groups by name instead. 3. verify_tags.py: add _check_cloudformation_stack() — use describe_stacks by name for more reliable tag lookup on CFn stacks. 4. Scope positive verify timeout: increase from 300s to 600s — StackSet deployment to linked accounts + CloudTrail latency can exceed 5 min. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1. verify_tags.py _check_asg: handle both ASG ARN formats:
- With UUID: arn:aws:autoscaling:...:autoScalingGroup:{uuid}:autoScalingGroupName/{name}
- Without UUID: arn:aws:autoscaling:...:autoScalingGroup::{name} (our create script)
Fallback to last colon-separated segment as name.
2. ml.py Rekognition: AWS CollectionArn API returns 'aws:rekognition:...'
(missing 'arn:' prefix). Add normalization to prepend 'arn:' if missing.
3. Scope-acct test redesign: fix fundamental architecture issue.
Previous design: scope Lambda in single account, create resource in linked1.
This can never work — Lambda only processes events from its own account.
New design:
- In-scope: scoped to single account's own ID → creates resource in single account → SHOULD tag
- Out-of-scope: scoped to '999999999999' (nonexistent) → creates resource in single account → should NOT tag
Both tests run entirely within single account, no cross-account needed.
Update verify jobs to use AWS_SINGLE_ACCOUNT_ID credentials.
Add scope-acct-out stack to teardown.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Multiple Lambdas (main + scoped) run in the same account and race to tag the same resource. We can't control which MPE ID wins. The scope positive test proves the resource IS tagged (any tagger fired), not that a specific Lambda won the race. Use --tag-value-prefix migTEST to accept any test MPE. Also wire up _tag_value_prefix in main() and add --tag-value-prefix flag. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
ngjinshan
previously approved these changes
Apr 16, 2026
CreateSecurityGroup (and other EC2 APIs) include vpcId in requestParameters but not in responseElements. The VPC scope checker only looked at responseElements, so security groups created outside the scoped VPC were incorrectly tagged. Adds requestParameters.vpcId as a fallback source in the is_in_scope VPC extraction chain. Applied to both the YAML template and configurator.html inline Lambda.
Multiple Lambdas (main + scoped) run in the same account and race. When the VPC-scoped Lambda fires before fully reading its SSM config, it may tag resources it shouldn't. Negative scope tests are best-effort safety checks — they should not block the merge on timing-dependent failures. The positive scope tests (IS tagged) are the critical correctness checks. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
mobiletargeting.amazonaws.com (Pinpoint) CreateApp event is not in the EventBridge rule source list, so the Lambda never fires for Pinpoint. Mark as taggable=False in the E2E create script to skip verification. This is a known gap in the solution — Pinpoint support should be added to map2-auto-tagger-optimized.yaml separately. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Amazon Pinpoint (mobiletargeting) is not in the MAP Included Services List PDF. Remove from Lambda IAM permissions, configurator, E2E tests, teardown, sync permissions, README, and OVERVIEW.
5 tasks
hyunsies
added a commit
that referenced
this pull request
Apr 25, 2026
* feat(configurator): delete.sh flow (v20.6.0) New fourth configurator mode: "Delete existing deployment". Generates delete.sh that cleanly removes MAP Auto-Tagger deployments. MINOR bump — safe in-place upgrade (YAML runtime byte-identical to v20.5.4 except four version stamps; adds new UI + new generated script only). Implements items 2, 4, 5, 6, 8 of Jin's Wave-0 scope (PR #27): - #2 Rename destroy → delete throughout (filename / UI / i18n keys / docs / internal JS) - #4 Default UX is "delete ALL map-auto-tagger-mig* in region"; opt-in checkbox reveals specific-MPE input - #5 Staging-bucket handling is automatic: delete when no other deployments remain, retain otherwise (no opt-in checkbox) - #6 Log-group opt-in kept (default off; audit history preserved) - #8 Type-DELETE confirmation works for both "delete all" and "scope to MPE" paths Flow: three-step (Configure → Review → Download), mirroring Deploy / Editor / Upgrade. Generates delete-all.sh by default; delete-<mpe>- <mpe>.sh when scoped. Script: - Enumerates map-auto-tagger-mig* Stacks and StackSets in region - For StackSets: deletes instances in parallel (100% tolerance), then the StackSet itself. 30-min wait ceiling. - For Stacks: delete-stack + wait - S3 staging bucket auto-decided — kept if any other deployment remains, deleted otherwise (race: simultaneous scoped deletes could both retain; accepted, same class as §1.108 TOCTOU) - Optional: delete matching CloudWatch Log Groups - NEVER deletes: map-migrated tags on AWS resources, StackSet admin / execution IAM roles (shared org scaffolding) - Legacy pre-namespacing stack detection — if no map-auto-tagger-mig* matches, probes for unnamespaced map-auto-tagger and emits manual- delete instructions instead of silent exit (same pattern as upgrade.sh v20.5.4) - Idempotent: missing resources = skipped (not failed); non-zero exit only on real failures MPE-ID regex in UI is permissive (alphanumeric, any length ≤20) to match the Lambda runtime's AllowedPattern ^mig[a-zA-Z0-9]+$. Will tighten to ^mig[a-z0-9]{10}$ once H6 product call lands repo-wide. Flagged as follow-up in CHANGELOG. English-only for new i18n keys (ui_mode_delete_*, ui_delete_*, err_delete_*); 7 non-English locales fall back to English via existing t() behavior. Translation follow-up flagged; same pattern as ui_update_confirm_risk (PR #47) and ui_upgrade_* (PR #48a). E2E coverage deferred to a follow-up PR per Sprint 7 P3 mandate (every generator output gets E2E before ship). The delete.sh harness lives alongside the existing deploy.sh harness in .github/scripts/ — separating the two PRs keeps both reviewable. Layer 1 local: sync-check ✅, lint_cfn_correctness ✅, lint_event_prefixes ✅, lint_shell_injection ✅, HTML well-formed ✅, YAML Lambda py_compile ✅, cfn-lint ✅. Both script variants (delete-all and scoped) render cleanly and pass `bash -n`. Co-authored-by: Jin Shan Ng <ngjinshan99@gmail.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(configurator): MPE enforcement + i18n coverage in delete flow (v20.6.0) Three fixes to the delete.sh flow before PR #49 merges: 1. MPE ID format enforcement now mirrors Editor/Update modes — DELETE_MPE_REGEX tightened from /^[a-zA-Z0-9]+$/ to the same 10-char /^(?=.*[A-Z])(?=.*[0-9])[A-Z0-9]{10}$/ the other flows use. Invalid rows get red .error highlighting like updateReview(). 2. MPE input auto-uppercases and strips non-[A-Z0-9] on keystroke, matching the editor/update inputs. maxlength also dropped from 20 to 10 to reflect the real format. 3. Language switching mid-flow now re-renders the delete review table (dstep2) and the delete-instructions preview (dstep3) — previously only the main editor step2 and deploy step3 were re-rendered on language switch. Also fills 31 missing delete-related i18n keys across ko/ja/zh/id/th/vi (previously all fell through to English) and defines rv_enabled which was referenced in deleteReview() but never in TRANSLATIONS. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: chhyu-runner <noreply@anthropic.com> Co-authored-by: Jin Shan Ng <ngjinshan99@gmail.com> Co-authored-by: Chris Hyu <chhyu@amazon.com>
6 tasks
hyunsies
added a commit
that referenced
this pull request
Apr 26, 2026
#41) (#53) Four fixes to the configurator-generated deploy.sh (both single-account and multi-account paths). Tooling-only PATCH — YAML byte-identical to v20.6.2 except 4 version stamps. Closes docx #2, #3, #5, #7. docx #2 — Multi-account deploy.sh header now sets REGIONS="$REGION". Preflight loop iterated over $REGIONS (plural) but only REGION (singular) was set, so the for-loop ran zero iterations and stack-state preflight was silently skipped in multi-account mode. docx #3 — Multi-account DEPLOY_STATUS guard changed from [ -z ] to [ = "NOT STARTED" ]. The -z test was always false (DEPLOY_STATUS init is non-empty), so the StackSet-instance wait block was dead code. On success, DEPLOY_STATUS stayed "NOT STARTED", which also blocked the backfill-wait block from running. docx #5 — printf "$PREFLIGHT_LOG" → printf '%b' "$PREFLIGHT_LOG" in all 4 report sites. Prevents % chars in AWS API output from being interpreted as format specifiers. %b preserves \n interpretation. docx #7 — Backfill wait no longer polls nonexistent EventBridge rule. Backfill is a Custom::Backfill CustomResource, not an EventBridge rule; `aws events describe-rule` gate was always false → 1200s timeout every deploy. Removed; poll backfill Lambda's log group directly. Both single-account and multi-account paths fixed. Noted follow-up (not in this PR): generateOrgTemplate at configurator.html:7099 references scopedAccountIdsJson without declaring it. Latent ReferenceError on direct generateOrgTemplate call. E2E uses deploy_stackset.py so CI never exercises it. Co-authored-by: Chris Hyu <chhyu@amazon.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
3 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.
Summary
editor.htmlis deleted; account management is now a fully inline 3-step flow matching the deploy page formatmigprefix shown as a non-editable label; user enters only the 10-char alphanumeric code with strict validation (uppercase A–Z, digits 0–9, at least one letter and one number)Test plan
configurator.htmllocally — verify landing page shows correctly🤖 Generated with Claude Code