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

iptables rules for servo-master host #108

Closed
wants to merge 1 commit into from
Closed

Conversation

@edunham
Copy link
Contributor

edunham commented Sep 1, 2015

This opens all of the ports currently available on servo-master, which is both saltmaster and buildmaster.

22 (&TCP6): SSH. Whitelist to bastion if we get one, otherwise allow from anywhere
80: nginx for Buildbot status page. Allow from anywhere.
4505, 4506: Salt master; allow from minions only
54856: Homu (python3)
9001, 8010: Buildbot
UDP & UDP6
123: NTP

The only other ports currently in use on the master are for dhclient, which is unnecessary since it has a static IP address.

It seems like Salt should have a better way to get minion IPs for line 112, but some searching revealed conflicting advice, so I figured it would be more efficient to just consult the PR's reviewer. I got the list from https://github.com/servo/servo/wiki/Buildbot-administration .

I checked the iptables state source and it looks like rules are only saved if they do not match an existing rule, so they won't get rewritten on each run.

Review on Reviewable

@metajack
Copy link
Contributor

metajack commented Sep 1, 2015

We'll need to set up static IPs for the EC2 slaves I guess and we'll have to use those IPs here.


Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks broke.


buildbot-master.sls, line 112 [r1] (raw file):
This is not going to work. The EC2 slaves do not have static IPs currently.


Comments from the review on Reviewable.io

@edunham
Copy link
Contributor Author

edunham commented Sep 1, 2015

@metajack Compared to leaving the ports open to the world or opening the ports to anyone in our EC2 region, setting up elastic IPs does look like the best option.

@bors-servo
Copy link
Contributor

bors-servo commented Sep 22, 2015

The latest upstream changes (presumably #96) made this pull request unmergeable. Please resolve the merge conflicts.

# doesn't seem like the right way.
{% for minion in '208.52.161.130', '208.52.161.128', '63.135.170.19',
'208.52.170.250', '66.228.48.56', '173.255.201.95', '45.79.167.177',
'72.14.176.110','96.126.114.185', '0.0.0.0' %}

This comment has been minimized.

@claudijd

claudijd Mar 18, 2016

Just an FYI here, but having '0.0.0.0' in the list I would suspect has an ANY effect, which makes the other IPs redundant.

@edunham edunham force-pushed the edunham:iptables branch 5 times, most recently from 7320ea0 to 37d1576 Mar 22, 2016
@edunham edunham force-pushed the edunham:iptables branch from 37d1576 to b30ddab Mar 22, 2016
@edunham edunham closed this May 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.