On 11/10/2011 10:30 AM, Martin Kosek wrote:
On Tue, 2011-11-08 at 20:41 +0100, Ondrej Hamada wrote:
https://fedorahosted.org/freeipa/ticket/1961
The 'Keytab' filed in output of all 'user-*' commands was changed to
'Kerberos keys available'. In order to do this change for 'user-*'
commands only, the flag 'has_keytab' had to be removed from common
output parametrs in ipalib/baseldap.py. This change also affected the
host.py and service.py, where the 'has_keytab' flag was added to their
local output params. Both host.py and service.py holds the old field
caption - 'Keytab' - because of compatibility with older clients.
Ondra, thanks for the patch. It looks OK, everything behaves as
expected.
I am still concerned about your patch formatting:
1) Patch naming does not follow FreeIPA conventions. You can check
others patch file names - mine, Rob's or Alexander's for example. The
patch file name should be freeipa-ohamada-2-<description>.patch. The
patch number should also be in your mail subject - it helps when
searching mails etc.
2) Patch title is wrong - you don't need to include [PATCH] in git
commit's title. This then makes it here twice.
3) Patch description is insufficient. I miss link to ticket and some
description. You only added it to the mail. When I am traversing FreeIPA
git logs, I must be able to quickly read what this patch does.
You would have seen all these conventions I wrote you about if you had
read some patches in freeipa-devel or had read some in FreeIPA git log.
Martin
Shame on me. Sorry for that.
Corrected patch attached.
--
Regards,
Ondrej Hamada
FreeIPA team
jabber: [email protected]
IRC: ohamada
From 3e5f5da083779866ab3d92d8d1cd789d417def26 Mon Sep 17 00:00:00 2001
From: Ondrej Hamada <[email protected]>
Date: Thu, 10 Nov 2011 13:46:16 +0100
Subject: [PATCH] Misleading Keytab field
The 'Keytab' field in output of all 'user-*' commands was changed
to 'Kerberos keys available'. In order to do this change for 'user-*'
commands only, the flag 'has_keytab' had to be removed from common
output parametrs in ipalib/baseldap.py. This change also affected the
host.py and service.py, where the 'has_keytab' flag was added to their
local output params. Both host.py and service.py holds the old field
caption - 'Keytab' - because of compatibility with older clients.
https://fedorahosted.org/freeipa/ticket/1961
---
ipalib/plugins/baseldap.py | 3 ---
ipalib/plugins/host.py | 3 +++
ipalib/plugins/service.py | 5 +++++
ipalib/plugins/user.py | 14 ++++++++++++++
4 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 88f312998d27ecd8311eb4204c95e1ca212afeb2..4fd5fe4a1e7ff2d8fac7d3a65379b4ae0c5eb554 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -36,9 +36,6 @@ from ipalib.util import json_serialize
from ipalib.dn import *
global_output_params = (
- Flag('has_keytab',
- label=_('Keytab'),
- ),
Flag('has_password',
label=_('Password'),
),
diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py
index 0f3f91565cedb699726421ec00cb8f7a93b821bb..6557880aa82598857251f3d8b80e6d3b326fbca6 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -162,6 +162,9 @@ def remove_fwd_ptr(ipaddr, host, domain, recordtype):
pass
host_output_params = (
+ Flag('has_keytab',
+ label=_('Keytab'),
+ ),
Str('managedby_host',
label='Managed by',
),
diff --git a/ipalib/plugins/service.py b/ipalib/plugins/service.py
index 048a6b4f04b761131108874fbd09962be33e8a80..dad3ded434d241ae55e1352889c577ba1a08d8c4 100644
--- a/ipalib/plugins/service.py
+++ b/ipalib/plugins/service.py
@@ -84,6 +84,9 @@ EXAMPLES:
""")
output_params = (
+ Flag('has_keytab',
+ label=_('Keytab'),
+ ),
Str('managedby_host',
label='Managed by',
),
@@ -358,6 +361,7 @@ class service_find(LDAPSearch):
member_attributes = ['managedby']
takes_options = LDAPSearch.takes_options
has_output_params = LDAPSearch.has_output_params + output_params
+
def pre_callback(self, ldap, filter, attrs_list, base_dn, scope, *args, **options):
# lisp style!
custom_filter = '(&(objectclass=ipaService)' \
@@ -392,6 +396,7 @@ class service_show(LDAPRetrieve):
doc=_('file to store certificate in'),
),
)
+ has_output_params = LDAPRetrieve.has_output_params + output_params
def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
self.obj.get_password_attributes(ldap, dn, entry_attrs)
diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index 273c68fb2457da7493227ae820c263ebf7598327..d3e63ef9a7bcf98d0bd34396b144134be38c17c3 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -68,6 +68,12 @@ EXAMPLES:
NO_UPG_MAGIC = '__no_upg__'
+user_output_params = (
+ Flag('has_keytab',
+ label=_('Kerberos keys available'),
+ ),
+ )
+
def validate_nsaccountlock(entry_attrs):
if 'nsaccountlock' in entry_attrs:
nsaccountlock = entry_attrs['nsaccountlock']
@@ -352,6 +358,8 @@ class user_add(LDAPCreate):
msg_summary = _('Added user "%(value)s"')
+ has_output_params = LDAPCreate.has_output_params + user_output_params
+
takes_options = LDAPCreate.takes_options + (
Flag('noprivate',
cli_name='noprivate',
@@ -477,6 +485,8 @@ class user_mod(LDAPUpdate):
msg_summary = _('Modified user "%(value)s"')
+ has_output_params = LDAPUpdate.has_output_params + user_output_params
+
def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
if 'mail' in entry_attrs:
entry_attrs['mail'] = self.obj._normalize_email(entry_attrs['mail'])
@@ -498,6 +508,7 @@ class user_find(LDAPSearch):
__doc__ = _('Search for users.')
member_attributes = ['memberof']
+ has_output_params = LDAPSearch.has_output_params + user_output_params
takes_options = LDAPSearch.takes_options + (
Flag('whoami',
@@ -532,6 +543,8 @@ api.register(user_find)
class user_show(LDAPRetrieve):
__doc__ = _('Display information about a user.')
+ has_output_params = LDAPRetrieve.has_output_params + user_output_params
+
def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
convert_nsaccountlock(entry_attrs)
self.obj._convert_manager(entry_attrs, **options)
@@ -566,6 +579,7 @@ class user_enable(LDAPQuery):
__doc__ = _('Enable a user account.')
has_output = output.standard_value
+ has_output_params = LDAPQuery.has_output_params + user_output_params
msg_summary = _('Enabled user account "%(value)s"')
def execute(self, *keys, **options):
--
1.7.6.4
_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel
