review_tools complains about netlink_driver having a string value in snap declaration

[Daniel Manrique]

This plug declaration :

{
  "allow-auto-connection": [
    {
      "slot-snap-id": [
        "whatever"
      ],
      "slot-names": [
        "netlink-foo"
      ],
      "slot-attributes": {
        "family": "24",
        "family-name": "a-family"
      },
      "plug-attributes": {
        "family-name": "$SLOT(family-name)"
      },
      "on-store": [
        "what-nice-store"
      ]
    }
  ]
}

and a snap with this in the snap.yaml:

slots:
  netlink-foo:
    interface: netlink-driver
    family: 24
    family-name: a-family

results in review-tools complaining as follows:

- declaration malformed (wrong type '24' for attribute 'family') declaration-snap-v2_valid_plugs (netlink-driver, allow-auto-connection_slot-attributes)

Looking at review-tools' interface_attribs I think review-tools wants the value in the snap-declaration to be an integer, but if I try to change the snap-declaration to:

      "slot-attributes": {
        "family": 24, <-------------- Making this an int
        "family-name": "a-family"
      },

then I can't even save the snap declaration:

netlink-driver must be a json-serialized dict. Dicts and lists are allowed as values, scalar values must be strings.

Does review-tools need to also (or instead) accept a string for "family"?

[Daniel Manrique (roadmr)]

Apologies, looks like that lateral validation is done in the Snap Store dashboard code, this is applied to the dictionary and it clearly will barf on anything that's not a list, dict or string (lists and dicts must additionally also contain only lists, dicts or strings themselves).

def _valid_items(items):
        is_valid = True
        # items must be strings, or (lists or dicts) of items
        for item in items:
            is_valid = (
                isinstance(item, basestring)
                or (isinstance(item, list) and _valid_items(item))
                or (isinstance(item, dict) and _valid_items(item.values()))
            )
            if not is_valid:
                raise ValidationError(error_msg)
        return is_valid

[Samuele Pedroni (pedronis)]

For clarify _valid_items is just a pre-check. The assertion code and the assertion service themselves would return an error on an int header as well. That is by design.

[Emilia Torino (emitorino)]

Fixed in https://git.launchpad.net/review-tools/commit/?h=20211105-1827UTC&id=4b6e1c0fab6a86a309b15f5f6326d4e420340dbd