Add support to private_vpc (ie: with no public address)

[luhhujbb]

This PR makes the following changes:

• support of private_vpc (ie: with public ip address like this : https://github.com/chaordic/flintrock/pull/4/files)
• add client-source option since in private mode, the IP returned by checkip.amazonaws.com will be a nonsense in most of cases (using a VPN, for instance) and it's sounds good to have all good ports configured automatically in this case, even if custom security groups can be added.
• client-source support both cidr address or security group.

Default behaviour remains identical except that it logs when flintrock detect private mode instead of raising an exception.
Fixes #14

[nchammas]

Hey @luhhujbb, thanks for submitting this PR. I will take a look at it over the holidays!

[nchammas]

Thank you for this contribution @luhhujbb, and my apologies about the delay in reviewing it.

Could you elaborate a bit on why we need to add a --client-source option, vs. just having the user specify additional security groups? It seems to me better to reuse the existing functionality for adding arbitrary rules via additional security groups.

Also, did you manage to run the full test suite against a private VPC (i.e. with USE_AWS_CREDENTIALS=true) to confirm that everything works as expected?

[luhhujbb]

Tests pass with USE_AWS_CREDENTIALS=true in the following condition:

• public subnet (default behaviour)
• private subnet, with client-source option configured with my vpn IP.

[nchammas]

Apologies again about the delayed back and forth. I like the direction of this PR, so please bear with me through the slow review review process. 😅

And thank you for running the full test suite. I am also trying to figure out how to test this on my side, since I've never setup a private VPC that can still talk to the outside world.

[nchammas]

This is looking good @luhhujbb. I will test this out tomorrow and try to get a private VPC working in my environment. I think I need to setup a NAT gateway to enable the private VPC to still talk to the outside world.

Do you mind if I push some commits to your branch? It will mostly be to tweak some method names or message text that is sent to the user. I've been very slow with the review feedback, so for small things I'd like to spare you the time and just do them myself.

If you're OK with that, let me know and just make sure that the "Allow edits from maintainers" checkbox is enabled in the right side-bar.

[luhhujbb]

Hi, @nchammas, of course you can edit, the box "Allow edits from maintainers" is checked.

[nchammas]

I've successfully created a private subnet that accesses the Internet via a NAT gateway on a public subnet in the same VPC. I've also successfully tested out this PR by launching a cluster in that environment. So far so good. 👍

I've pushed the latest changes from master onto your fork, @luhhujbb, and my next steps will be to retest your PR given the recent changes introduced by #285. I will also push some edits as noted in my previous comment.

(By the way, for the future you may want to make contributions from a branch on your fork, as opposed to from master. It will make pulling updates from the upstream source easier down the line.)

[kedar700]

Any update on when this might get merged

[nchammas]

Hello @kedar700. Sorry, I haven't been able to look at this PR recently.

I think this PR has the right implementation but, before it can be merged in, it needs some tweaks and needs merge conflicts to be resolved.

It's my fault for leaving this PR in limbo for so long. (Sorry @luhhujbb!)

[luhhujbb]

I've rebase from master. The python 3.5 compatibility was difficult to handle. Maybe it should be dropped since python 3.5 reaches its end of life.

authorize-access-from is now an array of configurable security-group/network cidr, it allows flexible configuration in a private environment.

[luhhujbb]

@nchammas I've just rebase. Waiting for your review. Thanks for your time.

[nchammas]

Took a quick look now. Looks good. Will take a closer look and retest later this week.

[nchammas]

@luhhujbb - I made some changes to the PR to tweak the wording or style in various places, and also to refactor how we are validating the CLI input. I also think I fixed some bugs, like .appends -> .append. I tested my changes by launching a cluster into a private VPC with both Hadoop and Spark installed.

1. What do you think of the latest changes?
2. Can you test and confirm that the current state of the PR still works for your private VPC use case?

[luhhujbb]

@nchammas First of all many thanks for your time !

1. I've check all your commits and it looks good.
2. I've successfully launch a production cluster using this version of flintrock. Everythinks is Ok. Using multipe "ec2-autorize-access-from" allows us to meet all our production use cases.

[nchammas]

Thanks for persisting through the lengthy review/refactor process! I'm glad we got this in.