openstack
/
neutron
Code Issues Proposed changes

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

Conflicts:
    neutron/tests/unit/services/logapi/test_logging_plugin.py

Change-Id: Icb92327a06486e168ce064532d819347e6031cc1
(cherry picked from commit 41f78c678b)
(cherry picked from commit 2c4ab468ae)
(cherry picked from commit d5fd9f035a)
(cherry picked from commit 7790cecfc8)
(cherry picked from commit 226367eed1)
This commit is contained in:
Rodolfo Alonso Hernandez 2021-10-05 08:50:52 +00:00 committed by Slawek Kaplonski
parent c10ba0f7ff
commit 7df245ee5b
5 changed files with 90 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
neutron/services/logapi/common/db_api.py
View File

@ -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

2
neutron/services/logapi/common/sg_callback.py
View File

@ -30,7 +30,7 @@ class SecurityGroupRuleCallBack(manager.ResourceCallBackBase):
sg_id = kwargs.get('security_group_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)

22
neutron/services/logapi/logging_plugin.py
View File

@ -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,

93
neutron/tests/unit/services/logapi/common/test_db_api.py
View File

@ -16,6 +16,7 @@
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
@ -27,21 +28,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):
@ -49,8 +53,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, \
@ -67,45 +71,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, \
@ -116,28 +141,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)
@ -151,7 +176,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)
@ -189,7 +214,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',
@ -261,7 +286,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(

22
neutron/tests/unit/services/logapi/test_logging_plugin.py
View File

@ -14,8 +14,6 @@
# under the License.
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
@ -26,7 +24,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
@ -70,25 +67,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: