[SSSD] Re: [PATCH] KRB5: Discern between expired & disabled AD user
On Tue, 2016-03-01 at 18:22 -0500, Simo Sorce wrote: > On Tue, 2016-03-01 at 22:34 +0100, Lukas Slebodnik wrote: > > On (01/03/16 12:05), Simo Sorce wrote: > > >On Tue, 2016-03-01 at 17:51 +0100, Lukas Slebodnik wrote: > > >> On (01/03/16 17:45), Lukas Slebodnik wrote: > > >> >On (31/01/16 11:53), Simo Sorce wrote: > > >> >>Expired != Disabled > > >> >>this change is intentional. > > >> >> > > >> >Yes, but explain it to Active directory :-) > > >> > > > >> >Attached is patch with workaround/hack > > >> >regression with expired AD users. > > >> > > > >> ENOPATCH > > >> > > >> LS > > > > > >I think a better approach is to return the KRBKDC error from the child > > >without mapping (or with an intermediate mapping) and have the IPA and > > >AD providers map it on their own. > > > > > It's not related to mapping KRBKDC error codes to internal error code. > > The main problem is that AD return the same error code for expired > > and disabled user. And ad provider used generic krb5 functions. > > > > BTW the same issue would be with id_provider ldap + > > auth_provider = krb5 with AD :-( > > I'm not sure how your proposal would help. > > I think AD returns additional information in edata, maybe we can use > that to do the proper mapping in the generic krb5 code. > > Absence of AD specific edata would indicate MIT mapping, presence would > allow us to use that additional data to figure out the correct mapping. > > Simo. > See MS-KILE[1] 2.2.1, I bet the two conditions returns two different windows Style errors in etext (not edata, sorry). [1] https://msdn.microsoft.com/en-us/library/cc233855.aspx Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] KRB5: Discern between expired & disabled AD user
On Tue, 2016-03-01 at 22:34 +0100, Lukas Slebodnik wrote: > On (01/03/16 12:05), Simo Sorce wrote: > >On Tue, 2016-03-01 at 17:51 +0100, Lukas Slebodnik wrote: > >> On (01/03/16 17:45), Lukas Slebodnik wrote: > >> >On (31/01/16 11:53), Simo Sorce wrote: > >> >>Expired != Disabled > >> >>this change is intentional. > >> >> > >> >Yes, but explain it to Active directory :-) > >> > > >> >Attached is patch with workaround/hack > >> >regression with expired AD users. > >> > > >> ENOPATCH > >> > >> LS > > > >I think a better approach is to return the KRBKDC error from the child > >without mapping (or with an intermediate mapping) and have the IPA and > >AD providers map it on their own. > > > It's not related to mapping KRBKDC error codes to internal error code. > The main problem is that AD return the same error code for expired > and disabled user. And ad provider used generic krb5 functions. > > BTW the same issue would be with id_provider ldap + > auth_provider = krb5 with AD :-( > I'm not sure how your proposal would help. I think AD returns additional information in edata, maybe we can use that to do the proper mapping in the generic krb5 code. Absence of AD specific edata would indicate MIT mapping, presence would allow us to use that additional data to figure out the correct mapping. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] KRB5: Discern between expired & disabled AD user
On (01/03/16 12:05), Simo Sorce wrote: >On Tue, 2016-03-01 at 17:51 +0100, Lukas Slebodnik wrote: >> On (01/03/16 17:45), Lukas Slebodnik wrote: >> >On (31/01/16 11:53), Simo Sorce wrote: >> >>Expired != Disabled >> >>this change is intentional. >> >> >> >Yes, but explain it to Active directory :-) >> > >> >Attached is patch with workaround/hack >> >regression with expired AD users. >> > >> ENOPATCH >> >> LS > >I think a better approach is to return the KRBKDC error from the child >without mapping (or with an intermediate mapping) and have the IPA and >AD providers map it on their own. > It's not related to mapping KRBKDC error codes to internal error code. The main problem is that AD return the same error code for expired and disabled user. And ad provider used generic krb5 functions. BTW the same issue would be with id_provider ldap + auth_provider = krb5 with AD :-( I'm not sure how your proposal would help. LS ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] KRB5: Discern between expired & disabled AD user
On Tue, 2016-03-01 at 17:51 +0100, Lukas Slebodnik wrote: > On (01/03/16 17:45), Lukas Slebodnik wrote: > >On (31/01/16 11:53), Simo Sorce wrote: > >>Expired != Disabled > >>this change is intentional. > >> > >Yes, but explain it to Active directory :-) > > > >Attached is patch with workaround/hack > >regression with expired AD users. > > > ENOPATCH > > LS I think a better approach is to return the KRBKDC error from the child without mapping (or with an intermediate mapping) and have the IPA and AD providers map it on their own. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] KRB5: Discern between expired & disabled AD user
On (01/03/16 17:45), Lukas Slebodnik wrote:
>On (31/01/16 11:53), Simo Sorce wrote:
>>Expired != Disabled
>>this change is intentional.
>>
>Yes, but explain it to Active directory :-)
>
>Attached is patch with workaround/hack
>regression with expired AD users.
>
ENOPATCH
LS
>From a2feac9bb5f6fde16d992a7e0b30cccb103a7000 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik
Date: Tue, 1 Mar 2016 15:13:45 +0100
Subject: [PATCH] KRB5: Discern between expired & disabled AD user
Active directory return krb5 error code KRB5KDC_ERR_CLIENT_REVOKED
"-1765328366/Clients credentials have been revoked" for expired and
disabled user. This is difference between AD and IPA
https://fedorahosted.org/sssd/ticket/2924.
Therefore we need to distinguish between these two states.
We can use AD attribute userAccountControl[1] and override
ERR_ACCOUNT_LOCKED returned from krb5_child to ERR_ACCOUNT_EXPIRED
for expired AD user.
[1] https://msdn.microsoft.com/en-us/library/ms680832%28v=vs.85%29.aspx
Related to:
https://fedorahosted.org/sssd/ticket/2924
---
src/providers/krb5/krb5_auth.c | 53 ++
1 file changed, 53 insertions(+)
diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c
index
f69245efdbac2e771e09c35706cd96552de8375d..bf689a947eabec3e1f1b0a21bd308a2d6aa01dda
100644
--- a/src/providers/krb5/krb5_auth.c
+++ b/src/providers/krb5/krb5_auth.c
@@ -33,6 +33,7 @@
#include
#include "util/util.h"
+#include "util/strtonum.h"
#include "util/find_uid.h"
#include "util/auth_utils.h"
#include "db/sysdb.h"
@@ -787,6 +788,10 @@ done:
}
}
+static void ad_discern_expired_disabled_user(struct krb5_auth_state *state,
+ const char *user,
+ int32_t *_msg_status);
+
static void krb5_auth_done(struct tevent_req *subreq)
{
struct tevent_req *req = tevent_req_callback_data(subreq, struct
tevent_req);
@@ -911,6 +916,8 @@ static void krb5_auth_done(struct tevent_req *subreq)
}
}
+ad_discern_expired_disabled_user(state, pd->user, &res->msg_status);
+
/* If the child request failed, but did not return an offline error code,
* return with the status */
switch (res->msg_status) {
@@ -1150,6 +1157,52 @@ done:
}
+static void ad_discern_expired_disabled_user(struct krb5_auth_state *state,
+ const char *user,
+ int32_t *_msg_status)
+{
+int ret;
+/* https://msdn.microsoft.com/en-us/library/ms680832%28v=vs.85%29.aspx */
+const int UAC_ACCOUNTDISABLE = 0x0002;
+int32_t msg_status = *_msg_status;
+struct ldb_result *res = NULL;
+const char *attrs[] = { SYSDB_AD_USER_ACCOUNT_CONTROL, NULL };
+
+/* Active directory return krb5 error code KRB5KDC_ERR_CLIENT_REVOKED
+ * "-1765328366/Clients credentials have been revoked" for expired and
+ * disabled user. This is difference between AD and IPA
+ * https://fedorahosted.org/sssd/ticket/2924.
+ *
+ * Therefore we need to distinguish between these two states.
+ * We can use AD attribute userAccountControl and override
+ * ERR_ACCOUNT_LOCKED returned from krb5_child to ERR_ACCOUNT_EXPIRED
+ * for expired user.
+ */
+if (msg_status == ERR_ACCOUNT_LOCKED) {
+ret = sysdb_get_user_attr_with_views(state, state->domain, user, attrs,
+ &res);
+if (ret == EOK && res->count == 1) {
+const char *uac_str;
+uac_str = ldb_msg_find_attr_as_string(res->msgs[0],
+
SYSDB_AD_USER_ACCOUNT_CONTROL,
+ NULL);
+if (uac_str != NULL) {
+uint32_t uac;
+char *endptr;
+errno_t error;
+
+uac = strtouint32(uac_str, &endptr, 10);
+error = errno;
+if (error == 0 && (uac & UAC_ACCOUNTDISABLE) == 0) {
+*_msg_status = ERR_ACCOUNT_EXPIRED;
+}
+}
+}
+
+talloc_zfree(res);
+}
+}
+
int krb5_auth_recv(struct tevent_req *req, int *pam_status, int *dp_err)
{
struct krb5_auth_state *state = tevent_req_data(req, struct
krb5_auth_state);
--
2.7.2
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] [PATCH] KRB5: Discern between expired & disabled AD user
On (31/01/16 11:53), Simo Sorce wrote: >Expired != Disabled >this change is intentional. > Yes, but explain it to Active directory :-) Attached is patch with workaround/hack regression with expired AD users. LS ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] sdap: improve filtering of multiple results in GC lookups
On Mon, Feb 29, 2016 at 11:17:14AM +0100, Jakub Hrozek wrote: > ACK. > > There were some downstream tests failing, but the same tests kept > failing even with a vanilla RHEL-7.2 package, so I think it's a fluke in > the test and not a regression. * master: 5ff7a765434ed0b4d37564ade26d7761d06f81c3 * sssd-1-13: 52ea2caa4d21a980902cd0f2fd77ceba25062a8c ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] IDMAP: Make parameter name more descriptive
On 03/01/2016 04:54 PM, Lukas Slebodnik wrote: On (01/03/16 15:54), Pavel Reichl wrote: I added one more similar patch. sss_idmap_calculate_range can accept domain SID or range identifier on its input. Previous parameter name was misleading. Could you explain why it need to be in two separate patches? LS ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org 2 separate changes? 2 different functions. I don't really care. ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] IDMAP: Make parameter name more descriptive
On (01/03/16 15:54), Pavel Reichl wrote: >I added one more similar patch. > >sss_idmap_calculate_range can accept domain SID or range identifier on its >input. Previous parameter name was misleading. Could you explain why it need to be in two separate patches? LS ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] remove user certificate if not found on the server
On Mon, Feb 29, 2016 at 02:05:37PM +0100, Sumit Bose wrote:
> On Thu, Feb 25, 2016 at 11:36:43AM +0100, Pavel Březina wrote:
> > On 02/25/2016 11:07 AM, Sumit Bose wrote:
> > >On Thu, Feb 25, 2016 at 10:53:03AM +0100, Pavel Březina wrote:
> > >>On 02/24/2016 02:34 PM, Sumit Bose wrote:
> > >>>On Wed, Feb 24, 2016 at 10:31:31AM +0100, Pavel Březina wrote:
> > On 02/23/2016 02:09 PM, Sumit Bose wrote:
> > >On Tue, Feb 23, 2016 at 01:24:30PM +0100, Pavel Březina wrote:
> > >>https://fedorahosted.org/sssd/ticket/2934
> > >
> > >> From 94ae3c5231dc7f1cd9f9d172d13a11a8afcacd16 Mon Sep 17 00:00:00
> > >> 2001
> > >>From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
> > >>Date: Tue, 23 Feb 2016 11:02:42 +0100
> > >>Subject: [PATCH] remove user certificate if not found on the server
> > >>
> > >>If the user is not found by cert lookup when the user is already
> > >>cached, two things may happen:
> > >>1) cert was removed from the user object
> > >>2) user was removed
> > >>
> > >>Instead of issuing another cert lookup we will just remove cert
> > >>attribute from the cache not touching the expiration timestamp so
> > >>the user may be updated later when needed.
> > >>
> > >>Resolves:
> > >>https://fedorahosted.org/sssd/ticket/2934
> > >>---
> > >> src/db/sysdb.h | 3 ++-
> > >> src/db/sysdb_ops.c | 47
> > >>
> > >> src/providers/ldap/ldap_id.c | 10 ++
> > >> 3 files changed, 59 insertions(+), 1 deletion(-)
> > >>
> > >>diff --git a/src/db/sysdb.h b/src/db/sysdb.h
> > >>index
> > >>2e797fd7fa39163c2ab6a10e51228e0f1af3f9e3..d074d4d661554a798151caee831cc672a927712f
> > >> 100644
> > >>--- a/src/db/sysdb.h
> > >>+++ b/src/db/sysdb.h
> > >>@@ -1154,7 +1154,8 @@ errno_t sysdb_search_user_by_cert(TALLOC_CTX
> > >>*mem_ctx,
> > >>const char *cert,
> > >>struct ldb_result **res);
> > >>
> > >>-
> > >>+errno_t sysdb_remove_cert(struct sss_domain_info *domain,
> > >>+ const char *cert);
> > >>
> > >> /* === Functions related to GPOs === */
> > >>
> > >>diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
> > >>index
> > >>ab0d59ca6db620dfbf7e74a93745df242b6fc3a3..aa688f42f9a6f7f0f86e1171df0a5f0346a59ea5
> > >> 100644
> > >>--- a/src/db/sysdb_ops.c
> > >>+++ b/src/db/sysdb_ops.c
> > >>@@ -3764,6 +3764,53 @@ errno_t sysdb_search_user_by_cert(TALLOC_CTX
> > >>*mem_ctx,
> > >> return sysdb_search_object_by_cert(mem_ctx, domain, cert,
> > >> user_attrs, res);
> > >> }
> > >>
> > >>+static errno_t sysdb_remove_user_cert(struct sss_domain_info *domain,
> > >>+ const char *name)
> > >>+{
> > >>+struct ldb_message_element el = { 0, SYSDB_USER_CERT, 0, NULL };
> > >>+struct sysdb_attrs attrs = { 1, &el };
> > >>+
> > >>+DEBUG(SSSDBG_TRACE_FUNC, "Removing certificate from user %s@%s",
> > >>+ name, domain->name);
> > >>+
> > >>+return sysdb_set_user_attr(domain, name, &attrs, SYSDB_MOD_DEL);
> > >
> > >I would recommend to use sysdb_set_entry_attr() because you already
> > >have
> > >the dn as res->msgs[0]->dn in sysdb_remove_cert. With this I think you
> > >can move everything into sysdb_remove_cert() as well without making to
> > >more complex or longer.
> > >
> > >bye,
> > >Sumit
> >
> > Good idea, see attached patch.
> >
> > >>>
> > >>>Thank you, the patch is working as expected and passes CI as well.
> > >>>Nevertheless see comment below.
> > >>>
> > From 04acbe4139a560049b66c15ce8526c16ef668d54 Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
> > Date: Tue, 23 Feb 2016 11:02:42 +0100
> > Subject: [PATCH] remove user certificate if not found on the server
> >
> > If the user is not found by cert lookup when the user is already
> > cached, two things may happen:
> > 1) cert was removed from the user object
> > 2) user was removed
> >
> > Instead of issuing another cert lookup we will just remove cert
> > attribute from the cache not touching the expiration timestamp so
> > the user may be updated later when needed.
> >
> > Resolves:
> > https://fedorahosted.org/sssd/ticket/2934
> > ---
> > src/db/sysdb.h | 3 ++-
> > src/db/sysdb_ops.c | 31 +++
> > src/providers/ldap/ldap_id.c | 10 ++
> > 3 files changed, 43 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/db/sysdb.h b/src/db/sysdb.h
> > index
> > 2e797fd7fa39163c2ab6a10e51228e0f1af3f9e3..d074d4d661554a798151caee831cc672a927712f
> > >>>
[SSSD] Re: [PATCH] IDMAP: Add minor performance improvements
On Mon, Feb 29, 2016 at 03:10:05PM +0100, Sumit Bose wrote: > On Wed, Feb 17, 2016 at 10:47:26AM +0100, Pavel Reichl wrote: > > On 02/15/2016 06:19 PM, Sumit Bose wrote: > > >On Tue, Jan 26, 2016 at 05:35:06PM +0100, Pavel Reichl wrote: > > >>>Hello, > > >>> > > >>>please see simple patch attached. > > >Hi Pavel, > > > > > >sorry for the delay. See comments below. > > > > > >bye, > > >Sumit > > > > > > > > Thanks for checking and for comments. Amended patch is attached. > > > > Bye. > > Thank you, I have no further comments. I didn't found any issues or > regressions in my testing and CI > http://sssd-ci.duckdns.org/logs/job/38/33/summary.html passed as well, > so ACK. > > bye, > Sumit * master: 012d334cec221d8abf86dffbbaf9649ec0a4b585 ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] CI: Use yum-deprecated instead of dnf
On (01/03/16 08:48), Dan Lavu wrote: >Thanks Nikolai and Lukas, > >Related to the run script, what version of Debian should we be testing >against? Seems like Wheezy is missing libnss-wrapper package in it's default >repos. > Wheezy is old stable and does not have all required dependencies. In readme file, there's written that we support debian testing. jessie is debian stable atm. But it also does not have all required dependencies. There's old cmocka. But it should work on jessie if you install dependencies from testing. or another hack. curl -O http://ftp.cz.debian.org/debian/pool/main/c/cmocka/libcmocka0_1.0.1-2_amd64.deb curl -O http://ftp.cz.debian.org/debian/pool/main/c/cmocka/libcmocka-dev_1.0.1-2_amd64.deb curl -O http://ftp.cz.debian.org/debian/pool/main/n/nss-wrapper/libnss-wrapper_1.1.1-1_amd64.deb curl -O http://ftp.cz.debian.org/debian/pool/main/u/uid-wrapper/libuid-wrapper_1.2.0+dfsg1-1_amd64.deb dpkg --install *.deb LS ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] IDMAP: Make parameter name more descriptive
I added one more similar patch.
sss_idmap_calculate_range can accept domain SID or range identifier on its
input. Previous parameter name was misleading.
>From c17976f5c9053e09ea2033284dc3545d02ba0644 Mon Sep 17 00:00:00 2001
From: Pavel Reichl
Date: Tue, 1 Mar 2016 08:41:24 -0500
Subject: [PATCH 1/2] IDMAP: Make parameter name more descriptive
Domain SID (not name) is part of identification string for helper range.
---
src/lib/idmap/sss_idmap.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/lib/idmap/sss_idmap.c b/src/lib/idmap/sss_idmap.c
index e3e9972b802748770a5f7440fa8ddc8ba75d3362..905087b7510f2524adc94d4a845f9454ad760311 100644
--- a/src/lib/idmap/sss_idmap.c
+++ b/src/lib/idmap/sss_idmap.c
@@ -536,13 +536,13 @@ idmap_error_code dom_check_collision(struct idmap_domain_info *dom_list,
static char*
generate_sec_slice_name(struct sss_idmap_ctx *ctx,
-const char *domain_name, uint32_t rid)
+const char *domain_sid, uint32_t rid)
{
const char *SEC_SLICE_NAME_FMT = "%s-%"PRIu32;
char *slice_name;
int len, len2;
-len = snprintf(NULL, 0, SEC_SLICE_NAME_FMT, domain_name, rid);
+len = snprintf(NULL, 0, SEC_SLICE_NAME_FMT, domain_sid, rid);
if (len <= 0) {
return NULL;
}
@@ -552,7 +552,7 @@ generate_sec_slice_name(struct sss_idmap_ctx *ctx,
return NULL;
}
-len2 = snprintf(slice_name, len + 1, SEC_SLICE_NAME_FMT, domain_name,
+len2 = snprintf(slice_name, len + 1, SEC_SLICE_NAME_FMT, domain_sid,
rid);
if (len != len2) {
ctx->free_func(slice_name, ctx->alloc_pvt);
--
2.4.3
>From 579a6fc3f8b8a8ccd08b0ede51af2a44af789ee9 Mon Sep 17 00:00:00 2001
From: Pavel Reichl
Date: Tue, 1 Mar 2016 09:46:26 -0500
Subject: [PATCH 2/2] IDMAP: Make parameter name more descriptive
Use more generic name for range identifier when calculating range for
new slice.
---
src/lib/idmap/sss_idmap.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/lib/idmap/sss_idmap.c b/src/lib/idmap/sss_idmap.c
index 905087b7510f2524adc94d4a845f9454ad760311..dbb152bb9afc3978f201c967ca31d6b67f7fc4b6 100644
--- a/src/lib/idmap/sss_idmap.c
+++ b/src/lib/idmap/sss_idmap.c
@@ -320,7 +320,7 @@ static bool check_dom_overlap(struct idmap_range_params *prim_range,
}
enum idmap_error_code sss_idmap_calculate_range(struct sss_idmap_ctx *ctx,
-const char *dom_sid,
+const char *range_id,
id_t *slice_num,
struct sss_idmap_range *_range)
{
@@ -362,8 +362,8 @@ enum idmap_error_code sss_idmap_calculate_range(struct sss_idmap_ctx *ctx,
*/
orig_slice = 0;
} else {
-/* Hash the domain sid string */
-hash_val = murmurhash3(dom_sid, strlen(dom_sid), 0xdeadbeef);
+/* Hash the range identifier string */
+hash_val = murmurhash3(range_id, strlen(range_id), 0xdeadbeef);
/* Now get take the modulus of the hash val and the max_slices
* to determine its optimal position in the range.
--
2.4.3
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] [PATCH] IDMAP: Make parameter name more descriptive
Hello,
Please see trivial patch attached, thanks
Bye.
>From 96e6f5cfe6d134748a4db248266fba774b32628b Mon Sep 17 00:00:00 2001
From: Pavel Reichl
Date: Tue, 1 Mar 2016 08:41:24 -0500
Subject: [PATCH] IDMAP: Make parameter name more descriptive
Domain SID (not name) is used for computaion of seed for ID mapping.
---
src/lib/idmap/sss_idmap.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/lib/idmap/sss_idmap.c b/src/lib/idmap/sss_idmap.c
index e3e9972b802748770a5f7440fa8ddc8ba75d3362..905087b7510f2524adc94d4a845f9454ad760311 100644
--- a/src/lib/idmap/sss_idmap.c
+++ b/src/lib/idmap/sss_idmap.c
@@ -536,13 +536,13 @@ idmap_error_code dom_check_collision(struct idmap_domain_info *dom_list,
static char*
generate_sec_slice_name(struct sss_idmap_ctx *ctx,
-const char *domain_name, uint32_t rid)
+const char *domain_sid, uint32_t rid)
{
const char *SEC_SLICE_NAME_FMT = "%s-%"PRIu32;
char *slice_name;
int len, len2;
-len = snprintf(NULL, 0, SEC_SLICE_NAME_FMT, domain_name, rid);
+len = snprintf(NULL, 0, SEC_SLICE_NAME_FMT, domain_sid, rid);
if (len <= 0) {
return NULL;
}
@@ -552,7 +552,7 @@ generate_sec_slice_name(struct sss_idmap_ctx *ctx,
return NULL;
}
-len2 = snprintf(slice_name, len + 1, SEC_SLICE_NAME_FMT, domain_name,
+len2 = snprintf(slice_name, len + 1, SEC_SLICE_NAME_FMT, domain_sid,
rid);
if (len != len2) {
ctx->free_func(slice_name, ctx->alloc_pvt);
--
2.4.3
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] CI: Use yum-deprecated instead of dnf
Thanks Nikolai and Lukas,
Related to the run script, what version of Debian should we be testing
against? Seems like Wheezy is missing libnss-wrapper package in it's
default repos.
+ sudo -p 'Need root permissions to install packages.
Enter sudo password for root: ' apt-get --yes install -- lcov valgrind
autoconf automake autopoint check cifs-utils clang dh-apparmor dnsutils
docbook-xml docbook-xsl gettext krb5-config libaugeas-dev libc-ares-dev
libcmocka-dev libcollection-dev libdbus-1-dev libdhash-dev
libglib2.0-dev libini-config-dev libkeyutils-dev libkrb5-dev
libldap2-dev libldb-dev libltdl-dev libnfsidmap-dev libnl-3-dev
libnl-route-3-dev libnspr4-dev libnss3-dev libpam0g-dev libpcre3-dev
libpopt-dev libsasl2-dev libselinux1-dev libsemanage1-dev
libsmbclient-dev libsystemd-dev libtalloc-dev libtdb-dev libtevent-dev
libtool libtool-bin libxml2-utils make python-dev python3-dev samba-dev
systemd xml-core xsltproc libssl-dev fakeroot libnss-wrapper
libuid-wrapper python-pytest python-ldap ldap-utils slapd
Reading package lists...
Building dependency tree...
Reading state information...
E: Unable to locate package libnss-wrapper
root@sssd2:~/sssd# cat /etc/debian_version
8.3
Thanks,
Dan
On 3/1/16 8:18 AM, Lukas Slebodnik wrote:
On (01/03/16 15:07), Nikolai Kondrashov wrote:
On 03/01/2016 02:56 PM, Lukas Slebodnik wrote:
On (01/03/16 13:30), Nikolai Kondrashov wrote:
On 03/01/2016 10:46 AM, Lukas Slebodnik wrote:
On (29/02/16 20:30), Nikolai Kondrashov wrote:
On 02/29/2016 07:27 PM, Lukas Slebodnik wrote:
On (29/02/16 18:54), Nikolai Kondrashov wrote:
keeping the "if" branches consistent and somewhat easier to interpret.
However, if you and others are more comfortable reading and using regex(3)
regexes, then it's fine.
+[ $# != 0 ] && sudo -p "$prompt" \
+yum-deprecated --assumeyes install -- "$@" |&
+awk 'BEGIN {s=0}
+ /^No package .* available.$/ {s=1}
+ {print}
+ END {exit s}'
+elif [[ "$DISTRO_BRANCH" == -redhat-* ]]; then
[ $# != 0 ] && sudo -p "$prompt" yum --assumeyes install -- "$@" |&
# Pass input to output, fail if a missing package is reported
# TODO Remove and switch to DNF once
I see that avoiding copy-pasting here would produce more complicated code, so
it's perhaps OK. However, it would be good then to copy-paste the comment
along with the TODO as well, and then perhaps add a note and a TODO regarding
BZ1215208.
We will still need awk trick for old style yum on el{6,7}. Therefore
I moved TODO to the 1st branch.
Well, yeah, the bug is Fedora-specific, but we still need to have a reminder
to remove the hack on RHEL as well. It will get fixed eventually.
Correct me if I'm wrong.
We need to awk workaround because yum does not fail if you want to install
unknown packages and there is not a bug for yum and I doubt they would
fix/change it in stable distributions. Therefore we will need to have
this solution there for yum anyway.
Yes. IIRC I reported this for yum originally, but the developers moved the bug
to dnf (fixing it later) and refused to fix it in yum.
I too doubt they'll fix it in stable distributions. However I expect they will
get dnf into RHEL (which is handled by the bottom branch) eventually (RHEL8?),
and will also fix that '--' bug. That's when a note would come in handy.
RHEL8 is far future atm. And I guess we will need to do more changes there :-)
Yes, it's far.
dnf behaves correctly in this manner. However it ignores separator "--"
between arguments and packages which is tracked in BZ1215208.
Yes, exactly.
Does it mean ACK to the last patch?
Alright, otherwise it's fine. ACK :)
Thank you
master:
* 73585f9af928913200999c5b3b983bb9266ee266
sssd-1-13:
* 8f0a510a8c324aa1fa0f318e340b554cd07baf8b
Dan,
it shouls work for you now.
report any other bugs
LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] CI: Use yum-deprecated instead of dnf
On (01/03/16 15:07), Nikolai Kondrashov wrote:
>On 03/01/2016 02:56 PM, Lukas Slebodnik wrote:
>>On (01/03/16 13:30), Nikolai Kondrashov wrote:
>>>On 03/01/2016 10:46 AM, Lukas Slebodnik wrote:
On (29/02/16 20:30), Nikolai Kondrashov wrote:
>On 02/29/2016 07:27 PM, Lukas Slebodnik wrote:
>>On (29/02/16 18:54), Nikolai Kondrashov wrote:
>>>keeping the "if" branches consistent and somewhat easier to interpret.
>>>However, if you and others are more comfortable reading and using
>>>regex(3)
>>>regexes, then it's fine.
>>>
+[ $# != 0 ] && sudo -p "$prompt" \
+yum-deprecated --assumeyes install -- "$@"
|&
+awk 'BEGIN {s=0}
+ /^No package .* available.$/ {s=1}
+ {print}
+ END {exit s}'
+elif [[ "$DISTRO_BRANCH" == -redhat-* ]]; then
[ $# != 0 ] && sudo -p "$prompt" yum --assumeyes install --
"$@" |&
# Pass input to output, fail if a missing package is
reported
# TODO Remove and switch to DNF once
>>>
>>>I see that avoiding copy-pasting here would produce more complicated
>>>code, so
>>>it's perhaps OK. However, it would be good then to copy-paste the comment
>>>along with the TODO as well, and then perhaps add a note and a TODO
>>>regarding
>>>BZ1215208.
>>>
>>We will still need awk trick for old style yum on el{6,7}. Therefore
>>I moved TODO to the 1st branch.
>
>Well, yeah, the bug is Fedora-specific, but we still need to have a
>reminder
>to remove the hack on RHEL as well. It will get fixed eventually.
>
Correct me if I'm wrong.
We need to awk workaround because yum does not fail if you want to install
unknown packages and there is not a bug for yum and I doubt they would
fix/change it in stable distributions. Therefore we will need to have
this solution there for yum anyway.
>>>
>>>Yes. IIRC I reported this for yum originally, but the developers moved the
>>>bug
>>>to dnf (fixing it later) and refused to fix it in yum.
>>>
>>>I too doubt they'll fix it in stable distributions. However I expect they
>>>will
>>>get dnf into RHEL (which is handled by the bottom branch) eventually
>>>(RHEL8?),
>>>and will also fix that '--' bug. That's when a note would come in handy.
>>>
>>RHEL8 is far future atm. And I guess we will need to do more changes there :-)
>
>Yes, it's far.
>
dnf behaves correctly in this manner. However it ignores separator "--"
between arguments and packages which is tracked in BZ1215208.
>>>
>>>Yes, exactly.
>>>
>>Does it mean ACK to the last patch?
>
>Alright, otherwise it's fine. ACK :)
>
Thank you
master:
* 73585f9af928913200999c5b3b983bb9266ee266
sssd-1-13:
* 8f0a510a8c324aa1fa0f318e340b554cd07baf8b
Dan,
it shouls work for you now.
report any other bugs
LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] CI: Use yum-deprecated instead of dnf
On 03/01/2016 02:56 PM, Lukas Slebodnik wrote:
On (01/03/16 13:30), Nikolai Kondrashov wrote:
On 03/01/2016 10:46 AM, Lukas Slebodnik wrote:
On (29/02/16 20:30), Nikolai Kondrashov wrote:
On 02/29/2016 07:27 PM, Lukas Slebodnik wrote:
On (29/02/16 18:54), Nikolai Kondrashov wrote:
keeping the "if" branches consistent and somewhat easier to interpret.
However, if you and others are more comfortable reading and using regex(3)
regexes, then it's fine.
+[ $# != 0 ] && sudo -p "$prompt" \
+yum-deprecated --assumeyes install -- "$@" |&
+awk 'BEGIN {s=0}
+ /^No package .* available.$/ {s=1}
+ {print}
+ END {exit s}'
+elif [[ "$DISTRO_BRANCH" == -redhat-* ]]; then
[ $# != 0 ] && sudo -p "$prompt" yum --assumeyes install -- "$@" |&
# Pass input to output, fail if a missing package is reported
# TODO Remove and switch to DNF once
I see that avoiding copy-pasting here would produce more complicated code, so
it's perhaps OK. However, it would be good then to copy-paste the comment
along with the TODO as well, and then perhaps add a note and a TODO regarding
BZ1215208.
We will still need awk trick for old style yum on el{6,7}. Therefore
I moved TODO to the 1st branch.
Well, yeah, the bug is Fedora-specific, but we still need to have a reminder
to remove the hack on RHEL as well. It will get fixed eventually.
Correct me if I'm wrong.
We need to awk workaround because yum does not fail if you want to install
unknown packages and there is not a bug for yum and I doubt they would
fix/change it in stable distributions. Therefore we will need to have
this solution there for yum anyway.
Yes. IIRC I reported this for yum originally, but the developers moved the bug
to dnf (fixing it later) and refused to fix it in yum.
I too doubt they'll fix it in stable distributions. However I expect they will
get dnf into RHEL (which is handled by the bottom branch) eventually (RHEL8?),
and will also fix that '--' bug. That's when a note would come in handy.
RHEL8 is far future atm. And I guess we will need to do more changes there :-)
Yes, it's far.
dnf behaves correctly in this manner. However it ignores separator "--"
between arguments and packages which is tracked in BZ1215208.
Yes, exactly.
Does it mean ACK to the last patch?
Alright, otherwise it's fine. ACK :)
Nick
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] CI: Use yum-deprecated instead of dnf
On (01/03/16 13:30), Nikolai Kondrashov wrote:
>On 03/01/2016 10:46 AM, Lukas Slebodnik wrote:
>>On (29/02/16 20:30), Nikolai Kondrashov wrote:
>>>On 02/29/2016 07:27 PM, Lukas Slebodnik wrote:
On (29/02/16 18:54), Nikolai Kondrashov wrote:
>keeping the "if" branches consistent and somewhat easier to interpret.
>However, if you and others are more comfortable reading and using regex(3)
>regexes, then it's fine.
>
>>+[ $# != 0 ] && sudo -p "$prompt" \
>>+yum-deprecated --assumeyes install -- "$@" |&
>>+awk 'BEGIN {s=0}
>>+ /^No package .* available.$/ {s=1}
>>+ {print}
>>+ END {exit s}'
>>+elif [[ "$DISTRO_BRANCH" == -redhat-* ]]; then
>> [ $# != 0 ] && sudo -p "$prompt" yum --assumeyes install --
>> "$@" |&
>> # Pass input to output, fail if a missing package is
>> reported
>> # TODO Remove and switch to DNF once
>
>I see that avoiding copy-pasting here would produce more complicated code,
>so
>it's perhaps OK. However, it would be good then to copy-paste the comment
>along with the TODO as well, and then perhaps add a note and a TODO
>regarding
>BZ1215208.
>
We will still need awk trick for old style yum on el{6,7}. Therefore
I moved TODO to the 1st branch.
>>>
>>>Well, yeah, the bug is Fedora-specific, but we still need to have a reminder
>>>to remove the hack on RHEL as well. It will get fixed eventually.
>>>
>>Correct me if I'm wrong.
>>We need to awk workaround because yum does not fail if you want to install
>>unknown packages and there is not a bug for yum and I doubt they would
>>fix/change it in stable distributions. Therefore we will need to have
>>this solution there for yum anyway.
>
>Yes. IIRC I reported this for yum originally, but the developers moved the bug
>to dnf (fixing it later) and refused to fix it in yum.
>
>I too doubt they'll fix it in stable distributions. However I expect they will
>get dnf into RHEL (which is handled by the bottom branch) eventually (RHEL8?),
>and will also fix that '--' bug. That's when a note would come in handy.
>
RHEL8 is far future atm. And I guess we will need to do more changes there :-)
>>dnf behaves correctly in this manner. However it ignores separator "--"
>>between arguments and packages which is tracked in BZ1215208.
>
>Yes, exactly.
>
Does it mean ACK to the last patch?
LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] sudo: use cache_req interface
On 02/25/2016 02:08 PM, Jakub Hrozek wrote:
On Tue, Feb 09, 2016 at 02:07:21PM +0100, Pavel Březina wrote:
First of the responders is converted -)
Sorry for the first delay in review. Before reading the code, I
submitted the patches to automated tests, so far I found:
Error: COMPILER_WARNING:
sssd-1.13.90/src/responder/sudo/sudosrv_get_sudorules.c:29: included_from:
Included from here.
sssd-1.13.90/src/responder/sudo/sudosrv_get_sudorules.c: scope_hint: In
function 'sudosrv_get_rules_done'
sssd-1.13.90/src/util/util.h:144:9: warning: 'debug_name' may be used
uninitialized in this function [-Wmaybe-uninitialized]
# sss_debug_fn(__FILE__, __LINE__, __FUNCTION__, \
# ^
sssd-1.13.90/src/responder/sudo/sudosrv_get_sudorules.c:225:17: note:
'debug_name' was declared here
# const char *debug_name;
# ^
# 142| int __debug_macro_level = level; \
# 143| if (DEBUG_IS_SET(__debug_macro_level)) { \
# 144|-> sss_debug_fn(__FILE__, __LINE__, __FUNCTION__, \
# 145|__debug_macro_level, \
# 146|format, ##__VA_ARGS__); \
But feel free to only fix the issue in your branch for now, at least until
all the automated downstream tests finish.
I think it is false positive, well it should be anyway, but here's the
diff squashed into forth patch:
diff --git a/src/responder/sudo/sudosrv_get_sudorules.c
b/src/responder/sudo/sudosrv_get_sudorules.c
index 1861e3f..04345f7 100644
--- a/src/responder/sudo/sudosrv_get_sudorules.c
+++ b/src/responder/sudo/sudosrv_get_sudorules.c
@@ -222,7 +222,7 @@ static errno_t sudosrv_cached_rules(TALLOC_CTX *mem_ctx,
{
unsigned int flags = SYSDB_SUDO_FILTER_NONE;
struct sysdb_attrs **rules;
-const char *debug_name;
+const char *debug_name = "unknown";
uint32_t num_rules;
errno_t ret;
const char *attrs[] = {SYSDB_OBJECTCLASS,
Another solution would be to add default branch into the switch in this
function.
From 4e06f2614d75c3ffa06f95820586fccc91109547 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
Date: Mon, 8 Feb 2016 14:02:59 +0100
Subject: [PATCH 01/19] sudo: remove unused structure sudo_dp_request
---
src/responder/sudo/sudosrv_private.h | 5 -
1 file changed, 5 deletions(-)
diff --git a/src/responder/sudo/sudosrv_private.h b/src/responder/sudo/sudosrv_private.h
index 186ed2cb5114d00524b41b801b5f32bac50f7153..f602f6a83cd52a54ce180d31febd1d991f32 100644
--- a/src/responder/sudo/sudosrv_private.h
+++ b/src/responder/sudo/sudosrv_private.h
@@ -79,11 +79,6 @@ struct sudo_dom_ctx {
bool check_provider;
};
-struct sudo_dp_request {
-struct cli_ctx *cctx;
-struct sss_domain_info *domain;
-};
-
struct sss_cmd_table *get_sudo_cmds(void);
errno_t sudosrv_cmd_done(struct sudo_cmd_ctx *cmd_ctx, int ret);
--
2.1.0
From 0b4b4239b92b949d181c51158e53359abfcaf8fd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
Date: Mon, 8 Feb 2016 14:30:16 +0100
Subject: [PATCH 02/19] sudo: use cache_req for initgroups
This is just blind code change, the next patch will improve it so
for example we don't do initgroups during query-parsing phase.
Resolves:
https://fedorahosted.org/sssd/ticket/1126
---
src/responder/sudo/sudosrv_cmd.c | 29 +--
src/responder/sudo/sudosrv_get_sudorules.c | 292 ++---
src/responder/sudo/sudosrv_private.h | 13 +-
src/responder/sudo/sudosrv_query.c | 114 ---
4 files changed, 47 insertions(+), 401 deletions(-)
diff --git a/src/responder/sudo/sudosrv_cmd.c b/src/responder/sudo/sudosrv_cmd.c
index dd636e949200dd49c1422a5789e9328dc4b25fb0..c68a6980ba817071e3766a0ddf7c158499d47aec 100644
--- a/src/responder/sudo/sudosrv_cmd.c
+++ b/src/responder/sudo/sudosrv_cmd.c
@@ -221,7 +221,7 @@ static int sudosrv_cmd(enum sss_sudo_type type, struct cli_ctx *cli_ctx)
goto done;
}
-req = sudosrv_parse_query_send(cmd_ctx, cli_ctx->rctx,
+req = sudosrv_parse_query_send(cmd_ctx, cmd_ctx->sudo_ctx,
query_body, query_len);
if (req == NULL) {
ret = ENOMEM;
@@ -239,8 +239,6 @@ done:
static void sudosrv_cmd_parse_query_done(struct tevent_req *req)
{
struct sudo_cmd_ctx *cmd_ctx = NULL;
-struct sudo_dom_ctx *dom_ctx = NULL;
-struct sudo_ctx *sudo_ctx = NULL;
errno_t ret;
cmd_ctx = tevent_req_callback_data(req, struct sudo_cmd_ctx);
@@ -254,8 +252,6 @@ static void sudosrv_cmd_parse_query_done(struct tevent_req *req)
goto done;
}
-cmd_ctx->check_next = cmd_ctx->domain == NULL;
-
switch (cmd_ctx->type) {
case SSS_SUDO_DEFAULTS:
DEBUG(SSSDBG_FUNC_DATA, "Requesting default options "
@@ -269,28 +265,7 @@ static void sudosrv_cmd_parse_query_done(struct tevent_req *req)
break;
}
-/* create domain ctx */
-
-dom_ctx = talloc_zero(cmd_ctx, struct sudo_dom_ctx);
-if (dom_ctx ==
[SSSD] Re: [PATCH] CI: Use yum-deprecated instead of dnf
On 03/01/2016 10:46 AM, Lukas Slebodnik wrote:
On (29/02/16 20:30), Nikolai Kondrashov wrote:
On 02/29/2016 07:27 PM, Lukas Slebodnik wrote:
On (29/02/16 18:54), Nikolai Kondrashov wrote:
keeping the "if" branches consistent and somewhat easier to interpret.
However, if you and others are more comfortable reading and using regex(3)
regexes, then it's fine.
+[ $# != 0 ] && sudo -p "$prompt" \
+yum-deprecated --assumeyes install -- "$@" |&
+awk 'BEGIN {s=0}
+ /^No package .* available.$/ {s=1}
+ {print}
+ END {exit s}'
+elif [[ "$DISTRO_BRANCH" == -redhat-* ]]; then
[ $# != 0 ] && sudo -p "$prompt" yum --assumeyes install -- "$@" |&
# Pass input to output, fail if a missing package is reported
# TODO Remove and switch to DNF once
I see that avoiding copy-pasting here would produce more complicated code, so
it's perhaps OK. However, it would be good then to copy-paste the comment
along with the TODO as well, and then perhaps add a note and a TODO regarding
BZ1215208.
We will still need awk trick for old style yum on el{6,7}. Therefore
I moved TODO to the 1st branch.
Well, yeah, the bug is Fedora-specific, but we still need to have a reminder
to remove the hack on RHEL as well. It will get fixed eventually.
Correct me if I'm wrong.
We need to awk workaround because yum does not fail if you want to install
unknown packages and there is not a bug for yum and I doubt they would
fix/change it in stable distributions. Therefore we will need to have
this solution there for yum anyway.
Yes. IIRC I reported this for yum originally, but the developers moved the bug
to dnf (fixing it later) and refused to fix it in yum.
I too doubt they'll fix it in stable distributions. However I expect they will
get dnf into RHEL (which is handled by the bottom branch) eventually (RHEL8?),
and will also fix that '--' bug. That's when a note would come in handy.
dnf behaves correctly in this manner. However it ignores separator "--"
between arguments and packages which is tracked in BZ1215208.
Yes, exactly.
Nick
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] CI: Use yum-deprecated instead of dnf
On (29/02/16 20:30), Nikolai Kondrashov wrote:
>On 02/29/2016 07:27 PM, Lukas Slebodnik wrote:
>>On (29/02/16 18:54), Nikolai Kondrashov wrote:
>>>On 02/29/2016 06:31 PM, Lukas Slebodnik wrote:
The rest of the required packages CI will attempt to install itself, using
-the distribution's package manager invoked through sudo.
+the distribution's package manager invoked through sudo. We need to use
+yum-deprecated instead of yum on Fedora >= 22 due to bug in dnf BZ1215208.
A sudo rule can be employed to selectively avoid password prompts on Red
Hat
distros:
ALL=(ALL:ALL) NOPASSWD: /usr/bin/yum --assumeyes install -- *
+ ALL=(ALL:ALL) NOPASSWD: /usr/bin/yum-deprecated --assumeyes
install -- *
>>>
>>>It would be good to mention the different rule required on RHEL here (CI
>>>still
>>>invokes "yum" there). Otherwise the instructions would be wrong and
>>>confusing.
>>>
>>It looks like I'm brainwashed today. I'm out of ideas for reasonable sentence.
>>Could you suggest something.
>
>Sure, let's try this:
>
>The rest of the required packages CI will attempt to install itself, using
>the distribution's package manager invoked through sudo.
>
>A sudo rule can be employed to selectively avoid password prompts on RHEL
>distros:
>
> ALL=(ALL:ALL) NOPASSWD: /usr/bin/yum --assumeyes install -- *
>
>on Fedora distros:
>
># We need to use yum-deprecated on Fedora because of BZ1215208.
> ALL=(ALL:ALL) NOPASSWD: /usr/bin/yum-deprecated --assumeyes
> install -- *
>
Thank you.
>>>keeping the "if" branches consistent and somewhat easier to interpret.
>>>However, if you and others are more comfortable reading and using regex(3)
>>>regexes, then it's fine.
>>>
+[ $# != 0 ] && sudo -p "$prompt" \
+yum-deprecated --assumeyes install -- "$@" |&
+awk 'BEGIN {s=0}
+ /^No package .* available.$/ {s=1}
+ {print}
+ END {exit s}'
+elif [[ "$DISTRO_BRANCH" == -redhat-* ]]; then
[ $# != 0 ] && sudo -p "$prompt" yum --assumeyes install -- "$@"
|&
# Pass input to output, fail if a missing package is reported
# TODO Remove and switch to DNF once
>>>
>>>I see that avoiding copy-pasting here would produce more complicated code, so
>>>it's perhaps OK. However, it would be good then to copy-paste the comment
>>>along with the TODO as well, and then perhaps add a note and a TODO regarding
>>>BZ1215208.
>>>
>>We will still need awk trick for old style yum on el{6,7}. Therefore
>>I moved TODO to the 1st branch.
>
>Well, yeah, the bug is Fedora-specific, but we still need to have a reminder
>to remove the hack on RHEL as well. It will get fixed eventually.
>
Correct me if I'm wrong.
We need to awk workaround because yum does not fail if you want to install
unknown packages and there is not a bug for yum and I doubt they would
fix/change it in stable distributions. Therefore we will need to have
this solution there for yum anyway.
dnf behaves correctly in this manner. However it ignores separator "--"
between arguments and packages which is tracked in BZ1215208.
LS
>From 031a1ff5c4753181937aac0a80cae4550697bfce Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik
Date: Mon, 29 Feb 2016 10:41:50 +0100
Subject: [PATCH] CI: Use yum-deprecated instead of dnf
/usr/bin/yum is provided by the dnf-yum package and call /usr/bin/dnf
on new fedora distributions. We should directly use old style yum
which was renamed to /usr/bin/yum-deprecated and is still part of
the yum package.
---
contrib/ci/README.md | 7 ++-
contrib/ci/distro.sh | 14 +++---
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/contrib/ci/README.md b/contrib/ci/README.md
index
50b73ec354f92e9909a57a82ec72ba1a8516aa1c..6bd2fd92c1bf2c0abe93a8ecb8432b6a7889e492
100644
--- a/contrib/ci/README.md
+++ b/contrib/ci/README.md
@@ -36,11 +36,16 @@ package and on Debian in `lsb-release`.
The rest of the required packages CI will attempt to install itself, using
the distribution's package manager invoked through sudo.
-A sudo rule can be employed to selectively avoid password prompts on Red Hat
+A sudo rule can be employed to selectively avoid password prompts on RHEL
distros:
ALL=(ALL:ALL) NOPASSWD: /usr/bin/yum --assumeyes install -- *
+on Fedora distros:
+
+# We need to use yum-deprecated on Fedora because of BZ1215208.
+ ALL=(ALL:ALL) NOPASSWD: /usr/bin/yum-deprecated --assumeyes install
-- *
+
and Debian-based distros:
ALL=(ALL:ALL) NOPASSWD: /usr/bin/apt-get --yes install -- *
diff --git a/contrib/ci/distro.sh b/contrib/ci/distro.sh
index
da797d02f4b110f9e2c074fc2c97f092ae7200af..374e55696d3f2519151b73ff0fc397c04ff48325
100644
--- a/contrib/ci/distro.sh
+++ b/contrib/ci/distro.sh
@@ -50,11 +50,19 @@ function distro_pkg_install()
{
decl
