New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

JSON output [5] #105

Merged

evgenyz merged 28 commits into OpenSCAP:master from Honny1:json-output-6

Oct 3, 2022

Member

Honny1 commented Aug 16, 2022 •

edited

This PR creates the ability to use templates for JSON output. And creates JSON-EVERYTHING format which is JSON contains all information that openscap-report can offer.

Depends on PR: #103

Honny1 added the dependency on another PR label

Honny1 force-pushed the json-output-6 branch from 0f798bc to f6fc355 Compare

August 18, 2022 13:09

Honny1 marked this pull request as ready for review

August 19, 2022 09:31

Honny1 marked this pull request as draft

August 21, 2022 21:58

Honny1 force-pushed the json-output-6 branch from f6fc355 to 446c6ad Compare

August 22, 2022 12:39

Honny1 marked this pull request as ready for review

August 22, 2022 12:59

Honny1 force-pushed the json-output-6 branch 3 times, most recently from d80acdd to 07dc415 Compare

August 24, 2022 09:36

Honny1 force-pushed the json-output-6 branch 3 times, most recently from 9b0c5d5 to 8b4b4bc Compare

September 2, 2022 14:21

Honny1 added 8 commits

September 9, 2022 13:48


          Add format JSON to cli

75001af


          Create report_generators sub package

0b6b97e


          Create a JSON generator shell

aef33c2


          Use report_generators sub package

74d49dc


          Create tests

eb5c4b4


          Ignore generated JSON reports

d43b967


          Generate json output from report structure

d7bfddc


          Use buildin function asdict

7ab6c21

Honny1 force-pushed the json-output-6 branch from 8b4b4bc to 486fe0a Compare

September 9, 2022 12:03

evgenyz reviewed

View reviewed changes

openscap_report/scap_results_parser/parsers/__init__.py Outdated

@@ @@ -2,7 +2,9 @@ @@
               # SPDX-License-Identifier: LGPL-2.1-or-later
               from .full_text_parser import FullTextParser
+              from .groupe_parser import GroupParser

Contributor

evgenyz Sep 9, 2022

Suggested change

      
            from .groupe_parser import GroupParser
          
            from .group_parser import GroupParser

openscap_report/scap_results_parser/parsers/__init__.py Outdated

@@ @@ -2,7 +2,9 @@ @@
               # SPDX-License-Identifier: LGPL-2.1-or-later
               from .full_text_parser import FullTextParser
+              from .groupe_parser import GroupParser
               from .info_of_test_parser import InfoOfTest

Contributor

evgenyz Sep 9, 2022

I'd recommend

Suggested change

      
            from .info_of_test_parser import InfoOfTest
          
            from .test_info_parser import InfoOfTest

openscap_report/report_generators/json.py Outdated

+                  def generate_report(self, debug_setting):
+                      logging.warning("JSON Format is experimental output!")
+                      template = self.env.get_template("base.json")
+                      json_data = json.loads(template.render(report=self.report, debug_setting=debug_setting))

Contributor

evgenyz Sep 9, 2022

I'm not quite sure that Jinja is needed here. You can just filter out keys from the report dict and then export it to JSON. Even if in the future there would be some more sophisticated logic (like keeping report.foo.bar and report.foo.baz but not report.foo.others) it'd be easier to just write a more complicated filter function rather than to siphon the whole report through Jinja engine.

Member Author

Honny1 Sep 11, 2022

You're right.

openscap_report/scap_results_parser/parsers/info_of_test_parser.py Outdated

+                      self._transform_tuples_to_dict(out)
+                      if out:
+                          return out
+                      return None

Contributor

evgenyz Sep 9, 2022

First of all you can just return out or None.

But I don't quite understand why you're trying to use None instead of an empty list if there are no elements. What benefit does it give? I can see a drawback, though. In situations when one would iterate over a list there always should be a check if the list is not None.

Member Author

Honny1 Sep 11, 2022 •

edited

By using None instead of an empty array, I mean that there is no data in the JSON for that item. A dictionary filter could solve this.

Contributor

evgenyz Sep 11, 2022

Sure, but why do you think that

foo {
   bars_list: None
}

is better than

foo {
   bars_list: []
}

?

Member Author

Honny1 Sep 11, 2022

