Delete log entries when SG or port is deleted
NOTE: this patch is an ammend of [1]. When a SG or a port is deleted, the related log entry should be too. A log entry has the following fields: - log.resource_id = SG ID - log.target_id = port ID [1] was deleting all log entries, related or not with the SG ID deleted. This is because "get_logs_bound_sg" returned all log entries, including those ones without any "resource_id" or "target_id". Now this method can return only the log entries related to a port or a SG, excluding those ones without those two parameters populated. Closes-Bug: #1939558 [1]https://review.opendev.org/c/openstack/neutron/+/804237 Change-Id: Icb92327a06486e168ce064532d819347e6031cc1
This commit is contained in:
parent
ecdc11a564
commit
41f78c678b
|
@ -180,7 +180,8 @@ def get_logs_bound_port(context, port_id):
|
|||
return [log for log in logs if is_bound(log)]
|
||||
|
||||
|
||||
def get_logs_bound_sg(context, sg_id, project_id=None):
|
||||
def get_logs_bound_sg(context, sg_id=None, project_id=None, port_id=None,
|
||||
exclusive=False):
|
||||
"""Return a list of log_resources bound to a security group"""
|
||||
|
||||
kwargs = {
|
||||
|
@ -189,19 +190,22 @@ def get_logs_bound_sg(context, sg_id, project_id=None):
|
|||
if project_id:
|
||||
kwargs['project_id'] = project_id
|
||||
|
||||
log_objs = log_object.Log.get_objects(
|
||||
context, **kwargs)
|
||||
|
||||
log_objs = log_object.Log.get_objects(context, **kwargs)
|
||||
log_resources = []
|
||||
for log_obj in log_objs:
|
||||
if log_obj.resource_id == sg_id:
|
||||
log_resources.append(log_obj)
|
||||
elif log_obj.target_id:
|
||||
port = port_objects.Port.get_object(
|
||||
context, id=log_obj.target_id)
|
||||
if sg_id in port.security_group_ids:
|
||||
if sg_id:
|
||||
if log_obj.resource_id == sg_id:
|
||||
log_resources.append(log_obj)
|
||||
elif not log_obj.resource_id and not log_obj.target_id:
|
||||
elif log_obj.target_id:
|
||||
# NOTE: optimize this method just returning the SGs per port.
|
||||
port = port_objects.Port.get_object(
|
||||
context, id=log_obj.target_id)
|
||||
if sg_id in port.security_group_ids:
|
||||
log_resources.append(log_obj)
|
||||
elif (not log_obj.resource_id and not log_obj.target_id and
|
||||
not exclusive):
|
||||
log_resources.append(log_obj)
|
||||
elif port_id and log_obj.target_id and log_obj.target_id == port_id:
|
||||
log_resources.append(log_obj)
|
||||
return log_resources
|
||||
|
||||
|
|
|
@ -30,7 +30,7 @@ class SecurityGroupRuleCallBack(manager.ResourceCallBackBase):
|
|||
sg_id = payload.resource_id
|
||||
|
||||
log_resources = db_api.get_logs_bound_sg(
|
||||
context, sg_id, project_id=context.project_id)
|
||||
context, sg_id=sg_id, project_id=context.project_id)
|
||||
if log_resources:
|
||||
self.resource_push_api(
|
||||
log_const.RESOURCE_UPDATE, context, log_resources)
|
||||
|
|
|
@ -30,6 +30,7 @@ from neutron.services.logapi.common import validators
|
|||
from neutron.services.logapi.drivers import manager as driver_mgr
|
||||
|
||||
|
||||
@registry.has_registry_receivers
|
||||
class LoggingPlugin(log_ext.LoggingPluginBase):
|
||||
"""Implementation of the Neutron logging api plugin."""
|
||||
|
||||
|
@ -43,23 +44,30 @@ class LoggingPlugin(log_ext.LoggingPluginBase):
|
|||
super(LoggingPlugin, self).__init__()
|
||||
self.driver_manager = driver_mgr.LoggingServiceDriverManager()
|
||||
self.validator_mgr = validators.ResourceValidateRequest.get_instance()
|
||||
registry.subscribe(
|
||||
self._clean_security_group_logs,
|
||||
resources.SECURITY_GROUP, events.AFTER_DELETE)
|
||||
|
||||
@property
|
||||
def supported_logging_types(self):
|
||||
# supported_logging_types are be dynamically loaded from log_drivers
|
||||
return self.driver_manager.supported_logging_types
|
||||
|
||||
def _clean_security_group_logs(self, resource, event, trigger, payload):
|
||||
context = payload.context.elevated()
|
||||
sg_id = payload.resource_id
|
||||
def _clean_logs(self, context, sg_id=None, port_id=None):
|
||||
with db_api.CONTEXT_WRITER.using(context):
|
||||
sg_logs = log_db_api.get_logs_bound_sg(context, sg_id)
|
||||
sg_logs = log_db_api.get_logs_bound_sg(
|
||||
context, sg_id=sg_id, port_id=port_id, exclusive=True)
|
||||
for log in sg_logs:
|
||||
self.delete_log(context, log['id'])
|
||||
|
||||
@registry.receives(resources.SECURITY_GROUP, [events.AFTER_DELETE])
|
||||
def _clean_logs_by_resource_id(self, resource, event, trigger, payload):
|
||||
# log.resource_id == SG
|
||||
self._clean_logs(payload.context.elevated(), sg_id=payload.resource_id)
|
||||
|
||||
@registry.receives(resources.PORT, [events.AFTER_DELETE])
|
||||
def _clean_logs_by_target_id(self, resource, event, trigger, payload):
|
||||
# log.target_id == port
|
||||
self._clean_logs(payload.context.elevated(),
|
||||
port_id=payload.resource_id)
|
||||
|
||||
@db_base_plugin_common.filter_fields
|
||||
@db_base_plugin_common.convert_result_to_dict
|
||||
def get_logs(self, context, filters=None, fields=None, sorts=None,
|
||||
|
|
|
@ -17,6 +17,7 @@ from unittest import mock
|
|||
|
||||
from neutron_lib import constants as const
|
||||
from neutron_lib import context
|
||||
from neutron_lib.db import api as n_db_api
|
||||
from neutron_lib.services.logapi import constants as log_const
|
||||
from neutron_lib.utils import net as net_utils
|
||||
from oslo_utils import uuidutils
|
||||
|
@ -28,21 +29,24 @@ from neutron.services.logapi.rpc import server as server_rpc
|
|||
from neutron.tests.unit.extensions import test_securitygroup as test_sg
|
||||
|
||||
|
||||
def _create_log(tenant_id, resource_id=None,
|
||||
def _create_log(context, project_id, resource_id=None,
|
||||
target_id=None, event='ALL', enabled=True,):
|
||||
|
||||
log_data = {
|
||||
'id': uuidutils.generate_uuid(),
|
||||
'name': 'test',
|
||||
'resource_type': 'security_group',
|
||||
'project_id': tenant_id,
|
||||
'project_id': project_id,
|
||||
'event': event,
|
||||
'enabled': enabled}
|
||||
if resource_id:
|
||||
log_data['resource_id'] = resource_id
|
||||
if target_id:
|
||||
log_data['target_id'] = target_id
|
||||
return log_object.Log(**log_data)
|
||||
with n_db_api.CONTEXT_WRITER.using(context):
|
||||
_log_obj = log_object.Log(context, **log_data)
|
||||
_log_obj.create()
|
||||
return _log_obj
|
||||
|
||||
|
||||
class LoggingDBApiTestCase(test_sg.SecurityGroupDBTestCase):
|
||||
|
@ -50,8 +54,8 @@ class LoggingDBApiTestCase(test_sg.SecurityGroupDBTestCase):
|
|||
def setUp(self):
|
||||
super(LoggingDBApiTestCase, self).setUp()
|
||||
self.context = context.get_admin_context()
|
||||
self.sg_id, self.port_id, self.tenant_id = self._create_sg_and_port()
|
||||
self.context.tenant_id = self.tenant_id
|
||||
self.sg_id, self.port_id, self._tenant_id = self._create_sg_and_port()
|
||||
self.context.tenant_id = self._tenant_id
|
||||
|
||||
def _create_sg_and_port(self):
|
||||
with self.network() as network, \
|
||||
|
@ -68,45 +72,66 @@ class LoggingDBApiTestCase(test_sg.SecurityGroupDBTestCase):
|
|||
return sg_id, port_id, tenant_id
|
||||
|
||||
def test_get_logs_bound_port(self):
|
||||
log = _create_log(target_id=self.port_id, tenant_id=self.tenant_id)
|
||||
log = _create_log(self.context, self._tenant_id,
|
||||
target_id=self.port_id)
|
||||
with mock.patch.object(log_object.Log, 'get_objects',
|
||||
return_value=[log]):
|
||||
self.assertEqual(
|
||||
[log], db_api.get_logs_bound_port(self.context, self.port_id))
|
||||
|
||||
# Test get log objects with required resource type
|
||||
calls = [mock.call(self.context, project_id=self.tenant_id,
|
||||
calls = [mock.call(self.context, project_id=self._tenant_id,
|
||||
resource_type=log_const.SECURITY_GROUP,
|
||||
enabled=True)]
|
||||
log_object.Log.get_objects.assert_has_calls(calls)
|
||||
|
||||
def test_get_logs_not_bound_port(self):
|
||||
fake_sg_id = uuidutils.generate_uuid()
|
||||
log = _create_log(resource_id=fake_sg_id, tenant_id=self.tenant_id)
|
||||
log = _create_log(self.context, self._tenant_id,
|
||||
resource_id=fake_sg_id)
|
||||
with mock.patch.object(log_object.Log, 'get_objects',
|
||||
return_value=[log]):
|
||||
self.assertEqual(
|
||||
[], db_api.get_logs_bound_port(self.context, self.port_id))
|
||||
|
||||
# Test get log objects with required resource type
|
||||
calls = [mock.call(self.context, project_id=self.tenant_id,
|
||||
calls = [mock.call(self.context, project_id=self._tenant_id,
|
||||
resource_type=log_const.SECURITY_GROUP,
|
||||
enabled=True)]
|
||||
log_object.Log.get_objects.assert_has_calls(calls)
|
||||
|
||||
def test_get_logs_bound_sg(self):
|
||||
log = _create_log(resource_id=self.sg_id, tenant_id=self.tenant_id)
|
||||
with mock.patch.object(log_object.Log, 'get_objects',
|
||||
return_value=[log]):
|
||||
self.assertEqual(
|
||||
[log], db_api.get_logs_bound_sg(
|
||||
self.context, self.sg_id, project_id=self.tenant_id))
|
||||
with self.network() as network, \
|
||||
self.subnet(network=network) as subnet, \
|
||||
self.port(subnet=subnet) as p1, \
|
||||
self.port(subnet=subnet, security_groups=[self.sg_id]) as p2:
|
||||
|
||||
# Test get log objects with required resource type
|
||||
calls = [mock.call(self.context, project_id=self.tenant_id,
|
||||
resource_type=log_const.SECURITY_GROUP,
|
||||
enabled=True)]
|
||||
log_object.Log.get_objects.assert_has_calls(calls)
|
||||
log = _create_log(self.context, self._tenant_id)
|
||||
log_sg = _create_log(self.context, self._tenant_id,
|
||||
resource_id=self.sg_id)
|
||||
log_port_no_sg = _create_log(self.context, self._tenant_id,
|
||||
target_id=p1['port']['id'])
|
||||
log_port_sg = _create_log(self.context, self._tenant_id,
|
||||
target_id=p2['port']['id'])
|
||||
self.assertEqual(
|
||||
[log, log_sg, log_port_sg],
|
||||
db_api.get_logs_bound_sg(self.context, sg_id=self.sg_id,
|
||||
project_id=self._tenant_id))
|
||||
self.assertEqual(
|
||||
[log_sg, log_port_sg],
|
||||
db_api.get_logs_bound_sg(self.context, sg_id=self.sg_id,
|
||||
project_id=self._tenant_id,
|
||||
exclusive=True))
|
||||
self.assertEqual(
|
||||
[log_port_no_sg],
|
||||
db_api.get_logs_bound_sg(
|
||||
self.context, project_id=self._tenant_id,
|
||||
port_id=p1['port']['id']))
|
||||
self.assertEqual(
|
||||
[log_port_sg],
|
||||
db_api.get_logs_bound_sg(
|
||||
self.context, project_id=self._tenant_id,
|
||||
port_id=p2['port']['id']))
|
||||
|
||||
def test_get_logs_not_bound_sg(self):
|
||||
with self.network() as network, \
|
||||
|
@ -117,28 +142,28 @@ class LoggingDBApiTestCase(test_sg.SecurityGroupDBTestCase):
|
|||
self.fmt, network['network']['id'],
|
||||
security_groups=[sg2_id])
|
||||
port2_id = self.deserialize(self.fmt, res)['port']['id']
|
||||
log = _create_log(target_id=port2_id, tenant_id=self.tenant_id)
|
||||
log = _create_log(self.context, self._tenant_id,
|
||||
target_id=port2_id)
|
||||
with mock.patch.object(log_object.Log, 'get_objects',
|
||||
return_value=[log]):
|
||||
self.assertEqual(
|
||||
[], db_api.get_logs_bound_sg(
|
||||
self.context, self.sg_id, project_id=self.tenant_id))
|
||||
self.context, self.sg_id, project_id=self._tenant_id))
|
||||
|
||||
# Test get log objects with required resource type
|
||||
calls = [mock.call(self.context, project_id=self.tenant_id,
|
||||
calls = [mock.call(self.context, project_id=self._tenant_id,
|
||||
resource_type=log_const.SECURITY_GROUP,
|
||||
enabled=True)]
|
||||
log_object.Log.get_objects.assert_has_calls(calls)
|
||||
|
||||
def test__get_ports_being_logged(self):
|
||||
log1 = _create_log(target_id=self.port_id,
|
||||
tenant_id=self.tenant_id)
|
||||
log2 = _create_log(resource_id=self.sg_id,
|
||||
tenant_id=self.tenant_id)
|
||||
log3 = _create_log(target_id=self.port_id,
|
||||
resource_id=self.tenant_id,
|
||||
tenant_id=self.tenant_id)
|
||||
log4 = _create_log(tenant_id=self.tenant_id)
|
||||
log1 = _create_log(self.context, self._tenant_id,
|
||||
target_id=self.port_id)
|
||||
log2 = _create_log(self.context, self._tenant_id,
|
||||
resource_id=self.sg_id)
|
||||
log3 = _create_log(self.context, self._tenant_id,
|
||||
target_id=self.port_id, resource_id=self.sg_id)
|
||||
log4 = _create_log(self.context, self._tenant_id)
|
||||
with mock.patch.object(
|
||||
validators, 'validate_log_type_for_port', return_value=True):
|
||||
ports_log1 = db_api._get_ports_being_logged(self.context, log1)
|
||||
|
@ -152,7 +177,7 @@ class LoggingDBApiTestCase(test_sg.SecurityGroupDBTestCase):
|
|||
self.assertEqual([self.port_id], ports_log4)
|
||||
|
||||
def test__get_ports_being_logged_not_supported_log_type(self):
|
||||
log = _create_log(tenant_id=self.tenant_id)
|
||||
log = _create_log(self.context, self._tenant_id)
|
||||
with mock.patch.object(
|
||||
validators, 'validate_log_type_for_port', return_value=False):
|
||||
ports_log = db_api._get_ports_being_logged(self.context, log)
|
||||
|
@ -190,7 +215,7 @@ class LoggingRpcCallbackTestCase(test_sg.SecurityGroupDBTestCase):
|
|||
security_groups=[sg_id])
|
||||
ports_rest = self.deserialize(self.fmt, res)
|
||||
port_id = ports_rest['port']['id']
|
||||
log = _create_log(resource_id=sg_id, tenant_id=tenant_id)
|
||||
log = _create_log(self.context, self._tenant_id, resource_id=sg_id)
|
||||
with mock.patch.object(
|
||||
server_rpc,
|
||||
'get_rpc_method',
|
||||
|
@ -262,7 +287,7 @@ class LoggingRpcCallbackTestCase(test_sg.SecurityGroupDBTestCase):
|
|||
)
|
||||
ports_rest = self.deserialize(self.fmt, res)
|
||||
port_id = ports_rest['port']['id']
|
||||
log = _create_log(tenant_id=tenant_id)
|
||||
log = _create_log(self.context, tenant_id)
|
||||
with mock.patch.object(
|
||||
log_object.Log, 'get_objects', return_value=[log]):
|
||||
with mock.patch.object(
|
||||
|
|
|
@ -15,8 +15,6 @@
|
|||
|
||||
from unittest import mock
|
||||
|
||||
from neutron_lib.callbacks import events
|
||||
from neutron_lib.callbacks import resources
|
||||
from neutron_lib import context
|
||||
from neutron_lib.plugins import constants as plugin_const
|
||||
from neutron_lib.plugins import directory
|
||||
|
@ -27,7 +25,6 @@ from neutron import manager
|
|||
from neutron.objects.logapi import logging_resource as log_object
|
||||
from neutron.objects import ports
|
||||
from neutron.objects import securitygroup as sg_object
|
||||
from neutron.services.logapi.common import db_api as log_db_api
|
||||
from neutron.services.logapi.common import exceptions as log_exc
|
||||
from neutron.services.logapi.common import sg_validate
|
||||
from neutron.tests.unit.services.logapi import base
|
||||
|
@ -71,25 +68,6 @@ class TestLoggingPlugin(base.BaseLogTestCase):
|
|||
new_callable=log_types).start()
|
||||
self.ctxt = context.Context('admin', 'fake_tenant')
|
||||
|
||||
def test__clean_security_group_logs(self):
|
||||
logs = [
|
||||
{'id': uuidutils.generate_uuid()},
|
||||
{'id': uuidutils.generate_uuid()}]
|
||||
sg_id = uuidutils.generate_uuid()
|
||||
expected_delete_calls = [
|
||||
mock.call(mock.ANY, log['id']) for log in logs]
|
||||
with mock.patch.object(log_db_api, 'get_logs_bound_sg',
|
||||
return_value=logs) as mock_get_logs, \
|
||||
mock.patch.object(
|
||||
self.log_plugin, 'delete_log') as mock_delete_log:
|
||||
payload = mock.Mock(
|
||||
context=self.ctxt, resource_id=sg_id)
|
||||
self.log_plugin._clean_security_group_logs(
|
||||
resources.SECURITY_GROUP, events.AFTER_DELETE, None, payload)
|
||||
mock_get_logs.assert_called_once_with(
|
||||
mock.ANY, sg_id)
|
||||
mock_delete_log.assert_has_calls(expected_delete_calls)
|
||||
|
||||
def test_get_logs(self):
|
||||
with mock.patch.object(log_object.Log, 'get_objects')\
|
||||
as get_objects_mock:
|
||||
|
|
Loading…
Reference in New Issue