slapos/slap: Stabilise connection_dict

connection_dict generated by client can be different in details from the
server side, so pass it thorugh a way how it is treat on the server.

Also, as we are going to be compatible with py3, calculate hash from
sorted items of a dict, instead of relying on side effect of py2 "ordered dict"

https://portingguide.readthedocs.io/en/latest/dicts.html#changed-key-order

Test are focused on client side, and the tricky cases are covered, they
somehow contain the protocol of client <--> server comparision.

slapos/slap/slap.py

@@ -39,16 +39,14 @@ __all__ = ["slap", "ComputerPartition", "Computer", "SoftwareRelease",
 import os
 import logging
 import re
-import hashlib
 from functools import wraps
 
 import six
 
-from .util import xml2dict
 from .exception import ResourceNotReady, ServerError, NotFoundError, \
           ConnectionError
 from .hateoas import SlapHateoasNavigator, ConnectionHelper
-from slapos.util import loads, dumps, bytes2str
+from slapos.util import loads, dumps, bytes2str, xml2dict, dict2xml, calculate_dict_hash
 
 from xml.sax import saxutils
 from zope.interface import implementer
@@ -608,6 +606,8 @@ class ComputerPartition(SlapRequester):
       return self._software_release_document
 
   def setConnectionDict(self, connection_dict, slave_reference=None):
+    # recreate and stabilise connection_dict that it would became the same as on server
+    connection_dict = xml2dict(dict2xml(connection_dict))
     if self.getConnectionParameterDict() == connection_dict:
       return
 
@@ -625,7 +625,7 @@ class ComputerPartition(SlapRequester):
 
       # Skip as nothing changed for the slave
       if connection_parameter_hash is not None and \
-        connection_parameter_hash == hashlib.sha256(str(connection_dict)).hexdigest():
+        connection_parameter_hash == calculate_dict_hash(connection_dict):
         return
 
     self._connection_helper.POST('setComputerPartitionConnectionXml', data={

slapos/tests/test_slap.py

@@ -29,13 +29,17 @@ import logging
 import os
 import unittest
 from six.moves.urllib import parse
+from six import PY3
 import tempfile
 import logging
 
+from collections import OrderedDict
 import httmock
 
+import mock
+
 import slapos.slap
-from slapos.util import dumps
+from slapos.util import dumps, calculate_dict_hash
 
 
 class UndefinedYetException(Exception):
@@ -958,6 +962,100 @@ class TestComputerPartition(SlapMixin):
       # XXX: Interface does not define return value
       computer_partition.error('some error')
 
+  def _test_setConnectionDict(
+    self, connection_dict, slave_reference=None, connection_xml=None,
+    getConnectionParameterDict=None, connection_parameter_hash=None):
+    getInstanceParameter = []
+    if connection_parameter_hash is not None:
+      getInstanceParameter = [
+        {
+          'slave_reference': slave_reference,
+          'connection-parameter-hash': connection_parameter_hash
+        }
+      ]
+    with \
+        mock.patch.object(
+          slapos.slap.ComputerPartition, '__init__', return_value=None), \
+        mock.patch.object(
+          slapos.slap.ComputerPartition, 'getConnectionParameterDict',
+          return_value=getConnectionParameterDict or {}), \
+        mock.patch.object(
+          slapos.slap.ComputerPartition, 'getInstanceParameter',
+          return_value=getInstanceParameter):
+      partition = slapos.slap.ComputerPartition()
+      partition._connection_helper = mock.Mock()
+      partition._computer_id = 'COMP-0'
+      partition._partition_id = 'PART-0'
+      partition._connection_helper.POST = mock.Mock()
+      partition.setConnectionDict(
+        connection_dict, slave_reference=slave_reference)
+      if connection_xml:
+        connection_xml = connection_xml.encode() if PY3 else connection_xml
+        partition._connection_helper.POST.assert_called_with(
+          'setComputerPartitionConnectionXml',
+          data={
+            'slave_reference': slave_reference,
+            'connection_xml': connection_xml,
+            'computer_partition_id': 'PART-0',
+            'computer_id': 'COMP-0'})
+      else:
+        partition._connection_helper.POST.assert_not_called()
+
+  def test_setConnectionDict(self):
+    self._test_setConnectionDict(
+      {'a': 'b'},
+      connection_xml='<marshal><dictionary id="i2"><string>a</string>'
+                     '<string>b</string></dictionary></marshal>')
+
+  def test_setConnectionDict_optimised(self):
+    self._test_setConnectionDict(
+      {'a': 'b'},
+      getConnectionParameterDict={'a': 'b'},
+      connection_xml=False)
+
+  def test_setConnectionDict_optimised_tricky(self):
+    self._test_setConnectionDict(
+      {u'a': u'b', 'b': '', 'c': None},
+      getConnectionParameterDict={'a': 'b', 'b': None, 'c': 'None'},
+      connection_xml=False)
+
+  def test_setConnectionDict_update(self):
+    self._test_setConnectionDict(
+      {'a': 'b'},
+      getConnectionParameterDict={'b': 'b'},
+      connection_xml='<marshal><dictionary id="i2"><string>a</string>'
+                     '<string>b</string></dictionary></marshal>')
+
+  def test_setConnectionDict_slave(self):
+    self._test_setConnectionDict(
+      {'a': 'b'},
+      slave_reference='SLAVE-0',
+      connection_xml='<marshal><dictionary id="i2"><string>a</string>'
+                     '<string>b</string></dictionary></marshal>')
+
+  def test_setConnectionDict_slave_expired_hash(self):
+    self._test_setConnectionDict(
+      {'a': 'b'},
+      slave_reference='SLAVE-0',
+      connection_parameter_hash='mess',
+      connection_xml='<marshal><dictionary id="i2"><string>a</string>'
+                     '<string>b</string></dictionary></marshal>')
+
+  def test_setConnectionDict_slave_hash(self):
+    self._test_setConnectionDict(
+      {'a': 'b'},
+      slave_reference='SLAVE-0',
+      connection_parameter_hash=calculate_dict_hash({'a': 'b'}),
+      connection_xml=False)
+
+  def test_setConnectionDict_slave_hash_tricky(self):
+    self._test_setConnectionDict(
+      {u'a': u'b', 'b': '', 'c': None},
+      slave_reference='SLAVE-0',
+      connection_parameter_hash=calculate_dict_hash({
+        'a': 'b', 'b': None, 'c': 'None'}),
+      connection_xml=False)
+
 
 class TestSoftwareRelease(SlapMixin):
   """

slapos/util.py

@@ -36,6 +36,8 @@ import sqlite3
 from xml_marshaller.xml_marshaller import dumps, loads
 from lxml import etree
 import six
+import hashlib
+import netaddr
 
 
 def mkdir_p(path, mode=0o700):
@@ -178,3 +180,12 @@ def xml2dict(xml):
         value = element.text
       result_dict[key] = value
   return result_dict
+
+
+def calculate_dict_hash(d):
+  return hashlib.sha256(
+    str2bytes(str(
+      sorted(
+        d.items()
+      )
+    ))).hexdigest()