[Freeipa-devel] [PATCH 0015] use ipaplatform.paths in kdc.conf.template

[Timo Aaltonen]

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

-- 
t
From 5798e8c04e716bc6fad01c8ea87473a1859eea28 Mon Sep 17 00:00:00 2001
From: Timo Aaltonen 
Date: Wed, 23 Mar 2016 00:32:52 +0200
Subject: [PATCH] Fix kdc.conf.template to use ipaplatform.paths.

https://fedorahosted.org/freeipa/ticket/5343
---
 install/share/kdc.conf.template  | 10 +-
 ipaplatform/base/paths.py|  3 +++
 ipaserver/install/krbinstance.py |  7 ++-
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/install/share/kdc.conf.template b/install/share/kdc.conf.template
index 0a51162..296b75b 100644
--- a/install/share/kdc.conf.template
+++ b/install/share/kdc.conf.template
@@ -8,10 +8,10 @@
   master_key_type = aes256-cts
   max_life = 7d
   max_renewable_life = 14d
-  acl_file = /var/kerberos/krb5kdc/kadm5.acl
-  dict_file = /usr/share/dict/words
+  acl_file = $KRB5KDC_KADM5_ACL
+  dict_file = $DICT_WORDS
   default_principal_flags = +preauth
-;  admin_keytab = /var/kerberos/krb5kdc/kadm5.keytab
-  pkinit_identity = FILE:/var/kerberos/krb5kdc/kdc.pem
-  pkinit_anchors = FILE:/var/kerberos/krb5kdc/cacert.pem
+;  admin_keytab = $KRB5KDC_KADM5_KEYTAB
+  pkinit_identity = FILE:$KDC_PEM
+  pkinit_anchors = FILE:$CACERT_PEM
  }
diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index 6f5806d..1b79015 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -237,10 +237,13 @@ class BasePathNamespace(object):
 SCHEMA_COMPAT_ULDIF = "/usr/share/ipa/schema_compat.uldif"
 IPA_JS_PLUGINS_DIR = "/usr/share/ipa/ui/js/plugins"
 UPDATES_DIR = "/usr/share/ipa/updates/"
+DICT_WORDS = "/usr/share/dict/words"
 CACHE_IPA_SESSIONS = "/var/cache/ipa/sessions"
 VAR_KERBEROS_KRB5KDC_DIR = "/var/kerberos/krb5kdc/"
 VAR_KRB5KDC_K5_REALM = "/var/kerberos/krb5kdc/.k5."
 CACERT_PEM = "/var/kerberos/krb5kdc/cacert.pem"
+KRB5KDC_KADM5_ACL = "/var/kerberos/krb5kdc/kadm5.acl"
+KRB5KDC_KADM5_KEYTAB = "/var/kerberos/krb5kdc/kadm5.keytab"
 KRB5KDC_KDC_CONF = "/var/kerberos/krb5kdc/kdc.conf"
 KDC_PEM = "/var/kerberos/krb5kdc/kdc.pem"
 VAR_LIB = "/var/lib"
diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py
index 03e3ed8..f560a6e 100644
--- a/ipaserver/install/krbinstance.py
+++ b/ipaserver/install/krbinstance.py
@@ -221,7 +221,12 @@ class KrbInstance(service.Service):
  DOMAIN=self.domain,
  HOST=self.host,
  SERVER_ID=installutils.realm_to_serverid(self.realm),
- REALM=self.realm)
+ REALM=self.realm,
+ KRB5KDC_KADM5_ACL=paths.KRB5KDC_KADM5_ACL,
+ DICT_WORDS=paths.DICT_WORDS,
+ KRB5KDC_KADM5_KEYTAB=paths.KRB5KDC_KADM5_KEYTAB,
+ KDC_PEM=paths.KDC_PEM,
+ CACERT_PEM=paths.CACERT_PEM)
 
 # IPA server/KDC is not a subdomain of default domain
 # Proper domain-realm mapping needs to be specified
-- 
2.7.3

-- 
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 0099] Look up HTTPD_USER's UID and GID during installation.

[Timo Aaltonen]

22.03.2016, 14:36, David Kupka kirjoitti:
> https://fedorahosted.org/freeipa/ticket/5712

sweet, thanks!


-- 
t

-- 
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 0012-0013] Improve ipaplatform.constants

[Timo Aaltonen]

18.03.2016, 12:30, Timo Aaltonen kirjoitti:
> 
> Fix some hardcoded uid/gid strings to help with porting.

rebased and simplified against current master.


-- 
t
From 424d3cf28f92a624b9970701a341dfa26370f616 Mon Sep 17 00:00:00 2001
From: Timo Aaltonen 
Date: Fri, 18 Mar 2016 12:22:33 +0200
Subject: [PATCH] ipaplatform: Move remaining user/group constants to
 ipaplatform.constants.

Use ipaplatform.constants in every corner instead of importing other bits or calling
some platform specific things, and remove most of the remaining hardcoded uid's.
---
 install/oddjob/com.redhat.idm.trust-fetch-domains |  3 ++-
 ipaplatform/base/constants.py |  5 +
 ipaplatform/base/services.py  | 12 ---
 ipaplatform/redhat/services.py| 26 ---
 ipaserver/install/bindinstance.py |  2 +-
 ipaserver/install/dns.py  |  4 ++--
 ipaserver/install/dnskeysyncinstance.py   |  9 
 ipaserver/install/dogtaginstance.py   |  1 -
 ipaserver/install/httpinstance.py |  2 +-
 ipaserver/install/odsexporterinstance.py  |  5 +++--
 ipaserver/install/opendnssecinstance.py   | 15 +++--
 11 files changed, 27 insertions(+), 57 deletions(-)

diff --git a/install/oddjob/com.redhat.idm.trust-fetch-domains b/install/oddjob/com.redhat.idm.trust-fetch-domains
index 6e8bfc6..7c70c41 100755
--- a/install/oddjob/com.redhat.idm.trust-fetch-domains
+++ b/install/oddjob/com.redhat.idm.trust-fetch-domains
@@ -8,6 +8,7 @@ from ipapython.dn import DN
 from ipalib.config import Env
 from ipalib.constants import DEFAULT_CONFIG
 from ipapython.ipautil import kinit_keytab
+from ipaplatform.constants import constants
 import sys
 import os
 import pwd
@@ -31,7 +32,7 @@ def retrieve_keytab(api, ccache_name, oneway_keytab_name, oneway_principal):
 raiseonerr=False)
 # Make sure SSSD is able to read the keytab
 try:
-sssd = pwd.getpwnam('sssd')
+sssd = pwd.getpwnam(constants.SSSD_USER)
 os.chown(oneway_keytab_name, sssd[2], sssd[3])
 except KeyError as e:
 # If user 'sssd' does not exist, we don't need to chown from root to sssd
diff --git a/ipaplatform/base/constants.py b/ipaplatform/base/constants.py
index 52af124..3e1c4c6 100644
--- a/ipaplatform/base/constants.py
+++ b/ipaplatform/base/constants.py
@@ -12,12 +12,17 @@ class BaseConstantsNamespace(object):
 DS_GROUP = 'dirsrv'
 HTTPD_USER = "apache"
 IPA_DNS_PACKAGE_NAME = "freeipa-server-dns"
+KDCPROXY_USER = "kdcproxy"
 NAMED_USER = "named"
+NAMED_GROUP = "named"
 PKI_USER = 'pkiuser'
 PKI_GROUP = 'pkiuser'
 # ntpd init variable used for daemon options
 NTPD_OPTS_VAR = "OPTIONS"
 # quote used for daemon options
 NTPD_OPTS_QUOTE = "\""
+ODS_USER = "ods"
+ODS_GROUP = "ods"
 # nfsd init variable used to enable kerberized NFS
 SECURE_NFS_VAR = "SECURE_NFS"
+SSSD_USER = "sssd"
diff --git a/ipaplatform/base/services.py b/ipaplatform/base/services.py
index 11d0c2a..641a654 100644
--- a/ipaplatform/base/services.py
+++ b/ipaplatform/base/services.py
@@ -181,18 +181,6 @@ class PlatformService(object):
 def get_config_dir(self, instance_name=""):
 return
 
-def get_user_name(self, instance_name=""):
-return
-
-def get_group_name(self, instance_name=""):
-return
-
-def get_binary_path(self):
-return
-
-def get_package_name(self):
-return
-
 
 class SystemdService(PlatformService):
 SYSTEMD_SRV_TARGET = "%s.target.wants"
diff --git a/ipaplatform/redhat/services.py b/ipaplatform/redhat/services.py
index 3c18dbc..92dae45 100644
--- a/ipaplatform/redhat/services.py
+++ b/ipaplatform/redhat/services.py
@@ -223,28 +223,6 @@ class RedHatCAService(RedHatService):
 self.wait_until_running()
 
 
-class RedHatNamedService(RedHatService):
-def get_user_name(self):
-return u'named'
-
-def get_group_name(self):
-return u'named'
-
-def get_binary_path(self):
-return paths.NAMED_PKCS11
-
-def get_package_name(self):
-return u"bind-pkcs11"
-
-
-class RedHatODSEnforcerdService(RedHatService):
-def get_user_name(self):
-return u'ods'
-
-def get_group_name(self):
-return u'ods'
-
-
 # Function that constructs proper Red Hat OS family-specific server classes for
 # services of specified name
 
@@ -257,10 +235,6 @@ def redhat_service_class_factory(name):
 return RedHatSSHService(name)
 if name in ('pki-tomcatd', 'pki_tomcatd'):
 return RedHatCAService(name)
-if name == 'named':
-return RedHatNamedService(name)
-if name in ('ods-enforcerd', 'ods_enforcerd'):
-return RedHatODSEnforcerdService(name)
 return RedHatService(name)
 
 
diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index 

Re: [Freeipa-devel] [PATCH] 0005 webui: topology graph: canvas resizes itself according to the window size

[Petr Vobornik]

On 03/21/2016 06:57 PM, Pavel Vomacka wrote:



1.
-width: 960,
-height: 500,

Graph even without this patch allows to set initial size in a
constructor, e.g.:

E.g. so he could also use:
  this.graph = new topology_graph.TopoGraph({
 nodes: data.nodes,
 links: data.links,
 suffixes: data.suffixes
 height: height,
 width: width
 });

IMO we should leave some default size there, e.g. the old 960x500 so
that the graph is shown even without explicit configuration.


Ok, I put the default size back, but into graph specification as you
write here.


Ah, I badly expressed myself, sorry. I wanted to leave the original
code on its place(TopoGraph). The above was just example what is
possible with or without the change because it is not obvious from code.

Default size is returned back now.


ACK

pushed to

master:
* e45f7314e1a2276671435703e190c8dabb320739 Resize topology graph canvas 
according to window size

ipa-4-3:
* ffdd64732b0325747b7922b0c9ce5a16a2b5652e Resize topology graph canvas 
according to window size

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

[Freeipa-devel] [DRAFT] FreeIPA 4.3.1 release notes

[Petr Vobornik]

Hello all,

I prepared the release notes on FreeIPA.org wiki:
http://www.freeipa.org/page/Releases/4.3.1

Updates or improvements to release notes page welcome. Particularly if
you think some bug fixes/improvements deserves to be noted out as a
highlight, please give a suggestion or edit the page directly

Thanks,
--
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 0029] Move user/group constants for PKI and DS into ipaplatform

[Martin Basti]

On 22.03.2016 10:43, Martin Basti wrote:



On 18.03.2016 11:53, Christian Heimes wrote:

On 2016-03-18 10:22, Martin Basti wrote:


On 29.02.2016 16:02, David Kupka wrote:

Hello Christian,
sorry for letting this patch rot for so long. I've forget about it 
the minute Fraser replied.
To compensate a little I've fixed pep8 error, rebased it and 
attaching two versions for master and for 4.3 branch.
I haven't found any missing cases and it works for me. If you're OK 
with the modified patches it can be pushed.


David

- Original Message -
From: "Christian Heimes" 
To: "Fraser Tweedale" 
Cc: "freeipa-devel" 
Sent: Wednesday, January 20, 2016 11:57:42 AM
Subject: Re: [Freeipa-devel] [PATCH 0029] Move user/group constants 
for PKI and DS into ipaplatform


On 2016-01-20 02:54, Fraser Tweedale wrote:

On Tue, Jan 19, 2016 at 02:20:27PM +0100, Christian Heimes wrote:
ipaplatform.constants has platform specific names for a couple of 
system
users like Apache HTTPD. The user names for PKI_USER, PKI_GROUP, 
DS_USER
and DS_GROUP are defined in other modules. Similar to #5587 the 
patch my

patch moves the constants into the platform module.

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

I see a few remaining cases:

ipaserver/install/dsinstance.py
712:pent = pwd.getpwnam("dirsrv")

ipatests/test_integration/test_backup_and_restore.py
167:self.master.run_command(['userdel', 'dirsrv'])
168:self.master.run_command(['userdel', 'pkiuser'])

ipaplatform/redhat/tasks.py
441:if name == 'pkiuser':

When these are included, ACK.

Good catch!

My new patch takes care of remaining cases.





Christian do you agree with proposed changes, can we push it?
Martin^2

Oh, the patch is still open? ACK!



Pushed to ipa-4-3: e3bf65f2df9c50873f0967b96a6a2a5975a87f79
Pushed to master: 49be6c8d3cc20902dbe8e92a74e31aed2fd21d9f


too-late-NACK

This patch broke ipa-restore.

please not that 2 modules are imported as same name in ipa_restore.py
from ipalib import api, errors, constants
from ipaplatform.constants import constants

2016-03-22T16:56:27Z DEBUG   File 
"/usr/lib/python2.7/site-packages/ipapython/admintool.py", line 171, in 
execute

return_value = self.run()
  File 
"/usr/lib/python2.7/site-packages/ipaserver/install/ipa_restore.py", 
line 218, in run

self.backup_dir, constants.FQDN)

Martin^2


--
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 0451] regression fix: catch Exception instead of more specific exception types

[Martin Basti]

On 22.03.2016 17:32, Martin Babinsky wrote:

On 03/22/2016 05:28 PM, Martin Basti wrote:

Patch attached, it fixes regression caused by mbasti-442 patch (master
only).



ACK


Pushed to master: d1e29fe60e75a6f40a780eb45e86f2ed87d00e5a

--
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 0451] regression fix: catch Exception instead of more specific exception types

[Martin Babinsky]

On 03/22/2016 05:28 PM, Martin Basti wrote:

Patch attached, it fixes regression caused by mbasti-442 patch (master
only).



ACK

--
Martin^3 Babinsky

--
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 0451] regression fix: catch Exception instead of more specific exception types

[Martin Basti]

Patch attached, it fixes regression caused by mbasti-442 patch (master 
only).
From cedddc4cf22fd4ccab44346e4a7472b9d16e390f Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Tue, 22 Mar 2016 16:39:39 +0100
Subject: [PATCH] Fix: catch Exception instead of more specific exception types

Regression caused by commit 491447cc5ab8c5eff2be57d609201cefb79f7053,
ValueErrori and AttributeError are too much specific for these cases, multiple types of
exception can be raised.
---
 ipalib/plugins/stageuser.py | 4 ++--
 ipapython/config.py | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/ipalib/plugins/stageuser.py b/ipalib/plugins/stageuser.py
index 510b66c43ef3d3b4bbd99637814809c75e15d450..9b40adf1c514e491543ace86ba10630f115a4c57 100644
--- a/ipalib/plugins/stageuser.py
+++ b/ipalib/plugins/stageuser.py
@@ -565,7 +565,7 @@ class stageuser_activate(LDAPQuery):
 try:
 v.decode('utf-8')
 self.log.debug("merge: %s:%r wiped" % (attr, v))
-except ValueError:
+except Exception:
 self.log.debug("merge %s: [no_print %s]" % (attr, v.__class__.__name__))
 if isinstance(entry_to[attr], (list, tuple)):
 # multi value attribute
@@ -581,7 +581,7 @@ class stageuser_activate(LDAPQuery):
 try:
 v.decode('utf-8')
 self.log.debug("Add: %s:%r" % (attr, v))
-except ValueError:
+except Exception:
 self.log.debug("Add %s: [no_print %s]" % (attr, v.__class__.__name__))
 
 if isinstance(entry_to[attr], (list, tuple)):
diff --git a/ipapython/config.py b/ipapython/config.py
index 0b70d057955f647acd6164895daa15ee96019d54..70afaffa2431271d3a00fdfad6e4d1a3be363f99 100644
--- a/ipapython/config.py
+++ b/ipapython/config.py
@@ -160,7 +160,7 @@ def __parse_config(discover_server = True):
 try:
 if not config.default_realm:
 config.default_realm = p.get("global", "realm")
-except AttributeError:
+except Exception:
 pass
 if discover_server:
 try:
@@ -172,7 +172,7 @@ def __parse_config(discover_server = True):
 try:
 if not config.default_domain:
 config.default_domain = p.get("global", "domain")
-except AttributeError:
+except Exception:
 pass
 
 def __discover_config(discover_server = True):
-- 
2.5.0

-- 
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 0432, 0450] stageuser-activate: noralize manager value + tests

[Martin Basti]

On 16.03.2016 09:13, Martin Basti wrote:



On 15.03.2016 10:46, David Kupka wrote:

On 08/03/16 12:02, Martin Basti wrote:

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

Patch attached.



Works for me, ACK.


Pushed to master: 4871cb5b549042f383ee883e527e773c0abe9d87
Pushed to ipa-4-3: 03743ba1d9191bf0d786116808dba4d7a3522b1f


Fix for tests.
Patch attached.
From 3110aa74401dc0872bbabe915387a13f03868e00 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 17 Mar 2016 14:24:00 +0100
Subject: [PATCH] Fix stageuser-activate - managers test

https://fedorahosted.org/freeipa/ticket/5481
---
 ipatests/test_xmlrpc/test_stageuser_plugin.py |  6 ++-
 ipatests/test_xmlrpc/test_user_plugin.py  |  2 +-
 ipatests/test_xmlrpc/tracker/user_plugin.py   | 58 ++-
 3 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_stageuser_plugin.py b/ipatests/test_xmlrpc/test_stageuser_plugin.py
