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

RFC 309: Loosely Coupled Cross Stack Ref #311

Closed
wants to merge 5 commits into from
Closed

Conversation

corrjo
Copy link
Contributor

@corrjo corrjo commented Apr 8, 2021


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

@corrjo
Copy link
Contributor Author

corrjo commented Apr 8, 2021

@corrjo corrjo changed the title lRFC: 309 Loosely Coupled Cross Stack Ref RFC: 309 Loosely Coupled Cross Stack Ref Apr 12, 2021
Comment on lines +65 to +66
Tight coupling means the resources you are dependent on wont go away, in some instances
this behavior prevents breaking consumers.
Copy link

@michaelfecher michaelfecher Apr 12, 2021

Choose a reason for hiding this comment

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

How do you handle updates? You would need to notify the consumer stacks, right? Or rely on versioning?
Same question for delete operation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm currently creating a dependency on the provider stack https://github.com/aws/aws-cdk/pull/13804/files#diff-d21961a629629b87c50d664aaf41594608e31d6ae693a2fc586062cdb3403eeeR175

The idea is when the provider is updated, it'll publish the new value and then cdk triggers a stack update on the dependent stacks and thats where they'll resolve the new parameter value.

But perhaps that dependency isn't enough to force an update of the consumer stack after the provide has been updated? I'll have to verify that it works as expected.

Copy link

Choose a reason for hiding this comment

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

I've put some comments in aws/aws-cdk#13184 (comment) that can be related to this

Copy link
Contributor

@rix0rrr rix0rrr left a comment

Choose a reason for hiding this comment

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

Thanks for writing up a proposal!

Some things I'd like to see this RFC address:

  • What the use cases we're unlocking? What are the problems we're addressing? (Not hand-wavey "things could happen", bus describe real, concrete cases please)
  • Why are the current solutions not good enough? (the stack.exportValue() method currently)
  • WHY is it okay to drop the hard-reference guarantees that exports provide? Either, convince the reader it's okay to take downtime while resources are being replaced, or otherwise, what are the assumptions you're making which will cause that to not occur in practice?
  • Do we ever want to support having both types of references in one stack, but pin-pointed for different types of resources? I think yes, what would that look like? Honestly, we can reuse context, but we don't check context at the stack level but at the producer construct, and that would probably do it...?
  • What does the migration path from hard references to weak references look like? You can't just switch over the flag because CFN won't let you deploy that. I think the flag needs to be tri-state: 'hard' | 'weak' | 'both', where you use both in the cutover period to still produce the required exports, but only consume the weak references.


### README

#### Loosely Coupled Stack References
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I'd prefer the term "weak cross-stack references"

Choose a reason for hiding this comment

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

@rix0rrr https://www.endoflineblog.com/cdk-tips-03-how-to-unblock-cross-stack-references describes the workaround necessary to delete a resource that's exported from one stack and consumed by another using the tightly-coupled approach. Moving to loosely-coupled puts the responsibility on the shoulders of the user, which, I think is reasonable, or at least it's an option I'd use if it were available.

I can't think of any case where the strong guarantees of export are enough of a benefit that I'd actually pick them over params. I would absolutely LOVE to be able to delete a stack which other stacks depend on. Because while it may sound like the wrong decisions and may even be the wrong decision most of the time, there are times when I could imagine it would be the best of a bunch of bad options.

I really like your observation about both mode though. I thing that the export model really needs to find a way to "still produce the required exports" in the case where resources are being deleted whether they're being replaced by params or just refactored out of the code in general.

@eladb eladb changed the title RFC: 309 Loosely Coupled Cross Stack Ref RFC 309: Loosely Coupled Cross Stack Ref Jun 22, 2021
@nija-at nija-at self-assigned this Jul 20, 2021
@nija-at
Copy link
Contributor

nija-at commented Jul 20, 2021

RFC Bar Raiser: @nija-at

Copy link
Contributor

@nija-at nija-at left a comment

Choose a reason for hiding this comment

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

Setting this to 'request changes' for comments to be addressed.

@skinny85 skinny85 linked an issue Sep 11, 2021 that may be closed by this pull request
@nija-at
Copy link
Contributor

nija-at commented Nov 25, 2021

Closing this RFC for now. Please re-open or create a new one if anyone is interested in picking this up.

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.

Parameter Store for cross stack references
6 participants