Fix parsing of Linux route non-kv flags (e.g. onlink)
Cherry-pick upstream fix for FACT-1916, fixing parsing of routes that
contain "plain" (i.e. non key-value pair) flags like "onlink".
Closes: #918250
Apollon Oikonomopoulos
4 years ago
| 0 | From: John Bond <jbond@wikimedia.org> | |
| 1 | Date: Thu, 2 May 2019 14:48:44 +0100 | |
| 2 | Subject: (FACT-1916): fix route parsing on Linux | |
| 3 | ||
| 4 | This PR removes the assumption that a every all values in the out put of | |
| 5 | ip route show includes key value pairs. !1283 first introduced this | |
| 6 | assumption fixing. This PR also removes the patch for FACT-1405 introduced | |
| 7 | in !1372 as it is no longer needed. one could further remove the code | |
| 8 | from !1464 to address FACT-1394 as that seems to be cause by the same | |
| 9 | issue, however i have left that in as one dosen't need to parse linkdown | |
| 10 | routes. | |
| 11 | ||
| 12 | Currently the none cases of values which are not key value pairs are | |
| 13 | * flags: linkdown, pervasive, onlink | |
| 14 | * params with optional values: mtu [lock] value | |
| 15 | ||
| 16 | We could add something like the following to the loop at | |
| 17 | https://github.com/puppetlabs/facter/blob/master/lib/src/facts/linux/networking_resolver.cc#L198 | |
| 18 | however this may be overkill as the flags and mtuy alwasy come after the | |
| 19 | dev and src paramaters. | |
| 20 | ||
| 21 | ```c++ | |
| 22 | unordered_set<string> route_flags { | |
| 23 | "linkdown", | |
| 24 | "pervasive", | |
| 25 | "onlink" | |
| 26 | } | |
| 27 | size_t step = 2; | |
| 28 | for (size_t i = dst_idx+1; i < parts.size(); i += step) { | |
| 29 | std::string key(parts[i].begin(), parts[i].end()); | |
| 30 | std::string value(parts[i+1].begin(), parts[i+1].end()); | |
| 31 | step = 2; | |
| 32 | if route_flags.find(key) { | |
| 33 | step = 1; | |
| 34 | continue; | |
| 35 | } else if (key == "mtu" and value == "lock"){ | |
| 36 | step = 3; | |
| 37 | continue; | |
| 38 | } | |
| 39 | if (key == "dev") { | |
| 40 | r.interface.assign(value); | |
| 41 | } | |
| 42 | if (key == "src") { | |
| 43 | r.source.assign(value); | |
| 44 | } | |
| 45 | } | |
| 46 | ``` | |
| 47 | ||
| 48 | I also wonder if its worth always passing `-d` to `ip route show` to | |
| 49 | ensure the route_type is always included but i'm unsure if that flag is | |
| 50 | available on all linux | |
| 51 | ||
| 52 | (cherry picked from commit 6a391557d6a38f0e3c6f8d20ebb2f8d6ba916d18) | |
| 53 | Signed-off-by: Apollon Oikonomopoulos <apoikos@debian.org> | |
| 54 | --- | |
| 55 | lib/src/facts/linux/networking_resolver.cc | 27 +++++++-------------------- | |
| 56 | 1 file changed, 7 insertions(+), 20 deletions(-) | |
| 57 | ||
| 58 | diff --git a/lib/src/facts/linux/networking_resolver.cc b/lib/src/facts/linux/networking_resolver.cc | |
| 59 | index 6f247a3..a3db340 100644 | |
| 60 | --- a/lib/src/facts/linux/networking_resolver.cc | |
| 61 | +++ b/lib/src/facts/linux/networking_resolver.cc | |
| 62 | @@ -150,31 +150,18 @@ namespace facter { namespace facts { namespace linux { | |
| 63 | return true; | |
| 64 | } | |
| 65 | ||
| 66 | - // remove trailing "onlink" or "pervasive" flags | |
| 67 | - while (parts.size() > 0) { | |
| 68 | - std::string last_token(parts.back().begin(), parts.back().end()); | |
| 69 | - if (last_token == "onlink" || last_token == "pervasive") | |
| 70 | - parts.pop_back(); | |
| 71 | - else | |
| 72 | - break; | |
| 73 | - } | |
| 74 | - | |
| 75 | - size_t dst_idx = 0; | |
| 76 | - if (parts.size() % 2 == 0) { | |
| 77 | - std::string route_type(parts[0].begin(), parts[0].end()); | |
| 78 | - if (known_route_types.find(route_type) == known_route_types.end()) { | |
| 79 | - LOG_WARNING("Could not process routing table entry: Expected a destination followed by key/value pairs, got '{1}'", line); | |
| 80 | - return true; | |
| 81 | - } else { | |
| 82 | - dst_idx = 1; | |
| 83 | - } | |
| 84 | + // FACT-1282 | |
| 85 | + std::string route_type(parts[0].begin(), parts[0].end()); | |
| 86 | + if (known_route_types.find(route_type) != known_route_types.end()) { | |
| 87 | + parts.erase(parts.begin()); | |
| 88 | } | |
| 89 | ||
| 90 | route r; | |
| 91 | - r.destination.assign(parts[dst_idx].begin(), parts[dst_idx].end()); | |
| 92 | + r.destination.assign(parts[0].begin(), parts[0].end()); | |
| 93 | + | |
| 94 | // Iterate over key/value pairs and add the ones we care | |
| 95 | // about to our routes entries | |
| 96 | - for (size_t i = dst_idx+1; i < parts.size(); i += 2) { | |
| 97 | + for (size_t i = 1; i < parts.size(); i += 2) { | |
| 98 | std::string key(parts[i].begin(), parts[i].end()); | |
| 99 | if (key == "dev") { | |
| 100 | r.interface.assign(parts[i+1].begin(), parts[i+1].end()); |