openstack/octavia
Code Issues Proposed changes

Fix listener update with SNI certificates

Browse Source 
SNI certificates were not being set in the database on listener update.
A listener GET would not show the certificates in the sni_container_refs
attribute. Also, the API was allowing set of SNI certs on non
TERMINATED_HTTPS listeners.

Task: 39042
Story: 2007421
Story: 2007430

Change-Id: If5b6411a0b7c75441a406234c2792ea68d35d0fe
This commit is contained in:
Carlos Goncalves 2020-03-12 20:30:31 +00:00
parent 409b89f141
commit 9e070e6e6d
10 changed files with 85 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
octavia/api/drivers/utils.py
View File

@ -220,19 +220,6 @@ def listener_dict_to_provider_dict(listener_dict, for_delete=False):
if 'tls_certificate_id' in new_listener_dict:
new_listener_dict['default_tls_container_ref'] = new_listener_dict.pop(
'tls_certificate_id')
if 'sni_containers' in new_listener_dict:
sni_refs = []
sni_containers = new_listener_dict.pop('sni_containers')
for sni in sni_containers:
if 'tls_container_id' in sni:
sni_refs.append(sni['tls_container_id'])
else:
raise exceptions.ValidationException(
detail=_('Invalid SNI container on listener'))
new_listener_dict['sni_container_refs'] = sni_refs
if 'sni_container_refs' in listener_dict:
listener_dict['sni_containers'] = listener_dict.pop(
'sni_container_refs')
if 'client_ca_tls_certificate_id' in new_listener_dict:
new_listener_dict['client_ca_tls_container_ref'] = (
new_listener_dict.pop('client_ca_tls_certificate_id'))
@ -245,8 +232,6 @@ def listener_dict_to_provider_dict(listener_dict, for_delete=False):
SNI_objs = []
for sni in listener_obj.sni_containers:
if isinstance(sni, dict):
if 'listener' in sni:
del sni['listener']
sni_obj = data_models.SNI(**sni)
SNI_objs.append(sni_obj)
elif isinstance(sni, str):
@ -297,6 +282,20 @@ def listener_dict_to_provider_dict(listener_dict, for_delete=False):
new_listener_dict['allowed_cidrs'] = [cidr_dict['cidr'] for
cidr_dict in cidrs_dict_list]
# Format the sni_containers -> sni_container_refs
sni_containers = new_listener_dict.pop('sni_containers', None)
if sni_containers:
new_listener_dict['sni_container_refs'] = []
for sni in sni_containers:
if isinstance(sni, dict):
new_listener_dict['sni_container_refs'].append(
sni['tls_container_id'])
elif isinstance(sni, str):
new_listener_dict['sni_container_refs'].append(sni)
else:
raise exceptions.ValidationException(
detail=_('Invalid SNI container on listener'))
# Remove the DB back references
if 'load_balancer' in new_listener_dict:
del new_listener_dict['load_balancer']

1
octavia/api/v2/types/listener.py
View File

@ -28,6 +28,7 @@ class BaseListenerType(types.BaseType):
_type_to_model_map = {
'admin_state_up': 'enabled',
'default_tls_container_ref': 'tls_certificate_id',
'sni_container_refs': 'sni_containers',
'client_ca_tls_container_ref': 'client_ca_tls_certificate_id',
'client_crl_container_ref': 'client_crl_container_id'}
_child_map = {}

8
octavia/common/data_models.py
View File

@ -540,6 +540,14 @@ class SNI(BaseDataModel):
self.listener = listener
self.tls_container_id = tls_container_id
# SQLAlchemy kindly attaches the whole listener object so
# let's keep this simple by overriding the to_dict for this
# object. Otherwise we recurse down the "ghost" listener object.
def to_dict(self, **kwargs):
return {'tls_container_id': self.tls_container_id,
'listener_id': self.listener_id,
'position': self.position}
class TLSContainer(BaseDataModel):

4
octavia/db/models.py
View File

