[Freeipa-devel] [PATCH 0030] Add option to specify SID using domain name to idrange-add/mod

[Alexander Bokovoy]

On Fri, 08 Feb 2013, Tomas Babej wrote:
> On 02/08/2013 03:25 PM, Alexander Bokovoy wrote:
>> On Mon, 04 Feb 2013, Tomas Babej wrote:
>>> Hi,
>>> 
>>> When adding/modifying an ID range for a trusted domain, the newly
>>> added option --dom-name can be used. This looks up SID of the
>>> trusted domain in LDAP and therefore the user is not required
>>> to write it down in CLI. If the lookup fails, error message
>>> asking the user to specify the SID manually is shown.
>>> 
>>> https://fedorahosted.org/freeipa/ticket/3133
>>> 
>>> Tomas
>> 
>>>> From 72f8802953edaaf5b9f7c34a38601fbccd681c8e Mon Sep 17 00:00:00 2001
>>> From: Tomas Babej <tbabej at redhat.com>
>>> Date: Mon, 4 Feb 2013 08:33:53 -0500
>>> Subject: [PATCH] Add option to specify SID using domain name to
>>> idrange-add/mod
>>> 
>>> When adding/modifying an ID range for a trusted domain, the newly
>>> added option --dom-name can be used. This looks up SID of the
>>> trusted domain in LDAP and therefore the user is not required
>>> to write it down in CLI. If the lookup fails, error message
>>> asking the user to specify the SID manually is shown.
>>> 
>>> https://fedorahosted.org/freeipa/ticket/3133
>>> ---
>>> ipalib/plugins/idrange.py | 78 
>>> +++++++++++++++++++++++++++++++++++++++++------
>>> ipaserver/dcerpc.py | 10 ++++++
>>> 2 files changed, 78 insertions(+), 10 deletions(-)
>>> 
>>> diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
>>> index 84e1057ac6b59b8ad99882a54e3288897338c978..77a75e4cabc18ca873be7cadcf870427d5b36ea0 
>>> 100644
>>> --- a/ipalib/plugins/idrange.py
>>> +++ b/ipalib/plugins/idrange.py
>>> @@ -197,6 +197,11 @@ class idrange(LDAPObject):
>>> cli_name='dom_sid',
>>> label=_('Domain SID of the trusted domain'),
>>> ),
>>> + Str('ipanttrusteddomainname?',
>>> + cli_name='dom_name',
>>> + flags=('no_search', 'virtual_attribute'),
>>> + label=_('Name of the trusted domain'),
>>> + ),
>> New options is added but API.txt wasn't changed. As result, 'make rpms'
>> does not work.
>> 
>> Could you please fix the patch and re-send it?
>> 
> Sorry about that.
> 
> Updated patch attached.
I have one small question regarding use of dom_sid/dom_name.

If both dom_sid and dom_name were specified, failing to resolve dom_name
would force command to raise exception.

I'm not sure this is right behavior. Probably we should detect that both
dom_sid and dom_name were specified and bail out earlier so that only
one of them is accepted. That would be clearer to users, wouldn't it?


-- 
/ Alexander Bokovoy