netplan:slyon/keyfile-0103-compat

Last commit made on 2021-10-11
Get this branch:
git clone -b slyon/keyfile-0103-compat https://git.launchpad.net/netplan

Branch merges

Branch information

Name:
slyon/keyfile-0103-compat
Repository:
lp:netplan

Recent commits

e697ee6... by Lukas Märdian

nm: Do not set missing gateway as unspecified address

Also avoid the deprecated trailing comma notation at the same time.

03878f1... by Lukas Märdian

nm:parse-nm: Hide NM's automatic route-scope detection

NetworkManager automatically detects a route's scope, depending on
destination IP ("to") and gateway ("via"). If no gateway is specified
(e.g. the unspecified address "0.0.0.0"/"::" in keyfile) it will assume
a "link"/"host" scope, otherwise it will assume a "global" scope.

747e02a... by Lukas Märdian

parse-nm: Fix typos

Co-authored-by: Simon Chopin <email address hidden>

57780ef... by Lukas Märdian

parse-nm: Handle missing gateway in keyfile routes, keep dns-search fallback

NM assumes a route to use the unspecified address as the gateway
(via = "0.0.0.0"/"::") if none is specified in the keyfile.
E.g. the route is only valid on the local network:
"ip route add NETWORK dev DEVICE [metric METRIC]"

netplan cannot differentiate between ipv4.dns-search and ipv6.dns-search
so keep it in the passthrough/fallback list as an override.

bc6eb0b... by Simon Chopin <email address hidden>

netplan: set: make it possible to unset a whole devtype subtree (LP: #1942930) (FR-1685) (#236)

Previously, trying to unset a whole devtype subtree, such as netplan set network.ethernets=null would fail. This PR fixes it by detecting this special case and manually deleting each netdef assigned to the subtype.

This approach is far from being performant, but it has the merit of working without needing too much new code.

COMMITS:
* tests: cli_get_set: use assertRaises to check for exceptions
This allows the test machinery to do some better error reporting when an
exception is raised, as it understands that the issue is an uncaught
exception, not a failed comparison.

* lib: names: add a function to parse a string into a devtype
As I'm expecting this kind of function to be useful for the parser, the
implementation is directly in the form of a macro modelled after its
counterpart.

* lib,netplan: tell Python about all the netdefs for a given devtype
This is implemented via iterators to limit the knowledge Python has of
the underlying memory model. The new symbols are prefixed with '_' to
denote that they are not to be used by public consumers.

We also expose process_yaml_hierarchy which was already part of the ABI,
as it seems needed for the Python helper implementation.

Contrary to the usual FFI calls, this one explicitly checks the
existence of the symbols and raises an exception if the symbols are not
found, as it is not totally unlikely that netplan.io and libnetplan0 be
upgraded out of sync.

V2: rename the C internal iterator struct, which hadn't evolved along
with the successive iterations on the code

fixup itar

* netplan: set: allow the removal of an entire devtype subtree
See LP: #1942930

Note that performance for this are horrendous, as we parse the whole
tree once for *each* removed netdef!

V2:
* Lukas as co-author, as he wrote the test!
* Clean up commented-out code in the tests

Co-authored-by: Lukas Märdian <email address hidden>

467e88a... by Simon Chopin <email address hidden>

API/ABI: restrict the symbol export to a determined public API (#227)

The goal of this PR is to properly determine what the public interface of libnetplan is. There is potentially a bit of ABI breakage, as symbols now default to hidden status unless explicitly marked otherwise, and said marking was done manually by looking at the current consumers I had on my system and trying to fill in the blanks.

The API, on the other hand, has been conscientiously broken, as most of what was exposed in the headers are now safely hidden away in our internal headers. This is by design, and AFAICT should not break existing code.

This PR depends on #230, as denoted by the merge commit in there. It should make independent review a bit easier :).

Given the fact that the main purpose of this PR is to limit the amount of symbols we export, we can safely assume that strictly speaking, there is ABI breakage. However, the whole point was to only expose the symbols that are in actual use out there in the real world.

As usual, the patchset has been edited expecting a review commit by commit, as the whole diff can be a bit daunting.

COMMITS:

* libnetplan: rework netplan,parse{-nm},util.h as public headers

This work involves splitting out some things from those headers into new
internal headers, and moving definitions around so that the public
headers are as self-contained as possible.

For the new internal headers, I decided to have a big "types.h" header
containing all the types used in a network definition. In itself, these
types aren't related to parsing except that the parser module is the
only producer, so I decided they could live on their own. This is also
the place where type-specific helpers can be found, such as
reset_netdef().

The various macros used to generate YAML were gathered up from both
source headers into a new yaml-helpers.h header, whereas the various
global state definitions were split off into their own headers, making
it easier to spot which areas of the code still rely on global state.
The remainder functions were moved into util-internal.h

This allows us to minimize the API exposed to the outside world.

V2:
  * Remove the extraneous struct net_definition line, as it is redundant
    with the typedef
  * Normalize all the symbol declarations to have the symbol name at the
    beginning of their own line, at least in the headers we touched.

V3:
  * Remove extraneous stdbool.h header
  * Split parse-helpers.h into yaml-helpers.h and util-internal.h
  * Add uuid as a dependency of the dbus generator to make it compile
    with the new lay of the land.
  * Add some missing copyright headers

* libnetplan: Properly mark the lib/bin interface

This patch cleanly marks which functions/objects are part of the public
library API, and which are meant to be consumed by our own binaries.

In order to reduce as much as possible the ABI dependencies between our
binaries and the library, I move the various generator modules into the
library as it make sense for them to access directly the objects. On the
other hand, both generate.c and dbus.c should be relatively trivial to
change to use getters and setters instead of direct structure access.

Those changes will be the object of later patches.

V3:
  * Makefile: consolidate the pkg-config calls and add back the LDFLAGS
    variable
  * Normalize the touched function declarations to always have the
    symbol name at the start of the line.

* libnetplan: add back ABI compatibility

Re-export (and recreate) various symbols that are needed by the
`generate` binary as shipped in the Ubuntu package 0.103-0ubuntu5

I have chosen a separate marker for those, in order to distinguish what
was exposed deliberately and by accident. That way, if we need to do a
SONAME bump, we can clean up the symbols marked with NETPLAN_ABI.

V3:
  * Normalize the declarations that are touched to always have the
    symbol name at the start of the line

* libnetplan: export Python-used symbols as internal

* libnetplan: hide all symbols by default

Only those marked by NETPLAN_{PUBLIC,INTERNAL,ABI} will be available to
the loader.

76ae706... by Nicolas Bock

Add support for additional `Link` options (#225) (LP: #1771740)

This change adds systemd support for additional configuration options
for interfaces.

* ReceiveChecksumOffload
* TransmitChecksumOffload
* TCPSegmentationOffload
* TCP6SegmentationOffload
* GenericSegmentationOffload
* GenericReceiveOffload
* LargeReceiveOffload

Closes: https://bugs.launchpad.net/netplan/+bug/1771740

Signed-off-by: Nicolas Bock <email address hidden>

23cb913... by Simon Chopin <email address hidden>

lib: unify how constant names are exposed and used (#230)

This patch moves all the various enum-to-string arrays into a single
compilation unit instead of exposing them directly into headers, and
exposes the data through simple functions.

This approach has multiple advantages:

* If we want to expose one or several of those conversion functions to
  the ABI, we now can add new constants without breaking the ABI, as the
  functions can deal with unknown constants in a sensible manner.
  Directly exposing the arrays in the headers as was previously done was
  a recipy for disaster, as the array is baked into the compiled client code,
  and will not be resized if we decide to add new values to the enums
  in new versions of the library.
* It's marginally faster to compile, as the compiler doesn't have to
  copy around multiple times the same arrays, only to deduplicate them
  at link time (if it even does that).

V2:
* Drop the np_ prefix in favor of the classical netplan_ prefix
* Whitespace fixes
* Copyright headers
* Remove a couple of defines that sneaked up where they didn't belong

730fbbd... by Lukas Märdian

Implement YAML state tracking and use it in the DBus API and netplan-try (LP: #1943120) (FR-1745) (#231)

Allow to pass an optional --state parameter to netplan try/apply describing a directory that contains a netplan configuration tree/state (i.e. /{etc,run,lib}/netplan/*.yaml).
Netplan will make use of this "old state" to calculate the delta of dropped interface definitions, like bridges/bonds/vlans/tunnels that have been configured before but are not part of the current YAML configuration anymore. It will then try to delete those virtual interfaces (via ip link del dev IFACE) if they still exist.

The same functionality is used to roll back a netplan try command that failed or was rejected.

Generally, the state needs to be provided manually. The DBus API (using io.netplan.Netplan.Config.Try/Apply) is an exception, as the previous state can be backed up automatically in this case.

COMMITS:
* cli:apply:configmanager: clear_virtual_links during apply
* tests:scenarios: check virtual interface cleanup
* doc:apply: update manpage
* cli:try: use clear_virtual_links from Apply()
* dbus: make use of new YAML state tracking
* dbus: properly create and clean the backup state dir
* cli:apply:clear_virtual_links: make devices a named parameter

85ef928... by Simon Chopin <email address hidden>

tests: normalize multiple keys on a single dict (#229)

The YAML normalizer has some special cases for some given keys. However,
the code block where those are dealt with used `elif` everywhere, which
meant that if a single dict had multiple keys needing to be dealt with,
only the first one would be fixed, and the rest would continue on, still
broken.

This bug is latent in the current codebase, but it showed up in a
patchset I'm working on, where by the game of butterfly effects the
rendered configuration would insist on having proper renderer for each
netdef. This key needs normalizing, but in the test cases where the
netdefs have other problematic properties, for instance the GSM config
in `test_cdma_config`, those pesky renderer lines would still show up
and screw up the test.

V2:
* Beat pycheckstyle into submission in a much more sensible fashion,
  thanks to @slyon