ansible-collection-cloudstack icon indicating copy to clipboard operation
ansible-collection-cloudstack copied to clipboard

Missing argument for cs_firewall: destcidrlist

Open nathanmcgarvey opened this issue 4 years ago • 4 comments

There is an argument to the "createEgressFirewallRule" api call named "destcidrlist". (Ref: https://cloudstack.apache.org/api/apidocs-4.15/apis/createEgressFirewallRule.html)

It seems that this was omitted from the cs_firewall module, or that that module wasn't updated to include destcidrlist when cloudstack 4.10 was released. (https://docs.cloudstack.apache.org/projects/archived-cloudstack-release-notes/en/4.10/api-changes.html)

This means that you cannot make an egress rule with limited destination subnets, though the very similar, but opposing "cidrlist" functionality seems to be implemented.

nathanmcgarvey avatar Jun 23 '21 00:06 nathanmcgarvey

@nathanmcgarvey this should already work:

- name: Allow inbound port 80/tcp
  ngine_io.cloudstack.cs_firewall:
    ip_address: 4.3.2.1
    zone: zone01
    port: 80
    cidr: 
      - 1.2.3.4/32
      - 1.2.3.5/32

or

- name: Allow inbound port 80/tcp
  ngine_io.cloudstack.cs_firewall:
    ip_address: 4.3.2.1
    zone: zone01
    port: 80
    cidrs: 
      - 1.2.3.4/32
      - 1.2.3.5/32

could you confirm?

resmo avatar Aug 08 '21 20:08 resmo

@resmo I think your example works for ingress rules, but not when doing egress rules. E.g:

- name: Allow only HTTP outbound traffic for an IP
  ngine_io.cloudstack.cs_firewall:
    network: my_network
    zone: zone01
    type: egress
    port: 80
    cidr: 10.101.1.20

That example from the source code allows for the egress from 10.101.1.20 to any destination subnet. If you want to limit that destination subnet, that's the destcidrlist API call that doesn't seem to exist in the module.

From https://cloudstack.apache.org/api/apidocs-4.15/apis/createEgressFirewallRule.html:

cidrlist | the cidr list to forward traffic from. Multiple entries must be separated by a single comma character (,). | false destcidrlist | the cidr list to forward traffic to. Multiple entries must be separated by a single comma character (,). | false

If I'm reading the source correctly, it's around here that the destcidrlist doesn't' exist:

Line 330 in cs_firewall.py:

          args = {
               'cidrlist': self.module.params.get('cidrs'),
               'protocol': self.module.params.get('protocol'),
               'startport': self.module.params.get('start_port'),
               'endport': self.get_or_fallback('end_port', 'start_port'),
               'icmptype': self.module.params.get('icmp_type'),
               'icmpcode': self.module.params.get('icmp_code')
           }

           fw_type = self.module.params.get('type')
           if not self.module.check_mode:
               if fw_type == 'egress':
                   args['networkid'] = self.get_network(key='id')
                   res = self.query_api('createEgressFirewallRule', **args)

nathanmcgarvey avatar Aug 09 '21 15:08 nathanmcgarvey

Thanks for the details again and sorry for not reading it carefully enough the first time, I see destcidrlist was new with 4.10. Expect an implementation soon-ish.

resmo avatar Aug 09 '21 18:08 resmo

resolved by #84

resmo avatar Aug 11 '21 16:08 resmo