Fix NFSHelper 0-length netmask bug
Add special case handling of 0-length netmasks in NFS helper. This is required because nfs-utils on Linux support CIDR style network access unless the netmask is length 0. Also split out utility function for wrapping IPv6 addresses with square brackets. Closes-bug: #1707946 Change-Id: Id907478837099b250a6c88b6b5ff50c214bcdbb2
This commit is contained in:
parent
5084efe621
commit
e3b6f4a40e
@ -14,9 +14,10 @@
|
|||||||
# under the License.
|
# under the License.
|
||||||
|
|
||||||
import copy
|
import copy
|
||||||
import netaddr
|
import ipaddress
|
||||||
import os
|
import os
|
||||||
import re
|
import re
|
||||||
|
import six
|
||||||
|
|
||||||
from oslo_log import log
|
from oslo_log import log
|
||||||
|
|
||||||
@ -190,16 +191,24 @@ class NFSHelper(NASHelperBase):
|
|||||||
if 'public_addresses' in server_copy:
|
if 'public_addresses' in server_copy:
|
||||||
for address in server_copy['public_addresses']:
|
for address in server_copy['public_addresses']:
|
||||||
public_addresses.append(
|
public_addresses.append(
|
||||||
self._get_parsed_address_or_cidr(address))
|
self._escaped_address(address))
|
||||||
server_copy['public_addresses'] = public_addresses
|
server_copy['public_addresses'] = public_addresses
|
||||||
|
|
||||||
for t in ['public_address', 'admin_ip', 'ip']:
|
for t in ['public_address', 'admin_ip', 'ip']:
|
||||||
address = server_copy.get(t)
|
address = server_copy.get(t)
|
||||||
if address is not None:
|
if address is not None:
|
||||||
server_copy[t] = self._get_parsed_address_or_cidr(address)
|
server_copy[t] = self._escaped_address(address)
|
||||||
|
|
||||||
return self.get_exports_for_share(server_copy, path)
|
return self.get_exports_for_share(server_copy, path)
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _escaped_address(address):
|
||||||
|
addr = ipaddress.ip_address(six.text_type(address))
|
||||||
|
if addr.version == 4:
|
||||||
|
return six.text_type(addr)
|
||||||
|
else:
|
||||||
|
return '[%s]' % six.text_type(addr)
|
||||||
|
|
||||||
def init_helper(self, server):
|
def init_helper(self, server):
|
||||||
try:
|
try:
|
||||||
self._ssh_exec(server, ['sudo', 'exportfs'])
|
self._ssh_exec(server, ['sudo', 'exportfs'])
|
||||||
@ -239,13 +248,13 @@ class NFSHelper(NASHelperBase):
|
|||||||
rules_options = '%s,no_subtree_check'
|
rules_options = '%s,no_subtree_check'
|
||||||
if access['access_level'] == const.ACCESS_LEVEL_RW:
|
if access['access_level'] == const.ACCESS_LEVEL_RW:
|
||||||
rules_options = ','.join((rules_options, 'no_root_squash'))
|
rules_options = ','.join((rules_options, 'no_root_squash'))
|
||||||
|
access_to = self._get_parsed_address_or_cidr(
|
||||||
|
access['access_to'])
|
||||||
self._ssh_exec(
|
self._ssh_exec(
|
||||||
server,
|
server,
|
||||||
['sudo', 'exportfs', '-o',
|
['sudo', 'exportfs', '-o',
|
||||||
rules_options % access['access_level'],
|
rules_options % access['access_level'],
|
||||||
':'.join((
|
':'.join((access_to, local_path))])
|
||||||
self._get_parsed_address_or_cidr(access['access_to']),
|
|
||||||
local_path))])
|
|
||||||
self._sync_nfs_temp_and_perm_files(server)
|
self._sync_nfs_temp_and_perm_files(server)
|
||||||
# Adding/Deleting specific rules
|
# Adding/Deleting specific rules
|
||||||
else:
|
else:
|
||||||
@ -255,7 +264,7 @@ class NFSHelper(NASHelperBase):
|
|||||||
(const.ACCESS_LEVEL_RO, const.ACCESS_LEVEL_RW))
|
(const.ACCESS_LEVEL_RO, const.ACCESS_LEVEL_RW))
|
||||||
|
|
||||||
for access in delete_rules:
|
for access in delete_rules:
|
||||||
access['access_to'] = self._get_parsed_address_or_cidr(
|
access_to = self._get_parsed_address_or_cidr(
|
||||||
access['access_to'])
|
access['access_to'])
|
||||||
try:
|
try:
|
||||||
self.validate_access_rules(
|
self.validate_access_rules(
|
||||||
@ -271,15 +280,15 @@ class NFSHelper(NASHelperBase):
|
|||||||
'to': access['access_to']})
|
'to': access['access_to']})
|
||||||
continue
|
continue
|
||||||
self._ssh_exec(server, ['sudo', 'exportfs', '-u',
|
self._ssh_exec(server, ['sudo', 'exportfs', '-u',
|
||||||
':'.join((access['access_to'], local_path))])
|
':'.join((access_to, local_path))])
|
||||||
if delete_rules:
|
if delete_rules:
|
||||||
self._sync_nfs_temp_and_perm_files(server)
|
self._sync_nfs_temp_and_perm_files(server)
|
||||||
for access in add_rules:
|
for access in add_rules:
|
||||||
access['access_to'] = self._get_parsed_address_or_cidr(
|
access_to = self._get_parsed_address_or_cidr(
|
||||||
access['access_to'])
|
access['access_to'])
|
||||||
found_item = re.search(
|
found_item = re.search(
|
||||||
re.escape(local_path) + '[\s\n]*' + re.escape(
|
re.escape(local_path) + '[\s\n]*' + re.escape(access_to),
|
||||||
access['access_to']), out)
|
out)
|
||||||
if found_item is not None:
|
if found_item is not None:
|
||||||
LOG.warning("Access rule %(type)s:%(to)s already "
|
LOG.warning("Access rule %(type)s:%(to)s already "
|
||||||
"exists for share %(name)s" % {
|
"exists for share %(name)s" % {
|
||||||
@ -296,18 +305,18 @@ class NFSHelper(NASHelperBase):
|
|||||||
server,
|
server,
|
||||||
['sudo', 'exportfs', '-o',
|
['sudo', 'exportfs', '-o',
|
||||||
rules_options % access['access_level'],
|
rules_options % access['access_level'],
|
||||||
':'.join((access['access_to'], local_path))])
|
':'.join((access_to, local_path))])
|
||||||
if add_rules:
|
if add_rules:
|
||||||
self._sync_nfs_temp_and_perm_files(server)
|
self._sync_nfs_temp_and_perm_files(server)
|
||||||
|
|
||||||
def _get_parsed_address_or_cidr(self, access_to):
|
@staticmethod
|
||||||
try:
|
def _get_parsed_address_or_cidr(access_to):
|
||||||
network = netaddr.IPNetwork(access_to)
|
network = ipaddress.ip_network(six.text_type(access_to))
|
||||||
except netaddr.AddrFormatError:
|
mask_length = network.prefixlen
|
||||||
raise exception.InvalidInput(
|
address = six.text_type(network.network_address)
|
||||||
reason=_("Invalid address or cidr supplied %s.") % access_to)
|
if mask_length == 0:
|
||||||
mask_length = network.netmask.netmask_bits()
|
# Special case because Linux exports don't support /0 netmasks
|
||||||
address = access_to.split('/')[0]
|
return '*'
|
||||||
if network.version == 4:
|
if network.version == 4:
|
||||||
if mask_length == 32:
|
if mask_length == 32:
|
||||||
return address
|
return address
|
||||||
|
@ -150,11 +150,11 @@ class NFSHelperTestCase(test.TestCase):
|
|||||||
add_rules = [
|
add_rules = [
|
||||||
test_generic.get_fake_access_rule('2.2.2.2', access_level),
|
test_generic.get_fake_access_rule('2.2.2.2', access_level),
|
||||||
test_generic.get_fake_access_rule('2.2.2.3', access_level),
|
test_generic.get_fake_access_rule('2.2.2.3', access_level),
|
||||||
test_generic.get_fake_access_rule('5.5.5.5/24', access_level)]
|
test_generic.get_fake_access_rule('5.5.5.0/24', access_level)]
|
||||||
delete_rules = [
|
delete_rules = [
|
||||||
test_generic.get_fake_access_rule('3.3.3.3', access_level),
|
test_generic.get_fake_access_rule('3.3.3.3', access_level),
|
||||||
test_generic.get_fake_access_rule('4.4.4.4', access_level, 'user'),
|
test_generic.get_fake_access_rule('4.4.4.4', access_level, 'user'),
|
||||||
test_generic.get_fake_access_rule('6.6.6.6/0', access_level)]
|
test_generic.get_fake_access_rule('0.0.0.0/0', access_level)]
|
||||||
self._helper.update_access(self.server, self.share_name, access_rules,
|
self._helper.update_access(self.server, self.share_name, access_rules,
|
||||||
add_rules=add_rules,
|
add_rules=add_rules,
|
||||||
delete_rules=delete_rules)
|
delete_rules=delete_rules)
|
||||||
@ -164,14 +164,14 @@ class NFSHelperTestCase(test.TestCase):
|
|||||||
mock.call(self.server, ['sudo', 'exportfs', '-u',
|
mock.call(self.server, ['sudo', 'exportfs', '-u',
|
||||||
':'.join(['3.3.3.3', local_path])]),
|
':'.join(['3.3.3.3', local_path])]),
|
||||||
mock.call(self.server, ['sudo', 'exportfs', '-u',
|
mock.call(self.server, ['sudo', 'exportfs', '-u',
|
||||||
':'.join(['6.6.6.6/0',
|
':'.join(['*',
|
||||||
local_path])]),
|
local_path])]),
|
||||||
mock.call(self.server, ['sudo', 'exportfs', '-o',
|
mock.call(self.server, ['sudo', 'exportfs', '-o',
|
||||||
expected_mount_options % access_level,
|
expected_mount_options % access_level,
|
||||||
':'.join(['2.2.2.2', local_path])]),
|
':'.join(['2.2.2.2', local_path])]),
|
||||||
mock.call(self.server, ['sudo', 'exportfs', '-o',
|
mock.call(self.server, ['sudo', 'exportfs', '-o',
|
||||||
expected_mount_options % access_level,
|
expected_mount_options % access_level,
|
||||||
':'.join(['5.5.5.5/24',
|
':'.join(['5.5.5.0/24',
|
||||||
local_path])]),
|
local_path])]),
|
||||||
])
|
])
|
||||||
self._helper._sync_nfs_temp_and_perm_files.assert_has_calls([
|
self._helper._sync_nfs_temp_and_perm_files.assert_has_calls([
|
||||||
@ -190,7 +190,7 @@ class NFSHelperTestCase(test.TestCase):
|
|||||||
|
|
||||||
@ddt.data('10.0.0.265', '10.0.0.1/33', '1001::10069', '1001::1000/129')
|
@ddt.data('10.0.0.265', '10.0.0.1/33', '1001::10069', '1001::1000/129')
|
||||||
def test__get_parsed_address_or_cidr_with_invalid_access(self, access):
|
def test__get_parsed_address_or_cidr_with_invalid_access(self, access):
|
||||||
self.assertRaises(exception.InvalidInput,
|
self.assertRaises(ValueError,
|
||||||
self._helper._get_parsed_address_or_cidr,
|
self._helper._get_parsed_address_or_cidr,
|
||||||
access)
|
access)
|
||||||
|
|
||||||
|
@ -0,0 +1,5 @@
|
|||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- Fixed application of access rules with type ``ip`` and netmask length 0 in
|
||||||
|
the ``NFSHelper`` plugin, affecting LVM and Generic drivers. Previously
|
||||||
|
these rules silently failed to apply.
|
Loading…
Reference in New Issue
Block a user