Introduce platform dependent test scenarios #3627

jan-cerny · 2018-11-27T16:33:15Z

We face a problem that for a single rule we need to have different sets
of test scenarios depending on target platform. For example, the rule
'auditd_audispd_encrypt_sent_records' exists on both RHEL 7 and RHEL 8,
but on each of them a different file path and a different string
is checked. We need to be able to define test scenarios that are
applicable only to selected platforms and should be skipped when testing
other platforms.

This will be achieved by using platform option in scenario script headers.
It works similar way we specify platforms in Bash remediations. We can
use multi_platform_rhel, multi_platform_fedora, etc.

We face a problem that for a single rule we need to have different sets of test scenarios depending on target platform. For example, the rule 'auditd_audispd_encrypt_sent_records' exists on both RHEL 7 and RHEL 8, but on each of them a different file path and a different string is checked. We need to be able to define test scenarios that are applicable only to selected platforms and should be skipped when testing other platforms. This will be achieved by using platform option in scenario script headers. It works similar way we specify platforms in Bash remediations. We can use `multi_platform_rhel`, `multi_platform_fedora`, etc.

pep8speaks · 2018-11-27T16:33:20Z

Hello @jan-cerny! Thanks for updating the PR.

In the file ssg/build_ovals.py, following are the PEP8 issues :

Line 89:80: E501 line too long (90 > 79 characters)
Line 90:80: E501 line too long (90 > 79 characters)
Line 164:80: E501 line too long (80 > 79 characters)
Line 184:80: E501 line too long (87 > 79 characters)
Line 185:80: E501 line too long (87 > 79 characters)
Line 299:80: E501 line too long (82 > 79 characters)
Line 301:80: E501 line too long (80 > 79 characters)
Line 303:80: E501 line too long (84 > 79 characters)

In the file ssg/build_remediations.py, following are the PEP8 issues :

Line 35:80: E501 line too long (89 > 79 characters)
Line 207:80: E501 line too long (81 > 79 characters)
Line 235:80: E501 line too long (85 > 79 characters)
Line 236:80: E501 line too long (82 > 79 characters)
Line 373:80: E501 line too long (80 > 79 characters)
Line 378:80: E501 line too long (81 > 79 characters)
Line 379:80: E501 line too long (81 > 79 characters)
Line 381:80: E501 line too long (82 > 79 characters)
Line 426:80: E501 line too long (92 > 79 characters)
Line 427:80: E501 line too long (89 > 79 characters)
Line 482:80: E501 line too long (81 > 79 characters)
Line 486:80: E501 line too long (82 > 79 characters)
Line 489:80: E501 line too long (80 > 79 characters)
Line 499:80: E501 line too long (80 > 79 characters)
Line 500:80: E501 line too long (81 > 79 characters)
Line 501:80: E501 line too long (82 > 79 characters)
Line 509:80: E501 line too long (80 > 79 characters)

In the file ssg/build_yaml.py, following are the PEP8 issues :

Line 64:80: E501 line too long (85 > 79 characters)
Line 182:80: E501 line too long (92 > 79 characters)
Line 421:80: E501 line too long (92 > 79 characters)
Line 444:80: E501 line too long (103 > 79 characters)
Line 532:80: E501 line too long (92 > 79 characters)
Line 548:80: E501 line too long (81 > 79 characters)
Line 549:80: E501 line too long (88 > 79 characters)
Line 556:80: E501 line too long (89 > 79 characters)
Line 557:80: E501 line too long (88 > 79 characters)
Line 565:80: E501 line too long (87 > 79 characters)
Line 615:80: E501 line too long (85 > 79 characters)
Line 619:80: E501 line too long (83 > 79 characters)
Line 620:80: E501 line too long (82 > 79 characters)
Line 626:80: E501 line too long (80 > 79 characters)
Line 637:80: E501 line too long (103 > 79 characters)
Line 648:80: E501 line too long (80 > 79 characters)
Line 722:80: E501 line too long (88 > 79 characters)
Line 723:80: E501 line too long (81 > 79 characters)

In the file ssg/constants.py, following are the PEP8 issues :

