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

Build profile bash scripts differently #11028

Merged
merged 4 commits into from
Aug 28, 2023

Conversation

jan-cerny
Copy link
Collaborator

Description:

This commit introduces a new script to generate profile oriented Bash remediation scripts. It replaces the current way that uses the oscap tool under the hood. The new script doesn't use oscap. It processes the data stream directly and it generates the remediation for all profiles at a single pass. The new script is faster than the current script by 80 %.

Rationale:

  • faster build
  • reduces dependency of our project on oscap

Review Hints:

Build content from the master branch and from this PR branch. Compare the files in the build/bash directory in the old and new version using tools like meld or diff.

@jan-cerny jan-cerny added enhancement General enhancements to the project. Infrastructure Our content build system labels Aug 25, 2023
@jan-cerny jan-cerny added this to the 0.1.70 milestone Aug 25, 2023
@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
Copy link
Member

Mab879 commented Aug 25, 2023

/packit retest-failed

This commit introduces a new script to generate profile oriented
Bash remediation scripts. It replaces the current way that uses
the `oscap` tool under the hood. The new script doesn't use `oscap`.
It processes the data stream directly and it generates the remediation
for all profiles at a single pass. The new script is faster than the
current script by 80 %.
@jan-cerny
Copy link
Collaborator Author

I have resolved Code Climate problems and I have rebased this PR on the top of the latest upstream master branch.

@Mab879 Mab879 self-assigned this Aug 25, 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.

The scripts do create identical files, but I do have a couple of questions.

@@ -0,0 +1,258 @@
#!/usr/bin/python3
Copy link
Member

Choose a reason for hiding this comment

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

This isn't useful since the script is 644.

commented_profile_description += commented_line
profile_id = profile.get("id")
xccdf_version_name = "1.2"
oscap_version = "1.3.8"
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 really want to hard code this?

"# Benchmark Version: %s\n"
"# XCCDF Version: %s\n"
"#\n"
"# This file was generated by OpenSCAP %s using:\n"
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how I feel about this. This is not true.

Maybe we want to change "can be generated with"

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 intentionally left it there to keep it identical and I was wondering if there will be any opinion on it. I agree with you, I will improve it in the next commit.

This commit will cause that the generated Bash script will be
slightly different than the Bash script generated by the
`oscap xccdf generate fix command`. It changes the header.
The reason is that we shouldn't say that the script has been
generated by OpenSCAP, that isn't true.
@jan-cerny
Copy link
Collaborator Author

I have add executable permission and improved the remediation header

@codeclimate
Copy link

codeclimate bot commented Aug 28, 2023

Code Climate has analyzed commit fc7698b and detected 2 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 2

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 53.3% (0.0% change).

View more on Code Climate.

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.

LGTM. Thanks for the PR!

@Mab879
Copy link
Member

Mab879 commented Aug 28, 2023

/packit retest-failed

@Mab879 Mab879 merged commit f9ee016 into ComplianceAsCode:master Aug 28, 2023
37 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement General enhancements to the project. Infrastructure Our content build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants