Allow restricting access rules
Access rules can now have the visibility of sensible fields restricted, as well as the deletion can be prevented. To do so, two new parameters were implemented in the access create command: `restrict_visibility` and `restrict_deletion`. In order to drop the delete restriction, users must specify the `unrestrict` option while issuing the share access delete command. Depends-On: Iea422c9d6bc99a81cd88c5f4b7055d6a1cf97fdc Change-Id: I31899a563c621e6f799e320fd990f9e510a8a9cc
This commit is contained in:
parent
1734b45fa5
commit
4eccd68794
@ -27,7 +27,7 @@ from manilaclient import utils
|
|||||||
|
|
||||||
LOG = logging.getLogger(__name__)
|
LOG = logging.getLogger(__name__)
|
||||||
|
|
||||||
MAX_VERSION = '2.81'
|
MAX_VERSION = '2.82'
|
||||||
MIN_VERSION = '2.0'
|
MIN_VERSION = '2.0'
|
||||||
DEPRECATED_VERSION = '1.0'
|
DEPRECATED_VERSION = '1.0'
|
||||||
_VERSIONED_METHOD_MAP = {}
|
_VERSIONED_METHOD_MAP = {}
|
||||||
|
@ -48,6 +48,7 @@ LOCK_SUMMARY_ATTRIBUTES = [
|
|||||||
|
|
||||||
RESOURCE_TYPE_MANAGERS = {
|
RESOURCE_TYPE_MANAGERS = {
|
||||||
'share': 'shares',
|
'share': 'shares',
|
||||||
|
'access_rule': 'share_access_rules'
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@ -84,6 +84,30 @@ class ShareAccessAllow(command.ShowOne):
|
|||||||
action='store_true',
|
action='store_true',
|
||||||
help=_("Wait for share access rule creation.")
|
help=_("Wait for share access rule creation.")
|
||||||
)
|
)
|
||||||
|
parser.add_argument(
|
||||||
|
"--lock-visibility",
|
||||||
|
action='store_true',
|
||||||
|
default=False,
|
||||||
|
help=_("Whether the sensitive fields of the access rule redacted "
|
||||||
|
"to other users. Only available with API version >= 2.82.")
|
||||||
|
)
|
||||||
|
parser.add_argument(
|
||||||
|
"--lock-deletion",
|
||||||
|
action='store_true',
|
||||||
|
default=False,
|
||||||
|
help=_("When enabled, a 'delete' lock will be placed against the "
|
||||||
|
"rule and the rule cannot be deleted while the lock "
|
||||||
|
"exists. Only available with API version >= 2.82.")
|
||||||
|
)
|
||||||
|
parser.add_argument(
|
||||||
|
'--lock-reason',
|
||||||
|
metavar="<lock_reason>",
|
||||||
|
type=str,
|
||||||
|
default=None,
|
||||||
|
help=_("Reason for locking the access rule. Should only be "
|
||||||
|
"provided alongside a deletion or visibility lock. "
|
||||||
|
"Only available with API version >= 2.82.")
|
||||||
|
)
|
||||||
return parser
|
return parser
|
||||||
|
|
||||||
def take_action(self, parsed_args):
|
def take_action(self, parsed_args):
|
||||||
@ -91,6 +115,28 @@ class ShareAccessAllow(command.ShowOne):
|
|||||||
|
|
||||||
share = apiutils.find_resource(share_client.shares,
|
share = apiutils.find_resource(share_client.shares,
|
||||||
parsed_args.share)
|
parsed_args.share)
|
||||||
|
lock_kwargs = {}
|
||||||
|
if parsed_args.lock_visibility:
|
||||||
|
lock_kwargs['lock_visibility'] = parsed_args.lock_visibility
|
||||||
|
if parsed_args.lock_deletion:
|
||||||
|
lock_kwargs['lock_deletion'] = parsed_args.lock_deletion
|
||||||
|
if parsed_args.lock_reason:
|
||||||
|
lock_kwargs['lock_reason'] = parsed_args.lock_reason
|
||||||
|
|
||||||
|
if (lock_kwargs
|
||||||
|
and share_client.api_version < api_versions.APIVersion(
|
||||||
|
"2.82")):
|
||||||
|
raise exceptions.CommandError(
|
||||||
|
'Restricted access rules are only available starting '
|
||||||
|
'from API version 2.82.')
|
||||||
|
|
||||||
|
if (lock_kwargs.get('lock_reason', None)
|
||||||
|
and not (lock_kwargs.get('lock_visibility', None)
|
||||||
|
or lock_kwargs.get('lock_deletion', None))):
|
||||||
|
raise exceptions.CommandError(
|
||||||
|
'Lock reason can only be set while locking the deletion or '
|
||||||
|
'visibility.')
|
||||||
|
|
||||||
properties = {}
|
properties = {}
|
||||||
if parsed_args.properties:
|
if parsed_args.properties:
|
||||||
if share_client.api_version >= api_versions.APIVersion("2.45"):
|
if share_client.api_version >= api_versions.APIVersion("2.45"):
|
||||||
@ -104,7 +150,8 @@ class ShareAccessAllow(command.ShowOne):
|
|||||||
access_type=parsed_args.access_type,
|
access_type=parsed_args.access_type,
|
||||||
access=parsed_args.access_to,
|
access=parsed_args.access_to,
|
||||||
access_level=parsed_args.access_level,
|
access_level=parsed_args.access_level,
|
||||||
metadata=properties
|
metadata=properties,
|
||||||
|
**lock_kwargs
|
||||||
)
|
)
|
||||||
if parsed_args.wait:
|
if parsed_args.wait:
|
||||||
if not oscutils.wait_for_status(
|
if not oscutils.wait_for_status(
|
||||||
@ -154,6 +201,13 @@ class ShareAccessDeny(command.Command):
|
|||||||
default=False,
|
default=False,
|
||||||
help=_("Wait for share access rule deletion")
|
help=_("Wait for share access rule deletion")
|
||||||
)
|
)
|
||||||
|
parser.add_argument(
|
||||||
|
"--unrestrict",
|
||||||
|
action='store_true',
|
||||||
|
default=False,
|
||||||
|
help=_("Seek access rule deletion despite restrictions. Only "
|
||||||
|
"available with API version >= 2.82.")
|
||||||
|
)
|
||||||
return parser
|
return parser
|
||||||
|
|
||||||
def take_action(self, parsed_args):
|
def take_action(self, parsed_args):
|
||||||
@ -161,9 +215,17 @@ class ShareAccessDeny(command.Command):
|
|||||||
|
|
||||||
share = apiutils.find_resource(share_client.shares,
|
share = apiutils.find_resource(share_client.shares,
|
||||||
parsed_args.share)
|
parsed_args.share)
|
||||||
|
kwargs = {}
|
||||||
|
if parsed_args.unrestrict:
|
||||||
|
if share_client.api_version < api_versions.APIVersion("2.82"):
|
||||||
|
raise exceptions.CommandError(
|
||||||
|
'Restricted access rules are only available starting from '
|
||||||
|
'API version 2.82.')
|
||||||
|
kwargs['unrestrict'] = True
|
||||||
|
|
||||||
error = None
|
error = None
|
||||||
try:
|
try:
|
||||||
share.deny(parsed_args.id)
|
share.deny(parsed_args.id, **kwargs)
|
||||||
if parsed_args.wait:
|
if parsed_args.wait:
|
||||||
if not oscutils.wait_for_delete(
|
if not oscutils.wait_for_delete(
|
||||||
manager=share_client.share_access_rules,
|
manager=share_client.share_access_rules,
|
||||||
@ -201,6 +263,30 @@ class ListShareAccess(command.Lister):
|
|||||||
'OPTIONAL: Default=None. '
|
'OPTIONAL: Default=None. '
|
||||||
'Available only for API microversion >= 2.45'),
|
'Available only for API microversion >= 2.45'),
|
||||||
)
|
)
|
||||||
|
parser.add_argument(
|
||||||
|
"--access-type",
|
||||||
|
metavar="<access_type>",
|
||||||
|
default=None,
|
||||||
|
help=_("Filter access rules by the access type.")
|
||||||
|
)
|
||||||
|
parser.add_argument(
|
||||||
|
"--access-key",
|
||||||
|
metavar="<access_key>",
|
||||||
|
default=None,
|
||||||
|
help=_("Filter access rules by the access key.")
|
||||||
|
)
|
||||||
|
parser.add_argument(
|
||||||
|
"--access-to",
|
||||||
|
metavar="<access_to>",
|
||||||
|
default=None,
|
||||||
|
help=_("Filter access rules by the access to field.")
|
||||||
|
)
|
||||||
|
parser.add_argument(
|
||||||
|
"--access-level",
|
||||||
|
metavar="<access_level>",
|
||||||
|
default=None,
|
||||||
|
help=_("Filter access rules by the access level.")
|
||||||
|
)
|
||||||
return parser
|
return parser
|
||||||
|
|
||||||
def take_action(self, parsed_args):
|
def take_action(self, parsed_args):
|
||||||
@ -208,9 +294,33 @@ class ListShareAccess(command.Lister):
|
|||||||
|
|
||||||
share = apiutils.find_resource(share_client.shares,
|
share = apiutils.find_resource(share_client.shares,
|
||||||
parsed_args.share)
|
parsed_args.share)
|
||||||
|
access_type = parsed_args.access_type
|
||||||
|
access_key = parsed_args.access_key
|
||||||
|
access_to = parsed_args.access_to
|
||||||
|
access_level = parsed_args.access_level
|
||||||
|
|
||||||
|
extended_filter_keys = {
|
||||||
|
'access_type': access_type,
|
||||||
|
'access_key': access_key,
|
||||||
|
'access_to': access_to,
|
||||||
|
'access_level': access_level
|
||||||
|
}
|
||||||
|
|
||||||
|
if (any(extended_filter_keys.values())
|
||||||
|
and share_client.api_version < api_versions.APIVersion(
|
||||||
|
"2.82")):
|
||||||
|
raise exceptions.CommandError(
|
||||||
|
'Filtering access rules by access_type, access_key, access_to '
|
||||||
|
'and access_level is available starting from API version '
|
||||||
|
'2.82.')
|
||||||
|
|
||||||
|
search_opts = {}
|
||||||
|
if share_client.api_version >= api_versions.APIVersion("2.82"):
|
||||||
|
for filter_key, filter_value in extended_filter_keys.items():
|
||||||
|
if filter_value:
|
||||||
|
search_opts[filter_key] = filter_value
|
||||||
|
|
||||||
if share_client.api_version >= api_versions.APIVersion("2.45"):
|
if share_client.api_version >= api_versions.APIVersion("2.45"):
|
||||||
search_opts = {}
|
|
||||||
if parsed_args.properties:
|
if parsed_args.properties:
|
||||||
search_opts = {
|
search_opts = {
|
||||||
'metadata': utils.extract_properties(
|
'metadata': utils.extract_properties(
|
||||||
|
@ -259,7 +259,11 @@ class OSCClientTestBase(base.ClientTestBase):
|
|||||||
|
|
||||||
def create_share_access_rule(self, share, access_type,
|
def create_share_access_rule(self, share, access_type,
|
||||||
access_to, properties=None,
|
access_to, properties=None,
|
||||||
access_level=None, wait=False):
|
access_level=None, wait=False,
|
||||||
|
lock_visibility=False,
|
||||||
|
lock_deletion=False,
|
||||||
|
lock_reason=None,
|
||||||
|
add_cleanup=False):
|
||||||
cmd = f'access create {share} {access_type} {access_to} '
|
cmd = f'access create {share} {access_type} {access_to} '
|
||||||
|
|
||||||
if access_level:
|
if access_level:
|
||||||
@ -267,7 +271,13 @@ class OSCClientTestBase(base.ClientTestBase):
|
|||||||
if properties:
|
if properties:
|
||||||
cmd += f'--properties {properties} '
|
cmd += f'--properties {properties} '
|
||||||
if wait:
|
if wait:
|
||||||
cmd += f'--wait'
|
cmd += '--wait '
|
||||||
|
if lock_visibility:
|
||||||
|
cmd += '--lock-visibility '
|
||||||
|
if lock_deletion:
|
||||||
|
cmd += '--lock-deletion '
|
||||||
|
if lock_reason:
|
||||||
|
cmd += f'--lock-reason {lock_reason}'
|
||||||
|
|
||||||
access_rule = self.dict_result('share', cmd)
|
access_rule = self.dict_result('share', cmd)
|
||||||
|
|
||||||
|
@ -37,7 +37,7 @@ LOCK_SUMMARY_ATTRIBUTES = [
|
|||||||
]
|
]
|
||||||
|
|
||||||
|
|
||||||
@utils.skip_if_microversion_not_supported('2.81')
|
@utils.skip_if_microversion_not_supported('2.82')
|
||||||
class ResourceLockTests(base.OSCClientTestBase):
|
class ResourceLockTests(base.OSCClientTestBase):
|
||||||
"""Lock CLI test cases"""
|
"""Lock CLI test cases"""
|
||||||
|
|
||||||
|
@ -10,11 +10,13 @@
|
|||||||
# License for the specific language governing permissions and limitations
|
# License for the specific language governing permissions and limitations
|
||||||
# under the License.
|
# under the License.
|
||||||
|
|
||||||
|
import ddt
|
||||||
|
from tempest.lib import exceptions as tempest_exc
|
||||||
|
|
||||||
from manilaclient.tests.functional.osc import base
|
from manilaclient.tests.functional.osc import base
|
||||||
|
|
||||||
import ddt
|
|
||||||
|
|
||||||
|
|
||||||
|
@ddt.ddt
|
||||||
class ShareAccessAllowTestCase(base.OSCClientTestBase):
|
class ShareAccessAllowTestCase(base.OSCClientTestBase):
|
||||||
|
|
||||||
def test_share_access_allow(self):
|
def test_share_access_allow(self):
|
||||||
@ -52,23 +54,63 @@ class ShareAccessAllowTestCase(base.OSCClientTestBase):
|
|||||||
self.assertEqual(access_rule['properties'], 'foo : bar')
|
self.assertEqual(access_rule['properties'], 'foo : bar')
|
||||||
self.assertEqual(access_rule['access_level'], 'ro')
|
self.assertEqual(access_rule['access_level'], 'ro')
|
||||||
|
|
||||||
|
@ddt.data(
|
||||||
|
{'lock_visibility': True, 'lock_deletion': True,
|
||||||
|
'lock_reason': None},
|
||||||
|
{'lock_visibility': False, 'lock_deletion': True,
|
||||||
|
'lock_reason': None},
|
||||||
|
{'lock_visibility': True, 'lock_deletion': False,
|
||||||
|
'lock_reason': 'testing'},
|
||||||
|
{'lock_visibility': True, 'lock_deletion': False,
|
||||||
|
'lock_reason': 'testing'},
|
||||||
|
)
|
||||||
|
@ddt.unpack
|
||||||
|
def test_share_access_allow_restrict(self, lock_visibility,
|
||||||
|
lock_deletion, lock_reason):
|
||||||
|
share = self.create_share()
|
||||||
|
access_rule = self.create_share_access_rule(
|
||||||
|
share=share['id'],
|
||||||
|
access_type='ip',
|
||||||
|
access_to='0.0.0.0/0',
|
||||||
|
wait=True,
|
||||||
|
lock_visibility=lock_visibility,
|
||||||
|
lock_deletion=lock_deletion,
|
||||||
|
lock_reason=lock_reason)
|
||||||
|
|
||||||
|
if lock_deletion:
|
||||||
|
self.assertRaises(
|
||||||
|
tempest_exc.CommandFailed,
|
||||||
|
self.openstack,
|
||||||
|
'share',
|
||||||
|
params=f'access delete {share["id"]} {access_rule["id"]}'
|
||||||
|
)
|
||||||
|
self.openstack(
|
||||||
|
'share',
|
||||||
|
params=f'access delete {share["id"]} {access_rule["id"]} '
|
||||||
|
f'--unrestrict --wait')
|
||||||
|
|
||||||
|
|
||||||
|
@ddt.ddt
|
||||||
class ShareAccessDenyTestCase(base.OSCClientTestBase):
|
class ShareAccessDenyTestCase(base.OSCClientTestBase):
|
||||||
def test_share_access_deny(self):
|
@ddt.data(True, False)
|
||||||
|
def test_share_access_deny(self, lock_deletion):
|
||||||
share = self.create_share()
|
share = self.create_share()
|
||||||
access_rule = self.create_share_access_rule(
|
access_rule = self.create_share_access_rule(
|
||||||
share=share['name'],
|
share=share['name'],
|
||||||
access_type='ip',
|
access_type='ip',
|
||||||
access_to='0.0.0.0/0',
|
access_to='0.0.0.0/0',
|
||||||
wait=True)
|
wait=True,
|
||||||
|
lock_deletion=lock_deletion)
|
||||||
|
|
||||||
access_rules = self.listing_result('share',
|
access_rules = self.listing_result('share',
|
||||||
f'access list {share["id"]}')
|
f'access list {share["id"]}')
|
||||||
num_access_rules = len(access_rules)
|
num_access_rules = len(access_rules)
|
||||||
|
|
||||||
self.openstack('share',
|
delete_params = (
|
||||||
params=f'access delete '
|
f'access delete {share["name"]} {access_rule["id"]} --wait')
|
||||||
f'{share["name"]} {access_rule["id"]} --wait')
|
if lock_deletion:
|
||||||
|
delete_params += ' --unrestrict'
|
||||||
|
self.openstack('share', params=delete_params)
|
||||||
|
|
||||||
access_rules = self.listing_result('share',
|
access_rules = self.listing_result('share',
|
||||||
f'access list {share["id"]}')
|
f'access list {share["id"]}')
|
||||||
@ -127,6 +169,28 @@ class ListShareAccessRulesTestCase(base.OSCClientTestBase):
|
|||||||
self.assertEqual(access_rule_properties['id'],
|
self.assertEqual(access_rule_properties['id'],
|
||||||
access_rule_properties[0]['ID'])
|
access_rule_properties[0]['ID'])
|
||||||
|
|
||||||
|
def test_share_access_list_with_filters(self):
|
||||||
|
share = self.create_share()
|
||||||
|
access_to_filter = '20.0.0.0/0'
|
||||||
|
self.create_share_access_rule(
|
||||||
|
share=share['name'],
|
||||||
|
access_type='ip',
|
||||||
|
access_to='0.0.0.0/0',
|
||||||
|
wait=True)
|
||||||
|
self.create_share_access_rule(
|
||||||
|
share=share['name'],
|
||||||
|
access_type='ip',
|
||||||
|
access_to=access_to_filter,
|
||||||
|
wait=True)
|
||||||
|
|
||||||
|
output = self.openstack(
|
||||||
|
'share',
|
||||||
|
params=f'access list {share["id"]} --access-to {access_to_filter}',
|
||||||
|
flags=f'--os-share-api-version 2.82')
|
||||||
|
access_rule_list = self.parser.listing(output)
|
||||||
|
|
||||||
|
self.assertTrue(len(access_rule_list) == 1)
|
||||||
|
|
||||||
|
|
||||||
class ShowShareAccessRulesTestCase(base.OSCClientTestBase):
|
class ShowShareAccessRulesTestCase(base.OSCClientTestBase):
|
||||||
def test_share_access_show(self):
|
def test_share_access_show(self):
|
||||||
|
@ -10,8 +10,10 @@
|
|||||||
# License for the specific language governing permissions and limitations
|
# License for the specific language governing permissions and limitations
|
||||||
# under the License.
|
# under the License.
|
||||||
#
|
#
|
||||||
|
|
||||||
from unittest import mock
|
from unittest import mock
|
||||||
|
|
||||||
|
import ddt
|
||||||
from osc_lib import exceptions
|
from osc_lib import exceptions
|
||||||
from osc_lib import utils as oscutils
|
from osc_lib import utils as oscutils
|
||||||
|
|
||||||
@ -48,6 +50,7 @@ class TestShareAccess(manila_fakes.TestShare):
|
|||||||
self.access_rules_mock.reset_mock()
|
self.access_rules_mock.reset_mock()
|
||||||
|
|
||||||
|
|
||||||
|
@ddt.ddt
|
||||||
class TestShareAccessCreate(TestShareAccess):
|
class TestShareAccessCreate(TestShareAccess):
|
||||||
|
|
||||||
def setUp(self):
|
def setUp(self):
|
||||||
@ -84,7 +87,7 @@ class TestShareAccessCreate(TestShareAccess):
|
|||||||
access_type="user",
|
access_type="user",
|
||||||
access="demo",
|
access="demo",
|
||||||
access_level=None,
|
access_level=None,
|
||||||
metadata={}
|
metadata={},
|
||||||
)
|
)
|
||||||
self.assertEqual(ACCESS_RULE_ATTRIBUTES, columns)
|
self.assertEqual(ACCESS_RULE_ATTRIBUTES, columns)
|
||||||
self.assertCountEqual(self.access_rule._info.values(), data)
|
self.assertCountEqual(self.access_rule._info.values(), data)
|
||||||
@ -100,7 +103,7 @@ class TestShareAccessCreate(TestShareAccess):
|
|||||||
("share", self.share.id),
|
("share", self.share.id),
|
||||||
("access_type", "user"),
|
("access_type", "user"),
|
||||||
("access_to", "demo"),
|
("access_to", "demo"),
|
||||||
('properties', ['key=value'])
|
('properties', ['key=value']),
|
||||||
]
|
]
|
||||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||||
columns, data = self.cmd.take_action(parsed_args)
|
columns, data = self.cmd.take_action(parsed_args)
|
||||||
@ -109,11 +112,92 @@ class TestShareAccessCreate(TestShareAccess):
|
|||||||
access_type="user",
|
access_type="user",
|
||||||
access="demo",
|
access="demo",
|
||||||
access_level=None,
|
access_level=None,
|
||||||
metadata={'key': 'value'}
|
metadata={'key': 'value'},
|
||||||
)
|
)
|
||||||
self.assertEqual(ACCESS_RULE_ATTRIBUTES, columns)
|
self.assertEqual(ACCESS_RULE_ATTRIBUTES, columns)
|
||||||
self.assertCountEqual(self.access_rule._info.values(), data)
|
self.assertCountEqual(self.access_rule._info.values(), data)
|
||||||
|
|
||||||
|
@ddt.data(
|
||||||
|
{'lock_visibility': True, 'lock_deletion': True,
|
||||||
|
'lock_reason': 'testing resource locks'},
|
||||||
|
{'lock_visibility': False, 'lock_deletion': True, 'lock_reason': None},
|
||||||
|
{'lock_visibility': True, 'lock_deletion': False, 'lock_reason': None},
|
||||||
|
)
|
||||||
|
@ddt.unpack
|
||||||
|
def test_share_access_create_restrict(self, lock_visibility,
|
||||||
|
lock_deletion, lock_reason):
|
||||||
|
arglist = [
|
||||||
|
self.share.id,
|
||||||
|
'user',
|
||||||
|
'demo',
|
||||||
|
'--properties', 'key=value'
|
||||||
|
]
|
||||||
|
verifylist = [
|
||||||
|
("share", self.share.id),
|
||||||
|
("access_type", "user"),
|
||||||
|
("access_to", "demo"),
|
||||||
|
('properties', ['key=value']),
|
||||||
|
]
|
||||||
|
allow_call_kwargs = {}
|
||||||
|
if lock_visibility:
|
||||||
|
arglist.append('--lock-visibility')
|
||||||
|
verifylist.append(('lock_visibility', lock_visibility))
|
||||||
|
allow_call_kwargs['lock_visibility'] = lock_visibility
|
||||||
|
if lock_deletion:
|
||||||
|
arglist.append('--lock-deletion')
|
||||||
|
verifylist.append(('lock_deletion', lock_deletion))
|
||||||
|
allow_call_kwargs['lock_deletion'] = lock_deletion
|
||||||
|
if lock_reason:
|
||||||
|
arglist.append('--lock-reason')
|
||||||
|
arglist.append(lock_reason)
|
||||||
|
verifylist.append(('lock_reason', lock_reason))
|
||||||
|
allow_call_kwargs['lock_reason'] = lock_reason
|
||||||
|
|
||||||
|
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||||
|
columns, data = self.cmd.take_action(parsed_args)
|
||||||
|
self.shares_mock.get.assert_called_with(self.share.id)
|
||||||
|
self.share.allow.assert_called_with(
|
||||||
|
access_type="user",
|
||||||
|
access="demo",
|
||||||
|
access_level=None,
|
||||||
|
metadata={'key': 'value'},
|
||||||
|
**allow_call_kwargs
|
||||||
|
)
|
||||||
|
self.assertEqual(ACCESS_RULE_ATTRIBUTES, columns)
|
||||||
|
self.assertCountEqual(self.access_rule._info.values(), data)
|
||||||
|
|
||||||
|
@ddt.data(
|
||||||
|
{'lock_visibility': True, 'lock_deletion': False},
|
||||||
|
{'lock_visibility': False, 'lock_deletion': True},
|
||||||
|
)
|
||||||
|
@ddt.unpack
|
||||||
|
def test_share_access_create_restrict_not_available(
|
||||||
|
self, lock_visibility, lock_deletion):
|
||||||
|
arglist = [
|
||||||
|
self.share.id,
|
||||||
|
'user',
|
||||||
|
'demo',
|
||||||
|
]
|
||||||
|
self.app.client_manager.share.api_version = api_versions.APIVersion(
|
||||||
|
"2.79")
|
||||||
|
verifylist = [
|
||||||
|
("share", self.share.id),
|
||||||
|
("access_type", "user"),
|
||||||
|
("access_to", "demo"),
|
||||||
|
("lock_visibility", lock_visibility),
|
||||||
|
("lock_deletion", lock_deletion),
|
||||||
|
("lock_reason", None),
|
||||||
|
]
|
||||||
|
if lock_visibility:
|
||||||
|
arglist.append('--lock-visibility')
|
||||||
|
if lock_deletion:
|
||||||
|
arglist.append('--lock-deletion')
|
||||||
|
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||||
|
self.assertRaises(
|
||||||
|
exceptions.CommandError,
|
||||||
|
self.cmd.take_action,
|
||||||
|
parsed_args)
|
||||||
|
|
||||||
def test_access_rule_create_access_level(self):
|
def test_access_rule_create_access_level(self):
|
||||||
arglist = [
|
arglist = [
|
||||||
self.share.id,
|
self.share.id,
|
||||||
@ -125,7 +209,7 @@ class TestShareAccessCreate(TestShareAccess):
|
|||||||
("share", self.share.id),
|
("share", self.share.id),
|
||||||
("access_type", "user"),
|
("access_type", "user"),
|
||||||
("access_to", "demo"),
|
("access_to", "demo"),
|
||||||
('access_level', 'ro')
|
('access_level', 'ro'),
|
||||||
]
|
]
|
||||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||||
columns, data = self.cmd.take_action(parsed_args)
|
columns, data = self.cmd.take_action(parsed_args)
|
||||||
@ -134,7 +218,7 @@ class TestShareAccessCreate(TestShareAccess):
|
|||||||
access_type="user",
|
access_type="user",
|
||||||
access="demo",
|
access="demo",
|
||||||
access_level='ro',
|
access_level='ro',
|
||||||
metadata={}
|
metadata={},
|
||||||
)
|
)
|
||||||
self.assertEqual(ACCESS_RULE_ATTRIBUTES, columns)
|
self.assertEqual(ACCESS_RULE_ATTRIBUTES, columns)
|
||||||
self.assertCountEqual(self.access_rule._info.values(), data)
|
self.assertCountEqual(self.access_rule._info.values(), data)
|
||||||
@ -150,7 +234,7 @@ class TestShareAccessCreate(TestShareAccess):
|
|||||||
("share", self.share.id),
|
("share", self.share.id),
|
||||||
("access_type", "user"),
|
("access_type", "user"),
|
||||||
("access_to", "demo"),
|
("access_to", "demo"),
|
||||||
("wait", True)
|
("wait", True),
|
||||||
]
|
]
|
||||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||||
columns, data = self.cmd.take_action(parsed_args)
|
columns, data = self.cmd.take_action(parsed_args)
|
||||||
@ -159,7 +243,7 @@ class TestShareAccessCreate(TestShareAccess):
|
|||||||
access_type="user",
|
access_type="user",
|
||||||
access="demo",
|
access="demo",
|
||||||
access_level=None,
|
access_level=None,
|
||||||
metadata={}
|
metadata={},
|
||||||
)
|
)
|
||||||
self.assertEqual(ACCESS_RULE_ATTRIBUTES, columns)
|
self.assertEqual(ACCESS_RULE_ATTRIBUTES, columns)
|
||||||
self.assertCountEqual(self.access_rule._info.values(), data)
|
self.assertCountEqual(self.access_rule._info.values(), data)
|
||||||
@ -176,7 +260,7 @@ class TestShareAccessCreate(TestShareAccess):
|
|||||||
("share", self.share.id),
|
("share", self.share.id),
|
||||||
("access_type", "user"),
|
("access_type", "user"),
|
||||||
("access_to", "demo"),
|
("access_to", "demo"),
|
||||||
("wait", True)
|
("wait", True),
|
||||||
]
|
]
|
||||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||||
|
|
||||||
@ -188,7 +272,7 @@ class TestShareAccessCreate(TestShareAccess):
|
|||||||
access_type="user",
|
access_type="user",
|
||||||
access="demo",
|
access="demo",
|
||||||
access_level=None,
|
access_level=None,
|
||||||
metadata={}
|
metadata={},
|
||||||
)
|
)
|
||||||
|
|
||||||
mock_logger.error.assert_called_with(
|
mock_logger.error.assert_called_with(
|
||||||
@ -198,6 +282,7 @@ class TestShareAccessCreate(TestShareAccess):
|
|||||||
self.assertCountEqual(self.access_rule._info.values(), data)
|
self.assertCountEqual(self.access_rule._info.values(), data)
|
||||||
|
|
||||||
|
|
||||||
|
@ddt.ddt
|
||||||
class TestShareAccessDelete(TestShareAccess):
|
class TestShareAccessDelete(TestShareAccess):
|
||||||
|
|
||||||
def setUp(self):
|
def setUp(self):
|
||||||
@ -213,21 +298,47 @@ class TestShareAccessDelete(TestShareAccess):
|
|||||||
# Get the command object to test
|
# Get the command object to test
|
||||||
self.cmd = osc_share_access_rules.ShareAccessDeny(self.app, None)
|
self.cmd = osc_share_access_rules.ShareAccessDeny(self.app, None)
|
||||||
|
|
||||||
def test_share_access_delete(self):
|
@ddt.data(True, False)
|
||||||
|
def test_share_access_delete(self, unrestrict):
|
||||||
arglist = [
|
arglist = [
|
||||||
self.share.id,
|
self.share.id,
|
||||||
self.access_rule.id
|
self.access_rule.id
|
||||||
]
|
]
|
||||||
verifylist = [
|
verifylist = [
|
||||||
("share", self.share.id),
|
("share", self.share.id),
|
||||||
("id", self.access_rule.id)
|
("id", self.access_rule.id),
|
||||||
]
|
]
|
||||||
|
deny_kwargs = {}
|
||||||
|
if unrestrict:
|
||||||
|
arglist.append('--unrestrict')
|
||||||
|
verifylist.append(("unrestrict", unrestrict))
|
||||||
|
deny_kwargs['unrestrict'] = unrestrict
|
||||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||||
result = self.cmd.take_action(parsed_args)
|
result = self.cmd.take_action(parsed_args)
|
||||||
self.shares_mock.get.assert_called_with(self.share.id)
|
self.shares_mock.get.assert_called_with(self.share.id)
|
||||||
self.share.deny.assert_called_with(self.access_rule.id)
|
self.share.deny.assert_called_with(
|
||||||
|
self.access_rule.id, **deny_kwargs)
|
||||||
self.assertIsNone(result)
|
self.assertIsNone(result)
|
||||||
|
|
||||||
|
def test_share_access_delete_unrestrict_not_available(self):
|
||||||
|
self.app.client_manager.share.api_version = api_versions.APIVersion(
|
||||||
|
"2.79")
|
||||||
|
arglist = [
|
||||||
|
self.share.id,
|
||||||
|
self.access_rule.id,
|
||||||
|
"--unrestrict"
|
||||||
|
]
|
||||||
|
verifylist = [
|
||||||
|
("share", self.share.id),
|
||||||
|
("id", self.access_rule.id),
|
||||||
|
("unrestrict", True)
|
||||||
|
]
|
||||||
|
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||||
|
self.assertRaises(
|
||||||
|
exceptions.CommandError,
|
||||||
|
self.cmd.take_action,
|
||||||
|
parsed_args)
|
||||||
|
|
||||||
def test_share_access_delete_wait(self):
|
def test_share_access_delete_wait(self):
|
||||||
arglist = [
|
arglist = [
|
||||||
self.share.id,
|
self.share.id,
|
||||||
@ -237,7 +348,7 @@ class TestShareAccessDelete(TestShareAccess):
|
|||||||
verifylist = [
|
verifylist = [
|
||||||
("share", self.share.id),
|
("share", self.share.id),
|
||||||
("id", self.access_rule.id),
|
("id", self.access_rule.id),
|
||||||
('wait', True)
|
('wait', True),
|
||||||
]
|
]
|
||||||
|
|
||||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||||
@ -246,7 +357,8 @@ class TestShareAccessDelete(TestShareAccess):
|
|||||||
result = self.cmd.take_action(parsed_args)
|
result = self.cmd.take_action(parsed_args)
|
||||||
|
|
||||||
self.shares_mock.get.assert_called_with(self.share.id)
|
self.shares_mock.get.assert_called_with(self.share.id)
|
||||||
self.share.deny.assert_called_with(self.access_rule.id)
|
self.share.deny.assert_called_with(
|
||||||
|
self.access_rule.id)
|
||||||
self.assertIsNone(result)
|
self.assertIsNone(result)
|
||||||
|
|
||||||
def test_share_access_delete_wait_error(self):
|
def test_share_access_delete_wait_error(self):
|
||||||
@ -270,6 +382,7 @@ class TestShareAccessDelete(TestShareAccess):
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@ddt.ddt
|
||||||
class TestShareAccessList(TestShareAccess):
|
class TestShareAccessList(TestShareAccess):
|
||||||
|
|
||||||
access_rules_columns = [
|
access_rules_columns = [
|
||||||
@ -339,6 +452,58 @@ class TestShareAccessList(TestShareAccess):
|
|||||||
self.assertEqual(self.access_rules_columns, columns)
|
self.assertEqual(self.access_rules_columns, columns)
|
||||||
self.assertEqual(tuple(self.values_list), tuple(data))
|
self.assertEqual(tuple(self.values_list), tuple(data))
|
||||||
|
|
||||||
|
@ddt.data(
|
||||||
|
{'access_to': '10.0.0.0/0', 'access_type': 'ip'},
|
||||||
|
{'access_key': '10.0.0.0/0', 'access_level': 'rw'},
|
||||||
|
)
|
||||||
|
def test_access_rules_list_access_filters(self, filters):
|
||||||
|
arglist = [
|
||||||
|
self.share.id,
|
||||||
|
]
|
||||||
|
|
||||||
|
verifylist = [
|
||||||
|
("share", self.share.id),
|
||||||
|
]
|
||||||
|
for filter_key, filter_value in filters.items():
|
||||||
|
filter_arg = filter_key.replace("_", "-")
|
||||||
|
arglist.append(f'--{filter_arg}')
|
||||||
|
arglist.append(filter_value)
|
||||||
|
verifylist.append((filter_key, filter_value))
|
||||||
|
|
||||||
|
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||||
|
columns, data = self.cmd.take_action(parsed_args)
|
||||||
|
|
||||||
|
self.shares_mock.get.assert_called_with(self.share.id)
|
||||||
|
self.access_rules_mock.access_list.assert_called_with(
|
||||||
|
self.share,
|
||||||
|
filters)
|
||||||
|
self.assertEqual(self.access_rules_columns, columns)
|
||||||
|
self.assertEqual(tuple(self.values_list), tuple(data))
|
||||||
|
|
||||||
|
@ddt.data(
|
||||||
|
{'access_to': '10.0.0.0/0', 'access_type': 'ip'},
|
||||||
|
{'access_key': '10.0.0.0/0', 'access_level': 'rw'},
|
||||||
|
)
|
||||||
|
def test_access_rules_list_access_filters_command_error(self, filters):
|
||||||
|
self.app.client_manager.share.api_version = api_versions.APIVersion(
|
||||||
|
"2.81")
|
||||||
|
arglist = [
|
||||||
|
self.share.id,
|
||||||
|
]
|
||||||
|
verifylist = [
|
||||||
|
("share", self.share.id),
|
||||||
|
]
|
||||||
|
for filter_key, filter_value in filters.items():
|
||||||
|
filter_arg = filter_key.replace("_", "-")
|
||||||
|
arglist.append(f'--{filter_arg}')
|
||||||
|
arglist.append(filter_value)
|
||||||
|
verifylist.append((filter_key, filter_value))
|
||||||
|
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||||
|
self.assertRaises(
|
||||||
|
exceptions.CommandError,
|
||||||
|
self.cmd.take_action,
|
||||||
|
parsed_args)
|
||||||
|
|
||||||
|
|
||||||
class TestShareAccessShow(TestShareAccess):
|
class TestShareAccessShow(TestShareAccess):
|
||||||
|
|
||||||
|
@ -50,7 +50,7 @@ class SharesTest(utils.TestCase):
|
|||||||
self.share.allow(access_type, access_to, access_level)
|
self.share.allow(access_type, access_to, access_level)
|
||||||
|
|
||||||
self.share.manager.allow.assert_called_once_with(
|
self.share.manager.allow.assert_called_once_with(
|
||||||
self.share, access_type, access_to, access_level, None)
|
self.share, access_type, access_to, access_level)
|
||||||
|
|
||||||
# Testcases for class ShareManager
|
# Testcases for class ShareManager
|
||||||
|
|
||||||
|
@ -75,14 +75,13 @@ class Share(base.MetadataCapableResource):
|
|||||||
"""Delete the specified share ignoring its current state."""
|
"""Delete the specified share ignoring its current state."""
|
||||||
self.manager.force_delete(self)
|
self.manager.force_delete(self)
|
||||||
|
|
||||||
def allow(self, access_type, access, access_level, metadata=None):
|
def allow(self, *args, **kwargs):
|
||||||
"""Allow access to a share."""
|
"""Allow access to a share."""
|
||||||
return self.manager.allow(
|
return self.manager.allow(self, *args, **kwargs)
|
||||||
self, access_type, access, access_level, metadata)
|
|
||||||
|
|
||||||
def deny(self, id):
|
def deny(self, id, **kwargs):
|
||||||
"""Deny access from IP to a share."""
|
"""Deny access from IP to a share."""
|
||||||
return self.manager.deny(self, id)
|
return self.manager.deny(self, id, **kwargs)
|
||||||
|
|
||||||
def access_list(self):
|
def access_list(self):
|
||||||
"""Get access list from a share."""
|
"""Get access list from a share."""
|
||||||
@ -570,7 +569,8 @@ class ShareManager(base.MetadataCapableManager):
|
|||||||
raise exceptions.CommandError(msg)
|
raise exceptions.CommandError(msg)
|
||||||
|
|
||||||
def _do_allow(self, share, access_type, access, access_level, action_name,
|
def _do_allow(self, share, access_type, access, access_level, action_name,
|
||||||
metadata=None):
|
metadata=None, lock_visibility=False,
|
||||||
|
lock_deletion=False, lock_reason=None):
|
||||||
"""Allow access to a share.
|
"""Allow access to a share.
|
||||||
|
|
||||||
:param share: either share object or text with its ID.
|
:param share: either share object or text with its ID.
|
||||||
@ -587,6 +587,12 @@ class ShareManager(base.MetadataCapableManager):
|
|||||||
access_params['access_level'] = access_level
|
access_params['access_level'] = access_level
|
||||||
if metadata:
|
if metadata:
|
||||||
access_params['metadata'] = metadata
|
access_params['metadata'] = metadata
|
||||||
|
if lock_visibility:
|
||||||
|
access_params['lock_visibility'] = lock_visibility
|
||||||
|
if lock_deletion:
|
||||||
|
access_params['lock_deletion'] = lock_deletion
|
||||||
|
if lock_reason:
|
||||||
|
access_params['lock_reason'] = lock_reason
|
||||||
access = self._action(action_name, share,
|
access = self._action(action_name, share,
|
||||||
access_params)[1]["access"]
|
access_params)[1]["access"]
|
||||||
return access
|
return access
|
||||||
@ -618,30 +624,53 @@ class ShareManager(base.MetadataCapableManager):
|
|||||||
return self._do_allow(
|
return self._do_allow(
|
||||||
share, access_type, access, access_level, "allow_access")
|
share, access_type, access, access_level, "allow_access")
|
||||||
|
|
||||||
@api_versions.wraps("2.45") # noqa
|
@api_versions.wraps("2.45", "2.81") # noqa
|
||||||
def allow(self, share, access_type, access, access_level, metadata=None): # noqa
|
def allow(self, share, access_type, access, access_level, metadata=None): # noqa
|
||||||
valid_access_types = ('ip', 'user', 'cert', 'cephx')
|
valid_access_types = ('ip', 'user', 'cert', 'cephx')
|
||||||
self._validate_access(access_type, access, valid_access_types,
|
self._validate_access(access_type, access, valid_access_types,
|
||||||
enable_ipv6=True)
|
enable_ipv6=True)
|
||||||
return self._do_allow(
|
return self._do_allow(
|
||||||
share, access_type, access, access_level, "allow_access", metadata)
|
share, access_type, access, access_level, "allow_access",
|
||||||
|
metadata=metadata)
|
||||||
|
|
||||||
def _do_deny(self, share, access_id, action_name):
|
@api_versions.wraps("2.82") # noqa
|
||||||
|
def allow(self, share, access_type, access, access_level, # pylint: disable=function-redefined # noqa F811
|
||||||
|
metadata=None, lock_visibility=False, lock_deletion=False,
|
||||||
|
lock_reason=None):
|
||||||
|
valid_access_types = ('ip', 'user', 'cert', 'cephx')
|
||||||
|
self._validate_access(access_type, access, valid_access_types,
|
||||||
|
enable_ipv6=True)
|
||||||
|
return self._do_allow(
|
||||||
|
share, access_type, access, access_level, "allow_access",
|
||||||
|
metadata=metadata, lock_visibility=lock_visibility,
|
||||||
|
lock_deletion=lock_deletion, lock_reason=lock_reason)
|
||||||
|
|
||||||
|
def _do_deny(self, share, access_id, action_name, unrestrict=False):
|
||||||
"""Deny access to a share.
|
"""Deny access to a share.
|
||||||
|
|
||||||
:param share: either share object or text with its ID.
|
:param share: either share object or text with its ID.
|
||||||
:param access_id: ID of share access rule
|
:param access_id: ID of share access rule
|
||||||
"""
|
"""
|
||||||
return self._action(action_name, share, {"access_id": access_id})
|
body = {
|
||||||
|
"access_id": access_id,
|
||||||
|
}
|
||||||
|
if unrestrict:
|
||||||
|
body['unrestrict'] = True
|
||||||
|
return self._action(action_name, share, body)
|
||||||
|
|
||||||
@api_versions.wraps("1.0", "2.6")
|
@api_versions.wraps("1.0", "2.6")
|
||||||
def deny(self, share, access_id):
|
def deny(self, share, access_id):
|
||||||
return self._do_deny(share, access_id, "os-deny_access")
|
return self._do_deny(share, access_id, "os-deny_access")
|
||||||
|
|
||||||
@api_versions.wraps("2.7") # noqa
|
@api_versions.wraps("2.7", "2.81") # noqa
|
||||||
def deny(self, share, access_id): # noqa
|
def deny(self, share, access_id): # noqa
|
||||||
return self._do_deny(share, access_id, "deny_access")
|
return self._do_deny(share, access_id, "deny_access")
|
||||||
|
|
||||||
|
@api_versions.wraps("2.82") # noqa
|
||||||
|
def deny(self, share, access_id, unrestrict=False): # noqa
|
||||||
|
return self._do_deny(share, access_id, "deny_access",
|
||||||
|
unrestrict=unrestrict)
|
||||||
|
|
||||||
def _do_access_list(self, share, action_name):
|
def _do_access_list(self, share, action_name):
|
||||||
"""Get access list to a share.
|
"""Get access list to a share.
|
||||||
|
|
||||||
|
@ -0,0 +1,9 @@
|
|||||||
|
---
|
||||||
|
features:
|
||||||
|
- |
|
||||||
|
It is now possible to restrict the visibility of access rules' sensitive
|
||||||
|
fields, as well as lock the access rule deletion while allowing access to
|
||||||
|
a share. A lock reason can also be provided.
|
||||||
|
- |
|
||||||
|
It is now possible to filter access rules while listing them by its
|
||||||
|
`access_to`, `access_type`, `access_key` and `access_level`.
|
Loading…
Reference in New Issue
Block a user