Re: [Freeipa-devel] [PATCHES 0200-0202] DNS fixes related to unsupported records
On 04/03/15 16:17, Martin Basti wrote:
Ticket: https://fedorahosted.org/freeipa/ticket/4930
0200: 4.1, master
Fixes traceback, which was raised if LDAP contained a record that was
marked as unsupported.
Now unsupported records are shown, if LDAP contains them.
0200: 4.1, master
Records marked as unsupported will not show options for editing parts.
0202: only master
Removes NSEC3PARAM record from record types. NSEC3PARAM can contain
only zone, value is allowed only in idnszone objectclass, so do not
confuse users.
and patches attached :-)
--
Martin Basti
From ec46d1059df2474762fb0434699f92cb645584bf Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Wed, 4 Mar 2015 12:52:16 +0100
Subject: [PATCH 1/3] DNS fix: do not traceback if unsupported records are in
LDAP
Show records which are unsupported, if they are in LDAP.
Those records are not editable, and web UI doesnt show them.
Fixes traceback caused by --structured option
Ticket: https://fedorahosted.org/freeipa/ticket/4930
---
ipalib/plugins/dns.py | 64 +--
1 file changed, 32 insertions(+), 32 deletions(-)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 9dc3ed0b021b7d9bb42053a48690047bd7a244a2..0e04a287e259a1f88ae5c973cf67ce680c61db7d 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -976,6 +976,17 @@ class ForwardRecord(DNSRecord):
reason=_('Cannot create reverse record for %(value)s: %(exc)s') \
% dict(value=record, exc=unicode(e)))
+class UnsupportedDNSRecord(DNSRecord):
+
+Records which are not supported by IPA CLI, but we allow to show them if
+LDAP contains these records.
+
+supported = False
+
+def _get_part_values(self, value):
+return tuple()
+
+
class ARecord(ForwardRecord):
rrtype = 'A'
rfc = 1035
@@ -1023,10 +1034,9 @@ class AFSDBRecord(DNSRecord):
),
)
-class APLRecord(DNSRecord):
+class APLRecord(UnsupportedDNSRecord):
rrtype = 'APL'
rfc = 3123
-supported = False
class CERTRecord(DNSRecord):
rrtype = 'CERT'
@@ -1062,10 +1072,9 @@ class CNAMERecord(DNSRecord):
),
)
-class DHCIDRecord(DNSRecord):
+class DHCIDRecord(UnsupportedDNSRecord):
rrtype = 'DHCID'
rfc = 4701
-supported = False
class DNAMERecord(DNSRecord):
rrtype = 'DNAME'
@@ -1076,10 +1085,9 @@ class DNAMERecord(DNSRecord):
),
)
-class DNSKEYRecord(DNSRecord):
+class DNSKEYRecord(UnsupportedDNSRecord):
rrtype = 'DNSKEY'
rfc = 4034
-supported = False
class DSRecord(DNSRecord):
rrtype = 'DS'
@@ -1114,20 +1122,18 @@ class DLVRecord(DSRecord):
rfc = 4431
-class HIPRecord(DNSRecord):
+class HIPRecord(UnsupportedDNSRecord):
rrtype = 'HIP'
rfc = 5205
-supported = False
-class KEYRecord(DNSRecord):
+class KEYRecord(UnsupportedDNSRecord):
+# managed by BIND itself
rrtype = 'KEY'
rfc = 2535
-supported = False # managed by BIND itself
-class IPSECKEYRecord(DNSRecord):
+class IPSECKEYRecord(UnsupportedDNSRecord):
rrtype = 'IPSECKEY'
rfc = 4025
-supported = False
class KXRecord(DNSRecord):
rrtype = 'KX'
@@ -1300,20 +1306,19 @@ class NSRecord(DNSRecord):
),
)
-class NSECRecord(DNSRecord):
+class NSECRecord(UnsupportedDNSRecord):
+# managed by BIND itself
rrtype = 'NSEC'
rfc = 4034
-supported = False # managed by BIND itself
-class NSEC3Record(DNSRecord):
+class NSEC3Record(UnsupportedDNSRecord):
rrtype = 'NSEC3'
rfc = 5155
-supported = False
-class NSEC3PARAMRecord(DNSRecord):
+class NSEC3PARAMRecord(UnsupportedDNSRecord):
+# this is part of zone in IPA
rrtype = 'NSEC3PARAM'
rfc = 5155
-supported = False # this is part of zone in IPA
def _validate_naptr_flags(ugettext, flags):
allowed_flags = u'SAUP'
@@ -1365,10 +1370,9 @@ class PTRRecord(DNSRecord):
),
)
-class RPRecord(DNSRecord):
+class RPRecord(UnsupportedDNSRecord):
rrtype = 'RP'
rfc = 1183
-supported = False
class SRVRecord(DNSRecord):
rrtype = 'SRV'
@@ -1403,20 +1407,19 @@ def _sig_time_validator(ugettext, value):
return _('the value does not follow MMDDHHMMSS time format')
-class SIGRecord(DNSRecord):
+class SIGRecord(UnsupportedDNSRecord):
+# managed by BIND itself
rrtype = 'SIG'
rfc = 2535
-supported = False # managed by BIND itself
-class SPFRecord(DNSRecord):
+class SPFRecord(UnsupportedDNSRecord):
rrtype = 'SPF'
rfc = 4408
-supported = False
-class RRSIGRecord(SIGRecord):
+class RRSIGRecord(UnsupportedDNSRecord):
+# managed by BIND itself
rrtype = 'RRSIG'
rfc = 4034
-supported = False # managed by BIND itself
class SSHFPRecord(DNSRecord):
rrtype = 'SSHFP'
@@ -1445,9 +1448,8 @@ class SSHFPRecord(DNSRecord):
return tuple(values)
[Freeipa-devel] [PATCHES 0200-0202] DNS fixes related to unsupported records
Ticket: https://fedorahosted.org/freeipa/ticket/4930 0200: 4.1, master Fixes traceback, which was raised if LDAP contained a record that was marked as unsupported. Now unsupported records are shown, if LDAP contains them. 0200: 4.1, master Records marked as unsupported will not show options for editing parts. 0202: only master Removes NSEC3PARAM record from record types. NSEC3PARAM can contain only zone, value is allowed only in idnszone objectclass, so do not confuse users. -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 134-136] extdom: handle ERANGE return code for getXXYYY_r()
On Wed, Mar 04, 2015 at 04:17:55PM +0200, Alexander Bokovoy wrote:
On Mon, 02 Mar 2015, Sumit Bose wrote:
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
index
20fdd62b20f28f5384cf83b8be5819f721c6c3db..84aeb28066f25f05a89d0c2d42e8b060e2399501
100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
@@ -49,6 +49,220 @@
#define MAX(a,b) (((a)(b))?(a):(b))
#define SSSD_DOMAIN_SEPARATOR '@'
+#define MAX_BUF (1024*1024*1024)
+
+
+
+static int get_buffer(size_t *_buf_len, char **_buf)
+{
+long pw_max;
+long gr_max;
+size_t buf_len;
+char *buf;
+
+pw_max = sysconf(_SC_GETPW_R_SIZE_MAX);
+gr_max = sysconf(_SC_GETGR_R_SIZE_MAX);
+
+if (pw_max == -1 gr_max == -1) {
+buf_len = 16384;
+} else {
+buf_len = MAX(pw_max, gr_max);
+}
Here you'd get buf_len equal to 1024 by default on Linux which is too
low for our use case. I think it would be beneficial to add one more
MAX(buf_len, 16384):
-if (pw_max == -1 gr_max == -1) {
-buf_len = 16384;
-} else {
-buf_len = MAX(pw_max, gr_max);
-}
+buf_len = MAX(16384, MAX(pw_max, gr_max));
with MAX(MAX(),..) you also get rid of if() statement as resulting
rvalue would be guaranteed to be positive.
done
The rest is going along the common lines but would it be better to
allocate memory once per LDAP client request rather than always ask for
it per each NSS call? You can guarantee a sequential use of the buffer
within the LDAP client request processing so there is no problem with
locks but having this memory re-allocated on subsequent
getpwnam()/getpwuid()/... calls within the same request processing seems
suboptimal to me.
ok, makes sense, I moved get_buffer() back to the callers.
New version attached.
bye,
Sumit
--
/ Alexander Bokovoy
From 0b4e302866f734b93176d9104bd78a2e55702c40 Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Tue, 24 Feb 2015 15:29:00 +0100
Subject: [PATCH 134/136] Add configure check for cwrap libraries
Currently only nss-wrapper is checked, checks for other crwap libraries
can be added e.g. as
AM_CHECK_WRAPPER(uid_wrapper, HAVE_UID_WRAPPER)
---
daemons/configure.ac | 24
1 file changed, 24 insertions(+)
diff --git a/daemons/configure.ac b/daemons/configure.ac
index
97cd25115f371e9e549d209401df9325c7e112c1..7c979fe2d0b91e9d71fe4ca5a50ad78a4de79298
100644
--- a/daemons/configure.ac
+++ b/daemons/configure.ac
@@ -236,6 +236,30 @@ PKG_CHECK_EXISTS(cmocka,
)
AM_CONDITIONAL([HAVE_CMOCKA], [test x$have_cmocka = xyes])
+dnl A macro to check presence of a cwrap (http://cwrap.org) wrapper on the
system
+dnl Usage:
+dnl AM_CHECK_WRAPPER(name, conditional)
+dnl If the cwrap library is found, sets the HAVE_$name conditional
+AC_DEFUN([AM_CHECK_WRAPPER],
+[
+FOUND_WRAPPER=0
+
+AC_MSG_CHECKING([for $1])
+PKG_CHECK_EXISTS([$1],
+ [
+AC_MSG_RESULT([yes])
+FOUND_WRAPPER=1
+ ],
+ [
+AC_MSG_RESULT([no])
+AC_MSG_WARN([cwrap library $1 not found, some tests
will not run])
+ ])
+
+AM_CONDITIONAL($2, [ test x$FOUND_WRAPPER = x1])
+])
+
+AM_CHECK_WRAPPER(nss_wrapper, HAVE_NSS_WRAPPER)
+
dnl -- dirsrv is needed for the extdom unit tests --
PKG_CHECK_MODULES([DIRSRV], [dirsrv = 1.3.0])
dnl -- sss_idmap is needed by the extdom exop --
--
2.1.0
From 79fe3779aeabc007365b183abc1474ab2bba6a7b Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Tue, 24 Feb 2015 15:33:39 +0100
Subject: [PATCH 135/136] extdom: handle ERANGE return code for getXXYYY_r()
calls
The getXXYYY_r() calls require a buffer to store the variable data of
the passwd and group structs. If the provided buffer is too small ERANGE
is returned and the caller can try with a larger buffer again.
Cmocka/cwrap based unit-tests for get*_r_wrapper() are added.
Resolves https://fedorahosted.org/freeipa/ticket/4908
---
.../ipa-slapi-plugins/ipa-extdom-extop/Makefile.am | 31 ++-
.../ipa-extdom-extop/ipa_extdom.h | 9 +
.../ipa-extdom-extop/ipa_extdom_cmocka_tests.c | 226 +
.../ipa-extdom-extop/ipa_extdom_common.c | 274 +++--
.../ipa-extdom-extop/test_data/group | 2 +
.../ipa-extdom-extop/test_data/passwd | 2 +
.../ipa-extdom-extop/test_data/test_setup.sh | 3 +
7 files changed, 469 insertions(+), 78 deletions(-)
create mode 100644
daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c
create mode 100644 daemons/ipa-slapi-plugins/ipa-extdom-extop/test_data/group
create mode 100644
[Freeipa-devel] [PATCH 140] extdom: migrate check-based test to cmocka
Hi,
this is the first patch for https://fedorahosted.org/freeipa/ticket/4922
which converts the check-based tests of the extdom plugin to cmocka.
bye,
Sumit
From e11c525d27ab19abbb16e12195a2ea9eb6421c80 Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Mon, 9 Feb 2015 18:12:01 +0100
Subject: [PATCH] extdom: migrate check-based test to cmocka
Besides moving the existing tests to cmocka two new tests are added
which were missing from the old tests.
Related to https://fedorahosted.org/freeipa/ticket/4922
---
.../ipa-slapi-plugins/ipa-extdom-extop/Makefile.am | 20 --
.../ipa-extdom-extop/ipa_extdom.h | 14 ++
.../ipa-extdom-extop/ipa_extdom_cmocka_tests.c | 156 +++-
.../ipa-extdom-extop/ipa_extdom_common.c | 28 +--
.../ipa-extdom-extop/ipa_extdom_tests.c| 203 -
5 files changed, 176 insertions(+), 245 deletions(-)
delete mode 100644
daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_tests.c
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/Makefile.am
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/Makefile.am
index
a1679812ef3c5de8c6e18433cbb991a99ad0b6c8..9c2fa1c6a5f95ba06b33c0a5b560939863a88f0e
100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/Makefile.am
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/Makefile.am
@@ -38,11 +38,6 @@ libipa_extdom_extop_la_LIBADD = \
TESTS =
check_PROGRAMS =
-if HAVE_CHECK
-TESTS += extdom_tests
-check_PROGRAMS += extdom_tests
-endif
-
if HAVE_CMOCKA
if HAVE_NSS_WRAPPER
TESTS_ENVIRONMENT = . ./test_data/test_setup.sh;
@@ -51,21 +46,6 @@ check_PROGRAMS += extdom_cmocka_tests
endif
endif
-extdom_tests_SOURCES = \
- ipa_extdom_tests.c \
- ipa_extdom_common.c \
- $(NULL)
-extdom_tests_CFLAGS = $(CHECK_CFLAGS)
-extdom_tests_LDFLAGS = \
- -rpath $(shell pkg-config --libs-only-L dirsrv | sed -e 's/-L//') \
- $(NULL)
-extdom_tests_LDADD = \
- $(CHECK_LIBS) \
- $(LDAP_LIBS)\
- $(DIRSRV_LIBS) \
- $(SSSNSSIDMAP_LIBS) \
- $(NULL)
-
extdom_cmocka_tests_SOURCES = \
ipa_extdom_cmocka_tests.c \
ipa_extdom_common.c \
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
index
0d5d55d2fb0ece95466b0225b145a4edeef18efa..65dd43ea35726db6231386a0fcbba9be1bd71412
100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
@@ -185,5 +185,19 @@ int getgrnam_r_wrapper(size_t buf_max, const char *name,
struct group *grp, char **_buf, size_t *_buf_len);
int getgrgid_r_wrapper(size_t buf_max, gid_t gid,
struct group *grp, char **_buf, size_t *_buf_len);
+int pack_ber_sid(const char *sid, struct berval **berval);
+int pack_ber_name(const char *domain_name, const char *name,
+ struct berval **berval);
+int pack_ber_user(struct ipa_extdom_ctx *ctx,
+ enum response_types response_type,
+ const char *domain_name, const char *user_name,
+ uid_t uid, gid_t gid,
+ const char *gecos, const char *homedir,
+ const char *shell, struct sss_nss_kv *kv_list,
+ struct berval **berval);
+int pack_ber_group(enum response_types response_type,
+ const char *domain_name, const char *group_name,
+ gid_t gid, char **members, struct sss_nss_kv *kv_list,
+ struct berval **berval);
void set_err_msg(struct extdom_req *req, const char *format, ...);
#endif /* _IPA_EXTDOM_H_ */
diff --git
a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c
index
0ca67030bf667ecd559443842cda166354ce54ce..87cb79aea38a901058de745fe986426479aa34b6
100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c
@@ -213,30 +213,46 @@ void test_getgrgid_r_wrapper(void **state)
free(buf);
}
+struct test_data {
+struct extdom_req *req;
+struct ipa_extdom_ctx *ctx;
+};
+
void extdom_req_setup(void **state)
{
-struct extdom_req *req;
+struct test_data *test_data;
-req = calloc(sizeof(struct extdom_req), 1);
-assert_non_null(req);
+test_data = calloc(sizeof(struct test_data), 1);
+assert_non_null(test_data);
-*state = req;
+test_data-req = calloc(sizeof(struct extdom_req), 1);
+assert_non_null(test_data-req);
+
+test_data-ctx = calloc(sizeof(struct ipa_extdom_ctx), 1);
+assert_non_null(test_data-req);
+
+*state = test_data;
}
void extdom_req_teardown(void **state)
{
-struct extdom_req *req;
+struct test_data *test_data;
-req =
Re: [Freeipa-devel] [PATCH] 0040 Add realm name to backup header file.
On 03/04/2015 02:11 PM, David Kupka wrote:
https://fedorahosted.org/freeipa/ticket/4896
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Honza proposed different approach. We can extract default.conf and use
it in to create api object with the right values.
I've implemented it and it works too additionally we don't need to
change the header content for now.
--
David Kupka
From 858452056af1b1080e647eaeb591dbd9a6e07a84 Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Wed, 4 Mar 2015 10:06:47 -0500
Subject: [PATCH] Restore default.conf and use it to build API.
When restoring ipa after uninstallation we need to extract and load
configuration of the restored environment.
https://fedorahosted.org/freeipa/ticket/4896
---
ipaserver/install/ipa_restore.py | 67 ++--
1 file changed, 51 insertions(+), 16 deletions(-)
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index efe3b9b1c0c10775b3a72b9d843924263526209a..0574fc26681f4dfb4d77b6c0779f119fd38785fc 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -27,7 +27,7 @@ from ConfigParser import SafeConfigParser
import ldif
import itertools
-from ipalib import api, errors
+from ipalib import api, errors, constants
from ipapython import version, ipautil, certdb, dogtag
from ipapython.ipautil import run, user_input
from ipapython import admintool
@@ -199,19 +199,43 @@ class Restore(admintool.AdminTool):
Directory Manager password required)
+def restore_default_conf(self):
+'''
+Restore paths.IPA_DEFAULT_CONF to temporary directory.
+
+Primary purpose of this method is to get cofiguration for api
+finalization when restoring ipa after uninstall.
+'''
+cwd = os.getcwd()
+os.chdir(self.dir)
+args = ['tar',
+'--xattrs',
+'--selinux',
+'-xzf',
+os.path.join(self.dir, 'files.tar'),
+paths.IPA_DEFAULT_CONF[1:],
+ ]
+
+(stdout, stderr, rc) = run(args, raiseonerr=False)
+if rc != 0:
+self.log.critical('Restoring %s failed: %s' %
+ (paths.IPA_DEFAULT_CONF, stderr))
+
+os.chdir(cwd)
+
+return os.path.join(self.dir, paths.IPA_DEFAULT_CONF)
+
+
def run(self):
options = self.options
super(Restore, self).run()
-api.bootstrap(in_server=False, context='restore')
-api.finalize()
-
self.backup_dir = self.args[0]
if not os.path.isabs(self.backup_dir):
self.backup_dir = os.path.join(BACKUP_DIR, self.backup_dir)
self.log.info(Preparing restore from %s on %s,
- self.backup_dir, api.env.host)
+ self.backup_dir, constants.FQDN)
self.header = os.path.join(self.backup_dir, 'header')
@@ -225,9 +249,6 @@ class Restore(admintool.AdminTool):
else:
restore_type = self.backup_type
-instances = [realm_to_serverid(api.env.realm), 'PKI-IPA']
-backends = ['userRoot', 'ipaca']
-
# These checks would normally be in the validate method but
# we need to know the type of backup we're dealing with.
if restore_type == 'FULL':
@@ -241,6 +262,8 @@ class Restore(admintool.AdminTool):
else:
installutils.check_server_configuration()
+self.init_api()
+
if options.instance:
instance_dir = (paths.VAR_LIB_SLAPD_INSTANCE_DIR_TEMPLATE %
options.instance)
@@ -248,10 +271,10 @@ class Restore(admintool.AdminTool):
raise admintool.ScriptError(
Instance %s does not exist % options.instance)
-instances = [options.instance]
+self.instances = [options.instance]
if options.backend:
-for instance in instances:
+for instance in self.instances:
db_dir = (paths.SLAPD_INSTANCE_DB_DIR_TEMPLATE %
(instance, options.backend))
if os.path.exists(db_dir):
@@ -260,9 +283,10 @@ class Restore(admintool.AdminTool):
raise admintool.ScriptError(
Backend %s does not exist % options.backend)
-backends = [options.backend]
+self.backends = [options.backend]
-for instance, backend in itertools.product(instances, backends):
+for instance, backend in itertools.product(self.instances,
+ self.backends):
db_dir = (paths.SLAPD_INSTANCE_DB_DIR_TEMPLATE %
(instance,
[Freeipa-devel] IPA 4.2 server upgrade refactoring - summary
Summary extracted from thread [Freeipa-devel] IPA Server upgrade 4.2 design Design page: http://www.freeipa.org/page/V4/Server_Upgrade_Refactoring * ipa-server-upgrade will not allow to use DM password, only LDAPI will be used for upgrade * upgrade files will be executed in alphabetical order, updater will not require number in update file name. But we should still keep the numbering in new upgrade files. * LDAP updates will be applied per file, in order specified in file (from top to bottom) * new directive in update files *plugin:plugin-name* will execute update plugins (renamed form update-plugin to plugin) * option --skip-version-check will override version check in ipactl and ipa-server-upgrade commands (was --force before, but this collides with existing --force option in ipactl) * huge warning, this may broke everything, in help, man, or CLI for --skip-version-check option * ipa-upgradeconfig will be removed * ipa-ldap-updater will be changed to not allow overall update. It stays as util for execute particular update files. How and when execute upgrades (after discussion with Honza) -- not updated in design page yet A) ipactl*: A.1) compare build platform and platform from last upgrade/installation (based on used ipaplatform file) A.1.i) if platform mismatch, raise error and prevent to start services A.2) version of LDAP data(+schema included) compared to current version (VENDOR_VERSION will be used) A.2.i) if version of LDAP data is newer than version of build, raise error and prevent services to start A.2.ii) if version of LDAP data is older than version of build, upgrade is required A.2.iii) if versions are the same, continue A.3) check if services requires update (this should be available after installer refactoring)** A.3.i) if any service requires configuration upgrade, upgrade is required A.3.ii) if any service raises an error about wrong configuration (which cannot be automatically fixed and requires manual fix by user), raise error and prevent to start services A.4.i) if upgrade is needed, ipactl will prevent to start services, and promt user to run ipa-server-upgrade manually (ipactl will not execute upgrade itself) A.4.ii) otherwise start services B) ipa-server-upgrade* B.0) services should be in shutdown state, if not, stop services (services will be started during upgrade on demand, then stopped) B.1) compare build platform and platform from last upgrade/installation (based on used ipaplatform file) B.1.i) if platform mismatch, raise error stop upgrade B.2) check version of LDAP data B.2.i) if LDAP data version is newer than build version, raise error stop upgrade B.2.ii) if LDAP data version is the same as build version, skip schema and LDAP data upgrade B.2.iii) if LDAP data version is older than build version -- data upgrade required B.3) Check if services require upgrade, detect errors as in A.3) (?? this step may not be there)** B.4) if data upgrade required, upgrade schema, then upgrade data, if successful store current build version as data version B.5) Run service upgrade (if needed?)** B.6) if upgrade is successful, inform user that the one can now start IPA (upgrade will not start IPA after it is done) * with --skip-version-check option, ipactl will start services, ipa-server-upgrade will upgrade everything ** services will handle local configuration upgrade by themselves. They will not use data version to decide if upgrade is required (TODO implementation details, Honza wants it in this way - sharing code with installers) Upgrade in different enviroments: 1) Upgrade during RPM transaction (as we do now) -- if it is possible, upgrade will be executed during RPM transaction, service will be started after upgrade (+ add messages IPA is currently upgrading, please wait) 2) Upgrade cannot be executed during RPM transaction (fedup, --no-script, containers) -- IPA will not start if update is required, user have to run upgrade manually, using ipa-server-upgrade command (+ log/print errors that there is upgrade required) Martin^2 -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] extdom: return LDAP_NO_SUCH_OBJECT to the client
Hi,
with this patch the extdom plugin will properly indicate to a client if
the search object does not exist instead of returning a generic error.
This is important for the client to act accordingly and improve
debugging possibilities.
bye,
Sumit
From 3895fa21524efc3a22bfb36b1a9aa34277b8dd46 Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Wed, 4 Mar 2015 13:39:04 +0100
Subject: [PATCH] extdom: return LDAP_NO_SUCH_OBJECT to the client
---
daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
index
a70ed20f1816a7e00385edae8a81dd5dad9e9362..a040f2beba073d856053429face2f464347b2524
100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
@@ -123,8 +123,12 @@ static int ipa_extdom_extop(Slapi_PBlock *pb)
ret = handle_request(ctx, req, ret_val);
if (ret != LDAP_SUCCESS) {
-rc = LDAP_OPERATIONS_ERROR;
-err_msg = Failed to handle the request.\n;
+if (ret == LDAP_NO_SUCH_OBJECT) {
+rc = LDAP_NO_SUCH_OBJECT;
+} else {
+rc = LDAP_OPERATIONS_ERROR;
+err_msg = Failed to handle the request.\n;
+}
goto done;
}
--
2.1.0
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 142] extdom: fix memory leak
Hi, while running 389ds with valgrind to see if my other patches introduced a memory leak I found an older one which is fixed by this patch. bye, Sumit From bb02cdc135fecc1766b17edd61554dbde9bccd0b Mon Sep 17 00:00:00 2001 From: Sumit Bose sb...@redhat.com Date: Wed, 4 Mar 2015 17:53:08 +0100 Subject: [PATCH] extdom: fix memory leak --- daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c | 1 + 1 file changed, 1 insertion(+) diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c index a040f2beba073d856053429face2f464347b2524..708d0e4a2fc9da4f87a24a49c945587049f7280f 100644 --- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c +++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c @@ -156,6 +156,7 @@ done: LOG(%s, err_msg); } slapi_send_ldap_result(pb, rc, NULL, err_msg, 0, NULL); +ber_bvfree(ret_val); free_req_data(req); return SLAPI_PLUGIN_EXTENDED_SENT_RESULT; } -- 2.1.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0001-0002] ipa-client-install NTP fixes
On 03/04/2015 10:58 AM, Martin Basti wrote: On 04/03/15 19:56, Nathan Kinder wrote: On 03/04/2015 10:41 AM, Rob Crittenden wrote: Nathan Kinder wrote: On 02/28/2015 01:13 PM, Nathan Kinder wrote: On 02/28/2015 01:07 PM, Rob Crittenden wrote: Nathan Kinder wrote: On 02/27/2015 01:18 PM, Nathan Kinder wrote: On 02/27/2015 01:08 PM, Rob Crittenden wrote: Nathan Kinder wrote: On 02/27/2015 12:20 PM, Rob Crittenden wrote: Nathan Kinder wrote: On 02/26/2015 12:55 AM, Martin Kosek wrote: On 02/26/2015 03:28 AM, Nathan Kinder wrote: Hi, The two attached patches address some issues that affect ipa-client-install when syncing time from the NTP server. Now that we use ntpd to perform the time sync, the client install can end up hanging forever when the server is not reachable (firewall issues, etc.). These patches address the issues in two different ways: 1 - Don't attempt to sync time when --no-ntp is specified. 2 - Implement a timeout capability that is used when we run ntpd to perform the time sync to prevent indefinite hanging. The one potentially contentious issue is that this introduces a new dependency on python-subprocess32 to allow us to have timeout support when using Python 2.x. This is packaged for Fedora, but I don't see it on RHEL or CentOS currently. It would need to be packaged there. https://fedorahosted.org/freeipa/ticket/4842 Thanks, -NGK Thanks for Patches. For the second patch, I would really prefer to avoid new dependency, especially if it's not packaged in RHEL/CentOS. Maybe we could use some workaround instead, as in: http://stackoverflow.com/questions/3733270/python-subprocess-timeout I don't like having to add an additional dependency either, but the alternative seems more risky. Utilizing the subprocess32 module (which is really just a backport of the normal subprocess module from Python 3.x) is not invasive for our code in ipautil.run(). Adding some sort of a thread that has to kill the spawned subprocess seems more risky (see the discussion about a race condition in the stackoverflow thread above). That said, I'm sure the thread/poll method can be made to work if you and others feel strongly that this is a better approach than adding a new dependency. Why not use /usr/bin/timeout from coreutils? That sounds like a perfectly good idea. I wasn't aware of it's existence (or it's possible that I forgot about it). Thanks for the suggestion! I'll test out a reworked version of the patch. Do you think that there is value in leaving the timeout capability centrally in ipautil.run()? We only need it for the call to 'ntpd' right now, but there might be a need for using a timeout for other commands in the future. The alternative is to just modify synconce_ntp() to use /usr/bin/timeout and leave ipautil.run() alone. I think it would require a lot of research. One of the programs spawned by this is pkicreate which could take quite some time, and spawning a clone in particular. It is definitely an interesting idea but I think it is safest for now to limit it to just NTP for now. What I meant was that we would have an optional keyword timeout parameter to ipautil.run() that defaults to None, just like my subprocess32 approach. If a timeout is not passed in, we would use subprocess.Popen() to run the specified command just like we do today. We would only actually pass the timeout parameter to ipautil.run() in synconce_ntp() for now, so no other commands would have a timeout in effect. The capability would be available for other commands this way though. Let me propose a patch with this implementation, and if you don't like it, we can leave ipautil.run() alone and restrict the changes to synconce_ntp(). An updated patch 0002 is attached that uses the approach mentioned above. Looks good. Not to nitpick to death but... Can you add timeout to ipaplatform/base/paths.py as BIN_TIMEOUT = /usr/bin/timeout and reference that instead? It's for portability. Sure. I was wondering if we should do something around a full path. And a question. I'm impatient. Should there be a notice that it will timeout after n seconds somewhere so people like me don't ^C after 2 seconds? Or is that just overkill and I need to learn patience? Probably both. :) There's always going to be someone out there who will do ctrl-C, so I think printing out a notice is a good idea. I'll add this. Stylistically, should we prefer p.returncode is 15 or p.returncode == 15? After some reading, it seems that '==' should be used. Small integers work with 'is', but '==' is the consistent way that equality of integers should be checked. I'll modify this. Another updated patch 0002 is attached that addresses Rob's review comments. Thanks, -NGK LGTM. Does someone else have time to test this? I also don't know if there is a policy on placement of new items in paths.py. Things are all over
[Freeipa-devel] [PATCHES 137-139] extdom: add err_msg member to request context
Hi,
this patch series improves error reporting of the extdom plugin
especially on the client side. Currently there is only SSSD ticket
https://fedorahosted.org/sssd/ticket/2463 . Shall I create a
corresponding FreeIPA ticket as well?
In the third patch I already added a handful of new error messages.
Suggestions for more messages are welcome.
bye,
Sumit
From 2e8e4abb7e79d44f0ce0560daeb7696d9641a684 Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Mon, 2 Feb 2015 00:52:10 +0100
Subject: [PATCH 137/139] extdom: add err_msg member to request context
---
daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h| 1 +
daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c | 1 +
daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c | 5 -
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
index
d4c851169ddadc869a59c53075f9fc7f33321085..421f6c6ea625aba2db7e9ffc84115b3647673699
100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
@@ -116,6 +116,7 @@ struct extdom_req {
gid_t gid;
} posix_gid;
} data;
+char *err_msg;
};
struct extdom_res {
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
index
448710993f551298d3a4cdcc19371b8432773478..27c1313cb1f6f614b0c74992d4a768c3051a86ae
100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
@@ -360,6 +360,7 @@ void free_req_data(struct extdom_req *req)
break;
}
+free(req-err_msg);
free(req);
}
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
index
e53f968db040a37fbd6a193f87b3671eeabda89d..a70ed20f1816a7e00385edae8a81dd5dad9e9362
100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
@@ -145,11 +145,14 @@ static int ipa_extdom_extop(Slapi_PBlock *pb)
rc = LDAP_SUCCESS;
done:
-free_req_data(req);
+if (req-err_msg != NULL) {
+err_msg = req-err_msg;
+}
if (err_msg != NULL) {
LOG(%s, err_msg);
}
slapi_send_ldap_result(pb, rc, NULL, err_msg, 0, NULL);
+free_req_data(req);
return SLAPI_PLUGIN_EXTENDED_SENT_RESULT;
}
--
2.1.0
From 80406c884eddeb2ebf35bd12a7ed1a8ddd4af2fe Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Mon, 2 Feb 2015 00:53:06 +0100
Subject: [PATCH 138/139] extdom: add add_err_msg() with test
---
.../ipa-extdom-extop/ipa_extdom.h | 1 +
.../ipa-extdom-extop/ipa_extdom_cmocka_tests.c | 43 ++
.../ipa-extdom-extop/ipa_extdom_common.c | 23
3 files changed, 67 insertions(+)
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
index
421f6c6ea625aba2db7e9ffc84115b3647673699..0d5d55d2fb0ece95466b0225b145a4edeef18efa
100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
@@ -185,4 +185,5 @@ int getgrnam_r_wrapper(size_t buf_max, const char *name,
struct group *grp, char **_buf, size_t *_buf_len);
int getgrgid_r_wrapper(size_t buf_max, gid_t gid,
struct group *grp, char **_buf, size_t *_buf_len);
+void set_err_msg(struct extdom_req *req, const char *format, ...);
#endif /* _IPA_EXTDOM_H_ */
diff --git
a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c
index
be736dd9c5af4d0b632f1dbc55033fdf738bad46..0ca67030bf667ecd559443842cda166354ce54ce
100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c
@@ -213,6 +213,47 @@ void test_getgrgid_r_wrapper(void **state)
free(buf);
}
+void extdom_req_setup(void **state)
+{
+struct extdom_req *req;
+
+req = calloc(sizeof(struct extdom_req), 1);
+assert_non_null(req);
+
+*state = req;
+}
+
+void extdom_req_teardown(void **state)
+{
+struct extdom_req *req;
+
+req = (struct extdom_req *) *state;
+
+free_req_data(req);
+}
+
+void test_set_err_msg(void **state)
+{
+struct extdom_req *req;
+
+req = (struct extdom_req *) *state;
+assert_null(req-err_msg);
+
+set_err_msg(NULL, NULL);
+assert_null(req-err_msg);
+
+set_err_msg(req, NULL);
+assert_null(req-err_msg);
+
+set_err_msg(req, Test [%s][%d]., ABCD, 1234);
+assert_non_null(req-err_msg);
+
Re: [Freeipa-devel] [PATCHES 0001-0002] ipa-client-install NTP fixes
Nathan Kinder wrote: On 02/28/2015 01:13 PM, Nathan Kinder wrote: On 02/28/2015 01:07 PM, Rob Crittenden wrote: Nathan Kinder wrote: On 02/27/2015 01:18 PM, Nathan Kinder wrote: On 02/27/2015 01:08 PM, Rob Crittenden wrote: Nathan Kinder wrote: On 02/27/2015 12:20 PM, Rob Crittenden wrote: Nathan Kinder wrote: On 02/26/2015 12:55 AM, Martin Kosek wrote: On 02/26/2015 03:28 AM, Nathan Kinder wrote: Hi, The two attached patches address some issues that affect ipa-client-install when syncing time from the NTP server. Now that we use ntpd to perform the time sync, the client install can end up hanging forever when the server is not reachable (firewall issues, etc.). These patches address the issues in two different ways: 1 - Don't attempt to sync time when --no-ntp is specified. 2 - Implement a timeout capability that is used when we run ntpd to perform the time sync to prevent indefinite hanging. The one potentially contentious issue is that this introduces a new dependency on python-subprocess32 to allow us to have timeout support when using Python 2.x. This is packaged for Fedora, but I don't see it on RHEL or CentOS currently. It would need to be packaged there. https://fedorahosted.org/freeipa/ticket/4842 Thanks, -NGK Thanks for Patches. For the second patch, I would really prefer to avoid new dependency, especially if it's not packaged in RHEL/CentOS. Maybe we could use some workaround instead, as in: http://stackoverflow.com/questions/3733270/python-subprocess-timeout I don't like having to add an additional dependency either, but the alternative seems more risky. Utilizing the subprocess32 module (which is really just a backport of the normal subprocess module from Python 3.x) is not invasive for our code in ipautil.run(). Adding some sort of a thread that has to kill the spawned subprocess seems more risky (see the discussion about a race condition in the stackoverflow thread above). That said, I'm sure the thread/poll method can be made to work if you and others feel strongly that this is a better approach than adding a new dependency. Why not use /usr/bin/timeout from coreutils? That sounds like a perfectly good idea. I wasn't aware of it's existence (or it's possible that I forgot about it). Thanks for the suggestion! I'll test out a reworked version of the patch. Do you think that there is value in leaving the timeout capability centrally in ipautil.run()? We only need it for the call to 'ntpd' right now, but there might be a need for using a timeout for other commands in the future. The alternative is to just modify synconce_ntp() to use /usr/bin/timeout and leave ipautil.run() alone. I think it would require a lot of research. One of the programs spawned by this is pkicreate which could take quite some time, and spawning a clone in particular. It is definitely an interesting idea but I think it is safest for now to limit it to just NTP for now. What I meant was that we would have an optional keyword timeout parameter to ipautil.run() that defaults to None, just like my subprocess32 approach. If a timeout is not passed in, we would use subprocess.Popen() to run the specified command just like we do today. We would only actually pass the timeout parameter to ipautil.run() in synconce_ntp() for now, so no other commands would have a timeout in effect. The capability would be available for other commands this way though. Let me propose a patch with this implementation, and if you don't like it, we can leave ipautil.run() alone and restrict the changes to synconce_ntp(). An updated patch 0002 is attached that uses the approach mentioned above. Looks good. Not to nitpick to death but... Can you add timeout to ipaplatform/base/paths.py as BIN_TIMEOUT = /usr/bin/timeout and reference that instead? It's for portability. Sure. I was wondering if we should do something around a full path. And a question. I'm impatient. Should there be a notice that it will timeout after n seconds somewhere so people like me don't ^C after 2 seconds? Or is that just overkill and I need to learn patience? Probably both. :) There's always going to be someone out there who will do ctrl-C, so I think printing out a notice is a good idea. I'll add this. Stylistically, should we prefer p.returncode is 15 or p.returncode == 15? After some reading, it seems that '==' should be used. Small integers work with 'is', but '==' is the consistent way that equality of integers should be checked. I'll modify this. Another updated patch 0002 is attached that addresses Rob's review comments. Thanks, -NGK LGTM. Does someone else have time to test this? I also don't know if there is a policy on placement of new items in paths.py. Things are all over the place and some have BIN_ prefix and some don't. rob ___ Freeipa-devel mailing
Re: [Freeipa-devel] [PATCHES 0001-0002] ipa-client-install NTP fixes
On 04/03/15 19:56, Nathan Kinder wrote: On 03/04/2015 10:41 AM, Rob Crittenden wrote: Nathan Kinder wrote: On 02/28/2015 01:13 PM, Nathan Kinder wrote: On 02/28/2015 01:07 PM, Rob Crittenden wrote: Nathan Kinder wrote: On 02/27/2015 01:18 PM, Nathan Kinder wrote: On 02/27/2015 01:08 PM, Rob Crittenden wrote: Nathan Kinder wrote: On 02/27/2015 12:20 PM, Rob Crittenden wrote: Nathan Kinder wrote: On 02/26/2015 12:55 AM, Martin Kosek wrote: On 02/26/2015 03:28 AM, Nathan Kinder wrote: Hi, The two attached patches address some issues that affect ipa-client-install when syncing time from the NTP server. Now that we use ntpd to perform the time sync, the client install can end up hanging forever when the server is not reachable (firewall issues, etc.). These patches address the issues in two different ways: 1 - Don't attempt to sync time when --no-ntp is specified. 2 - Implement a timeout capability that is used when we run ntpd to perform the time sync to prevent indefinite hanging. The one potentially contentious issue is that this introduces a new dependency on python-subprocess32 to allow us to have timeout support when using Python 2.x. This is packaged for Fedora, but I don't see it on RHEL or CentOS currently. It would need to be packaged there. https://fedorahosted.org/freeipa/ticket/4842 Thanks, -NGK Thanks for Patches. For the second patch, I would really prefer to avoid new dependency, especially if it's not packaged in RHEL/CentOS. Maybe we could use some workaround instead, as in: http://stackoverflow.com/questions/3733270/python-subprocess-timeout I don't like having to add an additional dependency either, but the alternative seems more risky. Utilizing the subprocess32 module (which is really just a backport of the normal subprocess module from Python 3.x) is not invasive for our code in ipautil.run(). Adding some sort of a thread that has to kill the spawned subprocess seems more risky (see the discussion about a race condition in the stackoverflow thread above). That said, I'm sure the thread/poll method can be made to work if you and others feel strongly that this is a better approach than adding a new dependency. Why not use /usr/bin/timeout from coreutils? That sounds like a perfectly good idea. I wasn't aware of it's existence (or it's possible that I forgot about it). Thanks for the suggestion! I'll test out a reworked version of the patch. Do you think that there is value in leaving the timeout capability centrally in ipautil.run()? We only need it for the call to 'ntpd' right now, but there might be a need for using a timeout for other commands in the future. The alternative is to just modify synconce_ntp() to use /usr/bin/timeout and leave ipautil.run() alone. I think it would require a lot of research. One of the programs spawned by this is pkicreate which could take quite some time, and spawning a clone in particular. It is definitely an interesting idea but I think it is safest for now to limit it to just NTP for now. What I meant was that we would have an optional keyword timeout parameter to ipautil.run() that defaults to None, just like my subprocess32 approach. If a timeout is not passed in, we would use subprocess.Popen() to run the specified command just like we do today. We would only actually pass the timeout parameter to ipautil.run() in synconce_ntp() for now, so no other commands would have a timeout in effect. The capability would be available for other commands this way though. Let me propose a patch with this implementation, and if you don't like it, we can leave ipautil.run() alone and restrict the changes to synconce_ntp(). An updated patch 0002 is attached that uses the approach mentioned above. Looks good. Not to nitpick to death but... Can you add timeout to ipaplatform/base/paths.py as BIN_TIMEOUT = /usr/bin/timeout and reference that instead? It's for portability. Sure. I was wondering if we should do something around a full path. And a question. I'm impatient. Should there be a notice that it will timeout after n seconds somewhere so people like me don't ^C after 2 seconds? Or is that just overkill and I need to learn patience? Probably both. :) There's always going to be someone out there who will do ctrl-C, so I think printing out a notice is a good idea. I'll add this. Stylistically, should we prefer p.returncode is 15 or p.returncode == 15? After some reading, it seems that '==' should be used. Small integers work with 'is', but '==' is the consistent way that equality of integers should be checked. I'll modify this. Another updated patch 0002 is attached that addresses Rob's review comments. Thanks, -NGK LGTM. Does someone else have time to test this? I also don't know if there is a policy on placement of new items in paths.py. Things are all over the place and some have BIN_ prefix and some don't. Yeah, I noticed this too. It didn't look like there was any organization. -NGK rob
Re: [Freeipa-devel] [PATCH 142] extdom: fix memory leak
On Wed, 04 Mar 2015, Nathan Kinder wrote:
On 03/04/2015 10:34 PM, Alexander Bokovoy wrote:
On Wed, 04 Mar 2015, Sumit Bose wrote:
Hi,
while running 389ds with valgrind to see if my other patches introduced
a memory leak I found an older one which is fixed by this patch.
bye,
Sumit
From bb02cdc135fecc1766b17edd61554dbde9bccd0b Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Wed, 4 Mar 2015 17:53:08 +0100
Subject: [PATCH] extdom: fix memory leak
---
daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c | 1 +
1 file changed, 1 insertion(+)
diff --git
a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
index
a040f2beba073d856053429face2f464347b2524..708d0e4a2fc9da4f87a24a49c945587049f7280f
100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
@@ -156,6 +156,7 @@ done:
LOG(%s, err_msg);
}
slapi_send_ldap_result(pb, rc, NULL, err_msg, 0, NULL);
+ber_bvfree(ret_val);
free_req_data(req);
return SLAPI_PLUGIN_EXTENDED_SENT_RESULT;
}
I can see in 389-ds code that it actually tries to remove the value in
the end of extended operation handling:
This below code snippet is freeing the extended operation request value
(SLAPI_EXT_OP_REQ_VALUE), not the return value (SLAPI_EXT_OP_RET_VAL).
If you look at check_and_send_extended_result() in the 389-ds code,
you'll see where the extended operation return value is sent, and it
doesn't perform a free. It is up to the plug-in to perform the free. A
good example of this in the 389-ds code is in the passwd_modify_extop()
function.
Sumit's code looks good to me. ACK.
Argh. Sorry for confusion of RET vs REQ. Morning, I need coffee!
ACK.
-NGK
slapi_pblock_set( pb, SLAPI_EXT_OP_REQ_OID, extoid );
slapi_pblock_set( pb, SLAPI_EXT_OP_REQ_VALUE, extval );
slapi_pblock_set( pb, SLAPI_REQUESTOR_ISROOT, pb-pb_op-o_isroot);
rc = plugin_call_exop_plugins( pb, extoid );
if ( SLAPI_PLUGIN_EXTENDED_SENT_RESULT != rc ) {
if ( SLAPI_PLUGIN_EXTENDED_NOT_HANDLED == rc ) {
lderr = LDAP_PROTOCOL_ERROR;/* no plugin
handled the op */
errmsg = unsupported extended operation;
} else {
errmsg = NULL;
lderr = rc;
}
send_ldap_result( pb, lderr, NULL, errmsg, 0, NULL );
}
free_and_return:
if (extoid)
slapi_ch_free((void **)extoid);
if (extval.bv_val)
slapi_ch_free((void **)extval.bv_val);
return;
--
/ Alexander Bokovoy
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 142] extdom: fix memory leak
On 03/04/2015 10:34 PM, Alexander Bokovoy wrote:
On Wed, 04 Mar 2015, Sumit Bose wrote:
Hi,
while running 389ds with valgrind to see if my other patches introduced
a memory leak I found an older one which is fixed by this patch.
bye,
Sumit
From bb02cdc135fecc1766b17edd61554dbde9bccd0b Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Wed, 4 Mar 2015 17:53:08 +0100
Subject: [PATCH] extdom: fix memory leak
---
daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c | 1 +
1 file changed, 1 insertion(+)
diff --git
a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
index
a040f2beba073d856053429face2f464347b2524..708d0e4a2fc9da4f87a24a49c945587049f7280f
100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
@@ -156,6 +156,7 @@ done:
LOG(%s, err_msg);
}
slapi_send_ldap_result(pb, rc, NULL, err_msg, 0, NULL);
+ber_bvfree(ret_val);
free_req_data(req);
return SLAPI_PLUGIN_EXTENDED_SENT_RESULT;
}
I can see in 389-ds code that it actually tries to remove the value in
the end of extended operation handling:
This below code snippet is freeing the extended operation request value
(SLAPI_EXT_OP_REQ_VALUE), not the return value (SLAPI_EXT_OP_RET_VAL).
If you look at check_and_send_extended_result() in the 389-ds code,
you'll see where the extended operation return value is sent, and it
doesn't perform a free. It is up to the plug-in to perform the free. A
good example of this in the 389-ds code is in the passwd_modify_extop()
function.
Sumit's code looks good to me. ACK.
-NGK
slapi_pblock_set( pb, SLAPI_EXT_OP_REQ_OID, extoid );
slapi_pblock_set( pb, SLAPI_EXT_OP_REQ_VALUE, extval );
slapi_pblock_set( pb, SLAPI_REQUESTOR_ISROOT, pb-pb_op-o_isroot);
rc = plugin_call_exop_plugins( pb, extoid );
if ( SLAPI_PLUGIN_EXTENDED_SENT_RESULT != rc ) {
if ( SLAPI_PLUGIN_EXTENDED_NOT_HANDLED == rc ) {
lderr = LDAP_PROTOCOL_ERROR;/* no plugin
handled the op */
errmsg = unsupported extended operation;
} else {
errmsg = NULL;
lderr = rc;
}
send_ldap_result( pb, lderr, NULL, errmsg, 0, NULL );
}
free_and_return:
if (extoid)
slapi_ch_free((void **)extoid);
if (extval.bv_val)
slapi_ch_free((void **)extval.bv_val);
return;
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] SPEC: Require python2 version of sssd bindings
On 02/27/2015 09:50 PM, Lukas Slebodnik wrote: ehlo, Please review attached patches and fix freeipa in fedora 22 ASAP. I think the most critical is 1st patch sh$ git grep SSSDConfig | grep import install/tools/ipa-upgradeconfig:import SSSDConfig ipa-client/ipa-install/ipa-client-automount:import SSSDConfig ipa-client/ipa-install/ipa-client-install:import SSSDConfig BTW package python-sssdconfig is provides since sssd-1.10.0alpha1 (2013-04-02) but it was not explicitely required. The latest python3 changes in sssd (fedora 22) is just a result of negligent packaging of freeipa. LS Fedora 22 was amended. Patch 1: ACK Patch 2: ACK Patch3: the package name is libsss_nss_idmap-python not python-libsss_nss_idmap which already is required in adtrust package -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] extdom: return LDAP_NO_SUCH_OBJECT to the client
On Wed, 04 Mar 2015, Sumit Bose wrote:
Hi,
with this patch the extdom plugin will properly indicate to a client if
the search object does not exist instead of returning a generic error.
This is important for the client to act accordingly and improve
debugging possibilities.
bye,
Sumit
From 3895fa21524efc3a22bfb36b1a9aa34277b8dd46 Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Wed, 4 Mar 2015 13:39:04 +0100
Subject: [PATCH] extdom: return LDAP_NO_SUCH_OBJECT to the client
---
daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
index
a70ed20f1816a7e00385edae8a81dd5dad9e9362..a040f2beba073d856053429face2f464347b2524
100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
@@ -123,8 +123,12 @@ static int ipa_extdom_extop(Slapi_PBlock *pb)
ret = handle_request(ctx, req, ret_val);
if (ret != LDAP_SUCCESS) {
-rc = LDAP_OPERATIONS_ERROR;
-err_msg = Failed to handle the request.\n;
+if (ret == LDAP_NO_SUCH_OBJECT) {
+rc = LDAP_NO_SUCH_OBJECT;
+} else {
+rc = LDAP_OPERATIONS_ERROR;
+err_msg = Failed to handle the request.\n;
+}
goto done;
}
--
2.1.0
ACK.
--
/ Alexander Bokovoy
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 142] extdom: fix memory leak
On Wed, 04 Mar 2015, Sumit Bose wrote:
Hi,
while running 389ds with valgrind to see if my other patches introduced
a memory leak I found an older one which is fixed by this patch.
bye,
Sumit
From bb02cdc135fecc1766b17edd61554dbde9bccd0b Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Wed, 4 Mar 2015 17:53:08 +0100
Subject: [PATCH] extdom: fix memory leak
---
daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
index
a040f2beba073d856053429face2f464347b2524..708d0e4a2fc9da4f87a24a49c945587049f7280f
100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
@@ -156,6 +156,7 @@ done:
LOG(%s, err_msg);
}
slapi_send_ldap_result(pb, rc, NULL, err_msg, 0, NULL);
+ber_bvfree(ret_val);
free_req_data(req);
return SLAPI_PLUGIN_EXTENDED_SENT_RESULT;
}
I can see in 389-ds code that it actually tries to remove the value in
the end of extended operation handling:
slapi_pblock_set( pb, SLAPI_EXT_OP_REQ_OID, extoid );
slapi_pblock_set( pb, SLAPI_EXT_OP_REQ_VALUE, extval );
slapi_pblock_set( pb, SLAPI_REQUESTOR_ISROOT, pb-pb_op-o_isroot);
rc = plugin_call_exop_plugins( pb, extoid );
if ( SLAPI_PLUGIN_EXTENDED_SENT_RESULT != rc ) {
if ( SLAPI_PLUGIN_EXTENDED_NOT_HANDLED == rc ) {
lderr = LDAP_PROTOCOL_ERROR;/* no plugin handled the
op */
errmsg = unsupported extended operation;
} else {
errmsg = NULL;
lderr = rc;
}
send_ldap_result( pb, lderr, NULL, errmsg, 0, NULL );
}
free_and_return:
if (extoid)
slapi_ch_free((void **)extoid);
if (extval.bv_val)
slapi_ch_free((void **)extval.bv_val);
return;
--
/ Alexander Bokovoy
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 399-401] Allow multiple API instances
Dne 3.3.2015 v 16:11 Martin Kosek napsal(a):
On 03/03/2015 04:09 PM, Jan Cholasta wrote:
Dne 3.3.2015 v 16:04 Tomas Babej napsal(a):
On 03/03/2015 04:01 PM, Martin Kosek wrote:
On 03/03/2015 03:49 PM, Jan Cholasta wrote:
Hi,
the attached patches provide an attempt to fix
https://fedorahosted.org/freeipa/ticket/3090.
Patch 401 serves as an example and modifies ipa-advise to use its own
API
instance for Advice plugins.
Honza
Thanks. At least patches 399 and 400 look reasonable short for 4.2.
So with these patches, could we also get rid of
temporary_ldap2_connection we
have in ipa-replica-install? Petr3 may have other examples he met in
the past...
I think we can. Shall I prepare a patch?
If it is reasonable simple, I would go for it. It would be another selling
point for your patches.
Done.
--
Jan Cholasta
From 2d92312d314569b9d5acdfb45b1cae54421e62f9 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Wed, 4 Mar 2015 09:32:44 +
Subject: [PATCH 4/5] ldap2: Use self API instance instead of ipalib.api
---
ipaserver/plugins/ldap2.py | 45 -
1 file changed, 32 insertions(+), 13 deletions(-)
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index ec491e9..3211b33 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -63,21 +63,40 @@ class ldap2(LDAPClient, CrudBackend):
def __init__(self, shared_instance=True, ldap_uri=None, base_dn=None,
schema=None):
-try:
-ldap_uri = ldap_uri or api.env.ldap_uri
-except AttributeError:
-ldap_uri = 'ldap://example.com'
+self.__ldap_uri = None
CrudBackend.__init__(self, shared_instance=shared_instance)
LDAPClient.__init__(self, ldap_uri)
+self.__base_dn = base_dn
+
+@property
+def api(self):
+self_api = super(ldap2, self).api
+if self_api is None:
+self_api = api
+return self_api
+
+@property
+def ldap_uri(self):
+try:
+return self.__ldap_uri or self.api.env.ldap_uri
+except AttributeError:
+return 'ldap://example.com'
+
+@ldap_uri.setter
+def ldap_uri(self, value):
+self.__ldap_uri = value
+
+@property
+def base_dn(self):
try:
-if base_dn is not None:
-self.base_dn = DN(base_dn)
+if self.__base_dn is not None:
+return DN(self.__base_dn)
else:
-self.base_dn = DN(api.env.basedn)
+return DN(self.api.env.basedn)
except AttributeError:
-self.base_dn = DN()
+return DN()
def _init_connection(self):
# Connectible.conn is a proxy to thread-local storage;
@@ -124,11 +143,11 @@ class ldap2(LDAPClient, CrudBackend):
_ldap.set_option(_ldap.OPT_DEBUG_LEVEL, debug_level)
with self.error_handler():
-force_updates = api.env.context in ('installer', 'updates')
+force_updates = self.api.env.context in ('installer', 'updates')
conn = IPASimpleLDAPObject(
self.ldap_uri, force_schema_updates=force_updates)
if self.ldap_uri.startswith('ldapi://') and ccache:
-conn.set_option(_ldap.OPT_HOST_NAME, api.env.host)
+conn.set_option(_ldap.OPT_HOST_NAME, self.api.env.host)
minssf = conn.get_option(_ldap.OPT_X_SASL_SSF_MIN)
maxssf = conn.get_option(_ldap.OPT_X_SASL_SSF_MAX)
# Always connect with at least an SSF of 56, confidentiality
@@ -297,7 +316,7 @@ class ldap2(LDAPClient, CrudBackend):
def get_ipa_config(self, attrs_list=None):
Returns the IPA configuration entry (dn, entry_attrs).
-dn = api.Object.config.get_dn()
+dn = self.api.Object.config.get_dn()
assert isinstance(dn, DN)
try:
@@ -334,7 +353,7 @@ class ldap2(LDAPClient, CrudBackend):
upg_dn = DN(('cn', 'UPG Definition'), ('cn', 'Definitions'), ('cn', 'Managed Entries'),
-('cn', 'etc'), api.env.basedn)
+('cn', 'etc'), self.api.env.basedn)
try:
upg_entries = self.conn.search_s(upg_dn, _ldap.SCOPE_BASE,
@@ -359,7 +378,7 @@ class ldap2(LDAPClient, CrudBackend):
principal = getattr(context, 'principal')
entry = self.find_entry_by_attr(krbprincipalname, principal,
-krbPrincipalAux, base_dn=api.env.basedn)
+krbPrincipalAux, base_dn=self.api.env.basedn)
sctrl = [GetEffectiveRightsControl(True, dn: + str(entry.dn))]
self.conn.set_option(_ldap.OPT_SERVER_CONTROLS, sctrl)
try:
--
2.1.0
From 4982e5dfaab377eb438c7adda9314937a71931ca Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Wed, 4 Mar 2015 09:35:06 +
Subject: [PATCH 5/5] replica-install: Use different API instance
Re: [Freeipa-devel] [PATCHES 399-401] Allow multiple API instances
On 03/04/2015 11:13 AM, Jan Cholasta wrote: Dne 3.3.2015 v 16:11 Martin Kosek napsal(a): On 03/03/2015 04:09 PM, Jan Cholasta wrote: Dne 3.3.2015 v 16:04 Tomas Babej napsal(a): On 03/03/2015 04:01 PM, Martin Kosek wrote: On 03/03/2015 03:49 PM, Jan Cholasta wrote: Hi, the attached patches provide an attempt to fix https://fedorahosted.org/freeipa/ticket/3090. Patch 401 serves as an example and modifies ipa-advise to use its own API instance for Advice plugins. Honza Thanks. At least patches 399 and 400 look reasonable short for 4.2. So with these patches, could we also get rid of temporary_ldap2_connection we have in ipa-replica-install? Petr3 may have other examples he met in the past... I think we can. Shall I prepare a patch? If it is reasonable simple, I would go for it. It would be another selling point for your patches. Done. Thanks, this looks great! It proves the point with the separate API object. LGTM, I will let Tomas to continue with standard review then. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0316] Fix crash triggered by zone objects with unexpected DN
On 02/24/2015 03:01 PM, Petr Spacek wrote: Hello, On 18.2.2015 10:36, Tomas Hozza wrote: On 12/16/2014 04:32 PM, Petr Spacek wrote: Hello, Fix crash triggered by zone objects with unexpected DN. https://fedorahosted.org/bind-dyndb-ldap/ticket/148 NACK. The patch seems to make no difference when using the reproducer from ticket 148 18-Feb-2015 10:34:09.067 running 18-Feb-2015 10:34:09.139 ldap_helper.c:4876: INSIST(task == inst-task) failed, back trace 18-Feb-2015 10:34:09.139 #0 0x55587a80 in ?? 18-Feb-2015 10:34:09.139 #1 0x7620781a in ?? 18-Feb-2015 10:34:09.139 #2 0x720b00b2 in ?? 18-Feb-2015 10:34:09.140 #3 0x71e7ccf9 in ?? 18-Feb-2015 10:34:09.140 #4 0x71e7d992 in ?? 18-Feb-2015 10:34:09.140 #5 0x720a7f3b in ?? 18-Feb-2015 10:34:09.140 #6 0x75dda52a in ?? 18-Feb-2015 10:34:09.140 #7 0x7508d79d in ?? 18-Feb-2015 10:34:09.140 exiting (due to assertion failure) Program received signal SIGABRT, Aborted. [Switching to Thread 0x7fffea7cd700 (LWP 1719)] 0x74fc18c7 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:55 55 return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig); Missing separate debuginfos, use: debuginfo-install cyrus-sasl-gssapi-2.1.26-19.fc21.x86_64 cyrus-sasl-lib-2.1.26-19.fc21.x86_64 cyrus-sasl-md5-2.1.26-19.fc21.x86_64 cyrus-sasl-plain-2.1.26-19.fc21.x86_64 gssproxy-0.3.1-4.fc21.x86_64 keyutils-libs-1.5.9-4.fc21.x86_64 libattr-2.4.47-9.fc21.x86_64 libdb-5.3.28-9.fc21.x86_64 libgcc-4.9.2-6.fc21.x86_64 libselinux-2.3-5.fc21.x86_64 nspr-4.10.8-1.fc21.x86_64 nss-3.17.4-1.fc21.x86_64 nss-softokn-freebl-3.17.4-1.fc21.x86_64 nss-util-3.17.4-1.fc21.x86_64 pcre-8.35-8.fc21.x86_64 sssd-client-1.12.3-4.fc21.x86_64 xz-libs-5.1.2-14alpha.fc21.x86_64 (gdb) bt #0 0x74fc18c7 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:55 #1 0x74fc352a in __GI_abort () at abort.c:89 #2 0x55587c29 in assertion_failed (file=optimized out, line=optimized out, type=optimized out, cond=optimized out) at ./main.c:220 #3 0x7620781a in isc_assertion_failed (file=file@entry=0x720bad2a ldap_helper.c, line=line@entry=4876, type=type@entry=isc_assertiontype_insist, cond=cond@entry=0x720baf04 task == inst-task) at assertions.c:57 #4 0x720b00b2 in syncrepl_update (chgtype=1, entry=0x70125590, inst=0x77fa3160) at ldap_helper.c:4876 #5 ldap_sync_search_entry (ls=optimized out, msg=optimized out, entryUUID=optimized out, phase=LDAP_SYNC_CAPI_ADD) at ldap_helper.c:5031 #6 0x71e7ccf9 in ldap_sync_search_entry (ls=ls@entry=0x7fffe40008c0, res=0x7fffe4003870) at ldap_sync.c:228 #7 0x71e7d992 in ldap_sync_init (ls=0x7fffe40008c0, mode=mode@entry=3) at ldap_sync.c:792 #8 0x720a7f3b in ldap_syncrepl_watcher (arg=0x77fa3160) at ldap_helper.c:5247 #9 0x75dda52a in start_thread (arg=0x7fffea7cd700) at pthread_create.c:310 #10 0x7508d79d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109 Thank you for catching this! I was using slightly different test which triggered the new code but by using different code path. This new version should be more robust. Please re-test it, thank you! ACK for version 2. No crash during testing ;) Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0197-0198] Fix uniqueness plugins upgrade
On Wed, 25 Feb 2015, Martin Basti wrote: Modifications: * All plugins are migrated into new configuration style. * I left attribute uniqueness plugin disabled, cn=uid uniqueness,cn=plugins,cn=config is checking the same attribute. * POST_UPDATE plugin for uid removed, I moved it to update file. Is it okay Alexander? I haven't found reason why we need to do it in update plugin. Thierry, I touched configuration of plugins, which user lifecycle requires, can you take look if I it does not break anything? Patches attached. ACK. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0040 Add realm name to backup header file.
https://fedorahosted.org/freeipa/ticket/4896
--
David Kupka
From c295d33db32152118013d4e85493b012dd687347 Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Wed, 4 Mar 2015 06:49:54 -0500
Subject: [PATCH] Add realm name to backup header file.
When ipa server is restored after uninstalation there is no way to tell
what Kerberos realm should be used.
https://fedorahosted.org/freeipa/ticket/4896
---
ipaserver/install/ipa_backup.py | 1 +
ipaserver/install/ipa_restore.py | 15 +--
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/ipaserver/install/ipa_backup.py b/ipaserver/install/ipa_backup.py
index 72d1475d6db92b6b9e715afdae85d169a036c085..a0b6196b0d5cbc631c2665fd84ada101791b7254 100644
--- a/ipaserver/install/ipa_backup.py
+++ b/ipaserver/install/ipa_backup.py
@@ -507,6 +507,7 @@ class Backup(admintool.AdminTool):
config.set('ipa', 'host', api.env.host)
config.set('ipa', 'ipa_version', str(version.VERSION))
config.set('ipa', 'version', '1')
+config.set('ipa', 'realm', api.env.realm)
dn = DN(('cn', api.env.host), ('cn', 'masters'), ('cn', 'ipa'), ('cn', 'etc'), api.env.basedn)
services_cns = []
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index efe3b9b1c0c10775b3a72b9d843924263526209a..ee7c285c20c2cad632df858541b274fba4b1a84e 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -203,16 +203,10 @@ class Restore(admintool.AdminTool):
options = self.options
super(Restore, self).run()
-api.bootstrap(in_server=False, context='restore')
-api.finalize()
-
self.backup_dir = self.args[0]
if not os.path.isabs(self.backup_dir):
self.backup_dir = os.path.join(BACKUP_DIR, self.backup_dir)
-self.log.info(Preparing restore from %s on %s,
- self.backup_dir, api.env.host)
-
self.header = os.path.join(self.backup_dir, 'header')
try:
@@ -225,6 +219,14 @@ class Restore(admintool.AdminTool):
else:
restore_type = self.backup_type
+if restore_type == 'FULL' and self.backup_realm:
+api.env.realm = self.backup_realm
+api.bootstrap(in_server=False, context='restore')
+api.finalize()
+
+self.log.info(Preparing restore from %s on %s,
+ self.backup_dir, api.env.host)
+
instances = [realm_to_serverid(api.env.realm), 'PKI-IPA']
backends = ['userRoot', 'ipaca']
@@ -649,6 +651,7 @@ class Restore(admintool.AdminTool):
self.backup_ipa_version = config.get('ipa', 'ipa_version')
self.backup_version = config.get('ipa', 'version')
self.backup_services = config.get('ipa', 'services').split(',')
+self.backup_realm = config.get('ipa', 'realm')
def extract_backup(self, keyring=None):
--
2.1.0
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 134-136] extdom: handle ERANGE return code for getXXYYY_r()
On Mon, 02 Mar 2015, Sumit Bose wrote:
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
index
20fdd62b20f28f5384cf83b8be5819f721c6c3db..84aeb28066f25f05a89d0c2d42e8b060e2399501
100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
@@ -49,6 +49,220 @@
#define MAX(a,b) (((a)(b))?(a):(b))
#define SSSD_DOMAIN_SEPARATOR '@'
+#define MAX_BUF (1024*1024*1024)
+
+
+
+static int get_buffer(size_t *_buf_len, char **_buf)
+{
+long pw_max;
+long gr_max;
+size_t buf_len;
+char *buf;
+
+pw_max = sysconf(_SC_GETPW_R_SIZE_MAX);
+gr_max = sysconf(_SC_GETGR_R_SIZE_MAX);
+
+if (pw_max == -1 gr_max == -1) {
+buf_len = 16384;
+} else {
+buf_len = MAX(pw_max, gr_max);
+}
Here you'd get buf_len equal to 1024 by default on Linux which is too
low for our use case. I think it would be beneficial to add one more
MAX(buf_len, 16384):
-if (pw_max == -1 gr_max == -1) {
-buf_len = 16384;
-} else {
-buf_len = MAX(pw_max, gr_max);
-}
+buf_len = MAX(16384, MAX(pw_max, gr_max));
with MAX(MAX(),..) you also get rid of if() statement as resulting
rvalue would be guaranteed to be positive.
The rest is going along the common lines but would it be better to
allocate memory once per LDAP client request rather than always ask for
it per each NSS call? You can guarantee a sequential use of the buffer
within the LDAP client request processing so there is no problem with
locks but having this memory re-allocated on subsequent
getpwnam()/getpwuid()/... calls within the same request processing seems
suboptimal to me.
--
/ Alexander Bokovoy
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0006 Limit deadlocks between DS plugin DNA and slapi-nis
https://fedorahosted.org/freeipa/ticket/4927
From 9eba76c07f08a19d24a07b358fb6e6b64082f8e0 Mon Sep 17 00:00:00 2001
From: root r...@vm-035.idm.lab.eng.brq.redhat.com
Date: Wed, 4 Mar 2015 11:11:45 +0100
Subject: [PATCH] Limit deadlocks between DS plugin DNA and slapi-nis
Deadlock can occur if DNA plugin (shared) config and Schema-compat plugin config
are updated at the same time.
Schema-compat should ignore update on DNA config.
---
install/updates/10-schema_compat.update | 5 +
1 file changed, 5 insertions(+)
diff --git a/install/updates/10-schema_compat.update b/install/updates/10-schema_compat.update
index b8c79012d121116f9cf53908fbe4be9d3d82..4484bdcce15736eeaffcc99edc694094b16fd0ed 100644
--- a/install/updates/10-schema_compat.update
+++ b/install/updates/10-schema_compat.update
@@ -22,6 +22,7 @@ remove: schema-compat-ignore-subtree: cn=changelog
remove: schema-compat-ignore-subtree: o=ipaca
add: schema-compat-restrict-subtree: '$SUFFIX'
add: schema-compat-restrict-subtree: 'cn=Schema Compatibility,cn=plugins,cn=config'
+add: schema-compat-ignore-subtree: 'cn=dna,cn=ipa,cn=etc,$SUFFIX'
# Change padding for host and userCategory so the pad returns the same value
# as the original, '' or -.
@@ -31,6 +32,7 @@ remove: schema-compat-ignore-subtree: cn=changelog
remove: schema-compat-ignore-subtree: o=ipaca
add: schema-compat-restrict-subtree: '$SUFFIX'
add: schema-compat-restrict-subtree: 'cn=Schema Compatibility,cn=plugins,cn=config'
+add: schema-compat-ignore-subtree: 'cn=dna,cn=ipa,cn=etc,$SUFFIX'
dn: cn=computers, cn=Schema Compatibility, cn=plugins, cn=config
default:objectClass: top
@@ -49,6 +51,7 @@ remove: schema-compat-ignore-subtree: cn=changelog
remove: schema-compat-ignore-subtree: o=ipaca
add: schema-compat-restrict-subtree: '$SUFFIX'
add: schema-compat-restrict-subtree: 'cn=Schema Compatibility,cn=plugins,cn=config'
+add: schema-compat-ignore-subtree: 'cn=dna,cn=ipa,cn=etc,$SUFFIX'
dn: cn=sudoers,cn=Schema Compatibility,cn=plugins,cn=config
add:schema-compat-entry-attribute: sudoOrder=%{sudoOrder}
@@ -58,12 +61,14 @@ remove: schema-compat-ignore-subtree: cn=changelog
remove: schema-compat-ignore-subtree: o=ipaca
add: schema-compat-restrict-subtree: '$SUFFIX'
add: schema-compat-restrict-subtree: 'cn=Schema Compatibility,cn=plugins,cn=config'
+add: schema-compat-ignore-subtree: 'cn=dna,cn=ipa,cn=etc,$SUFFIX'
dn: cn=groups,cn=Schema Compatibility,cn=plugins,cn=config
remove: schema-compat-ignore-subtree: cn=changelog
remove: schema-compat-ignore-subtree: o=ipaca
add: schema-compat-restrict-subtree: '$SUFFIX'
add: schema-compat-restrict-subtree: 'cn=Schema Compatibility,cn=plugins,cn=config'
+add: schema-compat-ignore-subtree: 'cn=dna,cn=ipa,cn=etc,$SUFFIX'
dn: cn=Schema Compatibility,cn=plugins,cn=config
# We need to run schema-compat pre-bind callback before
--
2.1.0
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
