Re: [Freeipa-devel] [PATCH] 369 Added CLI param and ACL for vault service operations.
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 copy&pasted 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.
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 copy&pasted 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"
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', , Gettext('A
dictionary representing an LDA
output: Output('summary', (, ), 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', , None)
output: Output('failed', , None)
output: Entry('result', , 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', , Gettext('A
dictionary representing an LDA
output: Output('summary', (, ), None)
out
Re: [Freeipa-devel] [PATCH] 369 Added CLI param and ACL for vault service operations.
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 copy&pasted 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? -- Petr Vobornik -- 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.
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 copy&pasted 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 Cholasta
From 81ff19e8b3a5a8bdbb412155f4e3155e192befd3 Mon Sep 17 00:00:00 2001
From: "Endi S. Dewata"
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| 76 +++---
4 files changed, 70 insertions(+), 23 deletions(-)
diff --git a/API.txt b/API.txt
index 04f2f89..9dbf86a 100644
--- a/API.txt
+++ b/API.txt
@@ -5434,13 +5434,14 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
output: Output('summary', (, ), 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', , None)
output: Output('failed', , None)
output: Entry('result', , 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', , Gettext('A dictionary representing an LDA
output: Output('summary', (, ), None)
output: PrimaryKey('value', None, None)
command: vault_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, exclu
Re: [Freeipa-devel] [PATCH] 369 Added CLI param and ACL for vault service operations.
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 copy&pasted 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"
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', , Gettext('A
dictionary representing an LDA
output: Output('summary', (, ), 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', , None)
output: Output('failed', , None)
output: Entry('result', , 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', , Gettext('A
dictionary representing an LDA
output: Output('summary', (, ), 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?', cli_name='service')
option: Flag('shared?', autofill=True, default=False)
option: Str('user*', alwaysask=True, cli_name='users', csv=True)
Re: [Freeipa-devel] [PATCH] 369 Added CLI param and ACL for vault service operations.
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 copy&pasted 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.
On 08/04/2015 02:20 PM, Simo Sorce wrote:
> 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.
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 copy&pasted Vault plugins maintained
forever, differing in just that.
If you know how to do this without copypasting, I will be fine with that.
--
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.
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.
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.
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
Re: [Freeipa-devel] [PATCH] 369 Added CLI param and ACL for vault service operations.
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.
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.
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
[Freeipa-devel] [PATCH] 369 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
--
Endi S. Dewata
From 9259bb2da81d323a15398c678bfc58e32434364a Mon Sep 17 00:00:00 2001
From: "Endi S. Dewata"
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 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| 40 ++---
VERSION| 4 ++--
install/share/vault.update | 1 +
ipalib/plugins/vault.py| 56 ++
4 files changed, 67 insertions(+), 34 deletions(-)
diff --git a/API.txt b/API.txt
index
6ab30ddab41715fdbccb4f37aa1852621bca62b4..0e1525da26b3b0f850f338b7bf2a83b043c9d399
100644
--- a/API.txt
+++ b/API.txt
@@ -5407,7 +5407,7 @@ option: Str('password?', cli_name='password')
option: Str('password_file?', cli_name='password_file')
option: Str('public_key_file?', cli_name='public_key_file')
option: Flag('raw', autofill=True, cli_name='raw', default=False,
exclude='webui')
-option: Str('service?')
+option: Str('servicename?', cli_name='service')
option: Str('setattr*', cli_name='setattr', exclude='webui')
option: Flag('shared?', autofill=True, default=False)
option: Str('username?', cli_name='user')
@@ -5425,7 +5425,7 @@ option: Bytes('ipavaultsalt', attribute=True,
cli_name='salt', multivalue=False,
option: Str('ipavaulttype', attribute=True, autofill=True, cli_name='type',
default=u'standard', multivalue=False, required=False)
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('servicename?', cli_name='service')
option: Flag('shared?', autofill=True, default=False)
option: Str('username?', cli_name='user')
option: Str('version?', exclude='webui')
@@ -5433,13 +5433,14 @@ output: Entry('result', , Gettext('A
dictionary representing an LDA
output: Output('summary', (, ), 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('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)
option: Str('username?', cli_name='user')
@@ -5448,13 +5449,14 @@ output: Output('completed', , None)
output: Output('failed', , None)
output: Entry('result', , 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('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)
option: Str('username?', cli_name='user')
@@ -5471,7 +5473,7 @@ option: Str('in?')
option: Str('password?', cli_name='password')
option: Str('password_file?', cli_name='password_file')
option: Flag('raw', autofill=True, cli_name='raw', default=False,
exclude='webui')
-option: Str('service?')
+option: Str('servicename?', cli_name='service')
option: Flag('shared?', autofill=True, default=False)
option: Str('username?', cli_name='user')
option: Str('version?', exclude='webui')
@@ -5484,7 +5486,7 @@ arg: Str('cn', attribute=True, cli_name='name',
maxlength=255, mult