Line 14:80: E501 line too long (82 > 79 characters)
Line 59:80: E501 line too long (92 > 79 characters)
Line 248:80: E501 line too long (83 > 79 characters)
Line 249:80: E501 line too long (83 > 79 characters)
Line 250:80: E501 line too long (83 > 79 characters)
Line 263:80: E501 line too long (80 > 79 characters)
Line 267:80: E501 line too long (81 > 79 characters)
Line 272:80: E501 line too long (84 > 79 characters)
Line 275:80: E501 line too long (80 > 79 characters)
Line 283:80: E501 line too long (82 > 79 characters)
Line 293:80: E501 line too long (88 > 79 characters)
Line 298:80: E501 line too long (82 > 79 characters)
Line 299:80: E501 line too long (83 > 79 characters)
Line 300:80: E501 line too long (87 > 79 characters)
Line 301:80: E501 line too long (87 > 79 characters)
Line 302:80: E501 line too long (83 > 79 characters)
Line 305:80: E501 line too long (85 > 79 characters)
Line 306:80: E501 line too long (102 > 79 characters)
Line 307:80: E501 line too long (86 > 79 characters)
Line 312:80: E501 line too long (93 > 79 characters)
Line 315:80: E501 line too long (89 > 79 characters)
Line 323:80: E501 line too long (82 > 79 characters)

In the file ssg/products.py, following are the PEP8 issues :

Line 96:80: E501 line too long (85 > 79 characters)

In the file tests/ssg_test_suite/common.py, following are the PEP8 issues :

Line 64:80: E501 line too long (90 > 79 characters)
Line 120:80: E501 line too long (87 > 79 characters)

In the file tests/ssg_test_suite/oscap.py, following are the PEP8 issues :

Line 67:80: E501 line too long (93 > 79 characters)
Line 81:80: E501 line too long (85 > 79 characters)
Line 132:80: E501 line too long (80 > 79 characters)
Line 505:80: E501 line too long (86 > 79 characters)
Line 517:80: E501 line too long (83 > 79 characters)
Line 526:80: E501 line too long (80 > 79 characters)
Line 540:80: E501 line too long (83 > 79 characters)
Line 555:80: E501 line too long (86 > 79 characters)
Line 579:80: E501 line too long (82 > 79 characters)
Line 585:80: E501 line too long (97 > 79 characters)

In the file tests/ssg_test_suite/rule.py, following are the PEP8 issues :

Line 36:28: W605 invalid escape sequence '.'
Line 36:30: W605 invalid escape sequence '-'
Line 36:32: W605 invalid escape sequence '\w'
Line 83:80: E501 line too long (87 > 79 characters)
Line 96:80: E501 line too long (83 > 79 characters)
Line 102:80: E501 line too long (89 > 79 characters)
Line 106:80: E501 line too long (80 > 79 characters)
Line 130:80: E501 line too long (83 > 79 characters)
Line 138:8: W605 invalid escape sequence '.'
Line 138:17: W605 invalid escape sequence '.'
Line 154:1: E302 expected 2 blank lines, found 1
Line 164:80: E501 line too long (82 > 79 characters)
Line 167:80: E501 line too long (86 > 79 characters)
Line 200:80: E501 line too long (93 > 79 characters)
Line 204:80: E501 line too long (89 > 79 characters)
Line 207:80: E501 line too long (88 > 79 characters)
Line 214:80: E501 line too long (89 > 79 characters)
Line 236:80: E501 line too long (80 > 79 characters)
Line 263:80: E501 line too long (89 > 79 characters)
Line 268:80: E501 line too long (99 > 79 characters)
Line 287:80: E501 line too long (94 > 79 characters)
Line 290:80: E501 line too long (82 > 79 characters)
Line 296:80: E501 line too long (85 > 79 characters)
Line 314:80: E501 line too long (83 > 79 characters)

There are no PEP8 issues in the file tests/ssg_test_suite/xml_operations.py !
In the file tests/test_suite.py, following are the PEP8 issues :

