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

Mathias Gug mathiaz at ubuntu.com
Thu Jul 31 02:53:59 UTC 2008


On Tue, Jul 29, 2008 at 07:11:05PM -0700, Steve Langasek wrote:
> Looks like progress.
> 
> I've attached an incremental patch including some fixes; detailed comments
> are available in-line below.
> 
> I've also published my patch as a bzr branch at
> <lp:~vorlon/openldap/debian-cnconfig/>.  (Note to
> the team: Mathias's work is available at
> <lp:~mathiaz/openldap/debian-cnconfig>).

I've merged most of the changes from vorlon's branch and pushed a
new version in my branch. See my comments below.

I've attached a patch against the latest version in the debian svn
repository.

> === modified file 'debian/slapd.postinst'
> --- debian/slapd.postinst	2008-07-20 02:30:18 +0000
> +++ debian/slapd.postinst	2008-07-28 20:37:35 +0000
> 
>  	if previous_version_older 2.4.7-4; then
>  		if ! migrate_checkpoint_and_slurpd; then
>  			db_input critical slapd/slurpd_obsolete || true
>  			db_go || true
>  		fi
> -		config_obsolete_schemacheck
> + 		if [ -f "${SLAPD_CONF}" ]; then
> + 			sed -i "s|^(schemacheck\s+)|#\$1|" "$SLAPD_CONF"
> + 		fi
> + 	fi
> 
> This looks like it might be a regression?  slapd.conf can have includes, and
> the schemacheck option could be buried in one of those, and if it is the
> config import will fail.  So unfortunately, I think we need to keep that
> functionality around until after the lenny release.

I've added a loop to go over the list of included files in slapd.conf
and fix the schemacheck option in them. 

> +		mkdir /etc/ldap/slapd.d/
> +		# Need to create an entry for the config database 
> +		# to set the rootdn and rootpw 
> +		# so that cn=config can be accessed.
> +		db_get slapd/internal/cfgadminpw
> 
> Well, it's unfortunate that we don't seem to be able to reliably reuse the
> existing value from the config, because that means more debconf templates to
> be translated. :/  Should it be possible in some cases to query this
> password from the slapd.conf+slapcat output, or is that too painful to
> consider?

It's possible - I was able to do such a thing when I first looked at
password handling. However it turns out that figuring out which database
should be used to extract the password value can be tricky and may
require user input.

> === added file 'debian/slapd.init.ldif'
> --- debian/slapd.init.ldif	1970-01-01 00:00:00 +0000
> +++ debian/slapd.init.ldif	2008-07-18 17:49:37 +0000
> 
> Note to self: run a comparison between a fresh install, and an upgrade from
> a stock config, to see how the resulting config dirs differ.

They differ a lot - the slapd conversion dumps a lot more defaults in
the cn=config.ldif file.

> @@ -129,18 +113,22 @@
>  # If the user wants us to dump the databases they are dumped to the 
>  # configured directory.
>  	
> -	local db suffix file dir failed
> +	local db suffix file dir failed slapcat_opts
>  
>  	database_dumping_enabled || return 0
>  
>  	dir=`database_dumping_destdir`
>  	echo >&2 "  Dumping to $dir: "
> -	parse_configuration_file
> -	for db in `get_database_list`; do
> -		suffix=`get_suffix $db`
> +	for suffix in `get_suffix`; do
> 
> As you rightly point out in your comments on get_suffix, dump_databases() is
> called from the preinst and therefore needs to understand slapd.conf.
> However, get_suffix doesn't understand slapd.conf include files; so I think
> this is another case where we need to keep around the nasty inline perl
> parser. :/
> 
> On the bright side, I think it means that get_suffix probably doesn't have
> to support slapd.conf.

I've come up with a different solution: slapd.conf support is needed in
three places that uses grep 'regex' to extract some information. I've
used a loop to go over the included files.

> @@ -645,8 +478,8 @@
>  		echo >&2 "  Moving old database directory to /var/backups:"
>  		move_old_database_away /var/lib/ldap
>  	fi
> +	create_ldap_directories
>  	create_new_slapd_conf "$basedn" "$backend"
> -	create_ldap_directories
>  	create_new_directory "$basedn" "$dc"
>  
>  	# Put the right permissions on this directory.
> 
> What's the reason for this change?

create_new_slapd_conf defines a database entry which requires the
db directory to exist otherwise slapadd fails. I've tried to split the
database creation step from the initial configuration step. However
slapadd can only be used once as pointed out by Howard [1].

[1]: http://www.openldap.org/lists/openldap-software/200807/msg00110.html

> +		cat <<-EOF
> +Loading the initial configuration from the ldif file (${init_ldif}) failed with the following
> +error while running slapadd:
>  EOF
> +		release_diagnostics "    "
>  		exit 1
>  	fi
> 
> Isn't the "failed" line redundant?

Yes.

> Why does this take out the warning about a missing
> /etc/ldap/schema/core.schema?  Should it be replaced with a warning about
> core.ldif...?

Are you refering to /etc/ldap/schema/core.ldif (which is used when
created a new configuration) or to
/etc/ldap/slapd.d/cn=config/cn=shema/core.ldif (which is used by slapd)
?

> @@ -735,49 +558,37 @@
>  	adminpass="$RET"
>   
>  	echo -n "  Creating initial LDAP directory... " >&2
> -
> -	cat <<-EOF | noisy_slapadd
> -		dn: $basedn
> -		objectClass: top
> -		objectClass: dcObject
> -		objectClass: organization
> -		o: $organization
> -		dc: $dc
> -		
> -		dn: cn=admin,$basedn
> -		objectClass: simpleSecurityObject
> -		objectClass: organizationalRole
> -		cn: admin
> -		description: LDAP administrator
> -		userPassword: {crypt}$adminpass
> -	EOF
> -
> +	init_ldif=`mktemp /tmp/slapd_init_dir.ldif.XXXXXXXXXX`
> +	echo "dn: $basedn
> +objectClass: top
> +objectClass: dcObject
> +objectClass: organization
> +o: $organization
> +dc: $dc
> +
> +dn: cn=admin,$basedn
> +objectClass: simpleSecurityObject
> +objectClass: organizationalRole
> +cn: admin
> +description: LDAP administrator
> +userPassword: $adminpass
> +" > ${init_ldif}
> +	capture_diagnostics slapadd -F "${SLAPD_CONF}" \
> +		-b "${basedn}" -l ${init_ldif} || failed=1
> +	if [ "$failed" ]; then
> +		echo failed. >&2
> +		echo >&2
> +		cat <<-EOF
> +Loading the initial directory structure from the ldif file (${init_ldif}) failed with the following
> +error while running slapadd:
> +EOF
> +		release_diagnostics "    "
> +		exit 1
> +	fi
> +	rm "${init_ldif}"
>  	echo "done." >&2
>  } 
>  # }}}
> 
> This particular change removes the need for noisy_slapadd to exist at all,
> but it also leaves behind tmp files on failure (as does the changed
> create_new_slapd_conf() function).  I think that in the case of
> create_new_directory where the ldif is small enough to fit on a screen, the
> original behavior might have been better, so we don't have to leave tmp
> files around for the user to inspect if they want to understand what
> happened.

We could probably dump the ldif file in the message and delete the
temporary file while still using the capture_diagnostics function.

> Do we actually need a separate internal question for this, or could it be
> stored in the existing slapd/internal/adminpw question?  AFAICS, with the
> current implementation they're never different, and I don't see anywhere
> that adminpw would be cleared before we're done with it.
> 
> In fact, it looks like create_new_slapd_conf doesn't even use the
> cfgadminpw, it uses slapd/internal/adminpw... :)

The main issue here is to be able to ask the user for the cn=config
password when migrating the configuration from slapd.conf to slapd.d.
I've tried to come up with a way to reuse adminpw in that case, but
didn't get anywhere. I may investigate this issue a little bit more.
Another option mentioned above is to try to reuse the existing rootpw in
the database. OTOH asking users to enter a new password makes sure that
they understand that a migration has been done and that they should
change their way of managing their slapd directory.

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


More information about the Pkg-openldap-devel mailing list