-
-
Notifications
You must be signed in to change notification settings - Fork 148
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
BREAKING: Redesign selinux::module defined type #195
BREAKING: Redesign selinux::module defined type #195
Conversation
seems fine to me. Does the single-dir-building still work idempotently and correctly remove the sources when puppet stops managing a resource? |
manifests/module.pp
Outdated
@@ -44,11 +53,11 @@ | |||
validate_absolute_path($::selinux::config::module_build_dir) | |||
validate_absolute_path($::selinux::refpolicy_makefile) | |||
|
|||
$module_dir = "${::selinux::config::module_build_dir}/${title}" | |||
$module_dir = $::selinux::config::module_build_dir | |||
$module_file = "${module_dir}/${title}" | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$::selinux::config::module_build_dir
should be $::selinux::module_build_dir
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's another instance in validate_absolute_path that didn't get highlighted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
::selinux::config::module_build_dir
!= ::selinux::module_build_root
and ::selinux::module_build_dir
does not exist.
you created this var here: https://github.com/voxpupuli/puppet-selinux/pull/189/files#diff-ca85ef38c9b99f20d2d1c8d646a10e77R47 - it confused me too know. maybe we could come up with something more intuitive? ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, yeah, the naming is too close together... Hm. is module_source_dir any better?
I suspect a problem when you manage policy A with .te and .fc and then update the manifest to only manage the .te. Then the .fc file will not get removed. |
@vinzent, do you think there's still something this needs, or could this be merged? I think the interface is fine as it is at least for my usecases, and also allows for backwards-compatible extension later on if needed. |
This commit also introduces an alternative 'simple' builder to refpolicy, and consequently the 'selinux-policy-devel' package is not always needed
@vinzent <https://github.com/vinzent>, do you think there's still
something this needs
I think it can be merged.
I've squashed the commits to one for you and one of mine. so history
looks better. what do you think?
|
@vinzent: looks good to me. |
If multiple refpolicy style modules are built and interfaces are used they need to reside in the same directory so the interfaces are found.
added a small fix for the selinux_module_refpolicy acceptance test. Upcoming selinux tools 2.6 (in f26) will not allow to grant usr_t some permissions (which is a good check). Re-run the acceptance tests on CentOS6, CentOS7, Fedora 24 and Fedora 25. All green. |
I added a duplicate label to get this out of the changelog in favour of the issue |
…cy_test Redesign selinux::module defined type
Implements whats discussed in #178.
Extends and closes #189
Closes #178
Closes #146