don't crash when using nested key flattening

[matejv]

When using one multiple instances of nested key flattening, the parser will crash on the second instance.

Example:

static:
    _process:
        - path: "?prefix.?mask"
...
  next-hops:
        _process: unnecessary
        next-hop:
            _process:
                - path: "?interface_ref.?next_hop_addr"
...

Added two prints to jsonp.py to make the problem stand out a bit more:

        print('PRE ', attribute, mapping.get("path", ""), data) 
        d = self.resolve_path(data, mapping.get("path", ""), mapping.get("default")) 
        print('POST', attribute, mapping.get("path", ""), d) 

Error:

DEBUG:napalm-yang:Parsing element protocol[static static]
DEBUG:napalm-yang:Parsing attribute: /network-instances/network-instance[name='global']/protocols/protocol[identifier='static' name='static']
DEBUG:napalm-yang:Parsing attribute: /network-instances/network-instance[name='global']/protocols/protocol[identifier='static' name='static']/bgp
DEBUG:napalm-yang:Parsing attribute: /network-instances/network-instance[name='global']/protocols/protocol[identifier='static' name='static']/bgp
DEBUG:napalm-yang:Parsing attribute: /network-instances/network-instance[name='global']/protocols/protocol[identifier='static' name='static']/state
DEBUG:napalm-yang:Parsing attribute: /network-instances/network-instance[name='global']/protocols/protocol[identifier='static' name='static']/local-aggregates
DEBUG:napalm-yang:Parsing attribute: /network-instances/network-instance[name='global']/protocols/protocol[identifier='static' name='static']/local-aggregates
DEBUG:napalm-yang:Parsing attribute: /network-instances/network-instance[name='global']/protocols/protocol[identifier='static' name='static']/name
DEBUG:napalm-yang:Parsing attribute: /network-instances/network-instance[name='global']/protocols/protocol[identifier='static' name='static']/name
DEBUG:napalm-yang:Parsing attribute: /network-instances/network-instance[name='global']/protocols/protocol[identifier='static' name='static']/static-routes
DEBUG:napalm-yang:Parsing attribute: /network-instances/network-instance[name='global']/protocols/protocol[identifier='static' name='static']/static-routes
DEBUG:napalm-yang:Parsing attribute: /network-instances/network-instance[name='global']/protocols/protocol[identifier='static' name='static']/static-routes/static
DEBUG:napalm-yang:Parsing attribute: /network-instances/network-instance[name='global']/protocols/protocol[identifier='static' name='static']/static-routes/static
('PRE ', 'static', '?prefix.?mask', OrderedDict([('#text', 'vrf mgmtVrf 5.5.5.5 255.255.255.255 FastEthernet1 192.168.5.5'), ('1.1.1.1', OrderedDict([('#text', '255.255.255.255 Null0 tag 555'), ('255.255.255.255', OrderedDict([('#text', 'Null0 tag 555'), ('Null0', OrderedDict([('#text', 'tag 555'), ('tag', OrderedDict([('#text', '555'), ('555', OrderedDict([('#standalone', True)]))]))]))]))])), ('2.2.2.2', OrderedDict([('#text', '255.255.255.255 10.1.1.1'), ('255.255.255.255', OrderedDict([('#text', '10.1.1.1'), ('10.1.1.1', OrderedDict([('#standalone', True)]))]))])), ('3.3.3.0', OrderedDict([('#text', '255.255.255.0 Vlan798 10.4.4.4 66'), ('255.255.255.0', OrderedDict([('#text', 'Vlan798 10.4.4.4 66'), ('Vlan797', OrderedDict([('#standalone', True)])), ('10.2.2.2', OrderedDict([('#text', '22 tag 5555'), ('22', OrderedDict([('#text', 'tag 5555'), ('tag', OrderedDict([('#text', '5555'), ('5555', OrderedDict([('#standalone', True)]))]))]))])), ('10.3.3.3', OrderedDict([('#text', '44'), ('44', OrderedDict([('#standalone', True)]))])), ('Vlan798', OrderedDict([('#text', '10.4.4.4 66'), ('10.4.4.4', OrderedDict([('#text', '66'), ('66', OrderedDict([('#standalone', True)]))]))]))]))])), ('vrf', OrderedDict([('#text', 'mgmtVrf 5.5.5.5 255.255.255.255 FastEthernet1 192.168.5.5'), ('mgmtVrf', OrderedDict([('#text', '5.5.5.5 255.255.255.255 FastEthernet1 192.168.5.5'), ('5.5.5.5', OrderedDict([('#text', '255.255.255.255 FastEthernet1 192.168.5.5'), ('255.255.255.255', OrderedDict([('#text', 'FastEthernet1 192.168.5.5'), ('FastEthernet1', OrderedDict([('#text', '192.168.5.5'), ('192.168.5.5', OrderedDict([('#standalone', True)]))]))]))]))]))]))]))
('POST', 'static', '?prefix.?mask', [OrderedDict([('#text', 'Null0 tag 555'), ('Null0', OrderedDict([('#text', 'tag 555'), ('tag', OrderedDict([('#text', '555'), ('555', OrderedDict([('#standalone', True)]))]))])), ('mask', '255.255.255.255'), ('prefix', '1.1.1.1')]), OrderedDict([('#text', '10.1.1.1'), ('10.1.1.1', OrderedDict([('#standalone', True)])), ('mask', '255.255.255.255'), ('prefix', '2.2.2.2')]), OrderedDict([('#text', 'Vlan798 10.4.4.4 66'), ('Vlan797', OrderedDict([('#standalone', True)])), ('10.2.2.2', OrderedDict([('#text', '22 tag 5555'), ('22', OrderedDict([('#text', 'tag 5555'), ('tag', OrderedDict([('#text', '5555'), ('5555', OrderedDict([('#standalone', True)]))]))]))])), ('10.3.3.3', OrderedDict([('#text', '44'), ('44', OrderedDict([('#standalone', True)]))])), ('Vlan798', OrderedDict([('#text', '10.4.4.4 66'), ('10.4.4.4', OrderedDict([('#text', '66'), ('66', OrderedDict([('#standalone', True)]))]))])), ('mask', '255.255.255.0'), ('prefix', '3.3.3.0')]), OrderedDict([('#text', '5.5.5.5 255.255.255.255 FastEthernet1 192.168.5.5'), ('5.5.5.5', OrderedDict([('#text', '255.255.255.255 FastEthernet1 192.168.5.5'), ('255.255.255.255', OrderedDict([('#text', 'FastEthernet1 192.168.5.5'), ('FastEthernet1', OrderedDict([('#text', '192.168.5.5'), ('192.168.5.5', OrderedDict([('#standalone', True)]))]))]))])), ('mask', 'mgmtVrf'), ('prefix', 'vrf')])])
DEBUG:napalm-yang:Parsing element static[1.1.1.1/32]
DEBUG:napalm-yang:Parsing attribute: /network-instances/network-instance[name='global']/protocols/protocol[identifier='static' name='static']/static-routes/static[prefix='1.1.1.1/32']
DEBUG:napalm-yang:Parsing attribute: /network-instances/network-instance[name='global']/protocols/protocol[identifier='static' name='static']/static-routes/static[prefix='1.1.1.1/32']/prefix
DEBUG:napalm-yang:Parsing attribute: /network-instances/network-instance[name='global']/protocols/protocol[identifier='static' name='static']/static-routes/static[prefix='1.1.1.1/32']/prefix
DEBUG:napalm-yang:Parsing attribute: /network-instances/network-instance[name='global']/protocols/protocol[identifier='static' name='static']/static-routes/static[prefix='1.1.1.1/32']/next-hops
DEBUG:napalm-yang:Parsing attribute: /network-instances/network-instance[name='global']/protocols/protocol[identifier='static' name='static']/static-routes/static[prefix='1.1.1.1/32']/next-hops
DEBUG:napalm-yang:Parsing attribute: /network-instances/network-instance[name='global']/protocols/protocol[identifier='static' name='static']/static-routes/static[prefix='1.1.1.1/32']/next-hops/next-hop
DEBUG:napalm-yang:Parsing attribute: /network-instances/network-instance[name='global']/protocols/protocol[identifier='static' name='static']/static-routes/static[prefix='1.1.1.1/32']/next-hops/next-hop
('PRE ', 'next_hop', '?interface_ref.?next_hop_addr', OrderedDict([('#text', 'Null0 tag 555'), ('Null0', OrderedDict([('#text', 'tag 555'), ('tag', OrderedDict([('#text', '555'), ('555', OrderedDict([('#standalone', True)]))]))])), ('mask', '255.255.255.255'), ('prefix', '1.1.1.1')]))
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
/home/matej/Projects/napalmyang/testmocked.py in <module>()
     46     running = napalm_yang.base.Root()
     47     running.add_model(model)
---> 48     running.parse_config(device=device)
     49 
     50 expected = load_json_model(base, profile, model, mode, case, "expected.json")

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/base.pyc in parse_config(self, device, profile, native, attrs)
    223         for v in attrs:
    224             parser = Parser(v, device=device, profile=profile, native=native, is_config=True)
--> 225             parser.parse()
    226 
    227     def parse_state(self, device=None, profile=None, native=None, attrs=None):

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in parse(self)
     83         if "parent" not in self.bookmarks:
     84             self.bookmarks["parent".format(self._yang_name)] = self.native
---> 85         self._parse(self._yang_name, self.model, self.mapping[self._yang_name])
     86 
     87     def _parse(self, attribute, model, mapping):

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse(self, attribute, model, mapping)
     89 
     90         if model._is_container in ("container", ):
---> 91             self._parse_container(attribute, model, mapping)
     92         elif model._yang_type in ("list", ):
     93             self._parse_list(attribute, model, mapping)

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse_container(self, attribute, model, mapping)
    127                 parser.parse()
    128             else:
--> 129                 self._parse(k, v, mapping[v._yang_name])
    130 
    131         # Restoring state

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse(self, attribute, model, mapping)
     91             self._parse_container(attribute, model, mapping)
     92         elif model._yang_type in ("list", ):
