fix(amplify-category-storage): make IAM policy names environment-specific - #14962
fix(amplify-category-storage): make IAM policy names environment-specific#14962w3lld1 wants to merge 2 commits into
Conversation
| this.cfnInputParams.s3UploadsPolicy = `Uploads_policy_${userInput.policyUUID}`; | ||
| this.cfnInputParams.authPolicyName = `s3_amplify_${userInput.policyUUID}`; | ||
| this.cfnInputParams.unauthPolicyName = `s3_amplify_${userInput.policyUUID}`; | ||
| const policyNameSuffix = `${userInput.policyUUID}_${this.context.amplify.getEnvInfo().envName}`; |
There was a problem hiding this comment.
New unguarded getEnvInfo().envName dependency yields '_undefined' names or a throw when env info is absent
Name generation now hard-depends on this.context.amplify.getEnvInfo().envName, a call this method never made before (the existing test had to add getEnvInfo to its mock context). There is no check that getEnvInfo() returns an object or that envName is non-empty.
const policyNameSuffix = `${userInput.policyUUID}_${this.context.amplify.getEnvInfo().envName}`;
Why it matters: getEnvInfo() returns no/blank envName (uninitialized env, headless/export/mock paths) → suffix becomes ${uuid}_undefined → all such deployments again share one policy name and re-collide (the very bug being fixed), OR getEnvInfo() throws → transform now fails on a path that previously succeeded (regression).
Suggestion: Read envName once, assert it is a non-empty string, and fail fast with a clear AmplifyError if it is missing, we can add a test for the missing-envName case alongside the happy-path test.
…olicy names The env-specific policy-name suffix read `getEnvInfo().envName` directly. When env info is absent or blank (uninitialized env / headless / export paths) the suffix silently became `..._undefined`, which puts every such deployment back on one shared policy name and re-collides -- the exact bug this change fixes -- or threw an opaque error on a path that previously succeeded. Read the env name once and validate it: fail fast with a clear `EnvironmentNotInitializedError` when it is missing or empty, so names never degrade to a shared `_undefined` suffix. Adds a regression test covering the missing-envName case alongside the existing happy-path test.
2 participants
Description of changes
I made generated S3 IAM policy names environment-specific by appending the current Amplify environment name to the existing policy UUID. This prevents two environments that share imported auth roles from trying to manage identically named inline policies.
I also added a focused regression test for the generated policy names and updated the existing stack-transform expectations.
Issue #, if available
Fixes #14961
Description of how you validated changes
yarn lerna run build --scope @aws-amplify/amplify-category-storage --include-dependencies --concurrency 4node ../../node_modules/jest/bin/jest.js --runInBand --coverage=falsefrompackages/amplify-category-storage(15 suites, 48 tests)I did not run cloud E2E tests or the macOS CI job locally.
Checklist
yarn testpasses for the changed packageBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.