Skip to content
This repository has been archived by the owner on Jul 7, 2021. It is now read-only.

Implement EFS (Elastic File System) blueprint #129

Merged
merged 4 commits into from
Apr 23, 2018

Conversation

danielkza
Copy link
Contributor

Add an Elastic File System blueprint.

It creates FileSystem instances and its associated MountTargets in a
specified list of Subnets, which makes things quite a bit easier than
creating them individually.

While it is uncommon, EFS allows specifying the IP addreses of its
internal instances manually, so we allow that too.

Since EFSs are commonly accessed from many different machines, a
convenience AllowedCIDRs variable is provided, that makes it easy to add
inbound rules to the newly-created security group for the FS.

Copy link
Contributor

@phobologic phobologic left a comment

Choose a reason for hiding this comment

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

Another comment about the AllowedCIDRs idea. Thanks for this @danielkza !

'Corresponds to Subnets listed in the same order.',
'default': ''
},
'AllowedCIDRs': {
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about using a TroposphereType here for the type SecurityGroupIngress? That'd make this more flexible. I have similar concerns to what I mentioned in the RDS blueprint though - we've found that managing the Rules outside of the stack where other resources are located has been helpful in the past. These days I'm even starting to lean torwards managing the entire SecurityGroup itself outside of the stack where the resources live, but that changes so infrequently that I don't think it matters as much.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Very fair point, I'll change it to use a TroposphereType entry. I actually gave up on EFS entirely though - it was unacceptably slow to store a small amount of data for website hosting :(

Copy link
Contributor

Choose a reason for hiding this comment

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

Iiinteresting - we haven't used it yet, though I have an inherent distrust of anything based on NFS after years of being abused by that protocol many, many years ago :)

@phobologic
Copy link
Contributor

Oh, also, there's an issue with the tests that needs to be resolved.

@danielkza
Copy link
Contributor Author

danielkza commented Aug 26, 2017

Updated with a new approach to handle the security groups: complete groups can now be defined and assigned through the SecurityGroups variable, and pre-existing groups can be added with the ExtraSecurityGroups var. It's not as easy as just passing the CIDRs, but it's much more flexible and easier than using a separate stack.

I also added a test to check if the blueprint works correctly. I also deployed it to my AWS account and it seemed to work fine.

Unfortunately this will be broken until a new stacker release is cut (with the TroposphereType changes). I'm fine with waiting or updating the requirement temporarily, either works.

@russellballestrini
Copy link
Contributor

russellballestrini commented Apr 12, 2018

@danielkza we have had a new stacker release since your last comment. Would you have a chance to pick this PR back up?

CC: @aarcro

Verified

This commit was signed with the committer’s verified signature.
Gigioliva Gianluigi Oliva
Add an Elastic File System blueprint.

It creates FileSystem instances and its associated MountTargets in a
specified list of Subnets, which makes things quite a bit easier than
creating them individually.

While it is uncommon, EFS allows specifying the IP addreses of its
internal instances manually, so we allow that too.

Since EFSs are commonly accessed from many different machines, a
convenience AllowedCIDRs variable is provided, that makes it easy to add
inbound rules to the newly-created security group for the FS.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
- Apply Tags to all supported resources, not just the filesystem
- Validate the number of SecurityGroups, Subnets and IpAddresses
@danielkza
Copy link
Contributor Author

@russellballestrini @aarcro Rebased to the latest master, and updated with some improvements 👍

@russellballestrini russellballestrini merged commit 76a8006 into remind101:master Apr 23, 2018
@russellballestrini
Copy link
Contributor

@danielkza thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants