Re: [Freeipa-devel] [PATCH 0093] perform connectivity checks for all topology suffixes during node deletion

[Martin Babinsky]

On 11/12/2015 04:24 PM, Martin Babinsky wrote:

On 11/12/2015 02:04 PM, Petr Vobornik wrote:

On 11/10/2015 05:43 PM, Martin Babinsky wrote:

On 11/04/2015 06:50 PM, Petr Vobornik wrote:

On 11/04/2015 01:30 PM, Martin Babinsky wrote:

On 10/30/2015 05:06 PM, Martin Babinsky wrote:

On 10/30/2015 03:38 PM, Petr Vobornik wrote:

On 10/30/2015 03:26 PM, Martin Babinsky wrote:

patch for https://fedorahosted.org/freeipa/ticket/5309

The ticket itself is about connectivity checks in topology
suffixes,
but
there is a code (install/tools/ipa-replica-manage starting at line
788
after applying my patch) which monitors whether the segments
pointing
to/from the deleted host are already deleted.

These checks are currently hardcoded for 'realm' prefix, should we
generalize them as well or is it a part of other effort?



Could be separate patch but yes.

Ok I have included it in the attached patch so that both of these
operations are performed for all suffixes.




Hmm, I'm thinking whether the 'check_last_link_managed' and
'check_deleted_segments' should not be called per-suffix, but should
themselves check all suffixes available. This could make the fix for
https://fedorahosted.org/freeipa/ticket/5409 also easier.



Depends if the output is reusable. If so then why not.
check_last_link_managed basically adds text to several
get_topology_connection_errors calls.


Attaching updated patch.



I'm not sure about (pseudo code):

 topo_errors = ([], [])
 for each suffix:
 topo_errors[0].extend(orig_errors)
 topo_errors[1].extend(new_errors)
 return topo_errors

In check_deleted_segments wait_for_segment_removal is per-suffix check
but uses topo_errors which contains errors from both suffices. Topo
erros are used to relax the check if topology is disconnected but this
might relax it too much.

I would change the errors to per-suffix as well, e.g.:
   topo_errors = {}
   for each suffix:
   topo_errors[suffix_name] = (orig_errors, new_errors)
   return topo_errors

Otherwise it looks OK (not tested yet).


I didn't realize that. I have modified the patch accordingly.



Attaching updated patch with changed docstring of 
'check_last_link_managed()'


--
Martin^3 Babinsky
From 3d8c659effc0eb7f5c000ae98195be509072b99e Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Fri, 30 Oct 2015 13:59:03 +0100
Subject: [PATCH 1/3] check for disconnected topology and deleted agreements
 for all suffices

The code in ipa-replica-manage which checks for disconnected topology and
deleted agreements during node removal was generalized so that it now performs
these checks for all suffixes to which the node belongs.

https://fedorahosted.org/freeipa/ticket/5309
---
 install/tools/ipa-replica-manage | 251 ++-
 1 file changed, 168 insertions(+), 83 deletions(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 2de6fd7993be290fefa5c2c7d07733c39d457ed6..ebbdf5c3301675d39d957f54d954a440ad5cbbc2 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -570,46 +570,97 @@ def check_last_link(delrepl, realm, dirman_passwd, force):
 else:
 return None
 
-def check_last_link_managed(api, masters, hostname, force):
+
+def map_masters_to_suffices(masters, suffices):
+masters_to_suffix = {}
+suffix_name_to_root = {
+s['iparepltopoconfroot'][0]: s['cn'][0] for s in suffices
+}
+
+for master in masters:
+managed_suffices = master['iparepltopomanagedsuffix']
+for suffix in managed_suffices:
+suffix_name = suffix_name_to_root[suffix]
+try:
+masters_to_suffix[suffix_name].append(master)
+except KeyError:
+masters_to_suffix[suffix_name] = [master]
+
+return masters_to_suffix
+
+
+def check_hostname_in_masters(hostname, masters):
+master_cns = {m['cn'][0] for m in masters}
+return hostname in master_cns
+
+
+def check_last_link_managed(api, hostname, masters, force):
 """
 Check if 'hostname' is safe to delete.
 
-:returns: tuple with lists of current and future errors in topology
-  (current_errors, new_errors)
+:returns: a dictionary of topology errors across all suffices in the form
+  {: (,
+  )}
 """
+suffices = api.Command.topologysuffix_find(u'')['result']
+suffix_to_masters = map_masters_to_suffices(masters, suffices)
+topo_errors_by_suffix = {}
+
+for suffix in suffices:
+suffix_name = suffix['cn'][0]
+suffix_members = suffix_to_masters[suffix_name]
+print("Checking connectivity in topology suffix '{0}'".format(
+suffix_name))
+if not check_hostname_in_masters(hostname, suffix_members):
+print(
+"'{0}' is not a part of topology suffix '{1}'".format(
+hostname, suffix_name
+)
+

[Freeipa-devel] [PATCH] BUILD: provide check target in custom Makefiles

[Lukas Slebodnik]

ehlo,

The automake generated makefiles have already a target check.
We need to provide this target also to non-generated
Makefiles so we can recursively call make check from
top level Makefile.

LS
>From ba0e01bb8dd754b24580615decf54b718ba07b2e Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Fri, 13 Nov 2015 06:54:12 +
Subject: [PATCH 7/8] BUILD: provide check target in custom Makefiles

The automake generated makefiles have already a target check.
We need to provide this target also to non-generated
Makefiles so we can recursively call make check from
top level Makefile
---
 Makefile   | 5 +
 install/po/Makefile.in | 2 ++
 ipapython/Makefile | 2 ++
 3 files changed, 9 insertions(+)

diff --git a/Makefile b/Makefile
index 
3c81466d3728022c1d9cf5bb216990f14a59b7e5..d2c37f1597a011af2bd9ef1a4f7ce87ac59620a3
 100644
--- a/Makefile
+++ b/Makefile
@@ -74,6 +74,11 @@ client: client-autogen
done
cd ipaplatform && $(PYTHON) setup.py build
 
+check: bootstrap-autogen server tests
+   @for subdir in $(SUBDIRS); do \
+   (cd $$subdir && $(MAKE) check) || exit 1; \
+   done
+
 bootstrap-autogen: version-update client-autogen
@echo "Building IPA $(IPA_VERSION)"
cd asn1; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr 
--sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi
diff --git a/install/po/Makefile.in b/install/po/Makefile.in
index 
e0bf2e0a1f3db14d485c7d787ae95e433703856f..713fcb5aa9143d718049bf691e7c3bc9e102803f
 100644
--- a/install/po/Makefile.in
+++ b/install/po/Makefile.in
@@ -78,6 +78,8 @@ C_POTFILES = $(C_FILES) $(H_FILES)
 
 all:
 
+check:
+
 SUFFIXES = .po .mo
 
 .po.mo:
diff --git a/ipapython/Makefile b/ipapython/Makefile
index 
8527643232fd1b14fc246bd36678631baac088db..a865ca7580f5270eca4383508ced23e8537da669
 100644
--- a/ipapython/Makefile
+++ b/ipapython/Makefile
@@ -10,6 +10,8 @@ all:
(cd $$subdir && $(MAKE) $@) || exit 1; \
done
 
+check:
+
 .PHONY: install
 install:
if [ "$(DESTDIR)" = "" ]; then \
-- 
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

[Freeipa-devel] [PATCH] ipa_kdb_tests: Fix test with default krb5.conf

[Lukas Slebodnik]

ehlo,

ipa_kdb_tests test failed for me on minimal f23.

LS
>From ba5ecf13cac1b4822651987ba5db9ed16214cb8f Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Thu, 12 Nov 2015 19:22:56 +
Subject: [PATCH 1/8] ipa_kdb_tests: Fix test with default krb5.conf

Default krb5.conf needn't have defined default_realm.
Unit tests should not rely on existing default value.
---
 daemons/ipa-kdb/tests/ipa_kdb_tests.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/daemons/ipa-kdb/tests/ipa_kdb_tests.c 
b/daemons/ipa-kdb/tests/ipa_kdb_tests.c
index 
edd4ae0975628d6b3abe9bab2852c990c9a8c590..9f577529c7f8a22da2bb1a01898e804b6bc6d259
 100644
--- a/daemons/ipa-kdb/tests/ipa_kdb_tests.c
+++ b/daemons/ipa-kdb/tests/ipa_kdb_tests.c
@@ -82,6 +82,10 @@ void setup(void **state)
 
 kerr = krb5_init_context(_ctx);
 assert_int_equal(kerr, 0);
+
+kerr = krb5_set_default_realm(krb5_ctx, "EXAMPLE.COM");
+assert_int_equal(kerr, 0);
+
 kerr = krb5_db_setup_lib_handle(krb5_ctx);
 assert_int_equal(kerr, 0);
 
-- 
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

[Freeipa-devel] [PATCHES] Fix few gcc warnings

[Lukas Slebodnik]

ehlo,

Few simple patches are attached.

LS
>From e7320a93df1f20e6f29e0a067187e70b41c10ef8 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Thu, 12 Nov 2015 19:25:15 +
Subject: [PATCH 2/8] ipa_kdb_tests: Remove unused variables

---
 daemons/ipa-kdb/tests/ipa_kdb_tests.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/daemons/ipa-kdb/tests/ipa_kdb_tests.c 
b/daemons/ipa-kdb/tests/ipa_kdb_tests.c
index 
9f577529c7f8a22da2bb1a01898e804b6bc6d259..59e69d0c645026e5ce0884431b85486e599a3af0
 100644
--- a/daemons/ipa-kdb/tests/ipa_kdb_tests.c
+++ b/daemons/ipa-kdb/tests/ipa_kdb_tests.c
@@ -126,9 +126,6 @@ void setup(void **state)
 _ctx->mspac->trusts[0].sid_blacklist_incoming[0]);
 assert_int_equal(ret, 0);
 
-struct dom_sid *sid_blacklist_incoming;
-int len_sid_blacklist_incoming;
-
 ipa_ctx->kcontext = krb5_ctx;
 kerr = krb5_db_set_context(krb5_ctx, ipa_ctx);
 assert_int_equal(kerr, 0);
@@ -317,7 +314,6 @@ void test_get_authz_data_types(void **state)
 char *ad_none_and_pad[] = {"NONE", "PAD", NULL};
 char *ad_global_pac_nfs_none[] = {"MS-PAC", "nfs:NONE", NULL};
 char *ad_global_pac_nfs_pad[] = {"MS-PAC", "nfs:PAD", NULL};
-krb5_context krb5_ctx;
 krb5_error_code kerr;
 struct ipadb_context *ipa_ctx;
 krb5_principal nfs_princ;
-- 
2.5.0

>From 935e22dc78f20e732cca0895899acae8ee233e10 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Thu, 12 Nov 2015 19:32:06 +
Subject: [PATCH 3/8] ipa_kdb_tests: Fix warning Wmissing-braces
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

tests/ipa_kdb_tests.c:254:9: warning: missing braces around initializer 
[-Wmissing-braces]
 {3, {BLACKLIST_SID"-1000", BLACKLIST_SID"-1001", BLACKLIST_SID"-1002"},
 ^
tests/ipa_kdb_tests.c:254:9: note: (near initialization for ‘test_data[6]’)
tests/ipa_kdb_tests.c:256:9: warning: missing braces around initializer 
[-Wmissing-braces]
 {0, NULL, 0 , NULL}
 ^
tests/ipa_kdb_tests.c:256:9: note: (near initialization for ‘test_data[7]’)
tests/ipa_kdb_tests.c:234:21: warning: missing braces around initializer 
[-Wmissing-braces]
 } test_data[] = {
 ^
---
 daemons/ipa-kdb/tests/ipa_kdb_tests.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/daemons/ipa-kdb/tests/ipa_kdb_tests.c 
b/daemons/ipa-kdb/tests/ipa_kdb_tests.c
index 
59e69d0c645026e5ce0884431b85486e599a3af0..7483a1799d4c8462f212ff6e8e4140c1c053d6a7
 100644
--- a/daemons/ipa-kdb/tests/ipa_kdb_tests.c
+++ b/daemons/ipa-kdb/tests/ipa_kdb_tests.c
@@ -252,8 +252,8 @@ void test_filter_logon_info(void **state)
  1, {DOM_SID_TRUST"-1002"}},
 /* all SIDs filtered*/
 {3, {BLACKLIST_SID"-1000", BLACKLIST_SID"-1001", BLACKLIST_SID"-1002"},
- 0, NULL},
-{0, NULL, 0 , NULL}
+ 0, {}},
+{0, {}, 0 , {}}
 };
 
 for (c = 0; test_data[c].sidcount != 0; c++) {
-- 
2.5.0

>From 215fd247ff9a6136c17228bf5e7369cb3f478512 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Thu, 12 Nov 2015 19:49:16 +
Subject: [PATCH 4/8] topology: Fix warning Wshadow
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

topology_pre.c: In function ‘ipa_topo_pre_add’:
topology_pre.c:509:15: warning: declaration of ‘errtxt’ shadows a previous 
local [-Wshadow]
 char *errtxt;
   ^
topology_pre.c:494:11: note: shadowed declaration is here
 char *errtxt  = NULL;
   ^
---
 daemons/ipa-slapi-plugins/topology/topology_pre.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/daemons/ipa-slapi-plugins/topology/topology_pre.c 
b/daemons/ipa-slapi-plugins/topology/topology_pre.c
index 
c6c22be24e9ef4d48e9a62e59ef20d7fb049568f..1788c6d3e9d95543d905054d9d1f31c40dddc045
 100644
--- a/daemons/ipa-slapi-plugins/topology/topology_pre.c
+++ b/daemons/ipa-slapi-plugins/topology/topology_pre.c
@@ -506,7 +506,6 @@ int ipa_topo_pre_add(Slapi_PBlock *pb)
 
 if (ipa_topo_is_entry_managed(pb)) {
 int rc = LDAP_UNWILLING_TO_PERFORM;
-char *errtxt;
 errtxt = slapi_ch_smprintf("Entry is managed by topology plugin."
" Adding of entry not allowed.\n");
 slapi_pblock_set(pb, SLAPI_PB_RESULT_TEXT, errtxt);
-- 
2.5.0

>From 6d494c1c02914fd9c169b980b1e01d14061bb729 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Fri, 13 Nov 2015 06:51:59 +
Subject: [PATCH 5/8] ipa-extdom-extop: Fix warning Wformat
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In file included from ipa_extdom_extop.c:41:0:
ipa_extdom_extop.c: In function ‘ipa_extdom_init_ctx’:
ipa_extdom_extop.c:203:9: warning: format ‘%d’ expects argument of type ‘int’,
  but argument 4 has type ‘size_t {aka long unsigned 
int}’ 

[Freeipa-devel] [PATCH] cmocka_tests: Do not use deprecated cmocka interface

[Lukas Slebodnik]

ehlo,

The cmocka-1.0 introduced new interface for tests
which is not compatible with the old one.
And the old interface is deprecated which caused compiled warnings.

LS
>From 15c7610984f8561bceca2a729fc4cc7e81a1d2b1 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Thu, 12 Nov 2015 19:43:56 +
Subject: [PATCH 6/8] cmocka_tests: Do not use deprecated cmocka interface

The cmocka-1.0 introduced new interface for tests
which is not compatible with the old one.
And the old interface is deprecated which caused compiled warnings.
---
 daemons/ipa-kdb/tests/ipa_kdb_tests.c  | 23 ++--
 .../ipa-slapi-plugins/ipa-cldap/ipa_cldap_tests.c  |  6 ++--
 .../ipa-extdom-extop/ipa_extdom_cmocka_tests.c | 32 --
 3 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/daemons/ipa-kdb/tests/ipa_kdb_tests.c 
b/daemons/ipa-kdb/tests/ipa_kdb_tests.c
index 
7483a1799d4c8462f212ff6e8e4140c1c053d6a7..5bc89e23410de4f779db44a9f2cf50bf521ef403
 100644
--- a/daemons/ipa-kdb/tests/ipa_kdb_tests.c
+++ b/daemons/ipa-kdb/tests/ipa_kdb_tests.c
@@ -72,7 +72,7 @@ struct test_ctx {
 #define DOM_SID_TRUST "S-1-5-21-4-5-6"
 #define BLACKLIST_SID "S-1-5-1"
 
-void setup(void **state)
+static int setup(void **state)
 {
 int ret;
 krb5_context krb5_ctx;
@@ -136,9 +136,11 @@ void setup(void **state)
 test_ctx->krb5_ctx = krb5_ctx;
 
 *state = test_ctx;
+
+return 0;
 }
 
-void teardown(void **state)
+static int teardown(void **state)
 {
 struct test_ctx *test_ctx;
 struct ipadb_context *ipa_ctx;
@@ -153,6 +155,8 @@ void teardown(void **state)
 krb5_free_context(test_ctx->krb5_ctx);
 
 talloc_free(test_ctx);
+
+return 0;
 }
 
 extern krb5_error_code filter_logon_info(krb5_context context,
@@ -468,12 +472,15 @@ void test_dom_sid_string(void **state)
 
 int main(int argc, const char *argv[])
 {
-const UnitTest tests[] = {
-unit_test_setup_teardown(test_get_authz_data_types, setup, teardown),
-unit_test_setup_teardown(test_filter_logon_info, setup, teardown),
-unit_test(test_string_to_sid),
-unit_test_setup_teardown(test_dom_sid_string, setup, teardown),
+const struct CMUnitTest tests[] = {
+cmocka_unit_test_setup_teardown(test_get_authz_data_types,
+setup, teardown),
+cmocka_unit_test_setup_teardown(test_filter_logon_info,
+setup, teardown),
+cmocka_unit_test(test_string_to_sid),
+cmocka_unit_test_setup_teardown(test_dom_sid_string,
+setup, teardown),
 };
 
-return run_tests(tests);
+return cmocka_run_group_tests(tests, NULL, NULL);
 }
diff --git a/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_tests.c 
b/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_tests.c
index 
8f579cb0c051551471a55273c4bce97717200ffd..48b9636fa4c7cc8d545f6adeb4283bfdf6472c2a
 100644
--- a/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_tests.c
+++ b/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_tests.c
@@ -59,10 +59,10 @@ void test_make_netbios_name(void **state)
 int main(int argc, const char *argv[])
 {
 
-const UnitTest tests[] = {
-unit_test(test_make_netbios_name),
+const struct CMUnitTest tests[] = {
+cmocka_unit_test(test_make_netbios_name),
 };
 
-return run_tests(tests);
+return cmocka_run_group_tests(tests, NULL, NULL);
 }
 
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 
ec553fe62c27738f258defc267fe761c72157df0..526f903d2416e62ee5781909c234bd5ee2d89183
 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
@@ -258,7 +258,7 @@ struct test_data {
 struct ipa_extdom_ctx *ctx;
 };
 
-void extdom_req_setup(void **state)
+static int  extdom_req_setup(void **state)
 {
 struct test_data *test_data;
 
@@ -272,9 +272,11 @@ void extdom_req_setup(void **state)
 assert_non_null(test_data->req);
 
 *state = test_data;
+
+return 0;
 }
 
-void extdom_req_teardown(void **state)
+static int  extdom_req_teardown(void **state)
 {
 struct test_data *test_data;
 
@@ -283,6 +285,8 @@ void extdom_req_teardown(void **state)
 free_req_data(test_data->req);
 free(test_data->ctx);
 free(test_data);
+
+return 0;
 }
 
 void test_set_err_msg(void **state)
@@ -433,18 +437,18 @@ void test_decode(void **state)
 
 int main(int argc, const char *argv[])
 {
-const UnitTest tests[] = {
-unit_test(test_getpwnam_r_wrapper),
-unit_test(test_getpwuid_r_wrapper),
-unit_test(test_getgrnam_r_wrapper),
-unit_test(test_getgrgid_r_wrapper),
-unit_test(test_get_user_grouplist),
-unit_test_setup_teardown(test_set_err_msg,
- 

[Freeipa-devel] [PATCH 0345 - 0346] Fix DNS test on fedora 23

[Martin Basti]

Patches attached.
From 982954b2d4b4105d7fafb9b57aa8f5f974679785 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 12 Nov 2015 17:06:34 +0100
Subject: [PATCH 1/2] Tests: DNS replace 192.0.2.0/24 with 198.18.0.0/15 range

192.0.2.0/24 is IANA reserved address that should not be used. netaddr
module check implemented for this address and IPA reject this address as
invalid.
198.18.0.0/15 is IANA reserved address for benchmark testing purpose, so
we can safely use this network.

http://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml
---
 ipatests/test_xmlrpc/test_dns_plugin.py | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_dns_plugin.py b/ipatests/test_xmlrpc/test_dns_plugin.py
index 4d3117f17f5f0355ef3f2e05a851717cfd0249a7..3de44e472286034d35ffe98ba07d2623ebff3363 100644
--- a/ipatests/test_xmlrpc/test_dns_plugin.py
+++ b/ipatests/test_xmlrpc/test_dns_plugin.py
@@ -273,8 +273,9 @@ arec3 = u'172.16.250.123'
 fwd_ip = u'172.16.31.80'
 allowtransfer_tofwd = u'%s;' % fwd_ip
 
-allowquery_restricted_in = u'!192.0.2/24;any;'
-allowquery_restricted_out = u'!192.0.2.0/24;any;'
+# 198.18.0.0/15 testing range reserved by RFC2544
+allowquery_restricted_in = u'!198.18.2.0/24;any;'
+allowquery_restricted_out = u'!198.18.2.0/24;any;'
 
 idnzone1 = u'\u010d.test.'
 idnzone1_punycoded = u'xn--bea.test.'
@@ -2958,7 +2959,7 @@ class test_dns(Declarative):
 'idnssoaexpire': [fuzzy_digits],
 'idnssoaminimum': [fuzzy_digits],
 'idnsallowtransfer': [u'172.16.31.80;'],
-'idnsallowquery': [u'!192.0.2.0/24;any;'],
+'idnsallowquery': [allowquery_restricted_out],
 'mxrecord': [u'0 ns1.dnszone.test.'],
 'locrecord': [u"49 11 42.400 N 16 36 29.600 E 227.64 10.00 10.00 0.10"],
 },
@@ -2996,7 +2997,7 @@ class test_dns(Declarative):
 'idnssoaexpire': [fuzzy_digits],
 'idnssoaminimum': [fuzzy_digits],
 'idnsallowtransfer': [u'172.16.31.80;'],
-'idnsallowquery': [u'!192.0.2.0/24;any;'],
+'idnsallowquery': [allowquery_restricted_out],
 'mxrecord': [u'0 ns1.dnszone.test.'],
 'locrecord': [u"49 11 42.400 N 16 36 29.600 E 227.64 10.00 10.00 0.10"],
 },
-- 
2.4.3

From c9e4d580f0e31e231d78976d91c751ac9f8f532f Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 12 Nov 2015 17:17:13 +0100
Subject: [PATCH 2/2] Tests: DNS various exceptions can be raised in test

Test 'Try to add SRV record to zone %r both via parts and a raw value'
can raise various exceptions which are all valid. Due to internal
representation IPA may raise exception for any of target, port,
priority, weight part.

This commit handles all of them.
---
 ipatests/test_xmlrpc/test_dns_plugin.py | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_dns_plugin.py b/ipatests/test_xmlrpc/test_dns_plugin.py
index 3de44e472286034d35ffe98ba07d2623ebff3363..5f692528eb9a5ae0dc488c663ab43cc47ffd29b3 100644
--- a/ipatests/test_xmlrpc/test_dns_plugin.py
+++ b/ipatests/test_xmlrpc/test_dns_plugin.py
@@ -1148,9 +1148,11 @@ class test_dns(Declarative):
  'srv_part_target' : u'foo.bar.',
  'srvrecord': [u"1 100 1234 %s" \
  % zone1_ns]}),
-expected=errors.ValidationError(name='srv_target',
-error=u'Raw value of a DNS record was already set by ' +
-u'"srv_rec" option'),
+expected=lambda x, output: (
+type(x) == errors.ValidationError and
+x.message.endswith(u'Raw value of a DNS record was already '
+   u'set by "srv_rec" option'),
+),
 ),
 
 dict(
-- 
2.4.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 0338] Drop configure.jar file

[Petr Vobornik]

On 11/12/2015 03:31 PM, Martin Basti wrote:



1. following first line should be removed as well:
 preferences_filename = paths.PREFERENCES_HTML
-if ipautil.file_exists(preferences_filename):

Which also means that PREFERENCES_HTML won't be used and can be removed.


Aaa sorry my bad, I wanted to remove it from paths, I just somehow
forgot to do that.
Updated patch attached.


ACK
--
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] [PATCH] SPEC: Run cmocka based uni test in %check phase

[Lukas Slebodnik]

ehlo,

this patch depends on
freeipa-lslebodn-0007-BUILD-provide-check-target-in-custom-Makefile.patch

LS
>From 507b57b4a166b0490d4217a9603a67577bb36036 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Fri, 13 Nov 2015 07:11:38 +
Subject: [PATCH 8/8] SPEC: Run cmocka based uni test in %check phase

---
 freeipa.spec.in | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 
c3ca3413ffc3850b849a69adbbae8476355f3c76..fec2a83fcc83c02423601d88500e789c83f7a7d0
 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -405,6 +405,9 @@ make IPA_VERSION_IS_GIT_SNAPSHOT=no %{?_smp_mflags} all
 make IPA_VERSION_IS_GIT_SNAPSHOT=no %{?_smp_mflags} client
 %endif # ONLY_CLIENT
 
+%check
+make %{?_smp_mflags} check VERBOSE=yes
+
 %install
 rm -rf %{buildroot}
 export SUPPORTED_PLATFORM=%{platform_module}
-- 
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 0093] perform connectivity checks for all topology suffixes during node deletion

[Martin Basti]

On 13.11.2015 14:33, Petr Vobornik wrote:

On 11/13/2015 10:46 AM, Martin Babinsky wrote:


Otherwise it looks OK (not tested yet).


I didn't realize that. I have modified the patch accordingly.




Attaching updated patch with changed docstring of
'check_last_link_managed()'



ACK


Pushed to master: a6cdafd37451c99c1a7492ce5901195405881bb2

--
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] [Update]Time-Based Account Policies

[Martin Basti]

On 13.11.2015 14:41, Simo Sorce wrote:

On 11/11/15 09:30, Martin Basti wrote:



On 11.11.2015 14:52, Martin Basti wrote:

Comments inline
Martin^2

On 11.11.2015 09:24, Stanislav Laznicka wrote:

On 11/05/2015 06:17 PM, Petr Spacek wrote:

On 4.11.2015 15:20, Martin Basti wrote:


Hello,

we (Standa and I) had offline discussion and I proposed following 
idea:


1) create new entry in LDAP for "time rule" instead of adding the 
time rule

string directly into HBACRule.
This will allow to reuse time rules among various HBAC Rules (and 
maybe in

future with sudo rules, etc.)
HBACrule gets only reference to time rule entry stored in LDAP db.
Good idea! I can see time rule entry 'working hours in Brno 
office' which is

linked to relevant HBAC rules.

This seems like a good idea. However, it might be a bit messy to have
even the least significant rules stored in separate objects. But I
agree. It brings some questions, though.

Imo to have separate entry for time rule is cleaner than add it
directly to HBAC rule.


I really disagree, see below.


Where would be a good spot to store these time rules?

As I originally thought that we can share time rules between HBAC,
SUDO and everything else, I couldn't be wrong more.

Example: HBAC admin have permission to edit HBAC rule, but doesn't
have permission to edit SUDO rule. The HBAC admin should be able to
edit time rules for HBAC rules, and cannot be able to edit time rules
of SUDO rules. Thus time rules must be separated between HBAC, SUDO
and others, and privilege that give the permission to modify HBAC
rule, must give permission to modify only HBAC time rules.

I suggest to add HBAC time rules to HBAC container.

After IRC discussion with pspacek and jcholast:

We should just create separated privileges to time rules and allow them
to be shared.
So they should be stored in new container in LDAP


I do not understand what this means.

And in general I am opposed to have a separate object on performance 
grounds (for clients) and also on the fact that is becomes tricky to 
keep objects in sync.
What exactly is the performance issue there? To download extra entry 
from LDAP?

The SSSD do the same sync with users and groups, doesn't it?


We then have to deal with cases where you delete a time object but an 
HBAC still references it and also assuring you have permissions to 
fully change an HBAC rule, you may end up in situations where you can 
change the HBAC rule for everything but the times (or vice versa).
IMO this should solve referint plugin if the time policy is removed, 
then it will be removed from HBAC rules as well.




So please, explain carefully what would require a separate time object.

On privileges alone I see no value in a separate privilege for time 
than for the HBAC object it applies to (preference for using the same 
object). I also see no technical reason to store the time rules for 
completely different stuff in the same tree.
Yes, there may be the odd case in which you want to have the same time 
rule for a sudo rule and an HBAC rule, we can make that easy in the 
interface by providing a "copy time rules from X" kind of interface.
My original suggestion was to have it separated, HBAC time policy under 
HBAC container and sudo time policy under sudo container. So HBAC admin 
will have access to the same subtree and the one can modify time policy 
for HBAC. However pspacek and jcholast disagree I will let them to 
explain the reasons.


Martin^2


Simo.





--
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] [Update]Time-Based Account Policies

[Martin Basti]

On 13.11.2015 16:40, Simo Sorce wrote:

On 13/11/15 10:17, Martin Basti wrote:



On 13.11.2015 14:41, Simo Sorce wrote:

On 11/11/15 09:30, Martin Basti wrote:



On 11.11.2015 14:52, Martin Basti wrote:

Comments inline
Martin^2

On 11.11.2015 09:24, Stanislav Laznicka wrote:

On 11/05/2015 06:17 PM, Petr Spacek wrote:

On 4.11.2015 15:20, Martin Basti wrote:


Hello,

we (Standa and I) had offline discussion and I proposed following
idea:

1) create new entry in LDAP for "time rule" instead of adding the
time rule
string directly into HBACRule.
This will allow to reuse time rules among various HBAC Rules (and
maybe in
future with sudo rules, etc.)
HBACrule gets only reference to time rule entry stored in LDAP db.

Good idea! I can see time rule entry 'working hours in Brno
office' which is
linked to relevant HBAC rules.
This seems like a good idea. However, it might be a bit messy to 
have

even the least significant rules stored in separate objects. But I
agree. It brings some questions, though.

Imo to have separate entry for time rule is cleaner than add it
directly to HBAC rule.


I really disagree, see below.


Where would be a good spot to store these time rules?

As I originally thought that we can share time rules between HBAC,
SUDO and everything else, I couldn't be wrong more.

Example: HBAC admin have permission to edit HBAC rule, but doesn't
have permission to edit SUDO rule. The HBAC admin should be able to
edit time rules for HBAC rules, and cannot be able to edit time rules
of SUDO rules. Thus time rules must be separated between HBAC, SUDO
and others, and privilege that give the permission to modify HBAC
rule, must give permission to modify only HBAC time rules.

I suggest to add HBAC time rules to HBAC container.

After IRC discussion with pspacek and jcholast:

We should just create separated privileges to time rules and allow 
them

to be shared.
So they should be stored in new container in LDAP


I do not understand what this means.

And in general I am opposed to have a separate object on performance
grounds (for clients) and also on the fact that is becomes tricky to
keep objects in sync.

What exactly is the performance issue there? To download extra entry
from LDAP?


Yes because now you have to download rules, parse them, find out what 
needs tro be downloaded and pull it, or wore just download all time rules



The SSSD do the same sync with users and groups, doesn't it?


No, by default we do not enumerate users and groups, and for HBAC 
rules we download only those that apply to the machine.


All this exactly to reduce the amount of time taken, and load on the 
server.




We then have to deal with cases where you delete a time object but an
HBAC still references it and also assuring you have permissions to
fully change an HBAC rule, you may end up in situations where you can
change the HBAC rule for everything but the times (or vice versa).

IMO this should solve referint plugin if the time policy is removed,
then it will be removed from HBAC rules as well.


This is *not* ok though, you do not want an HBAC rule that limits 
users to a specific time to suddenly become "any time" because someone 
removed a time rule and didn't know it was referenced by multiple rules.
These are exactly the type of issues you end up having when you split 
the rule in multiple objects.




So please, explain carefully what would require a separate time object.

On privileges alone I see no value in a separate privilege for time
than for the HBAC object it applies to (preference for using the same
object). I also see no technical reason to store the time rules for
completely different stuff in the same tree.
Yes, there may be the odd case in which you want to have the same time
rule for a sudo rule and an HBAC rule, we can make that easy in the
interface by providing a "copy time rules from X" kind of interface.

My original suggestion was to have it separated, HBAC time policy under
HBAC container and sudo time policy under sudo container. So HBAC admin
will have access to the same subtree and the one can modify time policy
for HBAC. However pspacek and jcholast disagree I will let them to
explain the reasons.


Seriopusly, the HBAC time policy should be (on the HBAC rule object) 
and I mean it as an additional object class with the policy embedded 
in the specific HBAC rule. This is more efficient and avoid issues 
like the one above.


Can you explain *why* you propose to have separate objects ? What's 
the rationale ? What are the failure modes ?


The main purpose is to have time policies like "Work hours in Brno", 
"Work hours in Brisbane" ... which can be associated with many HBAC 
rules, instead of adding a separate time policy for each HBAC rule.


Martin

Simo.



--
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] [Update]Time-Based Account Policies

[Simo Sorce]

On 13/11/15 10:17, Martin Basti wrote:



On 13.11.2015 14:41, Simo Sorce wrote:

On 11/11/15 09:30, Martin Basti wrote:



On 11.11.2015 14:52, Martin Basti wrote:

Comments inline
Martin^2

On 11.11.2015 09:24, Stanislav Laznicka wrote:

On 11/05/2015 06:17 PM, Petr Spacek wrote:

On 4.11.2015 15:20, Martin Basti wrote:


Hello,

we (Standa and I) had offline discussion and I proposed following
idea:

1) create new entry in LDAP for "time rule" instead of adding the
time rule
string directly into HBACRule.
This will allow to reuse time rules among various HBAC Rules (and
maybe in
future with sudo rules, etc.)
HBACrule gets only reference to time rule entry stored in LDAP db.

Good idea! I can see time rule entry 'working hours in Brno
office' which is
linked to relevant HBAC rules.

This seems like a good idea. However, it might be a bit messy to have
even the least significant rules stored in separate objects. But I
agree. It brings some questions, though.

Imo to have separate entry for time rule is cleaner than add it
directly to HBAC rule.


I really disagree, see below.


Where would be a good spot to store these time rules?

As I originally thought that we can share time rules between HBAC,
SUDO and everything else, I couldn't be wrong more.

Example: HBAC admin have permission to edit HBAC rule, but doesn't
have permission to edit SUDO rule. The HBAC admin should be able to
edit time rules for HBAC rules, and cannot be able to edit time rules
of SUDO rules. Thus time rules must be separated between HBAC, SUDO
and others, and privilege that give the permission to modify HBAC
rule, must give permission to modify only HBAC time rules.

I suggest to add HBAC time rules to HBAC container.

After IRC discussion with pspacek and jcholast:

We should just create separated privileges to time rules and allow them
to be shared.
So they should be stored in new container in LDAP


I do not understand what this means.

And in general I am opposed to have a separate object on performance
grounds (for clients) and also on the fact that is becomes tricky to
keep objects in sync.

What exactly is the performance issue there? To download extra entry
from LDAP?


Yes because now you have to download rules, parse them, find out what 
needs tro be downloaded and pull it, or wore just download all time rules



The SSSD do the same sync with users and groups, doesn't it?


No, by default we do not enumerate users and groups, and for HBAC rules 
we download only those that apply to the machine.


All this exactly to reduce the amount of time taken, and load on the server.



We then have to deal with cases where you delete a time object but an
HBAC still references it and also assuring you have permissions to
fully change an HBAC rule, you may end up in situations where you can
change the HBAC rule for everything but the times (or vice versa).

IMO this should solve referint plugin if the time policy is removed,
then it will be removed from HBAC rules as well.


This is *not* ok though, you do not want an HBAC rule that limits users 
to a specific time to suddenly become "any time" because someone removed 
a time rule and didn't know it was referenced by multiple rules.
These are exactly the type of issues you end up having when you split 
the rule in multiple objects.




So please, explain carefully what would require a separate time object.

On privileges alone I see no value in a separate privilege for time
than for the HBAC object it applies to (preference for using the same
object). I also see no technical reason to store the time rules for
completely different stuff in the same tree.
Yes, there may be the odd case in which you want to have the same time
rule for a sudo rule and an HBAC rule, we can make that easy in the
interface by providing a "copy time rules from X" kind of interface.

My original suggestion was to have it separated, HBAC time policy under
HBAC container and sudo time policy under sudo container. So HBAC admin
will have access to the same subtree and the one can modify time policy
for HBAC. However pspacek and jcholast disagree I will let them to
explain the reasons.


Seriopusly, the HBAC time policy should be (on the HBAC rule object) and 
I mean it as an additional object class with the policy embedded in the 
specific HBAC rule. This is more efficient and avoid issues like the one 
above.


Can you explain *why* you propose to have separate objects ? What's the 
rationale ? What are the failure modes ?


Simo.

--
Simo Sorce * Red Hat, Inc * New York

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [Update]Time-Based Account Policies

[Simo Sorce]

On 13/11/15 11:51, Martin Basti wrote:



On 13.11.2015 16:40, Simo Sorce wrote:

On 13/11/15 10:17, Martin Basti wrote:



On 13.11.2015 14:41, Simo Sorce wrote:

On 11/11/15 09:30, Martin Basti wrote:



On 11.11.2015 14:52, Martin Basti wrote:

Comments inline
Martin^2

On 11.11.2015 09:24, Stanislav Laznicka wrote:

On 11/05/2015 06:17 PM, Petr Spacek wrote:

On 4.11.2015 15:20, Martin Basti wrote:


Hello,

we (Standa and I) had offline discussion and I proposed following
idea:

1) create new entry in LDAP for "time rule" instead of adding the
time rule
string directly into HBACRule.
This will allow to reuse time rules among various HBAC Rules (and
maybe in
future with sudo rules, etc.)
HBACrule gets only reference to time rule entry stored in LDAP db.

Good idea! I can see time rule entry 'working hours in Brno
office' which is
linked to relevant HBAC rules.

This seems like a good idea. However, it might be a bit messy to
have
even the least significant rules stored in separate objects. But I
agree. It brings some questions, though.

Imo to have separate entry for time rule is cleaner than add it
directly to HBAC rule.


I really disagree, see below.


Where would be a good spot to store these time rules?

As I originally thought that we can share time rules between HBAC,
SUDO and everything else, I couldn't be wrong more.

Example: HBAC admin have permission to edit HBAC rule, but doesn't
have permission to edit SUDO rule. The HBAC admin should be able to
edit time rules for HBAC rules, and cannot be able to edit time rules
of SUDO rules. Thus time rules must be separated between HBAC, SUDO
and others, and privilege that give the permission to modify HBAC
rule, must give permission to modify only HBAC time rules.

I suggest to add HBAC time rules to HBAC container.

After IRC discussion with pspacek and jcholast:

We should just create separated privileges to time rules and allow
them
to be shared.
So they should be stored in new container in LDAP


I do not understand what this means.

And in general I am opposed to have a separate object on performance
grounds (for clients) and also on the fact that is becomes tricky to
keep objects in sync.

What exactly is the performance issue there? To download extra entry
from LDAP?


Yes because now you have to download rules, parse them, find out what
needs tro be downloaded and pull it, or wore just download all time rules


The SSSD do the same sync with users and groups, doesn't it?


No, by default we do not enumerate users and groups, and for HBAC
rules we download only those that apply to the machine.

All this exactly to reduce the amount of time taken, and load on the
server.



We then have to deal with cases where you delete a time object but an
HBAC still references it and also assuring you have permissions to
fully change an HBAC rule, you may end up in situations where you can
change the HBAC rule for everything but the times (or vice versa).

IMO this should solve referint plugin if the time policy is removed,
then it will be removed from HBAC rules as well.


This is *not* ok though, you do not want an HBAC rule that limits
users to a specific time to suddenly become "any time" because someone
removed a time rule and didn't know it was referenced by multiple rules.
These are exactly the type of issues you end up having when you split
the rule in multiple objects.



So please, explain carefully what would require a separate time object.

On privileges alone I see no value in a separate privilege for time
than for the HBAC object it applies to (preference for using the same
object). I also see no technical reason to store the time rules for
completely different stuff in the same tree.
Yes, there may be the odd case in which you want to have the same time
rule for a sudo rule and an HBAC rule, we can make that easy in the
interface by providing a "copy time rules from X" kind of interface.

My original suggestion was to have it separated, HBAC time policy under
HBAC container and sudo time policy under sudo container. So HBAC admin
will have access to the same subtree and the one can modify time policy
for HBAC. However pspacek and jcholast disagree I will let them to
explain the reasons.


Seriopusly, the HBAC time policy should be (on the HBAC rule object)
and I mean it as an additional object class with the policy embedded
in the specific HBAC rule. This is more efficient and avoid issues
like the one above.

Can you explain *why* you propose to have separate objects ? What's
the rationale ? What are the failure modes ?


The main purpose is to have time policies like "Work hours in Brno",
"Work hours in Brisbane" ... which can be associated with many HBAC
rules, instead of adding a separate time policy for each HBAC rule.


This seem a convenience issue, it can be addressed by UI allowing to use 
templates which can be stored in some other tree for reference.


Are there any other advantages to keeping references instead of adding 
the 

Re: [Freeipa-devel] [PATCHES] Fix few gcc warnings

[Martin Basti]

On 13.11.2015 09:32, Lukas Slebodnik wrote:

ehlo,

Few simple patches are attached.

LS



ACK

Pushed to master: be6ecac220a8182ace0c8b8444cc2ec23bcff214

-- 
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] [Update]Time-Based Account Policies

[Jakub Hrozek]

On Fri, Nov 13, 2015 at 10:40:27AM -0500, Simo Sorce wrote:
> On 13/11/15 10:17, Martin Basti wrote:
> >
> >
> >On 13.11.2015 14:41, Simo Sorce wrote:
> >>On 11/11/15 09:30, Martin Basti wrote:
> >>>
> >>>
> >>>On 11.11.2015 14:52, Martin Basti wrote:
> Comments inline
> Martin^2
> 
> On 11.11.2015 09:24, Stanislav Laznicka wrote:
> >On 11/05/2015 06:17 PM, Petr Spacek wrote:
> >>On 4.11.2015 15:20, Martin Basti wrote:
> >>
> >>>Hello,
> >>>
> >>>we (Standa and I) had offline discussion and I proposed following
> >>>idea:
> >>>
> >>>1) create new entry in LDAP for "time rule" instead of adding the
> >>>time rule
> >>>string directly into HBACRule.
> >>>This will allow to reuse time rules among various HBAC Rules (and
> >>>maybe in
> >>>future with sudo rules, etc.)
> >>>HBACrule gets only reference to time rule entry stored in LDAP db.
> >>Good idea! I can see time rule entry 'working hours in Brno
> >>office' which is
> >>linked to relevant HBAC rules.
> >This seems like a good idea. However, it might be a bit messy to have
> >even the least significant rules stored in separate objects. But I
> >agree. It brings some questions, though.
> Imo to have separate entry for time rule is cleaner than add it
> directly to HBAC rule.
> >>
> >>I really disagree, see below.
> >>
> >Where would be a good spot to store these time rules?
> As I originally thought that we can share time rules between HBAC,
> SUDO and everything else, I couldn't be wrong more.
> 
> Example: HBAC admin have permission to edit HBAC rule, but doesn't
> have permission to edit SUDO rule. The HBAC admin should be able to
> edit time rules for HBAC rules, and cannot be able to edit time rules
> of SUDO rules. Thus time rules must be separated between HBAC, SUDO
> and others, and privilege that give the permission to modify HBAC
> rule, must give permission to modify only HBAC time rules.
> 
> I suggest to add HBAC time rules to HBAC container.
> >>>After IRC discussion with pspacek and jcholast:
> >>>
> >>>We should just create separated privileges to time rules and allow them
> >>>to be shared.
> >>>So they should be stored in new container in LDAP
> >>
> >>I do not understand what this means.
> >>
> >>And in general I am opposed to have a separate object on performance
> >>grounds (for clients) and also on the fact that is becomes tricky to
> >>keep objects in sync.
~~~
I think this is even more important than performance.

> >What exactly is the performance issue there? To download extra entry
> >from LDAP?
> 
> Yes because now you have to download rules, parse them, find out what needs
> tro be downloaded and pull it, or wore just download all time rules

Yes, if each rule referenced a timerule, then we would have to do
something like:
for rule in all_rules_for_this_host:
dereference_attribute(rule_time_attr)

We'd probably just end up fetching all timerules and establishing the
relationship locally (which takes up computing time on the client).

> 
> >The SSSD do the same sync with users and groups, doesn't it?
> 
> No, by default we do not enumerate users and groups, and for HBAC rules we
> download only those that apply to the machine.
> 
> All this exactly to reduce the amount of time taken, and load on the server.

Right, we try to reduce the number of round-trips, but also the number
of separate objects we save to the cache and we try to avoid complex
logic to link objects to one another.

tl;dr - I agree with Simo.

-- 
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 0347] Fix CI tests domain_level ENV config

[Martin Basti]

Patch attached.

Following test should pass:
ipa-run-tests test_integration/test_testconfig.py --verbose


From 60357332fc24c90e61bfad7b8d05bd8411d37fe3 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Fri, 13 Nov 2015 19:04:41 +0100
Subject: [PATCH] Fix CI tests domain_level env config

---
 ipatests/test_integration/env_config.py  | 2 ++
 ipatests/test_integration/test_testconfig.py | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/ipatests/test_integration/env_config.py b/ipatests/test_integration/env_config.py
index 96062bef30ec067faa644dc060af8531f5e899c9..3596b6fd45342eaa45b03419e1ea1eca70c17f33 100644
--- a/ipatests/test_integration/env_config.py
+++ b/ipatests/test_integration/env_config.py
@@ -32,6 +32,7 @@ import six
 
 from ipapython import ipautil
 from ipatests.test_integration.config import Config, Domain
+from ipalib.constants import MAX_DOMAIN_LEVEL
 
 TESTHOST_PREFIX = 'TESTHOST_'
 
@@ -61,6 +62,7 @@ _setting_infos = (
 
 _SettingInfo('ipv6', 'IPv6SETUP', False),
 _SettingInfo('debug', 'IPADEBUG', False),
+_SettingInfo('domain_level', 'DOMAINLVL', MAX_DOMAIN_LEVEL),
 )
 
 
diff --git a/ipatests/test_integration/test_testconfig.py b/ipatests/test_integration/test_testconfig.py
index 5c40522ed6bf0e4715e3b7ad160fbf7fbfdca9bc..3b08bf2173f31fb0bf8b4f6dc38f3508f5aa47f8 100644
--- a/ipatests/test_integration/test_testconfig.py
+++ b/ipatests/test_integration/test_testconfig.py
@@ -59,7 +59,7 @@ DEFAULT_OUTPUT_ENV = {
 "ADADMINPW": "Secret123",
 "IPv6SETUP": "",
 "IPADEBUG": "",
-"DOMAINLVL": MAX_DOMAIN_LEVEL
+"DOMAINLVL": str(MAX_DOMAIN_LEVEL),
 }
 
 DEFAULT_INPUT_ENV = {
-- 
2.4.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] SPEC: Run cmocka based uni test in %check phase

[Lukas Slebodnik]

On (13/11/15 09:37), Lukas Slebodnik wrote:
>ehlo,
>
>this patch depends on
>freeipa-lslebodn-0007-BUILD-provide-check-target-in-custom-Makefile.patch
>
>LS

>>From 507b57b4a166b0490d4217a9603a67577bb36036 Mon Sep 17 00:00:00 2001
>From: Lukas Slebodnik 
>Date: Fri, 13 Nov 2015 07:11:38 +
>Subject: [PATCH 8/8] SPEC: Run cmocka based uni test in %check phase
>
>---
> freeipa.spec.in | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/freeipa.spec.in b/freeipa.spec.in
>index 
>c3ca3413ffc3850b849a69adbbae8476355f3c76..fec2a83fcc83c02423601d88500e789c83f7a7d0
> 100644
>--- a/freeipa.spec.in
>+++ b/freeipa.spec.in
>@@ -405,6 +405,9 @@ make IPA_VERSION_IS_GIT_SNAPSHOT=no %{?_smp_mflags} all
> make IPA_VERSION_IS_GIT_SNAPSHOT=no %{?_smp_mflags} client
> %endif # ONLY_CLIENT
> 
>+%check
>+make %{?_smp_mflags} check VERBOSE=yes
>+

Tests were not executed in mock because there were missing build
dependencies. Updated patch is attached.

LS
>From d33f9c7994be5ab8964db297609b5267ed012295 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Fri, 13 Nov 2015 07:11:38 +
Subject: [PATCH 8/8] SPEC: Run cmocka based unit test in %check phase

This patch also consolidate build dependencies for
c based unit tests
---
 freeipa.spec.in | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index c3ca341..075b2ca 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -78,7 +78,6 @@ BuildRequires:  python-lxml
 BuildRequires:  python-pyasn1 >= 0.0.9a
 BuildRequires:  python-qrcode-core >= 5.0.0
 BuildRequires:  python-dns >= 1.11.1
-BuildRequires:  check
 BuildRequires:  libsss_idmap-devel
 BuildRequires:  libsss_nss_idmap-devel >= 1.12.2
 BuildRequires:  java-headless
@@ -99,6 +98,10 @@ BuildRequires:  python-six
 BuildRequires:  python-jwcrypto
 BuildRequires:  custodia
 
+# Build dependencies for unit tests
+BuildRequires:  libcmocka-devel
+BuildRequires:  nss_wrapper
+
 %description
 IPA is an integrated solution to provide centrally managed Identity (users,
 hosts, services), Authentication (SSO, 2FA), and Authorization
@@ -405,6 +408,9 @@ make IPA_VERSION_IS_GIT_SNAPSHOT=no %{?_smp_mflags} all
 make IPA_VERSION_IS_GIT_SNAPSHOT=no %{?_smp_mflags} client
 %endif # ONLY_CLIENT
 
+%check
+make %{?_smp_mflags} check VERBOSE=yes
+
 %install
 rm -rf %{buildroot}
 export SUPPORTED_PLATFORM=%{platform_module}
-- 
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 0345 - 0346] Fix DNS test on fedora 23

[Petr Spacek]

On 13.11.2015 10:05, Martin Basti wrote:
> Patches attached.

ACK, test_xmlrpc/test_dns_plugin.py passed when I managed to get though test
process and all failures in other modules.

-- 
Petr^2 Spacek

-- 
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 0345 - 0346] Fix DNS test on fedora 23

[Martin Basti]

On 13.11.2015 13:21, Petr Spacek wrote:

On 13.11.2015 10:05, Martin Basti wrote:

Patches attached.

ACK, test_xmlrpc/test_dns_plugin.py passed when I managed to get though test
process and all failures in other modules.


Pushed to master: b0faf30eac6b75267923fac59ef0728d763d29fe

--
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 0338] Drop configure.jar file

[Martin Basti]

On 13.11.2015 10:17, Petr Vobornik wrote:

On 11/12/2015 03:31 PM, Martin Basti wrote:



1. following first line should be removed as well:
 preferences_filename = paths.PREFERENCES_HTML
-if ipautil.file_exists(preferences_filename):

Which also means that PREFERENCES_HTML won't be used and can be 
removed.



Aaa sorry my bad, I wanted to remove it from paths, I just somehow
forgot to do that.
Updated patch attached.


ACK

Pushed to master: 19044e87ac54200b7710b8ec5405175c3d749e76

--
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] [Update]Time-Based Account Policies

[Simo Sorce]

On 11/11/15 09:30, Martin Basti wrote:



On 11.11.2015 14:52, Martin Basti wrote:

Comments inline
Martin^2

On 11.11.2015 09:24, Stanislav Laznicka wrote:

On 11/05/2015 06:17 PM, Petr Spacek wrote:

On 4.11.2015 15:20, Martin Basti wrote:


Hello,

we (Standa and I) had offline discussion and I proposed following idea:

1) create new entry in LDAP for "time rule" instead of adding the time rule
string directly into HBACRule.
This will allow to reuse time rules among various HBAC Rules (and maybe in
future with sudo rules, etc.)
HBACrule gets only reference to time rule entry stored in LDAP db.

Good idea! I can see time rule entry 'working hours in Brno office' which is
linked to relevant HBAC rules.

This seems like a good idea. However, it might be a bit messy to have
even the least significant rules stored in separate objects. But I
agree. It brings some questions, though.

Imo to have separate entry for time rule is cleaner than add it
directly to HBAC rule.


I really disagree, see below.


Where would be a good spot to store these time rules?

As I originally thought that we can share time rules between HBAC,
SUDO and everything else, I couldn't be wrong more.

Example: HBAC admin have permission to edit HBAC rule, but doesn't
have permission to edit SUDO rule. The HBAC admin should be able to
edit time rules for HBAC rules, and cannot be able to edit time rules
of SUDO rules. Thus time rules must be separated between HBAC, SUDO
and others, and privilege that give the permission to modify HBAC
rule, must give permission to modify only HBAC time rules.

I suggest to add HBAC time rules to HBAC container.

After IRC discussion with pspacek and jcholast:

We should just create separated privileges to time rules and allow them
to be shared.
So they should be stored in new container in LDAP


I do not understand what this means.

And in general I am opposed to have a separate object on performance 
grounds (for clients) and also on the fact that is becomes tricky to 
keep objects in sync.


We then have to deal with cases where you delete a time object but an 
HBAC still references it and also assuring you have permissions to fully 
change an HBAC rule, you may end up in situations where you can change 
the HBAC rule for everything but the times (or vice versa).


So please, explain carefully what would require a separate time object.

On privileges alone I see no value in a separate privilege for time than 
for the HBAC object it applies to (preference for using the same 
object). I also see no technical reason to store the time rules for 
completely different stuff in the same tree.
Yes, there may be the odd case in which you want to have the same time 
rule for a sudo rule and an HBAC rule, we can make that easy in the 
interface by providing a "copy time rules from X" kind of interface.


Simo.



--
Simo Sorce * Red Hat, Inc * New York

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0093] perform connectivity checks for all topology suffixes during node deletion

[Petr Vobornik]

On 11/13/2015 10:46 AM, Martin Babinsky wrote:


Otherwise it looks OK (not tested yet).


I didn't realize that. I have modified the patch accordingly.




Attaching updated patch with changed docstring of
'check_last_link_managed()'



ACK

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