[SSSD] [WIKI] Contribute and DevelTips are duplicate
Hi, I've read the wiki according to # https://fedorahosted.org/sssd/ticket/2706 and I think that it could be helpful clean the page # https://fedorahosted.org/sssd/wiki/DevelTutorials from git topic and create new page for everything related to git. The motivation is, that: * Contribute briefly describe whole process on basic level, * DevelTips looks like How To, * DevelTutorials are more about technologies, libraries, build process..., * So there could be one page for everything about git. NOTES how we could edit wiki: https://fedorahosted.org/sssd/wiki/Contribute Contribute Contribution Policy Source Code Repository /* * There could be only a link to the repo and * reference to New Git page. */ Tips and tricks for developers QA, Development and Bug Triage Development Repositories Localization and Internationalization Design Pages Credits Latest Documentation and Presentations https://fedorahosted.org/sssd/wiki/DevelTips SSSD Devel page Are there any introductory tutorials available? /* + Reference to the new Git page */ When I debug an SSSD process in a debugger, it always gets killed with … Using valgrind to identify memory access problems Using strace to track the SSSD processes How do I track work-in-progress of other developers? /* * Is it * still valid? * * I tried link * for jhrozek and * his sssd.git * and the url * doesn't exist. */ Why does make check take so long? Using clang to perform static analysis of source code When I compile the SSSD from source there is an error that says … https://fedorahosted.org/sssd/wiki/DevelTutorials /* * Label @new-git-page means * that I would like move given paragraph to the New git page */ Talloc Tevent and tevent_req Coding Style Code Contributions /* @new-git-page */ Getting the source /* @new-git-page */ Building SSSD for development and debugging Unit tests Submitting a patch upstream /* @new-git-page */ Patch metadata /* @new-git-page */ Translation Contributions Devel Tips New Git page /* Maybe Git Tips? */ + paragraph about git setup from Contribute-Source Code Repository + some paragraphs of DevelTutorials-Code Contributions I am looking forward your opinions. Petr ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCHES] Wildcard lookups for InfoPipe
On 07/09/2015 12:17 PM, Jakub Hrozek wrote: It may be also better to move them into the sysdb module? I would do that if we anticipate more listing intefaces using this kind of filter. I disagree since in my opinion it belongs into sysdb, but I will not insist. If I read the code correctly, you always return only records that were updated. It this sufficient? I think we need to return all records that match the provided filter. Please check my logic here, but I think the way it's coded now is safer. On the back end side, we can't reasonably detect if an entry has been removed if we only use the wildcard lookup. So the back end can't reasonably decide which entries have been removed from the server. The reason is the limit. There might be more entries on the server side that those that match the filter, so what the back end sees is just a subset. And the entries outside the limit can be cached by direct lookups, so we simply shouldn't touch them.. What the code does now is only returns entries updated by the wildcard lookup. The entries that were not updated are not returned to the fronted -- they just sit in the cache until some application requests them with a direct lookup. The direct lookup can detect removal of the object on the server side and also remove the cached object. Does that make sense? Yes, thank you for explanation. I think the logic is sound. if (state-result == NULL || state-result-count == 0 || state-input-type == CACHE_REQ_USER_BY_FILTER || state-input-type == CACHE_REQ_GROUP_BY_FILTER) { ret = ENOENT; } else { I would like to avoid this type of code whenever possible. Can we replace it with a macro e.g. always_check_cache(state-input) or function? OK, done. Please feel free to suggest a better function name. Is it a valid operation to use a filter that needs to be parse into name and domain (user*@domain)? I don't think so, we have a specialized function for that. What function? *Patch #01 tests: Move N_ELEMENTS definition to tests/common.h* ACK *Patch #02 SYSDB: Add functions to look up multiple entries including name and custom filter* +static char *enum_filter(TALLOC_CTX *mem_ctx, + const char *base_filter, + const char *name_filter, + const char *addtl_filter) You are leaking memory here if any of the allocation fails. I know it will be freed in the caller but it is not a good practice. *Patch #03 DP: Add DP_WILDCARD and SSS_DP_WILDCARD_USER/SSS_DP_WILDCARD_GROUP* +} else if (info-type == SSS_DP_WILDCARD_USER || + info-type == SSS_DP_WILDCARD_GROUP) { +if (info-extra) { +filter = talloc_asprintf(info, %s=%s:%s, DP_WILDCARD, + info-opt_name, info-extra); +} else { +filter = talloc_asprintf(info, %s=%s, DP_WILDCARD, + info-opt_name); +} Do you think it is safe to use the same prefix for both users and groups look up? *Patch #04 cache_req: Extend cache_req with wildcard lookups* case CACHE_REQ_USER_BY_FILTER: case CACHE_REQ_GROUP_BY_FILTER: /* Nothing to do, adding a wildcard request to ncache doesn't * make sense */ /* Return true if the request can be cached or false if the cache_req * code needs to contact the DP every time */ Can you finish the sentence with dot so it is consistent with the rest of the code? static bool cache_req_cachable(struct cache_req_input *input) How about cache_req_avoid_cache? *Patch #05 UTIL: Add sss_filter_sanitize_ex* +const char has_all_allow_asterisk_expected[] = \\5c\\28user\\29*name; Can you add comment with unescaped characters so it is human-readable? *Patch #06 LDAP: Fetch users and groups using wildcards* @@ -953,6 +991,14 @@ static void groups_get_done(struct tevent_req *subreq) * group we have nothing to do here. */ break; +case BE_FILTER_WILDCARD: +/* We can't know if all users are up-to-date, especially in a large + * environment. Do not delete any records, let the responder fetch + * the entries they are requested in + */ +break; Copy and paste error... you meant groups here. *Patch #07 LDAP: Add sdap_get_and_parse_generic_send* ACK *Patch #08 LDAP: Use sdap_get_and_parse_generic_/_recv* ACK *Patch #09 LDAP: Add sdap_lookup_type enum* ACK *Patch #10 LDAP: Add the wildcard_limit option* +ldap_pwdlockout_dn = str, None, false This probably belongs to separate patch... *Patch #11 IFP: Add wildcard requests* size_t ifp_list_ctx_cap(struct ifp_list_ctx *list_ctx, size_t entries) ifp_list_ctx_remaining_capacity? I had no idea what the function does before I read its code. ___ sssd-devel mailing list
Re: [SSSD] [PATCH] KRB5: Use the right domain for case-sensitive flag
On 07/03/2015 11:55 AM, Jakub Hrozek wrote: Pavel, can you please check if this fix is valid for setups where the main domain and subdomain have different case sensitivity? LGTM, ci passed: http://sssd-ci.duckdns.org/logs/job/18/76/summary.html I did some basic smoke testing and feature around option krb5_map_user works. But I have to admit I wasn't able to configure the feature to work with IPA in trust with AD - which is the only use case relevant to the change, right? In my previous testing I mostly used proxy as id provider and krb as auth provider. ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
[SSSD] [PATCH] test common: sss_dp_get_account_recv() fix assignment
Hi, this patch fixes a simple copy-and-paste issue in the common test code. Since we currently always call mock_account_recv() with the second and third argument set to 0 and NULL respectively it doesn't became an issue earlier. bye, Sumit From 5d2a8ee6d6d26d276b3be4c97fe0bf9e5cf7d099 Mon Sep 17 00:00:00 2001 From: Sumit Bose sb...@redhat.com Date: Fri, 10 Jul 2015 12:37:43 +0200 Subject: [PATCH] test common: sss_dp_get_account_recv() fix assignment --- src/tests/cmocka/common_mock_resp_dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/cmocka/common_mock_resp_dp.c b/src/tests/cmocka/common_mock_resp_dp.c index 08d74179d8e2dc0561ea3e2d1e9b5580762ee633..a67ceee4a43184d9894c19a5c11161aca4e1caff 100644 --- a/src/tests/cmocka/common_mock_resp_dp.c +++ b/src/tests/cmocka/common_mock_resp_dp.c @@ -51,7 +51,7 @@ sss_dp_get_account_recv(TALLOC_CTX *mem_ctx, *dp_err = sss_mock_type(dbus_uint16_t); *dp_ret = sss_mock_type(dbus_uint32_t); -*dp_ret = sss_mock_type(dbus_uint32_t); +*err_msg = sss_mock_ptr_type(char *); cb = sss_mock_ptr_type(acct_cb_t); if (cb) { -- 2.1.0 ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCHES] DYNDNS: support mult. interfaces for dyndns_iface opt
On 10/07/15 16:55, Jakub Hrozek wrote: On Thu, Jul 09, 2015 at 01:17:44PM +0200, Pavel Reichl wrote: While reading #2558 I noticed that there is a further request relating to these patches - mbasti asks If there could be some mean how to send IPs of all interfaces? Is this a good idea in general? I suppose I could implement it with some special value for 'dyndns_iface' - ideally the special value would contain characters prohibited to be part of interface name if there are such... I would say this looks like a useful feature in general, but can you ask Martin why he needs this feature? I think the time has come for 1.13 development to start turning into bug fixing more, so unless there is really some particular use-case, please move the ticket into 1.14. ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel This is blocker for https://fedorahosted.org/freeipa/ticket/4249 Without this patch we cannot add dualstack and multihomed support to IPA clients, because when IPA adds all IP and IPv6 addresses to DNS, then SSSD just deletes all records and put there just one address. We want to support dualstack in IPA for long time. Several users wanted dualstack, even multihomed support. Martin2 -- Martin Basti ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCHES] DYNDNS: support mult. interfaces for dyndns_iface opt
On Fri, Jul 10, 2015 at 05:52:20PM +0200, Martin Basti wrote: On 10/07/15 16:55, Jakub Hrozek wrote: On Thu, Jul 09, 2015 at 01:17:44PM +0200, Pavel Reichl wrote: While reading #2558 I noticed that there is a further request relating to these patches - mbasti asks If there could be some mean how to send IPs of all interfaces? Is this a good idea in general? I suppose I could implement it with some special value for 'dyndns_iface' - ideally the special value would contain characters prohibited to be part of interface name if there are such... I would say this looks like a useful feature in general, but can you ask Martin why he needs this feature? I think the time has come for 1.13 development to start turning into bug fixing more, so unless there is really some particular use-case, please move the ticket into 1.14. ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel This is blocker for https://fedorahosted.org/freeipa/ticket/4249 Without this patch we cannot add dualstack and multihomed support to IPA clients, because when IPA adds all IP and IPv6 addresses to DNS, then SSSD just deletes all records and put there just one address. I can see the argument for dualstack -- we shouldn't remove v6 addresses if we're talking to the server over v4 and conversely. I suspect the multi-homed use-case is a bit less important, right? We want to support dualstack in IPA for long time. Several users wanted dualstack, even multihomed support. Is there a corresponding IPA ticket or some link to a freeipa-users thread? Please note I agree with the technical requirement, but there's limited time, so we should prioritize. ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] test common: sss_dp_get_account_recv() fix assignment
On Fri, Jul 10, 2015 at 05:48:57PM +0200, Sumit Bose wrote: Hi, this patch fixes a simple copy-and-paste issue in the common test code. Since we currently always call mock_account_recv() with the second and third argument set to 0 and NULL respectively it doesn't became an issue earlier. bye, Sumit From 5d2a8ee6d6d26d276b3be4c97fe0bf9e5cf7d099 Mon Sep 17 00:00:00 2001 From: Sumit Bose sb...@redhat.com Date: Fri, 10 Jul 2015 12:37:43 +0200 Subject: [PATCH] test common: sss_dp_get_account_recv() fix assignment --- src/tests/cmocka/common_mock_resp_dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/cmocka/common_mock_resp_dp.c b/src/tests/cmocka/common_mock_resp_dp.c index 08d74179d8e2dc0561ea3e2d1e9b5580762ee633..a67ceee4a43184d9894c19a5c11161aca4e1caff 100644 --- a/src/tests/cmocka/common_mock_resp_dp.c +++ b/src/tests/cmocka/common_mock_resp_dp.c @@ -51,7 +51,7 @@ sss_dp_get_account_recv(TALLOC_CTX *mem_ctx, *dp_err = sss_mock_type(dbus_uint16_t); *dp_ret = sss_mock_type(dbus_uint32_t); -*dp_ret = sss_mock_type(dbus_uint32_t); +*err_msg = sss_mock_ptr_type(char *); (Spoiler alert: Sumit already showed me the bug on IRC) ACK. CI: http://sssd-ci.duckdns.org/logs/job/18/78/summary.html ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCHES] DYNDNS: support mult. interfaces for dyndns_iface opt
On Thu, Jul 09, 2015 at 01:17:44PM +0200, Pavel Reichl wrote: While reading #2558 I noticed that there is a further request relating to these patches - mbasti asks If there could be some mean how to send IPs of all interfaces? Is this a good idea in general? I suppose I could implement it with some special value for 'dyndns_iface' - ideally the special value would contain characters prohibited to be part of interface name if there are such... I would say this looks like a useful feature in general, but can you ask Martin why he needs this feature? I think the time has come for 1.13 development to start turning into bug fixing more, so unless there is really some particular use-case, please move the ticket into 1.14. ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel