[Freeipa-devel] [PATCH 0027] Add checks for SELinux in install scripts

[Tomas Babej]

On Tue 19 Feb 2013 08:37:26 PM CET, Rob Crittenden wrote:
> Tomas Babej wrote:
>> On 02/04/2013 04:21 PM, Rob Crittenden wrote:
>>> Tomas Babej wrote:
>>>> On 01/30/2013 05:12 PM, Tomas Babej wrote:
>>>>> Hi,
>>>>>
>>>>> The checks make sure that SELinux is:
>>>>>   - installed and enabled (on server install)
>>>>>   - installed and enabled OR not installed (on client install)
>>>>>
>>>>> Please note that client installs with SELinux not installed are
>>>>> allowed since freeipa-client package has no dependency on SELinux.
>>>>> (any objections to this approach?)
>>>>>
>>>>> The (unsupported) option --allow-no-selinux has been added. It can
>>>>> used to bypass the checks.
>>>>>
>>>>> Parts of platform-dependant code were refactored to use newly added
>>>>> is_selinux_enabled() function.
>>>>>
>>>>> https://fedorahosted.org/freeipa/ticket/3359
>>>>>
>>>>> Tomas
>>>>
>>>> I forgot to edit the man pages. Thanks Rob!
>>>>
>>>> Updated patch attached.
>>>>
>>>> Tomas
>>>
>>> After a bit of off-line discussion I don't think we're quite ready yet
>>> to require SELinux by default on client installations (even with a
>>> flag to work around it). The feeling is this would be disruptive to
>>> existing automation.
>>>
>>> Can you still do the check but not enforce it, simply display a big
>>> warning if SELinux is disabled?
>>>
>>> rob
>>>
>>
>> Sure, here is the updated patch.
>>
>> I edited the commit message, RFE description and man pages according to
>> the new behaviour.
>>
>> Tomas
>
> The patch looks good, I'm just wondering about one thing. The default
> value for is_selinux_enabled() is True in ipapython/services.py.in.
>
> So this means that any non-Red Hat/non-Fedora system, by default, is
> going to assume that SELinux is enabled.
>
> My hesitation has to when we call check_selinux_status(). It may
> incorrectly error out. I suspect that the user would have to work
> around this using --allow-selinux-disabled but this wouldn't make a
> lot of sense since they actually do have SELinux disabled.

Yes, you're right. And the error message would not even be helpful since
it would tell the user to install policycoreutils package. This would 
be the
case both with server and client installs when selinux would not be 
installed
at all.

> What do you think?
>
> rob

Well we have 2 options as I see it:

1.) We can either return None as default, and add checks to
check_selinux_status, restore_context and install scripts that would
ensure that we behave properly when is_selinux_enabled() is not
implemented.

2.) We can remove the default value, since it would cause forementioned
crash and add comment that this function needs to be implemented
properly in every platform file.

I'm probably for option 2, there's no need to clutter the code with 
checks
that compensate for improper platform file implementations.

Tomas