Line 14:1: E402 module level import not at top of file
Line 15:1: E402 module level import not at top of file
Line 16:1: E402 module level import not at top of file
Line 17:1: E402 module level import not at top of file
Line 18:1: E402 module level import not at top of file
Line 19:1: E402 module level import not at top of file
Line 37:80: E501 line too long (83 > 79 characters)
Line 49:80: E501 line too long (83 > 79 characters)
Line 50:80: E501 line too long (80 > 79 characters)
Line 87:80: E501 line too long (82 > 79 characters)
Line 90:80: E501 line too long (82 > 79 characters)
Line 94:80: E501 line too long (81 > 79 characters)
Line 95:80: E501 line too long (80 > 79 characters)
Line 99:80: E501 line too long (83 > 79 characters)
Line 109:80: E501 line too long (83 > 79 characters)
Line 110:80: E501 line too long (83 > 79 characters)
Line 111:80: E501 line too long (83 > 79 characters)
Line 112:80: E501 line too long (81 > 79 characters)
Line 123:80: E501 line too long (82 > 79 characters)
Line 157:80: E501 line too long (86 > 79 characters)
Line 197:80: E501 line too long (80 > 79 characters)

In the file tests/unit/ssg_test_suite/test_matches_platform.py, following are the PEP8 issues :

Line 9:72: E712 comparison to True should be 'if cond is True:' or 'if cond:'
Line 15:72: E712 comparison to False should be 'if cond is False:' or 'if not cond:'
Line 21:72: E712 comparison to True should be 'if cond is True:' or 'if cond:'
Line 27:72: E712 comparison to True should be 'if cond is True:' or 'if cond:'
Line 33:72: E712 comparison to False should be 'if cond is False:' or 'if not cond:'
Line 40:72: E712 comparison to True should be 'if cond is True:' or 'if cond:'
Line 47:72: E712 comparison to True should be 'if cond is True:' or 'if cond:'
Line 54:72: E712 comparison to False should be 'if cond is False:' or 'if not cond:'
Line 60:72: E712 comparison to True should be 'if cond is True:' or 'if cond:'
Line 66:72: E712 comparison to True should be 'if cond is True:' or 'if cond:'
Line 72:72: E712 comparison to False should be 'if cond is False:' or 'if not cond:'
Line 79:72: E712 comparison to True should be 'if cond is True:' or 'if cond:'
Line 86:72: E712 comparison to True should be 'if cond is True:' or 'if cond:'
Line 93:72: E712 comparison to False should be 'if cond is False:' or 'if not cond:'
Line 101:72: E712 comparison to True should be 'if cond is True:' or 'if cond:'
Line 109:72: E712 comparison to True should be 'if cond is True:' or 'if cond:'
Line 117:72: E712 comparison to True should be 'if cond is True:' or 'if cond:'
Line 119:1: E302 expected 2 blank lines, found 1
Line 124:72: E712 comparison to False should be 'if cond is False:' or 'if not cond:'
Line 132:72: E712 comparison to False should be 'if cond is False:' or 'if not cond:'

Comment last updated on November 30, 2018 at 10:59 Hours UTC

yuumasato

Interesting concept, that the test scenario filtering/selection will be done using the platforms that the content is applicable to.

My main concern is that we will be maintaining another mapping in yet another place.
We should consolidate these mappings, in ssg/products.py there is already a mapping from platform name to product name going on, I think it makes sense to consolidate it there.

yuumasato · 2018-11-28T09:54:52Z

tests/README.md

+The header consists of comments (starting by `#`). Possible values are:
+- `platform` is a comma-separated list of platforms where the test scenario can be run. This is similar to `platform` used in our remediations. Examples of values: `multi_platform_rhel`, `Red Hat Enterprise Linux 7`, `multi_platform_all`. If `platform` is not specified in the header, `multi_platform_all` is assumed.
+- `profiles` is a comma-separated list of profiles which are using this scenario.
+- `remediation` is a comma-separated list of allowed remediation types (eg. `bash`, `ansible`).


There can be none option too, which specifies that a rule has no fix implemented at all.

Great catch! I'll update that.

yuumasato · 2018-11-28T10:33:38Z

tests/ssg_test_suite/common.py

