[Freeipa-devel] [PATCH] 0203 adtrust: remove ipanttrustpartner parameter

[Lukas Slebodnik]

On (10/06/16 11:01), Martin Kosek wrote:
>On 06/10/2016 10:01 AM, Martin Basti wrote:
>> 
>> 
>> On 09.06.2016 21:45, Alexander Bokovoy wrote:
>>> On Thu, 09 Jun 2016, Martin Basti wrote:
>>>>
>>>>
>>>> On 09.06.2016 17:56, Martin Babinsky wrote:
>>>>> On 06/06/2016 01:37 PM, Alexander Bokovoy wrote:
>>>>>> On Mon, 06 Jun 2016, Jan Cholasta wrote:
>>>>>>> On 6.6.2016 13:22, Martin Basti wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 06.06.2016 13:14, Alexander Bokovoy wrote:
>>>>>>>>> On Mon, 06 Jun 2016, Martin Basti wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 06.06.2016 12:36, Alexander Bokovoy wrote:
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> MS-ADTS spec requires that TrustPartner field should be equal to the
>>>>>>>>>>> commonName (cn) of the trust. We used it a bit wrongly to express
>>>>>>>>>>> trust relationship between parent and child domains. In fact, we
>>>>>>>>>>> have parent-child relationship recorded in the DN (child domains
>>>>>>>>>>> are part of the parent domain's container).
>>>>>>>>>>>
>>>>>>>>>>> Remove the argument that was never used externally but only
>>>>>>>>>>> supplied by
>>>>>>>>>>> trust-specific code inside the IPA framework.
>>>>>>>>>>>
>>>>>>>>>>> Part of https://fedorahosted.org/freeipa/ticket/5354
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Hello, how is handled backward compatibility here, you just removes
>>>>>>>>>> the option from API, without any additional logic for older clients.
>>>>>>>>> This is not used by the external clients at all. It is part of internal
>>>>>>>>> logic of the code in trust.py+com.redhat.trust.fetch-domains which
>>>>>>>>> always talk to the same server they are running on.
>>>>>>>>>
>>>>>>>>> @register()
>>>>>>>>> class trustdomain_add(LDAPCreate):
>>>>>>>>>  __doc__ = _('Allow access from the trusted domain')
>>>>>>>>>  NO_CLI = True
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> Yes sorry, not old IPA clients, but it was part of API, shown in API
>>>>>>>> browser, and since this was in API, it is set to stone. So If you think
>>>>>>>> that it is safe to be removed and nobody can hit this, I'm okay for
>>>>>>>> removing that option. Maybe we should at least wrote it to release notes
>>>>>>>> (I'll let Honza to express his feelings as API versioning/compatibility
>>>>>>>> sensei)
>>>>>>>
>>>>>>> IMHO it is safe to remove.
>>>>>>>
>>>>>>>>
>>>>>>>> And you forgot to increment api version in VERSION file
>>>>>> Updated patch attached, with a VERSION change.
>>>>>>
>>>>>>
>>>>>>
>>>>> ACK
>>>>>
>>>>
>>>> Is there any ticket for this?
>>> As I wrote in the commit message and in the email,
>>> it is part of https://fedorahosted.org/freeipa/ticket/5354
>>>
>> Sorry I misread that ticket in the commit message, because ipatool was unable
>> to parse it from commit message
>> 
>> Pushed to master: 185806432d6dfccc5cdd73815471ce60a575b073
>
>I see no link to this ticket in the commit message in
>https://git.fedorahosted.org/cgit/freeipa.git/commit/?id=185806432d6dfccc5cdd73815471ce60a575b073
>Did you push old version of this patch?
>
>In general, I would suggest using the patch format from
>http://www.freeipa.org/page/Contribute/Patch_Format
>It makes automation easier...
>
And it would be much easier for author with .git-commit-template
@see
https://git.fedorahosted.org/cgit/sssd.git/commit/?id=3d9edb4c510028def2df41aa7b0ce705b197e6fc

LS