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: self managed stackset dependencies #98

Merged
merged 1 commit into from
Apr 17, 2023

Conversation

cgeers
Copy link
Contributor

@cgeers cgeers commented Apr 17, 2023

adds an explicit dependency between MgmtAccEBRuleStackSet and associated self managed roles Even though there is an implicit dependency because of the use of Fn::GetAtt, we also need an explicit dependency to handle stack teardown gracefully. Adds cfn-lint exclusion

adds an explicit dependency between MgmtAccEBRuleStackSet and associated self managed roles
Even though there is an implicit dependency because of the use of Fn::GetAtt, we also need
an explicit dependency to handle stack teardown gracefully. Adds cfn-lint exclusion
@cgeers cgeers requested a review from a team as a code owner April 17, 2023 16:47
@sameer-in
Copy link
Contributor

@cgeers As per cfn-lint the dependency is implicit. so do we need to add it manually?

@cgeers
Copy link
Contributor Author

cgeers commented Apr 17, 2023

@cgeers As per cfn-lint the dependency is implicit. so do we need to add it manually?

We don't need to have our behavior dictated by default linting rules, and it's clear that with just the implicit dependency, the race still exists. Doing something seems better than doing nothing.

TBF though, I've not tested this enough to know if the explicit dependence is enough to properly trigger teardown order properly consistently, but it seems like a pretty reasonable course of action to me.

@sameer-in sameer-in self-requested a review April 17, 2023 21:24
@cgeers cgeers merged commit 3baa64f into main Apr 17, 2023
4 checks passed
@cgeers cgeers deleted the fix/self_managed_stackset_dependencies branch April 17, 2023 21:35
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.

None yet

3 participants