Merge lp:~gocept/landscape-client/bytes-versioning into lp:~landscape/landscape-client/trunk

[Steffen Allner]

Only allow bytestrings in landscape.lib.versioning is_version_higher() and sort_versions() functions: this is what Landscape uses in exchanged messages.

To consistently compare software versions, the helper method now only accepts bytes, as the version can come from a message which in turn results from a bpickled stream. To ensure backward compatibility to Python 2.7 versions of the landscape-server we have to keep them as bytes.

The constants defined throughout the project had to be adapted.

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: TRIAL_ARGS=-j4 make check
Result: Success
Revno: 964
Branch: lp:~gocept/landscape-client/bytes-versioning
Jenkins: https://ci.lscape.net/job/latch-test-xenial/3642/

review: Approve (test results)

[Данило Шеган (danilo)]

Looks good, thanks for splitting this up into a separate branch.

review: Approve

[Daniel Havlik (nilo)]

+1

review: Approve

=== modified file 'landscape/__init__.py'
--- landscape/__init__.py	2017-03-02 09:36:25 +0000
+++ landscape/__init__.py	2017-03-21 13:06:07 +0000
@@ -4,7 +4,7 @@
 
 # The minimum server API version that all Landscape servers are known to speak
 # and support. It can serve as fallback in case higher versions are not there.
-DEFAULT_SERVER_API = "3.2"
+DEFAULT_SERVER_API = b"3.2"
 
 # The highest server API version that the client is capable of speaking. The
 # client will use it, unless the server declares to support only a lower
@@ -22,7 +22,7 @@
 # 3.3:
 #  * Add new schema for the "registration" message, providing Juju information
 #
-SERVER_API = "3.3"
+SERVER_API = b"3.3"
 
 # The "client-api" field of outgoing messages will be set to this value, and
 # used by the server to know which schema do the message types accepted by the
@@ -54,4 +54,4 @@
 #    "info" field of the response payload for a fail registration, in case the
 #    account has too many pending computers.
 #
-CLIENT_API = "3.8"
+CLIENT_API = b"3.8"

=== modified file 'landscape/broker/exchange.py'
--- landscape/broker/exchange.py	2017-01-05 11:19:41 +0000
+++ landscape/broker/exchange.py	2017-03-21 13:06:07 +0000
@@ -771,7 +771,7 @@
 
         # Extract the server API from the payload. If it's not there it must
         # be 3.2, because it's the one that didn't have this field.
-        server_api = result.get("server-api", "3.2")
+        server_api = result.get("server-api", b"3.2")
 
         if is_version_higher(server_api, message_store.get_server_api()):
             # The server can handle a message API that is higher than the one

=== modified file 'landscape/lib/tests/test_versioning.py'
--- landscape/lib/tests/test_versioning.py	2014-08-22 08:25:25 +0000
+++ landscape/lib/tests/test_versioning.py	2017-03-21 13:06:07 +0000
@@ -10,21 +10,21 @@
         The C{is_version_higher} function returns C{True} if the first
         version is greater than the second.
         """
-        self.assertTrue(is_version_higher("3.2", "3.1"))
+        self.assertTrue(is_version_higher(b"3.2", b"3.1"))
 
     def test_equal(self):
         """
         The C{is_version_higher} function returns C{True} if the first
         version is the same as the second.
         """
-        self.assertTrue(is_version_higher("3.1", "3.1"))
+        self.assertTrue(is_version_higher(b"3.1", b"3.1"))
 
     def test_lower(self):
         """
         The C{is_version_higher} function returns C{False} if the first
         version is lower than the second.
         """
-        self.assertFalse(is_version_higher("3.1", "3.2"))
+        self.assertFalse(is_version_higher(b"3.1", b"3.2"))
 
 
 class SortVersionsTest(TestCase):
@@ -34,5 +34,5 @@
         The C{sort_versions} function sorts the given versions from the
         highest to the lowest.
         """
-        versions = ["3.2", "3.3", "3.1"]
-        self.assertEqual(["3.3", "3.2", "3.1"], sort_versions(versions))
+        versions = [b"3.2", b"3.3", b"3.1"]
+        self.assertEqual([b"3.3", b"3.2", b"3.1"], sort_versions(versions))

=== modified file 'landscape/lib/versioning.py'
--- landscape/lib/versioning.py	2014-08-22 08:25:25 +0000
+++ landscape/lib/versioning.py	2017-03-21 13:06:07 +0000
@@ -6,23 +6,30 @@
 def is_version_higher(version1, version2):
     """Check if a version is higher than another.
 
-    This takes two software versions in the usual "x.y" form
+    This takes two software versions in the usual b"x.y" form
     and split them on the decimal character, converting both parts
-    to ints, e.g. "3.2" becomes (3, 2).
+    to ints, e.g. b"3.2" becomes (3, 2).
 
     It then does a comparison of the two tuples, and returns C{True} if
     C{version1} is greater than or equal to C{version2}.
 
-    @param version1: The first version to compare.
-    @param version2: The second version to compare.
+    @param version1: The first version to compare as C{bytes}.
+    @param version2: The second version to compare as C{bytes}.
     @return: C{True} if the first version is greater than or equal to
         the second.
     """
+    version1 = version1.decode("ascii")
+    version2 = version2.decode("ascii")
     return StrictVersion(version1) >= StrictVersion(version2)
 
 
 def sort_versions(versions):
-    """Sort a list of software versions from the highest to the lowest."""
+    """Sort a list of software versions in from the highest to the lowest.
+
+    @param version: a C{list} of C{bytes} describing a version.
+    """
     strict_versions = sorted(
-        [StrictVersion(version) for version in versions], reverse=True)
-    return [str(strict_version) for strict_version in strict_versions]
+        [StrictVersion(version.decode("ascii")) for version in versions],
+        reverse=True)
+    return [str(strict_version).encode("ascii")
+            for strict_version in strict_versions]