Merge lp:~benji/landscape-client/bug-1546743-postinst-creates-file-in-non-existent-directory into lp:~landscape/landscape-client/trunk

[Benji York]

This branch fixes bug 1546743 which affects fresh client installs.

The client postinst script was attempting to create a file in the data
directory, but that directory is created by the client itself so if
the client had never been run before, the file creation would fail.

This branch modifies the postinst script to create the directory if
needed.

This branch fixes bug 1546743 which affects fresh client installs.

The client postinst script was attempting to create a file in the data
directory, but that directory is created by the client itself so if
the client had never been run before, the file creation would fail.

This branch modifies the postinst script to create the directory if
needed.

Testing instructions:

- build a new set of packages (see "make package")
- stop the client on the test machine
- remove the data_path directory (found in your client configuration)
- install -common and -client packages
- verify that the packages installed without error
- verify that the data_path directory exists
- verify that the data_path directory is owned by "landscape"

[Bogdana Vereha (bogdana)]

One minor comment inline.

review: Approve

[Adam Collard (adam-collard)]

I was going to say that you should use "install -o landscape -d $DATA_PATH" instead of the separate mkdir; chown. But then I realised that all we're trying to do is make a file in a directory, so we should be using "install -D -o landscape user-update-flag $DATA_PATH/user-update-flag" and drop the test for directory existence and all the chowns

Check the man page for install if you're not aware of it, it's used extensively in debian packages to avoid just these kinds of bugs :)

review: Needs Fixing

[lp:~benji/landscape-client/bug-1546743-postinst-creates-file-in-non-existent-directory]

831. By Benji York on 2016-02-18

use install instead of bashing on files and ownership ourselves

[Benji York (benji)]

> I was going to say that you should use "install -o landscape -d $DATA_PATH"
> instead of the separate mkdir; chown. But then I realised that all we're
> trying to do is make a file in a directory, so we should be using "install -D
> -o landscape user-update-flag $DATA_PATH/user-update-flag" and drop the test
> for directory existence and all the chowns

I didn't want to create the file and then move it (since the contents are mostly irrelevant anyway) so I went with creating the directory and file separately.

I also changed another instance of check-create-chmod to use install.

[Adam Collard (adam-collard)]

Looks good, +1

review: Approve

[Benji York (benji)]

Thanks for the good reviews.

=== modified file 'debian/landscape-client.init'
--- debian/landscape-client.init	2014-07-10 18:51:49 +0000
+++ debian/landscape-client.init	2016-02-18 15:58:51 +0000
@@ -40,10 +40,7 @@
 	start)
 		check_config
 		log_daemon_msg "Starting the $NAME daemon"
-		if [ ! -e $PIDDIR ]; then
-			mkdir $PIDDIR
-			chown landscape $PIDDIR
-		fi
+		install --owner=landscape --directory $PIDDIR
 		# Cleanup leftover sockets if there's no other landscape process
 		# running. This shouldn't be usually necessary, but it can
 		# happen in case the client crashed badly and the socket points

=== modified file 'debian/landscape-client.postinst'
--- debian/landscape-client.postinst	2016-02-10 19:12:36 +0000
+++ debian/landscape-client.postinst	2016-02-18 15:58:51 +0000
@@ -123,9 +123,10 @@
         # user information.  The flag file will be removed by the client when
         # the update completes.
         DATA_PATH="`grep ^data_path /etc/landscape/client.conf | cut -d= -f2 | tr -d '[[:space:]]'`"
+        install --owner=landscape --directory $DATA_PATH
         USER_UPDATE_FLAG_FILE="$DATA_PATH/user-update-flag"
+        install --owner=landscape /dev/null $USER_UPDATE_FLAG_FILE
         echo "This file indicates that the Landscape client needs to send updated user information to the server." >> $USER_UPDATE_FLAG_FILE
-        chown landscape $USER_UPDATE_FLAG_FILE
     ;;
 
     abort-upgrade|abort-remove|abort-deconfigure)