[Pkg-openldap-devel] RFR: Preliminary patch for cn=config support: new installs

[Mathias Gug]

Hi,

Here is new version of the patch. It includes upgrades. I've refactored
the code to not support slapd.conf anymore with the aim that the
maintainer scripts would only support slapd.d/. I've also removed all of
the code related to ldbm migration.

I've answered Steve's comments below.

Let me know what you think about it.

On Mon, Jul 14, 2008 at 07:23:32PM +0100, Steve Langasek wrote:
> On Fri, Jul 11, 2008 at 06:01:11PM -0400, Mathias Gug wrote:
> --- 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).

Fixed.

> === 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@ ? :)

No real reason. I was following convention used elsewhere. It's also to
make sure that the ldif cannot be used as is since it's a template.

> +	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?

True. However, this is only called when creating a new directory, which
requires dpkg-reconfigure or on install. The .config script will be run
and the admin password should be available. Adding a check to make sure
that the adminpw is not empty could be added.

> +		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?

I've replaced all noisy_slapadd with capture_diagnostics.

> 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.

I've removed the code.

> 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).

Refactored and got rid of the code.

-- 
Mathias Gug
Ubuntu Developer  http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cnconfig-migration_217.patch
Type: text/x-diff
Size: 30168 bytes
Desc: not available
Url : http://lists.alioth.debian.org/pipermail/pkg-openldap-devel/attachments/20080718/38ade2c4/attachment-0001.patch