+    "Fedora": [
+        "cpe:/o:fedoraproject:fedora:28",
+        "cpe:/o:fedoraproject:fedora:29",
+        "cpe:/o:fedoraproject:fedora:30"


Would it be possible to source these cpes from a place where we already track it?
For example the product.yml?

This mapping of product name to cpe could be outsourced to ssg/products.py

yuumasato · 2018-11-28T10:34:05Z

tests/ssg_test_suite/common.py

+}
+
+# Keys are strings used in test scenario header in 'platform' list
+TEST_MULTI_PLATFORMS = {


In ssg/products.py we already maintain a mapping of platforms to names.
I'd rather avoid maintaining a separate mapping, and extend that functionality to map from platforms to cpes.

@yuumasato Yes, I like the idea to have it defined at single place.

There are multiple problems with that:

The CPEs for each product are defined in the product-specific product.yml. We probably shouldn't parse these files in the test suite. It would be better to change the content build system to source it from a single place as well.

The CPEs for derivatives (CentOS, Scientific Linux) are not defined in product.yml but ssg/constants.py.

The code in ssg/products.py maps shortened names of products (eg. rhel7) to official names, eg. when you build rhel7 product you need to know that you should also include remediations marked as one of ['multi_platform_rhel','multi_platform_all', 'Red Hat Enterprise Linux 7']. But we need kind of opposite mapping - eg. when we specify multi_platform_rhel we would like to expand it to ['rhel6','rhel7'] and then convert it a list of CPEs.

I would like to have all the mappings defined at a single point. But I'm not sure if it's not a scope creep. Should it be done in a separate PR?

The CPEs for each product are defined in the product-specific product.yml. We probably shouldn't parse these files in the test suite. It would be better to change the content build system to source it from a single place as well.

I'm not sure it would be better, for someone writing content, or adding a new product, it wouldn't be straight forward to go into ssg/constants.py and add the mappings there.

I'm okay to go take this path for now, but in the end, I think all data specific for the product should be in its product.yml, for example, the product tag name, full name, and cpes.
And the build system would know what products are part of which multi_platform_ label, and it gathers the data and builds the mappings.

This mapping of product could even be extended to something else, something not tied to the product name, but feature based. This could be a "label" defined in the product.yml, that would aggregate rules, to products that provide the feature.
Some labels I can imagine:crypto_policy, fips, grub_legacy, grub2.

@yuumasato But that means that if we want to use it in test suite the test suite would have to parse all the product.ymls.

@jan-cerny yes. In the future we could.

yuumasato · 2018-11-28T10:34:27Z

tests/ssg_test_suite/rule.py

+                platform_cpes = set(common.TEST_PLATFORMS[platform])
+        except KeyError:
+            raise ValueError(
+                "Platform '%s' is not supported by the test suite." % platform)


What is the reason a platform is not supported for testing?
Is it because there wouldn't be a mapping from the product name to its cpe?

Yes, it's exactly that. I agree it's kind of stupid. It is related to previous comment. Basically the problem is that it isn't easy to get such a mapping now and we'll need to change the way we define product names and CPEs to prevent maintaining a separate list of mappings for the test suite.

Gather CPEs from product.yml to a single location. We can reuse the mapping in our SSG test suite, which would be difficult if they're not in a single place.

matejak · 2018-11-29T16:54:24Z

@jan-cerny If you have surplus, you may consider testing some of the newly introduced functionality in the SSG unit-testing framework. In that case, check out the content/tests/unit/ssg_test_suite folder.
As the implementation of platform identification may change in the future so it is in one place both for the build system and the test suite, having some easy tests for the _matches_platform function looks like as a good stretch goal.

jan-cerny · 2018-11-30T09:02:10Z

@matejak I have added a basic unit test for that.

Putting it to rule module was a problem if you don't have docker. Addressing: _____ ERROR collecting tests/unit/ssg_test_suite/test_matches_platform.py ______ ImportError while importing test module '/home/jenkins/workspace/scap-security-guide-pull-requests/label/fedora/tests/unit/ssg_test_suite/test_matches_platform.py'. Hint: make sure your test modules/packages have valid Python names. Traceback: ../../tests/unit/ssg_test_suite/test_matches_platform.py:3: in <module> from ssg_test_suite import rule ../../tests/ssg_test_suite/rule.py:12: in <module> import ssg_test_suite.oscap as oscap ../../tests/ssg_test_suite/oscap.py:13: in <module> from ssg_test_suite import test_env ../../tests/ssg_test_suite/test_env.py:8: in <module> import docker E ModuleNotFoundError: No module named 'docker'

jan-cerny · 2018-11-30T10:31:28Z

Unfortunately, I have discovered that in build_remediations.py we have a different logic that reuses the fact that build_remediations.py knows which product directory (eg. rhel6/, rhel7/) it works with. The test suite doesn't (couldn't) have this information, so we need an explicit mapping somewhere.

I have noticed that we could reuse my new code in the test suite and ssg.constants to refactor build_remediations.py and similar scripts. But I vote against doing this refactoring in this PR because that would be a scope creep.

jan-cerny · 2018-11-30T10:54:48Z

tests/ssg_test_suite/common.py

+    scenario_cpes = set()
+    for p in scenario_platforms:
+        scenario_cpes |= _get_platform_cpes(p)
+    print(scenario_cpes)


Please remove this!

scrutinizer-notifier · 2018-11-30T11:01:34Z

The inspection completed: 28 updated code elements

yuumasato

The tests look good to me.

yuumasato · 2018-12-03T10:08:21Z

tests/unit/ssg_test_suite/test_matches_platform.py

+
+
+def test_wrong_multi_platform():
+    scenario_platforms = ["multi_platform_fidorka"]


Fidorka! :)

jan-cerny added enhancement General enhancements to the project. Test Suite Update in Test Suite. labels Nov 27, 2018

jan-cerny added this to the 0.1.42 milestone Nov 27, 2018

Fix PEP8 problems

e5f4d82

jan-cerny changed the title ~~[WIP] Introduce platform dependent test scenarios~~ Introduce platform dependent test scenarios Nov 28, 2018

jan-cerny mentioned this pull request Nov 28, 2018

Fix auditd_audispd_encrypt_sent_records on Fedora and RHEL8 #3619

Merged

yuumasato requested changes Nov 28, 2018

View reviewed changes

jan-cerny added 3 commits November 29, 2018 11:34

Mention 'none' in the README

21950b4

Centralize CPEs in ssg.constants

44970ac

Gather CPEs from product.yml to a single location. We can reuse the mapping in our SSG test suite, which would be difficult if they're not in a single place.

Move multi_platform_list to constants

44f399c

jan-cerny added 5 commits November 29, 2018 18:15

Add FULL_NAME_TO_PRODUCT_MAPPING

76e01b8

Introduce mapping from multi_platform to a list of products

a8f5be6

Use global SSG constants in test suite

a98547d

Raise an exception when a wrong value is provided

5f30a81

Add a unit test for _matches_platform from ssg_test_suite

a1de95d

jan-cerny commented Nov 30, 2018

View reviewed changes

Remove superfluous print

5456e60

yuumasato approved these changes Dec 3, 2018

View reviewed changes

yuumasato merged commit 53139b1 into ComplianceAsCode:master Dec 3, 2018

yuumasato self-assigned this Dec 3, 2018

jan-cerny deleted the platform_tests branch December 3, 2018 13:23

yuumasato mentioned this pull request Feb 5, 2019

False test scenario results in test suite #3630

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce platform dependent test scenarios #3627

Introduce platform dependent test scenarios #3627

jan-cerny commented Nov 27, 2018

pep8speaks commented Nov 27, 2018 •

edited

yuumasato left a comment

yuumasato Nov 28, 2018

jan-cerny Nov 28, 2018

yuumasato Nov 28, 2018

yuumasato Nov 28, 2018

yuumasato Nov 28, 2018

jan-cerny Nov 28, 2018

yuumasato Nov 30, 2018

jan-cerny Nov 30, 2018

yuumasato Nov 30, 2018

yuumasato Nov 28, 2018

jan-cerny Nov 28, 2018

matejak commented Nov 29, 2018

jan-cerny commented Nov 30, 2018

jan-cerny commented Nov 30, 2018

jan-cerny Nov 30, 2018

scrutinizer-notifier commented Nov 30, 2018

yuumasato left a comment

yuumasato Dec 3, 2018



		def test_wrong_multi_platform():
		scenario_platforms = ["multi_platform_fidorka"]

Introduce platform dependent test scenarios #3627

Introduce platform dependent test scenarios #3627

Conversation

jan-cerny commented Nov 27, 2018

pep8speaks commented Nov 27, 2018 • edited

Comment last updated on November 30, 2018 at 10:59 Hours UTC

yuumasato left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matejak commented Nov 29, 2018

jan-cerny commented Nov 30, 2018

jan-cerny commented Nov 30, 2018

Choose a reason for hiding this comment

scrutinizer-notifier commented Nov 30, 2018

yuumasato left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pep8speaks commented Nov 27, 2018 •

edited