Re: [Freeipa-devel] [PATCH 0031] Deprecate options --dom-sid and --dom-name in idrange-mod

2013-05-31 Thread Tomas Babej

On 05/29/2013 03:24 PM, Ana Krivokapic wrote:

Hello,

This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3636



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


I edited your patch to use newly introduced prompt_param method as 
agreed in my patches 53-55 thread.


The functional part itself looks good, the tests though, are dependent 
on the environment. The particular
code branch of tests that is being executed depends on the fact whether 
any trust is estabilished on that

particular FreeIPA instance the test suite is being run on.

I suggest you create a mock trust LDAP entry as in my patch 57 that has 
been just pushed to master,
and test both cases (whether the interactive prompt behaves correctly 
both with the trust estabilished

and without it).

Maybe we should move the setUpClass/tearDownClass logic to tests/util.py 
to avoid code duplication.


Attaching the updated patch (apply on top of tbabej-55-3).

Tomas
From f8e4546fac4d785c151a0ce07745741287b63cbc Mon Sep 17 00:00:00 2001
From: Ana Krivokapic akriv...@redhat.com
Date: Tue, 28 May 2013 16:42:03 +0200
Subject: [PATCH] Require rid-base and secondary-rid-base options in
 idrange-add when trust exists

https://fedorahosted.org/freeipa/ticket/3634
---
 ipalib/plugins/idrange.py  | 34 +++-
 tests/test_cmdline/test_cli.py | 50 ++
 2 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index f83ced32888bc984d02fdbd8ab5ff4c512c3ec06..e85df3f69f6496f64cade98dd4448f19dec55b3b 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -342,7 +342,7 @@ class idrange_add(LDAPCreate):
 
 may be given for a new ID range for the local domain while
 
---rid-bas
+--rid-base
 --dom-sid
 
 must be given to add a new range for a trusted AD domain.
@@ -367,6 +367,9 @@ class idrange_add(LDAPCreate):
 
 Also ensure that secondary-rid-base is prompted for when rid-base is
 specified and vice versa, in case that dom-sid was not specified.
+
+Interactive mode should prompt for rid-base and secondary-rid-base
+if a trust is established.
 
 
 # dom-sid can be specified using dom-sid or dom-name options
@@ -396,6 +399,21 @@ class idrange_add(LDAPCreate):
 value = self.prompt_param(self.params['ipabaserid'])
 kw.update(dict(ipabaserid=value))
 
+trust_exists = api.Command['trust_find']()['count']
+
+if trust_exists:
+
+rid_base = kw.get('ipabaserid', None)
+secondary_rid_base = kw.get('ipasecondarybaserid', None)
+
+if rid_base is None:
+value = self.prompt_param(self.params['ipabaserid'])
+kw.update(dict(ipabaserid=value))
+
+if secondary_rid_base is None:
+value = self.prompt_param(self.params['ipasecondarybaserid'])
+kw.update(dict(ipasecondarybaserid=value))
+
 def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
 assert isinstance(dn, DN)
 
@@ -453,6 +471,20 @@ class idrange_add(LDAPCreate):
 error=_(Primary RID range and secondary RID range
  cannot overlap))
 
+# If a trust is established, base rid and secondary base rid
+# must be specified for local id range
+trust_exists = api.Command['trust_find']()['count']
+
+if trust_exists and not (
+is_set('ipabaserid') and is_set('ipasecondarybaserid')):
+raise errors.ValidationError(
+name='ID Range setup',
+error=_('You must specify both rid-base and '
+'secondary-rid-base options, because a trust is '
+'established.'
+)
+)
+
 entry_attrs['objectclass'].append('ipadomainidrange')
 
 return dn
diff --git a/tests/test_cmdline/test_cli.py b/tests/test_cmdline/test_cli.py
index bd1281e1d682b055ede9685a10a9cec91a3c76fd..7137ff4573f7de7699b0ff0b6ec86b305af49e00 100644
--- a/tests/test_cmdline/test_cli.py
+++ b/tests/test_cmdline/test_cli.py
@@ -325,3 +325,53 @@ class TestCLIParsing(object):
 force=False,
 version=API_VERSION
 )