---> 93             self._parse_list(attribute, model, mapping)
     94         else:
     95             self._parse_leaf(attribute, model, mapping)

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse_list(self, attribute, model, mapping)
    169 
    170             element_mapping = copy.deepcopy(mapping)
--> 171             self._parse(key, obj, element_mapping)
    172 
    173         # Restore state

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse(self, attribute, model, mapping)
     89 
     90         if model._is_container in ("container", ):
---> 91             self._parse_container(attribute, model, mapping)
     92         elif model._yang_type in ("list", ):
     93             self._parse_list(attribute, model, mapping)

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse_container(self, attribute, model, mapping)
    127                 parser.parse()
    128             else:
--> 129                 self._parse(k, v, mapping[v._yang_name])
    130 
    131         # Restoring state

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse(self, attribute, model, mapping)
     89 
     90         if model._is_container in ("container", ):
---> 91             self._parse_container(attribute, model, mapping)
     92         elif model._yang_type in ("list", ):
     93             self._parse_list(attribute, model, mapping)

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse_container(self, attribute, model, mapping)
    127                 parser.parse()
    128             else:
--> 129                 self._parse(k, v, mapping[v._yang_name])
    130 
    131         # Restoring state

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse(self, attribute, model, mapping)
     91             self._parse_container(attribute, model, mapping)
     92         elif model._yang_type in ("list", ):
---> 93             self._parse_list(attribute, model, mapping)
     94         else:
     95             self._parse_leaf(attribute, model, mapping)

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse_list(self, attribute, model, mapping)
    169 
    170             element_mapping = copy.deepcopy(mapping)
--> 171             self._parse(key, obj, element_mapping)
    172 
    173         # Restore state

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse(self, attribute, model, mapping)
     89 
     90         if model._is_container in ("container", ):
---> 91             self._parse_container(attribute, model, mapping)
     92         elif model._yang_type in ("list", ):
     93             self._parse_list(attribute, model, mapping)

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse_container(self, attribute, model, mapping)
    127                 parser.parse()
    128             else:
--> 129                 self._parse(k, v, mapping[v._yang_name])
    130 
    131         # Restoring state

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse(self, attribute, model, mapping)
     89 
     90         if model._is_container in ("container", ):
---> 91             self._parse_container(attribute, model, mapping)
     92         elif model._yang_type in ("list", ):
     93             self._parse_list(attribute, model, mapping)

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse_container(self, attribute, model, mapping)
    127                 parser.parse()
    128             else:
--> 129                 self._parse(k, v, mapping[v._yang_name])
    130 
    131         # Restoring state

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse(self, attribute, model, mapping)
     91             self._parse_container(attribute, model, mapping)
     92         elif model._yang_type in ("list", ):
