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

feat: several role improvements #8

Merged
merged 2 commits into from
Dec 6, 2023

Conversation

richm
Copy link
Contributor

@richm richm commented Nov 28, 2023

When adding files to the trustdb, wait until the server recognizes
that the trustdb is updated before returning.

Do not use compiled C programs for testing trust, just use
executable shell scripts.

Clean up after tests

Refactor the check of various EL versions for fapolicyd features - did not work correctly on CentOS and Fedora

make the default value for fapolicyd_setup_enable_service to be true - not sure why anyone would run the role but not want fapolicyd running - goes against every other role that manages a service

Signed-off-by: Rich Megginson [email protected]

@richm richm requested a review from spetrosi as a code owner November 28, 2023 21:00
@richm richm force-pushed the fix-test-cleanup branch 2 times, most recently from c5bc0c8 to e00a462 Compare November 29, 2023 00:32
@richm richm changed the title refactor: several role improvements feat: several role improvements Nov 29, 2023
@richm
Copy link
Contributor Author

richm commented Nov 29, 2023

@radosroka @Koncpa I realize this is a pretty big change - but with these changes, tests pass on every platform, both upstream and in our various downstream test suites

@richm
Copy link
Contributor Author

richm commented Nov 29, 2023

oh - and another change - AFAICT the tests do not need compiled binary executables - they just need executable files - the gcc and glibc packages take a few minutes to install over a slow connection, so better to just skip them - all we really care about is that fapolicyd allows the specified executable to be run, and disallows executables with a different name or checksum

@spetrosi
Copy link
Contributor

[citest]

2 similar comments
@richm
Copy link
Contributor Author

richm commented Nov 29, 2023

[citest]

@richm
Copy link
Contributor Author

richm commented Nov 29, 2023

[citest]

When adding files to the trustdb, wait until the server recognizes
that the trustdb is updated before returning.

Do not use compiled C programs for testing trust, just use
executable shell scripts.

Set vars such as __fapolicyd_trust_supported et. al. based on
os family and version rather than using distribution version.
One reason is that CentOS Stream was excluded from many features
but it should be included.

Clean up after tests

Other minor improvements

Signed-off-by: Rich Megginson <[email protected]>
@richm richm requested review from radosroka and Koncpa November 29, 2023 16:48
@richm
Copy link
Contributor Author

richm commented Dec 5, 2023

@Koncpa @radosroka @xjezda00 It seems that systemctl restart fapolicyd is the only reliable way to update the trustdb. When removing files without adding files, fapolicyd-cli --update does nothing on RHEL 8.8.

@richm
Copy link
Contributor Author

richm commented Dec 5, 2023

[citest]

@Koncpa Koncpa self-requested a review December 5, 2023 16:50
@richm richm merged commit 243dbed into linux-system-roles:main Dec 6, 2023
22 checks passed
@richm richm deleted the fix-test-cleanup branch December 6, 2023 14:19
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.

4 participants