+
+def test_idrange_add(self):
+
+Test idrange-add with interative prompt
+
+trust_exists = api.Command['trust_find']()['count']
+
+if trust_exists:
+# Pass rid-base and secondary-rid-base interactively
+with self.fake_stdin('5\n50\n'):
+self.check_command(
+'idrange_add range1 --base-id=1 --range-size=1',
+

Re: [Freeipa-devel] [PATCH 0031] Deprecate options --dom-sid and --dom-name in idrange-mod

2013-05-31 Thread Tomas Babej

On 05/31/2013 12:25 PM, Tomas Babej wrote:

On 05/29/2013 03:24 PM, Ana Krivokapic wrote:

Hello,

This patch addresses tickethttps://fedorahosted.org/freeipa/ticket/3636



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


I edited your patch to use newly introduced prompt_param method as 
agreed in my patches 53-55 thread.


The functional part itself looks good, the tests though, are dependent 
on the environment. The particular
code branch of tests that is being executed depends on the fact 
whether any trust is estabilished on that

particular FreeIPA instance the test suite is being run on.

I suggest you create a mock trust LDAP entry as in my patch 57 that 
has been just pushed to master,
and test both cases (whether the interactive prompt behaves correctly 
both with the trust estabilished

and without it).

Maybe we should move the setUpClass/tearDownClass logic to 
tests/util.py to avoid code duplication.


Attaching the updated patch (apply on top of tbabej-55-3).

Tomas


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Wrong thread, sorry. This applies to patch 30.

Tomas
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0031] Deprecate options --dom-sid and --dom-name in idrange-mod

2013-05-31 Thread Tomas Babej

On 05/31/2013 12:51 PM, Tomas Babej wrote:

On 05/31/2013 12:25 PM, Tomas Babej wrote:

On 05/29/2013 03:24 PM, Ana Krivokapic wrote:

Hello,

This patch addresses tickethttps://fedorahosted.org/freeipa/ticket/3636



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


I edited your patch to use newly introduced prompt_param method as 
agreed in my patches 53-55 thread.


The functional part itself looks good, the tests though, are 
dependent on the environment. The particular
code branch of tests that is being executed depends on the fact 
whether any trust is estabilished on that

particular FreeIPA instance the test suite is being run on.

I suggest you create a mock trust LDAP entry as in my patch 57 that 
has been just pushed to master,
and test both cases (whether the interactive prompt behaves correctly 
both with the trust estabilished

and without it).

Maybe we should move the setUpClass/tearDownClass logic to 
tests/util.py to avoid code duplication.


Attaching the updated patch (apply on top of tbabej-55-3).

Tomas


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Wrong thread, sorry. This applies to patch 30.

Tomas


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


I tested the *patch 31*, both with new and old client, works fine.

ACK

Tomas
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0031] Deprecate options --dom-sid and --dom-name in idrange-mod

2013-05-31 Thread Martin Kosek
On 05/31/2013 01:56 PM, Tomas Babej wrote:
 On 05/31/2013 12:51 PM, Tomas Babej wrote:
 On 05/31/2013 12:25 PM, Tomas Babej wrote:
 On 05/29/2013 03:24 PM, Ana Krivokapic wrote:
 Hello,

 This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3636



 ___
 Freeipa-devel mailing list
 Freeipa-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/freeipa-devel

 I edited your patch to use newly introduced prompt_param method as agreed in
 my patches 53-55 thread.

 The functional part itself looks good, the tests though, are dependent on
 the environment. The particular
 code branch of tests that is being executed depends on the fact whether any
 trust is estabilished on that
 particular FreeIPA instance the test suite is being run on.

 I suggest you create a mock trust LDAP entry as in my patch 57 that has been
 just pushed to master,
 and test both cases (whether the interactive prompt behaves correctly both
 with the trust estabilished
 and without it).

 Maybe we should move the setUpClass/tearDownClass logic to tests/util.py to
 avoid code duplication.

 Attaching the updated patch (apply on top of tbabej-55-3).

 Tomas


 ___
 Freeipa-devel mailing list
 Freeipa-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/freeipa-devel
 Wrong thread, sorry. This applies to patch 30.

 Tomas


 ___
 Freeipa-devel mailing list
 Freeipa-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/freeipa-devel

 I tested the *patch 31*, both with new and old client, works fine.
 
 ACK
 
 Tomas
 

Pushed to master.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel