openldap: add support to build the server

PATCH New Andreas Ehmanns 2 participants 6 messages Dec 17, 2015 – Jan 13, 2016

lore

Share & tools

lore

Andreas Ehmanns

author

2015-12-17 20:41 UTC #0 lore raw

PATCH 1/1 openldap: add support to build the server

Signed-off-by Andreas Ehmanns <[email protected]>

---
 package/openldap/Config.in   |  8 ++++++-
 package/openldap/S75slapd    | 51 ++++++++++++++++++++++++++++++++++++++++++++
 package/openldap/openldap.mk | 17 ++++++++++++++-
 3 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100644 package/openldap/S75slapd

package/openldap/Config.in +7 -1

index 3085a5c..30c0f65 100644
@@ -6,12 +6,18 @@ config BR2_PACKAGE_OPENLDAP
 	  OpenLDAP Software is an open source implementation of
 	  the Lightweight Directory Access Protocol.
 
-	  This only installs client-side support.
+	  This only installs library support.
 
 	  http://www.openldap.org/
 
 if BR2_PACKAGE_OPENLDAP
 
+config BR2_PACKAGE_OPENLDAP_SERVER
+	bool "openldap server binary"
+	select BR2_PACKAGE_BERKELEYDB
+	help
+	  Installs the OpenLDAP server slapd
+
 config BR2_PACKAGE_OPENLDAP_CLIENTS
 	bool "openldap client binaries"
 	default y

package/openldap/S75slapd new file +51 -0

51 lines not on this page · raw message

package/openldap/openldap.mk +16 -1

index 17bf991..bcb285a 100644
@@ -12,6 +12,17 @@ OPENLDAP_LICENSE_FILES = LICENSE
 OPENLDAP_INSTALL_STAGING = YES
 OPENLDAP_DEPENDENCIES = host-pkgconf
 
+ifeq ($(BR2_PACKAGE_OPENLDAP_SERVER),y)
+define OPENLDAP_USERS
+        ldap -1 ldap -1 * /var/run/openldap - - OpenLDAP server user
+endef
+
+define OPENLDAP_COPY_INITSCRIPT
+        $(INSTALL) -D -m 755 $(@D)/S75slapd $(TARGET_DIR)/etc/init.d/S75slapd
+endef
+OPENLDAP_POST_INSTALL_TARGET_HOOKS += OPENLDAP_COPY_INITSCRIPT
+endif
+
 ifeq ($(BR2_PACKAGE_OPENSSL),y)
 OPENLDAP_TLS = openssl
 OPENLDAP_DEPENDENCIES += openssl

@@ -44,7 +55,6 @@ OPENLDAP_CONF_ENV += ac_cv_func_memcmp_working=yes
 OPENLDAP_CONF_OPTS += \
 	--enable-syslog \
 	--disable-proctitle \
-	--disable-slapd \
 	--with-yielding-select \
 	--sysconfdir=/etc \
 	--enable-dynamic=$(if $(BR2_STATIC_LIBS),no,yes) \

@@ -52,6 +62,11 @@ OPENLDAP_CONF_OPTS += \
 	--with-mp=$(OPENLDAP_MP) \
 	CPPFLAGS="$(TARGET_CPPFLAGS) $(OPENLDAP_CPPFLAGS)"
 
+ifeq ($(BR2_PACKAGE_OPENLDAP_SERVER),)
+OPENLDAP_CONF_OPTS += \
+        --disable-slapd
+endif
+
 # Somehow, ${STRIP} does not percolates through to the shtool script
 # used to install the executables; thus, that script tries to run the
 # executable it is supposed to install, resulting in an error.

Signature · 1 line

2.1.4

Thomas Petazzoni

2015-12-29 11:19 UTC #1 lore raw

PATCH 1/1 openldap: add support to build the server

Andreas,

Thanks for this patch. I tried it, but there are a number of remaining issues to be resolved I believe.

First, if you disable BR2_PACKAGE_OPENLDAP_CLIENTS,
the /etc/openldap/slapd.conf file gets removed in a post installation hook, so slapd cannot start.

If you fix this, then the path to the pidfile (and argsfile) in slapd.conf are wrong, because they point to /var/run/, to which the ldap user has not write access.

If you fix this again, when you start slapd, it complains:

bdb_db_open: warning - no DB_CONFIG file found in directory /var/openldap-data: (2). Expect poor performance for suffix "dc=my-domain,dc=com".

