Re: [Freeipa-devel] [PATCH 0055] raise an exception when user tries to modify a local ID range

2015-08-12 Thread Tomas Babej


On 08/12/2015 04:38 PM, Tomas Babej wrote:
> 
> 
> On 08/10/2015 10:50 AM, Martin Babinsky wrote:
>> On 08/07/2015 05:25 PM, Tomas Babej wrote:
>>>
>>>
>>> On 08/07/2015 05:09 PM, Martin Babinsky wrote:
 On 08/07/2015 04:51 PM, Tomas Babej wrote:
>
>
> On 08/07/2015 04:22 PM, Martin Babinsky wrote:
>> Short term fix for https://fedorahosted.org/freeipa/ticket/4826
>>
>>
>>
>
> Hi,
>
> couple of minor issues:
>
> 1.) Please create a separate constant for the WARNING section, now this
> segment is copy-pasted at three different places in the plugin.
>
> 2.) It would be nice to fix the broken indentation in the help texts
> for
> ipa idrange-add/mod whlie poking at that part of the code.
>
 How should these sections be indented (especially the warning parts)?

>>>
>>> The section is indented using 4 spaces in "ipa help idrange-mod" (-add).
>>> I see no point in doing so, Additionally, being a separate block of
>>> text, it is not visually separated from the options block.
>>>
> 3.) 'ipa help idranges' does not produce any info, it error message
> needs to suggest 'ipa help idrange'
>
> Otherwise looks and works good.
>
> Tomas
>


>>
>> Attaching updated patch. I have tried to improve the visibility of the
>> warning message.
>>
> 
> Thanks, much better now.
> 
> ACK.
> 

Pushed to:
master: 55feea500be1f4ae7bf02ef3c48377a6751ca71d
ipa-4-2: 5738cdb1145f6bce7f31a6d29bd39ceadbe62c88

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0055] raise an exception when user tries to modify a local ID range

2015-08-12 Thread Tomas Babej


On 08/10/2015 10:50 AM, Martin Babinsky wrote:
> On 08/07/2015 05:25 PM, Tomas Babej wrote:
>>
>>
>> On 08/07/2015 05:09 PM, Martin Babinsky wrote:
>>> On 08/07/2015 04:51 PM, Tomas Babej wrote:


 On 08/07/2015 04:22 PM, Martin Babinsky wrote:
> Short term fix for https://fedorahosted.org/freeipa/ticket/4826
>
>
>

 Hi,

 couple of minor issues:

 1.) Please create a separate constant for the WARNING section, now this
 segment is copy-pasted at three different places in the plugin.

 2.) It would be nice to fix the broken indentation in the help texts
 for
 ipa idrange-add/mod whlie poking at that part of the code.

>>> How should these sections be indented (especially the warning parts)?
>>>
>>
>> The section is indented using 4 spaces in "ipa help idrange-mod" (-add).
>> I see no point in doing so, Additionally, being a separate block of
>> text, it is not visually separated from the options block.
>>
 3.) 'ipa help idranges' does not produce any info, it error message
 needs to suggest 'ipa help idrange'

 Otherwise looks and works good.

 Tomas

>>>
>>>
> 
> Attaching updated patch. I have tried to improve the visibility of the
> warning message.
> 

Thanks, much better now.

ACK.

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0055] raise an exception when user tries to modify a local ID range

2015-08-10 Thread Martin Babinsky

On 08/07/2015 05:25 PM, Tomas Babej wrote:



On 08/07/2015 05:09 PM, Martin Babinsky wrote:

On 08/07/2015 04:51 PM, Tomas Babej wrote:



On 08/07/2015 04:22 PM, Martin Babinsky wrote:

Short term fix for https://fedorahosted.org/freeipa/ticket/4826





Hi,

couple of minor issues:

1.) Please create a separate constant for the WARNING section, now this
segment is copy-pasted at three different places in the plugin.

2.) It would be nice to fix the broken indentation in the help texts for
ipa idrange-add/mod whlie poking at that part of the code.


How should these sections be indented (especially the warning parts)?



The section is indented using 4 spaces in "ipa help idrange-mod" (-add).
I see no point in doing so, Additionally, being a separate block of
text, it is not visually separated from the options block.


3.) 'ipa help idranges' does not produce any info, it error message
needs to suggest 'ipa help idrange'

Otherwise looks and works good.

Tomas






Attaching updated patch. I have tried to improve the visibility of the 
warning message.


--
Martin^3 Babinsky
From 258af243950697df8fccf04b7aee762e59b537e2 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Fri, 7 Aug 2015 15:44:57 +0200
Subject: [PATCH] idranges: raise an error when local IPA ID range is being
 modified

also show the message about the way UID/GID ranges are managed in FreeIPA in
the idrange-mod's help message

https://fedorahosted.org/freeipa/ticket/4826
---
 ipalib/plugins/idrange.py | 52 ++-
 1 file changed, 29 insertions(+), 23 deletions(-)

diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index fb198d79d4c14ffd5f7dc633c9f01a1465ff01d7..2cec05bd8f837fb27803b869bf33fe389126506c 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -31,6 +31,20 @@ if api.env.in_server and api.env.context in ['lite', 'server']:
 except ImportError:
 _dcerpc_bindings_installed = False
 
+ID_RANGE_VS_DNA_WARNING = """===
+WARNING:
+
+DNA plugin in 389-ds will allocate IDs based on the ranges configured for the
+local domain. Currently the DNA plugin *cannot* be reconfigured itself based
+on the local ranges set via this family of commands.
+
+Manual configuration change has to be done in the DNA plugin configuration for
+the new local range. Specifically, The dnaNextRange attribute of 'cn=Posix
+IDs,cn=Distributed Numeric Assignment Plugin,cn=plugins,cn=config' has to be
+modified to match the new range.
+===
+"""
+
 __doc__ = _("""
 ID ranges
 
@@ -139,17 +153,8 @@ this domain has the SID S-1-5-21-123-456-789-1010 then 1010 id the RID of the
 user. RIDs are unique in a domain, 32bit values and are used for users and
 groups.
 
-WARNING:
-
-DNA plugin in 389-ds will allocate IDs based on the ranges configured for the
-local domain. Currently the DNA plugin *cannot* be reconfigured itself based
-on the local ranges set via this family of commands.
-
-Manual configuration change has to be done in the DNA plugin configuration for
-the new local range. Specifically, The dnaNextRange attribute of 'cn=Posix
-IDs,cn=Distributed Numeric Assignment Plugin,cn=plugins,cn=config' has to be
-modified to match the new range.
-""")
+{0}
+""".format(ID_RANGE_VS_DNA_WARNING))
 
 register = Registry()
 
@@ -386,17 +391,8 @@ class idrange_add(LDAPCreate):
 
 must be given to add a new range for a trusted AD domain.
 
-WARNING:
-
-DNA plugin in 389-ds will allocate IDs based on the ranges configured for the
-local domain. Currently the DNA plugin *cannot* be reconfigured itself based
-on the local ranges set via this family of commands.
-
-Manual configuration change has to be done in the DNA plugin configuration for
-the new local range. Specifically, The dnaNextRange attribute of 'cn=Posix
-IDs,cn=Distributed Numeric Assignment Plugin,cn=plugins,cn=config' has to be
-modified to match the new range.
-""")
+{0}
+""".format(ID_RANGE_VS_DNA_WARNING))
 
 msg_summary = _('Added ID range "%(value)s"')
 
@@ -670,7 +666,10 @@ class idrange_show(LDAPRetrieve):
 
 @register()
 class idrange_mod(LDAPUpdate):
-__doc__ = _('Modify ID range.')
+__doc__ = _("""Modify ID range.
+
+{0}
+""".format(ID_RANGE_VS_DNA_WARNING))
 
 msg_summary = _('Modified ID range "%(value)s"')
 
@@ -688,6 +687,13 @@ class idrange_mod(LDAPUpdate):
 except errors.NotFound:
 self.obj.handle_not_found(*keys)
 
+if old_attrs['iparangetype'][0] == 'ipa-local':
+raise errors.ExecutionError(
+message=_('This command can not be used to change ID '
+  'allocation for local IPA domain. Run '
+  '`ipa help idrange` for more information')
+)
+
 is_set = lambda x: (x in entry_attrs) and (entry_attrs[x] is not None)
 in_updated_attrs = lambda x:\
 (x in entry_attrs and entry_attrs[x] is not None) or\
-- 
2.4.

Re: [Freeipa-devel] [PATCH 0055] raise an exception when user tries to modify a local ID range

2015-08-07 Thread Tomas Babej


On 08/07/2015 05:09 PM, Martin Babinsky wrote:
> On 08/07/2015 04:51 PM, Tomas Babej wrote:
>>
>>
>> On 08/07/2015 04:22 PM, Martin Babinsky wrote:
>>> Short term fix for https://fedorahosted.org/freeipa/ticket/4826
>>>
>>>
>>>
>>
>> Hi,
>>
>> couple of minor issues:
>>
>> 1.) Please create a separate constant for the WARNING section, now this
>> segment is copy-pasted at three different places in the plugin.
>>
>> 2.) It would be nice to fix the broken indentation in the help texts for
>> ipa idrange-add/mod whlie poking at that part of the code.
>>
> How should these sections be indented (especially the warning parts)?
> 

The section is indented using 4 spaces in "ipa help idrange-mod" (-add).
I see no point in doing so, Additionally, being a separate block of
text, it is not visually separated from the options block.

>> 3.) 'ipa help idranges' does not produce any info, it error message
>> needs to suggest 'ipa help idrange'
>>
>> Otherwise looks and works good.
>>
>> Tomas
>>
> 
> 

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0055] raise an exception when user tries to modify a local ID range

2015-08-07 Thread Martin Babinsky

On 08/07/2015 04:51 PM, Tomas Babej wrote:



On 08/07/2015 04:22 PM, Martin Babinsky wrote:

Short term fix for https://fedorahosted.org/freeipa/ticket/4826





Hi,

couple of minor issues:

1.) Please create a separate constant for the WARNING section, now this
segment is copy-pasted at three different places in the plugin.

2.) It would be nice to fix the broken indentation in the help texts for
ipa idrange-add/mod whlie poking at that part of the code.


How should these sections be indented (especially the warning parts)?


3.) 'ipa help idranges' does not produce any info, it error message
needs to suggest 'ipa help idrange'

Otherwise looks and works good.

Tomas




--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0055] raise an exception when user tries to modify a local ID range

2015-08-07 Thread Tomas Babej


On 08/07/2015 04:22 PM, Martin Babinsky wrote:
> Short term fix for https://fedorahosted.org/freeipa/ticket/4826
> 
> 
> 

Hi,

couple of minor issues:

1.) Please create a separate constant for the WARNING section, now this
segment is copy-pasted at three different places in the plugin.

2.) It would be nice to fix the broken indentation in the help texts for
ipa idrange-add/mod whlie poking at that part of the code.

3.) 'ipa help idranges' does not produce any info, it error message
needs to suggest 'ipa help idrange'

Otherwise looks and works good.

Tomas

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


[Freeipa-devel] [PATCH 0055] raise an exception when user tries to modify a local ID range

2015-08-07 Thread Martin Babinsky

Short term fix for https://fedorahosted.org/freeipa/ticket/4826

--
Martin^3 Babinsky
From fe39a50a0469880a9f574c893b82b8e52642aac7 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Fri, 7 Aug 2015 15:44:57 +0200
Subject: [PATCH] idranges: raise an error when local IPA ID range is being
 modified

also show the message about the way UID/GID ranges are managed in FreeIPA in
the idrange-mod's help message

https://fedorahosted.org/freeipa/ticket/4826
---
 ipalib/plugins/idrange.py | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index fb198d79d4c14ffd5f7dc633c9f01a1465ff01d7..2860cf10683633b53df689b94cc67e04349730ea 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -670,7 +670,19 @@ class idrange_show(LDAPRetrieve):
 
 @register()
 class idrange_mod(LDAPUpdate):
-__doc__ = _('Modify ID range.')
+__doc__ = _("""Modify ID range.
+
+WARNING:
+
+DNA plugin in 389-ds will allocate IDs based on the ranges configured for the
+local domain. Currently the DNA plugin *cannot* be reconfigured itself based
+on the local ranges set via this family of commands.
+
+Manual configuration change has to be done in the DNA plugin configuration for
+the new local range. Specifically, The dnaNextRange attribute of 'cn=Posix
+IDs,cn=Distributed Numeric Assignment Plugin,cn=plugins,cn=config' has to be
+modified to match the new range.
+""")
 
 msg_summary = _('Modified ID range "%(value)s"')
 
@@ -688,6 +700,13 @@ class idrange_mod(LDAPUpdate):
 except errors.NotFound:
 self.obj.handle_not_found(*keys)
 
+if old_attrs['iparangetype'][0] == 'ipa-local':
+raise errors.ExecutionError(
+message=_('This command can not be used to change ID '
+  'allocation for local IPA domain. Run '
+  '`ipa help idranges` for more information')
+)
+
 is_set = lambda x: (x in entry_attrs) and (entry_attrs[x] is not None)
 in_updated_attrs = lambda x:\
 (x in entry_attrs and entry_attrs[x] is not None) or\
-- 
2.4.3

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code