Bug: weird casting of `3_5` to `35` resulting in `KeyError`

[aqw]

This is easiest to explore in our inventory repo, but I will try to provide some context:

Some assets contain the following structure:

connectors:
  mDP: 1
  DP: 2 # 1 x out w/ MST
  HDMI: 1
  USB-A: 4
  USB-B: 1
  3_5: 1
  DVI: 1

However, when I try to perform any form of write action on that asset, it results in an error:

❱ onyo edit warehouse/display_dell_u2413f.abc123
ERROR: "'connectors.35'"

❱ onyo set --keys test=toast --asset warehouse/display_dell_u2413f.abc123
ERROR: "'connectors.35'"

Reading is a different story

❱ onyo cat warehouse/display_dell_u2413f.abc123
---
...
connectors:
  mDP: 1
  DP: 2 # 1 x out w/ MST
  HDMI: 1
  USB-A: 4
  USB-B: 1
  3_5: 1
  DVI: 1

The connectors key works, but something is weird/wrong with 3_5:

❱ onyo get --keys connectors connectors.DVI connectors.3_5 connectors.35 --path warehouse/display_dell_u2413f.abc123
 ────────────────────────────────────────────────────────────── 
  connectors   connectors.DVI   connectors.3_5   connectors.35  
 ────────────────────────────────────────────────────────────── 
  <dict>       1                <unset>          <unset>        
 ────────────────────────────────────────────────────────────── 

When I edit the asset manually and change 3_5 to 3-5, everything is fine again:

❱ onyo get --keys connectors connectors.3_5 connectors.35 connectors.3-5 --path warehouse/display_dell_u2413f.abc123
  ────────────────────────────────────────────────────────────── 
  connectors   connectors.3_5   connectors.35   connectors.3-5  
 ────────────────────────────────────────────────────────────── 
  <dict>       <unset>          <unset>         1               
 ────────────────────────────────────────────────────────────── 

When I enable debugging, I get the following traceback:

❱ onyo --debug set --keys test=toast --asset warehouse/display_dell_u2413f.abc123
...
ERROR: "'connectors.35'"
DEBUG:onyo:Traceback (most recent call last):
  File "/home/aqw/git/inm7/onyo/onyo/lib/utils.py", line 91, in __getitem__
    return effective_dict[parts[-1]]
           ~~~~~~~~~~~~~~^^^^^^^^^^^
  File "/home/aqw/.venvs/onyo/lib/python3.12/site-packages/ruamel/yaml/comments.py", line 853, in __getitem__
    return ordereddict.__getitem__(self, key)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
KeyError: '35'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/aqw/git/inm7/onyo/onyo/main.py", line 543, in main
    args.run(args)
  File "/home/aqw/git/inm7/onyo/onyo/cli/set.py", line 109, in set
    onyo_set(inventory=inventory,
  File "/home/aqw/git/inm7/onyo/onyo/lib/commands.py", line 82, in wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/home/aqw/git/inm7/onyo/onyo/lib/commands.py", line 1063, in onyo_set
    inventory.modify_asset(asset, new_content)
  File "/home/aqw/git/inm7/onyo/onyo/lib/inventory.py", line 437, in modify_asset
    self.raise_required_key_empty_value(new_asset)
  File "/home/aqw/git/inm7/onyo/onyo/lib/inventory.py", line 709, in raise_required_key_empty_value
    if any(v is None or not str(v).strip()
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/aqw/git/inm7/onyo/onyo/lib/inventory.py", line 710, in <genexpr>
    for k, v in asset.items()
                ^^^^^^^^^^^^^
  File "<frozen _collections_abc>", line 894, in __iter__
  File "/home/aqw/git/inm7/onyo/onyo/lib/utils.py", line 93, in __getitem__
    raise KeyError(f"'{key}'") from e
KeyError: "'connectors.35'"

This behavior is reproducible with onyo new and templates.

[aqw]

This turns out to be a "bug" in the YAML spec, and ruamel is correctly following their insanity.

• https://docs.saltproject.io/en/latest/topics/troubleshooting/yaml_idiosyncrasies.html#underscores-stripped-in-integer-definitions
• https://yaml.org/type/int.html

Words fail me.

I will fix our inventory repo to no longer use underscores with integers. And will add this to the list of fsck checks.

Closing.

[aqw]

Comment: I don't know whether this would have been protected against by treating all key names as strings. (#671). I think the key name would need to be quoted to begin with.

[bpoldrack]

Comment: I don't know whether this would have been protected against by treating all key names as strings.

I do currently think, that is the right direction. We convert keys to strings currently only within dot notation (and there we have to), and that is the actual problem here. We get an integer key and then end up accessing a (wrongly) stringified version of it, which fails. Instead, we need to stringify the keys we get when reading the YAML, and do so by utilizing ruamel's internal knowledge of how it's actually represented (roundtrip), so we get '3_5' in this case rather than str(35). Otherwise there likely will be a lot of corner cases where confusing errors happen. In this case here, we get an error about a key, that the user didn't say to do anything with. This type of thing will remain troublesome, if we don't solve it properly. But if it works as I currently think, there would be no need to restrict the YAML.

So - re-opening, since this needs addressing (and I think I know how).