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.