From 46a72db3059eef02f2f18e7328f2919593bda22e Mon Sep 17 00:00:00 2001 From: Huanhuan Li Date: Thu, 21 Jul 2022 15:03:52 +0800 Subject: [PATCH 1/5] fix: the parser "LvmConfig" raises exception * It fails to parse the content when there is "umask=077" in the global configuration. Signed-off-by: Huanhuan Li --- insights/parsers/lvm.py | 2 ++ insights/tests/parsers/lvm_test_data.py | 12 ++++++++++++ insights/tests/parsers/test_lvm.py | 7 ++++++- 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/insights/parsers/lvm.py b/insights/parsers/lvm.py index 0e82579085..329032c690 100644 --- a/insights/parsers/lvm.py +++ b/insights/parsers/lvm.py @@ -699,6 +699,8 @@ def _lvm_render(o): if isinstance(o, dict): parts = ['"%s": %s' % (k, _lvm_render(v)) for k, v in o.items()] return "{%s}" % ",".join(parts) + if isinstance(o, str) and o.isdigit(): + return '"%s"' % o return "%s" % o diff --git a/insights/tests/parsers/lvm_test_data.py b/insights/tests/parsers/lvm_test_data.py index cd25a7e579..b359910156 100644 --- a/insights/tests/parsers/lvm_test_data.py +++ b/insights/tests/parsers/lvm_test_data.py @@ -223,3 +223,15 @@ system_id="" host_id=0 }""".strip() + +LVMCONFIG2 = """ +global { + umask=077 + test=0 + units="r" + si_unit_consistency=1 + suffix=1 + activation=1 + fallback_to_lvm1=0 +} +""".strip() diff --git a/insights/tests/parsers/test_lvm.py b/insights/tests/parsers/test_lvm.py index ffb1ac1059..7642d91b78 100644 --- a/insights/tests/parsers/test_lvm.py +++ b/insights/tests/parsers/test_lvm.py @@ -5,7 +5,7 @@ from insights.parsers import SkipException from insights.parsers import lvm from insights.tests import context_wrap -from .lvm_test_data import LVMCONFIG +from .lvm_test_data import LVMCONFIG, LVMCONFIG2 WARNINGS_CONTENT = """ WARNING @@ -189,6 +189,11 @@ def test_lvmconfig(): assert p.data["global"]["thin_check_options"] == ["-q", "--clear-needs-check-flag"] +def test_lvmconfig_2(): + p = lvm.LvmConfig(context_wrap(LVMCONFIG2)) + assert p.data['global']['umask'] == '077' + + def test_vgsheading_warnings(): result = lvm.VgsHeadings(context_wrap(VGSHEADING_CONTENT)) assert len(result.warnings) == 6 From f6ec0e41f57ebb213898cac90895e5e50cc516bd Mon Sep 17 00:00:00 2001 From: Huanhuan Li Date: Fri, 22 Jul 2022 13:40:52 +0800 Subject: [PATCH 2/5] Only fix "umask=077" and keep the other strings as integer Signed-off-by: Huanhuan Li --- insights/parsers/lvm.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/insights/parsers/lvm.py b/insights/parsers/lvm.py index 329032c690..0d68d664ca 100644 --- a/insights/parsers/lvm.py +++ b/insights/parsers/lvm.py @@ -699,7 +699,8 @@ def _lvm_render(o): if isinstance(o, dict): parts = ['"%s": %s' % (k, _lvm_render(v)) for k, v in o.items()] return "{%s}" % ",".join(parts) - if isinstance(o, str) and o.isdigit(): + # for umask=077, it can not be parsed by json directly, transfer it to string first + if isinstance(o, str) and o.isdigit() and o.startswith('0') and len(o) > 1: return '"%s"' % o return "%s" % o From fe7ff0e8247584926554a6bdb0da81f25782d1cc Mon Sep 17 00:00:00 2001 From: Huanhuan Li Date: Tue, 26 Jul 2022 16:56:58 +0800 Subject: [PATCH 3/5] Transfer the value first before storing Signed-off-by: Huanhuan Li --- insights/parsers/lvm.py | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/insights/parsers/lvm.py b/insights/parsers/lvm.py index 0d68d664ca..c738fbe07e 100644 --- a/insights/parsers/lvm.py +++ b/insights/parsers/lvm.py @@ -695,20 +695,10 @@ def parse_content(self, content): self.data = lvm_conf_dict -def _lvm_render(o): - if isinstance(o, dict): - parts = ['"%s": %s' % (k, _lvm_render(v)) for k, v in o.items()] - return "{%s}" % ",".join(parts) - # for umask=077, it can not be parsed by json directly, transfer it to string first - if isinstance(o, str) and o.isdigit() and o.startswith('0') and len(o) > 1: - return '"%s"' % o - return "%s" % o - - @parser(Specs.lvmconfig) class LvmConfig(CommandParser): def parse_content(self, content): - dd = defaultdict(dict) + self.data = defaultdict(dict) key = None for line in content: line = line.rstrip() @@ -721,10 +711,16 @@ def parse_content(self, content): key = None elif line[0] == "\t": k, v = line.strip().split("=", 1) - dd[key][k] = v + # umask=077 will raise exception, and also no need to + # transfer it, just keep it as string + if k != 'umask': + try: + v = json.loads(v) + except Exception: + raise SkipException("Failed to parse line %s." % line) + self.data[key][k] = v else: pass # inferring this a stderr, so skipping - self.data = json.loads(_lvm_render(dict(dd))) @parser(Specs.lvm_system_devices) From 239b174a4d67c00a1796d16fe5e10646a32a389a Mon Sep 17 00:00:00 2001 From: Huanhuan Li Date: Thu, 28 Jul 2022 10:54:32 +0800 Subject: [PATCH 4/5] Keep data as dict Signed-off-by: Huanhuan Li --- insights/parsers/lvm.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/insights/parsers/lvm.py b/insights/parsers/lvm.py index c738fbe07e..5ef60fd84b 100644 --- a/insights/parsers/lvm.py +++ b/insights/parsers/lvm.py @@ -36,7 +36,6 @@ from __future__ import print_function import json -from collections import defaultdict from insights.parsers import ParseException, optlist_to_dict, SkipException from insights.specs import Specs @@ -698,7 +697,7 @@ def parse_content(self, content): @parser(Specs.lvmconfig) class LvmConfig(CommandParser): def parse_content(self, content): - self.data = defaultdict(dict) + self.data = dict() key = None for line in content: line = line.rstrip() @@ -717,8 +716,8 @@ def parse_content(self, content): try: v = json.loads(v) except Exception: - raise SkipException("Failed to parse line %s." % line) - self.data[key][k] = v + raise ParseException("Failed to parse line %s." % line) + self.data.setdefault(key, {}).update({k: v}) else: pass # inferring this a stderr, so skipping From 160cda3f5009d7f970b14677125250011b21dcf3 Mon Sep 17 00:00:00 2001 From: Huanhuan Li Date: Thu, 28 Jul 2022 11:12:44 +0800 Subject: [PATCH 5/5] Add test for ParseException Signed-off-by: Huanhuan Li --- insights/tests/parsers/lvm_test_data.py | 12 ++++++++++++ insights/tests/parsers/test_lvm.py | 9 +++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/insights/tests/parsers/lvm_test_data.py b/insights/tests/parsers/lvm_test_data.py index b359910156..ccfd730823 100644 --- a/insights/tests/parsers/lvm_test_data.py +++ b/insights/tests/parsers/lvm_test_data.py @@ -235,3 +235,15 @@ fallback_to_lvm1=0 } """.strip() + +LVMCONFIG3 = """ +global { + test=077 + test=0 + units="r" + si_unit_consistency=1 + suffix=1 + activation=1 + fallback_to_lvm1=0 +} +""".strip() diff --git a/insights/tests/parsers/test_lvm.py b/insights/tests/parsers/test_lvm.py index 7642d91b78..337fb97fcc 100644 --- a/insights/tests/parsers/test_lvm.py +++ b/insights/tests/parsers/test_lvm.py @@ -2,10 +2,10 @@ import pytest import doctest -from insights.parsers import SkipException +from insights.parsers import ParseException, SkipException from insights.parsers import lvm from insights.tests import context_wrap -from .lvm_test_data import LVMCONFIG, LVMCONFIG2 +from .lvm_test_data import LVMCONFIG, LVMCONFIG2, LVMCONFIG3 WARNINGS_CONTENT = """ WARNING @@ -194,6 +194,11 @@ def test_lvmconfig_2(): assert p.data['global']['umask'] == '077' +def test_lvmconfig_exception(): + with pytest.raises(ParseException): + lvm.LvmConfig(context_wrap(LVMCONFIG3)) + + def test_vgsheading_warnings(): result = lvm.VgsHeadings(context_wrap(VGSHEADING_CONTENT)) assert len(result.warnings) == 6