[Pkg-openldap-devel] RFR: Preliminary patch for cn=config support: new installs
Steve Langasek
vorlon at debian.org
Mon Jul 14 18:23:32 UTC 2008
On Fri, Jul 11, 2008 at 06:01:11PM -0400, Mathias Gug wrote:
> I've attached a preliminary patch to add support for the config backend
> for a new install. Upgrades are not supported yet. I'd like to get some
> feedback on it.
> The postinst script is using bash instead of sh as some string
> extraction is done when creating a new directory (the First letter of
> the backend needs to be capitalized when adding the ldif part related to
> a database configuration).
> Let me know what you think about it.
=== modified file 'debian/slapd.default'
--- debian/slapd.default 2007-12-21 05:46:23 +0000
+++ debian/slapd.default 2008-07-11 16:46:11 +0000
@@ -1,5 +1,5 @@
# Default location of the slapd.conf file. If empty, use the compiled-in
-# default (/etc/ldap/slapd.conf). If using the cn=config backend to store
+# default (/etc/ldap/slapd.d/). If using the cn=config backend to store
# configuration in LDIF, set this variable to the directory containing the
# cn=config data.
SLAPD_CONF=
Suggest:
# Location of the slapd configuration to use. If using the cn=config
# backend to store configuration in LDIF, set this variable to the
# directory containing the cn=config data; otherwise set it to the location
# of your slapd.conf file. If empty, use the compiled-in default
# (/etc/ldap/slapd.d).
=== modified file 'debian/slapd.init'
--- debian/slapd.init 2008-05-25 18:27:14 +0000
+++ debian/slapd.init 2008-07-11 19:06:51 +0000
Self-evidently ok :)
=== added file 'debian/slapd.init.ldif'
--- debian/slapd.init.ldif 1970-01-01 00:00:00 +0000
+++ debian/slapd.init.ldif 2008-07-11 19:06:51 +0000
Why this particular file name? I don't understand the significance of
"slapd.init.ldif".
There's an option to make slapd autoconvert between slapd.conf and
cn=config, isn't there? ISTR Howard mentioning this at UDS. If so, it
would be helpful to see the diff between this LDIF file and the
auto-generated stuff, for review.
+# Config db settings
+dn: olcDatabase=config,cn=config
+objectClass: olcDatabaseConfig
+olcDatabase: config
+olcRootDN: cn=admin,cn=config
+ at olcRootPW@
why not olcRootPW: @olcRootPW@ ? :)
+# Ensure read access to the base for things like
+# supportedSASLMechanisms. Without this you may
+# have problems with SASL not knowing what
+# mechanisms are available and the like.
+# Note that this is covered by the 'access to *'
+# ACL below too but if you change that as people
+# are wont to do you'll still need this if you
+# want SASL (and possible other things) to work
+# happily.
+olcAccess: to dn.base="" by * read
This seems to be set as an attribute on the database - is that right?
dn.base="" isn't part of this database definition, surely?
Otherwise, this looks fine.
=== modified file 'debian/slapd.postinst'
--- debian/slapd.postinst 2008-02-18 21:51:45 +0000
+++ debian/slapd.postinst 2008-07-11 21:26:57 +0000
=== modified file 'debian/slapd.postrm'
--- debian/slapd.postrm 2007-12-21 21:05:19 +0000
+++ debian/slapd.postrm 2008-07-11 21:26:57 +0000
Ok.
+ if use_config_backend ; then
+ conf_template="/usr/share/slapd/slapd.init.ldif"
+ # Get the admin password for the cn=config tree
+ db_get slapd/internal/adminpw
+ # adminpw can have / character which would break sed
+ # further down.
+ adminpass=$(echo $RET | sed -e 's|/|\\/|g')
Is this reliable? Don't we clear the adminpw out of debconf elsewhere
within the maintainer scripts?
+ # Need to have a version of the backend name whith the first
+ # letter capitalize (ex: olcBdbConfig or olcHdbConfig) to set
+ # the correct objectClass attribute in the db configuration.
+ local backend1="$(echo ${backend:0:1} | tr a-z A-Z)${backend:1}"
Horrid, but I haven't been able to figure out a better way. :)
+ sed <"$conf_template" \
+ -e "s/@olcRootPW@/olcRootPW: $adminpass/g" \
+ -e "s/@backend@/$backend/g" \
+ -e "s/@Backend@/$backend1/g" \
+ -e "s/@SUFFIX@/$basedn/g" \
+ -e "s/@ADMIN@/cn=admin,$basedn/g" \
+ | noisy_slapadd -F ${conf_new} -b "cn=config"
Does noisy_slapadd provide an advantage here, vs. using
"capture_diagnostics slapadd" and pointing the user to the source LDIF file
in case of an error?
+ else
<snip>
I think this part is ok for test purposes, but I believe that one of the
advantages of this switch is that the complexity of our maintainer scripts
goes *way* down... and we don't get this advantage if we have to keep
supporting slapd.conf as well. So I would actually prefer to not have the
cn=config package go to unstable with this stuff enabled - I think we should
sort out any upgrade problems, and then cut the slapd.conf handling out
completely and if users still want to use slapd.conf they're on their own
going forward.
@@ -827,11 +850,16 @@
db_get slapd/allow_ldap_v2
if [ "$RET" != "true" ]; then return 0; fi
- # If $SLAPD_CONF is a directory, the user is using cn=config. Assume
- # they know what they're doing.
- if [ -d "$SLAPD_CONF" ]; then return 0; fi
-
echo -n " Enabling LDAPv2 support... " >&2
+
+ # If $SLAPD_CONF is a directory, the user is using cn=config.
+ if [ -d "$SLAPD_CONF" ]; then
+ if ! grep -q -E '^olcAllows:[[:space:]]+bind_v2' "${SLAPD_CONF}/cn=config.ldif"; then
+ echo "olcAllows: bind_v2" >> "${SLAPD_CONF}/cn=config.ldif"
+ fi
+ return 0
+ fi
+
\o/
@@ -882,8 +910,8 @@
# Apply the same permissions as on a reference file to another file.
# Usage: apply_permissions <original> <new>
- chmod --reference="$1" "$2"
- chown --reference="$1" "$2"
+ chmod -R --reference="$1" "$2"
+ chown -R --reference="$1" "$2"
}
# }}}
install_new_slapd_conf() { # {{{
Are there any cases where this will actually be needed? AFAICS we'll never
be replacing a whole cn=config tree with another, so assign_permissions
should in fact never be called for us.
... or rather, there is one place where it will be called
(migrate_checkpoint_slurpd on upgrades) where I expect it to fail
completely. And in the case of a new install, install_new_slapd_conf()
would bypass assign_permissions anyway because the directory won't exist
yet.
So I think this bit of the patch is unnecessary.
@@ -903,8 +931,14 @@
if [ -e "$SLAPD_CONF" ]; then
assign_permissions "$SLAPD_CONF" "$conf_new"
elif [ -n "$SLAPD_GROUP" ] ; then
- chgrp "$SLAPD_GROUP" "$conf_new"
- chmod 640 "$conf_new"
+ # slapd needs to be able to change the slapd.d directory
+ # if cn=config is used.
+ if use_config_backend and [ -n "${SLAPD_USER}" ]; then
+ chown -R "${SLAPD_USER}" $conf_new
+ fi
+ chgrp -R "$SLAPD_GROUP" "$conf_new"
+ chmod -R u+rw,g+r,g-w,o-rwx "$conf_new"
+ find $conf_new -type d -exec chmod ug+x '{}' ';'
fi
mv "$conf_new" "$SLAPD_CONF"
}
Hmm, I think the chown shouldn't be contingent on whether SLAPD_GROUP is
set, it should only depend on whether SLAPD_USER itself is set.
I think you also want u=rwX,g=rX,o-rwx for a more complete set of perms
that's also expressed more simply (bypassing the 'find' command).
Overall, it looks like a good start, but I'd like to see upgrade support
included as well before committing this to the packaging trunk.
Cheers,
--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developer http://www.debian.org/
slangasek at ubuntu.com vorlon at debian.org
More information about the Pkg-openldap-devel
mailing list