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?
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.
Honza
--
Jan Cholasta