Re: [Freeipa-devel] [PATCH 0067] Add --use-posix option that forces trusted range type
On Monday 24 of June 2013 16:22:01 Petr Viktorin wrote:
On 06/20/2013 12:56 PM, Tomas Babej wrote:
On 06/17/2013 02:34 PM, Ana Krivokapic wrote:
On 06/06/2013 11:10 AM, Tomas Babej wrote:
Hi,
Adds --use-posix option to ipa trust-add command. It takes two
allowed values:
'yes' : the 'ipa-ad-trust-posix' range type is enforced
'no' : the 'ipa-ad-trust' range type is enforced
When --use-posix option is not specified, the range type should be
determined by ID range discovery.
https://fedorahosted.org/freeipa/ticket/3650
Tomas
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
The patch works nicely, but I have a few comments:
1) You added a new option to the API, but you forgot to bump the
IPA_API_VERSION_MINOR in the VERSION file.
2) Typo in commit message: shold instead of should.
3) This construct:
+if range_type is not None:
+if range_type != old_range_type:
can be replaced with a more readable variant which also avoids nested
ifs:
+if range_type and range_type != old_range_type:
All fixed.
4) Some unit tests to cover the behavior of the newly added option
would be nice.
This is not doable at the moment, we have no unit test framework to test
the trust-add command.
--
Regards,
Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Tomas
I don't know much about AD trusts, but a command-line/API option that
takes a 'yes' or 'no' string raised a tiny warning flag for me.
It looks like it's possible that there can be other range types in the
future than posix and algorithmic? If that's the case, there should be a
--range-type option instead. (If not, I'd still go for --range-type but
that would just be bikeshedding.)
In any case I think an explicit 'auto' option would be nice.
But that's just an outsider's view, maybe --use-posix makes more sense.
I replaced --use-posix with more versatile --range-type. It supports only
trust-related values for now, and can be extended.
Rebased patch attached.
AFAIK, for CLI changes there should be a a design page; is this covered
anywhere?
Design page for the umbrella ticket is here:
http://www.freeipa.org/page/V3/Use_posix_attributes_defined_in_AD
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-develFrom 784021ef93694bb19e8a30c509dbbec8cb06cdf0 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Tue, 25 Jun 2013 14:25:44 +0200
Subject: [PATCH] Add --range-type option that forces range type of the trusted
domain
Adds --range-type option to ipa trust-add command. It takes two
allowed values: 'ipa-ad-trust-posix' and 'ipa-ad-trust'.
When --range-type option is not specified, the range type should be
determined by ID range discovery.
https://fedorahosted.org/freeipa/ticket/3650
---
API.txt | 3 ++-
VERSION | 2 +-
ipalib/plugins/idrange.py | 4 ++--
ipalib/plugins/trust.py | 40 ++--
4 files changed, 43 insertions(+), 6 deletions(-)
diff --git a/API.txt b/API.txt
index 067955ef77332374bf242655187ef9f912b0c2c0..44b3dd444964c8dac595177f8601c82d0235eabe 100644
--- a/API.txt
+++ b/API.txt
@@ -3278,12 +3278,13 @@ output: Entry('result', type 'dict', Gettext('A dictionary representing an LDA
output: Output('summary', (type 'unicode', type 'NoneType'), None)
output: Output('value', type 'unicode', None)
command: trust_add
-args: 1,12,3
+args: 1,13,3
arg: Str('cn', attribute=True, cli_name='realm', multivalue=False, primary_key=True, required=True)
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
option: Int('base_id?', cli_name='base_id')
option: Int('range_size?', autofill=True, cli_name='range_size', default=20)
+option: StrEnum('range_type?', cli_name='range_type', values=(u'ipa-ad-trust-posix', u'ipa-ad-trust'))
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Str('realm_admin?', cli_name='admin')
option: Password('realm_passwd?', cli_name='password', confirm=False)
diff --git a/VERSION b/VERSION
index c713b18b7ce42db7fb290912ec6028342015f142..8606d724e6c8c785ba9d554ed3effa905573e25f 100644
--- a/VERSION
+++ b/VERSION
@@ -89,4 +89,4 @@ IPA_DATA_VERSION=2010061412
# #
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=60
Re: [Freeipa-devel] [PATCH 0067] Add --use-posix option that forces trusted range type
On 06/24/2013 04:22 PM, Petr Viktorin wrote: On 06/20/2013 12:56 PM, Tomas Babej wrote: On 06/17/2013 02:34 PM, Ana Krivokapic wrote: On 06/06/2013 11:10 AM, Tomas Babej wrote: Hi, Adds --use-posix option to ipa trust-add command. It takes two allowed values: 'yes' : the 'ipa-ad-trust-posix' range type is enforced 'no' : the 'ipa-ad-trust' range type is enforced When --use-posix option is not specified, the range type should be determined by ID range discovery. https://fedorahosted.org/freeipa/ticket/3650 Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel The patch works nicely, but I have a few comments: 1) You added a new option to the API, but you forgot to bump the IPA_API_VERSION_MINOR in the VERSION file. 2) Typo in commit message: shold instead of should. 3) This construct: +if range_type is not None: +if range_type != old_range_type: can be replaced with a more readable variant which also avoids nested ifs: +if range_type and range_type != old_range_type: All fixed. 4) Some unit tests to cover the behavior of the newly added option would be nice. This is not doable at the moment, we have no unit test framework to test the trust-add command. -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Tomas I don't know much about AD trusts, but a command-line/API option that takes a 'yes' or 'no' string raised a tiny warning flag for me. It looks like it's possible that there can be other range types in the future than posix and algorithmic? If that's the case, there should be a --range-type option instead. (If not, I'd still go for --range-type but that would just be bikeshedding.) In any case I think an explicit 'auto' option would be nice. But that's just an outsider's view, maybe --use-posix makes more sense. AFAIK, for CLI changes there should be a a design page; is this covered anywhere? It should be covered in the parent RFE ticket: https://fedorahosted.org/freeipa/ticket/2904 I see it is not there. This is still a task for Tomas or Alexander. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0067] Add --use-posix option that forces trusted range type
On 24.6.2013 16:22, Petr Viktorin wrote: I don't know much about AD trusts, but a command-line/API option that takes a 'yes' or 'no' string raised a tiny warning flag for me. It looks like it's possible that there can be other range types in the future than posix and algorithmic? If that's the case, there should be a --range-type option instead. (If not, I'd still go for --range-type but that would just be bikeshedding.) In any case I think an explicit 'auto' option would be nice. But that's just an outsider's view, maybe --use-posix makes more sense. +1 -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0067] Add --use-posix option that forces trusted range type
On 06/20/2013 12:56 PM, Tomas Babej wrote: On 06/17/2013 02:34 PM, Ana Krivokapic wrote: On 06/06/2013 11:10 AM, Tomas Babej wrote: Hi, Adds --use-posix option to ipa trust-add command. It takes two allowed values: 'yes' : the 'ipa-ad-trust-posix' range type is enforced 'no' : the 'ipa-ad-trust' range type is enforced When --use-posix option is not specified, the range type should be determined by ID range discovery. https://fedorahosted.org/freeipa/ticket/3650 Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel The patch works nicely, but I have a few comments: 1) You added a new option to the API, but you forgot to bump the IPA_API_VERSION_MINOR in the VERSION file. 2) Typo in commit message: shold instead of should. 3) This construct: +if range_type is not None: +if range_type != old_range_type: can be replaced with a more readable variant which also avoids nested ifs: +if range_type and range_type != old_range_type: All fixed. 4) Some unit tests to cover the behavior of the newly added option would be nice. This is not doable at the moment, we have no unit test framework to test the trust-add command. -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Tomas I don't know much about AD trusts, but a command-line/API option that takes a 'yes' or 'no' string raised a tiny warning flag for me. It looks like it's possible that there can be other range types in the future than posix and algorithmic? If that's the case, there should be a --range-type option instead. (If not, I'd still go for --range-type but that would just be bikeshedding.) In any case I think an explicit 'auto' option would be nice. But that's just an outsider's view, maybe --use-posix makes more sense. AFAIK, for CLI changes there should be a a design page; is this covered anywhere? -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0067] Add --use-posix option that forces trusted range type
On 06/17/2013 02:34 PM, Ana Krivokapic wrote:
On 06/06/2013 11:10 AM, Tomas Babej wrote:
Hi,
Adds --use-posix option to ipa trust-add command. It takes two
allowed values:
'yes' : the 'ipa-ad-trust-posix' range type is enforced
'no' : the 'ipa-ad-trust' range type is enforced
When --use-posix option is not specified, the range type should be
determined by ID range discovery.
https://fedorahosted.org/freeipa/ticket/3650
Tomas
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
The patch works nicely, but I have a few comments:
1) You added a new option to the API, but you forgot to bump the
IPA_API_VERSION_MINOR in the VERSION file.
2) Typo in commit message: shold instead of should.
3) This construct:
+if range_type is not None:
+if range_type != old_range_type:
can be replaced with a more readable variant which also avoids nested ifs:
+if range_type and range_type != old_range_type:
All fixed.
4) Some unit tests to cover the behavior of the newly added option
would be nice.
This is not doable at the moment, we have no unit test framework to test
the trust-add command.
--
Regards,
Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Tomas
From e3521edad9babacd77e50576c827c3f02a770514 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Wed, 5 Jun 2013 11:51:27 +0200
Subject: [PATCH] Add --use-posix option that forces trusted range type
Adds --use-posix option to ipa trust-add command. It takes two
allowed values:
'yes' : the 'ipa-ad-trust-posix' range type is enforced
'no' : the 'ipa-ad-trust' range type is enforced
When --use-posix option is not specified, the range type should be
determined by ID range discovery.
https://fedorahosted.org/freeipa/ticket/3650
---
API.txt | 3 ++-
VERSION | 2 +-
ipalib/plugins/trust.py | 43 +--
3 files changed, 36 insertions(+), 12 deletions(-)
diff --git a/API.txt b/API.txt
index 1313460de66d8e12fc7a068cda0cf30658bcdd1b..ee6602b27dfa76099d9b38b0539a8c852e8974a0 100644
--- a/API.txt
+++ b/API.txt
@@ -3339,7 +3339,7 @@ output: Entry('result', type 'dict', Gettext('A dictionary representing an LDA
output: Output('summary', (type 'unicode', type 'NoneType'), None)
output: Output('value', type 'unicode', None)
command: trust_add
-args: 1,12,3
+args: 1,13,3
arg: Str('cn', attribute=True, cli_name='realm', multivalue=False, primary_key=True, required=True)
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -3352,6 +3352,7 @@ option: Str('realm_server?', cli_name='server')
option: Str('setattr*', cli_name='setattr', exclude='webui')
option: Password('trust_secret?', cli_name='trust_secret', confirm=False)
option: StrEnum('trust_type', autofill=True, cli_name='type', default=u'ad', values=(u'ad',))
+option: StrEnum('use_posix?', cli_name='use_posix', values=(u'yes', u'no'))
option: Str('version?', exclude='webui')
output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (type 'unicode', type 'NoneType'), None)
diff --git a/VERSION b/VERSION
index a95ccb91457c4caf9767843951b8290b15b377d6..c713b18b7ce42db7fb290912ec6028342015f142 100644
--- a/VERSION
+++ b/VERSION
@@ -89,4 +89,4 @@ IPA_DATA_VERSION=2010061412
# #
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=59
+IPA_API_VERSION_MINOR=60
diff --git a/ipalib/plugins/trust.py b/ipalib/plugins/trust.py
index 3cb0ed98005ae5bd11b39f8ae01c9470d1bfc9c4..b096d1f1b2e223f12caa17d277a9ca03a1d0a667 100644
--- a/ipalib/plugins/trust.py
+++ b/ipalib/plugins/trust.py
@@ -290,6 +290,12 @@ sides.
default=20,
autofill=True
),
+StrEnum('use_posix?',
+cli_name='use_posix',
+label=_('Use POSIX attributes in ID range for the '
+'trusted domain'),
+values=(u'yes', u'no'),
+),
)
msg_summary = _('Added Active Directory trust for realm %(value)s')
@@ -330,23 +336,39 @@ sides.
dom_sid = new_obj['result']['ipanttrusteddomainsid'][0];
range_name = keys[-1].upper()+'_id_range'
+range_type = None
+
+# Force the given range type if --use-posix option was used
+if 'use_posix' in options:
+if options['use_posix'] == 'yes':
+range_type = u'ipa-ad-trust-posix'
+elif options['use_posix'] == 'no':
+
Re: [Freeipa-devel] [PATCH 0067] Add --use-posix option that forces trusted range type
On 06/20/2013 12:56 PM, Tomas Babej wrote: On 06/17/2013 02:34 PM, Ana Krivokapic wrote: On 06/06/2013 11:10 AM, Tomas Babej wrote: Hi, Adds --use-posix option to ipa trust-add command. It takes two allowed values: 'yes' : the 'ipa-ad-trust-posix' range type is enforced 'no' : the 'ipa-ad-trust' range type is enforced When --use-posix option is not specified, the range type should be determined by ID range discovery. https://fedorahosted.org/freeipa/ticket/3650 Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel The patch works nicely, but I have a few comments: 1) You added a new option to the API, but you forgot to bump the IPA_API_VERSION_MINOR in the VERSION file. 2) Typo in commit message: shold instead of should. 3) This construct: +if range_type is not None: +if range_type != old_range_type: can be replaced with a more readable variant which also avoids nested ifs: +if range_type and range_type != old_range_type: All fixed. 4) Some unit tests to cover the behavior of the newly added option would be nice. This is not doable at the moment, we have no unit test framework to test the trust-add command. -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Tomas ACK -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0067] Add --use-posix option that forces trusted range type
On 06/06/2013 11:10 AM, Tomas Babej wrote: Hi, Adds --use-posix option to ipa trust-add command. It takes two allowed values: 'yes' : the 'ipa-ad-trust-posix' range type is enforced 'no' : the 'ipa-ad-trust' range type is enforced When --use-posix option is not specified, the range type should be determined by ID range discovery. https://fedorahosted.org/freeipa/ticket/3650 Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel The patch works nicely, but I have a few comments: 1) You added a new option to the API, but you forgot to bump the IPA_API_VERSION_MINOR in the VERSION file. 2) Typo in commit message: shold instead of should. 3) This construct: +if range_type is not None: +if range_type != old_range_type: can be replaced with a more readable variant which also avoids nested ifs: +if range_type and range_type != old_range_type: 4) Some unit tests to cover the behavior of the newly added option would be nice. -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
