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

Add package_* test scenarios #6752

Merged
merged 14 commits into from Apr 6, 2021

Conversation

mildas
Copy link
Contributor

@mildas mildas commented Mar 25, 2021

Description:

Add test scenarios to package_*_installed and to package_*_removed rules from CIS profile.

Each rule has test scenario for installation and test scenario for removal of the package except libselinux. The libselinux package has only installation test scenario because after removal of the package, the system becomes unusable - commands start to print libselinux.so.1: cannot open shared object file: No such file or directory (libselinux is dependency of several core packages, e.g. systemd).

@@ -0,0 +1,3 @@
#!/bin/bash

yum install -y openldap-clients
Copy link
Member

Choose a reason for hiding this comment

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

What about using # packages = openldap-clients? I think it would perform better in case of combined mode as packages from this metadata should be installed at once before running the tests. Is that correct @matejak ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Benefit I see in the implemented approach is that it's clear that the installation is the actual test scenario.

With the # packages = ... approach, the installation will be part of test environment preparation and scenario will be empty (or it might be required to call some command in the scenario like true but nothing relevant to the test).

Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer Milan's approach with this test

Copy link
Member

Choose a reason for hiding this comment

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

Benefit I see in the implemented approach is that it's clear that the installation is the actual test scenario.

With the # packages = ... approach, the installation will be part of test environment preparation and scenario will be empty (or it might be required to call some command in the scenario like true but nothing relevant to the test).

Yes, my idea was to have it like that as fetching yum database in every test scenario will significantly increase the tests run time when testing all rules from a profile, for example in combined mode. Isn't it actually obvious from the filename of the test scenario that it is about installation/removal?
To summarize, I am not strictly against this approach, but I would still prefer if we would use the packages metadata. I think that test suite run times are pretty high and I don't want to increase them even more if we don't have to.

Copy link
Member

Choose a reason for hiding this comment

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

I see both sides, and I think that there may be a solution - if you declare that you need the yum package, the test environment will have the package metadata up-to-date, so it will be only the installation that takes time for that scenario, and that's IMO OK. Moreover, the test scenario clearly needs yum, so that's not a hack.
Without that, I agree that every scenario would fetch the metadata again, which I see as a great waste of time and of electrical power.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting idea. I have added that in last commit, can you confirm that it's what you meant?

Copy link
Member

Choose a reason for hiding this comment

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

@mildas Yes, thank you for updating test scenarios and thanks to @matejak for this clever idea :)

…arios where we want to test installation of a package
@matusmarhefka matusmarhefka self-assigned this Apr 6, 2021
@matusmarhefka matusmarhefka added this to the 0.1.56 milestone Apr 6, 2021
@matusmarhefka matusmarhefka merged commit 6c26666 into ComplianceAsCode:master Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants