openstack
/
manila
Code Issues Proposed changes

Add access levels for shares 

Add new migration which adds 'access_level' column to ShareAccessMapping table.
Change model description for ShareAccessMapping - include new column.

Add an ability to pass 'access_level' param to 'allow_access' method in share API,
pass 'access_level' to driver interface.

Add 'access_level' to return value of 'access_get_all' method.

Add unit and tempest tests.

Implements bp level-of-access-for-shares

Change-Id: I6c295b66261489544fc343948b960e39ec870b5c
This commit is contained in:
Julia Varlamova 2015-01-27 12:34:06 +03:00
parent b5edd4f9c9
commit c4b73508ec
9 changed files with 155 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
contrib/tempest/tempest/api/share/test_rules.py
View File

@ -45,6 +45,7 @@ class ShareIpRulesForNFSTest(base.BaseSharesTest):
access_type,
access_to)
self.assertIn(int(resp["status"]), test.HTTP_SUCCESS)
self.assertEqual('rw', rule['access_level'])
self.shares_client.wait_for_access_rule_status(self.share["id"],
rule["id"],
"active")
@ -65,6 +66,7 @@ class ShareIpRulesForNFSTest(base.BaseSharesTest):
access_type,
access_to)
self.assertIn(int(resp["status"]), test.HTTP_SUCCESS)
self.assertEqual('rw', rule['access_level'])
self.shares_client.wait_for_access_rule_status(self.share["id"],
rule["id"],
"active")
@ -73,6 +75,21 @@ class ShareIpRulesForNFSTest(base.BaseSharesTest):
rule["id"])
self.assertIn(int(resp["status"]), test.HTTP_SUCCESS)
@test.attr(type=["gate", ])
def test_create_delete_ro_access_rule(self):
resp, rule = self.shares_client.create_access_rule(self.share["id"],
'ip',
'2.2.2.2',
'ro')
self.assertIn(int(resp["status"]), test.HTTP_SUCCESS)
self.assertEqual('ro', rule['access_level'])
self.shares_client.wait_for_access_rule_status(self.share["id"],
rule["id"],
"active")
resp, _ = self.shares_client.delete_access_rule(self.share["id"],
rule["id"])
self.assertIn(int(resp["status"]), test.HTTP_SUCCESS)
class ShareIpRulesForCIFSTest(ShareIpRulesForNFSTest):
protocol = "cifs"
@ -90,19 +107,17 @@ class ShareUserRulesForNFSTest(base.BaseSharesTest):
msg = "USER rule tests for %s protocol are disabled" % cls.protocol
raise cls.skipException(msg)
__, cls.share = cls.create_share(cls.protocol)
cls.access_type = "user"
cls.access_to = CONF.share.username_for_user_rules
@test.attr(type=["gate", ])
def test_create_delete_user_rule(self):
# test data
access_type = "user"
access_to = CONF.share.username_for_user_rules
# create rule
resp, rule = self.shares_client.create_access_rule(self.share["id"],
access_type,
access_to)
self.access_type,
self.access_to)
self.assertIn(int(resp["status"]), test.HTTP_SUCCESS)
self.assertEqual('rw', rule['access_level'])
self.shares_client.wait_for_access_rule_status(self.share["id"],
rule["id"],
"active")
@ -111,6 +126,21 @@ class ShareUserRulesForNFSTest(base.BaseSharesTest):
rule["id"])
self.assertIn(int(resp["status"]), test.HTTP_SUCCESS)
@test.attr(type=["gate", ])
def test_create_delete_ro_access_rule(self):
resp, rule = self.shares_client.create_access_rule(self.share["id"],
self.access_type,
self.access_to,
'ro')
self.assertIn(int(resp["status"]), test.HTTP_SUCCESS)
self.assertEqual('ro', rule['access_level'])
self.shares_client.wait_for_access_rule_status(self.share["id"],
rule["id"],
"active")
resp, _ = self.shares_client.delete_access_rule(self.share["id"],
rule["id"])
self.assertIn(int(resp["status"]), test.HTTP_SUCCESS)
class ShareUserRulesForCIFSTest(ShareUserRulesForNFSTest):
protocol = "cifs"
@ -167,13 +197,14 @@ class ShareRulesTest(base.BaseSharesTest):
self.assertEqual(200, int(resp["status"]), msg)
# verify keys
keys = ["state", "id", "access_type", "access_to"]
keys = ["state", "id", "access_type", "access_to", "access_level"]
[self.assertIn(key, r.keys()) for r in rules for key in keys]
# verify values
self.assertEqual("active", rules[0]["state"])
self.assertEqual(self.access_type, rules[0]["access_type"])
self.assertEqual(self.access_to, rules[0]["access_to"])
self.assertEqual('rw', rules[0]["access_level"])
# our share id in list and have no duplicates
gen = [r["id"] for r in rules if r["id"] in rule["id"]]

18
contrib/tempest/tempest/api/share/test_rules_negative.py
View File

@ -84,6 +84,15 @@ class ShareIpRulesForNFSNegativeTest(base.BaseSharesTest):
self.shares_client.create_access_rule,
self.share["id"], "ip", "1.2.3.1/")
@test.attr(type=["negative", "gate", ])
def test_create_access_rule_with_wrong_level(self):
self.assertRaises(exceptions.BadRequest,
self.shares_client.create_access_rule,
self.share["id"],
'ip',
'2.2.2.2',
'su')
@test.attr(type=["negative", "gate", ])
def test_create_duplicate_of_ip_rule(self):
# test data
@ -172,6 +181,15 @@ class ShareUserRulesForNFSNegativeTest(base.BaseSharesTest):
access_type="user",
access_to="fakeuser")
@test.attr(type=["negative", "gate", ])
def test_create_access_rule_with_wrong_level(self):
self.assertRaises(exceptions.BadRequest,
self.shares_client.create_access_rule,
self.share["id"],
'user',
CONF.share.username_for_user_rules,
'su')
class ShareUserRulesForCIFSNegativeTest(ShareUserRulesForNFSNegativeTest):
protocol = "cifs"

5
contrib/tempest/tempest/services/share/json/shares_client.py
View File

@ -97,12 +97,13 @@ class SharesClient(rest_client.RestClient):
resp, body = self.get("shares/%s" % share_id)
return resp, self._parse_resp(body)
def create_access_rule(self, share_id,
access_type="ip", access_to="0.0.0.0"):
def create_access_rule(self, share_id, access_type="ip",
access_to="0.0.0.0", access_level=None):
post_body = {
"os-allow_access": {
"access_type": access_type,
"access_to": access_to,
"access_level": access_level,
}
}
body = json.dumps(post_body)

9
manila/api/contrib/share_actions.py
View File

@ -121,11 +121,11 @@ class ShareActionsController(wsgi.Controller):
def _allow_access(self, req, id, body):
"""Add share access rule."""
context = req.environ['manila.context']
access_data = body['os-allow_access']
share = self.share_api.get(context, id)
access_type = body['os-allow_access']['access_type']
access_to = body['os-allow_access']['access_to']
access_type = access_data['access_type']
access_to = access_data['access_to']
if access_type == 'ip':
self._validate_ip_range(access_to)
elif access_type == 'user':
@ -138,7 +138,8 @@ class ShareActionsController(wsgi.Controller):
raise webob.exc.HTTPBadRequest(explanation=exc_str)
try:
access = self.share_api.allow_access(
context, share, access_type, access_to)
context, share, access_type, access_to,
access_data.get('access_level'))
except exception.ShareAccessExists as e:
raise webob.exc.HTTPBadRequest(explanation=e.msg)
return {'access': access}

8
manila/common/constants.py
View File

@ -51,3 +51,11 @@ PING_PORTS = (
SERVICE_INSTANCE_SECGROUP_DATA = (
CIFS_PORTS + NFS_PORTS + SSH_PORTS + PING_PORTS)
ACCESS_LEVEL_RW = 'rw'
ACCESS_LEVEL_RO = 'ro'
ACCESS_LEVELS = (
ACCESS_LEVEL_RW,
ACCESS_LEVEL_RO,
)

41
manila/db/migrations/alembic/versions/211836bf835c_add_access_level.py Normal file
View File

@ -0,0 +1,41 @@
# Copyright 2015 Mirantis Inc.
# All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
"""add access level
Revision ID: 211836bf835c
Revises: 162a3e673105
Create Date: 2014-12-19 05:34:06.790159
"""
# revision identifiers, used by Alembic.
revision = '211836bf835c'
down_revision = '162a3e673105'
from alembic import op
import sqlalchemy as sa
from manila.common import constants
def upgrade():
op.add_column('share_access_map',
sa.Column('access_level', sa.String(2),
default=constants.ACCESS_LEVEL_RW))
def downgrade():
op.drop_column('share_access_map', 'access_level')

2
manila/db/sqlalchemy/models.py
View File

@ -264,6 +264,8 @@ class ShareAccessMapping(BASE, ManilaBase):
state = Column(Enum(STATE_NEW, STATE_ACTIVE,
STATE_DELETING, STATE_DELETED, STATE_ERROR),
default=STATE_NEW)
access_level = Column(Enum(*constants.ACCESS_LEVELS),
default=constants.ACCESS_LEVEL_RW)
class ShareSnapshot(BASE, ManilaBase):

17
manila/share/api.py
View File

@ -24,6 +24,7 @@ from oslo.utils import timeutils
import six
from manila.api import extensions
from manila.common import constants
from manila.db import base
from manila import exception
from manila.i18n import _
@ -450,7 +451,8 @@ class API(base.Base):
snapshots = results
return snapshots
def allow_access(self, ctx, share, access_type, access_to):
def allow_access(self, ctx, share, access_type, access_to,
access_level=None):
"""Allow access to share."""
if not share['host']:
msg = _("Share host is None")
@ -459,14 +461,20 @@ class API(base.Base):
msg = _("Share status must be available")
raise exception.InvalidShare(reason=msg)
policy.check_policy(ctx, 'share', 'allow_access')
values = {'share_id': share['id'],
'access_type': access_type,
'access_to': access_to}
values = {
'share_id': share['id'],
'access_type': access_type,
'access_to': access_to,
'access_level': access_level,
}
access = [a for a in self.db.share_access_get_all_by_type_and_access(
ctx, share['id'], access_type, access_to) if a['state'] != 'error']
if access:
raise exception.ShareAccessExists(access_type=access_type,
access=access_to)
if access_level not in constants.ACCESS_LEVELS + (None, ):
msg = _("Invalid share access level: %s.") % access_level
raise exception.InvalidShareAccess(reason=msg)
access = self.db.share_access_create(ctx, values)
self.share_rpcapi.allow_access(ctx, share, access)
return access
@ -501,6 +509,7 @@ class API(base.Base):
return [{'id': rule.id,
'access_type': rule.access_type,
'access_to': rule.access_to,
'access_level': rule.access_level,
'state': rule.state} for rule in rules]
def access_get(self, context, access_id):

37
manila/tests/share/test_api.py
View File

@ -17,6 +17,7 @@
import datetime
import uuid
import ddt
import mock
from oslo.config import cfg
from oslo.utils import timeutils
@ -89,6 +90,7 @@ def fake_access(id, **kwargs):
'share_id': 'fakeshareid',
'access_type': 'fakeacctype',
'access_to': 'fakeaccto',
'access_level': 'rw',
'state': 'fakeactive',
'STATE_NEW': 'fakenew',
'STATE_ACTIVE': 'fakeactive',
@ -156,6 +158,7 @@ _FAKE_LIST_OF_ALL_SNAPSHOTS = [
]
@ddt.ddt
class ShareAPITestCase(test.TestCase):
def setUp(self):
@ -921,23 +924,33 @@ class ShareAPITestCase(test.TestCase):
share_api.policy.check_policy.assert_called_once_with(
ctx, 'share', 'get_all_snapshots')
def test_allow_access(self):
@ddt.data(None, 'rw', 'ro')
def test_allow_access(self, level):
share = fake_share('fakeid', status='available')
values = {
'share_id': share['id'],
'access_type': 'fakeacctype',
'access_to': 'fakeaccto',
'access_level': level,
}
with mock.patch.object(db_driver, 'share_access_create',
mock.Mock(return_value='fakeacc')):
self.share_rpcapi.allow_access(self.context, share, 'fakeacc')
access = self.api.allow_access(self.context, share, 'fakeacctype',
'fakeaccto')
self.assertEqual(access, 'fakeacc')
db_driver.share_access_create.assert_called_once_with(
self.context, values)
share_api.policy.check_policy.assert_called_once_with(
self.context, 'share', 'allow_access')
self.stubs.Set(db_driver, 'share_access_create',
mock.Mock(return_value='fakeacc'))
access = self.api.allow_access(self.context, share,
'fakeacctype', 'fakeaccto',
level)
self.assertEqual(access, 'fakeacc')
self.share_rpcapi.allow_access.assert_called_once_with(
self.context, share, 'fakeacc')
db_driver.share_access_create.assert_called_once_with(
self.context, values)
share_api.policy.check_policy.assert_called_once_with(
self.context, 'share', 'allow_access')
def test_allow_access_invalid_access_level(self):
share = fake_share('fakeid', status='available')
self.assertRaises(exception.InvalidShareAccess, self.api.allow_access,
self.context, share, 'fakeacctype', 'fakeaccto',
'ab')
def test_allow_access_status_not_available(self):
share = fake_share('fakeid', status='error')
@ -1014,12 +1027,14 @@ class ShareAPITestCase(test.TestCase):
'id': 'fakeacc0id',
'access_type': 'fakeacctype',
'access_to': 'fakeaccto',
'access_level': 'rw',
'state': 'fakenew',
},
{
'id': 'fakeacc1id',
'access_type': 'fakeacctype',
'access_to': 'fakeaccto',
'access_level': 'rw',
'state': 'fakeerror',
},
]