add access_key to share_access_map

For backends with internal authentication system,
e.g. Ceph, that return ``access_key`` (credential) for
client identities that are granted share access:

* Retrieve ``access_key`` as return value of driver's
  update_access()

* Store ``access_key`` in ShareAccessMapping model

* Expose it in access_list API

APIImpact

DocImpact

Partially implements bp auth-access-keys

Co-Authored-By: John Spray <jspray@redhat.com>

Change-Id: I486064f117cf3001dba7735ca92a7d89aee3ce5b

api-ref/source/parameters.yaml

@@ -373,6 +373,13 @@ access_id:
   in: body
   required: true
   type: string
+access_key:
+  description: |
+    The access credential of the entity granted share access.
+  in: body
+  required: true
+  type: string
+  min_version: 2.21
 access_level:
   description: |
     The access level to the share.  To grant or deny

api-ref/source/samples/share-actions-grant-access-response.json

@@ -6,6 +6,7 @@
         "access_type": "ip",
         "access_to": "0.0.0.0/0",
         "access_level": "rw",
+        "access_key": null,
         "id": "a25b2df3-90bd-4add-afa6-5f0dbbd50452"
     }
 }

api-ref/source/samples/share-actions-list-access-rules-response.json

@@ -5,14 +5,16 @@
             "state": "error",
             "id": "507bf114-36f2-4f56-8cf4-857985ca87c1",
             "access_type": "cert",
-            "access_to": "example.com"
+            "access_to": "example.com",
+            "access_key": null
         },
         {
             "access_level": "rw",
             "state": "active",
             "id": "a25b2df3-90bd-4add-afa6-5f0dbbd50452",
             "access_type": "ip",
-            "access_to": "0.0.0.0/0"
+            "access_to": "0.0.0.0/0",
+            "access_key": null
         }
     ]
 }

api-ref/source/share-actions.inc

@@ -97,6 +97,7 @@ Response parameters
    - updated_at: updated_at_3
    - access_type: access_type_2
    - access_to: access_to_1
+   - access_key: access_key
    - access: access
    - access_level: access_level_2
    - id: id_7
@@ -169,6 +170,7 @@ Response parameters
 .. rest_parameters:: parameters.yaml
 
    - access_type: access_type_1
+   - access_key: access_key
    - access_to: access_to_1
    - access_level: access_level
    - state: state

manila/api/openstack/api_version_request.py

@@ -72,14 +72,14 @@ REST_API_VERSION_HISTORY = """
     * 2.19 - Share snapshot instances admin APIs
             (list/show/detail/reset-status).
     * 2.20 - Add MTU to the JSON response of share network show API.
-
+    * 2.21 - Add access_key to the response of access_list API.
 """
 
 # The minimum and maximum versions of the API supported
 # The default api version request is defined to be the
 # the minimum version of the API supported.
 _MIN_API_VERSION = "2.0"
-_MAX_API_VERSION = "2.20"
+_MAX_API_VERSION = "2.21"
 DEFAULT_API_VERSION = _MIN_API_VERSION
 
 

manila/api/openstack/rest_api_version_history.rst

@@ -126,3 +126,7 @@ user documentation.
 2.20
 ----
   Add MTU in share network show API.
+
+2.21
+----
+  Add access_key in access_list API.

manila/api/v1/shares.py

@@ -28,6 +28,7 @@ from webob import exc
 
 from manila.api import common
 from manila.api.openstack import wsgi
+from manila.api.views import share_accesses as share_access_views
 from manila.api.views import shares as share_views
 from manila import db
 from manila import exception
@@ -497,7 +498,8 @@ class ShareMixin(object):
                 access_data.get('access_level'))
         except exception.ShareAccessExists as e:
             raise webob.exc.HTTPBadRequest(explanation=e.msg)
-        return {'access': access}
+
+        return self._access_view_builder.view(req, access)
 
     def _deny_access(self, req, id, body):
         """Remove share access rule."""
@@ -521,8 +523,9 @@ class ShareMixin(object):
         context = req.environ['manila.context']
 
         share = self.share_api.get(context, id)
-        access_list = self.share_api.access_get_all(context, share)
-        return {'access_list': access_list}
+        access_rules = self.share_api.access_get_all(context, share)
+
+        return self._access_view_builder.list_view(req, access_rules)
 
     def _extend(self, req, id, body):
         """Extend size of a share."""
@@ -576,6 +579,7 @@ class ShareController(wsgi.Controller, ShareMixin, wsgi.AdminActionsMixin):
     def __init__(self):
         super(self.__class__, self).__init__()
         self.share_api = share.API()
+        self._access_view_builder = share_access_views.ViewBuilder()
 
     @wsgi.action('os-reset_status')
     def share_reset_status(self, req, id, body):

manila/api/v2/shares.py

@@ -18,6 +18,7 @@ from manila.api.openstack import wsgi
 from manila.api.v1 import share_manage
 from manila.api.v1 import share_unmanage
 from manila.api.v1 import shares
+from manila.api.views import share_accesses as share_access_views
 from manila.api.views import shares as share_views
 from manila import share
 
@@ -34,6 +35,7 @@ class ShareController(shares.ShareMixin,
     def __init__(self):
         super(self.__class__, self).__init__()
         self.share_api = share.API()
+        self._access_view_builder = share_access_views.ViewBuilder()
 
     @wsgi.Controller.api_version("2.4")
     def create(self, req, body):

manila/api/views/share_accesses.py

@@ -0,0 +1,61 @@
+# Copyright (c) 2016 Red Hat, 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.
+
+from manila.api import common
+
+
+class ViewBuilder(common.ViewBuilder):
+    """Model a share access API response as a python dictionary."""
+
+    _collection_name = 'share_accesses'
+    _detail_version_modifiers = [
+        "add_access_key",
+    ]
+
+    def list_view(self, request, accesses):
+        """View of a list of share accesses."""
+        return {'access_list': [self.summary_view(request, access)['access']
+                                for access in accesses]}
+
+    def summary_view(self, request, access):
+        """Summarized view of a single share access."""
+        access_dict = {
+            'id': access.get('id'),
+            'access_level': access.get('access_level'),
+            'access_to': access.get('access_to'),
+            'access_type': access.get('access_type'),
+            'state': access.get('state'),
+        }
+        self.update_versioned_resource_dict(
+            request, access_dict, access)
+        return {'access': access_dict}
+
+    def view(self, request, access):
+        """Generic view of a single share access."""
+        access_dict = {
+            'id': access.get('id'),
+            'share_id': access.get('share_id'),
+            'access_level': access.get('access_level'),
+            'access_to': access.get('access_to'),
+            'access_type': access.get('access_type'),
+            'state': access.get('state'),
+        }
+        self.update_versioned_resource_dict(
+            request, access_dict, access)
+        return {'access': access_dict}
+
+    @common.ViewBuilder.versioned_method("2.21")
+    def add_access_key(self, context, access_dict, access):
+        access_dict['access_key'] = access.get('access_key')

manila/db/api.py

@@ -415,6 +415,11 @@ def share_access_get_all_for_share(context, share_id):
     return IMPL.share_access_get_all_for_share(context, share_id)
 
 
+def share_access_update_access_key(context, access_id, access_key):
+    """Update the access_key field of a share access mapping."""
+    return IMPL.share_access_update_access_key(context, access_id, access_key)
+
+
 def share_access_get_all_for_instance(context, instance_id, session=None):
     """Get all access rules related to a certain share instance."""
     return IMPL.share_access_get_all_for_instance(

manila/db/migrations/alembic/versions/63809d875e32_add_access_key.py

@@ -0,0 +1,36 @@
+# 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_key
+
+Revision ID: 63809d875e32
+Revises: 493eaffd79e1
+Create Date: 2016-07-16 20:53:05.958896
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '63809d875e32'
+down_revision = '493eaffd79e1'
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+    op.add_column(
+        'share_access_map',
+        sa.Column('access_key', sa.String(255), nullable=True))
+
+
+def downgrade():
+    op.drop_column('share_access_map', 'access_key')

manila/db/sqlalchemy/api.py

@@ -1813,6 +1813,17 @@ def share_access_delete_all_by_share(context, share_id):
             filter_by(share_id=share_id).soft_delete()
 
 
+@require_context
+def share_access_update_access_key(context, access_id, access_key):
+    session = get_session()
+    with session.begin():
+        mapping = (session.query(models.ShareAccessMapping).
+                   filter_by(id=access_id).first())
+        mapping.update({'access_key': access_key})
+        mapping.save(session=session)
+        return mapping
+
+
 @require_context
 def share_instance_access_delete(context, mapping_id):
     session = get_session()

manila/db/sqlalchemy/models.py

@@ -538,6 +538,7 @@ class ShareAccessMapping(BASE, ManilaBase):
     share_id = Column(String(36), ForeignKey('shares.id'))
     access_type = Column(String(255))
     access_to = Column(String(255))
+    access_key = Column(String(255), nullable=True)
 
     access_level = Column(Enum(*constants.ACCESS_LEVELS),
                           default=constants.ACCESS_LEVEL_RW)

manila/share/access.py

@@ -17,6 +17,8 @@ from oslo_log import log
 import six
 
 from manila.common import constants
+from manila import exception
+from manila.i18n import _
 from manila.i18n import _LI
 from manila import utils
 
@@ -100,8 +102,9 @@ class ShareInstanceAccess(object):
                     delete_rules = []
 
         try:
+            access_keys = None
             try:
-                self.driver.update_access(
+                access_keys = self.driver.update_access(
                     context,
                     share_instance,
                     rules,
@@ -116,6 +119,15 @@ class ShareInstanceAccess(object):
                 self._update_access_fallback(add_rules, context, delete_rules,
                                              remove_rules, share_instance,
                                              share_server)
+
+            if access_keys:
+                self._validate_access_keys(rules, add_rules, delete_rules,
+                                           access_keys)
+
+                for access_id, access_key in access_keys.items():
+                    self.db.share_access_update_access_key(
+                        context, access_id, access_key)
+
         except Exception:
             self.db.share_instance_update_access_status(
                 context,
@@ -146,6 +158,32 @@ class ShareInstanceAccess(object):
                          "share instance: %s"),
                      share_instance['id'])
 
+    @staticmethod
+    def _validate_access_keys(access_rules, add_rules, delete_rules,
+                              access_keys):
+        if not isinstance(access_keys, dict):
+            msg = _("The access keys must be supplied as a dictionary that "
+                    "maps rule IDs to access keys.")
+            raise exception.Invalid(message=msg)
+
+        actual_rule_ids = sorted(access_keys)
+        expected_rule_ids = []
+        if not (add_rules or delete_rules):
+            expected_rule_ids = [rule['id'] for rule in access_rules]
+        else:
+            expected_rule_ids = [rule['id'] for rule in add_rules]
+        if actual_rule_ids != sorted(expected_rule_ids):
+            msg = (_("The rule IDs supplied: %(actual)s do not match the "
+                     "rule IDs that are expected: %(expected)s.")
+                   % {'actual': actual_rule_ids,
+                      'expected': expected_rule_ids})
+            raise exception.Invalid(message=msg)
+
+        for access_key in access_keys.values():
+            if not isinstance(access_key, six.string_types):
+                msg = (_("Access key %s is not string type.") % access_key)
+                raise exception.Invalid(message=msg)
+
     def _check_needs_refresh(self, context, rules, share_instance):
         rule_ids = set([rule['id'] for rule in rules])
         queried_rules = self.db.share_access_get_all_for_instance(

manila/share/api.py

@@ -1213,14 +1213,7 @@ class API(base.Base):
         # NOTE(tpsilva): refreshing share_access model
         access = self.db.share_access_get(ctx, access['id'])
 
-        return {
-            'id': access['id'],
-            'share_id': access['share_id'],
-            'access_type': access['access_type'],
-            'access_to': access['access_to'],
-            'access_level': access['access_level'],
-            'state': access['state'],
-        }
+        return access
 
     def allow_access_to_instance(self, context, share_instance, access):
         policy.check_policy(context, 'share', 'allow_access')
@@ -1302,12 +1295,7 @@ class API(base.Base):
         """Returns all access rules for share."""
         policy.check_policy(context, 'share', 'access_get_all')
         rules = self.db.share_access_get_all_for_share(context, share['id'])
-        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]
+        return rules
 
     def access_get(self, context, access_id):
         """Returns access rule with the id."""

manila/share/driver.py

@@ -532,6 +532,14 @@ class ShareDriver(object):
         :param delete_rules: Empty List or List of access rules which should be
                removed. access_rules doesn't contain these rules.
         :param share_server: None or Share server model
+        :returns: None, or a dictionary of ``access_id``, ``access_key`` as
+                  key: value pairs for the rules added, where, ``access_id``
+                  is the UUID (string) of the access rule, and ``access_key``
+                  is the credential (string) of the entity granted access.
+                  During recovery after error, the returned dictionary must
+                  contain ``access_id``, ``access_key`` for all the rules that
+                  the driver is ordered to resync, i.e. rules in the
+                  ``access_rules`` parameter.
         """
         raise NotImplementedError()
 

manila/tests/api/v1/test_shares.py

@@ -827,6 +827,9 @@ class ShareActionsTest(test.TestCase):
         self.mock_object(share_api.API,
                          'allow_access',
                          mock.Mock(return_value={'fake': 'fake'}))
+        self.mock_object(self.controller._access_view_builder, 'view',
+                         mock.Mock(return_value={'access':
+                                                 {'fake': 'fake'}}))
 
         id = 'fake_share_id'
         body = {'os-allow_access': access}
@@ -887,20 +890,23 @@ class ShareActionsTest(test.TestCase):
                           body)
 
     def test_access_list(self):
-        def _fake_access_get_all(*args, **kwargs):
-            return [{"state": "fakestatus",
-                     "id": "fake_share_id",
+        fake_access_list = [
+            {
+                "state": "fakestatus",
+                "id": "fake_access_id",
                 "access_type": "fakeip",
-                     "access_to": "127.0.0.1"}]
-
-        self.mock_object(share_api.API, "access_get_all",
-                         _fake_access_get_all)
+                "access_to": "127.0.0.1",
+            }
+        ]
+        self.mock_object(self.controller._access_view_builder, 'list_view',
+                         mock.Mock(return_value={'access_list':
+                                                 fake_access_list}))
         id = 'fake_share_id'
         body = {"os-access_list": None}
         req = fakes.HTTPRequest.blank('/v1/tenant1/shares/%s/action' % id)
+
         res_dict = self.controller._access_list(req, id, body)
-        expected = _fake_access_get_all()
-        self.assertEqual(expected, res_dict['access_list'])
+        self.assertEqual({'access_list': fake_access_list}, res_dict)
 
     def test_extend(self):
         id = 'fake_share_id'

manila/tests/api/v2/test_shares.py

@@ -1328,6 +1328,9 @@ class ShareActionsTest(test.TestCase):
         self.mock_object(share_api.API,
                          'allow_access',
                          mock.Mock(return_value={'fake': 'fake'}))
+        self.mock_object(self.controller._access_view_builder, 'view',
+                         mock.Mock(return_value={'access':
+                                                 {'fake': 'fake'}}))
 
         id = 'fake_share_id'
         body = {'allow_access': access}
@@ -1370,6 +1373,9 @@ class ShareActionsTest(test.TestCase):
         self.mock_object(share_api.API,
                          'allow_access',
                          mock.Mock(return_value={'fake': 'fake'}))
+        self.mock_object(self.controller._access_view_builder, 'view',
+                         mock.Mock(return_value={'access':
+                                                 {'fake': 'fake'}}))
 
         req = fakes.HTTPRequest.blank(
             '/v2/shares/%s/action' % share_id, version=version)
@@ -1419,20 +1425,23 @@ class ShareActionsTest(test.TestCase):
                           body)
 
     def test_access_list(self):
-        def _fake_access_get_all(*args, **kwargs):
-            return [{"state": "fakestatus",
-                     "id": "fake_share_id",
+        fake_access_list = [
+            {
+                "state": "fakestatus",
+                "id": "fake_access_id",
                 "access_type": "fakeip",
-                     "access_to": "127.0.0.1"}]
-
-        self.mock_object(share_api.API, "access_get_all",
-                         _fake_access_get_all)
+                "access_to": "127.0.0.1",
+            }
+        ]
+        self.mock_object(self.controller._access_view_builder, 'list_view',
+                         mock.Mock(return_value={'access_list':
+                                                 fake_access_list}))
         id = 'fake_share_id'
         body = {"os-access_list": None}
-        req = fakes.HTTPRequest.blank('/v1/tenant1/shares/%s/action' % id)
+        req = fakes.HTTPRequest.blank('/v2/tenant1/shares/%s/action' % id)
+
         res_dict = self.controller._access_list(req, id, body)
-        expected = _fake_access_get_all()
-        self.assertEqual(expected, res_dict['access_list'])
+        self.assertEqual({'access_list': fake_access_list}, res_dict)
 
     @ddt.unpack
     @ddt.data(

manila/tests/api/views/test_share_accesses.py

@@ -0,0 +1,80 @@
+# Copyright (c) 2016 Red Hat, 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.
+
+import ddt
+
+from manila.api.openstack import api_version_request as api_version
+from manila.api.views import share_accesses
+from manila import test
+from manila.tests.api import fakes
+
+
+@ddt.ddt
+class ViewBuilderTestCase(test.TestCase):
+
+    def setUp(self):
+        super(ViewBuilderTestCase, self).setUp()
+        self.builder = share_accesses.ViewBuilder()
+        self.fake_access = {
+            'id': 'fakeaccessid',
+            'share_id': 'fakeshareid',
+            'access_level': 'fakeaccesslevel',
+            'access_to': 'fakeacccessto',
+            'access_type': 'fakeaccesstype',
+            'state': 'fakeaccessstate',
+            'access_key': 'fakeaccesskey',
+        }
+
+    def test_collection_name(self):
+        self.assertEqual('share_accesses', self.builder._collection_name)
+
+    @ddt.data("2.20", "2.21")
+    def test_view(self, version):
+        req = fakes.HTTPRequest.blank('/shares', version=version)
+
+        result = self.builder.view(req, self.fake_access)
+
+        if (api_version.APIVersionRequest(version) <
+                api_version.APIVersionRequest("2.21")):
+            del self.fake_access['access_key']
+
+        self.assertEqual({'access': self.fake_access}, result)
+
+    @ddt.data("2.20", "2.21")
+    def test_summary_view(self, version):
+        req = fakes.HTTPRequest.blank('/shares', version=version)
+
+        result = self.builder.summary_view(req, self.fake_access)
+
+        if (api_version.APIVersionRequest(version) <
+                api_version.APIVersionRequest("2.21")):
+            del self.fake_access['access_key']
+        del self.fake_access['share_id']
+
+        self.assertEqual({'access': self.fake_access}, result)
+
+    @ddt.data("2.20", "2.21")
+    def test_list_view(self, version):
+        req = fakes.HTTPRequest.blank('/shares', version=version)
+        accesses = [self.fake_access, ]
+
+        result = self.builder.list_view(req, accesses)
+
+        if (api_version.APIVersionRequest(version) <
+                api_version.APIVersionRequest("2.21")):
+            del self.fake_access['access_key']
+        del self.fake_access['share_id']
+
+        self.assertEqual({'access_list': accesses}, result)

manila/tests/db/migrations/alembic/migrations_data_checks.py

@@ -920,3 +920,67 @@ class NewMTUColumnChecks(BaseMigrationChecks):
             self.test_case.assertTrue(db_result.rowcount >= len(ids))
             for record in db_result:
                 self.test_case.assertFalse(hasattr(record, 'mtu'))
+
+
+@map_to_migration('63809d875e32')
+class AddAccessKeyToShareAccessMapping(BaseMigrationChecks):
+    table_name = 'share_access_map'
+    access_key_column_name = 'access_key'
+
+    def setup_upgrade_data(self, engine):
+        share_data = {
+            'id': uuidutils.generate_uuid(),
+            'share_proto': "CEPHFS",
+            'size': 1,
+            'snapshot_id': None,
+            'user_id': 'fake',
+            'project_id': 'fake'
+        }
+        share_table = utils.load_table('shares', engine)
+        engine.execute(share_table.insert(share_data))
+
+        share_instance_data = {
+            'id': uuidutils.generate_uuid(),
+            'deleted': 'False',
+            'host': 'fake',
+            'share_id': share_data['id'],
+            'status': 'available',
+            'access_rules_status': 'active'
+        }
+        share_instance_table = utils.load_table('share_instances', engine)
+        engine.execute(share_instance_table.insert(share_instance_data))
+
+        share_access_data = {
+            'id': uuidutils.generate_uuid(),
+            'share_id': share_data['id'],
+            'access_type': 'cephx',
+            'access_to': 'alice',
+            'deleted': 'False'
+        }
+        share_access_table = utils.load_table(self.table_name, engine)
+        engine.execute(share_access_table.insert(share_access_data))
+
+        share_instance_access_data = {
+            'id': uuidutils.generate_uuid(),
+            'share_instance_id': share_instance_data['id'],
+            'access_id': share_access_data['id'],
+            'deleted': 'False'
+        }
+        share_instance_access_table = utils.load_table(
+            'share_instance_access_map', engine)
+        engine.execute(share_instance_access_table.insert(
+            share_instance_access_data))
+
+    def check_upgrade(self, engine, data):
+        share_access_table = utils.load_table(self.table_name, engine)
+        rows = engine.execute(share_access_table.select())
+        for row in rows:
+            self.test_case.assertTrue(hasattr(row,
+                                              self.access_key_column_name))
+
+    def check_downgrade(self, engine):
+        share_access_table = utils.load_table(self.table_name, engine)
+        rows = engine.execute(share_access_table.select())
+        for row in rows:
+            self.test_case.assertFalse(hasattr(row,
+                                               self.access_key_column_name))

manila/tests/db/sqlalchemy/test_api.py

@@ -120,6 +120,17 @@ class ShareAccessDatabaseAPITestCase(test.TestCase):
             "fake_status"
         )
 
+    @ddt.data(None, 'rhubarb')
+    def test_share_access_update_access_key(self, key_value):
+        share = db_utils.create_share()
+        access = db_utils.create_access(share_id=share['id'])
+
+        db_api.share_access_update_access_key(self.ctxt, access['id'],
+                                              key_value)
+
+        access = db_api.share_access_get(self.ctxt, access['id'])
+        self.assertEqual(key_value, access['access_key'])
+
 
 @ddt.ddt
 class ShareDatabaseAPITestCase(test.TestCase):

manila/tests/share/test_access.py

@@ -37,6 +37,10 @@ class ShareInstanceAccessTestCase(test.TestCase):
         self.share_instance = db_utils.create_share_instance(
             share_id=self.share['id'],
             access_rules_status=constants.STATUS_ERROR)
+        self.rule = db_utils.create_access(
+            id='fakeaccessid',
+            share_id=self.share['id'],
+            access_to='fakeaccessto')
 
     @ddt.data(True, False)
     def test_update_access_rules_maintenance_mode(self, maintenance_mode):
@@ -64,7 +68,8 @@ class ShareInstanceAccessTestCase(test.TestCase):
                          mock.Mock(return_value=existing_rules))
         self.mock_object(db, "share_instance_update_access_status",
                          mock.Mock())
-        self.mock_object(self.driver, "update_access", mock.Mock())
+        self.mock_object(self.driver, "update_access",
+                         mock.Mock(return_value=None))
         self.mock_object(self.share_access_helper,
                          "_remove_access_rules", mock.Mock())
         self.mock_object(self.share_access_helper, "_check_needs_refresh",
@@ -85,6 +90,87 @@ class ShareInstanceAccessTestCase(test.TestCase):
         db.share_instance_update_access_status.assert_called_with(
             self.context, share_instance['id'], constants.STATUS_ACTIVE)
 
+    @ddt.data(None, {'fakeaccessid': 'fakeaccesskey'})
+    def test_update_access_rules_returns_access_keys(self, access_keys):
+        share_instance = db_utils.create_share_instance(
+            id='fakeshareinstanceid',
+            share_id=self.share['id'],
+            access_rules_status=constants.STATUS_ACTIVE)
+        rules = [self.rule]
+
+        self.mock_object(db, "share_instance_get", mock.Mock(
+            return_value=share_instance))
+        self.mock_object(db, "share_access_get_all_for_instance",
+                         mock.Mock(return_value=rules))
+        self.mock_object(db, "share_instance_update_access_status",
+                         mock.Mock())
+        self.mock_object(db, "share_access_update_access_key",
+                         mock.Mock())
+        self.mock_object(self.driver, "update_access",
+                         mock.Mock(return_value=access_keys))
+        self.mock_object(self.share_access_helper,
+                         "_remove_access_rules", mock.Mock())
+        self.mock_object(self.share_access_helper, "_check_needs_refresh",
+                         mock.Mock(return_value=False))
+
+        self.share_access_helper.update_access_rules(
+            self.context, share_instance['id'], add_rules=rules)
+
+        self.driver.update_access.assert_called_once_with(
+            self.context, share_instance, rules, add_rules=rules,
+            delete_rules=[], share_server=None)
+        self.share_access_helper._remove_access_rules.assert_called_once_with(
+            self.context, [], share_instance['id'])
+        self.share_access_helper._check_needs_refresh.assert_called_once_with(
+            self.context, rules, share_instance)
+        if access_keys:
+            db.share_access_update_access_key.assert_called_with(
+                self.context, 'fakeaccessid', 'fakeaccesskey')
+        else:
+            self.assertFalse(db.share_access_update_access_key.called)
+        db.share_instance_update_access_status.assert_called_with(
+            self.context, share_instance['id'], constants.STATUS_ACTIVE)
+
+    @ddt.data({'maintenance_mode': True,
+               'access_keys': ['invalidaccesskey']},
+              {'maintenance_mode': True,
+               'access_keys': {'invalidaccessid': 'accesskey'}},
+              {'maintenance_mode': True,
+               'access_keys': {'fakeaccessid': 9}},
+              {'maintenance_mode': False,
+               'access_keys': {'fakeaccessid': 9}})
+    @ddt.unpack
+    def test_update_access_rules_invalid_access_keys(self, maintenance_mode,
+                                                     access_keys):
+        access_rules_status = (
+            constants.STATUS_ERROR if maintenance_mode
+            else constants.STATUS_ACTIVE)
+        share_instance = db_utils.create_share_instance(
+            id='fakeid',
+            share_id=self.share['id'],
+            access_rules_status=access_rules_status)
+
+        rules = [self.rule]
+        add_rules = [] if maintenance_mode else rules
+
+        self.mock_object(db, "share_instance_get", mock.Mock(
+            return_value=share_instance))
+        self.mock_object(db, "share_access_get_all_for_instance",
+                         mock.Mock(return_value=rules))
+        self.mock_object(db, "share_instance_update_access_status",
+                         mock.Mock())
+        self.mock_object(self.driver, "update_access",
+                         mock.Mock(return_value=access_keys))
+
+        self.assertRaises(exception.Invalid,
+                          self.share_access_helper.update_access_rules,
+                          self.context, share_instance['id'],
+                          add_rules=add_rules)
+
+        self.driver.update_access.assert_called_once_with(
+            self.context, share_instance, rules, add_rules=add_rules,
+            delete_rules=[], share_server=None)
+
     def test_update_access_rules_fallback(self):
         add_rules = [db_utils.create_access(share_id=self.share['id'])]
         delete_rules = [db_utils.create_access(share_id=self.share['id'])]
@@ -158,7 +244,8 @@ class ShareInstanceAccessTestCase(test.TestCase):
             return_value=share_instance))
         self.mock_object(db, "share_access_get_all_for_instance",
                          mock.Mock(return_value=original_rules))
-        mock_update_access = self.mock_object(self.driver, "update_access")
+        mock_update_access = self.mock_object(self.driver, "update_access",
+                                              mock.Mock(return_value=None))
         self.mock_object(self.share_access_helper, '_check_needs_refresh',
                          mock.Mock(side_effect=[True, False]))
 

manila/tests/share/test_api.py

@@ -101,6 +101,7 @@ def fake_access(id, **kwargs):
         'access_type': 'fakeacctype',
         'access_to': 'fakeaccto',
         'access_level': 'rw',
+        'access_key': None,
         'state': 'fakeactive',
         'STATE_NEW': 'fakenew',
         'STATE_ACTIVE': 'fakeactive',
@@ -1630,13 +1631,10 @@ class ShareAPITestCase(test.TestCase):
             'access_to': 'fake_access_to',
             'access_level': level,
         }
-        fake_access_expected = copy.deepcopy(values)
-        fake_access_expected.update({
+        fake_access = copy.deepcopy(values)
+        fake_access.update({
             'id': 'fake_access_id',
             'state': constants.STATUS_ACTIVE,
-        })
-        fake_access = copy.deepcopy(fake_access_expected)
-        fake_access.update({
             'deleted': 'fake_deleted',
             'deleted_at': 'fake_deleted_at',
             'instance_mappings': ['foo', 'bar'],
@@ -1650,7 +1648,7 @@ class ShareAPITestCase(test.TestCase):
             self.context, share, fake_access['access_type'],
             fake_access['access_to'], level)
 
-        self.assertEqual(fake_access_expected, access)
+        self.assertEqual(fake_access, access)
         self.share_rpcapi.allow_access.assert_called_once_with(
             self.context, utils.IsAMatcher(models.ShareInstance),
             fake_access)
@@ -1865,11 +1863,8 @@ class ShareAPITestCase(test.TestCase):
         self.mock_object(db_api, 'share_access_get_all_for_share',
                          mock.Mock(return_value=rules))
         actual = self.api.access_get_all(self.context, share)
-        for access in actual:
-            expected_access = values[access['id']]
-            expected_access.pop('share_id')
-            self.assertEqual(expected_access, access)
 
+        self.assertEqual(rules, actual)
         share_api.policy.check_policy.assert_called_once_with(
             self.context, 'share', 'access_get_all')
         db_api.share_access_get_all_for_share.assert_called_once_with(

manila/tests/share/test_manager.py

@@ -2419,6 +2419,10 @@ class ShareManagerTestCase(test.TestCase):
                 mock.Mock(side_effect=exception.ShareResourceNotFound(
                     share_id=share['id'])))
         if side_effect == 'delete_share':
+            self.mock_object(
+                self.share_manager.access_helper.driver, 'update_access',
+                mock.Mock(return_value=None)
+            )
             self.mock_object(
                 self.share_manager.driver, 'delete_share',
                 mock.Mock(side_effect=exception.ShareResourceNotFound(

manila_tempest_tests/config.py

@@ -34,7 +34,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.20",
+               default="2.21",
                help="The maximum api microversion is configured to be the "
                     "value of the latest microversion supported by Manila."),
     cfg.StrOpt("region",

manila_tempest_tests/tests/api/test_rules.py

@@ -424,7 +424,7 @@ class ShareRulesTest(base.BaseSharesTest):
         elif CONF.share.enable_cephx_rules_for_protocols:
             cls.protocol = CONF.share.enable_cephx_rules_for_protocols[0]
             cls.access_type = "cephx"
-            cls.access_to = "alice"
+            cls.access_to = "eve"
         cls.shares_v2_client.share_protocol = cls.protocol
         cls.share = cls.create_share()
 
@@ -465,7 +465,10 @@ class ShareRulesTest(base.BaseSharesTest):
                                                             version=version)
 
         # verify keys
-        for key in ("id", "access_type", "access_to", "access_level"):
+        keys = ("id", "access_type", "access_to", "access_level")
+        if utils.is_microversion_ge(version, '2.21'):
+            keys += ("access_key", )
+        for key in keys:
             [self.assertIn(key, r.keys()) for r in rules]
         for key in ('deleted', 'deleted_at', 'instance_mappings'):
             [self.assertNotIn(key, r.keys()) for r in rules]
@@ -474,6 +477,11 @@ class ShareRulesTest(base.BaseSharesTest):
         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"])
+        if utils.is_microversion_ge(version, '2.21'):
+            if self.access_type == 'cephx':
+                self.assertIsNotNone(rules[0]['access_key'])
+            else:
+                self.assertIsNone(rules[0]['access_key'])
 
         # our share id in list and have no duplicates
         gen = [r["id"] for r in rules if r["id"] in rule["id"]]

releasenotes/notes/add-access-key-to-share-access-map-2fda4c06a750e24e.yaml

@@ -0,0 +1,5 @@
+---
+features:
+  - Driver may return ``access_key``, an access credential, for client
+    identities granted share access.
+  - Added ``access_key`` to the JSON response of ``access_list`` API.