RFC: New "networkmanager.passthrough" structure (LP: #2080301)

[daniloegea]

Description

This PR proposes a new format for the networkmanager.passthrough section. The new format was a suggestion from the LP#2080301 bug reporter. It addresses the problem reported in the bug and is future proof against similar problems.

The main problem with the currently used format is that it's ambiguous. The keyfile [group].key=value format is encoded as a string separated by dots. As dots can be used in the key name, it becomes tricky to separate what is the group and what is the key name. A different character could be used but we could eventually hit the same problem.

Arguably we could do something like [group-name-between-brackets]key-name: value. Brackets are forbidden in group names (I think?!). But it still would be a change in the format so I opted for turning each entry into mappings.

Here is how it looks like:

  nm-devices:
    NM-4bf69677-e326-49b4-8e37-2acf3074f6c7:
      renderer: NetworkManager
      networkmanager:
        uuid: "4bf69677-e326-49b4-8e37-2acf3074f6c7"
        name: "tuntap7"
        passthrough:
          connection:
            type: "tun"
            autoconnect: "false"
            interface-name: "tuntap7"
          proxy: {}
          tun: {}
          ipv4:
            method: "manual"
            address1: "4.3.2.1/24,1.2.3.254"
            dns: "1.2.3.254;4.3.2.254;"
          ipv6:
            addr-gen-mode: "default"
            method: "manual"
            address1: "1:2:3::4/64,1:2:3::abcd"
            dns: "1:2:3::abcd;abcd:3:2::1;"

With the integration of netplan and Network Manager on Ubuntu, tweaking the passthrough configuration might be necessary every now and then due to things missing in netplan. This format makes it easier to do that.

The old format is still accepted for backwards compatibility. The parser still accepts it but will emit the new format. So YAMLs generated by Network Manager will be updated if the user modifies the connection and it will still work with existing YAMLs if they are never updated.

IMPORTANT: the datalist data structure was replaced with hash tables. This will make the order in which elements are added to keyfiles unpredictable. I'm not 100% sure that ordering would cause problems with configuration from the passthrough section. My main motivation to drop datalists is that GQuarks might leak memory. We hit this issue in our config_fuzzer test due to the number of GQuarks needed when a big number of entries are added to the datalist.

A couple of Network Manager autopkgtests are failing due to the change in the structure and will need to be fixed as well.

I understand it's a big change and we might decide to not merge it so I made it an RFC.

Checklist

• [ ] Runs make check successfully.
• [ ] Retains code coverage (make check-coverage).
• [ ] New/changed keys in YAML format are documented.
• [ ] (Optional) Adds example YAML for new feature.
• [ ] (Optional) Closes an open bug in Launchpad.

[daniloegea]

Thank you, Lukas.

I addressed a bunch of your comments and left answers for many others.

I think the main highlights are:

• netplan set will not work for updating fields inside the passthrough block. The reason is the way handle_generic_map works when it finds the same fields with different values. It also doesn't work for openvswitch.other-config for example due to the same problem. Adding and nullifying fields works fine.
• I updated the documentation too

I changed a lot of stuff, it's a good idea to go over all the code again (sorry for that).

[slyon]

FTR: This is an enhancement of https://github.com/canonical/netplan/pull/193