index 689682c9da5fc14a7d7bc12df94b477a4c603944..0ad017cbd61ec1db009fef917ea1be4ec8f357d8 100644
--- a/ipatests/test_xmlrpc/test_stageuser_plugin.py
+++ b/ipatests/test_xmlrpc/test_stageuser_plugin.py
@@ -442,7 +442,8 @@ class TestPreserved(XMLRPC_test):
 command = user.make_find_command(
 uid=user.uid, preserved=True, all=False)
 result = command()
-user.check_find(result, all=False)
+user.check_find(result, all=False,
+expected_override=dict(preserved=True))
 user.delete()
 
 def test_search_preserved_valid_all(self, user):
@@ -451,7 +452,8 @@ class TestPreserved(XMLRPC_test):
 command = user.make_find_command(
 uid=user.uid, preserved=True, all=True)
 result = command()
-user.check_find(result, all=True)
+user.check_find(result, all=True,
+expected_override=dict(preserved=True))
 user.delete()
 
 def test_retrieve_preserved(self, user):
diff --git a/ipatests/test_xmlrpc/test_user_plugin.py b/ipatests/test_xmlrpc/test_user_plugin.py
index 355573138fdf35d7f54471875dc955036ec5c64d..a70b90a994a243ea98a4d09ceec9a6b0aca49545 100644
--- a/ipatests/test_xmlrpc/test_user_plugin.py
+++ b/ipatests/test_xmlrpc/test_user_plugin.py
@@ -691,7 +691,7 @@ class TestManagers(XMLRPC_test):
 """ Find user by his manager's UID """
 command = user.make_find_command(manager=user2.uid)
 result = command()
-user.check_find(result)
+user.check_find(result, expected_override=dict(manager=[user2.uid]))
 
 def test_delete_both_user_and_manager(self, user, user2):
 """ Delete both user and its manager at once """
diff --git a/ipatests/test_xmlrpc/tracker/user_plugin.py b/ipatests/test_xmlrpc/tracker/user_plugin.py
index c72e17424da7630b6ed239ac42e7baec59be278c..216112db50ff909846e7bb900e76e961177cd10b 100644
--- a/ipatests/test_xmlrpc/tracker/user_plugin.py
+++ b/ipatests/test_xmlrpc/tracker/user_plugin.py
@@ -7,7 +7,7 @@ from ipapython.dn import DN
 
 import six
 
-from ipatests.util import assert_deepequal, get_group_dn, get_user_dn
+from ipatests.util import assert_deepequal, get_group_dn
 from ipatests.test_xmlrpc import objectclasses
 from ipatests.test_xmlrpc.xmlrpc_test import (
 fuzzy_digits, fuzzy_uuid, raises_exact)
@@ -21,24 +21,26 @@ class UserTracker(Tracker):
 """ Class for host plugin like tests """
 
 retrieve_keys = {
-u'uid', u'givenname', u'sn', u'homedirectory',
-u'loginshell', u'uidnumber', u'gidnumber', u'mail', u'ou',
-u'telephonenumber', u'title', u'memberof',
+u'dn', u'uid', u'givenname', u'sn', u'homedirectory', u'loginshell',
+u'uidnumber', u'gidnumber', u'mail', u'ou',
+u'telephonenumber', u'title', u'memberof', u'nsaccountlock',
 u'memberofindirect', u'ipauserauthtype', u'userclass',
 u'ipatokenradiusconfiglink', u'ipatokenradiususername',
-u'krbprincipalexpiration', u'usercertificate', u'dn', u'has_keytab',
-u'has_password', u'street', u'postalcode', u'facsimiletelephonenumber',
-u'carlicense', u'ipasshpubkey', u'sshpubkeyfp', u'nsaccountlock',
-u'memberof_group', u'l', u'mobile', u'krbextradata',
-u'krblastpwdchange', u'krbpasswordexpiration', u'pager', u'st',
-u'manager', u'preserved'}
+u'krbprincipalexpiration', u'usercertificate;binary',
+u'has_keytab', u'has_password', u'memberof_group', u'sshpubkeyfp',
+}
 
 retrieve_all_keys = retrieve_keys | {
-u'cn', u'ipauniqueid', u'objectclass', u'mepmanagedentry',
+u'usercertificate', u'street', u'postalcode',
+u'facsimiletelephonenumber', u'carlicense', u'ipasshpubkey',
+u'l', u'mobile', u'krbextradata', u'krblastpwdchange',
+u'krbpasswordexpiration', u'pager', u'st', u'manager', u'cn',
+u'ipauniqueid', u'objectclass', u'mepmanagedentry',
 u'displayname', u'gecos', u'initials', 

Re: [Freeipa-devel] [PATCH 0143-0144] different errors/warnings for different LDAP limit type exceeded

[Rob Crittenden]

Martin Babinsky wrote:

On 03/21/2016 12:25 PM, Jan Cholasta wrote:

On 21.3.2016 10:17, Petr Spacek wrote:

On 18.3.2016 13:49, Rob Crittenden wrote:

Martin Babinsky wrote:

These patches implement behavior agreed upon during discussion of
https://fedorahosted.org/freeipa/ticket/5677

However I'm not sure if we want to push them into 4-3 branch (the
ticket
is triaged into 4.3.2 milestone) since they modify the framework
behavior quite a bit.

If there is no need to have it there (CC'ing Milan since he is the
reporter), I would retriage it into 4.4 milestone.



+ desc="while getting entries (search base: '{}',"
+ "filter: {})".format(base_dn, filter))

This is going to expose parts of the DIT in an error message to
users. We have
tried in the past to hide the implementation. I'd propose logging the
error
and making the exception less verbose.


I agree with Rob here, we shouldn't expose internal stuff in error
messages for users.

In this particular case, even if we included internal stuff in the error
message, it should be the error message returned by the server rather
than this ad-hoc message.



IMHO it actually helps to print the DN. At very least the user can see
if the
error is happening always with the same DN or if it keeps changing.

In other words, for user it is not that important to understand
meaning of the
DN but it might be important to see if it is the same or not.


I can't imagine a situation where it would actually be useful for the
user (as opposed to the admin, who has access to logs) to know the base
DN of some arbitrary LDAP search operation. Could you give an example?


Right, attaching updated patches.


I may have suggested debug logging the detailed error. I was wrong. This 
should log at the error level so it always appears in the logs. This may 
be a spurious error and having the user turn on debug logging to capture 
the reasons would be asking a lot.


rob

--
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 016, 024, 025] First part of the replica promotion tests + testplan

[Oleg Fayans]

Hi Martin,

As per discussion, reverted 0025 patch and added try-catch in
prepare_host method to make replica promotion tests pass

On 03/22/2016 03:03 PM, Martin Basti wrote:
> 
> 
> On 22.03.2016 14:30, Oleg Fayans wrote:
>>
>> On 03/22/2016 02:06 PM, Martin Basti wrote:
>>>
>>> On 21.03.2016 15:54, Oleg Fayans wrote:
 Hi Lukas, Martin,

 Looks I've implemented the approach proposed by Martin. The issue seems
 to have gone (see the external_ca_out for external_ca test output).
 Would like you to take a look and tell me what'd you think.


 On 03/17/2016 08:37 PM, Lukas Slebodnik wrote:
> On (17/03/16 16:00), Oleg Fayans wrote:
>> Hi Lukas,
>>
>> On 03/17/2016 11:28 AM, Lukas Slebodnik wrote:
>>> On (10/03/16 23:09), Oleg Fayans wrote:
 Hi Martin,



 On 03/08/2016 08:18 PM, Martin Basti wrote:
> On 08.03.2016 18:24, Martin Basti wrote:
>> On 08.03.2016 12:38, Oleg Fayans wrote:
>>> The patches were rebased against the current master
>>>
>>> On 03/04/2016 05:33 PM, Martin Basti wrote:
 * old messages have been removed *
 1)
 this method is unused please remove it

 def test_kra_install_master(self):
> Well, in fact it is used twice: in both domain levels, so I'd
> better
> keep it:
>
> -bash-4.3$ ipa-run-tests
> test_integration/test_replica_promotion.py
> --collect-only
> 
>
>
>
>
> test session starts
> =
>
>
>
>
> platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.7.3
> rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile:
> pytest.ini
> plugins: sourceorder, multihost
> collected 8 items
> 
>   
> 
>   
>   
>   
>   
> 
>   
>   
> 
>   
>   
> 
>   
>   
>   
> 
>   
 aah my bad, I forgot that pytest executes it when it begins
 with test_*
 even in parent class
 2)
 Why are these there? I do not see any usage

 from env_config import get_global_config
 config = get_global_config()
> Removed
>
 3) nitpick
 +num_clients = 0
 this is set by default
> Removed
>
 otherwise LGTM

 Results of testing tomorrow.

 Martin^2

