Merge charm-advanced-routing:bug/1900866 into charm-advanced-routing:master

[Adam Dyess]

implement fwmark and iif policy rules

[Paul Goins (vultaire)]

Code looks good. Locally tested on an LXD ubuntu, works as advertised. +1.

review: Approve

[Peter Sabaini (peter-sabaini)]

Hey, thanks for the addition!

I have left some inline comments/remarks.

Otherwise lgtm.

review: Needs Fixing

[Peter Sabaini (peter-sabaini)]

Cheers, lgtm!

review: Approve

[Drew Freiberger (afreiberger)]

LGTM

review: Approve

diff --git a/src/lib/routing_entry.py b/src/lib/routing_entry.py
index d45170d..6a61e88 100644
--- a/src/lib/routing_entry.py
+++ b/src/lib/routing_entry.py
@@ -9,9 +9,11 @@ concrete implementations, that model a routing table
      RoutingEntryTable  RoutingEntryRoute  RoutingEntryRule
 """
 import collections
+import re
 import subprocess
 from abc import ABCMeta, abstractmethod, abstractproperty
 
+
 from charmhelpers.core import hookenv
 
 
@@ -233,6 +235,30 @@ class RoutingEntryRoute(RoutingEntryType):
 class RoutingEntryRule(RoutingEntryType):
     """RoutingEntryType used for rules."""
 
+    MARK_PATTERN_TXT = (
+        # 13 digit int | 8 digit hex value [optional 8 digit hex mask]
+        r"^(\d{1,13}|0x[0-9a-f]{1,8})(?:\/(0x[0-9a-f]{1,8}))?$"
+    )
+    MARK_PATTERN = re.compile(MARK_PATTERN_TXT, re.IGNORECASE)
+
+    @staticmethod
+    def fwmark_user(fwmark):
+        """
+        Convert user fwmark to match the output from ip rules list.
+        @param str fwmark: marking from the user config
+        @returns str in hex/hex format
+        """
+        match = RoutingEntryRule.MARK_PATTERN.search(fwmark)
+        if not match:
+            return None
+        hex_vals = match.groups()
+        as_ints = [
+            int(val, 16 if val.lower().startswith("0x") else 10)
+            for val in hex_vals
+            if val
+        ]
+        return "/".join(map(hex, as_ints))
+
     def __init__(self, config):
         """Object init function."""
         hookenv.log("Created {}".format(self.__class__.__name__), level=hookenv.INFO)
@@ -259,12 +285,18 @@ class RoutingEntryRule(RoutingEntryType):
         ip rule add from X.X.X.X/XX table mytable priority NNN
         # any dst, table main
         ip rule add from X.X.X.X/XX priority NNN
+        # any src, fwmark 0x1/0xF, iif bond0, table mytable
+        ip rule add from any fwmark 1/0xF iif bond0 table mytable priority NNN
         """
         cmd = ["ip", "rule", "add", "from", self.config["from-net"]]
-        opts = collections.OrderedDict(
-            {"to-net": "to", "table": "table", "priority": "priority"}
-        )
-        for opt, keyword in opts.items():
+        opts = [
+            ("fwmark", "fwmark"),
+            ("iif", "iif"),
+            ("to-net", "to"),
+            ("table", "table"),
+            ("priority", "priority"),
+        ]
+        for opt, keyword in opts:
             try:
                 cmd.extend([keyword, str(self.config[opt])])
             except KeyError:
@@ -291,9 +323,19 @@ class RoutingEntryRule(RoutingEntryType):
         """Ip rule add does not prevent duplicates in older kernel versions."""
         # https://patchwork.ozlabs.org/patch/624553/
         matchparams = ["from", self.config["from-net"]]
+
         to = self.config.get("to-net")
         if to and to != "all":  # ip rule omits to=all as it's implied
             matchparams.extend(("to", to))
+
+        fwmark = self.config.get("fwmark")
+        if fwmark:
+            matchparams.extend(("fwmark", fwmark))
+
+        iif = self.config.get("iif")
+        if iif:
+            matchparams.extend(("iif", iif))
+
         matchparams.extend(("lookup", self.config.get("table", "main")))
         matchline = " ".join(matchparams)
         prio = str(self.config.get("priority", ""))
diff --git a/src/lib/routing_validator.py b/src/lib/routing_validator.py
index 04d1ad6..bc3984c 100644
--- a/src/lib/routing_validator.py
+++ b/src/lib/routing_validator.py
@@ -18,8 +18,8 @@ from routing_entry import (
     RoutingEntryType,
 )
 
