[SSSD]Re: [PATCH] DATA_PROVIDER: BE_REQ as string in log message
On 08/18/2015 01:19 PM, Petr Cech wrote: On 08/05/2015 11:23 AM, Jakub Hrozek wrote: B) While writing a patch Lukas noticed another similar logging messages [sssd[pam]] [sss_dp_get_account_msg] (0x0400): Creating request for [LDAP][3][1][name=mof_user6] I investigated it. This is the same thing -- BE_REQ_*, but it is no longer in the provider, but in the responder. Can you please advise me where I could the function 'be_req2str' write? I think you should move it to separate file, as you don't want to share more code then necessary. There are 2 possibilities where to have this file a) in src/providers - responders already link with some modules from this folder, so I suppose it's viable Yes, this is a bit of hierarchy violation, but in the end we need to share the constants somehow. I think it's fine to keep the definite version in the providers/dp_* hierarchy, because that's where the interface is defined, the responder is a consumer. There is new patch attached. I think, that constants and const2str() functions should be in one place. I tried to suggest how we might share our constants. That's why I created a new header file in which we could move all the constants in the future. I am open to discussion. I look forward to your views. Petr Hello Petr, make dict check fails: /workspace/ci/label/rhel7/ci-build-debug/sssd-1.13.1/_inst/share/locale\ -g3 -O2 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -c ../src/providers/ldap/ldap_id.c -fPIC -DPIC -o src/providers/ldap/.libs/libsss_ldap_common_la-ldap_id.o ../src/providers/ldap/ldap_id.c:33:41: fatal error: providers/data_provider_req.h: No such file or directory #include providers/data_provider_req.h you can fix this by something like: Makefile.am @@ -584,6 +584,7 @@ dist_noinst_HEADERS = \ src/confdb/confdb_private.h \ src/confdb/confdb_setup.h \ src/providers/data_provider.h \ +src/providers/data_provider_req.h \ I think that data_provider_req.h should be included in data_provider.h, you could save a lot of changes in source files that require both of them.
[SSSD]Re: [PATCH] TESTS: ldap_id_cleanup timeouts
On 08/20/2015 01:50 PM, Petr Cech wrote: On 08/19/2015 08:26 PM, Michal Židek wrote: Hi! This is another patch to avoid failing tests in the CI (make-check-valgrind). This time the ldap_id_cleanup tests. Looks like the one second cache timeout was too short and the tests sometimes failed because they expected the entries to be still valid for a short while after they were added to sysdb. I saw the failures only on Fedora 20 CI machine. See the attached patch. Michal Hi, I just run your patch on my F22 VM and I see some trouble here... see attachment. Petr PS: I used clean GIT and your patch, nothing else. I know that this problem is another then you solved. But it is still issue. I just saw 2 more fails in the CI because of the short cache timeout. The problem you see, as you said as well, is a different one and I agree it should be solved as well but so far we were able to reproduce it on your computer only and I did not see fails in the CI because of that problem. I would suggest pushing this patch (if you ACK it that is) to fix CI and look at the problem you found later. Michal
Re: [SSSD] [PATCH] TESTS: Removing part of responder_cache_req-tests
On 08/21/2015 02:35 PM, Michal Židek wrote:
Hi,
some of the tests you deleted are valid and
should not be deleted.
Only those tests that rely on time(NULL)
being the same as the time of request
creation are invalid. All those that test old
entries or nonexistent entries are OK.
See comments inline.
I agree.
Those tests have another logic. So I returned them back.
Petr
From 63defe03797a8a9038e49400089a732bd35efaca Mon Sep 17 00:00:00 2001
From: Petr Cech pc...@redhat.com
Date: Fri, 21 Aug 2015 16:44:37 +0200
Subject: [PATCH] TESTS: Removing part of responder_cache_req-tests
If you call cache_req_[user|group]_by_filter_send() it than later calls
updated_[users|groups]_by_filter(), which adds filter that is called
recent. This filter causes that only [users|groups] added after the
request started are returned.
This patch removes tests which use
cache_req_[user|group]_by_filter_send(), because the logic of those
tests is corrupted. The tests create [users|groups] and after it, they
call cache_req_[user|group]_by_filter_send(). So it is obvious that it
is not in the right manner.
Possible fix is rewrite the tests to create the entries in the callback.
Resolves:
https://fedorahosted.org/sssd/ticket/2730
---
src/tests/cmocka/test_responder_cache_req.c | 211 ---
1 files changed, 0 insertions(+), 211 deletions(-)
diff --git a/src/tests/cmocka/test_responder_cache_req.c b/src/tests/cmocka/test_responder_cache_req.c
index 032fe429ac88b8cc9113976329ea04837f287276..bc6e2dc8f86a8fa8dc322da10fff4883f075ec7d 100644
--- a/src/tests/cmocka/test_responder_cache_req.c
+++ b/src/tests/cmocka/test_responder_cache_req.c
@@ -1710,54 +1710,6 @@ static void cache_req_user_by_filter_test_done(struct tevent_req *req)
ctx-tctx-done = true;
}
-void test_users_by_filter_valid(void **state)
-{
-struct cache_req_test_ctx *test_ctx = NULL;
-TALLOC_CTX *req_mem_ctx = NULL;
-struct tevent_req *req = NULL;
-const char *ldbname = NULL;
-errno_t ret;
-
-test_ctx = talloc_get_type_abort(*state, struct cache_req_test_ctx);
-test_ctx-create_user = true;
-
-ret = sysdb_store_user(test_ctx-tctx-dom, TEST_USER_NAME2, pwd, 1001, 1001,
- NULL, NULL, NULL, cn=TEST_USER_NAME2,dc=test, NULL,
- NULL, 1000, time(NULL));
-assert_int_equal(ret, EOK);
-
-req_mem_ctx = talloc_new(global_talloc_context);
-check_leaks_push(req_mem_ctx);
-
-/* Filters always go to DP */
-will_return(__wrap_sss_dp_get_account_send, test_ctx);
-mock_account_recv_simple();
-
-req = cache_req_user_by_filter_send(req_mem_ctx, test_ctx-tctx-ev,
-test_ctx-rctx,
-test_ctx-tctx-dom-name,
-test*);
-assert_non_null(req);
-tevent_req_set_callback(req, cache_req_user_by_filter_test_done, test_ctx);
-
-ret = test_ev_loop(test_ctx-tctx);
-assert_int_equal(ret, ERR_OK);
-assert_true(check_leaks_pop(req_mem_ctx));
-
-assert_non_null(test_ctx-result);
-assert_int_equal(test_ctx-result-count, 2);
-
-ldbname = ldb_msg_find_attr_as_string(test_ctx-result-msgs[0],
- SYSDB_NAME, NULL);
-assert_non_null(ldbname);
-assert_string_equal(ldbname, TEST_USER_NAME2);
-
-ldbname = ldb_msg_find_attr_as_string(test_ctx-result-msgs[1],
- SYSDB_NAME, NULL);
-assert_non_null(ldbname);
-assert_string_equal(ldbname, TEST_USER_NAME);
-}
-
void test_users_by_filter_filter_old(void **state)
{
struct cache_req_test_ctx *test_ctx = NULL;
@@ -1831,63 +1783,6 @@ void test_users_by_filter_notfound(void **state)
assert_true(check_leaks_pop(req_mem_ctx));
}
-static void test_users_by_filter_multiple_domains_valid(void **state)
-{
-struct cache_req_test_ctx *test_ctx = NULL;
-struct sss_domain_info *domain = NULL;
-TALLOC_CTX *req_mem_ctx = NULL;
-struct tevent_req *req = NULL;
-const char *ldbname = NULL;
-errno_t ret;
-
-test_ctx = talloc_get_type_abort(*state, struct cache_req_test_ctx);
-
-domain = find_domain_by_name(test_ctx-tctx-dom,
- responder_cache_req_test_d, true);
-assert_non_null(domain);
-
-ret = sysdb_store_user(domain, TEST_USER_NAME, pwd, 1000, 1000,
- NULL, NULL, NULL, cn=TEST_USER_NAME,dc=test, NULL,
- NULL, 1000, time(NULL));
-assert_int_equal(ret, EOK);
-
-ret = sysdb_store_user(domain, TEST_USER_NAME2, pwd, 1001, 1001,
- NULL, NULL, NULL, cn=TEST_USER_NAME2,dc=test, NULL,
- NULL, 1000, time(NULL));
-assert_int_equal(ret, EOK);
-
-req_mem_ctx = talloc_new(global_talloc_context);
-check_leaks_push(req_mem_ctx);
-
-/* Filters always go to DP */
-will_return(__wrap_sss_dp_get_account_send,
[SSSD]Re: [PATCH] TESTS: Removing part of responder_cache_req-tests
On 08/21/2015 02:18 PM, Petr Cech wrote:
Hi,
there is patch, which removes failing tests from responder_cache_req suite.
The reasons are mentioned in commit message.
There will be another patch, which will rewrite those tests.
Petr
Hi,
some of the tests you deleted are valid and
should not be deleted.
Only those tests that rely on time(NULL)
being the same as the time of request
creation are invalid. All those that test old
entries or nonexistent entries are OK.
See comments inline.
0001-TESTS-Removing-part-of-responder_cache_req-tests.patch
From 4518350e6dc7ddebad5b4c5d6ac2c560033e91ab Mon Sep 17 00:00:00 2001
From: Petr Cechpc...@redhat.com
Date: Fri, 21 Aug 2015 13:57:58 +0200
Subject: [PATCH] TESTS: Removing part of responder_cache_req-tests
If you call cache_req_[user|group]_by_filter_send() it than later calls
updated_[users|groups]_by_filter(), which adds filter that is called
recent. This filter causes that only [users|groups] added after the
request started are returned.
This patch removes tests which use
cache_req_[user|group]_by_filter_send(), because the logic of those
tests is corrupted. The tests create [users|groups] and after it, they
call cache_req_[user|group]_by_filter_send(). So it is obvious that it
is not in the right manner.
Possible fix is rewrite the tests to create the entries in the callback.
Resolves:
https://fedorahosted.org/sssd/ticket/2730
---
src/tests/cmocka/test_responder_cache_req.c | 383 ---
1 files changed, 0 insertions(+), 383 deletions(-)
diff --git a/src/tests/cmocka/test_responder_cache_req.c
b/src/tests/cmocka/test_responder_cache_req.c
index
032fe429ac88b8cc9113976329ea04837f287276..a6fe6737513f9436b1cd4e8f0cd9e6a3867f5105
100644
--- a/src/tests/cmocka/test_responder_cache_req.c
+++ b/src/tests/cmocka/test_responder_cache_req.c
@@ -1710,217 +1710,6 @@ static void cache_req_user_by_filter_test_done(struct
tevent_req *req)
ctx-tctx-done = true;
}
-void test_users_by_filter_valid(void **state)
-{
-struct cache_req_test_ctx *test_ctx = NULL;
-TALLOC_CTX *req_mem_ctx = NULL;
-struct tevent_req *req = NULL;
-const char *ldbname = NULL;
-errno_t ret;
-
-test_ctx = talloc_get_type_abort(*state, struct cache_req_test_ctx);
-test_ctx-create_user = true;
-
-ret = sysdb_store_user(test_ctx-tctx-dom, TEST_USER_NAME2, pwd, 1001,
1001,
- NULL, NULL, NULL, cn=TEST_USER_NAME2,dc=test,
NULL,
- NULL, 1000, time(NULL));
-assert_int_equal(ret, EOK);
-
-req_mem_ctx = talloc_new(global_talloc_context);
-check_leaks_push(req_mem_ctx);
-
-/* Filters always go to DP */
-will_return(__wrap_sss_dp_get_account_send, test_ctx);
-mock_account_recv_simple();
-
-req = cache_req_user_by_filter_send(req_mem_ctx, test_ctx-tctx-ev,
-test_ctx-rctx,
-test_ctx-tctx-dom-name,
-test*);
-assert_non_null(req);
-tevent_req_set_callback(req, cache_req_user_by_filter_test_done, test_ctx);
-
-ret = test_ev_loop(test_ctx-tctx);
-assert_int_equal(ret, ERR_OK);
-assert_true(check_leaks_pop(req_mem_ctx));
-
-assert_non_null(test_ctx-result);
-assert_int_equal(test_ctx-result-count, 2);
-
-ldbname = ldb_msg_find_attr_as_string(test_ctx-result-msgs[0],
- SYSDB_NAME, NULL);
-assert_non_null(ldbname);
-assert_string_equal(ldbname, TEST_USER_NAME2);
-
-ldbname = ldb_msg_find_attr_as_string(test_ctx-result-msgs[1],
- SYSDB_NAME, NULL);
-assert_non_null(ldbname);
-assert_string_equal(ldbname, TEST_USER_NAME);
-}
-
-void test_users_by_filter_filter_old(void **state)
This test seems valid to me.
-{
-struct cache_req_test_ctx *test_ctx = NULL;
-TALLOC_CTX *req_mem_ctx = NULL;
-struct tevent_req *req = NULL;
-const char *ldbname = NULL;
-errno_t ret;
-
-test_ctx = talloc_get_type_abort(*state, struct cache_req_test_ctx);
-test_ctx-create_user = true;
-
-/* This user was updated in distant past, so it wont't be reported by
- * the filter search */
-ret = sysdb_store_user(test_ctx-tctx-dom, TEST_USER_NAME2, pwd, 1001,
1001,
- NULL, NULL, NULL, cn=TEST_USER_NAME2,dc=test,
NULL,
- NULL, 1000, 1);
-assert_int_equal(ret, EOK);
-
-req_mem_ctx = talloc_new(global_talloc_context);
-check_leaks_push(req_mem_ctx);
-
-/* Filters always go to DP */
-will_return(__wrap_sss_dp_get_account_send, test_ctx);
-mock_account_recv_simple();
-
-req = cache_req_user_by_filter_send(req_mem_ctx, test_ctx-tctx-ev,
-test_ctx-rctx,
-test_ctx-tctx-dom-name,
-test*);
-assert_non_null(req);
Re: [SSSD] [PATCH] sssd: incorrect checks on length values during packet, decoding
On 07/23/2015 02:44 PM, Michal Židek wrote: Hi, see the attached patch for ticket https://fedorahosted.org/sssd/ticket/1697 I think this is a candidate to include in our coding guidelines. I agree. It is a candidate. Thanks, Michal ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel I build it, tests are OK and CI is here: http://sssd-ci.duckdns.org/logs/commit/4f/9768ec28a6327f4c865d4e7a5c547681f9a8af/2370/summary.html (The failure is not connected to this patch.) ACK Petr ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
[SSSD]Re: Re: [PATCH] DATA_PROVIDER: BE_REQ as string in log message
On 08/21/2015 01:08 PM, Pavel Reichl wrote: Hello Petr, make dict check fails: /workspace/ci/label/rhel7/ci-build-debug/sssd-1.13.1/_inst/share/locale\ -g3 -O2 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -c ../src/providers/ldap/ldap_id.c -fPIC -DPIC -o src/providers/ldap/.libs/libsss_ldap_common_la-ldap_id.o ../src/providers/ldap/ldap_id.c:33:41: fatal error: providers/data_provider_req.h: No such file or directory #include providers/data_provider_req.h you can fix this by something like: Makefile.am @@ -584,6 +584,7 @@ dist_noinst_HEADERS = \ src/confdb/confdb_private.h \ src/confdb/confdb_setup.h \ src/providers/data_provider.h \ +src/providers/data_provider_req.h \ I think that data_provider_req.h should be included in data_provider.h, you could save a lot of changes in source files that require both of them. Thanks. There is repaired patch attached. Petr From 7f154378f56a01ca65bfeba9985c605214d628b8 Mon Sep 17 00:00:00 2001 From: Petr Cech pc...@redhat.com Date: Tue, 18 Aug 2015 06:59:31 -0400 Subject: [PATCH] DATA_PROVIDER: BE_REQ as string in log message Add be_req2str() for translation BE_REQ to string. So we will have || Got request for [0x1001][FAST BE_REQ_USER][1][name=celestian] instead of || Got request for [0x1001][1][name=celestian] Function be_req2str() is used in data provider and in responder too. So this patch create new header file data_provider_req.h which delivers function be_req2str() and definitions of BE_REQ_*. Resolves: https://fedorahosted.org/sssd/ticket/2708 --- Makefile.am | 6 +++- src/providers/data_provider.h | 17 +- src/providers/data_provider_be.c| 3 +- src/providers/data_provider_req.c | 68 + src/providers/data_provider_req.h | 51 src/responder/common/responder_dp.c | 4 +-- 6 files changed, 129 insertions(+), 20 deletions(-) create mode 100644 src/providers/data_provider_req.c create mode 100644 src/providers/data_provider_req.h diff --git a/Makefile.am b/Makefile.am index f153ab0adf390880672a1681b386ea26426465cb..94920b29d7aab44085e401f8ada8555ab69fed6a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -446,7 +446,8 @@ SSSD_RESPONDER_OBJ = \ src/monitor/monitor_iface_generated.c \ src/monitor/monitor_iface_generated.h \ src/providers/data_provider_iface_generated.c \ -src/providers/data_provider_iface_generated.h +src/providers/data_provider_iface_generated.h \ +src/providers/data_provider_req.c SSSD_TOOLS_OBJ = \ src/tools/sss_sync_ops.c \ @@ -583,6 +584,7 @@ dist_noinst_HEADERS = \ src/confdb/confdb_private.h \ src/confdb/confdb_setup.h \ src/providers/data_provider.h \ +src/providers/data_provider_req.h \ src/providers/dp_backend.h \ src/providers/dp_dyndns.h \ src/providers/dp_ptask_private.h \ @@ -1193,6 +1195,7 @@ endif sssd_be_SOURCES = \ src/providers/data_provider_be.c \ +src/providers/data_provider_req.c \ src/providers/data_provider_fo.c \ src/providers/data_provider_opts.c \ src/providers/data_provider_callbacks.c \ @@ -1646,6 +1649,7 @@ simple_access_tests_SOURCES = \ src/providers/simple/simple_access.c \ src/providers/simple/simple_access_check.c \ src/providers/data_provider_be.c \ +src/providers/data_provider_req.c \ src/providers/data_provider_fo.c \ src/providers/data_provider_opts.c \ src/providers/data_provider_callbacks.c \ diff --git a/src/providers/data_provider.h b/src/providers/data_provider.h index 510c63ce41c99314ec8fcf11fffb2e66082e8951..39051b90c3aad96f62dcbb86a20bcfd8c954879b 100644 --- a/src/providers/data_provider.h +++ b/src/providers/data_provider.h @@ -43,6 +43,7 @@ #include sbus/sbus_client.h #include sss_client/sss_cli.h #include util/authtok.h +#include providers/data_provider_req.h #include providers/data_provider_iface_generated.h #define DATA_PROVIDER_VERSION 0x0001 @@ -131,22 +132,6 @@ #define BE_FILTER_CERT 6 #define BE_FILTER_WILDCARD 7 -#define BE_REQ_USER 0x0001 -#define BE_REQ_GROUP 0x0002 -#define BE_REQ_INITGROUPS0x0003 -#define BE_REQ_NETGROUP 0x0004 -#define BE_REQ_SERVICES 0x0005 -#define BE_REQ_SUDO_FULL 0x0006 -#define BE_REQ_SUDO_RULES0x0007 -#define BE_REQ_AUTOFS0x0009 -#define BE_REQ_HOST 0x0010 -#define BE_REQ_BY_SECID 0x0011 -#define BE_REQ_USER_AND_GROUP 0x0012 -#define BE_REQ_BY_UUID 0x0013 -#define BE_REQ_BY_CERT 0x0014 -#define BE_REQ_TYPE_MASK 0x00FF -#define BE_REQ_FAST 0x1000 - #define DP_SEC_ID secid #define DP_CERT cert /* sizeof() counts the trailing \0 so we must substract 1 for the string diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index
[SSSD]Re: More upstream CI tests
On 08/20/2015 12:38 AM, Jakub Hrozek wrote: as we're stabilizing the 1.13 branch and before we plan what we want to work on during the 1.14 development, we should use that time to write some more tests! Here are some areas where we could add tests. Please discuss or add your ideas, I would like to turn this list into tickets we can start implementing: * Extend the LDAP provider tests with more dynamic test cases. - add a user to a group, run sss_cache, assert id user displays the new group and getent group displays the new member - conversely with removing users from groups There are some more basic tests which can be implemented, on my original design page: https://fedorahosted.org/sssd/wiki/DesignDocs/CwrapLDAP I can help with this. * Background refresh - could be built atop the LDAP NSS tests as well. I think we have all the infrastructure in place. Caching/refreshing tests would be a very useful thing, I agree. We'd have to be careful with timing, though, so they're robust enough under load on our CI setup. * Local overrides integration test: - this could be relatively easy, just call the overrides tool and request the entry. Could be built atop the existing LDAP tests or even use the local provider. I can help with this. * Add a KDC - until we have a pam_wrapper, this would only be useful to test ldap_child, but adding the KDC instantiation might be worth it nonetheless - there is a protorype of KDC instantiation on the list for some time now, since we enabled rootless SSSD I can try helping with this. * IFP - could we reuse the existing sbus tests to spawn a custom bus? * SUDO - can we trick sudo into connecting to our test sssd instance? This is interesting. Wouldn't we have to have some sort of nss_wrapper here too, as we can't write to /etc/nsswitch.conf? Would sudo be fine with uid_wrapper? I wrote a ton of sudo tests in QE, so I can at least help with the test plan. Overall, I think I will be able to spare about two weeks after our autumn get-together to help with writing integration tests, maybe more later. Otherwise I'll need to work on session recording before that, so I have something to demo and talk about. I think the order of priority is roughly as above. I think the LDAP provider is critical enough to be well tested. The refresh and local override tests might be nice to have because we would be refactoring the NSS responder in 1.14, so we should have it tested. I'll be happy to hear other opionions, though! Thanks for bringing this up Jakub! Personally, I'm always glad to see more integration tests upstream. Nick
Re: [SSSD] [PATCH] DATA_PROVIDER: BE_REQ as string in log message
On 08/21/2015 03:40 PM, Petr Cech wrote: On 08/21/2015 01:08 PM, Pavel Reichl wrote: Hello Petr, make dict check fails: /workspace/ci/label/rhel7/ci-build-debug/sssd-1.13.1/_inst/share/locale\ -g3 -O2 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -c ../src/providers/ldap/ldap_id.c -fPIC -DPIC -o src/providers/ldap/.libs/libsss_ldap_common_la-ldap_id.o ../src/providers/ldap/ldap_id.c:33:41: fatal error: providers/data_provider_req.h: No such file or directory #include providers/data_provider_req.h you can fix this by something like: Makefile.am @@ -584,6 +584,7 @@ dist_noinst_HEADERS = \ src/confdb/confdb_private.h \ src/confdb/confdb_setup.h \ src/providers/data_provider.h \ +src/providers/data_provider_req.h \ I think that data_provider_req.h should be included in data_provider.h, you could save a lot of changes in source files that require both of them. Thanks. There is repaired patch attached. Petr Petr can you change data_provider_req.c to include providers/data_provider_req.h instead of providers/data_provider.h ? I originally thought that you will be able to include solely data_provider_req.h from responder_dp.c but I see that data_provider.h is required. But I still don't mind introducing data_provider_req.h. If you change this little nitpick I think I can ACK the patch. ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
[SSSD] Embedding Lua into SSSD
Hi everyone, I might be in a strange and careless mood today, but here is something I wanted to suggest since the time I saw the amount of logic that goes into SSSD and is implemented in C. What if we implement some of the complicated logic inside SSSD in Lua [1]? Lua is a language specifically designed to be embedded. It grew into popularity after it was embedded into World Of Warcraft. It was also embedded and is still being embedded into a bunch of other games. However, games is far from the only application for Lua. Lua is a simple and lightweight language, yet with lots of flexibility - grasping the essence of the whole reference manual [2] takes one evening and it's an entertaining read. The VM is small and fast. In fact some people use it on microcontrollers [3]. I personally embedded it into two C-based projects, one of which was already quite big, and another was a rewrite of a smallish, but complicated C project. In both cases, where before things were tedious and hard, they have become easy and fun. The first project was on an embedded platform. I also had some other, but smaller stuff done with it. Generally, Lua gets put into the middle with outside interfaces and performance-critical parts implemented in C. With a very simple interface to and from C one can put boundaries wherever necessary. Yes, the interface is simpler than Python's. The process is this: you implement some C libraries (shared/static) which are loadable from Lua (Lua-C interface) and then create a VM or several, load some Lua code and call into it (C-Lua interface). Some of the benefits are: Clearer logic (higher level language) Much less memory leaks and very little memory management (mostly in Lua-C interface). Faster development cycle (no compilation, edit and retry on a live daemon) Potentially easy plugin interface (yay for admin scripts!) Some of the drawbacks are: More difficult to do low level stuff (so we just don't do that) Lower performance than C (still good enough for games, and we can have the critical parts in C easily) Needs learning and development investment. Regarding the development investment, due to low cost of C/Lua interface implementation, it is practical to replace the relevant C code piece-by-piece, provided pieces are not too small (say, 2KLOC and up). Lastly, don't take this too seriously. I realise that this is disruptive and hard. However if you ever feel like SSSD has grown too big and complex, ping me, or just try Lua yourself :) Nick [1] http://www.lua.org/ [2] http://www.lua.org/manual/5.3/manual.html [3] http://www.eluaproject.net/ ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Switch ldap_user_certificate default to userCertificate; binary
On Fri, 2015-08-21 at 09:50 +0200, Jan Cholasta wrote: Hi, On 10.8.2015 12:59, Jakub Hrozek wrote: Hi, the attached patches fix #2742. The first one makes sure we can print the certificate (or any binary attribute, really) safely. We only need to make sure to escape the attribute values before saving them to sysdb, because then ldb guarantees terminating them. The second just switches the attribute value. I tested using this howto: http://www.freeipa.org/page/V4/User_Certificates#How_to_Test You'll also want to use a recent enough IPA version, one that fixes: https://fedorahosted.org/freeipa/ticket/5173 Then, on the client, call: dbus-send --print-reply \ --system \ --dest=org.freedesktop.sssd.infopipe \ /org/freedesktop/sssd/infopipe/Users \ org.freedesktop.sssd.infopipe.Users.FindByCertificate \ string:$( openssl x509 cert.pem ) The result will be an object path. LGTM, but I would think userCertificate;binary should be the default everywhere, i.e. generic LDAP, as that is the correct attribute name according to RFC 4523. IMHO when someone uses the standard name in generic LDAP, they should not be forced to change SSSD configuration because of it. +1 Simo -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] DATA_PROVIDER: BE_REQ as string in log message
On 08/21/2015 05:10 PM, Pavel Reichl wrote: Petr can you change data_provider_req.c to include providers/data_provider_req.h instead of providers/data_provider.h ? I originally thought that you will be able to include solely data_provider_req.h from responder_dp.c but I see that data_provider.h is required. But I still don't mind introducing data_provider_req.h. If you change this little nitpick I think I can ACK the patch. Pavel, you're right, that's mine main opinion to this issue. I am sorry, I need more focus and coffe. There is fixed (not repaired) patch. Petr From aebda5def026d7a0fc40c4034ef18ba97ada5f36 Mon Sep 17 00:00:00 2001 From: Petr Cech pc...@redhat.com Date: Tue, 18 Aug 2015 06:59:31 -0400 Subject: [PATCH] DATA_PROVIDER: BE_REQ as string in log message Add be_req2str() for translation BE_REQ to string. So we will have || Got request for [0x1001][FAST BE_REQ_USER][1][name=celestian] instead of || Got request for [0x1001][1][name=celestian] Function be_req2str() is used in data provider and in responder too. So this patch create new header file data_provider_req.h which delivers function be_req2str() and definitions of BE_REQ_*. Resolves: https://fedorahosted.org/sssd/ticket/2708 --- Makefile.am | 6 +++- src/providers/data_provider.h | 17 +- src/providers/data_provider_be.c| 3 +- src/providers/data_provider_req.c | 68 + src/providers/data_provider_req.h | 51 src/responder/common/responder_dp.c | 4 +-- 6 files changed, 129 insertions(+), 20 deletions(-) create mode 100644 src/providers/data_provider_req.c create mode 100644 src/providers/data_provider_req.h diff --git a/Makefile.am b/Makefile.am index f153ab0adf390880672a1681b386ea26426465cb..94920b29d7aab44085e401f8ada8555ab69fed6a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -446,7 +446,8 @@ SSSD_RESPONDER_OBJ = \ src/monitor/monitor_iface_generated.c \ src/monitor/monitor_iface_generated.h \ src/providers/data_provider_iface_generated.c \ -src/providers/data_provider_iface_generated.h +src/providers/data_provider_iface_generated.h \ +src/providers/data_provider_req.c SSSD_TOOLS_OBJ = \ src/tools/sss_sync_ops.c \ @@ -583,6 +584,7 @@ dist_noinst_HEADERS = \ src/confdb/confdb_private.h \ src/confdb/confdb_setup.h \ src/providers/data_provider.h \ +src/providers/data_provider_req.h \ src/providers/dp_backend.h \ src/providers/dp_dyndns.h \ src/providers/dp_ptask_private.h \ @@ -1193,6 +1195,7 @@ endif sssd_be_SOURCES = \ src/providers/data_provider_be.c \ +src/providers/data_provider_req.c \ src/providers/data_provider_fo.c \ src/providers/data_provider_opts.c \ src/providers/data_provider_callbacks.c \ @@ -1646,6 +1649,7 @@ simple_access_tests_SOURCES = \ src/providers/simple/simple_access.c \ src/providers/simple/simple_access_check.c \ src/providers/data_provider_be.c \ +src/providers/data_provider_req.c \ src/providers/data_provider_fo.c \ src/providers/data_provider_opts.c \ src/providers/data_provider_callbacks.c \ diff --git a/src/providers/data_provider.h b/src/providers/data_provider.h index 510c63ce41c99314ec8fcf11fffb2e66082e8951..39051b90c3aad96f62dcbb86a20bcfd8c954879b 100644 --- a/src/providers/data_provider.h +++ b/src/providers/data_provider.h @@ -43,6 +43,7 @@ #include sbus/sbus_client.h #include sss_client/sss_cli.h #include util/authtok.h +#include providers/data_provider_req.h #include providers/data_provider_iface_generated.h #define DATA_PROVIDER_VERSION 0x0001 @@ -131,22 +132,6 @@ #define BE_FILTER_CERT 6 #define BE_FILTER_WILDCARD 7 -#define BE_REQ_USER 0x0001 -#define BE_REQ_GROUP 0x0002 -#define BE_REQ_INITGROUPS0x0003 -#define BE_REQ_NETGROUP 0x0004 -#define BE_REQ_SERVICES 0x0005 -#define BE_REQ_SUDO_FULL 0x0006 -#define BE_REQ_SUDO_RULES0x0007 -#define BE_REQ_AUTOFS0x0009 -#define BE_REQ_HOST 0x0010 -#define BE_REQ_BY_SECID 0x0011 -#define BE_REQ_USER_AND_GROUP 0x0012 -#define BE_REQ_BY_UUID 0x0013 -#define BE_REQ_BY_CERT 0x0014 -#define BE_REQ_TYPE_MASK 0x00FF -#define BE_REQ_FAST 0x1000 - #define DP_SEC_ID secid #define DP_CERT cert /* sizeof() counts the trailing \0 so we must substract 1 for the string diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index d147630248f0a24f5a632760b55b9284a6928e40..d71a69cb8e2997975828236998ec0b0e3f353f07 100644 --- a/src/providers/data_provider_be.c +++ b/src/providers/data_provider_be.c @@ -1104,7 +1104,8 @@ static int be_get_account_info(struct sbus_request *dbus_req, void *user_data) return EOK; /* handled */ DEBUG(SSSDBG_FUNC_DATA, - Got request for [%#x][%d][%s]\n, type, attr_type, filter); + Got request for [%#x][%s][%d][%s]\n, type,
Re: [SSSD] [PATCH] DATA_PROVIDER: BE_REQ as string in log message
On 08/21/2015 05:49 PM, Petr Cech wrote: On 08/21/2015 05:10 PM, Pavel Reichl wrote: Petr can you change data_provider_req.c to include providers/data_provider_req.h instead of providers/data_provider.h ? I originally thought that you will be able to include solely data_provider_req.h from responder_dp.c but I see that data_provider.h is required. But I still don't mind introducing data_provider_req.h. If you change this little nitpick I think I can ACK the patch. Pavel, you're right, that's mine main opinion to this issue. I am sorry, I need more focus and coffe. There is fixed (not repaired) patch. Petr ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel ACK ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] Embedding Lua into SSSD
On Fri, 2015-08-21 at 20:01 +0300, Nikolai Kondrashov wrote: Hi everyone, I might be in a strange and careless mood today, but here is something I wanted to suggest since the time I saw the amount of logic that goes into SSSD and is implemented in C. What if we implement some of the complicated logic inside SSSD in Lua [1]? Lua is a language specifically designed to be embedded. It grew into popularity after it was embedded into World Of Warcraft. It was also embedded and is still being embedded into a bunch of other games. However, games is far from the only application for Lua. Lua is a simple and lightweight language, yet with lots of flexibility - grasping the essence of the whole reference manual [2] takes one evening and it's an entertaining read. The VM is small and fast. In fact some people use it on microcontrollers [3]. I personally embedded it into two C-based projects, one of which was already quite big, and another was a rewrite of a smallish, but complicated C project. In both cases, where before things were tedious and hard, they have become easy and fun. The first project was on an embedded platform. I also had some other, but smaller stuff done with it. Generally, Lua gets put into the middle with outside interfaces and performance-critical parts implemented in C. With a very simple interface to and from C one can put boundaries wherever necessary. Yes, the interface is simpler than Python's. The process is this: you implement some C libraries (shared/static) which are loadable from Lua (Lua-C interface) and then create a VM or several, load some Lua code and call into it (C-Lua interface). Some of the benefits are: Clearer logic (higher level language) Much less memory leaks and very little memory management (mostly in Lua-C interface). Faster development cycle (no compilation, edit and retry on a live daemon) Potentially easy plugin interface (yay for admin scripts!) Some of the drawbacks are: More difficult to do low level stuff (so we just don't do that) Lower performance than C (still good enough for games, and we can have the critical parts in C easily) Needs learning and development investment. Regarding the development investment, due to low cost of C/Lua interface implementation, it is practical to replace the relevant C code piece-by-piece, provided pieces are not too small (say, 2KLOC and up). Lastly, don't take this too seriously. I realise that this is disruptive and hard. However if you ever feel like SSSD has grown too big and complex, ping me, or just try Lua yourself :) Nick [1] http://www.lua.org/ [2] http://www.lua.org/manual/5.3/manual.html [3] http://www.eluaproject.net/ Hi Nick, can you provide an example of a piece of SSSD youd replace with Lua ? I am not asking for an implementation but a high level view of what a function looks like to day and what it would look like if we were going to use Lua (ideally with a list of primitives we'd still need to provide, to understand how much code Lua replaces for real. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
[SSSD]Re: Re: [PATCH] TESTS: ldap_id_cleanup timeouts
On 08/21/2015 01:33 PM, Michal Židek wrote: On 08/20/2015 01:50 PM, Petr Cech wrote: On 08/19/2015 08:26 PM, Michal Židek wrote: Hi! This is another patch to avoid failing tests in the CI (make-check-valgrind). This time the ldap_id_cleanup tests. Looks like the one second cache timeout was too short and the tests sometimes failed because they expected the entries to be still valid for a short while after they were added to sysdb. I saw the failures only on Fedora 20 CI machine. See the attached patch. Michal Hi, I just run your patch on my F22 VM and I see some trouble here... see attachment. Petr PS: I used clean GIT and your patch, nothing else. I know that this problem is another then you solved. But it is still issue. I just saw 2 more fails in the CI because of the short cache timeout. The problem you see, as you said as well, is a different one and I agree it should be solved as well but so far we were able to reproduce it on your computer only and I did not see fails in the CI because of that problem. I would suggest pushing this patch (if you ACK it that is) to fix CI and look at the problem you found later. Michal OK, I agree. There is new ticket about the mentioned bug: https://fedorahosted.org/sssd/ticket/2768 And there are CI tests: http://sssd-ci.duckdns.org/logs/job/23/57/summary.html (The failing is not connected to this patch.) ACK Petr
Re: [SSSD] [PATCH] Switch ldap_user_certificate default to userCertificate; binary
Hi, On 10.8.2015 12:59, Jakub Hrozek wrote: Hi, the attached patches fix #2742. The first one makes sure we can print the certificate (or any binary attribute, really) safely. We only need to make sure to escape the attribute values before saving them to sysdb, because then ldb guarantees terminating them. The second just switches the attribute value. I tested using this howto: http://www.freeipa.org/page/V4/User_Certificates#How_to_Test You'll also want to use a recent enough IPA version, one that fixes: https://fedorahosted.org/freeipa/ticket/5173 Then, on the client, call: dbus-send --print-reply \ --system \ --dest=org.freedesktop.sssd.infopipe \ /org/freedesktop/sssd/infopipe/Users \ org.freedesktop.sssd.infopipe.Users.FindByCertificate \ string:$( openssl x509 cert.pem ) The result will be an object path. LGTM, but I would think userCertificate;binary should be the default everywhere, i.e. generic LDAP, as that is the correct attribute name according to RFC 4523. IMHO when someone uses the standard name in generic LDAP, they should not be forced to change SSSD configuration because of it. Honza -- Jan Cholasta ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
