Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): Stacks render CloudFormation elements in nested Stages #10156

Merged
merged 3 commits into from
Sep 8, 2020

Conversation

rix0rrr
Copy link
Contributor

@rix0rrr rix0rrr commented Sep 3, 2020

In current recommended CDK Pipelines usage, the construct tree looks
like this:

App -> Stack1 -> Stage -> Stack2 -> Resource

It's an easy mistake to forget Stack2 (or make it a generic
Construct instead), in which case the hierachy looks like the one
below and Stack1 will render Resource in its CloudFormation template:

App -> Stack1 -> Stage -> Construct -> Resource

This case should not have been allowed in the first place: Stages
define a new assembly scope and so Stack1 does not exist from the
point of view of Resource.

Change Stack.of() to fail to find Stack1 in this case, so
that it becomes illegal to define Resource in this position.

Fixes #9792, relates to #9669.


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

In current recommended CDK Pipelines usage, the construct tree looks
like this:

```
App -> Stack1 -> Stage -> Stack2 -> Resource
```

It's an easy mistake to forget `Stack2` (or make it a generic
`Construct` instead), in which case the hierachy looks like the one
below and `Stack1` will render Resource in its CloudFormation template:

```
App -> Stack1 -> Stage -> Construct -> Resource
```

This case should not have been allowed in the first place: `Stage`s
define a new assembly scope and so `Stack1` does not exist from the
point of view of `Resource`.

Change `Stack.of()` to fail to find `Stack1` in this case, so
that it becomes illegal to define `Resource` in this position.

Fixes #9792, relates to #9669.
@rix0rrr rix0rrr requested a review from a team September 3, 2020 13:44
@rix0rrr rix0rrr self-assigned this Sep 3, 2020
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Sep 3, 2020
new core.CfnResource(app, 'Resource', {
type: 'Some::Resource',
});
}, /No stack could be identified for the construct/);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is likely 'new to the CDK' error and the message for someone like that is a bit vague.

Either something a bit more clear to tell the user that they've done something incorrectly or an action to correct it may help.

@mergify
Copy link
Contributor

mergify bot commented Sep 8, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@nija-at nija-at added the pr/do-not-merge This PR should not be merged at this time. label Sep 8, 2020
@rix0rrr rix0rrr removed the pr/do-not-merge This PR should not be merged at this time. label Sep 8, 2020
@mergify
Copy link
Contributor

mergify bot commented Sep 8, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify
Copy link
Contributor

mergify bot commented Sep 8, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit 5f36f6b into master Sep 8, 2020
@mergify mergify bot deleted the huijbers/stacks-outside-stages-dont-count branch September 8, 2020 12:47
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 5e91fca
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[core] Resources in Stages in Stacks are synthesized into the parent template
3 participants