Skip to content
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.

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

Implement a tool for parsing profiles and outputing rules #10455

Merged
merged 1 commit into from
May 9, 2023

Conversation

rhmdnd
Copy link
Collaborator

@rhmdnd rhmdnd commented Apr 11, 2023

This adds the basic skeleton of a tool that parses CIS profiles using an
XLSX parser and converts controls to rules that we can use in OpenSCAP.

The primary purpose is to help maintain CIS profiles by generating most
of the boilerplate rule template.

@github-actions
Copy link

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

Fedora Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

@Mab879 Mab879 self-assigned this Apr 11, 2023
@Mab879 Mab879 added the Infrastructure Our content build system label Apr 11, 2023
Copy link
Member

@Mab879 Mab879 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

I have few other items that are not in my code comments:

  • Please take look at the Code Climate findings. Especially the PEP8 findings.
  • Add some documentation about this script in docs/manual/developer/05_tools_and_utilities.md. Explaining what format of spreadsheets is needed would be helpful.

utils/generate_profile.py Show resolved Hide resolved
utils/generate_profile.py Outdated Show resolved Hide resolved

yaml.add_representer(literal_unicode, literal_unicode_representer)

parser = argparse.ArgumentParser(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please move this to a method

utils/generate_profile.py Outdated Show resolved Hide resolved
utils/generate_profile.py Outdated Show resolved Hide resolved
@rhmdnd
Copy link
Collaborator Author

rhmdnd commented Apr 19, 2023

Thanks for the review @Mab879 - I still need to get the documentation ironed out.

def parse(self):
return NotImplemented

class Section:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Section, Control, and Benchmark classes really could be their own library. That'd actually be pretty useful in other scripts, too.

self.description = None
self.level = None
self.remediation = None
self.rationale = None
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to bring a slack conversation over here and tag @Mab879 - would it be a good idea to merge this class with the control class from srg/controls.py?

Copy link
Collaborator Author

@rhmdnd rhmdnd Apr 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is another version of this in #10469

@Mab879 what are your thoughts on a shared library that modes a benchmark?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that shared library would sense.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok - I'll help get the deps squared away in #10474 and #10473. Then I'll start pulling the common bits into a shared library that we can import as a python-specific requirement.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I threw together a quick library https://github.com/rhmdnd/pycompliance

rhmdnd added a commit to rhmdnd/pycompliance that referenced this pull request Apr 26, 2023
This is a port from a patch proposed to another project:

  ComplianceAsCode/content#10455

Documentation and release will come after initial round of feedback.

import json
import yaml
from pycompliance import pycompliance
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is based on the latest content in https://github.com/rhmdnd/pycompliance

If folks are ok with that approach, I'll package it and release to PyPI.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it correct to say that this library will be focused on making compliance content in various formats (xslx, pdf) available in CaC content formats (i.e. controls, profiles, rule in yaml)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure if we'd want to put all the parsing in the pycompliance library. I kept it pretty limited so that it only focused on the common parts of my script and the script from #10469

@rhmdnd
Copy link
Collaborator Author

rhmdnd commented Apr 27, 2023

The tool currently outputs the following if you have the benchmark available locally:

$ python utils/generate_profile.py -i ~/Downloads/CIS_RedHat_OpenShift_Container_Platform_Benchmark_v1.3.0.xlsx -p ocp generate -c 2.3
documentation_complete: false
prodtype: ocp
title: |-
  Ensure that the --auto-tls argument is not set to true
description: |-
  Do not use self-signed certificates for TLS.
rationale: |-
  etcd is a highly-available key value store used by Kubernetes deployments for persistent storage of all of its REST API objects. These objects are sensitive in nature and should not be available to unauthenticated clients. You should enable the client authentication via valid certificates to secure the access to the etcd service.
severity: PLACEHOLDER
references: PLACEHOLDER
ocil: PLACEHOLDER
ocil_clause: PLACEHOLDER
warnings: PLACEHOLDER
template: PLACEHOLDER

utils/generate_profile.py Outdated Show resolved Hide resolved
@rhmdnd rhmdnd force-pushed the profile-parser-script branch 3 times, most recently from 10df58c to 5d30159 Compare April 27, 2023 20:50
Copy link
Member

@yuumasato yuumasato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like and support the direction the library and script are going

raise NotImplemented


class XLSXParser(Parser):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the XLSXParser should actually be in pycompliance library?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be. I figured once we take that step, we may get asked to support parsing additional formats (PDFs).

Comment on lines +78 to +88
cols = [
'section #',
'recommendation #',
'profile',
'title',
'assessment status',
'description',
'remediation procedure',
'rationale statement',
'audit procedure']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And in the future these columns would be easily customizable for other spreadsheets, not only CIS.


import json
import yaml
from pycompliance import pycompliance
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it correct to say that this library will be focused on making compliance content in various formats (xslx, pdf) available in CaC content formats (i.e. controls, profiles, rule in yaml)?

Copy link
Member

@yuumasato yuumasato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Mab879 Do you have anything to add?

@Mab879
Copy link
Member

Mab879 commented May 3, 2023

@Mab879 Do you have anything to add?

I would like some basic docs before this merged.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Used by openshift-ci bot. label May 3, 2023
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Used by openshift-ci bot. label May 3, 2023
@Mab879 Mab879 added this to the 0.1.68 milestone May 4, 2023
Copy link
Member

@Mab879 Mab879 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good other than that one code climate issue. Once that is fixed we should be good to merge.

This adds the basic skeleton of a tool that parses CIS profiles using an
XLSX parser and converts controls to rules that we can use in OpenSCAP.

The primary purpose is to help maintain CIS profiles by generating most
of the boilerplate rule template.
@rhmdnd
Copy link
Collaborator Author

rhmdnd commented May 4, 2023

Looks good other than that one code climate issue. Once that is fixed we should be good to merge.

Fixed - thanks for the reviews!

Copy link
Member

@Mab879 Mab879 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@codeclimate
Copy link

codeclimate bot commented May 4, 2023

Code Climate has analyzed commit 1c5812d and detected 0 issues on this pull request.

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

This pull request will bring the total coverage in the repository to 52.4% (0.0% change).

View more on Code Climate.

@rhmdnd
Copy link
Collaborator Author

rhmdnd commented May 4, 2023

Thanks!

I'll get a pycompliance release on PyPI tomorrow and we can iterate there.

@Mab879
Copy link
Member

Mab879 commented May 4, 2023

/packit build

@Mab879 Mab879 merged commit b47f3c2 into ComplianceAsCode:master May 9, 2023
32 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Our content build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants