Re: [Freeipa-devel] [PATCH] 369 Added CLI param and ACL for vault service operations.

2015-08-17 Thread Jan Cholasta

On 13.8.2015 18:23, Endi Sukma Dewata wrote:

On 8/13/2015 6:00 AM, Petr Vobornik wrote:

On 08/11/2015 08:42 AM, Jan Cholasta wrote:

On 10.8.2015 21:12, Endi Sukma Dewata wrote:

On 8/4/2015 10:32 AM, Endi Sukma Dewata wrote:

Martin, I do not think going on with business as usual is the right
thing to do here. We know this is going to bite.
I suggest Endy adds a *new* API if making it backwards compatible is
not
possible. The era of bumping whole API version must stop, the sooner
the
better.


My point is that we do not know yet how to do this kind of changes
long term.
So what I did not want to end up are 2 copypasted Vault plugins
maintained
forever, differing in just that.

If you know how to do this without copypasting, I will be fine with
that.


We probably can do it like this:
* the old plugin continues to provide Vault 1.0 functionality
* the new plugin will be a proxy to the old plugin except for the
parts
that have changed in Vault 1.1.

Or the other way around:
* the new plugin will provide Vault 1.1 functionality
* the old plugin will be a proxy to the new plugin except for the
parts
that needs to be maintained for Vault 1.0.

The first option is probably safer.

In any case, IPA 4.2.1 will only provide a single client for Vault
1.1,
but two services for Vault 1.0 and 1.1.


A new patch #369-1 is attached. It has been rebased on top of #372 and
#373 that fix the conflicting parameter while maintaining backward
compatibility.


I have modified the first version of the patch to maintain backward
compatibility and not require your patches #372 and #373. Should be much
easier to review. See attachment.


Jan approach seems better to me for 4.2.1. Endi, do you agree with the
changes? Could we proceed with the review?


Yes, please see the attached patch. I had to update it to remove buggy
code and revised the docs. I also had to rebase my other patches to make
sure they work with this patch.


Thanks, ACK.

Fixed commit message (removed the mention of servicename) and pushed to:
master: 0dd95a19ee87a04836f12ad4c1194ad31ac22b93
ipa-4-2: f2117475b8a49b37845529089ea2d5b48f27bfda

--
Jan Cholasta

--
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] 369 Added CLI param and ACL for vault service operations.

2015-08-13 Thread Endi Sukma Dewata

On 8/13/2015 6:00 AM, Petr Vobornik wrote:

On 08/11/2015 08:42 AM, Jan Cholasta wrote:

On 10.8.2015 21:12, Endi Sukma Dewata wrote:

On 8/4/2015 10:32 AM, Endi Sukma Dewata wrote:

Martin, I do not think going on with business as usual is the right
thing to do here. We know this is going to bite.
I suggest Endy adds a *new* API if making it backwards compatible is
not
possible. The era of bumping whole API version must stop, the sooner
the
better.


My point is that we do not know yet how to do this kind of changes
long term.
So what I did not want to end up are 2 copypasted Vault plugins
maintained
forever, differing in just that.

If you know how to do this without copypasting, I will be fine with
that.


We probably can do it like this:
* the old plugin continues to provide Vault 1.0 functionality
* the new plugin will be a proxy to the old plugin except for the parts
that have changed in Vault 1.1.

Or the other way around:
* the new plugin will provide Vault 1.1 functionality
* the old plugin will be a proxy to the new plugin except for the parts
that needs to be maintained for Vault 1.0.

The first option is probably safer.

In any case, IPA 4.2.1 will only provide a single client for Vault 1.1,
but two services for Vault 1.0 and 1.1.


A new patch #369-1 is attached. It has been rebased on top of #372 and
#373 that fix the conflicting parameter while maintaining backward
compatibility.


I have modified the first version of the patch to maintain backward
compatibility and not require your patches #372 and #373. Should be much
easier to review. See attachment.


Jan approach seems better to me for 4.2.1. Endi, do you agree with the
changes? Could we proceed with the review?


Yes, please see the attached patch. I had to update it to remove buggy 
code and revised the docs. I also had to rebase my other patches to make 
sure they work with this patch.


--
Endi S. Dewata
From c43df23159e5dafd47d5309b3b0f75de4870640b Mon Sep 17 00:00:00 2001
From: Endi S. Dewata edew...@redhat.com
Date: Tue, 11 Aug 2015 08:19:59 +0200
Subject: [PATCH] Added CLI param and ACL for vault service operations.

The CLIs to manage vault owners and members have been modified
to accept services with a new parameter. Due to name conflict,
the existing 'service' parameter has been renamed to
'servicename'.

A new ACL has been added to allow a service to create its own
service container.

https://fedorahosted.org/freeipa/ticket/5172
---
 API.txt|  12 ++-
 VERSION|   4 +-
 install/share/vault.update |   1 +
 ipalib/plugins/vault.py| 179 +
 4 files changed, 95 insertions(+), 101 deletions(-)

diff --git a/API.txt b/API.txt
index 
04f2f894f7667239d94a2a7278d0cc80704afeb5..9dbf86aedf2a1b62dabab21fb30bbceb2f0f237b
 100644
--- a/API.txt
+++ b/API.txt
@@ -5434,13 +5434,14 @@ output: Entry('result', type 'dict', Gettext('A 
dictionary representing an LDA
 output: Output('summary', (type 'unicode', type 'NoneType'), None)
 output: PrimaryKey('value', None, None)
 command: vault_add_member
-args: 1,9,3
+args: 1,10,3
 arg: Str('cn', attribute=True, cli_name='name', maxlength=255, 
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, 
required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
 option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
 option: Str('service?')
+option: Str('services', alwaysask=True, cli_name='services', csv=True, 
multivalue=True, required=False)
 option: Flag('shared?', autofill=True, default=False)
 option: Str('user*', alwaysask=True, cli_name='users', csv=True)
 option: Str('username?', cli_name='user')
@@ -5449,13 +5450,14 @@ output: Output('completed', type 'int', None)
 output: Output('failed', type 'dict', None)
 output: Entry('result', type 'dict', Gettext('A dictionary representing an 
LDAP entry', domain='ipa', localedir=None))
 command: vault_add_owner
-args: 1,9,3
+args: 1,10,3
 arg: Str('cn', attribute=True, cli_name='name', maxlength=255, 
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, 
required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
 option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
 option: Str('service?')
+option: Str('services', alwaysask=True, cli_name='services', csv=True, 
multivalue=True, required=False)
 option: Flag('shared?', autofill=True, default=False)
 option: Str('user*', alwaysask=True, cli_name='users', csv=True)
 option: Str('username?', cli_name='user')
@@ -5547,13 +5549,14 @@ output: Entry('result', 

Re: [Freeipa-devel] [PATCH] 369 Added CLI param and ACL for vault service operations.

2015-08-10 Thread Endi Sukma Dewata

On 8/4/2015 10:32 AM, Endi Sukma Dewata wrote:

Martin, I do not think going on with business as usual is the right
thing to do here. We know this is going to bite.
I suggest Endy adds a *new* API if making it backwards compatible is not
possible. The era of bumping whole API version must stop, the sooner the
better.


My point is that we do not know yet how to do this kind of changes
long term.
So what I did not want to end up are 2 copypasted Vault plugins
maintained
forever, differing in just that.

If you know how to do this without copypasting, I will be fine with that.


We probably can do it like this:
* the old plugin continues to provide Vault 1.0 functionality
* the new plugin will be a proxy to the old plugin except for the parts
that have changed in Vault 1.1.

Or the other way around:
* the new plugin will provide Vault 1.1 functionality
* the old plugin will be a proxy to the new plugin except for the parts
that needs to be maintained for Vault 1.0.

The first option is probably safer.

In any case, IPA 4.2.1 will only provide a single client for Vault 1.1,
but two services for Vault 1.0 and 1.1.


A new patch #369-1 is attached. It has been rebased on top of #372 and 
#373 that fix the conflicting parameter while maintaining backward 
compatibility.


--
Endi S. Dewata
From 7f461c8fe5d567e9ddad3684a60037cdd90e833c Mon Sep 17 00:00:00 2001
From: Endi S. Dewata edew...@redhat.com
Date: Thu, 30 Jul 2015 23:20:34 +0200
Subject: [PATCH] Added CLI param and ACL for vault service operations.

The CLIs to manage vault owners and members have been modified
to accept services in addition to users and groups. A new ACL
has been added to allow a service to create its own service
container.

https://fedorahosted.org/freeipa/ticket/5172
---
 API.txt| 12 
 VERSION|  4 ++--
 install/share/vault.update |  1 +
 ipalib/plugins/vault.py| 21 +++--
 4 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/API.txt b/API.txt
index 
9a777bd029d88f6882a9db341822544c6d1e7b5a..81527bf60bb440ddfdacb25d63e211b154182487
 100644
--- a/API.txt
+++ b/API.txt
@@ -5436,12 +5436,13 @@ output: Entry('result', type 'dict', Gettext('A 
dictionary representing an LDA
 output: Output('summary', (type 'unicode', type 'NoneType'), None)
 output: PrimaryKey('value', None, None)
 command: vault2_add_member
-args: 1,9,3
+args: 1,10,3
 arg: Str('cn', attribute=True, cli_name='name', maxlength=255, 
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, 
required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
 option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
+option: Str('service*', alwaysask=True, cli_name='services', csv=True)
 option: Str('servicename?', cli_name='service')
 option: Flag('shared?', autofill=True, default=False)
 option: Str('user*', alwaysask=True, cli_name='users', csv=True)
@@ -5451,12 +5452,13 @@ output: Output('completed', type 'int', None)
 output: Output('failed', type 'dict', None)
 output: Entry('result', type 'dict', Gettext('A dictionary representing an 
LDAP entry', domain='ipa', localedir=None))
 command: vault2_add_owner
-args: 1,9,3
+args: 1,10,3
 arg: Str('cn', attribute=True, cli_name='name', maxlength=255, 
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, 
required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
 option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
+option: Str('service*', alwaysask=True, cli_name='services', csv=True)
 option: Str('servicename?', cli_name='service')
 option: Flag('shared?', autofill=True, default=False)
 option: Str('user*', alwaysask=True, cli_name='users', csv=True)
@@ -5549,12 +5551,13 @@ output: Entry('result', type 'dict', Gettext('A 
dictionary representing an LDA
 output: Output('summary', (type 'unicode', type 'NoneType'), None)
 output: PrimaryKey('value', None, None)
 command: vault2_remove_member
-args: 1,9,3
+args: 1,10,3
 arg: Str('cn', attribute=True, cli_name='name', maxlength=255, 
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, 
required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
 option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
+option: Str('service*', alwaysask=True, cli_name='services', csv=True)
 option: Str('servicename?', 

Re: [Freeipa-devel] [PATCH] 369 Added CLI param and ACL for vault service operations.

2015-08-04 Thread Simo Sorce
On Tue, 2015-08-04 at 08:05 +0200, Martin Kosek wrote:
 On 08/03/2015 10:37 PM, Endi Sukma Dewata wrote:
  On 8/3/2015 2:47 PM, Martin Kosek wrote:
  On 08/03/2015 05:36 PM, Endi Sukma Dewata wrote:
  On 8/3/2015 2:31 AM, Martin Kosek wrote:
  On 07/31/2015 05:07 PM, Endi Sukma Dewata wrote:
  The CLIs to manage vault owners and members have been modified
  to accept services with a new parameter. Due to name conflict,
  the existing 'service' parameter has been renamed to
  'servicename'.
 
  A new ACL has been added to allow a service to create its own
  service container.
 
  https://fedorahosted.org/freeipa/ticket/5172
 
  I did not do a full review, I just saw some of the changes that made
  me worry -
  like renaming API command attribute. Wouldn't that make the older
  clients fail?
 
  See for example permission-* commands, we faced similar situation
  there and had
  to rename command attributes potentially used by old clients in the
  new format.
 
  Yes, it will break older clients. The problem is I cannot keep the legacy
  'service' parameter for backward compatibility:
 
   Str(
   'service?',
   cli_name='deprecated_service',
   doc=_('DEPRECATED: Service name of the service vault'),
   ),
   Str(
   'servicename?',
   cli_name='service',
   doc=_('Service name of the service vault'),
   ),
 
  because it will conflict with a new 'service' parameter:
 
  [Mon Aug 03 17:19:00.197040 2015] [wsgi:error] [pid 9409] ipa: ERROR:
  Failed to
  start IPA: cannot override NameSpace.service value Str('service?',
  cli_name='deprecated_service', doc=Gettext('DEPRECATED: Service name
  of the
  service vault', domain='ipa', localedir=None)) with Str('service*',
  alwaysask=True, cli_name='services', csv=True, doc=u'services to add',
  label=u'member service')
 
  that was added automatically when I added the 'service' attribute member:
 
   attribute_members = {
   'owner': ['user', 'group', 'service'],
   'member': ['user', 'group', 'service'],
   }
 
  If there's a way to use a different parameter name for the 'service'
  attribute
  member to avoid conflict with the legacy 'service' parameter please
  let me know.
 
  The other option is to force the client to upgrade to the same version.
 
  Please let me know. Thanks.
 
 
  Honza, do we have any other options than to break API between 4.2.0 and
  4.2.1?
  
  Another option is to keep 2 vault plugins. The old plugin (1.0) will handle 
  old
  IPA 4.2.0 clients. The new plugin (1.1) will handle the new IPA 4.2.1 
  clients.
  For this to work the plugins need to have different API URLs so they won't
  conflict/confuse the clients.
 
 We do not have that versions in FreeIPA (yet?).
 
  Please also note that my next patch that adds the ability to change vault 
  type,
  password, and keys will also require a client upgrade because the 
  functionality
  is mainly implemented on the client side. In this case API URL versioning 
  will
  be necessary.
 
 Adding new commands and/or attributes is a common thing in FreeIPA. We just do
 the work, bump the minor API version and that's it. We planned having better
 version support in FreeIPA 4.4, we will see how it goes.

Martin, I do not think going on with business as usual is the right
thing to do here. We know this is going to bite.
I suggest Endy adds a *new* API if making it backwards compatible is not
possible. The era of bumping whole API version must stop, the sooner the
better.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

-- 
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] 369 Added CLI param and ACL for vault service operations.

2015-08-04 Thread Martin Kosek
On 08/03/2015 10:37 PM, Endi Sukma Dewata wrote:
 On 8/3/2015 2:47 PM, Martin Kosek wrote:
 On 08/03/2015 05:36 PM, Endi Sukma Dewata wrote:
 On 8/3/2015 2:31 AM, Martin Kosek wrote:
 On 07/31/2015 05:07 PM, Endi Sukma Dewata wrote:
 The CLIs to manage vault owners and members have been modified
 to accept services with a new parameter. Due to name conflict,
 the existing 'service' parameter has been renamed to
 'servicename'.

 A new ACL has been added to allow a service to create its own
 service container.

 https://fedorahosted.org/freeipa/ticket/5172

 I did not do a full review, I just saw some of the changes that made
 me worry -
 like renaming API command attribute. Wouldn't that make the older
 clients fail?

 See for example permission-* commands, we faced similar situation
 there and had
 to rename command attributes potentially used by old clients in the
 new format.

 Yes, it will break older clients. The problem is I cannot keep the legacy
 'service' parameter for backward compatibility:

  Str(
  'service?',
  cli_name='deprecated_service',
  doc=_('DEPRECATED: Service name of the service vault'),
  ),
  Str(
  'servicename?',
  cli_name='service',
  doc=_('Service name of the service vault'),
  ),

 because it will conflict with a new 'service' parameter:

 [Mon Aug 03 17:19:00.197040 2015] [wsgi:error] [pid 9409] ipa: ERROR:
 Failed to
 start IPA: cannot override NameSpace.service value Str('service?',
 cli_name='deprecated_service', doc=Gettext('DEPRECATED: Service name
 of the
 service vault', domain='ipa', localedir=None)) with Str('service*',
 alwaysask=True, cli_name='services', csv=True, doc=u'services to add',
 label=u'member service')

 that was added automatically when I added the 'service' attribute member:

  attribute_members = {
  'owner': ['user', 'group', 'service'],
  'member': ['user', 'group', 'service'],
  }

 If there's a way to use a different parameter name for the 'service'
 attribute
 member to avoid conflict with the legacy 'service' parameter please
 let me know.

 The other option is to force the client to upgrade to the same version.

 Please let me know. Thanks.


 Honza, do we have any other options than to break API between 4.2.0 and
 4.2.1?
 
 Another option is to keep 2 vault plugins. The old plugin (1.0) will handle 
 old
 IPA 4.2.0 clients. The new plugin (1.1) will handle the new IPA 4.2.1 clients.
 For this to work the plugins need to have different API URLs so they won't
 conflict/confuse the clients.

We do not have that versions in FreeIPA (yet?).

 Please also note that my next patch that adds the ability to change vault 
 type,
 password, and keys will also require a client upgrade because the 
 functionality
 is mainly implemented on the client side. In this case API URL versioning will
 be necessary.

Adding new commands and/or attributes is a common thing in FreeIPA. We just do
the work, bump the minor API version and that's it. We planned having better
version support in FreeIPA 4.4, we will see how it goes.

Martin

-- 
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] 369 Added CLI param and ACL for vault service operations.

2015-08-04 Thread Endi Sukma Dewata

On 8/4/2015 8:51 AM, Martin Kosek wrote:

Please also note that my next patch that adds the ability to change vault type,
password, and keys will also require a client upgrade because the functionality
is mainly implemented on the client side. In this case API URL versioning will
be necessary.


Adding new commands and/or attributes is a common thing in FreeIPA. We just do
the work, bump the minor API version and that's it. We planned having better
version support in FreeIPA 4.4, we will see how it goes.


Martin, I do not think going on with business as usual is the right
thing to do here. We know this is going to bite.
I suggest Endy adds a *new* API if making it backwards compatible is not
possible. The era of bumping whole API version must stop, the sooner the
better.


My point is that we do not know yet how to do this kind of changes long term.
So what I did not want to end up are 2 copypasted Vault plugins maintained
forever, differing in just that.

If you know how to do this without copypasting, I will be fine with that.


We probably can do it like this:
* the old plugin continues to provide Vault 1.0 functionality
* the new plugin will be a proxy to the old plugin except for the parts 
that have changed in Vault 1.1.


Or the other way around:
* the new plugin will provide Vault 1.1 functionality
* the old plugin will be a proxy to the new plugin except for the parts 
that needs to be maintained for Vault 1.0.


The first option is probably safer.

In any case, IPA 4.2.1 will only provide a single client for Vault 1.1, 
but two services for Vault 1.0 and 1.1.


--
Endi S. Dewata

--
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] 369 Added CLI param and ACL for vault service operations.

2015-08-03 Thread Endi Sukma Dewata

On 8/3/2015 2:31 AM, Martin Kosek wrote:

On 07/31/2015 05:07 PM, Endi Sukma Dewata wrote:

The CLIs to manage vault owners and members have been modified
to accept services with a new parameter. Due to name conflict,
the existing 'service' parameter has been renamed to
'servicename'.

A new ACL has been added to allow a service to create its own
service container.

https://fedorahosted.org/freeipa/ticket/5172


I did not do a full review, I just saw some of the changes that made me worry -
like renaming API command attribute. Wouldn't that make the older clients fail?

See for example permission-* commands, we faced similar situation there and had
to rename command attributes potentially used by old clients in the new format.


Yes, it will break older clients. The problem is I cannot keep the 
legacy 'service' parameter for backward compatibility:


Str(
'service?',
cli_name='deprecated_service',
doc=_('DEPRECATED: Service name of the service vault'),
),
Str(
'servicename?',
cli_name='service',
doc=_('Service name of the service vault'),
),

because it will conflict with a new 'service' parameter:

[Mon Aug 03 17:19:00.197040 2015] [wsgi:error] [pid 9409] ipa: ERROR: 
Failed to start IPA: cannot override NameSpace.service value 
Str('service?', cli_name='deprecated_service', doc=Gettext('DEPRECATED: 
Service name of the service vault', domain='ipa', localedir=None)) with 
Str('service*', alwaysask=True, cli_name='services', csv=True, 
doc=u'services to add', label=u'member service')


that was added automatically when I added the 'service' attribute member:

attribute_members = {
'owner': ['user', 'group', 'service'],
'member': ['user', 'group', 'service'],
}

If there's a way to use a different parameter name for the 'service' 
attribute member to avoid conflict with the legacy 'service' parameter 
please let me know.


The other option is to force the client to upgrade to the same version.

Please let me know. Thanks.

--
Endi S. Dewata

--
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] 369 Added CLI param and ACL for vault service operations.

2015-08-03 Thread Martin Kosek
On 07/31/2015 05:07 PM, Endi Sukma Dewata wrote:
 The CLIs to manage vault owners and members have been modified
 to accept services with a new parameter. Due to name conflict,
 the existing 'service' parameter has been renamed to
 'servicename'.
 
 A new ACL has been added to allow a service to create its own
 service container.
 
 https://fedorahosted.org/freeipa/ticket/5172

I did not do a full review, I just saw some of the changes that made me worry -
like renaming API command attribute. Wouldn't that make the older clients fail?

See for example permission-* commands, we faced similar situation there and had
to rename command attributes potentially used by old clients in the new format.

-- 
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] 369 Added CLI param and ACL for vault service operations.

2015-08-03 Thread Martin Kosek

On 08/03/2015 05:36 PM, Endi Sukma Dewata wrote:

On 8/3/2015 2:31 AM, Martin Kosek wrote:

On 07/31/2015 05:07 PM, Endi Sukma Dewata wrote:

The CLIs to manage vault owners and members have been modified
to accept services with a new parameter. Due to name conflict,
the existing 'service' parameter has been renamed to
'servicename'.

A new ACL has been added to allow a service to create its own
service container.

https://fedorahosted.org/freeipa/ticket/5172


I did not do a full review, I just saw some of the changes that made me worry -
like renaming API command attribute. Wouldn't that make the older clients fail?

See for example permission-* commands, we faced similar situation there and had
to rename command attributes potentially used by old clients in the new format.


Yes, it will break older clients. The problem is I cannot keep the legacy
'service' parameter for backward compatibility:

 Str(
 'service?',
 cli_name='deprecated_service',
 doc=_('DEPRECATED: Service name of the service vault'),
 ),
 Str(
 'servicename?',
 cli_name='service',
 doc=_('Service name of the service vault'),
 ),

because it will conflict with a new 'service' parameter:

[Mon Aug 03 17:19:00.197040 2015] [wsgi:error] [pid 9409] ipa: ERROR: Failed to
start IPA: cannot override NameSpace.service value Str('service?',
cli_name='deprecated_service', doc=Gettext('DEPRECATED: Service name of the
service vault', domain='ipa', localedir=None)) with Str('service*',
alwaysask=True, cli_name='services', csv=True, doc=u'services to add',
label=u'member service')

that was added automatically when I added the 'service' attribute member:

 attribute_members = {
 'owner': ['user', 'group', 'service'],
 'member': ['user', 'group', 'service'],
 }

If there's a way to use a different parameter name for the 'service' attribute
member to avoid conflict with the legacy 'service' parameter please let me know.

The other option is to force the client to upgrade to the same version.

Please let me know. Thanks.



Honza, do we have any other options than to break API between 4.2.0 and 4.2.1?

--
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] 369 Added CLI param and ACL for vault service operations.

2015-08-03 Thread Endi Sukma Dewata

On 8/3/2015 2:47 PM, Martin Kosek wrote:

On 08/03/2015 05:36 PM, Endi Sukma Dewata wrote:

On 8/3/2015 2:31 AM, Martin Kosek wrote:

On 07/31/2015 05:07 PM, Endi Sukma Dewata wrote:

The CLIs to manage vault owners and members have been modified
to accept services with a new parameter. Due to name conflict,
the existing 'service' parameter has been renamed to
'servicename'.

A new ACL has been added to allow a service to create its own
service container.

https://fedorahosted.org/freeipa/ticket/5172


I did not do a full review, I just saw some of the changes that made
me worry -
like renaming API command attribute. Wouldn't that make the older
clients fail?

See for example permission-* commands, we faced similar situation
there and had
to rename command attributes potentially used by old clients in the
new format.


Yes, it will break older clients. The problem is I cannot keep the legacy
'service' parameter for backward compatibility:

 Str(
 'service?',
 cli_name='deprecated_service',
 doc=_('DEPRECATED: Service name of the service vault'),
 ),
 Str(
 'servicename?',
 cli_name='service',
 doc=_('Service name of the service vault'),
 ),

because it will conflict with a new 'service' parameter:

[Mon Aug 03 17:19:00.197040 2015] [wsgi:error] [pid 9409] ipa: ERROR:
Failed to
start IPA: cannot override NameSpace.service value Str('service?',
cli_name='deprecated_service', doc=Gettext('DEPRECATED: Service name
of the
service vault', domain='ipa', localedir=None)) with Str('service*',
alwaysask=True, cli_name='services', csv=True, doc=u'services to add',
label=u'member service')

that was added automatically when I added the 'service' attribute member:

 attribute_members = {
 'owner': ['user', 'group', 'service'],
 'member': ['user', 'group', 'service'],
 }

If there's a way to use a different parameter name for the 'service'
attribute
member to avoid conflict with the legacy 'service' parameter please
let me know.

The other option is to force the client to upgrade to the same version.

Please let me know. Thanks.



Honza, do we have any other options than to break API between 4.2.0 and
4.2.1?


Another option is to keep 2 vault plugins. The old plugin (1.0) will 
handle old IPA 4.2.0 clients. The new plugin (1.1) will handle the new 
IPA 4.2.1 clients. For this to work the plugins need to have different 
API URLs so they won't conflict/confuse the clients.


Please also note that my next patch that adds the ability to change 
vault type, password, and keys will also require a client upgrade 
because the functionality is mainly implemented on the client side. In 
this case API URL versioning will be necessary.


--
Endi S. Dewata

--
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