On Fri, 2012-02-24 at 14:17 +0100, Jan Cholasta wrote: > On 22.2.2012 09:13, Jan Zelený wrote: > >> On Tue, 2012-02-21 at 16:48 +0100, Jakub Hrozek wrote: > >>> On Tue, Feb 21, 2012 at 10:49:54AM +0100, Jan Zelený wrote: > >>>>> On Thu, Feb 09, 2012 at 06:05:30PM +0100, Jan Zelený wrote: > >>>>>>>> On Thu, 2012-02-09 at 13:46 +0100, Jakub Hrozek wrote: > >>>>>>>>> On Tue, Feb 07, 2012 at 01:40:39PM +0100, Jan Zelený wrote: > >>>>>>>>>> With all these changes happening in last two weeks, the IPA > >>>>>>>>>> hosts code was messy at best. This patch sorts out some of > >>>>>>>>>> the mess. I already did some testing: SELinux and HBAC > >>>>>>>>>> seem to be working correctly. Honza, please take a look > >>>>>>>>>> and test this patch with your SSH-related code. That is > >>>>>>>>>> the last code that uses host fetching. > >>>>>>>>>> > >>>>>>>>>> If this patch makes it to master, I plan to do some more > >>>>>>>>>> cleanup in the HBAC code which is closely related to the > >>>>>>>>>> code this patch cleans. > >>>>>>>>>> > >>>>>>>>>> Thanks > >>>>>>>>>> Jan > >>>>>>>>> > >>>>>>>>> None of the new options is documented or has a configAPI > >>>>>>>>> entry. If that's intentional to avoid breaking string > >>>>>>>>> freeze, then there should be a ticket to track adding them. > >>>>>>>> > >>>>>>>> They still need to be in the sssd.api.d/sssd-ipa.conf file. We > >>>>>>>> can omit the strings from SSSDConfig.py to avoid breaking > >>>>>>>> string freeze I guess. But the API needs to not choke on them > >>>>>>>> if they're specified in the config file. > >>>>>>>> > >>>>>>>> Is this change necessary for the 1.8.0 release, or can we just > >>>>>>>> fix it properly (with option changes) in 1.9.0? > >>>>>>> > >>>>>>> I guess we can leave it for 1.9, the current state isn't breaking > >>>>>>> anything AFAIK. > >>>>>>> > >>>>>>>>> ...or... > >>>>>>>>> > >>>>>>>>> I was actually thinking if we want to document the IPA > >>>>>>>>> attribute maps at all. The options clutter the manual page > >>>>>>>>> and their value is questionable. Would anyone oppose > >>>>>>>>> *removing* all the attribute maps from the IPA manual page? > >>>>>>>>> Or at the very least moving them into some section down at > >>>>>>>>> the bottom where noone would be confused by the options. > >>>>>>>> > >>>>>>>> I'm in favor of removing any manpage entries that are of > >>>>>>>> neglibible utility (in 1.9.0, not 1.8.0). > >>>>>>> > >>>>>>> Agreed. I'll file a ticket for that. > >>>>>>> > >>>>>>> I'll send updated patch for master in a moment > >>>>>> > >>>>>> Filed ticket https://fedorahosted.org/sssd/ticket/1187 > >>>>>> > >>>>>> Patches are in attachment. > >>>>>> > >>>>>> Jan > >>>>> > >>>>> The patch works fine but have you considered moving "sdap_attr_map > >>>>> *selinuxuser_map", host_map and hostgroup_map into struct > >>>>> ipa_options? > >>>>> > >>>>> It seems odd that the search bases are in struct ipa_options but maps > >>>>> are in sdap_options. Also ipa_options seems like a better fit for > >>>>> IPA-specific data. > >>>> > >>>> Done, patches are in attachment. > >>>> > >>>> Jan > >>> > >>> Ack > >> > >> Nack. This patch fails to compile with the error: > >> > >> /bin/sh ./libtool --tag=CC --mode=compile gcc -DHAVE_CONFIG_H -I. > >> -I/home/sgallagh/workspace/sssd -Wall -Iinclude -I.. > >> -I/home/sgallagh/workspace/sssd/include > >> -I/home/sgallagh/workspace/sssd/src/sss_client > >> -I/home/sgallagh/workspace/sssd/src -Iinclude -I. > >> -I/usr/include/dbus-1.0 -I/usr/lib64/dbus-1.0/include -DLIBDIR= > >> \"/usr/lib64\" -DVARDIR=\"/var\" -DSHLIBEXT=\"\" -DSSSD_LIBEXEC_PATH= > >> \"/usr/libexec/sssd\" -DSSSD_INTROSPECT_PATH=\"\" -DSSSD_CONF_DIR= > >> \"/etc/sssd\" -DSSS_NSS_SOCKET_NAME=\"/var/lib/sss/pipes/nss\" > >> -DSSS_PAM_SOCKET_NAME=\"/var/lib/sss/pipes/pam\" > >> -DSSS_PAM_PRIV_SOCKET_NAME=\"/var/lib/sss/pipes/private/pam\" > >> -DSSS_SUDO_SOCKET_NAME=\"/var/lib/sss/pipes/sudo\" > >> -DSSS_AUTOFS_SOCKET_NAME=\"/var/lib/sss/pipes/autofs\" > >> -DSSS_SSH_SOCKET_NAME=\"/var/lib/sss/pipes/ssh\" -DLOCALEDIR= > >> \"/usr/share/locale\" -Wall -Wshadow -Wstrict-prototypes > >> -Wpointer-arith -Wcast-qual -Wcast-align -Wwrite-strings > >> -Werror-implicit-function-declaration -fno-strict-aliasing -ggdb3 -O0 > >> -Wall -Wextra -Wno-unused-parameter -Wno-sign-compare -Wformat-security > >> -Werror -MT src/providers/ipa/libsss_ipa_la-ipa_hostid.lo -MD -MP -MF > >> src/providers/ipa/.deps/libsss_ipa_la-ipa_hostid.Tpo -c -o > >> src/providers/ipa/libsss_ipa_la-ipa_hostid.lo `test -f > >> 'src/providers/ipa/ipa_hostid.c' || echo > >> '/home/sgallagh/workspace/sssd/'`src/providers/ipa/ipa_hostid.c > >> libtool: compile: gcc -DHAVE_CONFIG_H -I. > >> -I/home/sgallagh/workspace/sssd -Wall -Iinclude -I.. > >> -I/home/sgallagh/workspace/sssd/include > >> -I/home/sgallagh/workspace/sssd/src/sss_client > >> -I/home/sgallagh/workspace/sssd/src -Iinclude -I. > >> -I/usr/include/dbus-1.0 -I/usr/lib64/dbus-1.0/include -DLIBDIR= > >> \"/usr/lib64\" -DVARDIR=\"/var\" -DSHLIBEXT=\"\" -DSSSD_LIBEXEC_PATH= > >> \"/usr/libexec/sssd\" -DSSSD_INTROSPECT_PATH=\"\" -DSSSD_CONF_DIR= > >> \"/etc/sssd\" -DSSS_NSS_SOCKET_NAME=\"/var/lib/sss/pipes/nss\" > >> -DSSS_PAM_SOCKET_NAME=\"/var/lib/sss/pipes/pam\" > >> -DSSS_PAM_PRIV_SOCKET_NAME=\"/var/lib/sss/pipes/private/pam\" > >> -DSSS_SUDO_SOCKET_NAME=\"/var/lib/sss/pipes/sudo\" > >> -DSSS_AUTOFS_SOCKET_NAME=\"/var/lib/sss/pipes/autofs\" > >> -DSSS_SSH_SOCKET_NAME=\"/var/lib/sss/pipes/ssh\" -DLOCALEDIR= > >> \"/usr/share/locale\" -Wall -Wshadow -Wstrict-prototypes -Wpointer-arith > >> -Wcast-qual -Wcast-align -Wwrite-strings > >> -Werror-implicit-function-declaration -fno-strict-aliasing -ggdb3 -O0 > >> -Wall -Wextra -Wno-unused-parameter -Wno-sign-compare -Wformat-security > >> -Werror -MT src/providers/ipa/libsss_ipa_la-ipa_hostid.lo -MD -MP -MF > >> src/providers/ipa/.deps/libsss_ipa_la-ipa_hostid.Tpo > >> -c /home/sgallagh/workspace/sssd/src/providers/ipa/ipa_hostid.c -fPIC > >> -DPIC -o src/providers/ipa/.libs/libsss_ipa_la-ipa_hostid.o > >> /home/sgallagh/workspace/sssd/src/providers/ipa/ipa_hostid.c: In > >> function 'hosts_get_connect_done': > >> /home/sgallagh/workspace/sssd/src/providers/ipa/ipa_hostid.c:231:42: > >> error: 'struct sdap_options' has no member named 'host_map' > >> /home/sgallagh/workspace/sssd/src/providers/ipa/ipa_hostid.c:232:42: > >> error: 'struct sdap_options' has no member named 'hostgroup_map' > >> make[1]: *** [src/providers/ipa/libsss_ipa_la-ipa_hostid.lo] Error 1 > >> make[1]: Leaving directory `/home/sgallagh/workspace/sssd/x86_64' > >> make: *** [check-recursive] Error 1 > >> > >> > >> > >> While fixing this, please rebase it atop my patch "[PATCH] LDAP: Only > >> use paging control on requests for multiple entries" so we don't have to > >> worry about merging it later. > > > > Thanks for catching that, my build script doesn't use experimental features > > at > > the moment. Rebased and fixed. > > > > Jan > > > > Two things: > > In ipa_common.c: > > + { "ipa_host_name", "cn", SYSDB_NAME, NULL }, > + { "ipa_host_fqdn", "fqdn", SYSDB_FQDN, NULL }, > > In IPA fqdn is the primary key of hosts, shouldn't it be mapped to > SYSDB_NAME instead of cn? >
It probably should have been, but we didn't do it that way initially and it'll be too much trouble to change it (particularly for existing deployments). > In ipa_hostid.c: > > subreq = ipa_host_info_send(state, state->ev, state->sysdb, > sdap_id_op_handle(state->op), > - ctx->opts, state->name, > - state->attrs, ctx->opts->host_map, > - IPA_OPTS_HOST, false, > + state->ctx->sdap_id_ctx->opts, state->name, > + state->ctx->ipa_opts->host_map, > + state->ctx->ipa_opts->hostgroup_map, > state->ctx->host_search_bases); > > Hostgroups aren't needed for SSH, so I think the > state->ctx->ipa_opts->hostgroup_map bit should be replaced with NULL. This is a nice-to-have for the future, but I don't think it's worth holding up this patch. I'm going to push it. Please open a ticket to track this change.
signature.asc
Description: This is a digitally signed message part
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
