pass valid props to root stack

[renevatium]

Problem

Currently, it is not possible to pass instantiation props to the Amplify root CDK Stack, and, by convention, any Nested Stack created through backend.createStack. This prevents certain resource deployments: An EdgeFunction for instance requires the region to be explicitly set. Furthermore, while Nested Stacks inherit their region from their parent stack, Amplify has a single root node convention, so multiple root stacks are not an option. Evaluation in CDK appears to be strict so the only way to do deploy an EdgeFunction from an Amplify project looks to be at instantiation of an explicit region on the root Stack.

Note: I used 'main' instead of 'root' because that is the naming convention used in the current code.

See #1663 for more information on trying to deploy an EdgeFunction with current version.

Changes

This implementation specifically avoids breaking changes, but does require argument mutation. The relevant args are not typed as const so this may be fine, but if it goes against an established convention, let me know as I can update it easily enough. This was needed because the relevant calls are being made as default argument values in both BackendFactory constructor and createDefaultStack respectively.

This PR introduces a mainStackProps argument to the defineBackend function. This gets passed down the callstack and used to define the root Stack in the AmplifyStack class.

Props for root CDK Stack

AWS documentation for aws-cdk-lib.StackProps

• Props are picked to ensure explicit addition of new StackProps is required.

  • description?: string
  • env?: Environment
  • tags?: { [key: string]: string }
  • analyticsReporting?: boolean
  • crossRegionReferences?: boolean
  • suppressTemplateIndentation?: boolean

• StackProps incompatible with Amplify's intended hierarchy, build or deployment processes are omitted:

  • stackName: Conflicts with dynamic resource naming.
  • synthesizer: Conflicts with managed deployments and resource references.
  • terminationProtection: Conflicts with sandbox/app delete.
  • permissionsBoundary: Conflicts with single root Stack ethos (i.e. Unable to create Role prior to defineBackend).

Props are passed down from defineBackend. e.g:

defineBackend({
  auth,
  storage,
}, {
  description: 'test-description',
  env: {
    region: 'us-east-1',
    account: '[YOUR_ACCOUNT_ID]'
  },
  tags: {
    'test-tag': 'test-tag-value',
  },
  analyticsReporting: true,
  crossRegionReferences: true,
  suppressTemplateIndentation: true,
})

Validation

All existing unit, e2e and deployment tests pass. Coverage is unchanged.

Using the npm proxy, I have fully deployed and tested the EdgeFunction case with various configurations.

This change simply passes props tested in aws-cdk-lib/core/test/stack.test.ts. i.e It already follows defined convention. As such, I am not sure what automated testing would actually be useful. I did try though, so please let me know if you would like me to some tests back in?

I started out writing unit tests, but struggled to find a way to validate anything useful there. I could potentially check that ${AWS::Region} does not exist in the generated Stack Template but that seems like a stretch.

I wrote e2e and deployment tests, but they don't really do much to actually test anything in scope and they add a lot of unnecessary overhead. The deployment tests for instance, require that I update the test stack generation process and/or write the tests without following the conventions established by the existing test structure. Again, if you would prefer these be in, I can put them back.

Checklist

• [] If this PR includes a functional change to the runtime behavior of the code, I have added or updated automated test coverage for this change.
• [] If this PR requires a change to the Project Architecture README, I have included that update in this PR.
• [] If this PR requires a docs update, I have linked to that docs PR above.
• [] If this PR modifies E2E tests, makes changes to resource provisioning, or makes SDK calls, I have run the PR checks with the run-e2e label set.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: 898a4ccb87eb3e0ab34a3e3bf72b0dd16f18e711

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@aws-amplify/backend	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[renevatium]

@sobolk Thank you for reviewing my PR.

Please let me know if I should rebase into a single commit at some point or if you prefer the history.

[sobolk]

@sobolk Thank you for reviewing my PR.

Please let me know if I should rebase into a single commit at some point or if you prefer the history.

We're using "squash merge" strategy for pull request so rebase is not necessary. It's actually not desired as it makes incremental reviews impossible to track in GitHub UI sometimes.

[josefaidt]

Hey @renevatium :wave: thanks for taking the time to file this! We agree this is something we'd like to address, though are actively discussing this internally to gauge the direction.

[renevatium]

Hi @josefaidt 👋 No problem at all, big proponent of Amplify and keen to be more a part of the community.

Considering it a point of conversation right now, I'll put my 2 cents in: Currently, this PR is a decent solution for my issue, but if I consider future flexibility, it does seem like exposing the explicit functionality necessary to define the App and Root Stack as independent objects and passing them into the (exposed) BackendFactory may be the most flexible state. I realize this conflicts with convention > config, but with CDK available in gen2, it feels like more custom config options/structure on the backend side will offer devs the most control with the least (necessary) limitation. Also, this would align with the current unit testing methodology, which doesn't use defineBackend, but rather explicitly defines App/Stack objects.

[sisygoboom]

Amplify needs this desperately, without it I can't add a function created via defineFunction to a vpc imported into a backend.createStack. Are you any closer to a solution @josefaidt - its been 3 months now? Error: Cannot retrieve value from context provider vpc-provider since account/region are not specified at the stack level. Configure "env" with an account and region when you define your stack.