ansible-network / network-engine

This role provides the foundation for building network roles by providing modules and plugins that are common to all Ansible Network roles.
GNU General Public License v3.0
112 stars 53 forks source link

JSON templating incorrectly changes the value to type int #151

Closed lancewood1 closed 6 years ago

lancewood1 commented 6 years ago

ISSUE TYPE

SUMMARY

The _coerce_to_native function in network-engine/lib/network_engine/plugins/template/__init__.py incorrectly casts values to type integer.

Any string value that looks like an integer is turned into an integer which causes issues. Many network constructs should not be converted from type string. For example, an ip prefix-list defined on a Cisco device could have a name similar to "0012345678" however once the that value goes through this process the prefix-list name is change to 12345678 of type integer.

The line value = int(value) is changing the variable type of objects that should be strings.

    def _coerce_to_native(self, value):
        if not isinstance(value, bool):
            try:
                value = int(value)
            except Exception:
                if value is None or len(value) == 0:
                    return None
                pass
        return value

STEPS TO REPRODUCE

text_parser:
  file: "ios_prefix_list_parser.yml"
  content: "{{ prefix_list_output.stdout[0] }}"

EXPECTED RESULTS

prefix_list["0012345678"] is defined

ACTUAL RESULTS

prefix_list[12345678] is defined

ganeshrn commented 6 years ago

Can you please share the parser template ios_prefix_list_parser.yml and the value of content prefix_list_output.stdout[0]

lancewood1 commented 6 years ago

This is the parser template, it is available as cisco_ios/parser_templates/config/show_ip_prefix_list.yaml

---
- name: match sections
  pattern_match:
    regex: "^ip"
    match_all: yes
    match_greedy: yes
  register: context

- name: match prefix values
  pattern_group:
    - name: match name
      pattern_match:
        regex: "ip prefix-list (?P<acl_name>.+): (\\d+) entries"
        content: "{{ item }}"
      register: name

    - name: match entries
      pattern_match:
        regex: "seq (\\d+) (permit|deny) (\\S+)(?: (le|ge) (\\d+))*"
        content: "{{ item }}"
        match_all: yes
      register: entry
  loop: "{{ context }}"
  register: entries

- name: template entries
  json_template:
    template:
      - key: name
        value: "{{ item.name.acl_name }}"
      - key: entries
        elements:
          - key: sequence
            value: "{{ it.matches.0 }}"
          - key: action
            value: "{{ it.matches.1 }}"
          - key: prefix
            value: "{{ it.matches.2 }}"
          - key: operator
            value: "{{ it.matches.4 }}"
          - key: length
            value: "{{ it.matches.5 }}"
        repeat_for: "{{ item.entry }}"
        repeat_var: it
  register: prefix_list_list
  export: yes
  loop: "{{ entries }}"

- name: template entries
  json_template:
    template:
      - key: "{{ item.name.acl_name }}"
        object:
          - key: entries
            elements:
              - key: sequence
                value: "{{ it.matches.0 }}"
              - key: action
                value: "{{ it.matches.1 }}"
              - key: prefix
                value: "{{ it.matches.2 }}"
              - key: operator
                value: "{{ it.matches.3 }}"
              - key: length
                value: "{{ it.matches.4 }}"
            repeat_for: "{{ item.entry }}"
            repeat_var: it
  loop: "{{ entries }}"
  export: yes
  export_as: dict
  extend: cisco_ios.config
  register: prefix_list_dict

- name: template acl and prefix
  json_template:
    template:
      - key: acl
        elements:
          - key: acl_name
            value: "{{ item.name.acl_name }}"
          - key: prefix
            value: "{{ it.matches.2 }}"
        repeat_for: "{{ item.entry }}"
        repeat_var: it
  register: prefix_list_basic
  export: yes
  loop: "{{ entries }}"

The prefix list output variable contains the following:

ip prefix-list 0012345678: 1 entries
  seq 10 permit 1.2.3.4/24 le 32
