Re: [Freeipa-devel] [PATCH] 356 Add trusconfig-show and trustconfig-mod commands

2013-02-11 Thread Martin Kosek
On 02/08/2013 05:30 PM, Alexander Bokovoy wrote:
 On Fri, 01 Feb 2013, Martin Kosek wrote:
 On 02/01/2013 03:55 PM, Alexander Bokovoy wrote:
 On Tue, 29 Jan 2013, Martin Kosek wrote:
 trust_output_params = (
 @@ -482,3 +499,158 @@ api.register(trust_mod)
 api.register(trust_del)
 api.register(trust_find)
 api.register(trust_show)
 +
 +
 +_trust_type_option = (
 +StrEnum('trust_type',
 +cli_name='type',
 +label=_('Trust type (ad for Active Directory, default)'),
 +values=(u'ad',),
 +default=u'ad',
 +autofill=True,
 +),
 +)
 We already have various trust type definitions in the same file. Maybe
 it makes sense to unify those somehow?

 Right, I unified those 2 separate trust_type option definitions.


 +def get_dn(self, *keys, **kwargs):
 +trust_type = kwargs.get('trust_type')
 +if trust_type is None:
 +raise errors.RequirementError(name='trust_type')
 +if kwargs['trust_type'] == u'ad':
 Perhaps better to define constants for the trust type values...

 I changed it a bit and now it uses a dict instead. I think its now more 
 general
 and extensible.


 +except ValueError:
 +# The search is performed for groups with posixgroup
 objectclass
 +# and not ipausergroup so that it can also match groups like
 +# Default SMG Group which does not have this objectclass.
 'Default SM_B_ Group'

 Fixed.


 Thanks for the unit tests too!


 You are welcome! I also generated API.txt which I forgot to do last time.
 Updated patch attached.
 ACK for the code but please add more documentation (below).
 
 Works like sharm. I tried also changing default fallback group to
 some IPA group, then back to Default SMB Group and it worked well. Also
 specifying non-existing group was noted and rejected.
 
 Please make sure to mention in the design page magic value 'Default SMB
 Group' and also that you can use any group with 'posixgroup'
 objectclass, and that 'Default SMB Group' is not visible through normal
 IPA tools.
 
 We need to write better documentation (online help) for trustconfig-mod.
 Basically, right now it helps no one to understand what is supposed to
 be done here.
 
 Once help is added, ACK.

Thanks for the review! RFE updated with information you mentioned.

I also added more info to trust online help (which you verified off-list).

Pushed to master, ipa-3-1.

Martin

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


Re: [Freeipa-devel] [PATCH] 356 Add trusconfig-show and trustconfig-mod commands

2013-02-08 Thread Alexander Bokovoy

On Fri, 01 Feb 2013, Martin Kosek wrote:

On 02/01/2013 03:55 PM, Alexander Bokovoy wrote:

On Tue, 29 Jan 2013, Martin Kosek wrote:

trust_output_params = (
@@ -482,3 +499,158 @@ api.register(trust_mod)
api.register(trust_del)
api.register(trust_find)
api.register(trust_show)
+
+
+_trust_type_option = (
+StrEnum('trust_type',
+cli_name='type',
+label=_('Trust type (ad for Active Directory, default)'),
+values=(u'ad',),
+default=u'ad',
+autofill=True,
+),
+)

We already have various trust type definitions in the same file. Maybe
it makes sense to unify those somehow?


Right, I unified those 2 separate trust_type option definitions.




+def get_dn(self, *keys, **kwargs):
+trust_type = kwargs.get('trust_type')
+if trust_type is None:
+raise errors.RequirementError(name='trust_type')
+if kwargs['trust_type'] == u'ad':

Perhaps better to define constants for the trust type values...


I changed it a bit and now it uses a dict instead. I think its now more general
and extensible.




+except ValueError:
+# The search is performed for groups with posixgroup objectclass
+# and not ipausergroup so that it can also match groups like
+# Default SMG Group which does not have this objectclass.

'Default SM_B_ Group'


Fixed.



Thanks for the unit tests too!



You are welcome! I also generated API.txt which I forgot to do last time.
Updated patch attached.

ACK for the code but please add more documentation (below).

Works like sharm. I tried also changing default fallback group to
some IPA group, then back to Default SMB Group and it worked well. Also
specifying non-existing group was noted and rejected.

Please make sure to mention in the design page magic value 'Default SMB
Group' and also that you can use any group with 'posixgroup'
objectclass, and that 'Default SMB Group' is not visible through normal
IPA tools.

We need to write better documentation (online help) for trustconfig-mod.
Basically, right now it helps no one to understand what is supposed to
be done here.

Once help is added, ACK.
--
/ Alexander Bokovoy

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


Re: [Freeipa-devel] [PATCH] 356 Add trusconfig-show and trustconfig-mod commands

2013-02-01 Thread Alexander Bokovoy

On Tue, 29 Jan 2013, Martin Kosek wrote:

trust_output_params = (
@@ -482,3 +499,158 @@ api.register(trust_mod)
api.register(trust_del)
api.register(trust_find)
api.register(trust_show)
+
+
+_trust_type_option = (
+StrEnum('trust_type',
+cli_name='type',
+label=_('Trust type (ad for Active Directory, default)'),
+values=(u'ad',),
+default=u'ad',
+autofill=True,
+),
+)

We already have various trust type definitions in the same file. Maybe
it makes sense to unify those somehow?


+def get_dn(self, *keys, **kwargs):
+trust_type = kwargs.get('trust_type')
+if trust_type is None:
+raise errors.RequirementError(name='trust_type')
+if kwargs['trust_type'] == u'ad':

Perhaps better to define constants for the trust type values...


+except ValueError:
+# The search is performed for groups with posixgroup objectclass
+# and not ipausergroup so that it can also match groups like
+# Default SMG Group which does not have this objectclass.

'Default SM_B_ Group'

Thanks for the unit tests too!


--
/ Alexander Bokovoy

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


Re: [Freeipa-devel] [PATCH] 356 Add trusconfig-show and trustconfig-mod commands

2013-02-01 Thread Martin Kosek
On 02/01/2013 03:55 PM, Alexander Bokovoy wrote:
 On Tue, 29 Jan 2013, Martin Kosek wrote:
 trust_output_params = (
 @@ -482,3 +499,158 @@ api.register(trust_mod)
 api.register(trust_del)
 api.register(trust_find)
 api.register(trust_show)
 +
 +
 +_trust_type_option = (
 +StrEnum('trust_type',
 +cli_name='type',
 +label=_('Trust type (ad for Active Directory, default)'),
 +values=(u'ad',),
 +default=u'ad',
 +autofill=True,
 +),
 +)
 We already have various trust type definitions in the same file. Maybe
 it makes sense to unify those somehow?

Right, I unified those 2 separate trust_type option definitions.

 
 +def get_dn(self, *keys, **kwargs):
 +trust_type = kwargs.get('trust_type')
 +if trust_type is None:
 +raise errors.RequirementError(name='trust_type')
 +if kwargs['trust_type'] == u'ad':
 Perhaps better to define constants for the trust type values...

I changed it a bit and now it uses a dict instead. I think its now more general
and extensible.

 
 +except ValueError:
 +# The search is performed for groups with posixgroup 
 objectclass
 +# and not ipausergroup so that it can also match groups like
 +# Default SMG Group which does not have this objectclass.
 'Default SM_B_ Group'

Fixed.

 
 Thanks for the unit tests too!
 

You are welcome! I also generated API.txt which I forgot to do last time.
Updated patch attached.

Martin
From 5296910eef8ef46c179f9cb0b10c3ebcc1d90a9f Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Fri, 25 Jan 2013 10:10:17 +0100
Subject: [PATCH] Add trusconfig-show and trustconfig-mod commands

Global trust configuration is generated ipa-adtrust-install script
is run. Add convenience commands to show auto-generated options
like SID or GUID or options chosen by user (NetBIOS). Most of these
options are not modifiable via trustconfig-mod command as it would
break current trusts.

Unit test file covering these new commands was added.

https://fedorahosted.org/freeipa/ticket/
---
 API.txt|  24 +
 VERSION|   2 +-
 ipalib/plugins/trust.py| 181 +++--
 tests/test_xmlrpc/test_trust_plugin.py | 159 +
 tests/test_xmlrpc/xmlrpc_test.py   |  10 ++
 5 files changed, 368 insertions(+), 8 deletions(-)
 create mode 100644 tests/test_xmlrpc/test_trust_plugin.py

diff --git a/API.txt b/API.txt
index 8fbfe6f5d8da44e991b8d1a36725fc6ace1f0616..6e5c8c5871bcfd320289291114c3c1534c400a54 100644
--- a/API.txt
+++ b/API.txt
@@ -3262,6 +3262,30 @@ 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)
 output: Output('value', type 'unicode', None)
+command: trustconfig_mod
+args: 0,9,3
+option: Str('addattr*', cli_name='addattr', exclude='webui')
+option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
+option: Str('delattr*', cli_name='delattr', exclude='webui')
+option: Str('ipantfallbackprimarygroup', attribute=True, autofill=False, cli_name='fallback_primary_group', multivalue=False, required=False)
+option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
+option: Flag('rights', autofill=True, default=False)
+option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: StrEnum('trust_type', autofill=True, cli_name='type', default=u'ad', values=(u'ad',))
+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)
+output: Output('value', type 'unicode', None)
+command: trustconfig_show
+args: 0,5,3
+option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
+option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
+option: Flag('rights', autofill=True, default=False)
+option: StrEnum('trust_type', autofill=True, cli_name='type', default=u'ad', values=(u'ad',))
+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)
+output: Output('value', type 'unicode', None)
 command: user_add
 args: 1,34,3
 arg: Str('uid', attribute=True, cli_name='login', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', primary_key=True, required=True)
diff --git a/VERSION b/VERSION
index 61f578dbfc9415f6f94a6612f198218c5a5e0c9a..37af5ef73b74500e0cd7397fb2c109332c049bc6 100644
--- a/VERSION
+++ b/VERSION
@@ -89,4 +89,4 @@ IPA_DATA_VERSION=2010061412
 #