Merge lp:~therve/landscape-client/config-permissions into lp:~landscape/landscape-client/trunk

[Thomas Herve]

The branch adds a chown call in the postinst for safety, and complains if we try to start the client with a config file that the landscape user can't read. In trunk it succeeds and use default values, which is less than useful. Maybe it should also fail if the config is not enough to run (ie can't register?), but I think it's enough for now.

[Geoff Teale (tealeg)]

+1 All good.

review: Approve

[Alberto Donato (ack)]

Looks good, +1!

review: Approve

=== modified file 'debian/landscape-client.postinst'
--- debian/landscape-client.postinst	2012-05-16 20:59:14 +0000
+++ debian/landscape-client.postinst	2012-10-15 08:20:24 +0000
@@ -82,6 +82,7 @@
         else
             # Fix non-private permissions
             chmod 0600 $CONFIG_FILE
+            chown landscape $CONFIG_FILE
         fi
 
         # old monolithic clients have a single persist data file and a single

=== modified file 'landscape/deployment.py'
--- landscape/deployment.py	2012-09-20 08:49:50 +0000
+++ landscape/deployment.py	2012-10-15 08:20:24 +0000
@@ -144,6 +144,9 @@
         # Parse configuration file, if found.
         if self.config:
             if os.path.isfile(self.config):
+                if not os.access(self.config, os.R_OK):
+                    sys.exit(
+                        "error: config file %s can't be read" % self.config)
                 self.load_configuration_file(self.config)
             elif not accept_nonexistent_config:
                 sys.exit("error: file not found: %s" % self.config)

=== modified file 'landscape/tests/test_deployment.py'
--- landscape/tests/test_deployment.py	2012-09-14 07:05:37 +0000
+++ landscape/tests/test_deployment.py	2012-10-15 08:20:24 +0000
@@ -297,6 +297,18 @@
         self.config.reload()
         self.assertEqual(self.config.hello, "world2")
 
+    def test_load_cant_read(self):
+        """
+        C{config.load} exits the process if the specific config file can't be
+        read because of permission reasons.
+        """
+        filename = self.makeFile("[client]\nhello = world1\n")
+        os.chmod(filename, 0)
+        error = self.assertRaises(
+            SystemExit, self.config.load, ["--config", filename])
+        self.assertEqual("error: config file %s can't be read" % filename,
+                         str(error))
+
     def test_data_directory_option(self):
         """Ensure options.data_path option can be read by parse_args."""
         options = self.parser.parse_args(["--data-path",