-
Notifications
You must be signed in to change notification settings - Fork 156
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: sns topic creation should not be idempotent #3809
Conversation
This is a re-roll of #3235. See this comment for why we are re-rolling this. closes #3288 Co-authored-by: VenelinMartinov [email protected]
@t0yv0 I recreated this because I totally botched trying to rebase from master after the upgrade. |
Does the PR have any schema changes?Does the PR have any schema changes?Looking good! No breaking changes found. Maintainer note: consult the runbook for dealing with any breaking changes. |
Is README.md missing any configuration options?
Please add a description for each of these options to |
+ if val, ok := snsGlobalMutex.LoadOrStore(topicArn, localMutex); ok { | ||
+ localMutex = val.(*sync.Mutex) | ||
+ } | ||
+ localMutex.Lock() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks right. Thank you! Much appreciated taking some extra care to avoid a global chokepoint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚢
When merging please update the merge commit with the original description - we build release notes from the commit messages and it's great to have a good summary there of what the users are getting. Much appreciated. |
This is a re-roll of #3235.
See this comment for why we are re-rolling this.
Previously the creation of an SNS topic was idempotent, so you could create multiple pulumi resources corresponding to the same SNS topic, causing confusing behaviour when one was deleted/replaced.
With this PR, trying to create a topic which exists will error.
Note that this is kind of a breaking change - pulumi programs where the same SNS topic was created through multiple pulumi resources would run fine before but will now error.
Note also the mutex around the listing and creating of SNS topics - this is needed to prevent race conditions within the same program. Race conditions with other programs are still possible and that is unavoidable because of the AWS API.
fixes #3288, fixes #2009.
Co-authored-by: VenelinMartinov [email protected]