[Freeipa-devel] [PATCHES 523-525] replica install: add remote connection check over API

[Jan Cholasta]

On 14.12.2015 10:27, Martin Basti wrote:
>
>
> On 14.12.2015 07:23, Jan Cholasta wrote:
>> On 11.12.2015 18:49, Tomas Babej wrote:
>>>
>>>
>>> On 12/11/2015 05:37 PM, Martin Basti wrote:
>>>>
>>>>
>>>> On 11.12.2015 15:40, Jan Cholasta wrote:
>>>>> On 11.12.2015 08:03, Jan Cholasta wrote:
>>>>>> On 11.12.2015 07:08, Jan Cholasta wrote:
>>>>>>> On 10.12.2015 15:56, Martin Babinsky wrote:
>>>>>>>> On 12/10/2015 09:48 AM, Jan Cholasta wrote:
>>>>>>>>> On 9.12.2015 16:38, Jan Cholasta wrote:
>>>>>>>>>> On 9.12.2015 14:52, Jan Cholasta wrote:
>>>>>>>>>>> On 9.12.2015 10:02, Jan Cholasta wrote:
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> the attached patches fix
>>>>>>>>>>>> <https://fedorahosted.org/freeipa/ticket/5497>.
>>>>>>>>>>>
>>>>>>>>>>> Note that this needs selinux-policy fix to work, so put SELinux
>>>>>>>>>>> into
>>>>>>>>>>> permissive mode for testing:
>>>>>>>>>>> <https://bugzilla.redhat.com/show_bug.cgi?id=1289930>.
>>>>>>>>>>
>>>>>>>>>> Updated patches attached.
>>>>>>>>>
>>>>>>>>> I screwed up a change in patch 524 and accidentally included a
>>>>>>>>> chunk of
>>>>>>>>> code in patch 525 that doesn't belong in it.
>>>>>>>>>
>>>>>>>>> Updated patches attached.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> Patches work as expected and I was not able to find any functional
>>>>>>>> problem.
>>>>>>>>
>>>>>>>> I have a question about the naming of the oddjob helper script: the
>>>>>>>> one
>>>>>>>> related to trusts is named 'com.redhat.idm.trust-fetch-domains',
>>>>>>>> and the
>>>>>>>> conncheck runner is named 'org.freeipa.server.conncheck'. I
>>>>>>>> don't want
>>>>>>>> to start another bikeshedding conversation but shouldn't we
>>>>>>>> named them
>>>>>>>> in a consistent fashion (either rename the first one in separate
>>>>>>>> patch
>>>>>>>> or rename the new helper to com.redhat.idm.server.conncheck)?
>>>>>>>>
>>>>>>>> I understand that as an upstream, we should go with the
>>>>>>>> 'org.freeipa.*'
>>>>>>>> convention, but having two helpers with different prefixes makes me
>>>>>>>> sad.
>>>>>>>
>>>>>>> If you look at the larger picture, org.freeipa is the consistent
>>>>>>> name.
>>>>>>> It makes me sad as well, but mistakes should be corrected. This is
>>>>>>> similar to how we use PEP8 in new code, but do not fix it in old
>>>>>>> code
>>>>>>> just for the sake of fixing it.
>>>>>>>
>>>>>>>>
>>>>>>>> That is a nitpick though, it does not affect the overall
>>>>>>>> functionality
>>>>>>>> of the patches so ACK.
>>>>>>>
>>>>>>> Thanks for the review. The current patch 523 breaks the trusts
>>>>>>> oddjob
>>>>>>> with SELinux in enforcing mode, I will send an update which corrects
>>>>>>> that, until bug 1289930 is fixed.
>>>>>>
>>>>>> Updated patches attached.
>>>>>
>>>>> Rebased on top of current master.
>>>>>
>>>>>
>>>>>
>>>> Just question, should be any kinited user allowed to run conncheck
>>>> via rpc?
>>>>
>>>> Martin^2
>>>
>>> I guess there's is little harm, any kinited user that was allowed to
>>> access the machine could perform the conncheck even without these
>>> patches:
>>
>> In the RPC check, the user must have the Replication Administrators
>> privilege, which by default only admins have.
>
> I tried to install replica with a regular user and conncheck passed.
> Martin^2

See patch 525.

-- 
Jan Cholasta