On 07/20/2016 10:50 AM, Jan Cholasta wrote:
On 20.7.2016 10:26, Florence Blanc-Renaud wrote:
On 07/18/2016 02:52 PM, Florence Blanc-Renaud wrote:
On 07/18/2016 08:20 AM, Jan Cholasta wrote:
Hi,

On 7.7.2016 16:40, Florence Blanc-Renaud wrote:
On 07/07/2016 01:23 PM, Petr Vobornik wrote:
On 07/05/2016 02:38 PM, Florence Blanc-Renaud wrote:
Hi,

the output of ipa selinuxusermap-add-hostgroup and
selinuxusermap-add-user does not display any more the host/host
group or
user/group that could not be added. This patch fixes this
regression by
adding the labels host/hostgroup/user/group to the list of
_failed_member_output_params of the class ClientMethod.


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


I've a feeling that this issue is more general and multiple commands
regressed. Would be good to check other member options, e.g. also in
user plugin.

Hi Petr,

you are right, a lot of other commands regressed. So far I checked
only
user and sudocmd but it is likely to be a long task. Are there
regression tests that could help me make sure that the fix is
exhaustive?

Flo

See attachment for a patch with an universal fix.

Honza

Hi Honza,

the patch fixes most of the issues. I still see some CLI that do not
print everything (while they used to before the regression):
ipa servicedelegationrule-add-member
ipa servicedelegationrule-remove-member
ipa servicedelegationtarget-add-member
ipa servicedelegationtarget-remove-member

And the following CLI do not print the failed members (but they never
did):
ipa automember-add-condition
ipa automember-remove-condition
ipa sudorule-add-allow-command
ipa sudorule-remove-allow-command
ipa sudorule-add-deny-command
ipa sudorule-remove-deny-command

It is probably ok to commit this patch and investigate in another ticket
the remaining issues,
Flo.


Hi,
please find a new version of the patch, thanks to Jan's help. This
version also fixes servicedelegation commands.

I would rather keep the patches separate, as the fixes are different.
Otherwise LGTM.

Hi Honza,

please find an updated version which handles only servicedelegation issue (I also attached your original patch for reference).
For the reviewers: both have to be applied to completely fix the ticket.
Flo.
>From 2fc614eac6465af6d55580c51600d287bf1160f5 Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud <f...@redhat.com>
Date: Wed, 20 Jul 2016 11:02:30 +0200
Subject: [PATCH] Show full error message for selinuxusermap-add-hostgroup

While investigating the issue for selinuxusermap-add-hostgroup,
we discovered that other commands were missing output.
A first patch fixes most of the issues:
freeipa-jcholast-677-frontend-copy-command-arguments-to-output-params-on-.patch

This patch fixes servicedelegation CLI, where
servicedelegation.takes_params was missing
ipaallowedtarget_servicedelegationtarget, ipaallowedtoimpersonate and
memberprincipal

https://fedorahosted.org/freeipa/ticket/6026
---
 ipaserver/plugins/servicedelegation.py | 53 ++++++++++------------------------
 1 file changed, 15 insertions(+), 38 deletions(-)

diff --git a/ipaserver/plugins/servicedelegation.py b/ipaserver/plugins/servicedelegation.py
index 958c3b739a2dd465c2b685672c3deb1af8c36e4e..6f38c36a30363755c80081d02bf4c86d829eae34 100644
--- a/ipaserver/plugins/servicedelegation.py
+++ b/ipaserver/plugins/servicedelegation.py
@@ -96,30 +96,6 @@ PROTECTED_CONSTRAINT_TARGETS = (
 )
 
 
-output_params = (
-    Str(
-        'ipaallowedtarget_servicedelegationtarget',
-        label=_('Allowed Target'),
-    ),
-    Str(
-        'ipaallowedtoimpersonate',
-        label=_('Allowed to Impersonate'),
-    ),
-    Str(
-        'memberprincipal',
-        label=_('Member principals'),
-    ),
-    Str(
-        'failed_memberprincipal',
-        label=_('Failed members'),
-    ),
-    Str(
-        'ipaallowedtarget',
-        label=_('Failed targets'),
-    ),
-)
-
-
 class servicedelegation(LDAPObject):
     """
     Service Constrained Delegation base object.
@@ -175,6 +151,21 @@ class servicedelegation(LDAPObject):
             label=_('Delegation name'),
             primary_key=True,
         ),
+        Str(
+            'ipaallowedtarget_servicedelegationtarget',
+            label=_('Allowed Target'),
+            flags={'virtual_attribute', 'no_create', 'no_update', 'no_search'},
+        ),
+        Str(
+            'ipaallowedtoimpersonate',
+            label=_('Allowed to Impersonate'),
+            flags={'no_create', 'no_update', 'no_search'},
+        ),
+        Str(
+            'memberprincipal',
+            label=_('Member principals'),
+            flags={'no_create', 'no_update', 'no_search'},
+        ),
     )
 
 
@@ -186,8 +177,6 @@ class servicedelegation_add_member(LDAPAddMember):
     principal_attr = 'memberprincipal'
     principal_failedattr = 'failed_memberprincipal'
 
-    has_output_params = LDAPAddMember.has_output_params + output_params
-
     def get_options(self):
         for option in super(servicedelegation_add_member, self).get_options():
             yield option
@@ -268,8 +257,6 @@ class servicedelegation_remove_member(LDAPRemoveMember):
     principal_attr = 'memberprincipal'
     principal_failedattr = 'failed_memberprincipal'
 
-    has_output_params = LDAPRemoveMember.has_output_params + output_params
-
     def get_options(self):
         for option in super(
                 servicedelegation_remove_member, self).get_options():
@@ -397,8 +384,6 @@ class servicedelegationrule_del(LDAPDelete):
 class servicedelegationrule_find(LDAPSearch):
     __doc__ = _('Search for service delegations rule.')
 
-    has_output_params = LDAPSearch.has_output_params + output_params
-
     msg_summary = ngettext(
         '%(count)d service delegation rule matched',
         '%(count)d service delegation rules matched', 0
@@ -409,8 +394,6 @@ class servicedelegationrule_find(LDAPSearch):
 class servicedelegationrule_show(LDAPRetrieve):
     __doc__ = _('Display information about a named service delegation rule.')
 
-    has_output_params = LDAPRetrieve.has_output_params + output_params
-
 
 @register()
 class servicedelegationrule_add_member(servicedelegation_add_member):
@@ -437,7 +420,6 @@ class servicedelegationrule_add_target(LDAPAddMember):
     attribute_members = {
         'ipaallowedtarget': ['servicedelegationtarget'],
     }
-    has_output_params = LDAPAddMember.has_output_params + output_params
 
 
 @register()
@@ -447,7 +429,6 @@ class servicedelegationrule_remove_target(LDAPRemoveMember):
     attribute_members = {
         'ipaallowedtarget': ['servicedelegationtarget'],
     }
-    has_output_params = LDAPRemoveMember.has_output_params + output_params
 
 
 @register()
@@ -492,8 +473,6 @@ class servicedelegationtarget_del(LDAPDelete):
 class servicedelegationtarget_find(LDAPSearch):
     __doc__ = _('Search for service delegation target.')
 
-    has_output_params = LDAPSearch.has_output_params + output_params
-
     msg_summary = ngettext(
         '%(count)d service delegation target matched',
         '%(count)d service delegation targets matched', 0
@@ -530,8 +509,6 @@ class servicedelegationtarget_find(LDAPSearch):
 class servicedelegationtarget_show(LDAPRetrieve):
     __doc__ = _('Display information about a named service delegation target.')
 
-    has_output_params = LDAPRetrieve.has_output_params + output_params
-
 
 @register()
 class servicedelegationtarget_add_member(servicedelegation_add_member):
-- 
2.7.4

>From 032a36129518b8f82551fea0c0adac7d8df55e9b Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Mon, 18 Jul 2016 07:37:31 +0200
Subject: [PATCH] frontend: copy command arguments to output params on client

In commit f554078291d682d59956998af97f7d3066fbe7e7 we stopped copying
command arguments to output params in order to remove redundancies and
reduce API schema in size. Since then, output params were removed from
API schema completely and are reconstructed on the client.

Not including arguments in output params hides failed members from member
commands' CLI output. To fix this, copy arguments to output params again,
but only on the client side.

https://fedorahosted.org/freeipa/ticket/6026
---
 ipaclient/frontend.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/ipaclient/frontend.py b/ipaclient/frontend.py
index e8eacc0..1525c88 100644
--- a/ipaclient/frontend.py
+++ b/ipaclient/frontend.py
@@ -95,6 +95,10 @@ class ClientMethod(ClientCommand, Method):
 
     def get_output_params(self):
         seen = set()
+        for param in self.params():
+            if param.name not in self.obj.params:
+                seen.add(param.name)
+                yield param
         for output_param in super(ClientMethod, self).get_output_params():
             seen.add(output_param.name)
             yield output_param
-- 
2.7.4

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

Reply via email to