>>> I applied all patches including workarounds, but test
>>> failed.
>>>
>>> ipatests.test_integration.test_replica_promotion.TestReplicaPromotionLevel0
>>>
>>>
>>>
>>>
>>>
>>>
>>> [ipa.ipatests.test_integration.host.Host.replica1.cmd51] RUN
>>> ['ipa-replica-install', '-U', '-p', 'Secret123', '-w',
>>> 'Secret123',
>>> '--setup-ca', '--ip-address', '192.168.144.102',
>>> '/root/ipatests/replica-info.gpg']
>>> [ipa.ipatests.test_integration.host.Host.replica1.cmd51]
>>> The host
>>> replica1.ipa.test already exists on the master server.
>>> [ipa.ipatests.test_integration.host.Host.replica1.cmd51]
>>> You should
>>> remove it before proceeding:
>>> [ipa.ipatests.test_integration.host.Host.replica1.cmd51]
>>> % ipa
>>> host-del replica1.ipa.test
>>> [ipa.ipatests.test_integration.host.Host.replica1.cmd51]
>>> ipa.ipapython.install.cli.install_tool(Replica): ERROR   
>>> The
>>> ipa-replica-install command failed. See
>>> /var/log/ipareplica-install.log for more information
>>> [ipa.ipatests.test_integration.host.Host.replica1.cmd51]
>>> Exit
>>> code: 3
>>> FAILED
> this is exactly the error that happens when a workaround for
> 5627
> is not
> applied. I have re-run the tests with all the patches and
> everything
> 

Re: [Freeipa-devel] [PATCH 0143-0144] different errors/warnings for different LDAP limit type exceeded

[Martin Babinsky]

On 03/21/2016 12:25 PM, Jan Cholasta wrote:

On 21.3.2016 10:17, Petr Spacek wrote:

On 18.3.2016 13:49, Rob Crittenden wrote:

Martin Babinsky wrote:

These patches implement behavior agreed upon during discussion of
https://fedorahosted.org/freeipa/ticket/5677

However I'm not sure if we want to push them into 4-3 branch (the
ticket
is triaged into 4.3.2 milestone) since they modify the framework
behavior quite a bit.

If there is no need to have it there (CC'ing Milan since he is the
reporter), I would retriage it into 4.4 milestone.



+ desc="while getting entries (search base: '{}',"
+ "filter: {})".format(base_dn, filter))

This is going to expose parts of the DIT in an error message to
users. We have
tried in the past to hide the implementation. I'd propose logging the
error
and making the exception less verbose.


I agree with Rob here, we shouldn't expose internal stuff in error
messages for users.

In this particular case, even if we included internal stuff in the error
message, it should be the error message returned by the server rather
than this ad-hoc message.



IMHO it actually helps to print the DN. At very least the user can see
if the
error is happening always with the same DN or if it keeps changing.

In other words, for user it is not that important to understand
meaning of the
DN but it might be important to see if it is the same or not.


I can't imagine a situation where it would actually be useful for the
user (as opposed to the admin, who has access to logs) to know the base
DN of some arbitrary LDAP search operation. Could you give an example?


Right, attaching updated patches.

--
Martin^3 Babinsky
From 8201c09d465020b2e8fd61ece5a69ed771224b6d Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Fri, 18 Mar 2016 09:49:41 +0100
Subject: [PATCH 1/2] differentiate between limit types when LDAP search
 exceeds configured limits

When LDAP search fails on exceeded limits, we should raise an specific
exception for the type of limit raised (size, time, administrative) so that
the consumer can distinguish between e.g. searches returning too many entries
and those timing out.

https://fedorahosted.org/freeipa/ticket/5677
---
 install/tools/ipa-httpd-kdcproxy |  6 ++--
 install/tools/ipactl |  6 ++--
 ipalib/errors.py | 28 +
 ipalib/plugins/automount.py  |  6 ++--
 ipalib/plugins/baseldap.py   | 14 +++--
 ipapython/ipaldap.py | 68 +---
 ipaserver/plugins/ldap2.py   |  5 +--
 7 files changed, 92 insertions(+), 41 deletions(-)

diff --git a/install/tools/ipa-httpd-kdcproxy b/install/tools/ipa-httpd-kdcproxy
index 5e67f61a6e2b3fe26532323d773bd502ac52f454..c14b3a721ca739c3412aaaeef0b0d5c71b0e0cf2 100755
--- a/install/tools/ipa-httpd-kdcproxy
+++ b/install/tools/ipa-httpd-kdcproxy
@@ -97,10 +97,8 @@ class KDCProxyConfig(object):
 def _find_entry(self, dn, attrs, filter, scope=IPAdmin.SCOPE_BASE):
 """Find an LDAP entry, handles NotFound and Limit"""
 try:
-entries, truncated = self.con.find_entries(
-filter, attrs, dn, scope, time_limit=self.time_limit)
-if truncated:
-raise errors.LimitsExceeded()
+entries = self.con.get_entries(
+dn, scope, filter, attrs, time_limit=self.time_limit)
 except errors.NotFound:
 self.log.debug('Entry not found: %s', dn)
 return None
diff --git a/install/tools/ipactl b/install/tools/ipactl
index fb1e890ea16af492e1dc4956d40a3fb6287d5837..b41b10c8a65fc8d4f2f4c169e08b2c89125909a4 100755
--- a/install/tools/ipactl
+++ b/install/tools/ipactl
@@ -160,14 +160,12 @@ def get_config(dirsrv):
 wait_for_open_ports(host, [int(port)], timeout=api.env.startup_timeout)
 con = IPAdmin(ldap_uri=api.env.ldap_uri)
 con.do_external_bind()
