diff mbox

[1/1] openldap: add support to build the server

Message ID 20160120235928.4feebe12@free-electrons.com
State Not Applicable
Headers show

Commit Message

Thomas Petazzoni Jan. 20, 2016, 10:59 p.m. UTC
Dear Andreas Ehmanns,

On Fri, 15 Jan 2016 10:40:09 +0100, Andreas Ehmanns wrote:
> Signed-off-by: Andreas Ehmanns <universeII@gmx.de>
> ---
>  package/Config.in            |  2 +-
>  package/openldap/Config.in   |  8 +++++++-
>  package/openldap/S75slapd    | 47 ++++++++++++++++++++++++++++++++++++++++++++
>  package/openldap/openldap.mk | 29 ++++++++++++++++++++++++++-
>  4 files changed, 83 insertions(+), 3 deletions(-)
>  create mode 100644 package/openldap/S75slapd

Thanks for respining. However, I am sorry, but it still doesn't work.
Problems encountered:

 1/ The /etc/openldap/slapd.conf file doesn't exist. This happens when
    you build with BR2_PACKAGE_OPENLDAP_CLIENTS disabled. I think I
    already mentioned this problem in a previous review of this patch.
    I fixed this problem by doing:


 3/ The sldap daemon doesn't start because it tries to write its PID
    file to /var/run/, where it doesn't have write permissions, while it
    should create it in /var/run/openldap. Here is the message I get in
    the logs:

Jan 20 22:55:48 buildroot local4.debug slapd[728]: unable to open pid file "/var/run/slapd.pid": 13 (Permission denied)
Jan 20 22:55:48 buildroot local4.debug slapd[728]: slapd stopped.

Also, your init script logging is not consistent with what we do in
other packages. You do:

        printf "Starting $DESC: $NAME: "
        start-stop-daemon -S -q -p $PIDFILE -x $DAEMON -- $ARGS
        echo "done."

While we normally do:

	printf "Starting dropbear sshd: "
	start-stop-daemon -S -q -p /var/run/dropbear.pid \
		--exec /usr/sbin/dropbear -- $DROPBEAR_ARGS
	[ $? = 0 ] && echo "OK" || echo "FAIL"

Also, there is something weird: when the daemon fails to start, it
doesn't show "done." (with your code) or "FAIL" (with my suggestion).
Can you have a look ?

Thanks,

Thomas

Comments

universe II Jan. 22, 2016, 9:50 a.m. UTC | #1
Dear Thomas,
this is very weird. Did you really use the latest patch? On my target 
the LDAP server is starting without any problems. It seems that we use 
different init scripts or something else is wrong. Please have a look at 
my answers below:

Am 20.01.2016 um 23:59 schrieb Thomas Petazzoni:
> Dear Andreas Ehmanns,
>
> On Fri, 15 Jan 2016 10:40:09 +0100, Andreas Ehmanns wrote:
>> Signed-off-by: Andreas Ehmanns <universeII@gmx.de>
>> ---
>>   package/Config.in            |  2 +-
>>   package/openldap/Config.in   |  8 +++++++-
>>   package/openldap/S75slapd    | 47 ++++++++++++++++++++++++++++++++++++++++++++
>>   package/openldap/openldap.mk | 29 ++++++++++++++++++++++++++-
>>   4 files changed, 83 insertions(+), 3 deletions(-)
>>   create mode 100644 package/openldap/S75slapd
> Thanks for respining. However, I am sorry, but it still doesn't work.
> Problems encountered:
>
>   1/ The /etc/openldap/slapd.conf file doesn't exist. This happens when
>      you build with BR2_PACKAGE_OPENLDAP_CLIENTS disabled. I think I
>      already mentioned this problem in a previous review of this patch.
>      I fixed this problem by doing:
Yes, you're right. I've overseen this comment in your previous email. 
Sorry for the inconvenience and thanks for patch.
I'll fix this.
> diff --git a/package/openldap/openldap.mk b/package/openldap/openldap.mk
> index 18509cc..fdf8c88 100644
> --- a/package/openldap/openldap.mk
> +++ b/package/openldap/openldap.mk
> @@ -91,9 +91,15 @@ OPENLDAP_CLIENTS = \
>          ldapsearch
>   define OPENLDAP_REMOVE_CLIENTS
>          $(RM) -f $(foreach p,$(OPENLDAP_CLIENTS),$(TARGET_DIR)/usr/bin/$(p))
> -       $(RM) -rf $(TARGET_DIR)/etc/openldap
>   endef
>   OPENLDAP_POST_INSTALL_TARGET_HOOKS += OPENLDAP_REMOVE_CLIENTS
>   endif
>   
> +define OPENLDAP_REMOVE_UNNEEDED_FILES
> +       $(RM) -f $(TARGET_DIR)/etc/openldap/*.default
> +       $(RM) -f $(TARGET_DIR)/etc/openldap/DB_CONFIG.example
> +endef
> +
> +OPENLDAP_POST_INSTALL_TARGET_HOOKS += OPENLDAP_REMOVE_UNNEEDED_FILES
> +
>   $(eval $(autotools-package))
>
>   2/ The /etc/openldap/slapd.conf file has permissions that do not allow
>      the slapd daemon to read it. I fixed this problem by:
When I build the rootfs this file has 644 permissions on the target and 
the ldap server starts without problems. Strange that it's different 
when I build the target rootfs and when you do it. Could it be that 
there is a unknown dependency to other packages?
Nevertheless I think it is a good idea to change the owner of this file 
to ldap:ldap.
I changed the init script as you proposed.
>
> diff --git a/package/openldap/S75slapd b/package/openldap/S75slapd
> index 0a5ff8e..8b8cf30 100644
> --- a/package/openldap/S75slapd
> +++ b/package/openldap/S75slapd
> @@ -15,6 +15,7 @@ case "$1" in
>           fi
>   
>           chown -R ldap:ldap /var/openldap-data
> +        chown ldap:ldap /etc/openldap/slapd.conf
>   
>           printf "Starting $DESC: $NAME: "
>           start-stop-daemon -S -q -p $PIDFILE -x $DAEMON -- $ARGS
>
>   3/ The sldap daemon doesn't start because it tries to write its PID
>      file to /var/run/, where it doesn't have write permissions, while it
>      should create it in /var/run/openldap. Here is the message I get in
>      the logs:
That's not correct. Looking at the init script you can see that the PID 
file is:
PIDFILE=/var/run/openldap/slapd.pid
and /var/run/openldap is owned by ldap:ldap. So the pid file can be 
created. I'm wondering why in your test the daemon tried to write to 
/var/run directly. Do you have a different init script?
>
> Jan 20 22:55:48 buildroot local4.debug slapd[728]: unable to open pid file "/var/run/slapd.pid": 13 (Permission denied)
> Jan 20 22:55:48 buildroot local4.debug slapd[728]: slapd stopped.
>
> Also, your init script logging is not consistent with what we do in
> other packages. You do:
>
>          printf "Starting $DESC: $NAME: "
>          start-stop-daemon -S -q -p $PIDFILE -x $DAEMON -- $ARGS
>          echo "done."
>
> While we normally do:
>
> 	printf "Starting dropbear sshd: "
> 	start-stop-daemon -S -q -p /var/run/dropbear.pid \
> 		--exec /usr/sbin/dropbear -- $DROPBEAR_ARGS
> 	[ $? = 0 ] && echo "OK" || echo "FAIL"
You're right. I fixed this.
>
> Also, there is something weird: when the daemon fails to start, it
> doesn't show "done." (with your code) or "FAIL" (with my suggestion).
> Can you have a look ?
Yes. The reason was a "set -e" in the init script. I removed it and the 
logging is fine now.
>
> Thanks,
>
> Thomas

Please review my comments and let me know, what you think. Especially 
the different behavior with the init script and the config file seems 
strange to me.

Regards,
Andreas
Thomas Petazzoni Jan. 22, 2016, 10:03 a.m. UTC | #2
Andreas,

On Fri, 22 Jan 2016 10:50:59 +0100, Andreas Ehmanns wrote:

> this is very weird. Did you really use the latest patch? On my target 
> the LDAP server is starting without any problems. It seems that we use 
> different init scripts or something else is wrong. Please have a look at 
> my answers below:

I think I used the latest version, i.e
http://patchwork.ozlabs.org/patch/567988/. But of course I can't be
sure since I've removed the commit from my Git tree.


> >   1/ The /etc/openldap/slapd.conf file doesn't exist. This happens when
> >      you build with BR2_PACKAGE_OPENLDAP_CLIENTS disabled. I think I
> >      already mentioned this problem in a previous review of this patch.
> >      I fixed this problem by doing:
> Yes, you're right. I've overseen this comment in your previous email. 
> Sorry for the inconvenience and thanks for patch.
> I'll fix this.

Thanks!

> >   2/ The /etc/openldap/slapd.conf file has permissions that do not allow
> >      the slapd daemon to read it. I fixed this problem by:
> When I build the rootfs this file has 644 permissions on the target and 
> the ldap server starts without problems. Strange that it's different 
> when I build the target rootfs and when you do it. Could it be that 
> there is a unknown dependency to other packages?

Hum, weird. I'll retest again.

> Nevertheless I think it is a good idea to change the owner of this file 
> to ldap:ldap.
> I changed the init script as you proposed.

Well, if the permissions are sufficient for the daemon to read the
configuration file, then there's no need to add stuff in the init
script to change the ownership of the file.

> >   3/ The sldap daemon doesn't start because it tries to write its PID
> >      file to /var/run/, where it doesn't have write permissions, while it
> >      should create it in /var/run/openldap. Here is the message I get in
> >      the logs:
> That's not correct. Looking at the init script you can see that the PID 
> file is:
> PIDFILE=/var/run/openldap/slapd.pid

Yes, but this is only passed to start-stop-daemon, and
start-stop-daemon does *not* create the PID file. This -p $PIDFILE
option only allows start-stop-daemon to verify that the PID file has
been created by the daemon.

Where is openldap told that its init script should be
in /var/run/openldap/ ?

> > Also, there is something weird: when the daemon fails to start, it
> > doesn't show "done." (with your code) or "FAIL" (with my suggestion).
> > Can you have a look ?
> Yes. The reason was a "set -e" in the init script. I removed it and the 
> logging is fine now.

Ah, yes, indeed. Thanks for the explanation!

Thomas
universe II Jan. 22, 2016, 10:58 a.m. UTC | #3
Thomas,
slapd writes the PID file to the location defined in 
/etc/openldap/slapd.conf

The configuration file defines "pidfile" as /var/run/openldap/slapd.pid. 
Maybe we have different conf files when we build the rootfs?
I had a look at a previous build without the new patch of the 
openldap.mk (where we delete unused files now).
Without this new feature you find two files in /etc/openldap:
   - slapd.conf with 644 permissions and pidfile defined as 
/var/run/openldap/slapd.pid
   - slapd.conf.default with 600 permission and pidfile defined as 
/var/run/slapd.pid
It seems that in your builds you have slapd.conf.default as conf file 
and in my builds I have slapd.conf used by slapd.

How could that happen?

Regards,
Andreas


Am 22.01.2016 um 11:03 schrieb Thomas Petazzoni:
> Andreas,
>
> On Fri, 22 Jan 2016 10:50:59 +0100, Andreas Ehmanns wrote:
>
>> this is very weird. Did you really use the latest patch? On my target
>> the LDAP server is starting without any problems. It seems that we use
>> different init scripts or something else is wrong. Please have a look at
>> my answers below:
> I think I used the latest version, i.e
> http://patchwork.ozlabs.org/patch/567988/. But of course I can't be
> sure since I've removed the commit from my Git tree.
>
>
>>>    1/ The /etc/openldap/slapd.conf file doesn't exist. This happens when
>>>       you build with BR2_PACKAGE_OPENLDAP_CLIENTS disabled. I think I
>>>       already mentioned this problem in a previous review of this patch.
>>>       I fixed this problem by doing:
>> Yes, you're right. I've overseen this comment in your previous email.
>> Sorry for the inconvenience and thanks for patch.
>> I'll fix this.
> Thanks!
>
>>>    2/ The /etc/openldap/slapd.conf file has permissions that do not allow
>>>       the slapd daemon to read it. I fixed this problem by:
>> When I build the rootfs this file has 644 permissions on the target and
>> the ldap server starts without problems. Strange that it's different
>> when I build the target rootfs and when you do it. Could it be that
>> there is a unknown dependency to other packages?
> Hum, weird. I'll retest again.
>
>> Nevertheless I think it is a good idea to change the owner of this file
>> to ldap:ldap.
>> I changed the init script as you proposed.
> Well, if the permissions are sufficient for the daemon to read the
> configuration file, then there's no need to add stuff in the init
> script to change the ownership of the file.
>
>>>    3/ The sldap daemon doesn't start because it tries to write its PID
>>>       file to /var/run/, where it doesn't have write permissions, while it
>>>       should create it in /var/run/openldap. Here is the message I get in
>>>       the logs:
>> That's not correct. Looking at the init script you can see that the PID
>> file is:
>> PIDFILE=/var/run/openldap/slapd.pid
> Yes, but this is only passed to start-stop-daemon, and
> start-stop-daemon does *not* create the PID file. This -p $PIDFILE
> option only allows start-stop-daemon to verify that the PID file has
> been created by the daemon.
>
> Where is openldap told that its init script should be
> in /var/run/openldap/ ?
>
>>> Also, there is something weird: when the daemon fails to start, it
>>> doesn't show "done." (with your code) or "FAIL" (with my suggestion).
>>> Can you have a look ?
>> Yes. The reason was a "set -e" in the init script. I removed it and the
>> logging is fine now.
> Ah, yes, indeed. Thanks for the explanation!
>
> Thomas
universe II Feb. 12, 2016, 9:26 a.m. UTC | #4
Dear all,
can someone please advise me how to continue with this patch?

Regards,
Andreas


Am 22.01.2016 um 11:58 schrieb Andreas Ehmanns:
> Thomas,
> slapd writes the PID file to the location defined in 
> /etc/openldap/slapd.conf
>
> The configuration file defines "pidfile" as 
> /var/run/openldap/slapd.pid. Maybe we have different conf files when 
> we build the rootfs?
> I had a look at a previous build without the new patch of the 
> openldap.mk (where we delete unused files now).
> Without this new feature you find two files in /etc/openldap:
>   - slapd.conf with 644 permissions and pidfile defined as 
> /var/run/openldap/slapd.pid
>   - slapd.conf.default with 600 permission and pidfile defined as 
> /var/run/slapd.pid
> It seems that in your builds you have slapd.conf.default as conf file 
> and in my builds I have slapd.conf used by slapd.
>
> How could that happen?
>
> Regards,
> Andreas
>
>
> Am 22.01.2016 um 11:03 schrieb Thomas Petazzoni:
>> Andreas,
>>
>> On Fri, 22 Jan 2016 10:50:59 +0100, Andreas Ehmanns wrote:
>>
>>> this is very weird. Did you really use the latest patch? On my target
>>> the LDAP server is starting without any problems. It seems that we use
>>> different init scripts or something else is wrong. Please have a 
>>> look at
>>> my answers below:
>> I think I used the latest version, i.e
>> http://patchwork.ozlabs.org/patch/567988/. But of course I can't be
>> sure since I've removed the commit from my Git tree.
>>
>>
>>>>    1/ The /etc/openldap/slapd.conf file doesn't exist. This happens 
>>>> when
>>>>       you build with BR2_PACKAGE_OPENLDAP_CLIENTS disabled. I think I
>>>>       already mentioned this problem in a previous review of this 
>>>> patch.
>>>>       I fixed this problem by doing:
>>> Yes, you're right. I've overseen this comment in your previous email.
>>> Sorry for the inconvenience and thanks for patch.
>>> I'll fix this.
>> Thanks!
>>
>>>>    2/ The /etc/openldap/slapd.conf file has permissions that do not 
>>>> allow
>>>>       the slapd daemon to read it. I fixed this problem by:
>>> When I build the rootfs this file has 644 permissions on the target and
>>> the ldap server starts without problems. Strange that it's different
>>> when I build the target rootfs and when you do it. Could it be that
>>> there is a unknown dependency to other packages?
>> Hum, weird. I'll retest again.
>>
>>> Nevertheless I think it is a good idea to change the owner of this file
>>> to ldap:ldap.
>>> I changed the init script as you proposed.
>> Well, if the permissions are sufficient for the daemon to read the
>> configuration file, then there's no need to add stuff in the init
>> script to change the ownership of the file.
>>
>>>>    3/ The sldap daemon doesn't start because it tries to write its PID
>>>>       file to /var/run/, where it doesn't have write permissions, 
>>>> while it
>>>>       should create it in /var/run/openldap. Here is the message I 
>>>> get in
>>>>       the logs:
>>> That's not correct. Looking at the init script you can see that the PID
>>> file is:
>>> PIDFILE=/var/run/openldap/slapd.pid
>> Yes, but this is only passed to start-stop-daemon, and
>> start-stop-daemon does *not* create the PID file. This -p $PIDFILE
>> option only allows start-stop-daemon to verify that the PID file has
>> been created by the daemon.
>>
>> Where is openldap told that its init script should be
>> in /var/run/openldap/ ?
>>
>>>> Also, there is something weird: when the daemon fails to start, it
>>>> doesn't show "done." (with your code) or "FAIL" (with my suggestion).
>>>> Can you have a look ?
>>> Yes. The reason was a "set -e" in the init script. I removed it and the
>>> logging is fine now.
>> Ah, yes, indeed. Thanks for the explanation!
>>
>> Thomas
>
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
>
universe II Feb. 25, 2016, 8:39 p.m. UTC | #5
Thomas,
what shall I do? Is that patch o.k.? Are there further things I should do?

Please let me know, how to process to finalize this patch.

Thanks,
Andreas

Am 22.01.2016 um 11:58 schrieb Andreas Ehmanns:
> Thomas,
> slapd writes the PID file to the location defined in 
> /etc/openldap/slapd.conf
>
> The configuration file defines "pidfile" as 
> /var/run/openldap/slapd.pid. Maybe we have different conf files when 
> we build the rootfs?
> I had a look at a previous build without the new patch of the 
> openldap.mk (where we delete unused files now).
> Without this new feature you find two files in /etc/openldap:
>   - slapd.conf with 644 permissions and pidfile defined as 
> /var/run/openldap/slapd.pid
>   - slapd.conf.default with 600 permission and pidfile defined as 
> /var/run/slapd.pid
> It seems that in your builds you have slapd.conf.default as conf file 
> and in my builds I have slapd.conf used by slapd.
>
> How could that happen?
>
> Regards,
> Andreas
>
>
> Am 22.01.2016 um 11:03 schrieb Thomas Petazzoni:
>> Andreas,
>>
>> On Fri, 22 Jan 2016 10:50:59 +0100, Andreas Ehmanns wrote:
>>
>>> this is very weird. Did you really use the latest patch? On my target
>>> the LDAP server is starting without any problems. It seems that we use
>>> different init scripts or something else is wrong. Please have a 
>>> look at
>>> my answers below:
>> I think I used the latest version, i.e
>> http://patchwork.ozlabs.org/patch/567988/. But of course I can't be
>> sure since I've removed the commit from my Git tree.
>>
>>
>>>>    1/ The /etc/openldap/slapd.conf file doesn't exist. This happens 
>>>> when
>>>>       you build with BR2_PACKAGE_OPENLDAP_CLIENTS disabled. I think I
>>>>       already mentioned this problem in a previous review of this 
>>>> patch.
>>>>       I fixed this problem by doing:
>>> Yes, you're right. I've overseen this comment in your previous email.
>>> Sorry for the inconvenience and thanks for patch.
>>> I'll fix this.
>> Thanks!
>>
>>>>    2/ The /etc/openldap/slapd.conf file has permissions that do not 
>>>> allow
>>>>       the slapd daemon to read it. I fixed this problem by:
>>> When I build the rootfs this file has 644 permissions on the target and
>>> the ldap server starts without problems. Strange that it's different
>>> when I build the target rootfs and when you do it. Could it be that
>>> there is a unknown dependency to other packages?
>> Hum, weird. I'll retest again.
>>
>>> Nevertheless I think it is a good idea to change the owner of this file
>>> to ldap:ldap.
>>> I changed the init script as you proposed.
>> Well, if the permissions are sufficient for the daemon to read the
>> configuration file, then there's no need to add stuff in the init
>> script to change the ownership of the file.
>>
>>>>    3/ The sldap daemon doesn't start because it tries to write its PID
>>>>       file to /var/run/, where it doesn't have write permissions, 
>>>> while it
>>>>       should create it in /var/run/openldap. Here is the message I 
>>>> get in
>>>>       the logs:
>>> That's not correct. Looking at the init script you can see that the PID
>>> file is:
>>> PIDFILE=/var/run/openldap/slapd.pid
>> Yes, but this is only passed to start-stop-daemon, and
>> start-stop-daemon does *not* create the PID file. This -p $PIDFILE
>> option only allows start-stop-daemon to verify that the PID file has
>> been created by the daemon.
>>
>> Where is openldap told that its init script should be
>> in /var/run/openldap/ ?
>>
>>>> Also, there is something weird: when the daemon fails to start, it
>>>> doesn't show "done." (with your code) or "FAIL" (with my suggestion).
>>>> Can you have a look ?
>>> Yes. The reason was a "set -e" in the init script. I removed it and the
>>> logging is fine now.
>> Ah, yes, indeed. Thanks for the explanation!
>>
>> Thomas
>
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
>
diff mbox

Patch

diff --git a/package/openldap/openldap.mk b/package/openldap/openldap.mk
index 18509cc..fdf8c88 100644
--- a/package/openldap/openldap.mk
+++ b/package/openldap/openldap.mk
@@ -91,9 +91,15 @@  OPENLDAP_CLIENTS = \
        ldapsearch
 define OPENLDAP_REMOVE_CLIENTS
        $(RM) -f $(foreach p,$(OPENLDAP_CLIENTS),$(TARGET_DIR)/usr/bin/$(p))
-       $(RM) -rf $(TARGET_DIR)/etc/openldap
 endef
 OPENLDAP_POST_INSTALL_TARGET_HOOKS += OPENLDAP_REMOVE_CLIENTS
 endif
 
+define OPENLDAP_REMOVE_UNNEEDED_FILES
+       $(RM) -f $(TARGET_DIR)/etc/openldap/*.default
+       $(RM) -f $(TARGET_DIR)/etc/openldap/DB_CONFIG.example
+endef
+
+OPENLDAP_POST_INSTALL_TARGET_HOOKS += OPENLDAP_REMOVE_UNNEEDED_FILES
+
 $(eval $(autotools-package))

 2/ The /etc/openldap/slapd.conf file has permissions that do not allow
    the slapd daemon to read it. I fixed this problem by:

diff --git a/package/openldap/S75slapd b/package/openldap/S75slapd
index 0a5ff8e..8b8cf30 100644
--- a/package/openldap/S75slapd
+++ b/package/openldap/S75slapd
@@ -15,6 +15,7 @@  case "$1" in
         fi
 
         chown -R ldap:ldap /var/openldap-data
+        chown ldap:ldap /etc/openldap/slapd.conf
 
         printf "Starting $DESC: $NAME: "
         start-stop-daemon -S -q -p $PIDFILE -x $DAEMON -- $ARGS