fortinet-ansible-dev / ansible-galaxy-fortios-collection

GNU General Public License v3.0
85 stars 49 forks source link

fmgr_pkg_firewall_policy - several parameters should be of type list instead of str #109

Closed eJay57 closed 3 years ago

eJay57 commented 3 years ago

Hello,

I think several parameters of the module fmgr_pkg_firewall_policy should be of type list instead of str.

Version of the FortiManager collection : 2.0.3 Version of the target FortiManager : 6.4.6

Try to run following task :

- name : "Create rule"
  fmgr_pkg_firewall_policy:
    adom: "test"
    pkg: "FW1637VD01"
    state: present
    pkg_firewall_policy:
      policyid: 0
      name: "testrule"
      srcaddr: "testaddr1"
      srcintf: "Internal"
      dstaddr: ["testaddr1", "testaddr2"]
      dstintf: "Internal"
      service: "SMTP"
      action: "accept"
      logtraffic: "all"
      schedule: "always"

Expected result : creation of a rule with 1 source object, 2 destination objects and 1 service object Result : fatal: [SV-2000AVU66]: FAILED! => {"ansible_facts": {"discovered_interpreter_python": "/usr/bin/python"}, "changed": false, "meta": {"request_url": "/pm/config/adom/test/pkg/FW1637VD01/firewall/policy", "response_code": -10131, "response_data": {"policyid": 68}, "response_message": "datasrc invalid. object: Policy package \"\" - firewall policy.68:dstaddr. detail: [\\'testaddr1\\', \\'testaddr2\\']. solution: datasrc invalid"}, "rc": -10131}

Running same task with only one object in the destination runs properly. This is because the module is expecting a string and we have to provide a list to the API if multiple objects needs to be set as source, destination, service, ...

Here are the problematic lines in the module fmgr_pkg_firewall_policy.py :

1060                'dstaddr': {
1061                    'required': False,
1062                    'type': 'str'

Changing these lines as following solves the issue :

1060                'dstaddr': {
1061                    'required': False,
1062                    'type': 'list'

Result of running the above task after the change in fmgr_pkg_firewall_policy.py :

changed: [SV-2000AVU66] => {"ansible_facts": {"discovered_interpreter_python": "/usr/bin/python"}, "changed": true, "meta": {"request_url": "/pm/config/adom/test/pkg/FW1637VD01/firewall/policy", "response_code": 0, "response_data": {"policyid": 68}, "response_message": "OK"}, "rc": 0}

These changes need to be done for every parameter that should accept multiple values (srcaddr, service, ...).

Before doing these changes, I tried every possible combination to pass multiple objects as a string but with no success (not described in the documentation). If there is a way to pass multiple objects as a string, I think it would be nice to have it documented. Otherwise, the module need to be fixed.

Regards.

Étienne

JieX19 commented 3 years ago

Hi @eJay57,

Thanks for your testing! I checked it and the dstaddr is a list. We will fix it soon. BTW, this module is related to fortimanager, so I will move it to this batch https://github.com/fortinet-ansible-dev/ansible-galaxy-fortimanager-collection/issues and you can raise and track any issues about fortimanager in it.

Duplicate issue # https://github.com/fortinet-ansible-dev/ansible-galaxy-fortimanager-collection/issues/26