feat: support multiple GuInstanceRoles in a stack #332
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
akash1810
force-pushed
the
aa-ssm-singleton
branch
2 times, most recently
from
9eca124
to
d94951f
Compare
akash1810
force-pushed
the
aa-ssm-singleton
branch
from
d94951f
to
49c13da
Compare
akash1810
force-pushed
the
aa-ssm-singleton
branch
3 times, most recently
from
8ddf842
to
d638337
Compare
akash1810
changed the title
akash1810
force-pushed
the
aa-ssm-singleton
branch
from
d638337
to
4d8f9f9
Compare
akash1810
commented
Mar 19, 2021
| @@ -36,4 +36,20 @@ describe("The GuInstanceRole construct", () => { | |||
| expect(stack).toCountResources("AWS::IAM::Role", 1); | |||
| expect(stack).toCountResources("AWS::IAM::Policy", 5); | |||
| }); | |||
|
|
|||
| it("should be possible to create multiple instance roles in a single stack", () => { | |||
There was a problem hiding this comment.
This is the most interesting part of this PR 😅 .
akash1810
added a commit
that referenced
this pull request
Mar 24, 2021
Following #337 and part of a bigger bit of work in #332, this makes `GuDistributionBucketParameter` a singleton as we only ever want one of them in a stack. BREAKING CHANGE: * The removal of the static `parameterName` field means `GuDistributionBucketParameter.parameterName` becomes `GuDistributionBucketParameter.getInstance(stack).id`
akash1810
force-pushed
the
aa-ssm-singleton
branch
from
4d8f9f9
to
8e0ff20
Compare
GuSSMRunCommandPolicy doesn't need to be declared multiple times in a multi-app stack, make it a singleton to enforce this.
akash1810
force-pushed
the
aa-ssm-singleton
branch
from
8e0ff20
to
12204d5
Compare
philmcmahon
approved these changes
Mar 24, 2021
|
🎉 This PR is included in version 6.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this change?
A
GuInstanceRoleprovides the basic permissions needed by EC2, it grants access to:#326 started the work to support the declaration of multiple apps in a stack, where an app is a collection of ASG, LB etc. However, when actually attempting to use the updated constructs in reality, the synth step failed due to multiple constructs using the same ID.
This change follows #341 and #337 and converts
GuSSMRunCommandPolicyandGuDescribeEC2Policyto singletons. This means these policies will only get created once regardless of how many roles use them - there will be one policy with multiple roles attached to it. This helps keep the stack DRY and also reduces the chance of reaching the max file size limits of cloudformation.The commits have been crafted to be standalone, reviewing them one by one might make the overall review easier.
Requires #341.
BREAKING CHANGE:
GuSSMRunCommandPolicyandGuDescribeEC2Policycan no longer be directly instantiatedDoes this change require changes to existing projects or CDK CLI?
Ultimately no, as
GuInstanceRolehasn't fundamentally changed.How to test
See new test!
How can we measure success?
It's even more possible to declare multiple apps within a stack.
Have we considered potential risks?
n/a