# SELinux Module RFC

**URL:** <https://forum.ansible.com/t/selinux-module-rfc/11841>\
**Category:** Ansible Project\
**Created:** [August 29, 2012, 10:29pm UTC](https://forum.ansible.com/t/selinux-module-rfc/11841 "2012-08-29T22:29:33Z")\
**Posts on this page:** 5\
**Page:** 1

<div class="post-metadata">

**Author:** ![Derek\_Carter1](https://avatars.discourse-cdn.com/v4/letter/d/5fc32e/32.png) [@Derek\_Carter1](https://forum.ansible.com/u/Derek_Carter1)\
**Post date:** [August 29, 2012, 10:29pm UTC](https://forum.ansible.com/t/selinux-module-rfc/11841/1 "2012-08-29T22:29:33Z")

</div>

I've written the first-draft of an SELinux module. I'm not sure it's  
ready for a pull request, but I would like your feedback.

Let's call it a code review if you like.

Here's the code:  
[https://github.com/goozbach/ansible/blob/selinux\_module/library/selinux](https://github.com/goozbach/ansible/blob/selinux_module/library/selinux)

---

<div class="post-metadata">

**Author:** ![Mark\_Theunissen](https://avatars.discourse-cdn.com/v4/letter/m/898d66/32.png) [@Mark\_Theunissen](https://forum.ansible.com/u/Mark_Theunissen)\
**Post date:** [August 29, 2012, 10:53pm UTC](https://forum.ansible.com/t/selinux-module-rfc/11841/2 "2012-08-29T22:53:18Z")

</div>

Hi Derek,

This part looks strange:

```auto

except ImportError:
    HAVE_SELINUX=False

    fail_json(msg='python-selinux required for this module')

```

I don’t think you’ll be able to call fail\_json like that, since it’s part of module\_common now. You should check for HAVE\_SELINUX in main() and fail there.

Try this:

try:  
import selinux  
except ImportError:  
HAVE\_SELINUX = False  
else:  
HAVE\_SELINUX = True

- Mark

---

<div class="post-metadata">

**Author:** ![Michael\_DeHaan](https://avatars.discourse-cdn.com/v4/letter/m/c4cdca/32.png) [@Michael\_DeHaan](https://forum.ansible.com/u/Michael_DeHaan)\
**Post date:** [August 29, 2012, 11:09pm UTC](https://forum.ansible.com/t/selinux-module-rfc/11841/3 "2012-08-29T23:09:58Z")

</div>

Looks really good.

Random comments.

1) the usage of 'with' requires python 2.6, which breaks our  
self-imposed requirement that modules be written to support python 2.4  
and later for EL 5 users. So alas, you must do open/close instead.

2) it seems a bit too restrictive to restrict the list of SELinux  
policy types to the ones that just Red Hat has provided, so I'd  
probably take that out

3) I would probably not set a default policy or mode, and require the  
user pick (required=True) so they are sure to make concious decisions  
and not be surprised.

4) minor style things, mostly my preferences

s/CHANGED/changed/, as it's not a constant  
kill the spaces inside the parens  
intra-line whitespace seperating different if-clauses is desirable  
don't hide a useful side effect in a logging-related operation where  
you have -- msgs.append(set\_policy(policy)), assign the variable and  
then add to msgs

What Mark says is true, though you could simply do: json.dumps({...}  
and sys.exit() at the time of import and it would be fine.

(Pretty sure set\_policy is just going to require the reboot, which you  
could just handle with a notify -- though you'd have to notify both  
the reboot and a delegated wait\_for if you wanted to be 100%  
foolproof, and that might be somewhat fiddly.)

5) Not required, but a type to manage selinux booleans seems pretty  
easy (getsebool/setsebool) under the covers and you usually have to  
fiddle with these more often than even the basic SELinux setup (which  
is often done once and only once at kickstart time). (This feels like  
a seperate module)

Good stuff!

--Michael

---

<div class="post-metadata">

**Author:** ![Stephen\_Fromm](https://avatars.discourse-cdn.com/v4/letter/s/bbce88/32.png) [@Stephen\_Fromm](https://forum.ansible.com/u/Stephen_Fromm)\
**Post date:** [August 29, 2012, 11:17pm UTC](https://forum.ansible.com/t/selinux-module-rfc/11841/4 "2012-08-29T23:17:16Z")

</div>

I’ve been sitting on a seboolean module to manage selinux booleans. It could use a little clean up before submitting. I can throw it somewhere for more eyeballs.

sf

---

<div class="post-metadata">

**Author:** ![Derek\_Carter1](https://avatars.discourse-cdn.com/v4/letter/d/5fc32e/32.png) [@Derek\_Carter1](https://forum.ansible.com/u/Derek_Carter1)\
**Post date:** [August 30, 2012, 5:06pm UTC](https://forum.ansible.com/t/selinux-module-rfc/11841/5 "2012-08-30T17:06:02Z")

</div>

> Looks really good.
> 
> 1) the usage of 'with' requires python 2.6, which breaks our  
> self-imposed requirement that modules be written to support python  
> 2.4 and later for EL 5 users. So alas, you must do open/close  
> instead.

Fixed

> 2) it seems a bit too restrictive to restrict the list of SELinux  
> policy types to the ones that just Red Hat has provided, so I'd  
> probably take that out
> 
> 3) I would probably not set a default policy or mode, and require the  
> user pick (required=True) so they are sure to make concious decisions  
> and not be surprised.

Good call, removed choices and made it required.

> (Pretty sure set\_policy is just going to require the reboot, which you could  
> just handle with a notify -- though you'd have to notify both the  
> reboot and a delegated wait\_for if you wanted to be 100% foolproof,  
> and that might be somewhat fiddly.)

I'm pretty sure I would want to leave the reboot out of this module and  
make it a standard practice when dealing with the playbook changing of  
the selinux policy/state. (ie the documentation shows how to do reboot  
and delegated wait\_for).