---> 93             self._parse_list(attribute, model, mapping)
     94         else:
     95             self._parse_leaf(attribute, model, mapping)

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse_list(self, attribute, model, mapping)
    169 
    170             element_mapping = copy.deepcopy(mapping)
--> 171             self._parse(key, obj, element_mapping)
    172 
    173         # Restore state

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse(self, attribute, model, mapping)
     89 
     90         if model._is_container in ("container", ):
---> 91             self._parse_container(attribute, model, mapping)
     92         elif model._yang_type in ("list", ):
     93             self._parse_list(attribute, model, mapping)

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse_container(self, attribute, model, mapping)
    127                 parser.parse()
    128             else:
--> 129                 self._parse(k, v, mapping[v._yang_name])
    130 
    131         # Restoring state

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse(self, attribute, model, mapping)
     89 
     90         if model._is_container in ("container", ):
---> 91             self._parse_container(attribute, model, mapping)
     92         elif model._yang_type in ("list", ):
     93             self._parse_list(attribute, model, mapping)

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse_container(self, attribute, model, mapping)
    127                 parser.parse()
    128             else:
--> 129                 self._parse(k, v, mapping[v._yang_name])
    130 
    131         # Restoring state

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse(self, attribute, model, mapping)
     91             self._parse_container(attribute, model, mapping)
     92         elif model._yang_type in ("list", ):
---> 93             self._parse_list(attribute, model, mapping)
     94         else:
     95             self._parse_leaf(attribute, model, mapping)

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parser.pyc in _parse_list(self, attribute, model, mapping)
    145 
    146         for key, block, extra_vars in self.parser.parse_list(attribute, mapping["_process"],
--> 147                                                              self.bookmarks):
    148             logger.debug("Parsing element {}[{}]".format(attribute, key))
    149 

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parsers/base.pyc in parse_list(self, attribute, mapping, bookmarks)
     90             data = self.resolve_path(bookmarks, m.get("from", "parent"))
     91 
---> 92             for key, block, extra_vars in self._parse_list_default(attribute, m, data):
     93                 yield key, block, extra_vars
     94 

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parsers/jsonp.py in _parse_list_default(self, attribute, mapping, data, key)
     73 
     74         print('PRE ', attribute, mapping.get("path", ""), data)
---> 75         d = self.resolve_path(data, mapping.get("path", ""), mapping.get("default"))
     76         print('POST', attribute, mapping.get("path", ""), d)
     77         regexp = mapping.get('regexp')

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parsers/base.pyc in resolve_path(self, my_dict, path, default, check_presence)
     43                     #    result.append((k, v))
     44                     #    continue
---> 45                     r = self.resolve_path(v, ".".join(path_split), default, check_presence)
     46                     if not r:
     47                         break

/home/matej/Projects/napalmyang/napalm-yang/napalm_yang/parsers/base.pyc in resolve_path(self, my_dict, path, default, check_presence)
     31                     if isinstance(b, dict):
     32                         b = [b]
---> 33                     p, var_name = p.split(":")
     34                     try:
     35                         iterator = {e[var_name]: e for e in b}.items()

ValueError: need more than 1 value to unpack

The touples ('mask', '255.255.255.255'), ('prefix', '1.1.1.1') in the last PRE line are the problem.

This fix will just pass them through unchanged.

[dbarrosop]

Yay! You found a bug on what is probably one of the few cases I forgot to test XD Would you mind adding a test?

https://github.com/napalm-automation/napalm-yang/blob/develop/test/unit/test_parser.py#L107

This code is delicate and critical and confusing enough that deserve it's own set of tests :P

[matejv]

\o/ yay I broke it!

All right, one test coming up. I just have one more issue to resolve before.

[matejv]

OK, that was a bit more work then expected. The new test_parser definitions don't allow for calling parse_list recursively, so I added an option to pass bookmarks in test data.

Also the tests fail now, because of a problem described in #80

Hopefully the documentation makes it clear what I'm trying to do.

[dbarrosop]

Turns out I fixed this in parallel to you for some other reason... In any case, I took a look at how you were parsing the next-hops and came up with this:

https://github.com/napalm-automation/napalm-yang/compare/ArnesSI-resolve_path-fix#diff-7c66c87122818b2e7345e13b3f209eeeR124

As you can see I did it in one single rule. I also took into account metrics although I am not sure it's correct. Could you try and let me know if it works?

[dbarrosop]

I don't think this one is needed, feel free to let me know otherwise.