[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#932335: marked as done (buster-pu: package facter/3.11.0-2+deb10u1)



Your message dated Sat, 07 Sep 2019 14:34:49 +0100
with message-id <[🔎] f49e2985d8466065c49c03185c24465a32228fb5.camel@adam-barratt.org.uk>
and subject line Closing bugs for fixes including in 10.1 point release
has caused the Debian Bug report #932335,
regarding buster-pu: package facter/3.11.0-2+deb10u1
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
932335: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=932335
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
Package: release.debian.org
Severity: normal
Tags: buster
User: release.debian.org@packages.debian.org
Usertags: pu

Dear SRMs,

I would like to update facter in Buster to fix #918250, whereby facter 
fails to parse routes with the `onlink' flag, producing a lot of noise 
in the process and possibly acquiring a distorted view of the network.  
ifupdown adds the `onlink' flag to gateway routes by default, so this 
tends to affect a lot of systems.

The issue has been fixed upstream, and backporting the fix is trivial — 
in fact I have already uploaded a fixed version in unstable. Full source 
debdiff against buster attached.

Regards,
Apollon
diff -Nru facter-3.11.0/debian/changelog facter-3.11.0/debian/changelog
--- facter-3.11.0/debian/changelog	2019-04-02 08:24:17.000000000 -0300
+++ facter-3.11.0/debian/changelog	2019-07-17 17:04:05.000000000 -0300
@@ -1,3 +1,9 @@
+facter (3.11.0-2+deb10u1) buster; urgency=medium
+
+  * Fix parsing of Linux route non-kv flags (e.g. onlink) (Closes: #918250)
+
+ -- Apollon Oikonomopoulos <apoikos@debian.org>  Wed, 17 Jul 2019 17:04:05 -0300
+
 facter (3.11.0-2) unstable; urgency=medium
 
   * Acknowledge 3.11.0-1.1 NMU. Thanks to gregor herrmann!
diff -Nru facter-3.11.0/debian/patches/0006-FACT-1916-fix-route-parsing-on-Linux.patch facter-3.11.0/debian/patches/0006-FACT-1916-fix-route-parsing-on-Linux.patch
--- facter-3.11.0/debian/patches/0006-FACT-1916-fix-route-parsing-on-Linux.patch	1969-12-31 21:00:00.000000000 -0300
+++ facter-3.11.0/debian/patches/0006-FACT-1916-fix-route-parsing-on-Linux.patch	2019-07-17 17:03:15.000000000 -0300
@@ -0,0 +1,101 @@
+From: John Bond <jbond@wikimedia.org>
+Date: Thu, 2 May 2019 14:48:44 +0100
+Subject: (FACT-1916): fix route parsing on Linux
+
+This PR removes the assumption that a every all values in the out put of
+ip route show includes key value pairs.  !1283 first introduced this
+assumption fixing.  This PR also removes the patch for FACT-1405 introduced
+in !1372 as it is no longer needed.  one could further remove the code
+from !1464 to address FACT-1394 as that seems to be cause by the same
+issue, however i have left that in as one dosen't need to parse linkdown
+routes.
+
+Currently the none cases of values which are not key value pairs are
+ * flags: linkdown, pervasive, onlink
+ * params with optional values:  mtu [lock] value
+
+ We could add something like the following to the loop at
+https://github.com/puppetlabs/facter/blob/master/lib/src/facts/linux/networking_resolver.cc#L198
+however this may be overkill as the flags and mtuy alwasy come after the
+dev and src paramaters.
+
+```c++
+unordered_set<string> route_flags {
+  "linkdown",
+  "pervasive",
+  "onlink"
+}
+size_t step = 2;
+for (size_t i = dst_idx+1; i < parts.size(); i += step) {
+    std::string key(parts[i].begin(), parts[i].end());
+    std::string value(parts[i+1].begin(), parts[i+1].end());
+    step = 2;
+    if route_flags.find(key) {
+      step = 1;
+      continue;
+    } else if (key == "mtu" and value == "lock"){
+      step = 3;
+      continue;
+    }
+    if (key == "dev") {
+        r.interface.assign(value);
+    }
+    if (key == "src") {
+        r.source.assign(value);
+    }
+}
+```
+
+I also wonder if its worth always passing `-d` to `ip route show` to
+ensure the route_type is always included but i'm unsure if that flag is
+available on all linux
+
+(cherry picked from commit 6a391557d6a38f0e3c6f8d20ebb2f8d6ba916d18)
+Signed-off-by: Apollon Oikonomopoulos <apoikos@debian.org>
+---
+ lib/src/facts/linux/networking_resolver.cc | 27 +++++++--------------------
+ 1 file changed, 7 insertions(+), 20 deletions(-)
+
+diff --git a/lib/src/facts/linux/networking_resolver.cc b/lib/src/facts/linux/networking_resolver.cc
+index 6f247a3..a3db340 100644
+--- a/lib/src/facts/linux/networking_resolver.cc
++++ b/lib/src/facts/linux/networking_resolver.cc
+@@ -150,31 +150,18 @@ namespace facter { namespace facts { namespace linux {
+                 return true;
+             }
+ 
+-            // remove trailing "onlink" or "pervasive" flags
+-            while (parts.size() > 0) {
+-                std::string last_token(parts.back().begin(), parts.back().end());
+-                if (last_token == "onlink" || last_token == "pervasive")
+-                    parts.pop_back();
+-                else
+-                    break;
+-            }
+-
+-            size_t dst_idx = 0;
+-            if (parts.size() % 2 == 0) {
+-                std::string route_type(parts[0].begin(), parts[0].end());
+-                if (known_route_types.find(route_type) == known_route_types.end()) {
+-                    LOG_WARNING("Could not process routing table entry: Expected a destination followed by key/value pairs, got '{1}'", line);
+-                    return true;
+-                } else {
+-                    dst_idx = 1;
+-                }
++            // FACT-1282
++            std::string route_type(parts[0].begin(), parts[0].end());
++            if (known_route_types.find(route_type) != known_route_types.end()) {
++                parts.erase(parts.begin());
+             }
+ 
+             route r;
+-            r.destination.assign(parts[dst_idx].begin(), parts[dst_idx].end());
++            r.destination.assign(parts[0].begin(), parts[0].end());
++
+             // Iterate over key/value pairs and add the ones we care
+             // about to our routes entries
+-            for (size_t i = dst_idx+1; i < parts.size(); i += 2) {
++            for (size_t i = 1; i < parts.size(); i += 2) {
+                 std::string key(parts[i].begin(), parts[i].end());
+                 if (key == "dev") {
+                     r.interface.assign(parts[i+1].begin(), parts[i+1].end());
diff -Nru facter-3.11.0/debian/patches/series facter-3.11.0/debian/patches/series
--- facter-3.11.0/debian/patches/series	2019-04-02 08:21:58.000000000 -0300
+++ facter-3.11.0/debian/patches/series	2019-07-17 17:03:31.000000000 -0300
@@ -3,3 +3,4 @@
 disable-facter-smoke.patch
 rapidjson-1.1-compat.patch
 fix-custom-facts-overriding-core.patch
+0006-FACT-1916-fix-route-parsing-on-Linux.patch

--- End Message ---
--- Begin Message ---
Version: 10.1

Hi,

The fixes referenced by each of these bugs were included in today's
buster point release.

Regards,

Adam

--- End Message ---

Reply to: