On 24.2.2012 20:54, Stephen Gallagher wrote:
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.
I have included the change in my SSH cleanup patchset.
Honza
--
Jan Cholasta
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