@ -502,8 +502,8 @@ class Listener(base_models.BASE, base_models.IdMixin,
default_pool = orm.relationship("Pool", uselist=False,
back_populates="_default_listeners")
sni_containers = orm.relationship(
'SNI', cascade='delete', uselist=True,
backref=orm.backref('listener', uselist=False))
'SNI', cascade='all,delete-orphan',
uselist=True, backref=orm.backref('listener', uselist=False))
l7policies = orm.relationship(
'L7Policy', uselist=True, order_by='L7Policy.position',

15
octavia/db/repositories.py
View File

@ -1073,11 +1073,16 @@ class ListenerRepository(BaseRepository):
default_pool_id = model_kwargs.get('default_pool_id')
if default_pool_id:
self._pool_check(session, default_pool_id, listener_id=id)
sni_containers = model_kwargs.pop('sni_containers', [])
for container_ref in sni_containers:
sni = models.SNI(listener_id=id,
tls_certificate_id=container_ref)
listener_db.sni_containers.append(sni)
if 'sni_containers' in model_kwargs:
# sni_container_refs is being updated. It is either being set
# or unset/cleared. We need to update in DB side.
containers = model_kwargs.pop('sni_containers', []) or []
listener_db.sni_containers = []
if containers:
listener_db.sni_containers = [
models.SNI(listener_id=id,
tls_container_id=container_ref)
for container_ref in containers]
if 'allowed_cidrs' in model_kwargs:
# allowed_cidrs is being updated. It is either being set or
# unset/cleared. We need to update in DB side.

5
octavia/tests/common/sample_data_models.py
View File

@ -454,9 +454,8 @@ class SampleDriverDataModels(object):
lib_consts.CONNECTION_LIMIT: 10000,
constants.TLS_CERTIFICATE_ID: self.default_tls_container_ref,
lib_consts.DEFAULT_POOL: self.test_pool1_dict,
constants.SNI_CONTAINERS: [
{constants.TLS_CONTAINER_ID: self.sni_container_ref_1},
{constants.TLS_CONTAINER_ID: self.sni_container_ref_2}],
constants.SNI_CONTAINERS: [self.sni_container_ref_1,
self.sni_container_ref_2],
constants.PEER_PORT: 55,
lib_consts.L7POLICIES: self.test_l7policies,
lib_consts.INSERT_HEADERS: {},

32
octavia/tests/functional/api/v2/test_listener.py
View File

@ -2249,7 +2249,29 @@ class TestListener(base.BaseAPITest):
# TODO(johnsom) Fix this when there is a noop certificate manager
@mock.patch('octavia.common.tls_utils.cert_parser.load_certificates_data')
def test_update_with_sni_data(self, mock_cert_data):
def test_update_tls_terminated_with_sni_data(self, mock_cert_data):
cert2 = data_models.TLSContainer(certificate='cert 2')
cert3 = data_models.TLSContainer(certificate='cert 3')
mock_cert_data.return_value = {'sni_certs': [cert2, cert3]}
sni_id1 = uuidutils.generate_uuid()
sni_id2 = uuidutils.generate_uuid()
listener = self.create_listener(
constants.PROTOCOL_TERMINATED_HTTPS, 80, self.lb_id,
default_tls_container_ref=uuidutils.generate_uuid())
self.set_lb_status(self.lb_id)
listener_path = self.LISTENER_PATH.format(
listener_id=listener['listener']['id'])
get_listener = self.get(listener_path).json['listener']
self.assertEqual([], get_listener.get('sni_container_refs'))
self.put(listener_path,
self._build_body({'sni_container_refs': [sni_id1, sni_id2]}))
get_listener = self.get(listener_path).json['listener']
self.assertItemsEqual([sni_id1, sni_id2],
get_listener.get('sni_container_refs'))
# TODO(johnsom) Fix this when there is a noop certificate manager
@mock.patch('octavia.common.tls_utils.cert_parser.load_certificates_data')
def test_update_non_tls_terminated_with_sni_data(self, mock_cert_data):
cert2 = data_models.TLSContainer(certificate='cert 2')
cert3 = data_models.TLSContainer(certificate='cert 3')
mock_cert_data.return_value = {'sni_certs': [cert2, cert3]}
@ -2262,8 +2284,12 @@ class TestListener(base.BaseAPITest):
listener_id=listener['listener']['id'])
get_listener = self.get(listener_path).json['listener']
self.assertEqual([], get_listener.get('sni_container_refs'))
self.put(listener_path,
self._build_body({'sni_container_refs': [sni_id1, sni_id2]}))
body = self._build_body({'sni_container_refs': [sni_id1, sni_id2]})
response = self.put(listener_path, body, status=400).json
self.assertEqual(
"Validation failure: Certificate container references are only "
"allowed on TERMINATED_HTTPS protocol listeners.",
response['faultstring'])
get_listener = self.get(listener_path).json['listener']
self.assertEqual([], get_listener.get('sni_container_refs'))

10
octavia/tests/functional/db/test_repositories.py
View File

@ -2384,6 +2384,16 @@ class TestListenerRepositoryTest(BaseRepositoryTest):
new_listener = self.listener_repo.get(self.session, id=listener.id)
self.assertEqual(name_change, new_listener.name)
def test_update_with_sni(self):
listener = self.create_listener(self.FAKE_UUID_1, 80)
container1 = {'listener_id': listener.id,
'tls_container_id': self.FAKE_UUID_2}
container1_dm = models.SNI(**container1)
self.listener_repo.update(self.session, listener.id,
sni_containers=[self.FAKE_UUID_2])
new_listener = self.listener_repo.get(self.session, id=listener.id)
self.assertIn(container1_dm, new_listener.sni_containers)
def test_delete(self):
listener = self.create_listener(self.FAKE_UUID_1, 80)
self.listener_repo.delete(self.session, id=listener.id)

2
octavia/tests/unit/api/drivers/test_utils.py
View File

@ -288,7 +288,7 @@ class TestUtils(base.TestCase):
'sni_certs': [cert2, cert3]}
# Test with bad SNI content
test_listener = copy.deepcopy(self.sample_data.test_listener1_dict)
test_listener['sni_containers'] = [{}]
test_listener['sni_containers'] = [()]
self.assertRaises(exceptions.ValidationException,
utils.listener_dict_to_provider_dict,
test_listener)

8
releasenotes/notes/fix-api-listener-update-sni-containers-6595c52e2de1f621.yaml Normal file
View File

@ -0,0 +1,8 @@
---
fixes:
- |
Fixed an issue where setting of SNI containers were not being applied on
listener update API calls.
- |
Fixed an Octavia API validation on listener update where SNI containers
could be set on non-TERMINATED_HTTPS listeners.