It should probably be fixed by using DB_CONFIG.example as DB_CONFIG in /var/openldap-data/.

Some more comments below.

package/openldap/S75slapd line 18

Quoted 8 lines · Andreas Ehmanns

package/openldap/S75slapd new file +8 -0

+case "$1" in
+    start)
+        if [ ! -d /var/run/openldap ]; then
+            install -d -o ldap -g ldap -m 755 /var/run/openldap
+        fi
+
+        if [ ! -d /var/openldap-data ]; then
+            install -d -o ldap -g ldap -m 755 /var/openldap-data

This directory should be 700 according to the slapd documentation:

==
# The database directory MUST exist prior to running slapd AND

# should only be accessible by the slapd and slap tools.
# Mode 700 recommended.
directory       %LOCALSTATEDIR%/openldap-data
==

package/openldap/S75slapd line 21

Quoted 3 lines

package/openldap/S75slapd new file +3 -0
+        else
+            chown -R ldap:ldap /var/openldap-data
+        fi

It is not clear why you need this. /var is a persistent directory, so I believe all you need here is an unconditional:

chown -R ldap:ldap /var/openldap-data

Setting the permission to 700 can be done by a OPENLDAP_PERMISSIONS variable in the .mk file. Ideally, we would also be able to define the user/group, but we currently can't do this by referencing symbolic user/groups, only by explicit UID/GID, and we don't know the UID/GID that will be allocated to the ldap user/group. So I think we should:

1/ Set the permission in OPENLDAP_PERMISSIONS
2/ Set the owner/group in the S75slapd script

package/openldap/S75slapd line 24

Quoted 3 lines

package/openldap/S75slapd new file +3 -0

+
+        printf "Starting $DESC: $NAME: "
+        start-stop-daemon -S -b -n $NAME -a $DAEMON -- $ARGS

You can add:

-p /var/run/slapd/slapd.pid

Why do you pass -n ? And why do you use -a instead of -x ?

See S50dropbear in the Buildroot sources for a good example of an init script.

package/openldap/S75slapd line 29

Quoted 5 lines

package/openldap/S75slapd new file +5 -0

+        echo "done."
+        ;;
+    stop)
+        printf "Stopping $DESC: $NAME: "
+        start-stop-daemon -K -n $NAME

Same here.

Also add the "-q" option

package/openldap/S75slapd line 40

Quoted 11 lines

package/openldap/S75slapd new file +11 -0

+        echo "done."
+        ;;
+    restart)
+        printf "Restarting $DESC: $NAME: "
+        $0 stop
+        $0 start
+        echo "done."
+        ;;
+    reload)
+        printf "Reloading $DESC: $NAME: "
+        killall -HUP $(basename ${DAEMON})

I think it's better to use the pid file here, no?

	   kill -HUP $(cat /var/run/slapd/slapd.pid)

Quoted 16 lines

package/openldap/S75slapd new file +11 -0

+        echo "done."
+        ;;
+    *)
+        echo "Usage: $0 {start|stop|restart|reload}"
+        exit 1
+        ;;
+esac
+
+exit 0
+
+

package/openldap/openldap.mk +0 -0

index 17bf991..bcb285a 100644

package/openldap/openldap.mk OPENLDAP_LICENSE_FILES = LICENSE · line 21

Quoted 10 lines

package/openldap/openldap.mk +7 -0

@@ -12,6 +12,17 @@ OPENLDAP_LICENSE_FILES = LICENSE
 OPENLDAP_INSTALL_STAGING = YES
 OPENLDAP_DEPENDENCIES = host-pkgconf
 
+ifeq ($(BR2_PACKAGE_OPENLDAP_SERVER),y)
+define OPENLDAP_USERS
+        ldap -1 ldap -1 * /var/run/openldap - - OpenLDAP server user
+endef
+
+define OPENLDAP_COPY_INITSCRIPT
+        $(INSTALL) -D -m 755 $(@D)/S75slapd $(TARGET_DIR)/etc/init.d/S75slapd

$(@D)/S75slapd does not exist. So this means you never rebuilt your package :-)

$(@D) is the source directory of openldap. You want to replace this
with: $(OPENLDAP_PKGDIR)/S75slapd

package/openldap/openldap.mk OPENLDAP_LICENSE_FILES = LICENSE · line 23

Quoted 2 lines

package/openldap/openldap.mk +2 -0

+endef
+OPENLDAP_POST_INSTALL_TARGET_HOOKS += OPENLDAP_COPY_INITSCRIPT

Shouldn't be a post install target hook. Instead, do this:

define OPENLDAP_INIT_SYSV

	$(INSTALL) -D -m 755 $(OPENLDAP_PKGDIR)/S75slapd $(TARGET_DIR)/etc/init.d/S75slapd

endef

and it will automatically install the init script of the chosen init system is sysV compatible.

Quoted 14 lines

package/openldap/openldap.mk +2 -1

+endif
+
 ifeq ($(BR2_PACKAGE_OPENSSL),y)
 OPENLDAP_TLS = openssl
 OPENLDAP_DEPENDENCIES += openssl

@@ -44,7 +55,6 @@ OPENLDAP_CONF_ENV += ac_cv_func_memcmp_working=yes
 OPENLDAP_CONF_OPTS += \
 	--enable-syslog \
 	--disable-proctitle \
-	--disable-slapd \
 	--with-yielding-select \
 	--sysconfdir=/etc \
 	--enable-dynamic=$(if $(BR2_STATIC_LIBS),no,yes) \

package/openldap/openldap.mk OPENLDAP_CONF_OPTS += \ · line 68

Quoted 7 lines

package/openldap/openldap.mk +4 -0

@@ -52,6 +62,11 @@ OPENLDAP_CONF_OPTS += \
 	--with-mp=$(OPENLDAP_MP) \
 	CPPFLAGS="$(TARGET_CPPFLAGS) $(OPENLDAP_CPPFLAGS)"
 
+ifeq ($(BR2_PACKAGE_OPENLDAP_SERVER),)
+OPENLDAP_CONF_OPTS += \
+        --disable-slapd
+endif

Please do:

ifeq ($(BR2_PACKAGE_OPENLDAP_SERVER),y)
OPENLDAP_CONF_OPTS += --enable-slapd
else
OPENLDAP_CONF_OPTS += --disable-slapd
endif

Which is a bit more explicit.

Could you rework your patch to solve those different issues, and send an updated version?

Thanks!

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Andreas Ehmanns

author

2016-01-03 14:07 UTC #2 lore raw

PATCH 1/1 openldap: add support to build the server

Thomas,
thanks for your detailed answer. Actually I'm on holiday but will be back on 11th of January.
Then I will have a look at all your comments, fix the issues and prepare a new patch.

Regards,
Andreas

Am 29.12.2015 um 12:19 schrieb Thomas Petazzoni:

Quoted 176 lines

Andreas,
Thanks for this patch. I tried it, but there are a number of remaining issues to be resolved I believe.
First, if you disable BR2_PACKAGE_OPENLDAP_CLIENTS,
the /etc/openldap/slapd.conf file gets removed in a post installation hook, so slapd cannot start.
If you fix this, then the path to the pidfile (and argsfile) in slapd.conf are wrong, because they point to /var/run/, to which the ldap user has not write access.
If you fix this again, when you start slapd, it complains:
bdb_db_open: warning - no DB_CONFIG file found in directory /var/openldap-data: (2). Expect poor performance for suffix "dc=my-domain,dc=com".
It should probably be fixed by using DB_CONFIG.example as DB_CONFIG in /var/openldap-data/.
Some more comments below.
package/openldap/S75slapd line 18
Quoted 8 lines · Andreas Ehmanns
package/openldap/S75slapd new file +8 -0
+case "$1" in
+    start)
+        if [ ! -d /var/run/openldap ]; then
+            install -d -o ldap -g ldap -m 755 /var/run/openldap
+        fi
+
+        if [ ! -d /var/openldap-data ]; then
+            install -d -o ldap -g ldap -m 755 /var/openldap-data
This directory should be 700 according to the slapd documentation:
==
# The database directory MUST exist prior to running slapd AND
# should only be accessible by the slapd and slap tools.
# Mode 700 recommended.
directory       %LOCALSTATEDIR%/openldap-data
==
package/openldap/S75slapd line 21
Quoted 3 lines
package/openldap/S75slapd new file +3 -0
+        else
+            chown -R ldap:ldap /var/openldap-data
+        fi
It is not clear why you need this. /var is a persistent directory, so I believe all you need here is an unconditional:
chown -R ldap:ldap /var/openldap-data
Setting the permission to 700 can be done by a OPENLDAP_PERMISSIONS variable in the .mk file. Ideally, we would also be able to define the user/group, but we currently can't do this by referencing symbolic user/groups, only by explicit UID/GID, and we don't know the UID/GID that will be allocated to the ldap user/group. So I think we should:
1/ Set the permission in OPENLDAP_PERMISSIONS
2/ Set the owner/group in the S75slapd script
package/openldap/S75slapd line 24
Quoted 3 lines
package/openldap/S75slapd new file +3 -0
+
+        printf "Starting $DESC: $NAME: "
+        start-stop-daemon -S -b -n $NAME -a $DAEMON -- $ARGS
You can add:
-p /var/run/slapd/slapd.pid
Why do you pass -n ? And why do you use -a instead of -x ?
See S50dropbear in the Buildroot sources for a good example of an init script.
package/openldap/S75slapd line 29
Quoted 5 lines
package/openldap/S75slapd new file +5 -0
+        echo "done."
+        ;;
+    stop)
+        printf "Stopping $DESC: $NAME: "
+        start-stop-daemon -K -n $NAME
Same here.
Also add the "-q" option
package/openldap/S75slapd line 40
Quoted 11 lines
package/openldap/S75slapd new file +11 -0
+        echo "done."
+        ;;
+    restart)
+        printf "Restarting $DESC: $NAME: "
+        $0 stop
+        $0 start
+        echo "done."
+        ;;
+    reload)
+        printf "Reloading $DESC: $NAME: "
+        killall -HUP $(basename ${DAEMON})
I think it's better to use the pid file here, no?
	   kill -HUP $(cat /var/run/slapd/slapd.pid)
Quoted 16 lines
package/openldap/S75slapd new file +11 -0
+        echo "done."
+        ;;
+    *)
+        echo "Usage: $0 {start|stop|restart|reload}"
+        exit 1
+        ;;
+esac
+
+exit 0
+
+
package/openldap/openldap.mk +0 -0
index 17bf991..bcb285a 100644
package/openldap/openldap.mk OPENLDAP_LICENSE_FILES = LICENSE · line 21
Quoted 10 lines
package/openldap/openldap.mk +7 -0
@@ -12,6 +12,17 @@ OPENLDAP_LICENSE_FILES = LICENSE
  OPENLDAP_INSTALL_STAGING = YES
  OPENLDAP_DEPENDENCIES = host-pkgconf
 
+ifeq ($(BR2_PACKAGE_OPENLDAP_SERVER),y)
+define OPENLDAP_USERS
+        ldap -1 ldap -1 * /var/run/openldap - - OpenLDAP server user
+endef
+
+define OPENLDAP_COPY_INITSCRIPT
+        $(INSTALL) -D -m 755 $(@D)/S75slapd $(TARGET_DIR)/etc/init.d/S75slapd
$(@D)/S75slapd does not exist. So this means you never rebuilt your package :-)
$(@D) is the source directory of openldap. You want to replace this
with: $(OPENLDAP_PKGDIR)/S75slapd
package/openldap/openldap.mk OPENLDAP_LICENSE_FILES = LICENSE · line 23
Quoted 2 lines
package/openldap/openldap.mk +2 -0
+endef
+OPENLDAP_POST_INSTALL_TARGET_HOOKS += OPENLDAP_COPY_INITSCRIPT
Shouldn't be a post install target hook. Instead, do this:
define OPENLDAP_INIT_SYSV
	$(INSTALL) -D -m 755 $(OPENLDAP_PKGDIR)/S75slapd $(TARGET_DIR)/etc/init.d/S75slapd
endef
and it will automatically install the init script of the chosen init system is sysV compatible.
Quoted 14 lines
package/openldap/openldap.mk +2 -1
+endif
+
  ifeq ($(BR2_PACKAGE_OPENSSL),y)
  OPENLDAP_TLS = openssl
  OPENLDAP_DEPENDENCIES += openssl
@@ -44,7 +55,6 @@ OPENLDAP_CONF_ENV += ac_cv_func_memcmp_working=yes
  OPENLDAP_CONF_OPTS += \
  	--enable-syslog \
  	--disable-proctitle \
-	--disable-slapd \
  	--with-yielding-select \
  	--sysconfdir=/etc \
  	--enable-dynamic=$(if $(BR2_STATIC_LIBS),no,yes) \
package/openldap/openldap.mk OPENLDAP_CONF_OPTS += \ · line 68
Quoted 7 lines
package/openldap/openldap.mk +4 -0
@@ -52,6 +62,11 @@ OPENLDAP_CONF_OPTS += \
  	--with-mp=$(OPENLDAP_MP) \
  	CPPFLAGS="$(TARGET_CPPFLAGS) $(OPENLDAP_CPPFLAGS)"
 
+ifeq ($(BR2_PACKAGE_OPENLDAP_SERVER),)
+OPENLDAP_CONF_OPTS += \
+        --disable-slapd
+endif
Please do:
ifeq ($(BR2_PACKAGE_OPENLDAP_SERVER),y)
OPENLDAP_CONF_OPTS += --enable-slapd
else
OPENLDAP_CONF_OPTS += --disable-slapd
endif
Which is a bit more explicit.
Could you rework your patch to solve those different issues, and send an updated version?
Thanks!
Thomas

Andreas Ehmanns

author

2016-01-12 21:02 UTC #3 lore raw

PATCH 1/1 openldap: add support to build the server

In reply to Thomas Petazzoni

Thomas,
I reworked the patch and incorporated your findings. Please have a look at my comments below and let me know what you think.

Am 29.12.2015 um 12:19 schrieb Thomas Petazzoni:

Quoted 13 lines

Andreas,
Thanks for this patch. I tried it, but there are a number of remaining issues to be resolved I believe.
First, if you disable BR2_PACKAGE_OPENLDAP_CLIENTS,
the /etc/openldap/slapd.conf file gets removed in a post installation hook, so slapd cannot start.
If you fix this, then the path to the pidfile (and argsfile) in slapd.conf are wrong, because they point to /var/run/, to which the ldap user has not write access.

Quoted 8 lines

If you fix this again, when you start slapd, it complains:
bdb_db_open: warning - no DB_CONFIG file found in directory /var/openldap-data: (2). Expect poor performance for suffix "dc=my-domain,dc=com".
It should probably be fixed by using DB_CONFIG.example as DB_CONFIG in /var/openldap-data/.

My aim was to add the OpenLDAP server as provided by the package and only make the changes necessary to allow the server to start up without terminating.
slapd.conf is the default configuration provided by the package which is a good starting point for people to setup their own configuration and database. Of course everyone using the LDAP server has to make its own configuration and database setup but this can't be provided or preconfigured by buildroot.

Quoted 4 lines

Some more comments below.
On Thu, 17 Dec 2015 21:41:19 +0100, Andreas Ehmanns wrote:

Quoted 9 lines

package/openldap/S75slapd line 18
Quoted 8 lines
package/openldap/S75slapd new file +8 -0
+case "$1" in
+    start)
+        if [ ! -d /var/run/openldap ]; then
+            install -d -o ldap -g ldap -m 755 /var/run/openldappackage
+        fi
+
+        if [ ! -d /var/openldap-data ]; then
+            install -d -o ldap -g ldap -m 755 /var/openldap-data
This directory should be 700 according to the slapd documentation:

O.k., changed it.

Quoted 8 lines

==
# The database directory MUST exist prior to running slapd AND
# should only be accessible by the slapd and slap tools.
# Mode 700 recommended.
directory       %LOCALSTATEDIR%/openldap-data
==

Quoted 5 lines

package/openldap/S75slapd line 21
Quoted 3 lines
package/openldap/S75slapd new file +3 -0
+        else
+            chown -R ldap:ldap /var/openldap-data
+        fi
It is not clear why you need this. /var is a persistent directory, so I believe all you need here is an unconditional:

o.k., changed it.

Quoted 12 lines

chown -R ldap:ldap /var/openldap-data
Setting the permission to 700 can be done by a OPENLDAP_PERMISSIONS variable in the .mk file. Ideally, we would also be able to define the user/group, but we currently can't do this by referencing symbolic user/groups, only by explicit UID/GID, and we don't know the UID/GID that will be allocated to the ldap user/group. So I think we should:
1/ Set the permission in OPENLDAP_PERMISSIONS
2/ Set the owner/group in the S75slapd script

Quoted 6 lines

package/openldap/S75slapd line 24
Quoted 3 lines
package/openldap/S75slapd new file +3 -0
+
+        printf "Starting $DESC: $NAME: "
+        start-stop-daemon -S -b -n $NAME -a $DAEMON -- $ARGS
You can add:
-p /var/run/slapd/slapd.pid

Slapd manages its own pid file. Why should start-stop-daemon create an additional pid file

Quoted 2 lines

Why do you pass -n ? And why do you use -a instead of -x ?

O.k., changed -a to -x
I thought that I need -n to be able to do a kill when shutting down the server when NOT using pid file from start-stop-daemon. This was my understanding from other init scripts. Am I wrong?

Quoted 4 lines

See S50dropbear in the Buildroot sources for a good example of an init script.

Quoted 8 lines

package/openldap/S75slapd line 29
Quoted 5 lines
package/openldap/S75slapd new file +5 -0
+        echo "done."
+        ;;
+    stop)
+        printf "Stopping $DESC: $NAME: "
+        start-stop-daemon -K -n $NAME
Same here.
Also add the "-q" option

O.k., added -q option

Quoted 12 lines

package/openldap/S75slapd line 40

Quoted 12 lines

package/openldap/S75slapd new file +11 -0

+        echo "done."
+        ;;
+    restart)
+        printf "Restarting $DESC: $NAME: "
+        $0 stop
+        $0 start
+        echo "done."
+        ;;
+    reload)
+        printf "Reloading $DESC: $NAME: "
+        killall -HUP $(basename ${DAEMON})

Quoted 3 lines

I think it's better to use the pid file here, no?
	   kill -HUP $(cat /var/run/slapd/slapd.pid)

See comment above. Slapd is managing its own pid file.

Quoted 30 lines

Quoted 17 lines
package/openldap/S75slapd new file +11 -0
+        echo "done."
+        ;;
+    *)
+        echo "Usage: $0 {start|stop|restart|reload}"
+        exit 1
+        ;;
+esac
+
+exit 0
+
+
package/openldap/openldap.mk +0 -0
index 17bf991..bcb285a 100644
package/openldap/openldap.mk OPENLDAP_LICENSE_FILES = LICENSE · line 21
Quoted 10 lines
package/openldap/openldap.mk +7 -0
@@ -12,6 +12,17 @@ OPENLDAP_LICENSE_FILES = LICENSE
  OPENLDAP_INSTALL_STAGING = YES
  OPENLDAP_DEPENDENCIES = host-pkgconf
 
+ifeq ($(BR2_PACKAGE_OPENLDAP_SERVER),y)
+define OPENLDAP_USERS
+        ldap -1 ldap -1 * /var/run/openldap - - OpenLDAP server user
+endef
+
+define OPENLDAP_COPY_INITSCRIPT
+        $(INSTALL) -D -m 755 $(@D)/S75slapd $(TARGET_DIR)/etc/init.d/S75slapd
$(@D)/S75slapd does not exist. So this means you never rebuilt your package :-)

Quoted 2 lines

$(@D) is the source directory of openldap. You want to replace this
with: $(OPENLDAP_PKGDIR)/S75slapd

Fixed an re-tested.

Quoted 8 lines

package/openldap/openldap.mk OPENLDAP_LICENSE_FILES = LICENSE · line 23
Quoted 3 lines
package/openldap/openldap.mk +2 -0
+endef
+OPENLDAP_POST_INSTALL_TARGET_HOOKS += OPENLDAP_COPY_INITSCRIPT
Shouldn't be a post install target hook. Instead, do this:
define OPENLDAP_INIT_SYSV
	$(INSTALL) -D -m 755 $(OPENLDAP_PKGDIR)/S75slapd $(TARGET_DIR)/etc/init.d/S75slapd
endef

O.k., changed it.

Quoted 27 lines

and it will automatically install the init script of the chosen init system is sysV compatible.

Quoted 14 lines

package/openldap/openldap.mk +2 -1

+endif
+
  ifeq ($(BR2_PACKAGE_OPENSSL),y)
  OPENLDAP_TLS = openssl
  OPENLDAP_DEPENDENCIES += openssl

@@ -44,7 +55,6 @@ OPENLDAP_CONF_ENV += ac_cv_func_memcmp_working=yes
  OPENLDAP_CONF_OPTS += \
  	--enable-syslog \
  	--disable-proctitle \
-	--disable-slapd \
  	--with-yielding-select \
  	--sysconfdir=/etc \
  	--enable-dynamic=$(if $(BR2_STATIC_LIBS),no,yes) \

package/openldap/openldap.mk OPENLDAP_CONF_OPTS += \ · line 68

