diff --git a/manila/share/access.py b/manila/share/access.py index d8b2816a54..10628485ee 100644 --- a/manila/share/access.py +++ b/manila/share/access.py @@ -373,6 +373,13 @@ class ShareInstanceAccess(ShareInstanceAccessDatabaseMixin): rules_to_be_removed_from_db, share_server): driver_rule_updates = {} + share_protocol = share_instance['share_proto'].lower() + if (not self.driver.ipv6_implemented and + share_protocol == 'nfs'): + add_rules = self._filter_ipv6_rules(add_rules) + delete_rules = self._filter_ipv6_rules(delete_rules) + access_rules_to_be_on_share = self._filter_ipv6_rules( + access_rules_to_be_on_share) try: driver_rule_updates = self.driver.update_access( context, @@ -463,10 +470,10 @@ class ShareInstanceAccess(ShareInstanceAccessDatabaseMixin): return access_rules_to_be_on_share @staticmethod - def _filter_ipv6_rules(rules, share_instance_proto): + def _filter_ipv6_rules(rules): filtered = [] for rule in rules: - if rule['access_type'] == 'ip' and share_instance_proto == 'nfs': + if rule['access_type'] == 'ip': ip_version = ipaddress.ip_network( six.text_type(rule['access_to'])).version if 6 == ip_version: @@ -496,13 +503,6 @@ class ShareInstanceAccess(ShareInstanceAccessDatabaseMixin): access_rules_to_be_on_share = [ r for r in existing_rules_in_db if r['id'] not in delete_rule_ids ] - share = self.db.share_get(context, share_instance['share_id']) - si_proto = share['share_proto'].lower() - if not self.driver.ipv6_implemented: - add_rules = self._filter_ipv6_rules(add_rules, si_proto) - delete_rules = self._filter_ipv6_rules(delete_rules, si_proto) - access_rules_to_be_on_share = self._filter_ipv6_rules( - access_rules_to_be_on_share, si_proto) return access_rules_to_be_on_share, add_rules, delete_rules def _check_needs_refresh(self, context, share_instance_id): diff --git a/manila/tests/share/test_access.py b/manila/tests/share/test_access.py index 3234a01133..900a9da3af 100644 --- a/manila/tests/share/test_access.py +++ b/manila/tests/share/test_access.py @@ -13,10 +13,12 @@ # License for the specific language governing permissions and limitations # under the License. -import itertools - import ddt import mock +import random + +import itertools +import six from manila.common import constants from manila import context @@ -721,55 +723,71 @@ class ShareInstanceAccessTestCase(test.TestCase): self.assertEqual(states[0], rule_1['state']) self.assertEqual(states[-1], rule_4['state']) - @ddt.data(('nfs', True), ('cifs', False), ('ceph', False)) + @ddt.data(('nfs', True, False), ('nfs', False, True), + ('cifs', True, False), ('cifs', False, False), + ('cephx', True, False), ('cephx', False, False)) @ddt.unpack - def test__filter_ipv6_rules(self, proto, filtered): + def test__update_rules_through_share_driver(self, proto, + enable_ipv6, filtered): + self.driver.ipv6_implemented = enable_ipv6 + share_instance = {'share_proto': proto} + pass_rules, fail_rules = self._get_pass_rules_and_fail_rules() + pass_add_rules, fail_add_rules = self._get_pass_rules_and_fail_rules() + pass_delete_rules, fail_delete_rules = ( + self._get_pass_rules_and_fail_rules()) + test_rules = pass_rules + fail_rules + test_add_rules = pass_add_rules + fail_add_rules + test_delete_rules = pass_delete_rules + fail_delete_rules + + fake_expect_driver_update_rules = pass_rules + update_access_call = self.mock_object( + self.access_helper.driver, 'update_access', + mock.Mock(return_value=pass_rules)) + driver_update_rules = ( + self.access_helper._update_rules_through_share_driver( + self.context, share_instance=share_instance, + access_rules_to_be_on_share=test_rules, + add_rules=test_add_rules, + delete_rules=test_delete_rules, + rules_to_be_removed_from_db=test_rules, + share_server=None)) + + if filtered: + update_access_call.assert_called_once_with( + self.context, share_instance, + pass_rules, add_rules=pass_add_rules, + delete_rules=pass_delete_rules, share_server=None) + else: + update_access_call.assert_called_once_with( + self.context, share_instance, test_rules, + add_rules=test_add_rules, delete_rules=test_delete_rules, + share_server=None) + self.assertEqual(fake_expect_driver_update_rules, driver_update_rules) + + def _get_pass_rules_and_fail_rules(self): + random_value = six.text_type(random.randint(10, 32)) pass_rules = [ { 'access_type': 'ip', - 'access_to': '1.1.1.1' + 'access_to': '1.1.1.' + random_value, }, { 'access_type': 'ip', - 'access_to': '1.1.1.0/24' + 'access_to': '1.1.%s.0/24' % random_value, }, { 'access_type': 'user', - 'access_to': 'fake_user' + 'access_to': 'fake_user' + random_value, }, ] fail_rules = [ { 'access_type': 'ip', - 'access_to': '1001::1001' + 'access_to': '1001::' + random_value, }, { 'access_type': 'ip', - 'access_to': '1001::/64' + 'access_to': '%s::/64' % random_value, }, ] - test_rules = pass_rules + fail_rules - filtered_rules = self.access_helper._filter_ipv6_rules( - test_rules, proto) - if filtered: - self.assertEqual(pass_rules, filtered_rules) - else: - self.assertEqual(test_rules, filtered_rules) - - def test__get_rules_to_send_to_driver(self): - self.driver.ipv6_implemented = False - - share = db_utils.create_share(status=constants.STATUS_AVAILABLE) - share_instance = share['instance'] - db_utils.create_access(share_id=share['id'], access_to='1001::/64', - state=constants.ACCESS_STATE_ACTIVE) - self.mock_object( - self.access_helper, 'get_and_update_share_instance_access_rules', - mock.Mock(side_effect=self.access_helper. - get_and_update_share_instance_access_rules)) - - access_rules_to_be_on_share, add_rules, delete_rules = ( - self.access_helper._get_rules_to_send_to_driver( - self.context, share_instance)) - self.assertEqual([], add_rules) - self.assertEqual([], delete_rules) + return pass_rules, fail_rules diff --git a/manila_tempest_tests/tests/api/test_rules.py b/manila_tempest_tests/tests/api/test_rules.py index 229263ed7b..0b44a5c8e9 100644 --- a/manila_tempest_tests/tests/api/test_rules.py +++ b/manila_tempest_tests/tests/api/test_rules.py @@ -93,15 +93,17 @@ class ShareIpRulesForNFSTest(base.BaseSharesTest): @tc.attr(base.TAG_POSITIVE, base.TAG_BACKEND) @ddt.data(*itertools.chain( - itertools.product({'1.0', '2.9', '2.37', LATEST_MICROVERSION}, - {utils.rand_ip()}), - itertools.product({'2.37', LATEST_MICROVERSION}, - {utils.rand_ipv6_ip()}) + itertools.product({'1.0', '2.9', '2.37', LATEST_MICROVERSION}, {4}), + itertools.product({'2.38', LATEST_MICROVERSION}, {6}) )) @ddt.unpack def test_create_delete_access_rules_with_one_ip(self, version, - access_to): + ip_version): + if ip_version == 4: + access_to = utils.rand_ip() + else: + access_to = utils.rand_ipv6_ip() # create rule if utils.is_microversion_eq(version, '1.0'): rule = self.shares_client.create_access_rule( @@ -145,14 +147,15 @@ class ShareIpRulesForNFSTest(base.BaseSharesTest): @tc.attr(base.TAG_POSITIVE, base.TAG_BACKEND) @ddt.data(*itertools.chain( - itertools.product({'1.0', '2.9', '2.37', LATEST_MICROVERSION}, - {utils.rand_ip(network=True)}), - itertools.product({'2.37', LATEST_MICROVERSION}, - {utils.rand_ipv6_ip(network=True)}) + itertools.product({'1.0', '2.9', '2.37', LATEST_MICROVERSION}, {4}), + itertools.product({'2.38', LATEST_MICROVERSION}, {6}) )) @ddt.unpack - def test_create_delete_access_rule_with_cidr(self, version, access_to): - + def test_create_delete_access_rule_with_cidr(self, version, ip_version): + if ip_version == 4: + access_to = utils.rand_ip(network=True) + else: + access_to = utils.rand_ipv6_ip(network=True) # create rule if utils.is_microversion_eq(version, '1.0'): rule = self.shares_client.create_access_rule( diff --git a/releasenotes/notes/bug-1707066-deny-ipv6-access-in-error-bce379ee310060f6.yaml b/releasenotes/notes/bug-1707066-deny-ipv6-access-in-error-bce379ee310060f6.yaml new file mode 100644 index 0000000000..5462776aac --- /dev/null +++ b/releasenotes/notes/bug-1707066-deny-ipv6-access-in-error-bce379ee310060f6.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - The access-allow API accepts ipv6 rules and ignores them if the + configured backend does not support ipv6 access rules. + However, when the access-deny API is invoked to remove such + rules, they used to be stuck in "denying" state. This bug has been + fixed and ipv6 access rules can be denied successfully.