fortinet-ansible-dev / ansible-galaxy-fortimanager-collection

GNU General Public License v3.0
16 stars 15 forks source link

fmgr_pm_pkg does not conform to Ansible spec (spaces and hyphens in YAML keys) #24

Open JonTheNiceGuy opened 3 years ago

JonTheNiceGuy commented 3 years ago

Here's the example code block from the module documentation:

- hosts: fortimanager-inventory
  collections:
    - fortinet.fortimanager
  connection: httpapi
  vars:
     ansible_httpapi_use_ssl: True
     ansible_httpapi_validate_certs: False
     ansible_httpapi_port: 443
  tasks:
   - name: no description
     fmgr_pm_pkg:
        bypass_validation: False
        workspace_locking_adom: <value in [global, custom adom including root]>
        workspace_locking_timeout: 300
        rc_succeeded: [0, -2, -3, ...]
        rc_failed: [-2, -3, ...]
        adom: <your own value>
        pkg_path: <your own value>
        pm_pkg:
           name: <value of string>
           obj ver: <value of integer>
           oid: <value of integer>
           package setting:
              central-nat: <value in [disable, enable]>
              consolidated-firewall-mode: <value in [disable, enable]>
              fwpolicy-implicit-log: <value in [disable, enable]>
              fwpolicy6-implicit-log: <value in [disable, enable]>
              inspection-mode: <value in [proxy, flow]>
              ngfw-mode: <value in [profile-based, policy-based]>
              ssl-ssh-profile: <value of string>
           scope member:
             -
                 name: <value of string>
                 vdom: <value of string>
           type: <value in [pkg, folder]>

Please note that the following value is considered incorrect in YAML:

Also note that when I wrapped these keys in quotes, I got a further error:

Playbook:

---
- name: Create cluster policies
  hosts: FortiManager
  gather_facts: false
  collections:
    - fortinet.fortimanager
  tasks:
    - name: Create the newpolicy package
      fmgr_pm_pkg:
        bypass_validation: False
        adom: root
        pkg_path: ""
        pm_pkg:
          type: pkg
          name: cluster_policy
          "package setting":
            "fwpolicy-implicit-log": enable
            "inspection-mode": proxy
            "ngfw-mode": policy-based
          "scope member":
            - name: firewall_cluster
              vdom: root

Error:

TASK [Create the newpolicy package] *************************************************************************************************************
task path: /home/spriggsj/Work/2021-04-06 Azure FortiOS FortiManager FortiAnalyzer/Ansible/configure_policy.yml:8
fatal: [JSFN-FortiManager]: FAILED! => {
    "ansible_facts": {
        "discovered_interpreter_python": "/usr/bin/python3"
    },
    "changed": false,
    "invocation": {
        "module_args": {
            "adom": "root",
            "bypass_validation": false,
            "pkg_path": "",
            "pm_pkg": {
                "name": "cluster_policy",
                "obj ver": null,
                "oid": null,
                "package setting": {
                    "central-nat": null,
                    "consolidated-firewall-mode": null,
                    "fwpolicy-implicit-log": "enable",
                    "fwpolicy6-implicit-log": null,
                    "inspection-mode": "proxy",
                    "ngfw-mode": "policy-based",
                    "ssl-ssh-profile": null
                },
                "scope member": [
                    {
                        "name": "cluster_policy",
                        "vdom": "root"
                    }
                ],
                "type": "pkg"
            },
            "rc_failed": null,
            "rc_succeeded": null,
            "workspace_locking_adom": null,
            "workspace_locking_timeout": 300
        }
    },
    "meta": {
        "request_url": "/pm/pkg/adom/root",
        "response_code": -10000,
        "response_data": [],
        "response_message": "invalid value"
    },
    "rc": -10000
}
jpforcioli commented 3 years ago

do you mind, clarifying what's the issue? If you don't quote those labels-with-space, it doesn't work?

JonTheNiceGuy commented 3 years ago

There are two separate issues here. I'm happy to raise a fresh one for one of these items, if required.

Issue 1. If I use the quoted with spaces values (as per the playbook included) I get the error message listed at the bottom of the issue. I don't have an idea on which diagnose debug commands I need to run to compare the working actions (creating a policy package via the webUI) versus a non-working action (using this module via the playbook).

Issue 2. Your example code (the top block) is syntactically wrong (you can't have YAML dictionary keys with hyphens or spaces, unless they're quoted). Ideally, your ansible module generator would replace invalid characters (hyphens, spaces) with underscores, and translate them automatically. I believe this is what happens with the FortiOS modules?

chillancezen commented 3 years ago

hi Jon,

in your case you get -10000 error code, it's not caused by unquoting or keying with whitespace or keying with hyphens. could you help confirm the device cluster_policy does exist in adom root? I try my env, if a wrong device is given, it will give the same error, however after I fixed it with a correct device, it works fine even if unquoting the keys.

---
- name: Create cluster policies
  hosts: fortimanager01
  collections:
    - fortinet.fortimanager
  connection: httpapi
  tasks:
    - name: Create the newpolicy package
      fmgr_pm_pkg_adom:
        enable_log: True
        adom: 'root'
        pm_pkg_adom:
          type: pkg
          name: cluster_policy
          package setting:
            fwpolicy-implicit-log: enable
            inspection-mode: proxy
            ngfw-mode: policy-based
          scope member:
            - name: FGVM02TM20012347
              vdom: root

please note there are three modules for packages. https://ansible-galaxy-fortimanager-docs.readthedocs.io/en/galaxy-2.0.3/faq.html (we are planing to enrich FAQ continuously in the future).

and the result:

changed: [fortimanager01] => {
    "changed": true,
    "invocation": {
        "module_args": {
            "adom": "root",
            "bypass_validation": false,
            "enable_log": true,
            "pm_pkg_adom": {
                "name": "cluster_policy",
                "obj ver": null,
                "oid": null,
                "package setting": {
                    "central-nat": null,
                    "consolidated-firewall-mode": null,
                    "fwpolicy-implicit-log": "enable",
                    "fwpolicy6-implicit-log": null,
                    "inspection-mode": "proxy",
                    "ngfw-mode": "policy-based",
                    "ssl-ssh-profile": null
                },
                "scope member": [
                    {
                        "name": "FGVM02TM20012347",
                        "vdom": "root"
                    }
                ],
                "type": "pkg"
            },
            "proposed_method": null,
            "rc_failed": null,
            "rc_succeeded": null,
            "workspace_locking_adom": null,
            "workspace_locking_timeout": 300
        }
    },
    "meta": {
        "request_url": "/pm/pkg/adom/root",
        "response_code": 0,
        "response_data": [],
        "response_message": "OK"
    },
    "rc": 0
}
META: ran handlers
META: ran handlers

PLAY RECAP ***********************************************************************************************************************************************************************************************************************************
fortimanager01             : ok=2    changed=1    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

we are going to release 2.0.3 in very few days.

anyway, thank you for the issue, yes naming conventions does not kind of conform to Ansible spec, However Ansible loosely accepts irregular parameters as well. One thing is certain, if we fix them immediately, the backward compatibility will be broken, legacy users may not be able to run with latest collection, so we consider to fix in next major release: 3.x.x(according semantic versioning, major release allow incompatible changes).