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

remove obsolete selinux modules #23568

Closed
wants to merge 3 commits into from

Conversation

plsph
Copy link
Contributor

@plsph plsph commented Dec 29, 2021

Add cleanup actions to post installation step by identifying removed modules from OLD_MODS list, that is copy from previous ebuild and remove it from selinux module store if installed.

One must maintain OLD_MODS list on each new ebuild by copying it from previous MODS list.

@gentoo-bot
Copy link

Pull Request assignment

Submitter: @plsph
Areas affected: ebuilds
Packages affected: sec-policy/selinux-base-policy

sec-policy/selinux-base-policy: @gentoo/selinux

Linked bugs

No bugs to link found. If your pull request references any of the Gentoo bug reports, please add appropriate GLEP 66 tags to the commit message and request reassignment.

If you do not receive any reply to this pull request, please open or link a bug to attract the attention of maintainers.

Missing GCO sign-off

Please read the terms of Gentoo Certificate of Origin and acknowledge them by adding a sign-off to all your commits.


In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Docs: Code of ConductCopyright policy (expl.) ● DevmanualGitHub PRsProxy-maint guide

@gentoo-bot gentoo-bot added assigned PR successfully assigned to the package maintainer(s). no bug found No Bug/Closes found in the commits. no signoff One or more commits do not indicate GCO sign-off. labels Dec 29, 2021
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-12-29 20:11 UTC
Newest commit scanned: df04fd5
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/1cd733992d/output.html

Signed-off-by: Grzegorz Filo <gf578@wp.pl>
@plsph plsph force-pushed the remove_obsolete_selinux_modules branch from df04fd5 to e409b5d Compare December 30, 2021 14:13
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-12-30 14:25 UTC
Newest commit scanned: e409b5d
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/1df98198c6/output.html

@perfinion perfinion self-requested a review December 31, 2021 20:26
Copy link
Member

@perfinion perfinion left a comment

Choose a reason for hiding this comment

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

Good catch on the hotplug removal! this looks pretty good, just would prefer a small change which should be easier to maintain going forward :)

Thanks and happy new year!

@@ -27,6 +27,7 @@ BDEPEND="
sys-apps/checkpolicy
sys-devel/m4"

OLD_MODS="application authlogin bootloader clock consoletype cron dmesg fstools getty hostname hotplug init iptables libraries locallogin logging lvm miscfiles modutils mount mta netutils nscd portage raid rsync selinuxutil setrans ssh staff storage su sysadm sysnetwork systemd tmpfiles udev userdomain usermanage unprivuser xdg"
Copy link
Member

Choose a reason for hiding this comment

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

ooh good catch on the hotplug removal. This mostly looks good but i think i'd rather change OLD_MODS to just be the list of modules to remove instead of needing to be a copy of all of them. that'd be less of a maintenance burden i think,
The issue is I have bumps scripted so copying MODS to OLD_MODS is complicated. also I'll keep the modules to remove list for at least a years worth of packages just in case someone does not update regularly and skips some versions

can we change this to OLD_MODS="hotplug" (or maybe DEL_MODS is nicer?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, no problem, that just need to be aligned with upstream to keep DEL_MODS list up to date.

@@ -105,12 +107,26 @@ pkg_postinst() {
COMMAND="${COMMAND} -i ${i}.pp"
done

for i in ${OLD_MODS}; do
if [ -n "${MODS##*$i*}" ]; then
DEL_MODS="${DEL_MODS} ${i}"
Copy link
Member

Choose a reason for hiding this comment

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

Once the line above is changed to just the list of modules to remove then this loop can be changed to die instead.

maybe something like:

for i in ${OLD_MODS}; do
    [[ ${MODS} != *${i}* ]] || die "Duplicate module in MODS and OLD_MODS: ${i}"
done

Signed-off-by: Grzegorz Filo <gf578@wp.pl>
@plsph plsph force-pushed the remove_obsolete_selinux_modules branch from 7d9c2b8 to 922deb3 Compare January 1, 2022 17:12
@plsph
Copy link
Contributor Author

plsph commented Jan 1, 2022

Sorry for force push, I keep forgeting about Certificate of Origin.
This should work as described.
Have a nice new year.

@plsph plsph requested a review from perfinion January 1, 2022 17:16
@@ -27,6 +27,7 @@ BDEPEND="
sys-apps/checkpolicy
sys-devel/m4"

DEL_MODS="hotplug"
Copy link
Contributor

Choose a reason for hiding this comment

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

Tiny nit: I think DEL_MODS should go underneath MODS

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2022-01-01 17:25 UTC
Newest commit scanned: 922deb3
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/2a813a5040/output.html

Signed-off-by: Grzegorz Filo <gf578@wp.pl>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2022-01-01 22:11 UTC
Newest commit scanned: 33e48c2
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/c692464138/output.html

@perfinion
Copy link
Member

About the DCO, it looks like your git is misconfigured, i had to use git commit --amend --author to make the author field match up with your signed-off-by name. Look at git config user.email and user.name I think should do it.

I made some minor changes:

  • Moved the die up into src_prepare, its not good to die in the postinst stage.
  • removed the quotes in [[ "${MODS}" != *${i}* ]] otherwise it doesn't glob and wont match.
  • The if [[ -n ${DEL_MODS} ]] was unnecessary around the for loop.
  • Added to the -9999 package too cuz future bumps copy from the -9999 package

Happy New Year and Thanks for the PR!

@gentoo-bot gentoo-bot closed this in dff6170 Jan 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). no bug found No Bug/Closes found in the commits. no signoff One or more commits do not indicate GCO sign-off.
Projects
None yet
5 participants