From 4b09fbfbab15c4bb83e76560a6eb921814364a27 Mon Sep 17 00:00:00 2001 From: Pratik Mallya Date: Mon, 23 Nov 2015 13:48:18 -0600 Subject: [PATCH] Correctly determine when SSL termination config changes The Rackspace::Cloud::Loadbalancer resource does not diff different versions of sslTermination config correctly, leading it to be stuck in check_create_complete forever. This patch has the required logic to make that check correctly. Change-Id: I2fc3e9bf144f15a4bc48ad3996b8dde3338d361c Closes-Bug: #1519069 --- .../rackspace/resources/cloud_loadbalancer.py | 20 ++++++++++++++-- .../tests/test_cloud_loadbalancer.py | 23 +++++++++++++------ 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/contrib/rackspace/rackspace/resources/cloud_loadbalancer.py b/contrib/rackspace/rackspace/resources/cloud_loadbalancer.py index 079fbfa4cb..f962cbbaf5 100644 --- a/contrib/rackspace/rackspace/resources/cloud_loadbalancer.py +++ b/contrib/rackspace/rackspace/resources/cloud_loadbalancer.py @@ -612,7 +612,6 @@ class CloudLoadBalancer(resource.Resource): old_ssl_term = lb.get_ssl_termination() new_ssl_term = self.properties[self.SSL_TERMINATION] - new_ssl_term['enabled'] = True if not self._ssl_term_needs_update(old_ssl_term, new_ssl_term): return True @@ -765,7 +764,24 @@ class CloudLoadBalancer(resource.Resource): return self._needs_update_comparison_bool(old, new) # bool def _ssl_term_needs_update(self, old, new): - return self._needs_update_comparison_nullable(old, new) # dict + if new is None: + return self._needs_update_comparison_nullable( + old, new) # dict + + old_cp = copy.deepcopy(old) + new_cp = copy.deepcopy(new) + # private_key is not returned by api + if self.SSL_TERMINATION_PRIVATEKEY in new_cp: + del new_cp[self.SSL_TERMINATION_PRIVATEKEY] + # enabled is returned by api but not present in schema + if 'enabled' in old_cp: + del old_cp['enabled'] + # if new keys are empty, they don't require update + extra_keys = set(new_cp.keys()) - set(old_cp.keys()) + for key in extra_keys: + if new_cp[key] is None or six.text_type(new_cp[key]) == u'': + del new_cp[key] + return self._needs_update_comparison_nullable(old_cp, new_cp) # dict def _access_list_needs_update(self, old, new): old = set([frozenset(s.items()) for s in old]) diff --git a/contrib/rackspace/rackspace/tests/test_cloud_loadbalancer.py b/contrib/rackspace/rackspace/tests/test_cloud_loadbalancer.py index 37a1f83c49..fb254cc50b 100644 --- a/contrib/rackspace/rackspace/tests/test_cloud_loadbalancer.py +++ b/contrib/rackspace/rackspace/tests/test_cloud_loadbalancer.py @@ -669,7 +669,6 @@ class LoadBalancerTest(common.HeatTestCase): 'secureTrafficOnly': False } ssl_termination_api = copy.deepcopy(ssl_termination_template) - ssl_termination_api['enabled'] = True template = self._set_template(self.lb_template, sslTermination=ssl_termination_template) @@ -678,7 +677,13 @@ class LoadBalancerTest(common.HeatTestCase): self.expected_body) self.m.StubOutWithMock(fake_lb, 'get_ssl_termination') fake_lb.get_ssl_termination().AndReturn({}) - fake_lb.get_ssl_termination().AndReturn(ssl_termination_api) + fake_lb.get_ssl_termination().AndReturn({ + 'securePort': 443, + 'certificate': 'fawefwea', + 'intermediateCertificate': "intermediate_certificate", + 'secureTrafficOnly': False, + 'enabled': True, + }) self.m.StubOutWithMock(fake_lb, 'add_ssl_termination') fake_lb.add_ssl_termination(**ssl_termination_api) @@ -1116,6 +1121,7 @@ class LoadBalancerTest(common.HeatTestCase): } ssl_termination_api = copy.deepcopy(ssl_termination_template) ssl_termination_api['enabled'] = True + del ssl_termination_api['privatekey'] template = self._set_template( self.lb_template, sslTermination=ssl_termination_template, protocol="HTTP", httpsRedirect=True) @@ -1353,8 +1359,8 @@ class LoadBalancerTest(common.HeatTestCase): self.m.StubOutWithMock(fake_lb, 'get_ssl_termination') fake_lb.get_ssl_termination().AndReturn({}) fake_lb.get_ssl_termination().AndReturn({ - 'securePort': 443, 'privatekey': private_key, 'certificate': cert, - 'secureTrafficOnly': False, 'intermediateCertificate': ''}) + 'securePort': 443, 'certificate': cert, + 'secureTrafficOnly': False, 'enabled': True}) self.m.StubOutWithMock(fake_lb, 'add_ssl_termination') fake_lb.add_ssl_termination( @@ -1372,7 +1378,6 @@ class LoadBalancerTest(common.HeatTestCase): 'securePort': 443, 'privatekey': private_key, 'certificate': cert, 'intermediateCertificate': '', 'secureTrafficOnly': False} ssl_termination_api = copy.deepcopy(ssl_termination_template) - ssl_termination_api['enabled'] = True lb_name = list(six.iterkeys(template['Resources']))[0] template['Resources'][lb_name]['Properties']['sslTermination'] = \ ssl_termination_template @@ -1388,7 +1393,9 @@ class LoadBalancerTest(common.HeatTestCase): self.m.StubOutWithMock(fake_lb, 'add_ssl_termination') fake_lb.add_ssl_termination(**ssl_termination_api) - fake_lb.get_ssl_termination().AndReturn(ssl_termination_api) + fake_lb.get_ssl_termination().AndReturn({ + 'securePort': 443, 'certificate': cert, + 'secureTrafficOnly': False, 'enabled': True}) self.m.ReplayAll() scheduler.TaskRunner(rsrc.create)() @@ -1402,7 +1409,9 @@ class LoadBalancerTest(common.HeatTestCase): fake_lb) self.m.StubOutWithMock(fake_lb, 'get_ssl_termination') - fake_lb.get_ssl_termination().AndReturn(ssl_termination_api) + fake_lb.get_ssl_termination().AndReturn({ + 'securePort': 443, 'certificate': cert, + 'secureTrafficOnly': False}) self.m.StubOutWithMock(fake_lb, 'delete_ssl_termination') fake_lb.delete_ssl_termination()