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

[Jan Cholasta]

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.

-- 
Jan Cholasta
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-523.4-build-put-oddjob-scripts-into-separate-directory.patch
Type: text/x-patch
Size: 1625 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151211/a4135906/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-524.4-replica-install-add-remote-connection-check-over-API.patch
Type: text/x-patch
Size: 28609 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151211/a4135906/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-525.4-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/a4135906/attachment-0002.bin>