From 0d93e53fd05c44b62e3456b7580c9de8135e6b5a Mon Sep 17 00:00:00 2001

From: Emanuele Giuseppe Esposito <eesposit@redhat.com>

Date: Mon, 2 May 2022 14:21:24 +0200

Subject: [PATCH 1/4] Add native NetworkManager support (#1224)

RH-Author: Emanuele Giuseppe Esposito <eesposit@redhat.com>

RH-MergeRequest: 57: Add native NetworkManager support (#1224)

RH-Commit: [1/2] 56b9ed40840a4930c421c2749e8aa385097bef93

RH-Bugzilla: 2059872

RH-Acked-by: Vitaly Kuznetsov <vkuznets@redhat.com>

RH-Acked-by: Jon Maloy <jmaloy@redhat.com>

RH-Acked-by: Eduardo Otubo <otubo@redhat.com>

commit feda344e6cf9d37b09bc13cf333a717d1654c26c

Author: Lubomir Rintel <lkundrak@v3.sk>

Date:   Fri Feb 25 23:33:20 2022 +0100

    Add native NetworkManager support (#1224)

    Fedora currently relies on sysconfig/ifcfg renderer. This is not too great,

    because Fedora (also RHEL since version 8) dropped support for the legacy

    network service that uses ifcfg files long ago.

    In turn, Fedora ended up patching cloud-init downstream to utilize

    NetworkManager's ifcfg compatibility mode [1]. This seems to have worked

    for a while, nevertheless the NetworkManager's ifcfg backend is reaching

    the end of its useful life too [2].

    [1] https://src.fedoraproject.org/rpms/cloud-init/blob/rawhide/f/cloud-init-21.3-nm-controlled.patch

    [2] https://fedoraproject.org/wiki/Changes/NoIfcfgFiles

    Let's not mangle things downstream and make vanilla cloud-init work great

    on Fedora instead.

    This also means that the sysconfig compatibility with

    Network Manager was removed.

    Firstly, this relies upon the fact that you can get ifcfg support by adding

    it to NetworkManager.conf. That is not guaranteed and certainly will not

    be case in future.

    Secondly, cloud-init always generates configuration with

    NM_CONTROLLED=no, so the generated ifcfg files are no good for

    NetworkManager. Fedora patches around this by just removing those lines

    in their cloud-init package.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

---

 cloudinit/cmd/devel/net_convert.py     |   14 +-

 cloudinit/net/activators.py            |   25 +-

 cloudinit/net/network_manager.py       |  377 +++++++

 cloudinit/net/renderers.py             |    3 +

 cloudinit/net/sysconfig.py             |   37 +-

 tests/unittests/test_net.py            | 1270 +++++++++++++++++++++---

 tests/unittests/test_net_activators.py |   93 +-

 7 files changed, 1625 insertions(+), 194 deletions(-)

 create mode 100644 cloudinit/net/network_manager.py

diff --git a/cloudinit/cmd/devel/net_convert.py b/cloudinit/cmd/devel/net_convert.py

index 18b1e7ff..647fe07b 100755

--- a/cloudinit/cmd/devel/net_convert.py

+++ b/cloudinit/cmd/devel/net_convert.py

@@ -7,7 +7,14 @@ import os

 import sys

 from cloudinit import distros, log, safeyaml

-from cloudinit.net import eni, netplan, network_state, networkd, sysconfig

+from cloudinit.net import (

+    eni,

+    netplan,

+    network_manager,

+    network_state,

+    networkd,

+    sysconfig,

+)

 from cloudinit.sources import DataSourceAzure as azure

 from cloudinit.sources import DataSourceOVF as ovf

 from cloudinit.sources.helpers import openstack

@@ -74,7 +81,7 @@ def get_parser(parser=None):

     parser.add_argument(

         "-O",

         "--output-kind",

-        choices=["eni", "netplan", "networkd", "sysconfig"],

+        choices=["eni", "netplan", "networkd", "sysconfig", "network-manager"],

         required=True,

         help="The network config format to emit",

     )

@@ -148,6 +155,9 @@ def handle_args(name, args):

     elif args.output_kind == "sysconfig":

         r_cls = sysconfig.Renderer

         config = distro.renderer_configs.get("sysconfig")

+    elif args.output_kind == "network-manager":

+        r_cls = network_manager.Renderer

+        config = distro.renderer_configs.get("network-manager")

     else:

         raise RuntimeError("Invalid output_kind")

diff --git a/cloudinit/net/activators.py b/cloudinit/net/activators.py

index e80c26df..edbc0c06 100644

--- a/cloudinit/net/activators.py

+++ b/cloudinit/net/activators.py

@@ -1,15 +1,14 @@

 # This file is part of cloud-init. See LICENSE file for license information.

 import logging

-import os

 from abc import ABC, abstractmethod

 from typing import Iterable, List, Type

 from cloudinit import subp, util

 from cloudinit.net.eni import available as eni_available

 from cloudinit.net.netplan import available as netplan_available

+from cloudinit.net.network_manager import available as nm_available

 from cloudinit.net.network_state import NetworkState

 from cloudinit.net.networkd import available as networkd_available

-from cloudinit.net.sysconfig import NM_CFG_FILE

 LOG = logging.getLogger(__name__)

@@ -124,20 +123,24 @@ class IfUpDownActivator(NetworkActivator):

 class NetworkManagerActivator(NetworkActivator):

     @staticmethod

     def available(target=None) -> bool:

-        """Return true if network manager can be used on this system."""

-        config_present = os.path.isfile(

-            subp.target_path(target, path=NM_CFG_FILE)

-        )

-        nmcli_present = subp.which("nmcli", target=target)

-        return config_present and bool(nmcli_present)

+        """Return true if NetworkManager can be used on this system."""

+        return nm_available(target=target)

     @staticmethod

     def bring_up_interface(device_name: str) -> bool:

-        """Bring up interface using nmcli.

+        """Bring up connection using nmcli.

         Return True is successful, otherwise return False

         """

-        cmd = ["nmcli", "connection", "up", "ifname", device_name]

+        from cloudinit.net.network_manager import conn_filename

+

+        filename = conn_filename(device_name)

+        cmd = ["nmcli", "connection", "load", filename]

+        if _alter_interface(cmd, device_name):

+            cmd = ["nmcli", "connection", "up", "filename", filename]

+        else:

+            _alter_interface(["nmcli", "connection", "reload"], device_name)

+            cmd = ["nmcli", "connection", "up", "ifname", device_name]

         return _alter_interface(cmd, device_name)

     @staticmethod

@@ -146,7 +149,7 @@ class NetworkManagerActivator(NetworkActivator):

         Return True is successful, otherwise return False

         """

-        cmd = ["nmcli", "connection", "down", device_name]

+        cmd = ["nmcli", "device", "disconnect", device_name]

         return _alter_interface(cmd, device_name)

diff --git a/cloudinit/net/network_manager.py b/cloudinit/net/network_manager.py

new file mode 100644

index 00000000..79b0fe0b

--- /dev/null

+++ b/cloudinit/net/network_manager.py

@@ -0,0 +1,377 @@

+# Copyright 2022 Red Hat, Inc.

+#

+# Author: Lubomir Rintel <lkundrak@v3.sk>

+# Fixes and suggestions contributed by James Falcon, Neal Gompa,

+# Zbigniew Jędrzejewski-Szmek and Emanuele Giuseppe Esposito.

+#

+# This file is part of cloud-init. See LICENSE file for license information.

+

+import configparser

+import io

+import itertools

+import os

+import uuid

+

+from cloudinit import log as logging

+from cloudinit import subp, util

+

+from . import renderer

+from .network_state import is_ipv6_addr, subnet_is_ipv6

+

+NM_RUN_DIR = "/etc/NetworkManager"

+NM_LIB_DIR = "/usr/lib/NetworkManager"

+LOG = logging.getLogger(__name__)

+

+

+class NMConnection:

+    """Represents a NetworkManager connection profile."""

+

+    def __init__(self, con_id):

+        """

+        Initializes the connection with some very basic properties,

+        notably the UUID so that the connection can be referred to.

+        """

+

+        # Chosen by fair dice roll

+        CI_NM_UUID = uuid.UUID("a3924cb8-09e0-43e9-890b-77972a800108")

+

+        self.config = configparser.ConfigParser()

+        # Identity option name mapping, to achieve case sensitivity

+        self.config.optionxform = str

+

+        self.config["connection"] = {

+            "id": f"cloud-init {con_id}",

+            "uuid": str(uuid.uuid5(CI_NM_UUID, con_id)),

+        }

+

+        # This is not actually used anywhere, but may be useful in future

+        self.config["user"] = {

+            "org.freedesktop.NetworkManager.origin": "cloud-init"

+        }

+

+    def _set_default(self, section, option, value):

+        """

+        Sets a property unless it's already set, ensuring the section

+        exists.

+        """

+

+        if not self.config.has_section(section):

+            self.config[section] = {}

+        if not self.config.has_option(section, option):

+            self.config[section][option] = value

+

+    def _set_ip_method(self, family, subnet_type):

+        """

+        Ensures there's appropriate [ipv4]/[ipv6] for given family

+        appropriate for given configuration type

+        """

+

+        method_map = {

+            "static": "manual",

+            "dhcp6": "dhcp",

+            "ipv6_slaac": "auto",

+            "ipv6_dhcpv6-stateless": "auto",

+            "ipv6_dhcpv6-stateful": "auto",

+            "dhcp4": "auto",

+            "dhcp": "auto",

+        }

+

+        # Ensure we got an [ipvX] section

+        self._set_default(family, "method", "disabled")

+

+        try:

+            method = method_map[subnet_type]

+        except KeyError:

+            # What else can we do

+            method = "auto"

+            self.config[family]["may-fail"] = "true"

+

+        # Make sure we don't "downgrade" the method in case

+        # we got conflicting subnets (e.g. static along with dhcp)

+        if self.config[family]["method"] == "dhcp":

+            return

+        if self.config[family]["method"] == "auto" and method == "manual":

+            return

+

+        self.config[family]["method"] = method

+        self._set_default(family, "may-fail", "false")

+        if family == "ipv6":

+            self._set_default(family, "addr-gen-mode", "stable-privacy")

+

+    def _add_numbered(self, section, key_prefix, value):

+        """

+        Adds a numbered property, such as address<n> or route<n>, ensuring

+        the appropriate value gets used for <n>.

+        """

+

+        for index in itertools.count(1):

+            key = f"{key_prefix}{index}"

+            if not self.config.has_option(section, key):

+                self.config[section][key] = value

+                break

+

+    def _add_address(self, family, subnet):

+        """

+        Adds an ipv[46]address<n> property.

+        """

+

+        value = subnet["address"] + "/" + str(subnet["prefix"])

+        self._add_numbered(family, "address", value)

+

+    def _add_route(self, family, route):

+        """

+        Adds a ipv[46].route<n> property.

+        """

+

+        value = route["network"] + "/" + str(route["prefix"])

+        if "gateway" in route:

+            value = value + "," + route["gateway"]

+        self._add_numbered(family, "route", value)

+

+    def _add_nameserver(self, dns):

+        """

+        Extends the ipv[46].dns property with a name server.

+        """

+

+        # FIXME: the subnet contains IPv4 and IPv6 name server mixed

+        # together. We might be getting an IPv6 name server while

+        # we're dealing with an IPv4 subnet. Sort this out by figuring

+        # out the correct family and making sure a valid section exist.

+        family = "ipv6" if is_ipv6_addr(dns) else "ipv4"

+        self._set_default(family, "method", "disabled")

+

+        self._set_default(family, "dns", "")

+        self.config[family]["dns"] = self.config[family]["dns"] + dns + ";"

+

+    def _add_dns_search(self, family, dns_search):

+        """

+        Extends the ipv[46].dns-search property with a name server.

+        """

+

+        self._set_default(family, "dns-search", "")

+        self.config[family]["dns-search"] = (

+            self.config[family]["dns-search"] + ";".join(dns_search) + ";"

+        )

+

+    def con_uuid(self):

+        """

+        Returns the connection UUID

+        """

+        return self.config["connection"]["uuid"]

+

+    def valid(self):

+        """

+        Can this be serialized into a meaningful connection profile?

+        """

+        return self.config.has_option("connection", "type")

+

+    @staticmethod

+    def mac_addr(addr):

+        """

+        Sanitize a MAC address.

+        """

+        return addr.replace("-", ":").upper()

+

+    def render_interface(self, iface, renderer):

+        """

+        Integrate information from network state interface information

+        into the connection. Most of the work is done here.

+        """

+

+        # Initialize type & connectivity

+        _type_map = {

+            "physical": "ethernet",

+            "vlan": "vlan",

+            "bond": "bond",

+            "bridge": "bridge",

+            "infiniband": "infiniband",

+            "loopback": None,

+        }

+

+        if_type = _type_map[iface["type"]]

+        if if_type is None:

+            return

+        if "bond-master" in iface:

+            slave_type = "bond"

+        else:

+            slave_type = None

+

+        self.config["connection"]["type"] = if_type

+        if slave_type is not None:

+            self.config["connection"]["slave-type"] = slave_type

+            self.config["connection"]["master"] = renderer.con_ref(

+                iface[slave_type + "-master"]

+            )

+

+        # Add type specific-section

+        self.config[if_type] = {}

+

+        # These are the interface properties that map nicely

+        # to NetworkManager properties

+        _prop_map = {

+            "bond": {

+                "mode": "bond-mode",

+                "miimon": "bond_miimon",

+                "xmit_hash_policy": "bond-xmit-hash-policy",

+                "num_grat_arp": "bond-num-grat-arp",

+                "downdelay": "bond-downdelay",

+                "updelay": "bond-updelay",

+                "fail_over_mac": "bond-fail-over-mac",

+                "primary_reselect": "bond-primary-reselect",

+                "primary": "bond-primary",

+            },

+            "bridge": {

+                "stp": "bridge_stp",

+                "priority": "bridge_bridgeprio",

+            },

+            "vlan": {

+                "id": "vlan_id",

+            },

+            "ethernet": {},

+            "infiniband": {},

+        }

+

+        device_mtu = iface["mtu"]

+        ipv4_mtu = None

+

+        # Deal with Layer 3 configuration

+        for subnet in iface["subnets"]:

+            family = "ipv6" if subnet_is_ipv6(subnet) else "ipv4"

+

+            self._set_ip_method(family, subnet["type"])

+            if "address" in subnet:

+                self._add_address(family, subnet)

+            if "gateway" in subnet:

+                self.config[family]["gateway"] = subnet["gateway"]

+            for route in subnet["routes"]:

+                self._add_route(family, route)

+            if "dns_nameservers" in subnet:

+                for nameserver in subnet["dns_nameservers"]:

+                    self._add_nameserver(nameserver)

+            if "dns_search" in subnet:

+                self._add_dns_search(family, subnet["dns_search"])

+            if family == "ipv4" and "mtu" in subnet:

+                ipv4_mtu = subnet["mtu"]

+

+        if ipv4_mtu is None:

+            ipv4_mtu = device_mtu

+        if not ipv4_mtu == device_mtu:

+            LOG.warning(

+                "Network config: ignoring %s device-level mtu:%s"

+                " because ipv4 subnet-level mtu:%s provided.",

+                iface["name"],

+                device_mtu,

+                ipv4_mtu,

+            )

+

+        # Parse type-specific properties

+        for nm_prop, key in _prop_map[if_type].items():

+            if key not in iface:

+                continue

+            if iface[key] is None:

+                continue

+            if isinstance(iface[key], bool):

+                self.config[if_type][nm_prop] = (

+                    "true" if iface[key] else "false"

+                )

+            else:

+                self.config[if_type][nm_prop] = str(iface[key])

+

+        # These ones need special treatment

+        if if_type == "ethernet":

+            if iface["wakeonlan"] is True:

+                # NM_SETTING_WIRED_WAKE_ON_LAN_MAGIC

+                self.config["ethernet"]["wake-on-lan"] = str(0x40)

+            if ipv4_mtu is not None:

+                self.config["ethernet"]["mtu"] = str(ipv4_mtu)

+            if iface["mac_address"] is not None:

+                self.config["ethernet"]["mac-address"] = self.mac_addr(

+                    iface["mac_address"]

+                )

+        if if_type == "vlan" and "vlan-raw-device" in iface:

+            self.config["vlan"]["parent"] = renderer.con_ref(

+                iface["vlan-raw-device"]

+            )

+        if if_type == "bridge":

+            # Bridge is ass-backwards compared to bond

+            for port in iface["bridge_ports"]:

+                port = renderer.get_conn(port)

+                port._set_default("connection", "slave-type", "bridge")

+                port._set_default("connection", "master", self.con_uuid())

+            if iface["mac_address"] is not None:

+                self.config["bridge"]["mac-address"] = self.mac_addr(

+                    iface["mac_address"]

+                )

+        if if_type == "infiniband" and ipv4_mtu is not None:

+            self.config["infiniband"]["transport-mode"] = "datagram"

+            self.config["infiniband"]["mtu"] = str(ipv4_mtu)

+            if iface["mac_address"] is not None:

+                self.config["infiniband"]["mac-address"] = self.mac_addr(

+                    iface["mac_address"]

+                )

+

+        # Finish up

+        if if_type == "bridge" or not self.config.has_option(

+            if_type, "mac-address"

+        ):

+            self.config["connection"]["interface-name"] = iface["name"]

+

+    def dump(self):

+        """

+        Stringify.

+        """

+

+        buf = io.StringIO()

+        self.config.write(buf, space_around_delimiters=False)

+        header = "# Generated by cloud-init. Changes will be lost.\n\n"

+        return header + buf.getvalue()

+

+

+class Renderer(renderer.Renderer):

+    """Renders network information in a NetworkManager keyfile format."""

+

+    def __init__(self, config=None):

+        self.connections = {}

+

+    def get_conn(self, con_id):

+        return self.connections[con_id]

+

+    def con_ref(self, con_id):

+        if con_id in self.connections:

+            return self.connections[con_id].con_uuid()

+        else:

+            # Well, what can we do...

+            return con_id

+

+    def render_network_state(self, network_state, templates=None, target=None):

+        # First pass makes sure there's NMConnections for all known

+        # interfaces that have UUIDs that can be linked to from related

+        # interfaces

+        for iface in network_state.iter_interfaces():

+            self.connections[iface["name"]] = NMConnection(iface["name"])

+

+        # Now render the actual interface configuration

+        for iface in network_state.iter_interfaces():

+            conn = self.connections[iface["name"]]

+            conn.render_interface(iface, self)

+

+        # And finally write the files

+        for con_id, conn in self.connections.items():

+            if not conn.valid():

+                continue

+            name = conn_filename(con_id, target)

+            util.write_file(name, conn.dump(), 0o600)

+

+

+def conn_filename(con_id, target=None):

+    target_con_dir = subp.target_path(target, NM_RUN_DIR)

+    con_file = f"cloud-init-{con_id}.nmconnection"

+    return f"{target_con_dir}/system-connections/{con_file}"

+

+

+def available(target=None):

+    target_nm_dir = subp.target_path(target, NM_LIB_DIR)

+    return os.path.exists(target_nm_dir)

+

+

+# vi: ts=4 expandtab

diff --git a/cloudinit/net/renderers.py b/cloudinit/net/renderers.py

index c755f04c..7edc34b5 100644

--- a/cloudinit/net/renderers.py

+++ b/cloudinit/net/renderers.py

@@ -8,6 +8,7 @@ from . import (

     freebsd,

     netbsd,

     netplan,

+    network_manager,

     networkd,

     openbsd,

     renderer,

@@ -19,6 +20,7 @@ NAME_TO_RENDERER = {

     "freebsd": freebsd,

     "netbsd": netbsd,

     "netplan": netplan,

+    "network-manager": network_manager,

     "networkd": networkd,

     "openbsd": openbsd,

     "sysconfig": sysconfig,

@@ -28,6 +30,7 @@ DEFAULT_PRIORITY = [

     "eni",

     "sysconfig",

     "netplan",

+    "network-manager",

     "freebsd",

     "netbsd",

     "openbsd",

diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py

index 362e8d19..c3b0c795 100644

--- a/cloudinit/net/sysconfig.py

+++ b/cloudinit/net/sysconfig.py

@@ -5,8 +5,6 @@ import io

 import os

 import re

-from configobj import ConfigObj

-

 from cloudinit import log as logging

 from cloudinit import subp, util

 from cloudinit.distros.parsers import networkmanager_conf, resolv_conf

@@ -66,24 +64,6 @@ def _quote_value(value):

         return value

-def enable_ifcfg_rh(path):

-    """Add ifcfg-rh to NetworkManager.cfg plugins if main section is present"""

-    config = ConfigObj(path)

-    if "main" in config:

-        if "plugins" in config["main"]:

-            if "ifcfg-rh" in config["main"]["plugins"]:

-                return

-        else:

-            config["main"]["plugins"] = []

-

-        if isinstance(config["main"]["plugins"], list):

-            config["main"]["plugins"].append("ifcfg-rh")

-        else:

-            config["main"]["plugins"] = [config["main"]["plugins"], "ifcfg-rh"]

-        config.write()

-        LOG.debug("Enabled ifcfg-rh NetworkManager plugins")

-

-

 class ConfigMap(object):

     """Sysconfig like dictionary object."""

@@ -1031,8 +1011,6 @@ class Renderer(renderer.Renderer):

             netrules_content = self._render_persistent_net(network_state)

             netrules_path = subp.target_path(target, self.netrules_path)

             util.write_file(netrules_path, netrules_content, file_mode)

-        if available_nm(target=target):

-            enable_ifcfg_rh(subp.target_path(target, path=NM_CFG_FILE))

         sysconfig_path = subp.target_path(target, templates.get("control"))

         # Distros configuring /etc/sysconfig/network as a file e.g. Centos

@@ -1071,14 +1049,9 @@ def _supported_vlan_names(rdev, vid):

 def available(target=None):

-    sysconfig = available_sysconfig(target=target)

-    nm = available_nm(target=target)

-    return util.system_info()["variant"] in KNOWN_DISTROS and any(

-        [nm, sysconfig]

-    )

-

+    if not util.system_info()["variant"] in KNOWN_DISTROS:

+        return False

-def available_sysconfig(target=None):

     expected = ["ifup", "ifdown"]

     search = ["/sbin", "/usr/sbin"]

     for p in expected:

@@ -1095,10 +1068,4 @@ def available_sysconfig(target=None):

     return False

-def available_nm(target=None):

-    if not os.path.isfile(subp.target_path(target, path=NM_CFG_FILE)):

-        return False

-    return True

-

-

 # vi: ts=4 expandtab

diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py

index 591241b3..ef21ad76 100644

--- a/tests/unittests/test_net.py

+++ b/tests/unittests/test_net.py

@@ -21,6 +21,7 @@ from cloudinit.net import (

     interface_has_own_mac,

     natural_sort_key,

     netplan,

+    network_manager,

     network_state,

     networkd,

     renderers,

@@ -611,6 +612,37 @@ dns = none

                 ),

             ),

         ],

+        "expected_network_manager": [

+            (

+                "".join(

+                    [

+                        "etc/NetworkManager/system-connections",

+                        "/cloud-init-eth0.nmconnection",

+                    ]

+                ),

+                """

+# Generated by cloud-init. Changes will be lost.

+

+[connection]

+id=cloud-init eth0

+uuid=1dd9a779-d327-56e1-8454-c65e2556c12c

+type=ethernet

+

+[user]

+org.freedesktop.NetworkManager.origin=cloud-init

+

+[ethernet]

+mac-address=FA:16:3E:ED:9A:59

+

+[ipv4]

+method=manual

+may-fail=false

+address1=172.19.1.34/22

+route1=0.0.0.0/0,172.19.3.254

+

+""".lstrip(),

+            ),

+        ],

     },

     {

         "in_data": {

@@ -1073,6 +1105,50 @@ NETWORK_CONFIGS = {

                 USERCTL=no"""

             ),

         },

+        "expected_network_manager": {

+            "cloud-init-eth1.nmconnection": textwrap.dedent(

+                """\

+                # Generated by cloud-init. Changes will be lost.

+

+                [connection]

+                id=cloud-init eth1

+                uuid=3c50eb47-7260-5a6d-801d-bd4f587d6b58

+                type=ethernet

+

+                [user]

+                org.freedesktop.NetworkManager.origin=cloud-init

+

+                [ethernet]

+                mac-address=CF:D6:AF:48:E8:80

+

+                """

+            ),