Merge lp:~gocept/landscape-client/py3-manager-keystonetoken into lp:~landscape/landscape-client/trunk

[Steffen Allner]

This is the Py3 port of landscape.manager.keystonetoken.

All the changes fix bytes/unicode issues. Notably, ConfigParser in Python 3 is constrained to unicode, even for values. So we have to dance around that using surrogateescape.

This MP finalized the port of landscape.manager to Python 3.

In landscape.manager.keystonetoken, a potentially binary token is read from a config file. This file would be decoded with utf-8 per default in Python 3, which needs the "surrogateescape" error handler to work properly. Consequently, we have to read in the file and decode it manually.

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: TRIAL_ARGS=-j4 make ci-check
Result: Success
Revno: 987
Branch: lp:~gocept/landscape-client/py3-manager-keystonetoken
Jenkins: https://ci.lscape.net/job/latch-test-xenial/3810/

review: Approve (test results)

[Alberto Donato (ack)]

+1

review: Approve

[Steffen Allner (sallner)]

Alberto, thanks for the review.

[Michael Howitz (mh-gocept)]

LGTM.

review: Approve

[Eric Snow (ericsnowcurrently)]

LGTM

It wasn't clear at first why we need to do the whole read_binary_file() dance. However, I figured out that ConfigParser is strictly unicode-based in py3, even for values. :/

review: Approve

=== modified file 'landscape/manager/keystonetoken.py'
--- landscape/manager/keystonetoken.py	2017-01-23 12:14:33 +0000
+++ landscape/manager/keystonetoken.py	2017-03-29 09:23:27 +0000
@@ -1,9 +1,12 @@
 import os
 import logging
 
+from twisted.python.compat import _PY3
+
 from landscape.compat import ConfigParser, NoOptionError
 from landscape.monitor.plugin import DataWatcher
 from landscape.lib.persist import Persist
+from landscape.lib.fs import read_binary_file
 
 
 KEYSTONE_CONFIG_FILE = "/etc/keystone/keystone.conf"
@@ -47,11 +50,24 @@
             return None
 
         config = ConfigParser()
-        config.read(self._keystone_config_file)
+        if _PY3:
+            # We need to use the surrogateescape error handler as the
+            # admin_token my contain arbitrary bytes. The ConfigParser in
+            # Python 2 on the other hand does not support read_string.
+            config_str = read_binary_file(
+                self._keystone_config_file).decode("utf-8", "surrogateescape")
+            config.read_string(config_str)
+        else:
+            config.read(self._keystone_config_file)
         try:
             admin_token = config.get("DEFAULT", "admin_token")
         except NoOptionError:
             logging.error("KeystoneToken: No admin_token found in %s"
                           % (self._keystone_config_file))
             return None
+        # There is no support for surrogateescape in Python 2, but we actually
+        # have bytes in this case anyway.
+        if _PY3:
+            admin_token = admin_token.encode("utf-8", "surrogateescape")
+
         return admin_token

=== modified file 'landscape/manager/tests/test_keystonetoken.py'
--- landscape/manager/tests/test_keystonetoken.py	2013-07-15 09:19:02 +0000
+++ landscape/manager/tests/test_keystonetoken.py	2017-03-29 09:23:27 +0000
@@ -46,17 +46,19 @@
         self.makeFile(
             path=self.keystone_file,
             content="[DEFAULT]\nadmin_token = foobar")
-        self.assertEqual("foobar", self.plugin.get_data())
+        # As we allow arbitrary bytes, we also need bytes here.
+        self.assertEqual(b"foobar", self.plugin.get_data())
 
     def test_get_keystone_token_non_utf8(self):
         """
         The data can be arbitrary bytes.
         """
-        content = "[DEFAULT]\nadmin_token = \xff"
+        content = b"[DEFAULT]\nadmin_token = \xff"
         self.makeFile(
             path=self.keystone_file,
-            content=content)
-        self.assertEqual("\xff", self.plugin.get_data())
+            content=content,
+            mode="wb")
+        self.assertEqual(b"\xff", self.plugin.get_data())
 
     def test_get_message(self):
         """
@@ -69,7 +71,7 @@
         self.plugin.register(self.manager)
         message = self.plugin.get_message()
         self.assertEqual(
-            {'type': 'keystone-token', 'data': 'foobar'},
+            {'type': 'keystone-token', 'data': b'foobar'},
             message)
         message = self.plugin.get_message()
         self.assertIs(None, message)

=== modified file 'py3_ready_tests'
--- py3_ready_tests	2017-03-29 09:23:27 +0000
+++ py3_ready_tests	2017-03-29 09:23:27 +0000
@@ -3,13 +3,14 @@
 landscape.user.tests
 landscape.broker.tests
 landscape.package.tests
-landscape.tests.test_configuration
-landscape.tests.test_watchdog
-landscape.tests.test_reactor
-landscape.tests.test_schema
-
-landscape.manager.tests.test_scriptexecution
-landscape.manager.tests.test_customgraph
-landscape.manager.tests.test_usermanager
-landscape.manager.tests.test_shutdownmanager
-landscape.manager.tests.test_aptsources
+landscape.manager.tests
+landscape.tests
+
+
+
+
+
+
+
+
+