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

Render components to HTML #10709

Merged
merged 9 commits into from
Jun 21, 2023
Merged

Conversation

jan-cerny
Copy link
Collaborator

@jan-cerny jan-cerny commented Jun 12, 2023

Description:

This commit adds a script render_components.py that renders
mappings from components to rules in HTML format. It's also
added to the pages that we publish online.

Rationale:

The goal is to enable easy sharing of component mappings
outside of the team and simplify the review of component rules
for content developers.

Review Hints:

build the content, and then run
python3 utils/render_components.py $(pwd) /tmp/components
firefox /tmp/components/index.html

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Jun 12, 2023
@openshift-ci
Copy link

openshift-ci bot commented Jun 12, 2023

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

@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

@jan-cerny
Copy link
Collaborator Author

I have rebased this PR on the top of the latest upstream master branch.

I have some additional thoughts:

  • To show meaningful content in the rules section we need to use resolved rules but these are different for each product therefore I have decided to generate the content mapping per-product. Do you think that the current form is useful? Would it be better to have only one page for each component and show all the products and rules in product in that single page instead?
  • It could be useful to have a module for a nice rendering of rule and rule details. We already have multiple parts of code that deals with that but isn't reusable. What do you think?

@jan-cerny jan-cerny added Infrastructure Our content build system Highlight This PR/Issue should make it to the featured changelog. labels Jun 19, 2023
@jan-cerny jan-cerny marked this pull request as ready for review June 19, 2023 09:28
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Jun 19, 2023
@jan-cerny jan-cerny added this to the 0.1.69 milestone Jun 19, 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 please take look at my comments.

utils/template_renderer.py Outdated Show resolved Hide resolved
<h3>Relevant packages:</h3>
{{% if component.packages %}}
<ul>
{{% for p in component.packages %}}
Copy link
Member

Choose a reason for hiding this comment

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

I should suggest spelling out package vs using p for clarity.

utils/render_components.py Show resolved Hide resolved
utils/render_components.py Show resolved Hide resolved
<h3>Relevant groups:</h3>
{{% if component.groups %}}
<ul>
{{% for g in component.groups %}}
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to use group here for clarity?

<h3>Changelog:</h3>
{{% if component.changelog %}}
<ul>
{{% for chg in component.changelog %}}
Copy link
Member

Choose a reason for hiding this comment

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

I might recommend renaming chg to entry or changelog_entry.

@@ -10,3 +10,5 @@ rules:
- service_fapolicyd_enabled
templates:
- file_policy_blocked
changelog:
Copy link
Member

Choose a reason for hiding this comment

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

Do we have this schema documented anywhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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


def create_index_html(output_dir, product_components):
index_path = os.path.join(output_dir, "index.html")
title = "List of components"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
title = "List of components"
title = "List of Components"

Title case

@jan-cerny
Copy link
Collaborator Author

I have created a wrapper function to avoid using the private method, I have used long variable names, changed to use the --root option, add typing hints, changed case title, renamed variables and quote bash variables.

This commit unifies code from multiple similar functions to
a single function `render_template`. This function can be used
to render different Jinja templates. The change prevents
code duplication and allows reusing the code in future.
This change will make the build system actually read the
`changelog` key in component YAML files. The values will be stored
in the Component object, so that it can be read later in application.
This commit adds a script `render_components.py` that renders
mappings from components to rules in HTML format. It's also
added to the pages that we publish online.

The goal is to enable easy sharing of component mappings
outside of the team and simplify the review of component rules
for content developers.
This function is created to avoid referencing the private function
_get_jinja_environment outside of the module where it is defined.
This unifies the command line interface with other similar
scripts, eg. rule_dir_json.py.
@codeclimate
Copy link

codeclimate bot commented Jun 21, 2023

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

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

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

View more on Code Climate.

@Mab879 Mab879 merged commit 249781e into ComplianceAsCode:master Jun 21, 2023
32 of 34 checks passed
@jan-cerny jan-cerny deleted the components4 branch June 23, 2023 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Highlight This PR/Issue should make it to the featured changelog. Infrastructure Our content build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants