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

[Jan Cholasta]

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.

-- 
Jan Cholasta
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-523.3-build-put-oddjob-scripts-into-separate-directory.patch
Type: text/x-patch
Size: 1690 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151211/dd807911/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-524.3-replica-install-add-remote-connection-check-over-API.patch
Type: text/x-patch
Size: 29120 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151211/dd807911/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-525.3-replica-promotion-use-host-credentials-for-connectio.patch
Type: text/x-patch
Size: 2344 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151211/dd807911/attachment-0002.bin>