Reorganize env and product yaml #6754

yuumasato · 2021-03-25T16:27:27Z

Description:

This adds 393fbdc and ea7dad8 on top of Add rules from the Policy to profiles based on prodtype (Includes DRAFT ANSSI profiles for RHCOS) #6725 reorganzing a bit the env_yaml and the product_yaml.
Moves loading of product_cpes to get_product_yaml().

Rationale:

A few aspects that were product specific were being amalgamated in open_environment()

pep8speaks · 2021-03-25T16:27:41Z

Hello @yuumasato! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-04-29 13:09:56 UTC

openscap-ci · 2021-03-25T16:27:45Z

Changes identified:
Others:
Changes in Python files.

Show details

Others:
Python abstract syntax tree change found in build-scripts/build_templated_content.py.
Python abstract syntax tree change found in build-scripts/combine_ovals.py.
Python abstract syntax tree change found in build-scripts/combine_remediations.py.
Python abstract syntax tree change found in build-scripts/compile_profiles.py.
Python abstract syntax tree change found in build-scripts/cpe_generate.py.
Python abstract syntax tree change found in build-scripts/yaml_to_shorthand.py.
Python abstract syntax tree change found in ssg/build_cpe.py.
Python abstract syntax tree change found in ssg/build_derivatives.py.
Python abstract syntax tree change found in ssg/build_yaml.py.
Python file ssg/environment.py is newly added.
Python abstract syntax tree change found in ssg/oval.py.
Python abstract syntax tree change found in ssg/products.py.
Python abstract syntax tree change found in ssg/yaml.py.
Python abstract syntax tree change found in tests/ssg_test_suite/common.py.
Python abstract syntax tree change found in tests/test_parse_affected.py.
Python abstract syntax tree change found in tests/unit/ssg-module/test_controls.py.
Python abstract syntax tree change found in utils/render-policy.py.
Python abstract syntax tree change found in utils/rule_dir_json.py.

Recommended tests to execute:
(cd build && cmake ../ && ctest -j4)

yuumasato · 2021-03-25T16:28:12Z

Does this reorg make sense?

openshift-ci-robot · 2021-03-25T16:28:30Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

vojtapolasek

Looks good, thank you. This is important work. It seems that it builds correctly.
There are some notices from pep8 bot, could you check it please?
Also there has been another utility merged recently - render-rules.py. Is it also affected?

vojtapolasek · 2021-04-21T15:02:43Z

build-scripts/compile_profiles.py

@@ -54,7 +54,7 @@ def get_env_yaml(build_config_yaml, product_yaml):
    if build_config_yaml is None or product_yaml is None:
        return None

-    env_yaml = ssg.yaml.open_environment(build_config_yaml, product_yaml)
+    env_yaml = ssg.environment.open_environment(build_config_yaml, product_yaml)


I notice that in other files where you did this change, you also altered the import statement. Here you did not. Nothing seems to break during build, but I rather ask.

The script didn't have an import for the yaml module. So I just followed the same format and replaced ssg.yaml with ssg.environment.

I think this happens because ssg.build_yaml already imports ssg.{yaml->environment} and so you don't strictly need to duplicate the import.

vojtapolasek · 2021-04-22T10:28:20Z

/retest

vojtapolasek · 2021-04-22T10:38:44Z

Seems that there is a problem with platform called ocp4.8.

cipherboy · 2021-04-26T17:38:20Z

ssg/products.py

+    return result
+
+
+def get_product_yaml(product_yaml_path):


Edit: I can't read, ssg.environment was added with open_environment which does what question one asks.

~~Two~~ One question here:

~~Does this need to be tempered by the build YAML? If you look at e.g., combine_ovals.py~~:
env_yaml = ssg.yaml.open_environment(args.build_config_yaml, args.product_yaml)

Would something like load_product_yaml be clearer? Especially if we add a get_product_yaml_path(product) -- we could make the product YAML path be inferred by the actual product. Just a thought.

I like the idea of the two functions.

I think this will actually lead to too many changes...I'd like to do this in a subsequent PR, and discuss some of the changes.

I just renamed the current function to load_product_yaml.

build-scripts/cpe_generate.py

cipherboy · 2021-04-26T17:49:45Z

tests/ssg_test_suite/common.py

@@ -171,7 +173,9 @@ def _get_platform_cpes(platform):
            raise ValueError
        platform_cpes = set()
        for p in products:
-            p_cpes = ProductCPEs(p)
+            product_yaml_path = os.path.join(ssg_root, p, "product.yml")


Here's where I was thinking a get_product_yaml(product) form would be most useful. Then it would be load_product_yaml(path) elsewhere. My 2c.

cipherboy

I probably should've batched into a review but then I kept looking :-)

I didn't follow all the changes in build_yaml.py but otherwise looks good to me. Will let others do the final review and go-ahead, seems like there's still a build failure possibly and some conflicts?

Separate into products.py functions that are related to products, including checking its data and deriving properties. Relocate open_environment() away from yaml.py, this function is more about loading build environment configurations than yaml loading per se. This should ease the life of scripts to only want to load data from the product, but don't care about the environment. (How dare we have scripts that are not eco friendly?) Note: this commit breaks the build as other scripts need to be updated. It is done as a separate commit to help understand the changes.

This commits just follows the previous commit, in which env and product yaml were decoupled.

This moves incorporation of CPE names into the loading process, opposed to during oputput process. We should be information complete once we are "loaded". The product_yaml should contain data for the product being built, accessibility of ProductCPEs from product_yaml makes for cleaner code and more easily expandable.

yuumasato

Thank you for the review, folks.
I've just rebased to fix the conflict, and addressed the build issue with platform ocp-4.8

build-scripts/cpe_generate.py

yuumasato · 2021-04-28T14:24:05Z

ssg/products.py

+    return result
+
+
+def get_product_yaml(product_yaml_path):


I like the idea of the two functions.

yuumasato · 2021-04-28T15:26:50Z

@cipherboy @vojtapolasek This is ready to merge if your reviews are still positive.

I'll look into adding get_product_yaml_path in a subsequent PR.

vojtapolasek · 2021-04-29T13:01:30Z

/retest

A rule can be loaded from yaml in any context, even when building unrelated products. A rule is completely loaded, and then its prodtype is assessed against the product being built. This limits the conversion of platform names to cpe names to situations where the rule is compatible with the product.

vojtapolasek · 2021-04-30T09:05:46Z

/retest

Since ComplianceAsCode#6754 was merged before ComplianceAsCode#6906's last update (and before ComplianceAsCode#6906 was merged), it missed the move of ssg.yaml.open_environment into ssg.environment.open_environment. This fixes autoprodtyper.py to understand the new location of this function. Signed-off-by: Alexander Scheel <alex.scheel@canonical.com>

openshift-ci-robot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Mar 25, 2021

yuumasato force-pushed the reorganize_env_and_product_yaml branch 3 times, most recently from 31fce83 to 3042004 Compare April 6, 2021 10:06

yuumasato force-pushed the reorganize_env_and_product_yaml branch 2 times, most recently from 5b4ccbb to 0dc3f4b Compare April 6, 2021 11:30

yuumasato marked this pull request as ready for review April 6, 2021 13:39

openshift-ci-robot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Apr 6, 2021

yuumasato requested review from matejak and vojtapolasek and removed request for matejak April 20, 2021 16:20

vojtapolasek self-assigned this Apr 21, 2021

vojtapolasek reviewed Apr 21, 2021

View reviewed changes

yuumasato force-pushed the reorganize_env_and_product_yaml branch 2 times, most recently from 0b032aa to 87b6c3a Compare April 22, 2021 09:43

cipherboy reviewed Apr 26, 2021

View reviewed changes

build-scripts/cpe_generate.py Show resolved Hide resolved

cipherboy reviewed Apr 26, 2021

View reviewed changes

cipherboy approved these changes Apr 26, 2021

View reviewed changes

cipherboy mentioned this pull request Apr 27, 2021

Sort references, identifiers in rule.yml #6882

Merged

openshift-ci bot added the needs-rebase Used by openshift-ci bot. label Apr 27, 2021

yuumasato added 3 commits April 27, 2021 18:44

Fix scripts follow to env and product yaml split

8a0866f

This commits just follows the previous commit, in which env and product yaml were decoupled.

Address style issues

8702b3e

yuumasato force-pushed the reorganize_env_and_product_yaml branch from 87b6c3a to 81d53dc Compare April 28, 2021 14:20

yuumasato commented Apr 28, 2021

View reviewed changes

yuumasato added this to the 0.1.56 milestone Apr 28, 2021

openshift-ci bot removed the needs-rebase Used by openshift-ci bot. label Apr 28, 2021

yuumasato force-pushed the reorganize_env_and_product_yaml branch from 1520544 to 1c856aa Compare April 29, 2021 13:07

yuumasato added 2 commits April 29, 2021 15:09

For clarity, rename function to load_product_yaml

756abee

yuumasato force-pushed the reorganize_env_and_product_yaml branch from 1c856aa to 756abee Compare April 29, 2021 13:09

vojtapolasek merged commit 8f53742 into ComplianceAsCode:master Apr 30, 2021

yuumasato deleted the reorganize_env_and_product_yaml branch April 30, 2021 14:05

vojtapolasek mentioned this pull request May 3, 2021

RHEL7 - rules that are not applicable to containers are evaluated in container scan #6938

Closed

cipherboy mentioned this pull request May 10, 2021

Fix autoprodtyper's open_environment call #6989

Merged

yuumasato mentioned this pull request Jun 3, 2021

Fix import of open_environment method #7076

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reorganize env and product yaml #6754

Reorganize env and product yaml #6754

yuumasato commented Mar 25, 2021 •

edited

pep8speaks commented Mar 25, 2021 •

edited

openscap-ci commented Mar 25, 2021 •

edited

yuumasato commented Mar 25, 2021

openshift-ci-robot commented Mar 25, 2021

vojtapolasek left a comment

vojtapolasek Apr 21, 2021

yuumasato Apr 22, 2021

cipherboy Apr 26, 2021

vojtapolasek commented Apr 22, 2021

vojtapolasek commented Apr 22, 2021

cipherboy Apr 26, 2021 •

edited

yuumasato Apr 28, 2021

yuumasato Apr 28, 2021 •

edited

yuumasato Apr 28, 2021

cipherboy Apr 26, 2021

cipherboy left a comment •

edited

yuumasato left a comment

yuumasato Apr 28, 2021

yuumasato commented Apr 28, 2021

vojtapolasek commented Apr 29, 2021

vojtapolasek commented Apr 30, 2021

Reorganize env and product yaml #6754

Reorganize env and product yaml #6754

Conversation

yuumasato commented Mar 25, 2021 • edited

Description:

Rationale:

pep8speaks commented Mar 25, 2021 • edited

Comment last updated at 2021-04-29 13:09:56 UTC

openscap-ci commented Mar 25, 2021 • edited

yuumasato commented Mar 25, 2021

openshift-ci-robot commented Mar 25, 2021

vojtapolasek left a comment

Choose a reason for hiding this comment

vojtapolasek Apr 21, 2021

Choose a reason for hiding this comment

yuumasato Apr 22, 2021

Choose a reason for hiding this comment

cipherboy Apr 26, 2021

Choose a reason for hiding this comment

vojtapolasek commented Apr 22, 2021

vojtapolasek commented Apr 22, 2021

cipherboy Apr 26, 2021 • edited

Choose a reason for hiding this comment

yuumasato Apr 28, 2021

Choose a reason for hiding this comment

yuumasato Apr 28, 2021 • edited

Choose a reason for hiding this comment

yuumasato Apr 28, 2021

Choose a reason for hiding this comment

cipherboy Apr 26, 2021

Choose a reason for hiding this comment

cipherboy left a comment • edited

Choose a reason for hiding this comment

yuumasato left a comment

Choose a reason for hiding this comment

yuumasato Apr 28, 2021

Choose a reason for hiding this comment

yuumasato commented Apr 28, 2021

vojtapolasek commented Apr 29, 2021

vojtapolasek commented Apr 30, 2021

yuumasato commented Mar 25, 2021 •

edited

pep8speaks commented Mar 25, 2021 •

edited

openscap-ci commented Mar 25, 2021 •

edited

cipherboy Apr 26, 2021 •

edited

yuumasato Apr 28, 2021 •

edited

cipherboy left a comment •

edited