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

CloudFormation: stacks might become undeployable because replacement breaks resource relationship constraints #847

Closed
rix0rrr opened this issue Oct 4, 2018 · 2 comments
Labels
@aws-cdk/aws-cloudformation Related to AWS CloudFormation closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/medium Medium work item – several days of effort feature/enhancement A new API to make things easier or more intuitive. A catch-all for general feature requests. feature-request A feature should be added or improved. needs-design This feature request needs additional design work. p1

Comments

@rix0rrr
Copy link
Contributor

rix0rrr commented Oct 4, 2018

The situation

  • I have an (ELBv2) Load Balancer, associated to a TargetGroup (indirectly, but still).
  • A TargetGroup can only ever be associated with one Load Balancer.
  • I've deployed my LoadBalancer with internetFacing = false, and now I want to switch it to internetFacing = true. This change causes replacement of the load balancer.

The problem

For rollbackability, CloudFormation is going to create a new Load Balancer first, and try to associate it with the TargetGroup (which didn't change). This breaks the "Target Group can only ever be associated with one Load Balancer" constraint, and the deployment fails.

This means I can never deploy this update anymore!

The workaround

What needs to happen is that the Target Group gets replaced as well. This will happen if it's logical ID changes, so an easy "enough" workaround is to change its construct ID.

Still, this is not a great experience because:

  • It's not obvious to people that this will get them unstuck (anecdata: I got halfway through writing this post before realizing that would have helped me and I work on this tool 😳). The error message certainly doesn't help, and it's going to be hard to catch it and make the toolkit give hints because it's going to be service-specific.
  • You might not always control the construct IDs to a degree you'd want, especially when they are further down the construct tree or hidden in abstractions.

The better solution

Resources that have a constraint to other resources should have some way of incorporating the immutable values of the other construct into their own logical ID hash.

That way, we can automatically regenerate the ID of the TargetGroup if something on the LB changes, and cause the replacement that we want to happen.

This is feasible because we have the list of immutable properties in our schema, so we can actually calculate this hash.

Potential downsides

  • Might be non-obvious why logical ID changes and replacements happen.
  • Would this cause replacement in cases where you wouldn't want this to happen?
  • What if our schema isn't correct/up-to-date?
@rix0rrr rix0rrr added help wanted We are asking the community to submit a PR to resolve this issue. 🤔 design labels Oct 4, 2018
@eladb
Copy link
Contributor

eladb commented Oct 5, 2018

Interesting! What comes to mind is the little "hack" we implemented for API Gateway deployment resources, which use a hash of the APIGW model to invalidate the logical ID of the deployment, so it will be recreated every time the model changes.

The implementation is here, but maybe we can move it to cdk.Resource.

@debora-ito debora-ito added the @aws-cdk/aws-cloudformation Related to AWS CloudFormation label Nov 7, 2018
@srchase srchase added feature-request A feature should be added or improved. needs-design and removed design labels Jan 3, 2019
@eladb eladb self-assigned this Aug 12, 2019
@eladb eladb added the effort/medium Medium work item – several days of effort label Jan 23, 2020
@SomayaB SomayaB added needs-discussion This issue/PR requires more discussion with community. needs-design This feature request needs additional design work. and removed status/needs-design help wanted We are asking the community to submit a PR to resolve this issue. needs-discussion This issue/PR requires more discussion with community. labels Feb 25, 2020
@eladb eladb added the p1 label Aug 17, 2020
@eladb eladb assigned rix0rrr and unassigned eladb Aug 17, 2020
@ericzbeard ericzbeard added the feature/enhancement A new API to make things easier or more intuitive. A catch-all for general feature requests. label Apr 6, 2021
@github-actions
Copy link

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Jun 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-cloudformation Related to AWS CloudFormation closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/medium Medium work item – several days of effort feature/enhancement A new API to make things easier or more intuitive. A catch-all for general feature requests. feature-request A feature should be added or improved. needs-design This feature request needs additional design work. p1
Projects
None yet
Development

No branches or pull requests

6 participants