refactor: rewrite is_valid_port() (#7360)

* refactor: speed up is_valid_port()
Convert is_valid_port to Config
Several Config optimizations
Update documentation

* fix getCombined() when os key is not set, but global is.
Add more tests

LibreNMS/Config.php

@@ -37,6 +37,15 @@ class Config
     public static function get($key, $default = null)
     {
         global $config;
+
+        if (isset($config[$key])) {
+            return $config[$key];
+        }
+
+        if (!str_contains($key, '.')) {
+            return $default;
+        }
+
         $keys = explode('.', $key);
 
         $curr = &$config;
@@ -90,9 +99,18 @@ class Config
      */
     public static function getOsSetting($os, $key, $default = null)
     {
-        if ($os) {
+        global $config;
-            $os_key = "os.$os.$key";
 
+        if ($os) {
+            if (isset($config['os'][$os][$key])) {
+                return $config['os'][$os][$key];
+            }
+
+            if (!str_contains($key, '.')) {
+                return self::get($key, $default);
+            }
+
+            $os_key = "os.$os.$key";
             if (self::has($os_key)) {
                 return self::get($os_key);
             }
@@ -113,6 +131,21 @@ class Config
      */
     public static function getCombined($os, $key, $default = array())
     {
+        global $config;
+
+        if (!self::has($key)) {
+            return self::get("os.$os.$key", $default);
+        }
+
+        if (!isset($config['os'][$os][$key])) {
+            if (!str_contains($key, '.')) {
+                return self::get($key, $default);
+            }
+            if (!self::has("os.$os.$key")) {
+                return self::get($key, $default);
+            }
+        }
+
         return array_unique(array_merge(
             (array)self::get($key, $default),
             (array)self::getOsSetting($os, $key, $default)
@@ -147,6 +180,15 @@ class Config
     public static function has($key)
     {
         global $config;
+
+        if (isset($config[$key])) {
+            return true;
+        }
+
+        if (!str_contains($key, '.')) {
+            return false;
+        }
+
         $keys = explode('.', $key);
         $last = array_pop($keys);
 

doc/Developing/os/Settings.md

@@ -11,6 +11,28 @@ For example, to set an alternate icon for ios:
 $config['os']['ios']['icon'] = 'fuzzybunny';
 ```
 
+### Ignoring Interfaces
+See also: [Global Ignoring Interfaces Config](../../Support/Configuration.md#interfaces-to-be-ignored)
+
+> These settings are merged with the global settings, so you can only undo global ones with good_if
+
+```yaml
+empty_ifdescr: false # allow empty ifDescr
+bad_if: # ifDescr (substring, case insensitive)
+    - lp0
+bad_if_regexp: # ifDescr (regex, case insensitive)
+    - "/^ng[0-9]+$/"
+bad_ifname_regexp: # ifName (regex, case insensitive)
+    - "/^xdsl_channel /"
+bad_ifalias_regexp: # ifAlias (regex, case insensitive)
+    - "/^vlan/"
+bad_iftype: # ifType (substring)
+    - sonet
+good_if: # ignore all other bad_if settings ifDescr (substring, case insensitive)
+    - virtual
+
+```
+
 
 ### Storage Settings
 See also: [Global Storage Config](../../Support/Configuration.md#storage-configuration)

includes/definitions/catos.yaml

@@ -32,3 +32,5 @@ discovery_modules:
 discovery:
     - sysDescr:
         - Cisco Catalyst Operating System Software
+bad_if:
+    - vlan

includes/definitions/dlink.yaml

@@ -10,3 +10,6 @@ discovery:
     - sysDescr_regex:
         - '/^DES-/'
         - '/^DGS-/'
+good_if:
+    - po
+    - vlan

includes/functions.php

@@ -11,6 +11,7 @@
  *
  */
 
+use LibreNMS\Config;
 use LibreNMS\Exceptions\HostExistsException;
 use LibreNMS\Exceptions\HostIpExistsException;
 use LibreNMS\Exceptions\HostUnreachableException;
@@ -979,95 +980,75 @@ function include_dir($dir, $regex = "")
     }
 }
 
+/**
+ * Check if port is valid to poll.
+ * Settings: empty_ifdescr, good_if, bad_if, bad_if_regexp, bad_ifname_regexp, bad_ifalias_regexp, bad_iftype
+ *
+ * @param array $port
+ * @param array $device
+ * @return bool
+ */
 function is_port_valid($port, $device)
 {
-
+    // check empty values first
-    global $config;
+    if (empty($port['ifDescr'])) {
-
-    if (empty($port['ifDescr']) && empty($port['ifAlias']) && empty($port['ifName'])) {
         // If these are all empty, we are just going to show blank names in the ui
-        $valid = 0;
+        if (empty($port['ifAlias']) && empty($port['ifName'])) {
-    } else {
+            return false;
-        $valid = 1;
-        $if = strtolower($port['ifDescr']);
-        $ifname = strtolower($port['ifName']);
-        $ifalias = strtolower($port['ifAlias']);
-        $fringe = $config['bad_if'];
-        if (is_array($config['os'][$device['os']]['bad_if'])) {
-            $fringe = array_merge($config['bad_if'], $config['os'][$device['os']]['bad_if']);
         }
-        $config['good_if'] = $config['good_if'] ?: array();
+
-        if (is_array($config['os'][$device['os']]['good_if'])) {
+        // ifDescr should not be empty unless it is explicitly allowed
-            $good_if = array_merge($config['good_if'], $config['os'][$device['os']]['good_if']);
+        if (!Config::getOsSetting($device['os'], 'empty_ifdescr', false)) {
-        }
+            return false;
-        foreach ($fringe as $bi) {
-            if (stristr($if, $bi)) {
-                if (!str_contains($good_if, $if)) {
-                    $valid = 0;
-                    d_echo("ignored : $bi : $if");
-                }
-            }
-        }
-        if (is_array($config['bad_if_regexp'])) {
-            $fringe = $config['bad_if_regexp'];
-            if (is_array($config['os'][$device['os']]['bad_if_regexp'])) {
-                $fringe = array_merge($config['bad_if_regexp'], $config['os'][$device['os']]['bad_if_regexp']);
-            }
-            foreach ($fringe as $bi) {
-                if (preg_match($bi ."i", $if)) {
-                    $valid = 0;
-                    d_echo("ignored : $bi : " . $if);
-                }
-            }
-        }
-        if (is_array($config['bad_ifname_regexp'])) {
-            $fringe = $config['bad_ifname_regexp'];
-            if (is_array($config['os'][$device['os']]['bad_ifname_regexp'])) {
-                $fringe = array_merge($config['bad_ifname_regexp'], $config['os'][$device['os']]['bad_ifname_regexp']);
-            }
-            foreach ($fringe as $bi) {
-                if (preg_match($bi ."i", $ifname)) {
-                    $valid = 0;
-                    d_echo("ignored : $bi : ".$ifname);
-                }
-            }
-        }
-        if (is_array($config['bad_ifalias_regexp'])) {
-            $fringe = $config['bad_ifalias_regexp'];
-            if (is_array($config['os'][$device['os']]['bad_ifalias_regexp'])) {
-                $fringe = array_merge($config['bad_ifalias_regexp'], $config['os'][$device['os']]['bad_ifalias_regexp']);
-            }
-            foreach ($fringe as $bi) {
-                if (preg_match($bi ."i", $ifalias)) {
-                    $valid = 0;
-                    d_echo("ignored : $bi : ".$ifalias);
-                }
-            }
-        }
-        if (is_array($config['bad_iftype'])) {
-            $fringe = $config['bad_iftype'];
-            if (is_array($config['os'][$device['os']]['bad_iftype'])) {
-                $fringe = array_merge($config['bad_iftype'], $config['os'][$device['os']]['bad_iftype']);
-            }
-            foreach ($fringe as $bi) {
-                if (stristr($port['ifType'], $bi)) {
-                    $valid = 0;
-                    d_echo("ignored ifType : ".$port['ifType']." (matched: ".$bi." )");
-                }
-            }
-        }
-        if (empty($port['ifDescr']) && !$config['os'][$device['os']]['empty_ifdescr']) {
-            $valid = 0;
-        }
-        if ($device['os'] == "catos" && strstr($if, "vlan")) {
-            $valid = 0;
-        }
-        if ($device['os'] == "dlink") {
-            $valid = 1;
         }
     }
 
-    return $valid;
+    $ifDescr = $port['ifDescr'];
+    $ifName  = $port['ifName'];
+    $ifAlias = $port['ifAlias'];
+    $ifType  = $port['ifType'];
+
+    if (str_contains($ifDescr, Config::getOsSetting($device['os'], 'good_if'), true)) {
+        return true;
+    }
+
+    foreach (Config::getCombined($device['os'], 'bad_if') as $bi) {
+        if (str_contains($ifDescr, $bi, true)) {
+            d_echo("ignored by ifDescr: $ifDescr (matched: $bi)\n");
+            return false;
+        }
+    }
+
+    foreach (Config::getCombined($device['os'], 'bad_if_regexp') as $bir) {
+        if (preg_match($bir ."i", $ifDescr)) {
+            d_echo("ignored by ifDescr: $ifDescr (matched: $bir)\n");
+            return false;
+        }
+    }
+
+    foreach (Config::getCombined($device['os'], 'bad_ifname_regexp') as $bnr) {
+        if (preg_match($bnr ."i", $ifName)) {
+            d_echo("ignored by ifName: $ifName (matched: $bnr)\n");
+            return false;
+        }
+    }
+
+
+    foreach (Config::getCombined($device['os'], 'bad_ifalias_regexp') as $bar) {
+        if (preg_match($bar ."i", $ifAlias)) {
+            d_echo("ignored by ifName: $ifAlias (matched: $bar)\n");
+            return false;
+        }
+    }
+
+    foreach (Config::getCombined($device['os'], 'bad_iftype') as $bt) {
+        if (str_contains($ifType, $bt)) {
+            d_echo("ignored by ifType: $ifType (matched: $bt )\n");
+            return false;
+        }
+    }
+
+    return true;
 }
 
 function scan_new_plugins()

tests/ConfigTest.php

@@ -97,6 +97,12 @@ class ConfigTest extends \PHPUnit_Framework_TestCase
         $config['os']['nullos']['num'] = array('two', 'three');
         $config['assoc'] = array('a' => 'same', 'b' => 'same');
         $config['os']['nullos']['assoc'] = array('b' => 'different', 'c' => 'still same');
+        $config['os']['nullos']['osset'] = true;
+        $config['gset'] = true;
+
+        $this->assertTrue(Config::getCombined('nullos', 'non-existent', true), 'Did not return default value on non-existent key');
+        $this->assertTrue(Config::getCombined('nullos', 'osset', false), 'Did not return OS value when global value is not set');
+        $this->assertTrue(Config::getCombined('nullos', 'gset', false), 'Did not return global value when OS value is not set');
 
         $combined =  Config::getCombined('nullos', 'num');
         sort($combined);