In terms of python code, it is better to use an empty array. As for the JSON content, I think it's better to use null because it's more obvious that there's no data or this data is optional. I think it is about a discussion about JSON content. Which is the best value to use for cases when there is no data?

Contributor

evgenyz Sep 12, 2022

You're right, let's discuss it in the JSON structure issue. But here is some homework for you: https://www.infoq.com/presentations/Null-References-The-Billion-Dollar-Mistake-Tony-Hoare/ :)

openscap_report/scap_results_parser/parsers/rule_parser.py Outdated

+                          references.append(Reference(referenc.get("href"), referenc.text))
+                      if references:
+                          return references
+                      return None

Contributor

evgenyz Sep 9, 2022

None instead of [], why?

openscap_report/scap_results_parser/parsers/rule_parser.py Outdated

+                          identifiers.append(Identifier(identifier.get("system"), identifier.text))
+                      if identifiers:
+                          return identifiers
+                      return None

Contributor

evgenyz Sep 9, 2022

None instead of [], why?

openscap_report/scap_results_parser/parsers/rule_parser.py Outdated

-                      return warnings
+                      if warnings:
+                          return warnings
+                      return None

Contributor

evgenyz Sep 9, 2022

None instead of [], why?

openscap_report/scap_results_parser/parsers/rule_parser.py Outdated

-                      return output
+                      if output:
+                          return output
+                      return None

Contributor

evgenyz Sep 9, 2022

None instead of [], why?

openscap_report/scap_results_parser/parsers/rule_parser.py Outdated

+                  def get_rules(self):
+                      rules = {}
+                      for rule in self.root.findall(".//xccdf:Rule", NAMESPACES):
+                          rule = self.process_rule(rule)

Contributor

evgenyz Sep 9, 2022

This kind of shadowing could lead to a lot of troubles in a dynamically typed language. If you can have a Rule class entity and a rule xml sub-document it's better to introduce a naming scheme which won't allow you to mix them up. For example for all xml entities it could be _els (rule_el).

Honny1 mentioned this pull request

Discussion of JSON output #90

Closed

Honny1 force-pushed the json-output-6 branch 3 times, most recently from 223f566 to e166e04 Compare

September 14, 2022 14:49

Honny1 mentioned this pull request

JSON output [2] #96

Closed

Honny1 changed the title ~~JSON output [6]~~ JSON output [5]

Honny1 force-pushed the json-output-6 branch from e166e04 to fa1c005 Compare

September 15, 2022 14:24


          Create objects Identifier and Reference

78ceff9

Honny1 added 8 commits

September 15, 2022 16:32


          Specify data types of Group

8bd62f3


          Specify data types of OvalNode

b5b3b54


          Specify data types of OvalObject


          Specify data types of OvalTest

f5e12b1


          Remove default id of Remediation

496d2c9


          Specify data types of Report

26dc6fe


          Specify data types of Rule

9c5c02a


          Fix test of remediation

f3fffda

Honny1 force-pushed the json-output-6 branch 2 times, most recently from f409022 to 79e8a41 Compare

September 15, 2022 14:55

Member Author

Honny1 commented Sep 15, 2022

@evgenyz I changed the code according to your review. And I dropped the json-output-2 branch because there was code there that replace empty arrays with none.

Honny1 requested a review from evgenyz

September 15, 2022 15:02

Honny1 added 11 commits

September 16, 2022 14:27


          Remove None value from definition ID

2328e18


          Remove None comment

85c6931


          Rework assembly of OVAL and CPE trees

bb2dd41


          Rework structure SCAPResultsParser class

d77fe50

* Move methots of parser to separate classes to impruve organisation of
code.
* Reduce using empty structures in __init__ methodes and impruve
overall code quality.
* Impruve code strucutre for better tesing.


          Fix tests according to change of structure of SCAPResultsParser class

62da294


          Break methodes to smaller methods

f67455f


          Rename directory

1d35fca


          Use filter for generation JSON

130d42a


          Create output format JSON-EVERYTHING

1f778fa

JSON-EVERYTHING contains everyhing what openscap-report can offer.


          Rename groupe_parser to group_parser and info_of_test_parser to test_…

6b312b7

…info_parser


          Fix mixing of Rule class and rule XML element

4f7456c

Honny1 force-pushed the json-output-6 branch from 79e8a41 to 4f7456c Compare

September 16, 2022 12:32

evgenyz merged commit 712e7a7 into OpenSCAP:master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment