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

Missing argument for cs_firewall: destcidrlist #76

Open
nathanmcgarvey opened this issue Jun 23, 2021 · 4 comments · May be fixed by #84
Open

Missing argument for cs_firewall: destcidrlist #76

nathanmcgarvey opened this issue Jun 23, 2021 · 4 comments · May be fixed by #84
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@nathanmcgarvey
Copy link

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.

@resmo
Copy link
Member

resmo commented Aug 8, 2021

@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 resmo added documentation Improvements or additions to documentation good first issue Good for newcomers waiting waiting for feedback labels Aug 8, 2021
@nathanmcgarvey
Copy link
Author

nathanmcgarvey commented Aug 9, 2021

@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)

@resmo
Copy link
Member

resmo commented Aug 9, 2021

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 resmo added enhancement New feature or request and removed documentation Improvements or additions to documentation good first issue Good for newcomers waiting waiting for feedback labels Aug 9, 2021
@resmo resmo added this to the 2.2.0 milestone Aug 9, 2021
@resmo resmo self-assigned this Aug 9, 2021
@resmo resmo linked a pull request Aug 11, 2021 that will close this issue
@resmo
Copy link
Member

resmo commented Aug 11, 2021

resolved by #84

@resmo resmo modified the milestones: 2.2.0, 2.3.x Oct 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants