Correctly process templated Ansible conditionals and introduce os_linux platform #9959

vojtapolasek · 2022-12-12T12:51:28Z

Description:

change the way in which resolved CPE items are created - in past they were overwritten each time a platform had been loaded
when conditionals for CPE items are processed in template.py, immediately propagate them to currently loaded CPE items as well as platforms.
Also store CPE items and platforms whose conditionals have been altered by templating to the disk.
if you for some reason do not wish the templated conditional to apply, just use the "backends" dictionary to disable some remediation language
as a proof, introduce quite simple os_linux platform which can restrict applicability to OS family

Rationale:

render template conditionals correctly
introduce new templated platform

Review hints:

modify some rule, e.g. chronyd_or_ntpd_set_maxpoll and ad to the platform os_linux[rhel] or os_linux[fedora]. Observe the Ansible remediation of the selected rule.

github-actions · 2022-12-12T13:19:45Z

Start a new ephemeral environment with changes proposed in this pull request:

Fedora Environment

Oracle Linux 8 Environment

jan-cerny · 2022-12-12T13:36:38Z

build-scripts/update_cpe_conditionals.py

+        "e.g.: ~/scap-security-guide/rhel7/product.yml"
+    )
+    p.add_argument(
+        "--remediation-type", required=True, action="append",


It probably should have a dest with remediation_types with s at the end because it's a list and it can hold multiple values. Later, it looks confusing when you iterate over something that suggests that is a single value, ie. for language in args.remediation_type looks worse than for language in args.remediation_types.

Then, you have an inconsistency - it's a "Remediation type" for the user but a "language" internally.

Fixed in 64ce401

jan-cerny · 2022-12-12T13:38:03Z

build-scripts/update_cpe_conditionals.py

+    return p.parse_args()
+
+
+def main():


please split the main into multiple smaller functions

fixed in: c198cc2

jan-cerny · 2022-12-12T14:07:12Z

shared/templates/platform_os_linux/ansible.template

+    ansible_distribution_version is version("{{{ spec.ver }}}", "{{{ spec.op }}}")
+  {{% endfor %}}
+{{%- endmacro -%}}
+{{%- if SPECS|length() >= 1 -%}}


What is SPECS and where is it defined?

I am sorry for confusion, I removed this part of the code. It will be brought in later when the support for versions is implemented.

jan-cerny · 2022-12-12T14:29:49Z

build-scripts/update_cpe_conditionals.py

+    for language in args.remediation_type:
+        for platform_file in os.listdir(args.platforms_dir):


Is it possible to swap the loops so that the YAML file is loaded only once?

fixed in c198cc2

jan-cerny · 2022-12-12T14:34:14Z

build-scripts/update_cpe_conditionals.py

+import ssg.build_remediations as remediations
+from ssg.build_cpe import ProductCPEs, CPEALFactRef
+import ssg.environment
+


The script looks like the logic could be integrated into other build scripts. Have you considered that? For example, you could load the bash/ansible conditional into platforms at the moment in which these are resolved.

I am currently exploring a possibility to merge it into build_templated_content.py build script.

jan-cerny · 2022-12-12T14:40:06Z

shared/templates/platform_os_linux/ansible.template

+{{%- set ansible_os_release_name_cond = 'ansible_distribution == "' + OS_ID_ANSIBLE + '"' -%}}
+{{%- macro ansible_os_release_conditional(specs) -%}}
+  {{%- for spec in specs -%}}
+    ansible_distribution_version is version("{{{ spec.ver }}}", "{{{ spec.op }}}")


I assume that this means that this PR depends on the versions feature introduced in #9945. Is that correct?

No, sorry for confusion. I force pushed the last commit and removed version support from Ansible remediation.

vojtapolasek · 2022-12-14T12:32:46Z

@jan-cerny I have rewritten the PR. There is no additional build phase now, everything happens while templating content in template.py.

jan-cerny

I have tried to use the os_linux platform in multiple rules in Fedora content and various combinations of it and it all works.

The changes look good to me.

Unfortunately, we have some CI problem with testing farm CS 9 job, but it's clearly unrelated to this PR and the problem appears also in other PRs that are recently triggered CI, eg. #9968.

jan-cerny · 2022-12-14T14:52:19Z

/test e2e-aws-ocp4-high-node

…rameters passed to platform up to now the creation of new CPE items was unconditional it means that even if resolved CPE items already existed, they got rewritten whenever the platform was being loaded. This is not desired - each CPE item should get resolved just once.

…ciated CPE items This conditional is later used when creating remediations.

previously, the representation was sometimes stored as string, sometimes as XML ElementTree. Platforms need to be saved and loaded as needed and so this needs to be unified.

originally, the function did also the file writing but I need to split it so that I can better influence what to do with the result of templating. In case of platforms and CPE items, I do not want the filled template to be written into a file, I want to incorporate it into the CPE item / template and save them to disk as needed. This commit changes tthe function and adapt templating of rules to the new API. Templating of platforms and CPEs will be handled in separate commit.

make sure that as soon as the template is filled, it is inserted into the CPE item at runtime and also the CPE item is stored to disk.

…items update their templated conditionals This ensures that changes to conditionals of templated CPE items propagate to platforms. Platforms are also stored to disk because they are loaded in later build stages.

add associated template together with Ansible remediation. Currently, only the OS name can be specified.

result of rebase against master which brings support for versions

vojtapolasek · 2022-12-16T09:16:39Z

Rebased against master after the support for specifying versions within platforms has been merged.
Currently the os_linux is unversioned. I will add versions in subsequent PR not to polute this PR.

codeclimate · 2022-12-16T10:00:12Z

Code Climate has analyzed commit 3469d5e and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 40.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 49.8% (-0.1% change).

View more on Code Climate.

jan-cerny · 2022-12-16T10:25:15Z

/retest

openshift-ci · 2022-12-16T11:40:00Z

@vojtapolasek: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-ocp4-stig	`3469d5e`	link	true	`/test e2e-aws-ocp4-stig`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

jan-cerny

I have tried to use the os_release platform in multiple rules in Fedora content and then I tried to run the scan and I also use it in some of the complex platform expression together with the platform expressions. The applicability evaluation always ended as I expected.

vojtapolasek added Infrastructure Our content build system CPE-AL CPE Applicability Language labels Dec 12, 2022

vojtapolasek added this to the 0.1.66 milestone Dec 12, 2022

jan-cerny reviewed Dec 12, 2022

View reviewed changes

vojtapolasek force-pushed the cpe_os_linux_template branch 2 times, most recently from 910f88b to 096ad52 Compare December 13, 2022 10:54

jan-cerny self-assigned this Dec 14, 2022

vojtapolasek force-pushed the cpe_os_linux_template branch from c198cc2 to 54afd39 Compare December 14, 2022 12:29

jan-cerny approved these changes Dec 14, 2022

View reviewed changes

vojtapolasek added 9 commits December 16, 2022 10:04

add a new function to platforms which collects conditionals from asso…

946afe1

…ciated CPE items This conditional is later used when creating remediations.

always store the xml representation of platform as string

ba9bb5e

previously, the representation was sometimes stored as string, sometimes as XML ElementTree. Platforms need to be saved and loaded as needed and so this needs to be unified.

add conditional setter to CPEItem

476541b

save templated remediations into CPE items

a74bf6f

make sure that as soon as the template is filled, it is inserted into the CPE item at runtime and also the CPE item is stored to disk.

add new parametrized platform os_linux

7cdf7bd

add associated template together with Ansible remediation. Currently, only the OS name can be specified.

make the os_linux unversioned (so far)

3469d5e

result of rebase against master which brings support for versions

vojtapolasek force-pushed the cpe_os_linux_template branch from 54afd39 to 3469d5e Compare December 16, 2022 09:14

jan-cerny approved these changes Dec 16, 2022

View reviewed changes

jan-cerny merged commit ea7e78d into ComplianceAsCode:master Dec 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Correctly process templated Ansible conditionals and introduce os_linux platform #9959

Correctly process templated Ansible conditionals and introduce os_linux platform #9959

vojtapolasek commented Dec 12, 2022 •

edited

github-actions bot commented Dec 12, 2022

jan-cerny Dec 12, 2022

vojtapolasek Dec 13, 2022

jan-cerny Dec 12, 2022

vojtapolasek Dec 13, 2022

jan-cerny Dec 12, 2022

vojtapolasek Dec 12, 2022

jan-cerny Dec 12, 2022

vojtapolasek Dec 13, 2022

jan-cerny Dec 12, 2022

vojtapolasek Dec 13, 2022

jan-cerny Dec 12, 2022

vojtapolasek Dec 12, 2022

vojtapolasek commented Dec 14, 2022

jan-cerny left a comment

jan-cerny commented Dec 14, 2022

vojtapolasek commented Dec 16, 2022

codeclimate bot commented Dec 16, 2022

jan-cerny commented Dec 16, 2022

openshift-ci bot commented Dec 16, 2022

jan-cerny left a comment

		for language in args.remediation_type:
		for platform_file in os.listdir(args.platforms_dir):

Correctly process templated Ansible conditionals and introduce os_linux platform #9959

Correctly process templated Ansible conditionals and introduce os_linux platform #9959

Conversation

vojtapolasek commented Dec 12, 2022 • edited

Description:

Rationale:

Review hints:

github-actions bot commented Dec 12, 2022

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

vojtapolasek commented Dec 14, 2022

jan-cerny left a comment

Choose a reason for hiding this comment

jan-cerny commented Dec 14, 2022

vojtapolasek commented Dec 16, 2022

codeclimate bot commented Dec 16, 2022

jan-cerny commented Dec 16, 2022

openshift-ci bot commented Dec 16, 2022

jan-cerny left a comment

Choose a reason for hiding this comment

vojtapolasek commented Dec 12, 2022 •

edited