[GCE] Validate security group at the time of creation
Openstack Security Group should be validated if they are compatible with GCE firewall rules. If not we should raise approriate error. This fix processes security group info in BEFORE_RESPONSE event of security group and rollbacks earlier created security group if not compatible. We can not use BEFORE_CREATE/PRECOMMIT_CREATE as they do not contain required security group rules info. Change-Id: I5f1fc67208085ef399f3dcfe5fdec63d4f2ffc51 Closes-bug: #1709002
This commit is contained in:
parent
7613ef6103
commit
f7085c1883
|
@ -11,8 +11,7 @@ License for the specific language governing permissions and limitations
|
|||
under the License.
|
||||
"""
|
||||
|
||||
import random
|
||||
|
||||
from googleapiclient import errors as gce_errors
|
||||
from neutron._i18n import _
|
||||
from neutron.callbacks import events
|
||||
from neutron.callbacks import registry
|
||||
|
@ -26,6 +25,8 @@ from neutron_lib import exceptions as e
|
|||
from oslo_log import log
|
||||
|
||||
import ipaddr
|
||||
import random
|
||||
|
||||
try:
|
||||
from neutron_lib.plugins import directory
|
||||
except ImportError:
|
||||
|
@ -66,11 +67,15 @@ class GceMechanismDriver(api.MechanismDriver):
|
|||
events.BEFORE_DELETE)
|
||||
registry.subscribe(self.secgroup_callback, resources.SECURITY_GROUP,
|
||||
events.BEFORE_UPDATE)
|
||||
registry.subscribe(self.secgroup_callback, resources.SECURITY_GROUP,
|
||||
events.BEFORE_RESPONSE)
|
||||
|
||||
registry.subscribe(self.secgroup_callback,
|
||||
resources.SECURITY_GROUP_RULE, events.BEFORE_DELETE)
|
||||
registry.subscribe(self.secgroup_callback,
|
||||
resources.SECURITY_GROUP_RULE, events.BEFORE_UPDATE)
|
||||
registry.subscribe(self.secgroup_callback,
|
||||
resources.SECURITY_GROUP_RULE, events.BEFORE_CREATE)
|
||||
|
||||
def _gce_network_name(self, context):
|
||||
return 'net-' + context.current[api.ID]
|
||||
|
@ -152,7 +157,7 @@ class GceMechanismDriver(api.MechanismDriver):
|
|||
def _gce_secgrp_id(self, openstack_id):
|
||||
return "secgrp-" + openstack_id
|
||||
|
||||
def _convert_secgrp_rule_to_gce(self, rule, network_link):
|
||||
def _convert_secgrp_rule_to_gce(self, rule, network_link, validate=False):
|
||||
if rule['ethertype'] != 'IPv4':
|
||||
raise sg.SecurityGroupRuleInvalidEtherType(
|
||||
ethertype=rule['ethertype'], values=('IPv4', ))
|
||||
|
@ -164,8 +169,9 @@ class GceMechanismDriver(api.MechanismDriver):
|
|||
'allowed': [{}],
|
||||
'destinationRanges': [],
|
||||
}
|
||||
gce_rule['name'] = self._gce_secgrp_id(rule['id'])
|
||||
gce_rule['network'] = network_link
|
||||
if not validate:
|
||||
gce_rule['name'] = self._gce_secgrp_id(rule['id'])
|
||||
gce_rule['network'] = network_link
|
||||
|
||||
directions = {
|
||||
'ingress': 'INGRESS',
|
||||
|
@ -217,13 +223,22 @@ class GceMechanismDriver(api.MechanismDriver):
|
|||
operation = gceutils.create_firewall_rule(compute, project, gce_rule)
|
||||
gceutils.wait_for_operation(compute, project, operation)
|
||||
|
||||
def _validate_secgrp_rule(self, rule):
|
||||
try:
|
||||
self._convert_secgrp_rule_to_gce(
|
||||
rule, network_link=None, validate=True)
|
||||
except Exception as e:
|
||||
LOG.exception("An error occurred while creating security "
|
||||
"group: %s" % e)
|
||||
raise e
|
||||
|
||||
def _update_secgrp_rule(self, context, rule_id):
|
||||
compute, project = self.gce_svc, self.gce_project
|
||||
name = self._gce_secgrp_id(rule_id)
|
||||
try:
|
||||
gce_firewall_info = gceutils.get_firewall_rule(
|
||||
compute, project, name)
|
||||
except gceutils.HttpError:
|
||||
except gce_errors.HttpError:
|
||||
return
|
||||
|
||||
try:
|
||||
|
@ -254,7 +269,7 @@ class GceMechanismDriver(api.MechanismDriver):
|
|||
"as firewall rule update not GCE compatible." % name)
|
||||
operation = gceutils.delete_firewall_rule(compute, project, name)
|
||||
gceutils.wait_for_operation(compute, project, operation)
|
||||
except gceutils.HttpError:
|
||||
except gce_errors.HttpError:
|
||||
pass
|
||||
|
||||
def _create_secgrp_rules_if_needed(self, context, secgrp_ids):
|
||||
|
@ -276,10 +291,24 @@ class GceMechanismDriver(api.MechanismDriver):
|
|||
try:
|
||||
gce_rule_name = self._gce_secgrp_id(secgrp_rule['id'])
|
||||
gceutils.get_firewall_rule(compute, project, gce_rule_name)
|
||||
except gceutils.HttpError:
|
||||
except gce_errors.HttpError:
|
||||
self._create_secgrp_rule(context, secgrp_rule,
|
||||
network_link)
|
||||
|
||||
def _validate_secgrp(self, context, secgrp):
|
||||
secgrp_rules = secgrp['security_group_rules']
|
||||
try:
|
||||
for secgrp_rule in secgrp_rules:
|
||||
self._validate_secgrp_rule(secgrp_rule)
|
||||
except Exception as e:
|
||||
try:
|
||||
core_plugin = NeutronManager.get_plugin()
|
||||
except AttributeError:
|
||||
core_plugin = directory.get_plugin()
|
||||
LOG.info('Rollback create security group: %s' % secgrp['id'])
|
||||
core_plugin.delete_security_group(context, secgrp['id'])
|
||||
raise e
|
||||
|
||||
def _update_secgrp(self, context, secgrp_id):
|
||||
try:
|
||||
core_plugin = NeutronManager.get_plugin()
|
||||
|
@ -315,6 +344,7 @@ class GceMechanismDriver(api.MechanismDriver):
|
|||
return True
|
||||
|
||||
def secgroup_callback(self, resource, event, trigger, **kwargs):
|
||||
LOG.debug("Secgrp_callback: %s %s %s" % (resource, event, kwargs))
|
||||
if resource == resources.SECURITY_GROUP_RULE:
|
||||
context = kwargs['context']
|
||||
if event == events.BEFORE_DELETE:
|
||||
|
@ -323,11 +353,16 @@ class GceMechanismDriver(api.MechanismDriver):
|
|||
elif event == events.BEFORE_UPDATE:
|
||||
rule_id = kwargs['security_group_rule_id']
|
||||
self._update_secgrp_rule(context, rule_id)
|
||||
elif event == events.BEFORE_CREATE:
|
||||
rule = kwargs['security_group_rule']
|
||||
self._validate_secgrp_rule(rule)
|
||||
elif resource == resources.SECURITY_GROUP:
|
||||
if event == events.BEFORE_DELETE:
|
||||
context = kwargs['context']
|
||||
security_group_id = kwargs.get('security_group_id')
|
||||
if security_group_id:
|
||||
self._delete_secgrp(context, security_group_id)
|
||||
else:
|
||||
LOG.warn("Security group ID not found in delete request")
|
||||
security_group_id = kwargs['security_group_id']
|
||||
self._delete_secgrp(context, security_group_id)
|
||||
elif event == events.BEFORE_RESPONSE:
|
||||
if kwargs['method_name'] == 'security_group.create.end':
|
||||
context = kwargs['context']
|
||||
secgrp = kwargs['data']['security_group']
|
||||
self._validate_secgrp(context, secgrp)
|
||||
|
|
|
@ -10,14 +10,16 @@ WARRANTIES OR CONDITIONS OF ANY KIND, either expressed or implied. See the
|
|||
License for the specific language governing permissions and limitations
|
||||
under the License.
|
||||
"""
|
||||
|
||||
import httplib2
|
||||
import mock
|
||||
import os
|
||||
|
||||
from googleapiclient import errors as gce_errors
|
||||
from neutron.extensions import securitygroup as sg
|
||||
from neutron.manager import NeutronManager
|
||||
from neutron.plugins.ml2.drivers.gce.mech_gce import GceMechanismDriver
|
||||
from neutron.plugins.ml2.drivers.gce.mech_gce import SecurityGroupInvalidDirection # noqa
|
||||
from neutron.plugins.ml2.drivers.gce.mech_gce import \
|
||||
SecurityGroupInvalidDirection # noqa
|
||||
from neutron.tests import base
|
||||
from neutron.tests.common.gce import gce_mock
|
||||
from neutron.tests.common.gce.gce_mock import FakeNeutronManager
|
||||
|
@ -138,7 +140,7 @@ class GCENeutronTestCase(test_sg.SecurityGroupsTestCase, base.BaseTestCase):
|
|||
sg_rule = self.get_fake_sg_rule()
|
||||
gce_rule = self._driver._convert_secgrp_rule_to_gce(
|
||||
sg_rule, NETWORK_LINK)
|
||||
self.assertTrue(isinstance(gce_rule, dict))
|
||||
self.assertIsInstance(gce_rule, dict)
|
||||
|
||||
@mock.patch('neutron.common.gceutils.wait_for_operation')
|
||||
@mock.patch('neutron.common.gceutils.create_firewall_rule')
|
||||
|
@ -153,6 +155,13 @@ class GCENeutronTestCase(test_sg.SecurityGroupsTestCase, base.BaseTestCase):
|
|||
self._driver.gce_project,
|
||||
gce_mock.fake_operation())
|
||||
|
||||
@mock.patch(neutron_get_plugin)
|
||||
def test_validate_sg_rule(self, mock_plugin):
|
||||
mock_plugin.side_effect = FakeNeutronManager
|
||||
sg_rule = self.get_fake_sg_rule()
|
||||
self.assertIsNone(
|
||||
self._driver._validate_secgrp_rule(sg_rule))
|
||||
|
||||
@mock.patch(neutron_get_plugin)
|
||||
@mock.patch('neutron.common.gceutils.wait_for_operation')
|
||||
@mock.patch('neutron.common.gceutils.update_firewall_rule')
|
||||
|
@ -179,3 +188,22 @@ class GCENeutronTestCase(test_sg.SecurityGroupsTestCase, base.BaseTestCase):
|
|||
mock_delete.assert_called_once_with(self._driver.gce_svc,
|
||||
self._driver.gce_project,
|
||||
"secgrp-" + sg_rule['id'])
|
||||
|
||||
@mock.patch('neutron.common.gceutils.wait_for_operation')
|
||||
@mock.patch('neutron.common.gceutils.delete_firewall_rule')
|
||||
def test_delete_sg_rule_with_error(self, mock_delete, mock_wait):
|
||||
http_error = gce_errors.HttpError(
|
||||
httplib2.Response({
|
||||
'status': 404,
|
||||
'reason': 'Not Found'
|
||||
}),
|
||||
content='')
|
||||
mock_delete.side_effect = http_error
|
||||
mock_wait.side_effect = gce_mock.wait_for_operation
|
||||
sg_rule = self.get_fake_sg_rule()
|
||||
self.assertIsNone(
|
||||
self._driver._delete_secgrp_rule(self.context, sg_rule['id']))
|
||||
mock_delete.assert_called_once_with(self._driver.gce_svc,
|
||||
self._driver.gce_project,
|
||||
"secgrp-" + sg_rule['id'])
|
||||
mock_wait.assert_not_called()
|
||||
|
|
Loading…
Reference in New Issue