From cf2b00324dc1b23e10b319b9370f88a2f86b3d27 Mon Sep 17 00:00:00 2001 From: Magesh GV Date: Mon, 5 Jan 2015 18:09:09 +0530 Subject: [PATCH] Enforce parent redirect in all update scenarios 1) Fixes Parent redirect enforcement when Parent ruleset is created after the child ruleset is asoociated with PTGs 2) Fixes child ruleset add/delete on parent ruleset update Change-Id: I2771d6ce2fc3dcac3e9ec3f7a9556933cd9aae48 Closes-bug: 1407636 --- .../grouppolicy/drivers/resource_mapping.py | 15 ++ .../grouppolicy/test_resource_mapping.py | 153 +++++++++++++++++- 2 files changed, 167 insertions(+), 1 deletion(-) diff --git a/gbpservice/neutron/services/grouppolicy/drivers/resource_mapping.py b/gbpservice/neutron/services/grouppolicy/drivers/resource_mapping.py index efd7d9c43..7047f3b0e 100644 --- a/gbpservice/neutron/services/grouppolicy/drivers/resource_mapping.py +++ b/gbpservice/neutron/services/grouppolicy/drivers/resource_mapping.py @@ -752,6 +752,8 @@ class ResourceMappingDriver(api.PolicyDriver): if context.current['child_policy_rule_sets']: self._recompute_policy_rule_sets( context, context.current['child_policy_rule_sets']) + self._handle_redirect_action( + context, context.current['child_policy_rule_sets']) @log.log def update_policy_rule_set_precommit(self, context): @@ -777,7 +779,17 @@ class ResourceMappingDriver(api.PolicyDriver): to_recompute = (set(context.original['child_policy_rule_sets']) & set(context.current['child_policy_rule_sets'])) self._recompute_policy_rule_sets(context, to_recompute) + # Handle any Redirects from the current Policy Rule Set self._handle_redirect_action(context, [context.current['id']]) + # Handle Update/Delete of Redirects for any child Rule Sets + if (set(context.original['child_policy_rule_sets']) != + set(context.current['child_policy_rule_sets'])): + if context.original['child_policy_rule_sets']: + self._handle_redirect_action( + context, context.original['child_policy_rule_sets']) + if context.current['child_policy_rule_sets']: + self._handle_redirect_action( + context, context.current['child_policy_rule_sets']) @log.log def delete_policy_rule_set_precommit(self, context): @@ -800,6 +812,9 @@ class ResourceMappingDriver(api.PolicyDriver): # Delete SGs for sg in sg_list: self._delete_sg(context._plugin_context, sg) + if context.current['child_policy_rule_sets']: + self._handle_redirect_action( + context, context.current['child_policy_rule_sets']) @log.log def delete_network_service_policy_postcommit(self, context): diff --git a/gbpservice/neutron/tests/unit/services/grouppolicy/test_resource_mapping.py b/gbpservice/neutron/tests/unit/services/grouppolicy/test_resource_mapping.py index 29dbea21b..1d946ffe9 100644 --- a/gbpservice/neutron/tests/unit/services/grouppolicy/test_resource_mapping.py +++ b/gbpservice/neutron/tests/unit/services/grouppolicy/test_resource_mapping.py @@ -1423,7 +1423,11 @@ class TestPolicyRuleSet(ResourceMappingTestCase): consumer_ptg_id, scs_id_list): self.assertEqual(sc_instance['provider_ptg_id'], provider_ptg_id) self.assertEqual(sc_instance['consumer_ptg_id'], consumer_ptg_id) - self.assertEqual(scs_id_list, sc_instance['servicechain_specs']) + # REVISIT(Magesh): List api for chain instance retrieves specs in + # different order. Functionally the order is fine on create/update + # and verified on devstack. + self.assertEqual(set(scs_id_list), + set(sc_instance['servicechain_specs'])) def test_redirect_to_chain(self): scs_id = self._create_servicechain_spec() @@ -1824,6 +1828,153 @@ class TestPolicyRuleSet(ResourceMappingTestCase): sc_instances = self.deserialize(self.fmt, res) self.assertEqual(len(sc_instances['servicechain_instances']), 0) + def test_enforce_parent_redirect_after_ptg_create(self): + scs_id = self._create_servicechain_spec() + _, classifier_id, policy_rule_id = self._create_tcp_redirect_rule( + "20:90", scs_id) + + child_prs = self.create_policy_rule_set( + name="prs", policy_rules=[policy_rule_id]) + child_prs_id = child_prs['policy_rule_set']['id'] + + self._verify_prs_rules(child_prs_id) + + provider_ptg = self.create_policy_target_group( + name="ptg1", provided_policy_rule_sets={child_prs_id: None}) + provider_ptg_id = provider_ptg['policy_target_group']['id'] + consumer_ptg = self.create_policy_target_group( + name="ptg2", + consumed_policy_rule_sets={child_prs_id: None}) + consumer_ptg_id = consumer_ptg['policy_target_group']['id'] + + self._verify_prs_rules(child_prs_id) + sc_node_list_req = self.new_list_request(SERVICECHAIN_INSTANCES) + res = sc_node_list_req.get_response(self.ext_api) + sc_instances = self.deserialize(self.fmt, res) + # We should have one service chain instance created now + self.assertEqual(len(sc_instances['servicechain_instances']), 1) + sc_instance = sc_instances['servicechain_instances'][0] + self.assertEqual(sc_instance['provider_ptg_id'], provider_ptg_id) + self.assertEqual(sc_instance['consumer_ptg_id'], consumer_ptg_id) + self.assertEqual(sc_instance['classifier_id'], classifier_id) + self.assertEqual(len(sc_instance['servicechain_specs']), 1) + + parent_scs_id = self._create_servicechain_spec(node_types='FIREWALL') + parent_action = self.create_policy_action( + name="action2", action_type=gconst.GP_ACTION_REDIRECT, + action_value=parent_scs_id) + parent_action_id = parent_action['policy_action']['id'] + parent_policy_rule = self.create_policy_rule( + name='pr1', policy_classifier_id=classifier_id, + policy_actions=[parent_action_id]) + parent_policy_rule_id = parent_policy_rule['policy_rule']['id'] + + parent_prs = self.create_policy_rule_set( + name="c1", policy_rules=[parent_policy_rule_id], + child_policy_rule_sets=[child_prs_id]) + parent_prs_id = parent_prs['policy_rule_set']['id'] + + self._verify_prs_rules(child_prs_id) + sc_node_list_req = self.new_list_request(SERVICECHAIN_INSTANCES) + res = sc_node_list_req.get_response(self.ext_api) + sc_instances = self.deserialize(self.fmt, res) + # We should have a new service chain instance created now from both + # parent and child specs + self.assertEqual(len(sc_instances['servicechain_instances']), 1) + sc_instance = sc_instances['servicechain_instances'][0] + self._assert_proper_chain_instance(sc_instance, provider_ptg_id, + consumer_ptg_id, + [parent_scs_id, scs_id]) + + # Delete parent ruleset and verify that the parent spec association + # is removed from servicechain instance + self.delete_policy_rule_set( + parent_prs_id, expected_res_status=webob.exc.HTTPNoContent.code) + self._verify_prs_rules(child_prs_id) + sc_node_list_req = self.new_list_request(SERVICECHAIN_INSTANCES) + res = sc_node_list_req.get_response(self.ext_api) + sc_instances = self.deserialize(self.fmt, res) + self.assertEqual(len(sc_instances['servicechain_instances']), 1) + sc_instance = sc_instances['servicechain_instances'][0] + self._assert_proper_chain_instance(sc_instance, provider_ptg_id, + consumer_ptg_id, [scs_id]) + + req = self.new_delete_request( + 'policy_target_groups', consumer_ptg_id) + res = req.get_response(self.ext_api) + self.assertEqual(res.status_int, webob.exc.HTTPNoContent.code) + sc_node_list_req = self.new_list_request(SERVICECHAIN_INSTANCES) + res = sc_node_list_req.get_response(self.ext_api) + sc_instances = self.deserialize(self.fmt, res) + self.assertEqual(len(sc_instances['servicechain_instances']), 0) + + def test_parent_ruleset_update_for_redirect(self): + scs_id = self._create_servicechain_spec() + _, classifier_id, policy_rule_id = self._create_tcp_redirect_rule( + "20:90", scs_id) + + child_prs = self.create_policy_rule_set( + name="prs", policy_rules=[policy_rule_id]) + child_prs_id = child_prs['policy_rule_set']['id'] + + self._verify_prs_rules(child_prs_id) + + parent_scs_id = self._create_servicechain_spec(node_types='FIREWALL') + parent_action = self.create_policy_action( + name="action2", action_type=gconst.GP_ACTION_REDIRECT, + action_value=parent_scs_id) + parent_action_id = parent_action['policy_action']['id'] + parent_policy_rule = self.create_policy_rule( + name='pr1', policy_classifier_id=classifier_id, + policy_actions=[parent_action_id]) + parent_policy_rule_id = parent_policy_rule['policy_rule']['id'] + + parent_prs = self.create_policy_rule_set( + name="c1", policy_rules=[parent_policy_rule_id]) + parent_prs_id = parent_prs['policy_rule_set']['id'] + + provider_ptg = self.create_policy_target_group( + name="ptg1", provided_policy_rule_sets={child_prs_id: None}) + provider_ptg_id = provider_ptg['policy_target_group']['id'] + consumer_ptg = self.create_policy_target_group( + name="ptg2", + consumed_policy_rule_sets={child_prs_id: None}) + consumer_ptg_id = consumer_ptg['policy_target_group']['id'] + + self._verify_prs_rules(child_prs_id) + sc_node_list_req = self.new_list_request(SERVICECHAIN_INSTANCES) + res = sc_node_list_req.get_response(self.ext_api) + sc_instances = self.deserialize(self.fmt, res) + # We should have one service chain instance created now + self.assertEqual(len(sc_instances['servicechain_instances']), 1) + sc_instance = sc_instances['servicechain_instances'][0] + self._assert_proper_chain_instance(sc_instance, provider_ptg_id, + consumer_ptg_id, [scs_id]) + + self.update_policy_rule_set(parent_prs_id, expected_res_status=200, + child_policy_rule_sets=[child_prs_id]) + + self._verify_prs_rules(child_prs_id) + sc_node_list_req = self.new_list_request(SERVICECHAIN_INSTANCES) + res = sc_node_list_req.get_response(self.ext_api) + sc_instances = self.deserialize(self.fmt, res) + # We should have a new service chain instance created now from both + # parent and child specs + self.assertEqual(len(sc_instances['servicechain_instances']), 1) + sc_instance = sc_instances['servicechain_instances'][0] + self._assert_proper_chain_instance(sc_instance, provider_ptg_id, + consumer_ptg_id, + [parent_scs_id, scs_id]) + + req = self.new_delete_request( + 'policy_target_groups', consumer_ptg_id) + res = req.get_response(self.ext_api) + self.assertEqual(res.status_int, webob.exc.HTTPNoContent.code) + sc_node_list_req = self.new_list_request(SERVICECHAIN_INSTANCES) + res = sc_node_list_req.get_response(self.ext_api) + sc_instances = self.deserialize(self.fmt, res) + self.assertEqual(len(sc_instances['servicechain_instances']), 0) + def test_shared_policy_rule_set_create_negative(self): self.create_policy_rule_set(shared=True, expected_res_status=400)