openstack
/
manila
Code Issues Proposed changes

Merge "Enable IPv6 in manila(allow access)"

This commit is contained in:
Jenkins 2017-07-18 14:50:45 +00:00 committed by Gerrit Code Review
parent 0ebb762c0c 031dd4b335
commit 6a9b665b5c
15 changed files with 188 additions and 140 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
manila/api/common.py
View File

@ -13,12 +13,15 @@
# License for the specific language governing permissions and limitations
# under the License.
import ipaddress
import os
import re
import six
import string
from oslo_config import cfg
from oslo_log import log
from oslo_utils import encodeutils
from six.moves.urllib import parse
import webob
@ -343,32 +346,6 @@ def validate_username(access):
raise webob.exc.HTTPBadRequest(explanation=exc_str)
def validate_ip_range(ip_range):
ip_range = ip_range.split('/')
exc_str = ('Supported ip format examples:\n'
'\t10.0.0.2, 10.0.0.0/24')
if len(ip_range) > 2:
raise webob.exc.HTTPBadRequest(explanation=exc_str)
if len(ip_range) == 2:
try:
prefix = int(ip_range[1])
if prefix < 0 or prefix > 32:
raise ValueError()
except ValueError:
msg = 'IP prefix should be in range from 0 to 32.'
raise webob.exc.HTTPBadRequest(explanation=msg)
ip_range = ip_range[0].split('.')
if len(ip_range) != 4:
raise webob.exc.HTTPBadRequest(explanation=exc_str)
for item in ip_range:
try:
if 0 <= int(item) <= 255:
continue
raise ValueError()
except ValueError:
raise webob.exc.HTTPBadRequest(explanation=exc_str)
def validate_cephx_id(cephx_id):
if not cephx_id:
raise webob.exc.HTTPBadRequest(explanation=_(
@ -389,14 +366,27 @@ def validate_cephx_id(cephx_id):
'Ceph IDs may not contain periods.'))
def validate_ip(access_to, enable_ipv6):
try:
if enable_ipv6:
validator = ipaddress.ip_network
else:
validator = ipaddress.IPv4Network
validator(six.text_type(access_to))
except ValueError as error:
err_msg = encodeutils.exception_to_unicode(error)
raise webob.exc.HTTPBadRequest(explanation=err_msg)
def validate_access(*args, **kwargs):
access_type = kwargs.get('access_type')
access_to = kwargs.get('access_to')
enable_ceph = kwargs.get('enable_ceph')
enable_ipv6 = kwargs.get('enable_ipv6')
if access_type == 'ip':
validate_ip_range(access_to)
validate_ip(access_to, enable_ipv6)
elif access_type == 'user':
validate_username(access_to)
elif access_type == 'cert':

4
manila/api/openstack/api_version_request.py
View File

@ -107,6 +107,8 @@ REST_API_VERSION_HISTORY = """
* 2.36 - Added like filter support in ``shares``, ``snapshots``,
``share-networks``, ``share-groups`` list APIs.
* 2.37 - Added /messages APIs.
* 2.38 - Support IPv6 validation in allow_access API to enable IPv6 in
manila.
"""
@ -114,7 +116,7 @@ REST_API_VERSION_HISTORY = """
# The default api version request is defined to be the
# minimum version of the API supported.
_MIN_API_VERSION = "2.0"
_MAX_API_VERSION = "2.37"
_MAX_API_VERSION = "2.38"
DEFAULT_API_VERSION = _MIN_API_VERSION

4
manila/api/openstack/rest_api_version_history.rst
View File

@ -214,3 +214,7 @@ user documentation.
2.37
----
Added /messages APIs.
2.38
----
Support IPv6 format validation in allow_access API to enable IPv6.

5
manila/api/v1/shares.py
View File

@ -372,7 +372,7 @@ class ShareMixin(object):
@wsgi.Controller.authorize('allow_access')
def _allow_access(self, req, id, body, enable_ceph=False,
allow_on_error_status=False):
allow_on_error_status=False, enable_ipv6=False):
"""Add share access rule."""
context = req.environ['manila.context']
access_data = body.get('allow_access', body.get('os-allow_access'))
@ -394,7 +394,8 @@ class ShareMixin(object):
access_to = access_data['access_to']
common.validate_access(access_type=access_type,
access_to=access_to,
enable_ceph=enable_ceph)
enable_ceph=enable_ceph,
enable_ipv6=enable_ipv6)
try:
access = self.share_api.allow_access(
context, share, access_type, access_to,

11
manila/api/v2/share_snapshots.py
View File

@ -159,7 +159,7 @@ class ShareSnapshotsController(share_snapshots.ShareSnapshotMixin,
msg = _("Required parameter %s is empty.") % parameter
raise exc_response(explanation=msg)
def _allow(self, req, id, body):
def _allow(self, req, id, body, enable_ipv6=False):
context = req.environ['manila.context']
if not (body and self.is_valid_body(body, 'allow_access')):
@ -175,7 +175,9 @@ class ShareSnapshotsController(share_snapshots.ShareSnapshotMixin,
access_type = access_data['access_type']
access_to = access_data['access_to']
common.validate_access(access_type=access_type, access_to=access_to)
common.validate_access(access_type=access_type,
access_to=access_to,
enable_ipv6=enable_ipv6)
snapshot = self.share_api.get_snapshot(context, id)
@ -270,7 +272,10 @@ class ShareSnapshotsController(share_snapshots.ShareSnapshotMixin,
@wsgi.response(202)
@wsgi.Controller.authorize
def allow_access(self, req, id, body=None):
return self._allow(req, id, body)
enable_ipv6 = False
if req.api_version_request >= api_version.APIVersionRequest("2.38"):
enable_ipv6 = True
return self._allow(req, id, body, enable_ipv6)
@wsgi.Controller.api_version('2.32')
@wsgi.action('deny_access')

2
manila/api/v2/shares.py
View File

@ -343,6 +343,8 @@ class ShareController(shares.ShareMixin,
kwargs['enable_ceph'] = True
if req.api_version_request >= api_version.APIVersionRequest("2.28"):
kwargs['allow_on_error_status'] = True
if req.api_version_request >= api_version.APIVersionRequest("2.38"):
kwargs['enable_ipv6'] = True
return self._allow_access(*args, **kwargs)

16
manila/tests/api/test_common.py
View File

@ -259,19 +259,23 @@ class MiscFunctionsTest(test.TestCase):
def test_validate_cephx_id_valid(self, test_id):
common.validate_cephx_id(test_id)
@ddt.data(['ip', '1.1.1.1', False], ['user', 'alice', False],
['cert', 'alice', False], ['cephx', 'alice', True],
['ip', '172.24.41.0/24', False],)
@ddt.data(['ip', '1.1.1.1', False, False], ['user', 'alice', False, False],
['cert', 'alice', False, False], ['cephx', 'alice', True, False],
['ip', '172.24.41.0/24', False, False],
['ip', '1001::1001', False, True],
['ip', '1001::1000/120', False, True])
@ddt.unpack
def test_validate_access(self, access_type, access_to, ceph):
def test_validate_access(self, access_type, access_to, ceph, enable_ipv6):
common.validate_access(access_type=access_type, access_to=access_to,
enable_ceph=ceph)
enable_ceph=ceph, enable_ipv6=enable_ipv6)
@ddt.data(['ip', 'alice', False], ['ip', '1.1.1.0/10/12', False],
['ip', '255.255.255.265', False], ['ip', '1.1.1.0/34', False],
['cert', '', False], ['cephx', 'client.alice', True],
['group', 'alice', True], ['cephx', 'alice', False],
['cephx', '', True], ['user', 'bob', False])
['cephx', '', True], ['user', 'bob', False],
['ip', '1001::1001/256', False],
['ip', '1001:1001/256', False],)
@ddt.unpack
def test_validate_access_exception(self, access_type, access_to, ceph):
self.assertRaises(webob.exc.HTTPBadRequest, common.validate_access,

10
manila/tests/api/v2/test_share_snapshots.py
View File

@ -364,7 +364,11 @@ class ShareSnapshotAPITest(test.TestCase):
self.assertEqual(expected, actual['snapshot_access_list'])
def test_allow_access(self):
@ddt.data(('1.1.1.1', '2.32'),
('1.1.1.1', '2.38'),
('1001::1001', '2.38'))
@ddt.unpack
def test_allow_access(self, ip_address, version):
share = db_utils.create_share(mount_snapshot_support=True)
snapshot = db_utils.create_snapshot(
status=constants.STATUS_AVAILABLE, share_id=share['id'])
@ -372,7 +376,7 @@ class ShareSnapshotAPITest(test.TestCase):
access = {
'id': 'fake_id',
'access_type': 'ip',
'access_to': '1.1.1.1',
'access_to': ip_address,
'state': 'new',
}
@ -384,7 +388,7 @@ class ShareSnapshotAPITest(test.TestCase):
mock.Mock(return_value=access))
body = {'allow_access': access}
req = fakes.HTTPRequest.blank('/snapshots/%s/action' % snapshot['id'],
version='2.32')
version=version)
actual = self.controller.allow_access(req, snapshot['id'], body)

92
manila/tests/api/v2/test_shares.py
View File

@ -1892,17 +1892,37 @@ class ShareActionsTest(test.TestCase):
self.controller = shares.ShareController()
self.mock_object(share_api.API, 'get', stubs.stub_share_get)
@ddt.unpack
@ddt.data(
{'access_type': 'ip', 'access_to': '127.0.0.1'},
{'access_type': 'user', 'access_to': '1' * 4},
{'access_type': 'user', 'access_to': '1' * 255},
{'access_type': 'user', 'access_to': 'fake\\]{.-_\'`;}['},
{'access_type': 'user', 'access_to': 'MYDOMAIN\\Administrator'},
{'access_type': 'cert', 'access_to': 'x'},
{'access_type': 'cert', 'access_to': 'tenant.example.com'},
{'access_type': 'cert', 'access_to': 'x' * 64},
{"access": {'access_type': 'ip', 'access_to': '127.0.0.1'},
"version": "2.7"},
{"access": {'access_type': 'user', 'access_to': '1' * 4},
"version": "2.7"},
{"access": {'access_type': 'user', 'access_to': '1' * 255},
"version": "2.7"},
{"access": {'access_type': 'user', 'access_to': 'fake\\]{.-_\'`;}['},
"version": "2.7"},
{"access": {'access_type': 'user',
'access_to': 'MYDOMAIN\\Administrator'},
"version": "2.7"},
{"access": {'access_type': 'cert', 'access_to': 'x'},
"version": "2.7"},
{"access": {'access_type': 'cert', 'access_to': 'tenant.example.com'},
"version": "2.7"},
{"access": {'access_type': 'cert', 'access_to': 'x' * 64},
"version": "2.7"},
{"access": {'access_type': 'ip', 'access_to': 'ad80::abaa:0:c2:2'},
"version": "2.38"},
{"access": {'access_type': 'ip', 'access_to': 'AD80:ABAA::'},
"version": "2.38"},
{"access": {'access_type': 'ip', 'access_to': 'AD80::/36'},
"version": "2.38"},
{"access": {'access_type': 'ip', 'access_to': 'AD80:ABAA::/128'},
"version": "2.38"},
{"access": {'access_type': 'ip', 'access_to': '127.0.0.1'},
"version": "2.38"},
)
def test_allow_access(self, access):
def test_allow_access(self, access, version):
self.mock_object(share_api.API,
'allow_access',
mock.Mock(return_value={'fake': 'fake'}))
@ -1913,31 +1933,55 @@ class ShareActionsTest(test.TestCase):
body = {'allow_access': access}
expected = {'access': {'fake': 'fake'}}
req = fakes.HTTPRequest.blank(
'/v2/tenant1/shares/%s/action' % id, version="2.7")
'/v2/tenant1/shares/%s/action' % id, version=version)
res = self.controller.allow_access(req, id, body)
self.assertEqual(expected, res)
@ddt.unpack
@ddt.data(
{'access_type': 'error_type', 'access_to': '127.0.0.1'},
{'access_type': 'ip', 'access_to': 'localhost'},
{'access_type': 'ip', 'access_to': '127.0.0.*'},
{'access_type': 'ip', 'access_to': '127.0.0.0/33'},
{'access_type': 'ip', 'access_to': '127.0.0.256'},
{'access_type': 'user', 'access_to': '1'},
{'access_type': 'user', 'access_to': '1' * 3},
{'access_type': 'user', 'access_to': '1' * 256},
{'access_type': 'user', 'access_to': 'root^'},
{'access_type': 'cert', 'access_to': ''},
{'access_type': 'cert', 'access_to': ' '},
{'access_type': 'cert', 'access_to': 'x' * 65},
{"access": {'access_type': 'error_type',
'access_to': '127.0.0.1'},
"version": "2.7"},
{"access": {'access_type': 'ip', 'access_to': 'localhost'},
"version": "2.7"},
{"access": {'access_type': 'ip', 'access_to': '127.0.0.*'},
"version": "2.7"},
{"access": {'access_type': 'ip', 'access_to': '127.0.0.0/33'},
"version": "2.7"},
{"access": {'access_type': 'ip', 'access_to': '127.0.0.256'},
"version": "2.7"},
{"access": {'access_type': 'user', 'access_to': '1'},
"version": "2.7"},
{"access": {'access_type': 'user', 'access_to': '1' * 3},
"version": "2.7"},
{"access": {'access_type': 'user', 'access_to': '1' * 256},
"version": "2.7"},
{"access": {'access_type': 'user', 'access_to': 'root^'},
"version": "2.7"},
{"access": {'access_type': 'cert', 'access_to': ''},
"version": "2.7"},
{"access": {'access_type': 'cert', 'access_to': ' '},
"version": "2.7"},
{"access": {'access_type': 'cert', 'access_to': 'x' * 65},
"version": "2.7"},
{"access": {'access_type': 'ip', 'access_to': 'ad80::abaa:0:c2:2'},
"version": "2.37"},
{"access": {'access_type': 'ip', 'access_to': '127.4.0.3/33'},
"version": "2.38"},
{"access": {'access_type': 'ip', 'access_to': 'AD80:ABAA::*'},
"version": "2.38"},
{"access": {'access_type': 'ip', 'access_to': 'AD80::/129'},
"version": "2.38"},
{"access": {'access_type': 'ip', 'access_to': 'ad80::abaa:0:c2:2/64'},
"version": "2.38"},
)
def test_allow_access_error(self, access):
def test_allow_access_error(self, access, version):
id = 'fake_share_id'
body = {'allow_access': access}
req = fakes.HTTPRequest.blank('/v2/tenant1/shares/%s/action' % id,
version="2.7")
version=version)
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.allow_access, req, id, body)

2
manila_tempest_tests/config.py
View File

@ -30,7 +30,7 @@ ShareGroup = [
help="The minimum api microversion is configured to be the "
"value of the minimum microversion supported by Manila."),
cfg.StrOpt("max_api_microversion",
default="2.37",
default="2.38",
help="The maximum api microversion is configured to be the "
"value of the latest microversion supported by Manila."),
cfg.StrOpt("region",

29
manila_tempest_tests/tests/api/test_rules.py
View File

@ -13,9 +13,9 @@
# License for the specific language governing permissions and limitations
# under the License.
import itertools
import ddt
import itertools
from tempest import config
from tempest.lib import exceptions as lib_exc
import testtools
@ -92,11 +92,15 @@ class ShareIpRulesForNFSTest(base.BaseSharesTest):
cls.access_to = "2.2.2.2"
@tc.attr(base.TAG_POSITIVE, base.TAG_BACKEND)
@ddt.data(*set(['1.0', '2.9', '2.27', '2.28', LATEST_MICROVERSION]))
def test_create_delete_access_rules_with_one_ip(self, version):
# test data
access_to = "1.1.1.1"
@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()})
))
@ddt.unpack
def test_create_delete_access_rules_with_one_ip(self, version,
access_to):
# create rule
if utils.is_microversion_eq(version, '1.0'):
@ -140,11 +144,14 @@ class ShareIpRulesForNFSTest(base.BaseSharesTest):
rule_id=rule["id"], share_id=self.share['id'], version=version)
@tc.attr(base.TAG_POSITIVE, base.TAG_BACKEND)
@ddt.data(*set(['1.0', '2.9', '2.27', '2.28', LATEST_MICROVERSION]))
def test_create_delete_access_rule_with_cidr(self, version):
# test data
access_to = "1.2.3.4/32"
@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)})
))
@ddt.unpack
def test_create_delete_access_rule_with_cidr(self, version, access_to):
# create rule
if utils.is_microversion_eq(version, '1.0'):

71
manila_tempest_tests/tests/api/test_rules_negative.py
View File

@ -46,60 +46,23 @@ class ShareIpRulesForNFSNegativeTest(base.BaseSharesMixedTest):
cls.snap = cls.create_snapshot_wait_for_active(cls.share["id"])
@tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
@ddt.data('shares_client', 'shares_v2_client')
def test_create_access_rule_ip_with_wrong_target_1(self, client_name):
self.assertRaises(lib_exc.BadRequest,
getattr(self, client_name).create_access_rule,
self.share["id"], "ip", "1.2.3.256")
@tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
@ddt.data('shares_client', 'shares_v2_client')
def test_create_access_rule_ip_with_wrong_target_2(self, client_name):
self.assertRaises(lib_exc.BadRequest,
getattr(self, client_name).create_access_rule,
self.share["id"], "ip", "1.1.1.-")
@tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
@ddt.data('shares_client', 'shares_v2_client')
def test_create_access_rule_ip_with_wrong_target_3(self, client_name):
self.assertRaises(lib_exc.BadRequest,
getattr(self, client_name).create_access_rule,
self.share["id"], "ip", "1.2.3.4/33")
@tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
@ddt.data('shares_client', 'shares_v2_client')
def test_create_access_rule_ip_with_wrong_target_4(self, client_name):
self.assertRaises(lib_exc.BadRequest,
getattr(self, client_name).create_access_rule,
self.share["id"], "ip", "1.2.3.*")
@tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
@ddt.data('shares_client', 'shares_v2_client')
def test_create_access_rule_ip_with_wrong_target_5(self, client_name):
self.assertRaises(lib_exc.BadRequest,
getattr(self, client_name).create_access_rule,
self.share["id"], "ip", "1.2.3.*/23")
@tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
@ddt.data('shares_client', 'shares_v2_client')
def test_create_access_rule_ip_with_wrong_target_6(self, client_name):
self.assertRaises(lib_exc.BadRequest,
getattr(self, client_name).create_access_rule,
self.share["id"], "ip", "1.2.3.1|23")
@tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
@ddt.data('shares_client', 'shares_v2_client')
def test_create_access_rule_ip_with_wrong_target_7(self, client_name):
self.assertRaises(lib_exc.BadRequest,
getattr(self, client_name).create_access_rule,
self.share["id"], "ip", "1.2.3.1/-1")
@tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
@ddt.data('shares_client', 'shares_v2_client')
def test_create_access_rule_ip_with_wrong_target_8(self, client_name):
self.assertRaises(lib_exc.BadRequest,
getattr(self, client_name).create_access_rule,
self.share["id"], "ip", "1.2.3.1/")
@ddt.data('1.2.3.256',
'1.1.1.-',
'1.2.3.4/33',
'1.2.3.*',
'1.2.3.*/23',
'1.2.3.1|23',
'1.2.3.1/-1',
'1.2.3.1/',
'ad80::abaa:0:c2:2/-3',
'AD80:ABAA::|26',
'2001:DB8:2de:0:0:0:0:e13:200a',
)
def test_create_access_rule_ip_with_wrong_target(self, ip_address):
for client_name in ['shares_client', 'shares_v2_client']:
self.assertRaises(lib_exc.BadRequest,
getattr(self, client_name).create_access_rule,
self.share["id"], "ip", ip_address)
@tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
@ddt.data('shares_client', 'shares_v2_client')

8
manila_tempest_tests/tests/api/test_snapshot_rules_negative.py
View File

@ -48,10 +48,10 @@ class SnapshotIpRulesForNFSNegativeTest(
@tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
@ddt.data("1.2.3.256", "1.1.1.-", "1.2.3.4/33", "1.2.3.*", "1.2.3.*/23",
"1.2.3.1|23", "1.2.3.1/", "1.2.3.1/-1", "fe00::1",
"fe80::217:f2ff:fe07:ed62", "2001:db8::/48", "::1/128",
"ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff",
"2001:0db8:0000:85a3:0000:0000:ac1f:8001")
"1.2.3.1|23", "1.2.3.1/", "1.2.3.1/-1",
"fe80:217:f2ff:fe07:ed62", "2001:db8::1/148",
"ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff",
"2001:0db8:0000:85a3:0000:0000:ac1f:8001/64")
def test_create_access_rule_ip_with_wrong_target(self, target):
self.assertRaises(lib_exc.BadRequest,
self.shares_v2_client.create_snapshot_access_rule,

26
manila_tempest_tests/utils.py
View File

@ -13,6 +13,7 @@
# License for the specific language governing permissions and limitations
# under the License.
from netaddr import ip
import random
import re
@ -90,16 +91,33 @@ def skip_if_microversion_lt(microversion):
return lambda f: f
def rand_ip():
def rand_ip(network=False):
"""This uses the TEST-NET-3 range of reserved IP addresses.
Using this range, which are reserved solely for use in
documentation and example source code, should avoid any potential
conflicts in real-world testing.
"""
TEST_NET_3 = '203.0.113.'
final_octet = six.text_type(random.randint(0, 255))
return TEST_NET_3 + final_octet
test_net_3 = '203.0.113.'
address = test_net_3 + six.text_type(random.randint(0, 255))
if network:
mask_length = six.text_type(random.randint(24, 32))
address = '/'.join((address, mask_length))
ip_network = ip.IPNetwork(address)
return '/'.join((six.text_type(ip_network.network), mask_length))
return address
def rand_ipv6_ip(network=False):
"""This uses the IPv6 documentation range of 2001:DB8::/32"""
ran_add = ["%x" % random.randrange(0, 16**4) for i in range(6)]
address = "2001:0DB8:" + ":".join(ran_add)
if network:
mask_length = six.text_type(random.randint(32, 128))
address = '/'.join((address, mask_length))
ip_network = ip.IPNetwork(address)
return '/'.join((six.text_type(ip_network.network), mask_length))
return address
def choose_matching_backend(share, pools, share_type):

4
releasenotes/notes/add-ipv6-32d89161a9a1e0b4.yaml Normal file
View File

@ -0,0 +1,4 @@
---
features:
- Validation of IPv6 based addresses has been added
for allow access API when access type is IP.