ip prefix-list 9876543210: 1 entries
  seq 10 permit 2.4.6.8/24 le 32
ganeshrn commented 6 years ago

~~@lancewood1 With the fix in PR https://github.com/ansible-network/network-engine/pull/153 a type option can be added to json_template directive to enforce value is converted to the given type. For eg:~~

~~- name: template entries json_template: template:

Can you please check if this patch fixes the issue.

Please ignore above comment.

lancewood1 commented 6 years ago

@ganeshrn This seems to work when the type option is applied as you have shown above. However, when the template uses the elements keyword the type is merely added to the dictionary. Referring to the parser template above, I was only able to keep the prefix-list name a string for the json_template using the registered variable prefix_list_list. Both the dictionary output and basic output contain integer prefix list names.

This is the one that works as expected:

- name: template entries
  json_template:
    template:
      - key: name
        value: "{{ item.name.acl_name }}"
        type: str
      - key: entries
        elements:
          - key: sequence
            value: "{{ it.matches.0 }}"
          - key: action
            value: "{{ it.matches.1 }}"
          - key: prefix
            value: "{{ it.matches.2 }}"
          - key: operator
            value: "{{ it.matches.4 }}"
          - key: length
            value: "{{ it.matches.5 }}"
        repeat_for: "{{ item.entry }}"
        repeat_var: it
  register: prefix_list_list
  export: yes
  loop: "{{ entries }}"

The other uses of json_template do not, for example:

- name: template acl and prefix
  json_template:
    template:
      - key: acl
        elements:
          - key: acl_name
            value: "{{ item.name.acl_name }}"
            type: str
          - key: prefix
            value: "{{ it.matches.2 }}"
        repeat_for: "{{ item.entry }}"
        repeat_var: it
  register: prefix_list_basic
  export: yes
  loop: "{{ entries }}"

Results in output like this:

"prefix_list_basic" : [
  {
      "acl" : [
           [
               {
                   "type": "str"
                   "key": "acl_name"
                   "value": 0012345678
               },
               {
                   "key": "prefix"
                   "value": "1.2.3.4/24"
               }
            ]
         ]
    }
ganeshrn commented 6 years ago

Yes that’s right. The fix I proposed earlier is not correct that’s why I have closed the PR. I am working on alternate way to fix the issue.

ganeshrn commented 6 years ago

@lancewood1 Ansible 2.7 version onwards supports native jinja2 datatype conversion, hence handling of native type conversion is not required in the json_template code.

To enable native jinja2 conversion enusre Ansible versions is >= 2.7 and jinja2 version is >=2.10 Also, need to enable jinja2_native config variable in Ansible config file

[defaults]
jinja2_native= True

With this fix if in case you want to convert the value to int, the native type should to be added in template variable. for eg:

- name: template entries
  json_template:
    template:
      - key: name
        value: "{{ item.name.acl_name|int }}"
      - key: entries
        elements:
          - key: sequence
            value: "{{ it.matches.0 }}"
          - key: action
            value: "{{ it.matches.1 }}"
          - key: prefix
            value: "{{ it.matches.2 }}"
          - key: operator
            value: "{{ it.matches.4 }}"
          - key: length
            value: "{{ it.matches.5 | default(None)}}"
        repeat_for: "{{ item.entry }}"
        repeat_var: it
  register: prefix_list_list
  export: yes
  loop: "{{ entries }}"

Output:

       "prefix_list_list": [
            {
                "entries": [
                    [
                        {
                            "key": "sequence",
                            "value": "10"
                        },
                        {
                            "key": "action",
                            "value": "permit"
                        },
                        {
                            "key": "prefix",
                            "value": "1.2.3.4/24"
                        },
                        {
                            "key": "operator",
                            "value": "32"
                        },
                        {
                            "key": "length",
                            "value": null
                        }
                    ]
                ],
                "name": 12345678
            }