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

JSON templating incorrectly changes the value to type int #151

Closed
lancewood1 opened this issue Oct 3, 2018 · 6 comments
Closed

JSON templating incorrectly changes the value to type int #151

lancewood1 opened this issue Oct 3, 2018 · 6 comments
Assignees

Comments

@lancewood1
Copy link

ISSUE TYPE

  • Bug Report

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
Copy link
Member

ganeshrn commented Oct 4, 2018

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

@lancewood1
Copy link
Author

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 added a commit to ganeshrn/network-engine that referenced this issue Oct 5, 2018
Fixes ansible-network#151

*  Add support for type option in json_template
@ganeshrn
Copy link
Member

ganeshrn commented Oct 5, 2018

@lancewood1 With the fix in PR #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:
- key: name
value: "{{ item.name.acl_name }}"
type: str

Can you please check if this patch fixes the issue.

Please ignore above comment.

@lancewood1
Copy link
Author

@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
Copy link
Member

ganeshrn commented Oct 5, 2018

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 added a commit to ganeshrn/network-engine that referenced this issue Oct 6, 2018
Fixes ansible-network#151

*  The current implementation tries to convert the value
   to int irrespective to the actual desired type of data.
*  Ansible version 2.7 onwards supports native jinja2 types
   eg: value: "{{ item.name.acl_name|float }}"
   To convert to native jinja2 type by default need to set
   `jinja2_native` config varaible in `defaults` setion within
   configuration file.
   Note: this required jinja2 version to be >= 2.10
@ganeshrn
Copy link
Member

ganeshrn commented Oct 6, 2018

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

@ganeshrn ganeshrn self-assigned this Oct 8, 2018
trishnaguha pushed a commit that referenced this issue Oct 9, 2018
* Fix native type conversion in json_template

Fixes #151

*  The current implementation tries to convert the value
   to int irrespective to the actual desired type of data.
*  Ansible version 2.7 onwards supports native jinja2 types
   eg: value: "{{ item.name.acl_name|float }}"
   To convert to native jinja2 type by default need to set
   `jinja2_native` config varaible in `defaults` setion within
   configuration file.
   Note: this required jinja2 version to be >= 2.10

* Fix CI failure

* Update doc

* Add example usage in doc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants