Skip to content

Commit

Permalink
[stable-2.9] vmware_host_firewall_manager: fix ansible#61332 (ansible…
Browse files Browse the repository at this point in the history 
…#63567)

Since ansible#56733, we were not able to apply
firewall rules with no `allowed_hosts` key.

closes: ansible#61332

In addition, this patch ensures the `allowed_hosts` key accepts a dict,
instead of a dict in a single entry list.

```yaml
vmware_host_firewall_manager:
  esxi_hostname: "{{ esxi1 }}"
  rules:
    - name: NFC
      enabled: True
      allowed_hosts:
        - all_ip: False
          ip_address:
            - "1.2.3.4"
```

Should be written:

```yaml
vmware_host_firewall_manager:
  esxi_hostname: "{{ esxi1 }}"
  rules:
    - name: NFC
      enabled: True
      allowed_hosts:
        all_ip: False
        ip_address:
          - "1.2.3.4"
```

(cherry picked from commit ab2aaca)
  • Loading branch information
@goneri
goneri committed Nov 4, 2019
1 parent c486638 commit 67d8ec7
Show file tree
Hide file tree
Showing 4 changed files with 170 additions and 109 deletions.
4 changes: 4 additions & 0 deletions changelogs/fragments/vmware_host_firewall_manager_fix_61332.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
bugfixes:
- vmware_host_firewall_manager - Ensure we can set rule with no ``allowed_hosts`` key (https://github.com/ansible/ansible/issues/61332)
minor_changes:
- vmware_host_firewall_manager - ``allowed_hosts`` excpects a dict as parameter, list is deprecated
174 changes: 98 additions & 76 deletions lib/ansible/modules/cloud/vmware/vmware_host_firewall_manager.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,23 +99,24 @@
- name: gdbserver
enabled: True
allowed_hosts:
- all_ip: False
ip_address:
192.168.20.10
all_ip: False
ip_address:
- 192.168.20.10
- 192.168.20.11
- name: CIMHttpServer
enabled: True
allowed_hosts:
- all_ip: False
ip_network:
192.168.100.0/24
all_ip: False
ip_network:
- 192.168.100.0/24
- name: remoteSerialPort
enabled: True
allowed_hosts:
- all_ip: False
ip_address:
192.168.100.11
ip_network:
192.168.200.0/24
all_ip: False
ip_address:
- 192.168.100.11
ip_network:
- 192.168.200.0/24
delegate_to: localhost
'''

Expand Down Expand Up @@ -203,6 +204,43 @@ def gather_rule_set(self):
temp_rule_dict['allowed_hosts'] = rule_allow_host
self.firewall_facts[host.name][rule_set_obj.key] = temp_rule_dict

def check_params(self):
rules_by_host = {}
for host in self.hosts:
rules_by_host[host.name] = self.firewall_facts[host.name].keys()

for rule_option in self.rule_options:
rule_name = rule_option.get('name')
if rule_name is None:
self.module.fail_json(msg="Please specify rule.name for rule set"
" as it is required parameter.")
hosts_with_rule_name = [h for h, r in rules_by_host.items() if rule_name in r]
hosts_without_rule_name = set([i.name for i in self.hosts]) - set(hosts_with_rule_name)
if hosts_without_rule_name:
self.module.fail_json(msg="rule named '%s' wasn't found on hosts: %s" % (
rule_name, hosts_without_rule_name))

if 'enabled' not in rule_option:
self.module.fail_json(msg="Please specify rules.enabled for rule set"
" %s as it is required parameter." % rule_name)

allowed_hosts = rule_option.get('allowed_hosts', {})
ip_addresses = allowed_hosts.get('ip_address', [])
ip_networks = allowed_hosts.get('ip_network', [])
for ip_address in ip_addresses:
try:
ipaddress.ip_address(ip_address)
except ValueError:
self.module.fail_json(msg="The provided IP address %s is not a valid IP"
" for the rule %s" % (ip_address, rule_name))

for ip_network in ip_networks:
try:
ipaddress.ip_network(ip_network)
except ValueError:
self.module.fail_json(msg="The provided IP network %s is not a valid network"
" for the rule %s" % (ip_network, rule_name))

def ensure(self):
"""
Function to ensure rule set configuration
Expand All @@ -216,75 +254,47 @@ def ensure(self):
firewall_system = host.configManager.firewallSystem
if firewall_system is None:
continue

results['rule_set_state'][host.name] = dict()

results['rule_set_state'][host.name] = {}
for rule_option in self.rule_options:
rule_name = rule_option.get('name', None)
if rule_name is None:
self.module.fail_json(msg="Please specify rule.name for rule set"
" as it is required parameter.")
if rule_name not in self.firewall_facts[host.name]:
self.module.fail_json(msg="rule named '%s' wasn't found." % rule_name)

rule_enabled = rule_option.get('enabled', None)
if rule_enabled is None:
self.module.fail_json(msg="Please specify rules.enabled for rule set"
" %s as it is required parameter." % rule_name)

# validate IP addresses are valid
rule_config = rule_option.get('allowed_hosts', None)

if 'ip_address' in rule_config[0].keys():
for ip_addr in rule_config[0]['ip_address']:
try:
ip = ipaddress.ip_address(ip_addr)
except ValueError:
self.module.fail_json(msg="The provided IP address %s is not a valid IP"
" for the rule %s" % (ip_addr, rule_name))

# validate provided subnets are valid networks
if 'ip_network' in rule_config[0].keys():
for ip_net in rule_config[0]['ip_network']:
try:
network_validation = ipaddress.ip_network(ip_net)
except ValueError:
self.module.fail_json(msg="The provided network %s is not a valid network"
" for the rule %s" % (ip_net, rule_name))

current_rule_state = self.firewall_facts[host.name][rule_name]['enabled']
if current_rule_state != rule_enabled:
if current_rule_state != rule_option['enabled']:
try:
if not self.module.check_mode:
if rule_enabled:
if rule_option['enabled']:
firewall_system.EnableRuleset(id=rule_name)
else:
firewall_system.DisableRuleset(id=rule_name)
# keep track of changes as we go
enable_disable_changed = True
except vim.fault.NotFound as not_found:
self.module.fail_json(msg="Failed to enable rule set %s as"
" rule set id is unknown : %s" % (rule_name,
to_native(not_found.msg)))
" rule set id is unknown : %s" % (
rule_name,
to_native(not_found.msg)))
except vim.fault.HostConfigFault as host_config_fault:
self.module.fail_json(msg="Failed to enabled rule set %s as an internal"
" error happened while reconfiguring"
" rule set : %s" % (rule_name,
to_native(host_config_fault.msg)))
" rule set : %s" % (
rule_name,
to_native(host_config_fault.msg)))

# save variables here for comparison later and change tracking
# also covers cases where inputs may be null
permitted_networking = self.firewall_facts[host.name][rule_name]
rule_allows_all = permitted_networking['allowed_hosts']['all_ip']
playbook_allows_all = rule_config[0]['all_ip']
rule_allowed_ip = set(permitted_networking['allowed_hosts']['ip_address'])
playbook_allowed_ip = set(rule_config[0].get('ip_address', ''))
rule_allowed_ips = set(permitted_networking['allowed_hosts']['ip_address'])
rule_allowed_networks = set(permitted_networking['allowed_hosts']['ip_network'])
playbook_allowed_networks = set(rule_config[0].get('ip_network', ''))

allowed_hosts = rule_option.get('allowed_hosts', {})
playbook_allows_all = allowed_hosts.get('all_ip', False)
playbook_allowed_ips = set(allowed_hosts.get('ip_address', []))
playbook_allowed_networks = set(allowed_hosts.get('ip_network', []))

# compare what is configured on the firewall rule with what the playbook provides
allowed_all_ips_different = bool(rule_allows_all != playbook_allows_all)
ip_list_different = bool(rule_allowed_ip != playbook_allowed_ip)
ip_list_different = bool(rule_allowed_ips != playbook_allowed_ips)
ip_network_different = bool(rule_allowed_networks != playbook_allowed_networks)

# apply everything here in one function call
Expand All @@ -295,16 +305,16 @@ def ensure(self):
# setup spec
firewall_spec = vim.host.Ruleset.RulesetSpec()
firewall_spec.allowedHosts = vim.host.Ruleset.IpList()
firewall_spec.allowedHosts.allIp = rule_config[0].get('all_ip', True)
firewall_spec.allowedHosts.ipAddress = rule_config[0].get('ip_address', None)
firewall_spec.allowedHosts.allIp = playbook_allows_all
firewall_spec.allowedHosts.ipAddress = list(playbook_allowed_ips)
firewall_spec.allowedHosts.ipNetwork = []

if 'ip_network' in rule_config[0].keys():
for allowed_network in rule_config[0].get('ip_network', None):
tmp_ip_network_spec = vim.host.Ruleset.IpNetwork()
tmp_ip_network_spec.network = allowed_network.split("/")[0]
tmp_ip_network_spec.prefixLength = int(allowed_network.split("/")[1])
firewall_spec.allowedHosts.ipNetwork.append(tmp_ip_network_spec)
for i in playbook_allowed_networks:
address, mask = i.split('/')
tmp_ip_network_spec = vim.host.Ruleset.IpNetwork()
tmp_ip_network_spec.network = address
tmp_ip_network_spec.prefixLength = int(mask)
firewall_spec.allowedHosts.ipNetwork.append(tmp_ip_network_spec)

firewall_system.UpdateRuleset(id=rule_name, spec=firewall_spec)
except vim.fault.NotFound as not_found:
Expand All @@ -321,19 +331,22 @@ def ensure(self):
" error happened while applying the reconfiguration:"
" %s" % (rule_name, to_native(runtime_fault.msg)))

results['rule_set_state'][host.name][rule_name] = dict(current_state=rule_enabled,
previous_state=current_rule_state,
desired_state=rule_enabled,
current_allowed_all=playbook_allows_all,
previous_allowed_all=permitted_networking['allowed_hosts']['all_ip'],
desired_allowed_all=playbook_allows_all,
current_allowed_ip=playbook_allowed_ip,
previous_allowed_ip=set(permitted_networking['allowed_hosts']['ip_address']),
desired_allowed_ip=playbook_allowed_ip,
current_allowed_networks=playbook_allowed_networks,
previous_allowed_networks=set(permitted_networking['allowed_hosts']['ip_network']),
desired_allowed_networks=playbook_allowed_networks
)
results['rule_set_state'][host.name][rule_name] = {
'current_state': rule_option['enabled'],
'previous_state': current_rule_state,
'desired_state': rule_option['enabled'],
'allowed_hosts': {
'current_allowed_all': playbook_allows_all,
'previous_allowed_all': permitted_networking['allowed_hosts']['all_ip'],
'desired_allowed_all': playbook_allows_all,
'current_allowed_ip': playbook_allowed_ips,
'previous_allowed_ip': set(permitted_networking['allowed_hosts']['ip_address']),
'desired_allowed_ip': playbook_allowed_ips,
'current_allowed_networks': playbook_allowed_networks,
'previous_allowed_networks': set(permitted_networking['allowed_hosts']['ip_network']),
'desired_allowed_networks': playbook_allowed_networks,
}
}

if enable_disable_changed or allowed_ip_changed:
fw_change_list.append(True)
Expand All @@ -359,7 +372,16 @@ def main():
supports_check_mode=True
)

for rule_option in module.params.get("rules", []):
if 'allowed_hosts' in rule_option:
if isinstance(rule_option['allowed_hosts'], list):
if len(rule_option['allowed_hosts']) == 1:
allowed_hosts = rule_option['allowed_hosts'][0]
rule_option['allowed_hosts'] = allowed_hosts
module.deprecate('allowed_hosts should be a dict, not a list', '2.13')

vmware_firewall_manager = VmwareFirewallManager(module)
vmware_firewall_manager.check_params()
vmware_firewall_manager.ensure()


Expand Down
17 changes: 17 additions & 0 deletions test/integration/targets/prepare_vmware_tests/tasks/teardown.yml
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,21 @@
---

- name: Clean up the firewall rules
vmware_host_firewall_manager:
cluster_name: '{{ ccr1 }}'
rules:
- name: vvold
enabled: False
- name: CIMHttpServer
enabled: True
allowed_hosts:
all_ip: True
- name: NFC
enabled: True
allowed_hosts:
all_ip: True
ignore_errors: yes

- name: Remove the VM prepared by prepare_vmware_tests
vmware_guest:
hostname: "{{ vcenter_hostname }}"
Expand Down
Loading

0 comments on commit 67d8ec7

Please sign in to comment.