-TABLE_NAME_PATTERN = "[a-zA-Z0-9]+[a-zA-Z0-9-]*"
-TABLE_NAME_PATTERN_RE = "^{}$".format(TABLE_NAME_PATTERN)
+TABLE_NAME_PATTERN = r"[a-zA-Z0-9]+[a-zA-Z0-9-]*"
+TABLE_NAME_PATTERN_RE = r"^{}$".format(TABLE_NAME_PATTERN)
 
 
 class RoutingConfigValidatorError(Exception):
@@ -240,6 +240,8 @@ class RoutingConfigValidator:
         )
 
         # Verify items in configuration
+        self.verify_rule_mark(conf)
+        self.verify_rule_iif(conf)
         self.verify_rule_from_net(conf)
         self.verify_rule_to_net(conf)
         self.verify_rule_table(conf)
@@ -247,6 +249,40 @@ class RoutingConfigValidator:
 
         RoutingEntryType.add_entry(RoutingEntryRule(conf))
 
+    def verify_rule_mark(self, conf):
+        """
+        Verify fwmark config.
+
+        "fwmark" key isn't required.
+        if fwmark is correct and from-net is unset -- set from-net=all
+        """
+        fwmark = conf.get("fwmark")
+        if not fwmark:
+            return None
+
+        fwmark_hex = RoutingEntryRule.fwmark_user(fwmark)
+        from_net = conf.get("from-net")
+        if not fwmark_hex:
+            msg = "fwmark {} is in the wrong format".format(fwmark)
+            self.report_error(msg)
+        else:
+            # now that its valid, update the config with hex version
+            conf["fwmark"] = fwmark_hex
+
+        if fwmark_hex and not from_net:
+            conf["from-net"] = "all"
+
+    def verify_rule_iif(self, conf):
+        """
+        Verify rule input interface.
+
+        "iif" key isn't required, but verify the network device exists
+        """
+        iif = conf.get("iif")
+        if iif and iif not in netifaces.interfaces():
+            msg = "Device {} does not exist".format(iif)
+            self.report_error(msg)
+
     def verify_rule_from_net(self, conf):
         """Verify rule source network.
 
diff --git a/src/tests/unit/test_RoutingConfigValidator.py b/src/tests/unit/test_RoutingConfigValidator.py
new file mode 100644
index 0000000..fc7504f
--- /dev/null
+++ b/src/tests/unit/test_RoutingConfigValidator.py
@@ -0,0 +1,32 @@
+"""routing validator unit testing module."""
+
+import pytest
+
+import routing_validator
+
+
[e-mail](
+    "fwmark",
+    [
+        "abc",  # not valid hex -- 0xabc is right
+        "1 0x0f",  # space isn't a valid delimiter
+        "99999999999999",  # too many digits
+        "0x1000000000",  # too many hex digits
+        "2/0x1000000000",  # too many hex digits
+        "1/1",  # mask must be hex
+    ],
+    ids=[
+        "not valid hex -- 0xabc is right",
+        "space isn't a valid delimiter",
+        "too many digits",
+        "too many hex digits",
+        "too many hex digits in mask",
+        "mask must be hex",
+    ],
+)
+def test_routing_validate_rule_fwmark_failure(fwmark):
+    """Test different versions of user input on fwmark."""
+    validator = routing_validator.RoutingConfigValidator()
+    with pytest.raises(routing_validator.RoutingConfigValidatorError) as ie:
+        validator.verify_rule({"fwmark": fwmark})
+    ie.match("fwmark {} is in the wrong format".format(fwmark))
diff --git a/src/tests/unit/test_RoutingEntryRule.py b/src/tests/unit/test_RoutingEntryRule.py
index fe8724a..c9e8f67 100644
--- a/src/tests/unit/test_RoutingEntryRule.py
+++ b/src/tests/unit/test_RoutingEntryRule.py
@@ -33,6 +33,24 @@ import routing_entry
             True,
             id="From-ToAll-Table-Prio-Found=True",
         ),
+        pytest.param(
+            {
+                "from-net": "all",
+                "fwmark": "0x10/0xff",
+                "iif": "lo",
+                "to-net": "10.0.0.0/24",
+                "table": "SF1",
+                "priority": 100,
+            },
+            (
+                b"0:\tfrom all lookup local\n"
+                b"100:\tfrom all to 10.0.0.0/24 fwmark 0x10/0xff iif lo lookup SF1\n"
+                b"32766:\tfrom all lookup main\n"
+                b"32767:\tfrom all lookup default\n"
+            ),
+            True,
+            id="FromAll-ToNet-Fwmark-Iif-Table-Prio-Found=True",
+        ),
     ],
 )
 def test_routing_entry_rule_is_duplicate(