Quoted 7 lines

package/openldap/openldap.mk +4 -0

@@ -52,6 +62,11 @@ OPENLDAP_CONF_OPTS += \
  	--with-mp=$(OPENLDAP_MP) \
  	CPPFLAGS="$(TARGET_CPPFLAGS) $(OPENLDAP_CPPFLAGS)"
 
+ifeq ($(BR2_PACKAGE_OPENLDAP_SERVER),)
+OPENLDAP_CONF_OPTS += \
+        --disable-slapd
+endif

Please do:

Quoted 7 lines

ifeq ($(BR2_PACKAGE_OPENLDAP_SERVER),y)
OPENLDAP_CONF_OPTS += --enable-slapd
else
OPENLDAP_CONF_OPTS += --disable-slapd
endif
Which is a bit more explicit.

Agree. Changed it.

Quoted 7 lines

Could you rework your patch to solve those different issues, and send an updated version?
Thanks!
Thomas

Instead of a patch I attached you the two files under discussion (openldap.mk and S75slapd) to make review easier. If you agree with the changes I will send a full patch using git.

Regards,
Andreas
-------------- next part --------------
#!/bin/sh
DAEMON=/usr/libexec/slapd
NAME=slapd
DESC="OpenLDAP server"
CONF=/etc/openldap/slapd.conf

ARGS="-u ldap -g ldap"

set -e

case "$1" in
start)
if [ ! -d /var/run/openldap ]; then
install -d -o ldap -g ldap -m 755 /var/run/openldap
fi

chown -R ldap:ldap /var/openldap-data

printf "Starting $DESC: $NAME: " start-stop-daemon -S -q -n $NAME -x $DAEMON -- $ARGS echo "done."
;;
stop)
printf "Stopping $DESC: $NAME: " start-stop-daemon -K -n $NAME
echo "done."
;;
restart)
printf "Restarting $DESC: $NAME: "
$0 stop
$0 start
echo "done."
;;
reload)
printf "Reloading $DESC: $NAME: "
killall -HUP $(basename ${DAEMON})
echo "done."
;;
*)
echo "Usage: $0 {start|stop|restart|reload}" exit 1
;;
esac

exit 0

-------------- next part --------------
A non-text attachment was scrubbed...
Name: openldap.mk
Type: text/x-makefile
Size: 2618 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20160112/b34f7114/attachment.bin>

Thomas Petazzoni

2016-01-12 21:09 UTC #4 lore raw

PATCH 1/1 openldap: add support to build the server

Andreas,

Quoted 2 lines · Andreas Ehmanns

I reworked the patch and incorporated your findings. Please have a look at my comments below and let me know what you think.

Thanks! See below my comments.

Quoted 12 lines

Quoted 4 lines
If you fix this, then the path to the pidfile (and argsfile) in slapd.conf are wrong, because they point to /var/run/, to which the ldap user has not write access.
Quoted 8 lines
If you fix this again, when you start slapd, it complains:
bdb_db_open: warning - no DB_CONFIG file found in directory /var/openldap-data: (2). Expect poor performance for suffix "dc=my-domain,dc=com".
It should probably be fixed by using DB_CONFIG.example as DB_CONFIG in /var/openldap-data/.

Quoted 8 lines

My aim was to add the OpenLDAP server as provided by the package and only make the changes necessary to allow the server to start up without terminating.
slapd.conf is the default configuration provided by the package which is a good starting point for people to setup their own configuration and database. Of course everyone using the LDAP server has to make its own configuration and database setup but this can't be provided or preconfigured by buildroot.

Right, but in general we try in Buildroot to provide a sane/minimal default configuration that "works" out of the box. It is a bit weird to have such a warning when the slapd daemon starts. But OK, it's not a very big issue either, we can always leave it as it is for now for this aspect.

Quoted 3 lines

Quoted 1 line
-p /var/run/slapd/slapd.pid
Slapd manages its own pid file. Why should start-stop-daemon create an additional pid file

start-stop-daemon will not create an additional pid file with just the -p option. Only if you pass the -m option in addition to -p. With -p, start-stop-daemon will only verify that the process has created the pid file. From the start-stop-daemon manpage:

-p, --pidfile pid-file
Check whether a process has created the file pid-file. Note:
using this matching option alone might cause unintended pro? cesses to be acted on, if the old process terminated without being able to remove the pid-file.

-m, --make-pidfile
Used when starting a program that does not create its own pid file. This option will make start-stop-daemon create the file referenced with --pidfile and place the pid into it just before executing the process. Note, the file will only be removed when stopping the program if --remove-pidfile is used. Note: This feature may not work in all cases. Most notably when the program being executed forks from its main process. Because of this, it is usually only useful when combined with the --background option.

Quoted 5 lines

Quoted 1 line
Why do you pass -n ? And why do you use -a instead of -x ?
O.k., changed -a to -x
I thought that I need -n to be able to do a kill when shutting down the server when NOT using pid file from start-stop-daemon. This was my understanding from other init scripts. Am I wrong?

If you specify -p, I think doing the name-based check with -n is useless.

Quoted 5 lines

Quoted 4 lines
Quoted 1 line
+ killall -HUP $(basename ${DAEMON})
I think it's better to use the pid file here, no?
	   kill -HUP $(cat /var/run/slapd/slapd.pid)
See comment above. Slapd is managing its own pid file.

And? It doesn't prevent us from using it, right?

Thanks!

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Andreas Ehmanns

author

2016-01-13 17:31 UTC #5 lore raw

PATCH 1/1 openldap: add support to build the server

Thomas,
thanks for your comments. I will incorporate them, test it and send the patch in the next two days.

Regards,
Andreas

Am 12.01.2016 um 22:09 schrieb Thomas Petazzoni:

Quoted 78 lines

Andreas,
Quoted 2 lines · Andreas Ehmanns
I reworked the patch and incorporated your findings. Please have a look at my comments below and let me know what you think.
Thanks! See below my comments.
Quoted 12 lines
Quoted 4 lines
If you fix this, then the path to the pidfile (and argsfile) in slapd.conf are wrong, because they point to /var/run/, to which the ldap user has not write access.
Quoted 8 lines
If you fix this again, when you start slapd, it complains:
bdb_db_open: warning - no DB_CONFIG file found in directory /var/openldap-data: (2). Expect poor performance for suffix "dc=my-domain,dc=com".
It should probably be fixed by using DB_CONFIG.example as DB_CONFIG in /var/openldap-data/.
Quoted 8 lines
My aim was to add the OpenLDAP server as provided by the package and only make the changes necessary to allow the server to start up without terminating.
slapd.conf is the default configuration provided by the package which is a good starting point for people to setup their own configuration and database. Of course everyone using the LDAP server has to make its own configuration and database setup but this can't be provided or preconfigured by buildroot.
Right, but in general we try in Buildroot to provide a sane/minimal default configuration that "works" out of the box. It is a bit weird to have such a warning when the slapd daemon starts. But OK, it's not a very big issue either, we can always leave it as it is for now for this aspect.
Quoted 3 lines
Quoted 1 line
-p /var/run/slapd/slapd.pid
Slapd manages its own pid file. Why should start-stop-daemon create an additional pid file
start-stop-daemon will not create an additional pid file with just the -p option. Only if you pass the -m option in addition to -p. With -p, start-stop-daemon will only verify that the process has created the pid file. From the start-stop-daemon manpage:
-p, --pidfile pid-file
Check whether a process has created the file pid-file. Note:
using this matching option alone might cause unintended pro? cesses to be acted on, if the old process terminated without being able to remove the pid-file.
-m, --make-pidfile
Used when starting a program that does not create its own pid file. This option will make start-stop-daemon create the file referenced with --pidfile and place the pid into it just before executing the process. Note, the file will only be removed when stopping the program if --remove-pidfile is used. Note: This feature may not work in all cases. Most notably when the program being executed forks from its main process. Because of this, it is usually only useful when combined with the --background option.
Quoted 5 lines
Quoted 1 line
Why do you pass -n ? And why do you use -a instead of -x ?
O.k., changed -a to -x
I thought that I need -n to be able to do a kill when shutting down the server when NOT using pid file from start-stop-daemon. This was my understanding from other init scripts. Am I wrong?
If you specify -p, I think doing the name-based check with -n is useless.
Quoted 5 lines
Quoted 4 lines
Quoted 1 line
+ killall -HUP $(basename ${DAEMON})
I think it's better to use the pid file here, no?
	   kill -HUP $(cat /var/run/slapd/slapd.pid)
See comment above. Slapd is managing its own pid file.
And? It doesn't prevent us from using it, right?
Thanks!
Thomas