ansible: update iptables playbook

[richardlau]

• Add a filter to skip over hosts that do not have an IP address, which
  can happen if they have been removed from the main inventory but not
  from secrets. Also filter out hosts that use hostnames instead of
  numeric IP addresses (i.e. the Windows hosts).
  Allow the proxy host look up when ansible is run in --check mode.

• Add filter for looking up IP addresses via dig. Add entries for
  hosts that set their ip to a hostname instead of a numeric IP
  address -- in the iptables rules we are adding these commented with
  the supplied ip hostname rather than the name of the host.

[richardlau]

I've used this to update the firewalls for #2628.

I'll look at updating the documentation in https://github.com/nodejs/build/blob/master/ansible/MANUAL_STEPS.md#adding-firewall-entries-for-jenkins-workers. FWIW the ansible script updates the live iptables state, but iptables-save > /etc/iptables/rules.v4 still needs to be run to persist the changes across reboots.

[rvagg]

I'm not familiar with this script but the changes seem sensible to me

[richardlau]

I'm not familiar with this script but the changes seem sensible to me

I only came across while setting up the replacement ci-release server. It's additive (i.e doesn't remove things from the iptables rules) so is reasonably safe to run.

I've added a follow up commit to handle the case with the Windows hosts which specify their ip in the inventory as a hostname. We're currently specifying those in the iptables rules with a comment referencing the hostname supplied as ip instead of the name of the host in the inventory. For example:

  - test:
    - azure:
        msft-win10_vcbt2015-x64-1: {ip: nodejs.eastus2.cloudapp.azure.com}

in the inventory currently has this iptables rule:

-A jnlp -s 137.116.94.125/32 -m comment --comment "nodejs.eastus2.cloudapp.azure.com" -j ACCEPT

instead of

-A jnlp -s 137.116.94.125/32 -m comment --comment test-azure_msft-win10_vcbt2015-x64-1 -j ACCEPT

[mhdawson]

@richardlau should this land?

[richardlau]

@richardlau should this land?

Yes, done. I tested the playbook again after rebasing before merging.