jtriley / StarCluster

StarCluster is an open source cluster-computing toolkit for Amazon's Elastic Compute Cloud (EC2).
http://star.mit.edu/cluster
GNU Lesser General Public License v3.0
582 stars 308 forks source link

use DNS names instead of IP addresses in permissions (done, but what to call it?) #150

Open bspratt opened 12 years ago

bspratt commented 12 years ago

I've implemented an alternative to cidr_ip that lets you lock down a port range to a DNS name. It resolves DNS and populates cidr_ip at startup (and yes, the cidr value becomes stale if the DNS record changes during cluster lifetime, but there you have it - AWS doesn't support DNS names in security groups).

Working name is "cidr_dns", which I don't love - anyone have an alternate suggestion?

jtriley commented 12 years ago

@bspratt Awesome! Do you have the code in a branch somewhere I could take a look at? I'm assuming you always set the cidir range to /32 when populating cidr_ip?

I need to double check but I believe it's possible to add multiple cidr_ip ranges to a given security group rule and if that's the case we could allow cidr_ip to be a list and then make your new DNS option a list as well and call it 'allow_hosts' or something. What do you think?

jtriley commented 12 years ago

or 'require_hosts'

bspratt commented 12 years ago

Correct, you end up with a rule like this: Allow 80 (HTTP) 173.157.204.231/32

Yeah, I think "require_hosts" is good.

I like the idea of a cidr_ip list, although I think that might actually have to result in multiple rules - one CIDR per rule IIRC.

As long as we're messing with cidr_ip, what if it was allowed to accept DNS names (and we ditched the require_hosts thing)? They'd have to be structured somehow so that they weren't mistaken for security groups - maybe something like uri:mydomain.org .

I'll go ahead and push my current "cidr_dns" code up to my Github fork. Then I guess I have to enable you for poking around in there, I'll have to go figure out how that's done.

jtriley commented 12 years ago

@bspratt I just experimented and was able to add another CIDR to an existing rule so I think multiple CIDR's per rule is OK but I'll need to do some more testing. I'm fine with extending cidr_ip and not adding another option so long as the current implementation works for existing users. Given that we're resolving the DNS ourselves it should be easy to distinguish an IP from a dns name (via regex) from the config so I dont think the "uri:" prefix is necessary but I could be wrong.

Just simply pushing your branch/code is enough so that I can take a look at it. No need to enable anything...

bspratt commented 12 years ago

Yeah, I hadn't checked to see if my fork was public or not, it is: https://github.com/bspratt/StarCluster.git

My only concern is that we can reliably tell URIs from security groups. I suppose security groups never have . in them, and URIs always do, and an actual IP CIDR is easy to match.

bspratt commented 12 years ago

OK, cidr_ip now handles actual CIDR references as well as URIs, and it handles multiple entries as a space seperated list. Pushed to my fork on github.

jtriley commented 11 years ago

This should be closed when #182 gets merged