[Freeipa-devel] [PATCH 0184] Always return absolute idnsname in dnszone commands
Jan Cholasta
jcholast at redhat.com
Fri Jan 23 07:22:31 UTC 2015
Dne 20.1.2015 v 12:49 Martin Basti napsal(a):
> On 15/01/15 16:07, Jan Cholasta wrote:
>> Dne 15.1.2015 v 15:39 Martin Basti napsal(a):
>>> On 15/01/15 15:07, Jan Cholasta wrote:
>>>> Dne 15.1.2015 v 14:58 Martin Basti napsal(a):
>>>>> On 15/01/15 14:25, Jan Cholasta wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Dne 15.1.2015 v 13:27 Martin Basti napsal(a):
>>>>>>> On 15/01/15 13:17, Martin Basti wrote:
>>>>>>>> https://fedorahosted.org/freeipa/ticket/4722
>>>>>>>>
>>>>>>>> Patch attached.
>>>>>>>>
>>>>>>> Fast fix.
>>>>>>>
>>>>>>> Updated patch attached.
>>>>>>
>>>>>> 1) Forward zone commands are not fixed.
>>>>> FWzones are new and always normalized to absolute name in ldap
>>>>
>>>> Would you bet your money on that? Better be safe than sorry,
>>>> especially when it's just a matter of moving the code around (right?)
>>>>
>>>>>>
>>>>>> 2) It seems that the primary key returned by -mod, -del and -show
>>>>>> (.result.value) is made absolute somewhere else in the code. Would it
>>>>>> be possible to do it in one place?
>>>>> IMO it is not possible.
>>>>>
>>>>> Value is generated from key, and key is normalized to absolute zone
>>>>> before calling execute.
>>>>>
>>>>> LDAPUpdate:
>>>>> ...
>>>>> if self.obj.primary_key:
>>>>> pkey = keys[-1]
>>>>> else:
>>>>> pkey = None
>>>>>
>>>>> return dict(result=entry_attrs, value=pkey_to_value(pkey,
>>>>> options))
>>>>>
>>>>> The idnsname attribute is just taken from LDAP without any
>>>>> normalization
>>>>
>>>> Right.
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> 3) Attribute values returned from LDAP are never None, so the if
>>>>>> should be "if 'idnsname' in entry_attrs:".
>>>>> Ok I will revert the change I made.
>>>>>>
>>>>>> 4) If idnsname always has only single value, use
>>>>>> "entry_attrs.single_value['idnsname'] =
>>>>>> entry_attrs.single_value['idnsname'].make_absolute()"
>>>>> Thanks
>>>>>>
>>>>>> Honza
>>>>>>
>>>>>
>>>>> Updated patch attached.
>>>>>
>>>>
>>>>
>>> Updated patch attached.
>>>
>>
>> Thanks.
>>
>> Is there a reason why you put the _make_zone_absolute calls in
>> dnszone_* and dnsforwardzone_* instead of DNSZoneBase_*?
>>
> I moved callback into Base classes.
> Patch attached.
>
1) Multi-line statements should generally use parentheses for implicit
line continuation rather than backslashes:
+ entry_attrs.single_value['idnsname'] = (
+ entry_attrs.single_value['idnsname'].make_absolute())
2) You can remove the DN asserts from dnszone_*, they are already
asserted in DNSZoneBase_*.
3) Do not just ignore return values of super().post_callback():
def post_callback(self, something, ...):
something = super(...).post_callback(something, ...)
...
return something
--
Jan Cholasta
More information about the Freeipa-devel
mailing list