Skip to content

fix(gen2-migration): skip Identity Pool when Gen1 auth is User Pool-only - #14971

Merged
sharonyajain merged 3 commits into
devfrom
fix/gen2-migration-optional-identity-pool-14742
Aug 21, 2026
Merged

fix(gen2-migration): skip Identity Pool when Gen1 auth is User Pool-only#14971
sharonyajain merged 3 commits into
devfrom
fix/gen2-migration-optional-identity-pool-14742

Conversation

@sharonyajain

Copy link
Copy Markdown
Contributor

Problem

When migrating a Gen1 Amplify project to Gen2 using amplify gen2-migration generate, the migration tool creates Identity Pool configuration even when the Gen1 project was configured with "User Sign-Up & Sign-In only" — which provisions only a Cognito User Pool with no Identity Pool dependency.

This changes the auth architecture from JWT-based (User Pool authorizer) to IAM-based (which requires an Identity Pool), creates orphaned IAM roles, and breaks ampx generate outputs if the Identity Pool is later removed via CDK escape hatches.

Closes #14742

Root Cause

auth.generator.ts unconditionally called this.gen1App.resourceMetaOutput(this.resource, 'IdentityPoolId') which throws if the key is absent. Even when present (Gen1 always creates an Identity Pool), the renderer emitted Identity Pool escape-hatch code without checking whether the pool was actually in use.

Changes

  • gen1-app.ts: Add tryResourceMetaOutput() — a non-throwing variant that returns undefined for absent output keys
  • auth.generator.ts: Use tryResourceMetaOutput for IdentityPoolId; skip fetchIdentityPool when absent
  • auth.renderer.ts: Guard all Identity Pool escape hatches (cfnIdentityPool, cognitoIdentityProviders push, addPropertyDeletionOverride) on !!options.identityPool
  • auth.generator.test.ts: Add regression test for User Pool-only configuration (no IdentityPoolId in output)

Testing

  • New test: generates auth without Identity Pool when IdentityPoolId is absent
  • Verifies fetchIdentityPool is never called
  • Verifies generated code contains no cfnIdentityPool, cognitoIdentityProviders, or cognitoProviders references
  • Existing tests still pass (they all provide IdentityPoolId in their meta)

@sharonyajain
sharonyajain requested review from a team as code owners August 18, 2026 23:27
@sharonyajain
sharonyajain force-pushed the fix/gen2-migration-optional-identity-pool-14742 branch 9 times, most recently from e1fe2d6 to 39ddc83 Compare August 19, 2026 13:20
@sharonyajain

Copy link
Copy Markdown
Contributor Author

CI Failure Note

The build-and-test check failure is a pre-existing repo-wide issue unrelated to this PR's changes. The same failure reproduces on other PRs (e.g. feat/gen2-migration-non-js-functionsrun 32198932303).

Root cause

GitHub Actions recently deprecated Node 20 on runners and forces all actions to run under Node 24. Under Node 24, Jest's process exit behavior is stricter with open async handles — specifically Socket listeners from AWS SDK client mocks in the gen2-migration snapshot tests prevent cli-internal from exiting cleanly (exit code 1 from open handles, not from test failures).

All cli-internal test suites PASS (including the new regression test added in this PR). The exit code 1 comes from Jest being unable to shut down the process after tests complete.

What this PR includes to mitigate

  • SpinningLogger singleton SIGINT handler — fixes the MaxListenersExceededWarning: 11 SIGINT listeners accumulation (each SpinningLogger instance was registering a new handler; now uses a static singleton pattern)
  • --forceExit --detectOpenHandles on the cli-internal test script — matches the pattern already used by amplify-appsync-simulator and amplify-provider-awscloudformation in this repo

What remains (out of scope for this PR)

The Node 24 runner upgrade requires either:

  1. Fixing the remaining Socket handle leaks in the AWS SDK mock clients used by gen2-migration tests, OR
  2. Adding timeout-minutes to the workflow, OR
  3. Splitting the cli-internal test suite into shards

This blocks all PRs to this repo equally and should be tracked as a separate infrastructure issue.

@sharonyajain
sharonyajain force-pushed the fix/gen2-migration-optional-identity-pool-14742 branch 3 times, most recently from 743ee9c to 8b529dd Compare August 19, 2026 14:15
@sharonyajain
sharonyajain requested a review from sarayev August 19, 2026 14:22
When a Gen1 project is configured with 'User Sign-Up & Sign-In only' (no
Identity Pool), the migration tool now correctly omits Identity Pool escape
hatches and cognitoIdentityProviders registration from the generated code.

Previously, the generator unconditionally fetched IdentityPoolId from
amplify-meta.json (throwing if absent) and always emitted Identity Pool
references in applyEscapeHatches(). This caused User Pool-only setups to
gain an unwanted Identity Pool, changing the auth architecture from JWT-based
to IAM-based.

Changes:
- Add Gen1App.tryResourceMetaOutput() for optional output keys
- Make IdentityPoolId lookup non-throwing via tryResourceMetaOutput
- Skip fetchIdentityPool when no IdentityPoolId is present
- Guard cfnIdentityPool escape hatches on identityPool presence
- Skip cognitoProvidersPushStatements when no Identity Pool exists
- Add regression test for User Pool-only configuration
- Fix SpinningLogger to register SIGINT handler once (singleton pattern)
  instead of per-instance, preventing MaxListenersExceededWarning and
  allowing clean process exit under Node 24

Closes #14742
@sharonyajain
sharonyajain force-pushed the fix/gen2-migration-optional-identity-pool-14742 branch from 8b529dd to cc79b34 Compare August 19, 2026 14:26
@sharonyajain
sharonyajain requested a review from soberm August 19, 2026 15:03
Comment thread packages/amplify-cli/package.json Outdated
Comment thread packages/amplify-cli/src/commands/gen2-migration/_common/spinning-logger.ts Outdated
… Pool fix

- Remove --forceExit from test script; real handle leak resolved by
  SpinningLogger singleton + MockClients cleanup (full suite exits clean)
- Dedup resourceMetaOutput by delegating _meta traversal to tryResourceMetaOutput
- SpinningLogger unregisters from instances on spinner deactivation so the
  set only holds active spinners instead of growing unbounded
soberm
soberm previously approved these changes Aug 20, 2026
@sharonyajain
sharonyajain removed the request for review from sarayev August 20, 2026 12:58
@sarayev

sarayev commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

The destination branch should be dev, main is for releases. See here: https://github.com/aws-amplify/amplify-cli/blob/dev/CONTRIBUTING.md?plain=1#L120

@sharonyajain
sharonyajain changed the base branch from main to dev August 20, 2026 13:16
@sharonyajain
sharonyajain dismissed soberm’s stale review August 20, 2026 13:16

The base branch was changed.

@sharonyajain
sharonyajain merged commit 0784724 into dev Aug 21, 2026
5 checks passed
@sharonyajain
sharonyajain deleted the fix/gen2-migration-optional-identity-pool-14742 branch August 21, 2026 09:19
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.

(gen2-migration) generate adds Identity Pool when Gen1 auth was configured as User Sign-Up & Sign-In only (User Pool only)

3 participants