-res, truncated = con.find_entries(
+res = con.get_entries(
+base,
 filter=srcfilter,
 attrs_list=attrs,
-base_dn=base,
 scope=con.SCOPE_SUBTREE,
 time_limit=10)
-if truncated:
-raise errors.LimitsExceeded()
 except errors.NetworkError:
 # LSB status code 3: program is not running
 raise IpactlError("Failed to get list of services to probe status:\n" +
diff --git a/ipalib/errors.py b/ipalib/errors.py
index 52b770027081448827007d8af00143046d59de0a..2507e13dcdc6c11b04a9ea1d50d3356f4e21986a 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -1612,6 +1612,34 @@ class TaskTimeout(DatabaseError):
 format = _("%(task)s LDAP task timeout, Task DN: '%(task_dn)s'")
 
 
+class TimeLimitExceeded(LimitsExceeded):
+"""
+**4214** Raised when time limit for the operation is exceeded.
+"""
+
+errno = 4214
+format = _('Configured time limit exceeded')
+
+
+class SizeLimitExceeded(LimitsExceeded):
+"""
+**4215** 

Re: [Freeipa-devel] [PATCH 016, 024, 025] First part of the replica promotion tests + testplan

[Martin Basti]

On 22.03.2016 14:30, Oleg Fayans wrote:


On 03/22/2016 02:06 PM, Martin Basti wrote:


On 21.03.2016 15:54, Oleg Fayans wrote:

Hi Lukas, Martin,

Looks I've implemented the approach proposed by Martin. The issue seems
to have gone (see the external_ca_out for external_ca test output).
Would like you to take a look and tell me what'd you think.


On 03/17/2016 08:37 PM, Lukas Slebodnik wrote:

On (17/03/16 16:00), Oleg Fayans wrote:

Hi Lukas,

On 03/17/2016 11:28 AM, Lukas Slebodnik wrote:

On (10/03/16 23:09), Oleg Fayans wrote:

Hi Martin,



On 03/08/2016 08:18 PM, Martin Basti wrote:

On 08.03.2016 18:24, Martin Basti wrote:

On 08.03.2016 12:38, Oleg Fayans wrote:

The patches were rebased against the current master

On 03/04/2016 05:33 PM, Martin Basti wrote:

* old messages have been removed *

1)
this method is unused please remove it

def test_kra_install_master(self):

Well, in fact it is used twice: in both domain levels, so I'd
better
keep it:

-bash-4.3$ ipa-run-tests
test_integration/test_replica_promotion.py
--collect-only




test session starts
=



platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.7.3
rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile:
pytest.ini
plugins: sourceorder, multihost
collected 8 items

  

  
  
  
  

  
  

  
  

  
  
  

  

aah my bad, I forgot that pytest executes it when it begins
with test_*
even in parent class

2)
Why are these there? I do not see any usage

from env_config import get_global_config
config = get_global_config()

Removed


3) nitpick
+num_clients = 0
this is set by default

Removed


otherwise LGTM

Results of testing tomorrow.

Martin^2


I applied all patches including workarounds, but test failed.

ipatests.test_integration.test_replica_promotion.TestReplicaPromotionLevel0





[ipa.ipatests.test_integration.host.Host.replica1.cmd51] RUN
['ipa-replica-install', '-U', '-p', 'Secret123', '-w',
'Secret123',
'--setup-ca', '--ip-address', '192.168.144.102',
'/root/ipatests/replica-info.gpg']
[ipa.ipatests.test_integration.host.Host.replica1.cmd51]
The host
replica1.ipa.test already exists on the master server.
[ipa.ipatests.test_integration.host.Host.replica1.cmd51]
You should
remove it before proceeding:
[ipa.ipatests.test_integration.host.Host.replica1.cmd51] % ipa
host-del replica1.ipa.test
[ipa.ipatests.test_integration.host.Host.replica1.cmd51]
ipa.ipapython.install.cli.install_tool(Replica): ERRORThe
ipa-replica-install command failed. See
/var/log/ipareplica-install.log for more information
[ipa.ipatests.test_integration.host.Host.replica1.cmd51] Exit
code: 3
FAILED

this is exactly the error that happens when a workaround for
5627
is not
applied. I have re-run the tests with all the patches and
everything
passed. Could you please double-check, whether patch 0027 was
applied
correctly?

bash-4.3$ ipa-run-tests
test_integration/test_replica_promotion.py
--pdb




test session starts
=



platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.7.3
rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile:
pytest.ini
plugins: sourceorder, multihost
collected 8 items

test_integration/test_replica_promotion.py 





8 passed in 7561.93 seconds
=





I will


And it needs ticket, otherwise it will not be in 4-3 branch.

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

NACK

1)
ipatests.test_integration.test_replica_promotion.TestReplicaPromotionLevel0



[ipa.ipatests.test_integration.host.Host.replica2.ParamikoTransport]

RUN ['ipa-replica-install', '-U', '-p', 'Secret123', '-w',
'Secret123', '--setup-ca', '--ip-address', '192.168.200.103', '-r',
'IPA.TEST']
[ipa.ipatests.test_integration.host.Host.replica2.cmd65] RUN
['ipa-replica-install', '-U', '-p', 'Secret123', '-w', 'Secret123',
'--setup-ca', '--ip-address', '192.168.200.103', '-r', 'IPA.TEST']
[ipa.ipatests.test_integration.host.Host.replica2.cmd65] IPA
client is
already configured on this system, ignoring the --domain, --server,
--realm, --hostname, --password and --keytab options.
[ipa.ipatests.test_integration.host.Host.replica2.cmd65] Your
system
may be partly configured.
[ipa.ipatests.test_integration.host.Host.replica2.cmd65] Run
/usr/sbin/ipa-server-install --uninstall to clean up.
[ipa.ipatests.test_integration.host.Host.replica2.cmd65]
[ipa.ipatests.test_integration.host.Host.replica2.cmd65]

Re: [Freeipa-devel] [PATCH 016, 024, 025] First part of the replica promotion tests + testplan

[Oleg Fayans]

On 03/22/2016 02:06 PM, Martin Basti wrote:
> 
> 
> On 21.03.2016 15:54, Oleg Fayans wrote:
>> Hi Lukas, Martin,
>>
>> Looks I've implemented the approach proposed by Martin. The issue seems
>> to have gone (see the external_ca_out for external_ca test output).
>> Would like you to take a look and tell me what'd you think.
>>
>>
>> On 03/17/2016 08:37 PM, Lukas Slebodnik wrote:
>>> On (17/03/16 16:00), Oleg Fayans wrote:
 Hi Lukas,

 On 03/17/2016 11:28 AM, Lukas Slebodnik wrote:
> On (10/03/16 23:09), Oleg Fayans wrote:
>> Hi Martin,
>>
>>
>>
>> On 03/08/2016 08:18 PM, Martin Basti wrote:
>>>
>>> On 08.03.2016 18:24, Martin Basti wrote:

 On 08.03.2016 12:38, Oleg Fayans wrote:
> The patches were rebased against the current master
>
> On 03/04/2016 05:33 PM, Martin Basti wrote:
>> * old messages have been removed *
>> 1)
>> this method is unused please remove it
>>
>>def test_kra_install_master(self):
>>> Well, in fact it is used twice: in both domain levels, so I'd
>>> better
>>> keep it:
>>>
>>> -bash-4.3$ ipa-run-tests
>>> test_integration/test_replica_promotion.py
>>> --collect-only
>>> 
>>>
>>>
>>>
>>> test session starts
>>> =
>>>
>>>
>>>
>>> platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.7.3
>>> rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile:
>>> pytest.ini
>>> plugins: sourceorder, multihost
>>> collected 8 items
>>> 
>>>  
>>>
>>>  
>>>  
>>>  
>>>  
>>>
>>>  
>>>  
>>>
>>>  
>>>  
>>>
>>>  
>>>  
>>>  
>>>
>>>  
>> aah my bad, I forgot that pytest executes it when it begins
>> with test_*
>> even in parent class
>> 2)
>> Why are these there? I do not see any usage
>>
>> from env_config import get_global_config
>> config = get_global_config()
>>> Removed
>>>
>> 3) nitpick
>> +num_clients = 0
>> this is set by default
>>> Removed
>>>
>> otherwise LGTM
>>
>> Results of testing tomorrow.
>>
>> Martin^2
>>
> I applied all patches including workarounds, but test failed.
>
> ipatests.test_integration.test_replica_promotion.TestReplicaPromotionLevel0
>
>
>
>
>
> [ipa.ipatests.test_integration.host.Host.replica1.cmd51] RUN
> ['ipa-replica-install', '-U', '-p', 'Secret123', '-w',
> 'Secret123',
> '--setup-ca', '--ip-address', '192.168.144.102',
> '/root/ipatests/replica-info.gpg']
> [ipa.ipatests.test_integration.host.Host.replica1.cmd51]
> The host
> replica1.ipa.test already exists on the master server.
> [ipa.ipatests.test_integration.host.Host.replica1.cmd51]
> You should
> remove it before proceeding:
> [ipa.ipatests.test_integration.host.Host.replica1.cmd51] % ipa
> host-del replica1.ipa.test
> [ipa.ipatests.test_integration.host.Host.replica1.cmd51]
> ipa.ipapython.install.cli.install_tool(Replica): ERRORThe
> ipa-replica-install command failed. See
> /var/log/ipareplica-install.log for more information
> [ipa.ipatests.test_integration.host.Host.replica1.cmd51] Exit
> code: 3
> FAILED
>>> this is exactly the error that happens when a workaround for
>>> 5627
>>> is not
>>> applied. I have re-run the tests with all the patches and
>>> everything
>>> passed. Could you please double-check, whether patch 0027 was
>>> applied
>>> correctly?
>>>
>>> bash-4.3$ ipa-run-tests
>>> test_integration/test_replica_promotion.py
>>> --pdb
>>> 
>>>
>>>
>>>
>>> test session starts
>>> =
>>>
>>>
>>>
>>> platform linux2 -- Python 2.7.10 -- py-1.4.30 -- 

Re: [Freeipa-devel] [PATCH 016, 024, 025] First part of the replica promotion tests + testplan

[Martin Basti]

On 21.03.2016 15:54, Oleg Fayans wrote:

Hi Lukas, Martin,

Looks I've implemented the approach proposed by Martin. The issue seems
to have gone (see the external_ca_out for external_ca test output).
Would like you to take a look and tell me what'd you think.


On 03/17/2016 08:37 PM, Lukas Slebodnik wrote:

On (17/03/16 16:00), Oleg Fayans wrote:

Hi Lukas,

On 03/17/2016 11:28 AM, Lukas Slebodnik wrote:

On (10/03/16 23:09), Oleg Fayans wrote:

Hi Martin,



On 03/08/2016 08:18 PM, Martin Basti wrote:


On 08.03.2016 18:24, Martin Basti wrote:


On 08.03.2016 12:38, Oleg Fayans wrote:

The patches were rebased against the current master

On 03/04/2016 05:33 PM, Martin Basti wrote:

* old messages have been removed *

1)
this method is unused please remove it

   def test_kra_install_master(self):

Well, in fact it is used twice: in both domain levels, so I'd better
keep it:

-bash-4.3$ ipa-run-tests test_integration/test_replica_promotion.py
--collect-only



test session starts
=


platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.7.3
rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile:
pytest.ini
plugins: sourceorder, multihost
collected 8 items

 
   
 
 
 
 
   
 
 
   
 
 
   
 
 
 
   
 

aah my bad, I forgot that pytest executes it when it begins with test_*
even in parent class

2)
Why are these there? I do not see any usage

from env_config import get_global_config
config = get_global_config()

Removed


3) nitpick
+num_clients = 0
this is set by default

Removed


otherwise LGTM

Results of testing tomorrow.

Martin^2


I applied all patches including workarounds, but test failed.

ipatests.test_integration.test_replica_promotion.TestReplicaPromotionLevel0




[ipa.ipatests.test_integration.host.Host.replica1.cmd51] RUN
['ipa-replica-install', '-U', '-p', 'Secret123', '-w', 'Secret123',
'--setup-ca', '--ip-address', '192.168.144.102',
'/root/ipatests/replica-info.gpg']
[ipa.ipatests.test_integration.host.Host.replica1.cmd51] The host
replica1.ipa.test already exists on the master server.
[ipa.ipatests.test_integration.host.Host.replica1.cmd51] You should
remove it before proceeding:
[ipa.ipatests.test_integration.host.Host.replica1.cmd51] % ipa
host-del replica1.ipa.test
[ipa.ipatests.test_integration.host.Host.replica1.cmd51]
ipa.ipapython.install.cli.install_tool(Replica): ERRORThe
ipa-replica-install command failed. See
/var/log/ipareplica-install.log for more information
[ipa.ipatests.test_integration.host.Host.replica1.cmd51] Exit
code: 3
FAILED

this is exactly the error that happens when a workaround for 5627
is not
applied. I have re-run the tests with all the patches and everything
passed. Could you please double-check, whether patch 0027 was applied
correctly?

bash-4.3$ ipa-run-tests test_integration/test_replica_promotion.py
--pdb



test session starts
=


platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.7.3
rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile:
pytest.ini
plugins: sourceorder, multihost
collected 8 items

test_integration/test_replica_promotion.py 




8 passed in 7561.93 seconds
=




I will


And it needs ticket, otherwise it will not be in 4-3 branch.

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

NACK

1)
ipatests.test_integration.test_replica_promotion.TestReplicaPromotionLevel0


[ipa.ipatests.test_integration.host.Host.replica2.ParamikoTransport]
RUN ['ipa-replica-install', '-U', '-p', 'Secret123', '-w',
'Secret123', '--setup-ca', '--ip-address', '192.168.200.103', '-r',
'IPA.TEST']
[ipa.ipatests.test_integration.host.Host.replica2.cmd65] RUN
['ipa-replica-install', '-U', '-p', 'Secret123', '-w', 'Secret123',
'--setup-ca', '--ip-address', '192.168.200.103', '-r', 'IPA.TEST']
[ipa.ipatests.test_integration.host.Host.replica2.cmd65] IPA client is
already configured on this system, ignoring the --domain, --server,
--realm, --hostname, --password and --keytab options.
[ipa.ipatests.test_integration.host.Host.replica2.cmd65] Your system
may be partly configured.
[ipa.ipatests.test_integration.host.Host.replica2.cmd65] Run
/usr/sbin/ipa-server-install --uninstall to clean up.
[ipa.ipatests.test_integration.host.Host.replica2.cmd65]
[ipa.ipatests.test_integration.host.Host.replica2.cmd65]
ipa.ipapython.install.cli.install_tool(Replica): ERRORYou must
provide a file generated by ipa-replica-prepare to create a replica

Re: [Freeipa-devel] [TEST][Patch-0027] Fixed test failure during in-tree session, ticket N 5736

[Martin Basti]

On 21.03.2016 09:11, Oleg Fayans wrote:

Hi Martin,

On 03/16/2016 03:35 PM, Martin Basti wrote:


On 16.03.2016 15:13, Martin Basti wrote:


On 16.03.2016 14:59, Oleg Fayans wrote:

Hi Martin

On 03/16/2016 02:39 PM, Martin Basti wrote:

On 16.03.2016 10:59, Oleg Fayans wrote:

With this patch applied integration tests pass and in-tree tests are
gracefully skipped.

@mkubik, It is not possible to put the decorator to util.py as per our
discussion, because it uses tasks, so tasks must be imported. But
tasks
already import util, which leads to circular imports. So I've put
it to
tasks.py




NACK

1)
Use right ticket in commit message (#5723)

But (#5736) is exactly the issue that is being addressed. Probably note
both tickets in the commit message?

But as I wrote in ticket #5736, this ticket should be closed, because
issue is caused by ticket which is not finished yet, so we should
continue just with original ticket.

Done


2)
Link to ticket should be last in the commit message

Done


3)
dereplicafy

3a)
wrong doc string, it removes *only* replicas not clients

No, in fact it removes both:
uninstall_replica(args[0].master, host)
uninstall_client(host)

Both tasks have raiseonerr set to False, which means that even if
replica was not installed but the client was - it will also be removed

I see just
for host in args[0].replicas

I don't see any
for host in args[0].clients
there

Also uninstall_client should not be there. ipa-server-install
--uninstall removes client too. The extra call of uninstall client is
IMO there just because an ancient bug that is already fixed.

That's done because some tests install client separately and then
deliberately install replica the wrong way to test that the installer
fails in a predicted way. That's why this separate uninstall_client
call. The doc string was corrected.



3b)
can we rename it to something different? (replicas_cleanup,
replicas_uninstall, replicas_teardown)

replicas_cleanup, or even topo_cleanup sounds OK to me.

replicas_cleanup it is


4)
Please fix commit message
- Wile trated correctly
- followiong
- rewrote -> rewrite

Will do

Done


5)
decorator
+def wrapped(*args):
+func(*args)
+for host in args[0].replicas:

Shouldn't be there try-finally around func() call, or something?

No, the wrapped function is a test_* method: if it fails we need to see
the original failure

but if something raise an exception in func(), cleanup will not be
executed.

You can do
In [4]: try:
...: raise ValueError('Hello')
...: finally:
...: try:
...: raise ValueError('Cleanup')
...: except Exception:
...: pass
...:
---

ValueErrorTraceback (most recent call
last)
 in ()
   1 try:
> 2 raise ValueError('Hello')
   3 finally:
   4 try:
   5 raise ValueError('Cleanup')

ValueError: Hello

On the other hand, I do not want cleanup with --pdb option, so maybe it
should just fail


Are you sure that there is no need to return result of func()?

The same applies here: we never return results from test_* methods

ok

*) Please create additional patch that will add licence there



Will do :)



The license-related patch is attached too


Patch 0029 pushed to:
master: c2042900382190b1c9d7a44bd719cacd804749b3
ipa-4-3: 1d5b8b8781e5d6300c5029bdd68c6ddf98f6ecd3


Patch 27 is on review

--
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 0099] Look up HTTPD_USER's UID and GID during installation.

[David Kupka]

https://fedorahosted.org/freeipa/ticket/5712
--
David Kupka
From 00959a382a34bfd77539443cd51b8033ca9c3ee1 Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Tue, 22 Mar 2016 09:40:43 +0100
Subject: [PATCH] Look up HTTPD_USER's UID and GID during installation.

Those values differ among distributions and there is no guarantee that they're
reserved. It's better to look them up based on HTTPD_USER's name.

https://fedorahosted.org/freeipa/ticket/5712
---
 install/share/custodia.conf.template  | 4 ++--
 ipaserver/install/custodiainstance.py | 6 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/install/share/custodia.conf.template b/install/share/custodia.conf.template
index 688229a50854cd9521b0ae323f30a1c5b729b26f..d9de4d77f90931c089f2179731783430f85ed6f1 100644
--- a/install/share/custodia.conf.template
+++ b/install/share/custodia.conf.template
@@ -5,8 +5,8 @@ auditlog = $IPA_CUSTODIA_AUDIT_LOG
 
 [auth:simple]
 handler = custodia.httpd.authenticators.SimpleCredsAuth
-uid = 48
-gid = 48
+uid = $UID
+gid = $GID
 
 [auth:header]
 handler = custodia.httpd.authenticators.SimpleHeaderAuth
diff --git a/ipaserver/install/custodiainstance.py b/ipaserver/install/custodiainstance.py
index dbe36af6d7af23fa859dcb78f3dc24224fd8fd07..424e0797b682d312c07ebf86a13c27164cae6faf 100644
--- a/ipaserver/install/custodiainstance.py
+++ b/ipaserver/install/custodiainstance.py
@@ -3,6 +3,7 @@
 from ipapython.secrets.kem import IPAKEMKeys
 from ipapython.secrets.client import CustodiaClient
 from ipaplatform.paths import paths
+from ipaplatform.constants import constants
 from service import SimpleServiceInstance
 from ipapython import ipautil
 from ipapython.ipa_log_manager import root_logger
@@ -14,6 +15,7 @@ from jwcrypto.common import json_decode
 import shutil
 import os
 import tempfile
+import pwd
 
 
 class CustodiaInstance(SimpleServiceInstance):
@@ -30,10 +32,12 @@ class CustodiaInstance(SimpleServiceInstance):
 def __config_file(self):
 template_file = os.path.basename(self.config_file) + '.template'
 template = os.path.join(ipautil.SHARE_DIR, template_file)
+httpd_info = pwd.getpwnam(constants.HTTPD_USER)
 sub_dict = dict(IPA_CUSTODIA_CONF_DIR=paths.IPA_CUSTODIA_CONF_DIR,
 IPA_CUSTODIA_SOCKET=paths.IPA_CUSTODIA_SOCKET,
 IPA_CUSTODIA_AUDIT_LOG=paths.IPA_CUSTODIA_AUDIT_LOG,
-LDAP_URI=installutils.realm_to_ldapi_uri(self.realm))
+LDAP_URI=installutils.realm_to_ldapi_uri(self.realm),
+UID=httpd_info.pw_uid, GID=httpd_info.pw_gid)
 conf = ipautil.template_file(template, sub_dict)
 fd = open(self.config_file, "w+")
 fd.write(conf)
-- 
2.5.5

-- 
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 0141] ipa-replica-manage: print traceback on unexpected error when in verbose mode

[Martin Basti]

On 18.03.2016 14:02, Martin Babinsky wrote:

On 03/18/2016 01:38 PM, Martin Basti wrote:



On 15.03.2016 16:03, Martin Babinsky wrote:

On 03/14/2016 06:26 PM, Martin Basti wrote:



On 10.03.2016 15:59, Martin Babinsky wrote:

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




NACK

1)
Maybe we should print traceback in verbose mode for RuntimeError as
well.

2)
IMO would be better to print traceback first and then, print error

Martin^2


Attaching updated patch.


I changed my mind, RuntimeError should stay as it is now. Sorry


No problem.


ACK

Pushed to master: e7e1b8c58ed592e8957b4a25838a9e7814ddd01a

--
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 0139] otptoken-add: improve the robustness of QR code printing to tty

[Martin Babinsky]

On 03/16/2016 02:17 PM, Martin Babinsky wrote:

On 03/16/2016 01:35 PM, Nathaniel McCallum wrote:

On Wed, 2016-03-16 at 07:25 +0100, Jan Cholasta wrote:

On 15.3.2016 22:22, Nathaniel McCallum wrote:


On Tue, 2016-03-15 at 17:54 +0100, Martin Babinsky wrote:


On 03/15/2016 03:36 PM, Martin Babinsky wrote:



On 03/09/2016 07:06 AM, Jan Cholasta wrote:



On 8.3.2016 17:45, Martin Babinsky wrote:



On 03/08/2016 05:35 PM, Jan Cholasta wrote:



Hi,

On 8.3.2016 16:21, Martin Babinsky wrote:



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

1) Instead of checking for utf-8 in particular, I would
prefer a more
robust approach:

try:
   qr = qrcode.QRCode()
   qr.add_data('test')
   qr.make()
   qr.print_ascii(tty=True)
except UnicodeError:
   # it is not printable
else:
   # it is printable


Now you mean the check in the _check_qrcode_capability() or
the
_print_qrcode() method itself?

_check_qrcode_capability() of course.








2) There is no os.isatty() check to see if stdout is
actually
a tty.


This check is performed inside both print_ascii() and
print_tty()
methods of QRCode object, but you probably mean that I
should
put the
check also into _check_qrcode_capability() method, right?

Yes. If stdout is not a tty, we should at least not tty=True
in
print_ascii().








Honza


Attaching updated patch. After the discussion with other
developers
we
decided to just print warnings when non-UTF-8 encoding is used
and
tty
width is smaller that the QR code size.




Found some minor errors in the patch, attaching updated version.

NACK

This patch has the major problem that tokens are added but then
unusable because they can't be provisioned to the devices. You need
to
check if qrcode output is possible before the token is added to
LDAP.

We discussed this on the IPA devel meeting and the decision was that
since the otpauth URI is always displayed, a warning is sufficient
when
the QR code cannot be printed.

If you disagree, could you explain why the URI is not sufficient for
provisioning the token?


I guess that is okay.



Thank you Nathaniel.

Jan had some offline comments to the patch. Attaching updated version.




Attaching updated patches.

--
Martin^3 Babinsky
From b0e6802b1d310b2c5959ea9d45b6740dee8e238d Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Tue, 8 Mar 2016 15:56:52 +0100
Subject: [PATCH] otptoken-add: improve the robustness of QR code printing

The python-qrcode print_ascii() method does not work in terminals with
non-UTF-8 encoding. When this is the case do not render QR code but print a
warning instead. Also print a warning when the QR code size is greater that
terminal width if the output is a tty.

https://fedorahosted.org/freeipa/ticket/5700
---
 ipalib/messages.py |  8 ++
 ipalib/plugins/otptoken.py | 72 +-
 2 files changed, 73 insertions(+), 7 deletions(-)

diff --git a/ipalib/messages.py b/ipalib/messages.py
index 5d723b2c0ae7be07a5c89757b07ca353f23bf22e..681fc2bda611bca4510add15b74e7f786f5cc182 100644
--- a/ipalib/messages.py
+++ b/ipalib/messages.py
@@ -342,6 +342,14 @@ class BrokenTrust(PublicMessage):
"running 'ipa trust-add' again.")
 
 
+class ResultFormattingError(PublicMessage):
+"""
+**13019** Unable to correctly format some part of the result
+"""
+errno = 13019
+type = "warning"
+
+
 def iter_messages(variables, base):
 """Return a tuple with all subclasses
 """
diff --git a/ipalib/plugins/otptoken.py b/ipalib/plugins/otptoken.py
index 846155dfb3af53ad65f3a4f488629837d10a2bce..f0e0d43680f474f8b2bac9a5d3844f776d641de2 100644
--- a/ipalib/plugins/otptoken.py
+++ b/ipalib/plugins/otptoken.py
@@ -18,23 +18,28 @@
 # along with this program.  If not, see .
 
 from __future__ import print_function
+import sys
 
 from ipalib.plugins.baseldap import DN, LDAPObject, LDAPAddMember, LDAPRemoveMember
 from ipalib.plugins.baseldap import LDAPCreate, LDAPDelete, LDAPUpdate, LDAPSearch, LDAPRetrieve
 from ipalib import api, Int, Str, Bool, DateTime, Flag, Bytes, IntEnum, StrEnum, Password, _, ngettext
+from ipalib.messages import add_message, ResultFormattingError
 from ipalib.plugable import Registry
 from ipalib.errors import PasswordMismatch, ConversionError, LastMemberError, NotFound, ValidationError
 from ipalib.request import context
 from ipalib.frontend import Local
 from ipaplatform.paths import paths
 from ipapython.nsslib import NSSConnection
+from ipapython.version import API_VERSION
 
 import base64
+import locale
 import uuid
 import qrcode
 import os
 
 import six
+from six import StringIO
 from six.moves import urllib
 
 if six.PY3:
@@ -350,17 +355,70 @@ class otptoken_add(LDAPCreate):
 _convert_owner(self.api.Object.user, entry_attrs, options)
 return super(otptoken_add, self).post_callback(ldap, dn, entry_attrs, *keys, **options)
 
+def _get_qrcode(self, output, 

Re: [Freeipa-devel] [PATCH] Added fix for notifying user about account expiration in Web UI

[Abhijeet Kasurde]

Hi All,

Please find the updated patches as per review comments.

On 03/18/2016 07:39 PM, Petr Vobornik wrote:

On 03/18/2016 02:21 PM, Abhijeet Kasurde wrote:

Hi All,

Please review these patches.

Fixes : https://fedorahosted.org/freeipa/ticket/5077

Thanks,
Abhijeet Kasurde



'invalid' is a default and right now is meant for invalid password(not 
correct, see below). So by reading the patch, it will break the case 
when user sets invalid password.


Better would be to process kinit output in rpcserver.py:login_password 
and set e.g: 'krbprincipal-expired' reason.


Then add it to a list of known errors in ipa.js:login_password:498. We 
should probaly add also 'invalid-password' to the list.


Then do the change as in this patch but only with: 
'krbprincipal-expired'.


If 'invalid-password' is added to the list of know errors then we 
should change the default error from "The password or username you 
entered is incorrect. " to e.g.: 'Login failed from unknown reason"



Thanks Petr for suggestions.

Thanks,
Abhijeet Kasurde
From 908b71768f1cce792d5111434dbb73c71a4cedc3 Mon Sep 17 00:00:00 2001
From: Abhijeet Kasurde 
Date: Tue, 22 Mar 2016 15:41:36 +0530
Subject: [PATCH] Added fix for notifying user about Kerberos principal
 expiration in WebUI

- User is now notified about "Kerberos Principal expiration" message instead of
  "Wrong username or password" message.
- User is also notified about "Invalid password" message instead of
  generic error message.

Signed-off-by: Abhijeet Kasurde 
---
 install/ui/src/freeipa/ipa.js |  7 +--
 install/ui/src/freeipa/widgets/LoginScreen.js | 13 +++--
 ipalib/errors.py  |  8 +++-
 ipaserver/rpcserver.py| 13 +++--
 4 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/install/ui/src/freeipa/ipa.js b/install/ui/src/freeipa/ipa.js
index 29af4048740894c6d46b5419a941e2a48cd68775..e241ad30ddc7492fd3e21daa051516ef46a93014 100644
--- a/install/ui/src/freeipa/ipa.js
+++ b/install/ui/src/freeipa/ipa.js
@@ -5,7 +5,7 @@
  *John Dennis 
  *Petr Vobornik 
  *
- * Copyright (C) 2010 Red Hat
+ * Copyright (C) 2010-2016 Red Hat
  * see file 'COPYING' for use and warranty information
  *
  * This program is free software; you can redistribute it and/or modify
@@ -495,7 +495,10 @@ IPA.login_password = function(username, password) {
 
 //change result from invalid only if we have a header which we
 //understand
-if (reason === 'password-expired' || reason === 'denied') {
+if (reason === 'password-expired' ||
+reason === 'denied' ||
+reason === 'krbprincipal-expired' ||
+reason === 'invalid-password') {
 result = reason;
 }
 }
diff --git a/install/ui/src/freeipa/widgets/LoginScreen.js b/install/ui/src/freeipa/widgets/LoginScreen.js
index 17f891e0ee1d200eb4c9aa881dafcac5fc2c86da..a9f70cce7f8bda01efc1b98f88765aff3c17b73c 100644
--- a/install/ui/src/freeipa/widgets/LoginScreen.js
+++ b/install/ui/src/freeipa/widgets/LoginScreen.js
@@ -1,7 +1,7 @@
 /*  Authors:
  *Petr Vobornik 
  *
- * Copyright (C) 2013 Red Hat
+ * Copyright (C) 2013-2016 Red Hat
  * see file 'COPYING' for use and warranty information
  *
  * This program is free software; you can redistribute it and/or modify
@@ -57,7 +57,7 @@ define(['dojo/_base/declare',
 "configured" +
 " the browser correctly, then click Login. ",
 
-form_auth_failed: "The password or username you entered is incorrect. ",
+form_auth_failed: "Login failed due to an unknown reason. ",
 
 krb_auth_failed: "Authentication with Kerberos failed",
 
@@ -67,6 +67,9 @@ define(['dojo/_base/declare',
 
 denied: "Sorry you are not allowed to access this service.",
 
+krbprincipal_expired: "Kerberos Principal you entered is expired.",
+
+invalid_password: "The password you entered is incorrect. ",
 
 //nodes:
 login_btn_node: null,
@@ -231,6 +234,12 @@ define(['dojo/_base/declare',
 } else if (result === 'password-expired') {
 this.set('view', 'reset');
 val_summary.add_info('login', this.password_expired);
+} else if (result === 'krbprincipal-expired') {
+password_f.set_value('');
+val_summary.add_error('login', this.krbprincipal_expired);
+} else if (result === 'invalid-password') {
+password_f.set_value('');
+val_summary.add_error('login', this.invalid_password);
 } else {
 password_f.set_value('');
 val_summary.add_error('login', this.form_auth_failed);
diff --git a/ipalib/errors.py 

Re: [Freeipa-devel] Converting plugin output

[Christian Heimes]

On 2016-03-21 12:02, Jan Cholasta wrote:
> Hi,
> 
> On 18.3.2016 15:26, Christian Heimes wrote:
>> Hi,
>>
>> I'd like to use FreeIPA's RPC interface from Ansible directly. But the
>> output of plugins is rather unfriendly and unpythonic:
>>
> print(api.Command.dnsconfig_show())
>> {u'result': {u'dn': u'cn=dns,dc=ipa,dc=example', u'idnsallowsyncptr':
>> (u'FALSE',)}, u'value': None, u'summary': None}
>>
>> Please notice (u'FALSE',) instead of False.
> 
> This is how the framework does things - there is no internal consistency
> and no singular place where coding of values is handled, lot of the
> output is generated by ad-hoc code somewhere in post_callbacks.
> Unfortunately this is not easily fixable.

Yes, it's a bit unfortunate. FreeIPA has a rich and powerful RPC-API.
The under-documented and nested output makes the RPCs hard to use from
Python code. I'd wish we had something like JSON schema for input and
output documentation.

>> But it is failing for some plugins like user_find(). The plugin returns
>> u'memberof_group': (u'admins', u'trust admins'). However
>> global_output_params defines the value as an optional and single valued
>> string:
>>
>>  Str('memberof_group?', label=_('Member of groups')).
>>
>> I think the definition is wrong. memberof_group and some other fields
>> should be defined as optional and multivalued fields insteads. Even the
>> field's label uses a plural form.
>>
>> What do you think?
> 
> Yes, the definition is wrong, but I don't think it's worth fixing, since
> you can't rely on a single-value param having a single value in the
> output for any other command and param anyway.

I think it's a low-hanging fruit. All memberof and indirectmemberof
params should be multivalued. That's an easy fix.

Christian



signature.asc
Description: OpenPGP digital signature
-- 
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 0029] Move user/group constants for PKI and DS into ipaplatform

[Martin Basti]

On 18.03.2016 11:53, Christian Heimes wrote:

On 2016-03-18 10:22, Martin Basti wrote:


On 29.02.2016 16:02, David Kupka wrote:

Hello Christian,
sorry for letting this patch rot for so long. I've forget about it the minute 
Fraser replied.
To compensate a little I've fixed pep8 error, rebased it and attaching two 
versions for master and for 4.3 branch.
I haven't found any missing cases and it works for me. If you're OK with the 
modified patches it can be pushed.

David

- Original Message -
From: "Christian Heimes" 
To: "Fraser Tweedale" 
Cc: "freeipa-devel" 
Sent: Wednesday, January 20, 2016 11:57:42 AM
Subject: Re: [Freeipa-devel] [PATCH 0029] Move user/group constants for PKI and 
DS into ipaplatform

On 2016-01-20 02:54, Fraser Tweedale wrote:

On Tue, Jan 19, 2016 at 02:20:27PM +0100, Christian Heimes wrote:

ipaplatform.constants has platform specific names for a couple of system
users like Apache HTTPD. The user names for PKI_USER, PKI_GROUP, DS_USER
and DS_GROUP are defined in other modules. Similar to #5587 the patch my
patch moves the constants into the platform module.

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

I see a few remaining cases:

ipaserver/install/dsinstance.py
712:pent = pwd.getpwnam("dirsrv")

ipatests/test_integration/test_backup_and_restore.py
167:self.master.run_command(['userdel', 'dirsrv'])
168:self.master.run_command(['userdel', 'pkiuser'])

ipaplatform/redhat/tasks.py
441:if name == 'pkiuser':

When these are included, ACK.

Good catch!

My new patch takes care of remaining cases.





Christian do you agree with proposed changes, can we push it?
Martin^2

Oh, the patch is still open? ACK!



Pushed to ipa-4-3: e3bf65f2df9c50873f0967b96a6a2a5975a87f79
Pushed to master: 49be6c8d3cc20902dbe8e92a74e31aed2fd21d9f

--
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 0428] SPEC: do not execute upgrade when ipa server is not installed

[Martin Basti]

On 18.03.2016 14:12, Martin Babinsky wrote:

On 03/02/2016 07:26 AM, Jan Cholasta wrote:

On 1.3.2016 20:36, Rob Crittenden wrote:

Martin Basti wrote:



On 01.03.2016 20:13, Rob Crittenden wrote:

Martin Basti wrote:

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

Patch attached.


Would it be safer to integrate this into ipa-upgrade itself? You'd 
just

need to return 0 for the case where IPA isn't installed.

rob

How about the case when ipa-server-upgrade is called by user from CLI?
It should fail because IPA is not installed, instead of returning
success. That check is in specfile anyway due service restart.

Martin^2


Yeah, I was hoping you'd miss that :-)

It just seems to me, as you point out, that it should check when run by
anything, user or spec, so adding it only to the spec seems wrong. I'm
not a huge fan of option bloat but that would be one way around this,
--graceful-exit or something. Could make it a hidden option if you
wanted.


I don't think adding the option is worth the effort, as we will be soon
moving away from running the upgrade script directly from the spec file
to a service based solution (#4552, #5373).

I am not a big fan of adding a special option and the related magic to 
the upgrade code itself either.


I will ACK this patch unless there is some strong opposition towards 
this approach.



Pushed to:
master: 4f25b296054076abf3d6e44f6b2e7552f993fb1c
ipa-4-3: 0bd34fa78952b1abefde290f5b3bf7b4a67e73dc

--
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] [PATCHES 0442-0449] Pylint: sunday code cleanup

[Martin Basti]

On 21.03.2016 16:52, Petr Spacek wrote:

On 21.3.2016 13:50, Lukas Slebodnik wrote:

On (21/03/16 12:30), Martin Basti wrote:

On 21.03.2016 10:33, Christian Heimes wrote:

On 2016-03-21 10:29, Petr Spacek wrote:

On 20.3.2016 21:56, Martin Basti wrote:

Patches attached.

I do not really like
freeipa-mbasti-0442-pylint-remove-bare-except
because it replaces most of

try: ... except:

with

try: ... except Exception:


which AFAIK does not add any value. It would be better to replace Exception
with more specific exception so the code raises an error instead of continuing
when something really unexpected happens.

It adds some value. A bare except also excepts signals like
KeyboardInterrupt and SystemExit. except Exception doesn't block these
exceptions.

But yes, more specific exceptions are better.

Christian





'except Exception' is another pylint check :D

I replaced bare except with a particular exception in cases where it was
clear. For other occurrences of bare except it covers too much Exception
types, so catch Exception is more sensible, or I need crystal ball to detect
what kind of exceptions can be raised there.


Agree.

It can be changed to more specific exceptions type of Exception in future.
This change is less risky.

pylint passed on fedora {23, 24, rawhide}

ACK

Okay, it makes sense.


master:
* 491447cc5ab8c5eff2be57d609201cefb79f7053 pylint: remove bare except
* e93e89e1ae27e4f0ef23001f6c1247c45695ae24 Pylint: fix definition of 
global variables
* 5add0f94cf9253a72224ccaf5be38540468ea589 Pylint: enable 
pointless-except check

* d46cd5d956d1c03b863bf90d0fd0ff4870448183 Pylint: enable reimported check
* 195e50b93b63e4f30ce83dbcfef278727d48aea2 Pylint: use list 
comprehension instead of iteration
* b66028af1815fbf7666b82ebeaa81ad56996a74f Pylint: import max one module 
per line
* da0318d4d7dd369be136449e686b6fb46d0cc5d8 Pylint: remove 
unnecessary-semicolon

* 4a396dd68b1bc6cc68765f502f7e952a087064a8 Pylint: enable invalid-name check

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