Merge "Consolidate waiters methods"
diff --git a/manila_tempest_tests/tests/api/base.py b/manila_tempest_tests/tests/api/base.py
index b657935..0a82cf8 100755
--- a/manila_tempest_tests/tests/api/base.py
+++ b/manila_tempest_tests/tests/api/base.py
@@ -1268,7 +1268,8 @@
         cls.os_admin.domains_client = (
             cls.os_admin.identity_v3.DomainsClient() if
             CONF.identity.auth_version == 'v3' else None)
-        cls.admin_project_member_client = cls.create_user_and_get_client()
+        cls.admin_project_member_client = cls.create_user_and_get_client(
+            project=cls.admin_project, add_member_role=True)
 
         if CONF.share.multitenancy_enabled:
             admin_share_network_id = cls.provide_share_network(
@@ -1283,7 +1284,7 @@
             cls.alt_shares_v2_client.share_network_id = alt_share_network_id
 
     @classmethod
-    def create_user_and_get_client(cls, project=None):
+    def create_user_and_get_client(cls, project=None, add_member_role=True):
         """Create a user in specified project & set share clients for user
 
         The user will have all roles specified in tempest.conf
@@ -1308,9 +1309,12 @@
             username, password, project, email)
         cls.class_project_users_created.append(user)
 
-        for conf_role in CONF.auth.tempest_roles:
-            cls.os_admin.creds_client.assign_user_role(
-                user, project, conf_role)
+        tempest_roles_to_assign = CONF.auth.tempest_roles or []
+        if "member" not in tempest_roles_to_assign and add_member_role:
+            tempest_roles_to_assign.append("member")
+
+        for role in tempest_roles_to_assign:
+            cls.os_admin.creds_client.assign_user_role(user, project, role)
 
         user_creds = cls.os_admin.creds_client.get_credentials(
             user, project, password)
diff --git a/manila_tempest_tests/tests/api/test_public_shares.py b/manila_tempest_tests/tests/api/test_public_shares.py
new file mode 100644
index 0000000..6b0ef69
--- /dev/null
+++ b/manila_tempest_tests/tests/api/test_public_shares.py
@@ -0,0 +1,106 @@
+#    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 tempest import config
+from tempest.lib.common.utils import data_utils
+from tempest.lib import decorators
+from testtools import testcase as tc
+
+from manila_tempest_tests.tests.api import base
+
+CONF = config.CONF
+LATEST_MICROVERSION = CONF.share.max_api_microversion
+
+
+class PublicSharesTest(base.BaseSharesMixedTest):
+
+    @classmethod
+    def resource_setup(cls):
+        super(PublicSharesTest, cls).resource_setup()
+        # create share_type
+        share_type = cls._create_share_type()
+        cls.share_type_id = share_type['id']
+
+    @decorators.idempotent_id('557a0474-9e30-47b4-a766-19e2afb13e66')
+    @tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND)
+    def test_list_shares_public_with_detail(self):
+        # The default RBAC policy in manila only allows admin users with
+        # system scope to create public shares since the Stein release
+        public_share = self.create_share(
+            name='public_share - must be visible to all projects in the cloud',
+            description='public_share_desc',
+            share_type_id=self.share_type_id,
+            is_public=True,
+            cleanup_in_class=False,
+            client=self.admin_shares_v2_client,
+            version=LATEST_MICROVERSION
+        )
+        private_share = self.create_share(
+            name='private_share',
+            description='private share in the primary user project',
+            share_type_id=self.share_type_id,
+            is_public=False,
+            cleanup_in_class=False,
+            version=LATEST_MICROVERSION
+        )
+
+        params = {'is_public': True}
+        shares = self.alt_shares_v2_client.list_shares_with_detail(params)
+
+        keys = [
+            'status', 'description', 'links', 'availability_zone',
+            'created_at', 'share_proto', 'name', 'snapshot_id', 'id',
+            'size', 'project_id', 'is_public',
+        ]
+        [self.assertIn(key, sh.keys()) for sh in shares for key in keys]
+
+        retrieved_public_share = [
+            share for share in shares if share['id'] == public_share['id']
+        ]
+        msg = 'expected id lists %s times in share list' % (
+            len(retrieved_public_share))
+        self.assertEqual(1, len(retrieved_public_share), msg)
+        self.assertTrue(retrieved_public_share[0]['is_public'])
+
+        self.assertFalse(any([s['id'] == private_share['id'] for s in shares]))
+
+    @decorators.idempotent_id('e073182e-459d-4e08-9300-5bc964ca806b')
+    @tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND)
+    def test_update_share_set_is_public(self):
+        share_name = data_utils.rand_name('tempest-share-name')
+        share = self.create_share(name=share_name,
+                                  description='a share we will update',
+                                  share_type_id=self.share_type_id,
+                                  is_public=False,
+                                  cleanup_in_class=False,
+                                  version=LATEST_MICROVERSION)
+
+        share = self.shares_v2_client.get_share(share['id'])
+        self.assertEqual(share_name, share['name'])
+        self.assertEqual('a share we will update', share['description'])
+        self.assertFalse(share['is_public'])
+
+        # update share, manila's default RBAC only allows administrator
+        # users with a system scope token to update a private share to public
+        new_name = data_utils.rand_name('tempest-new-share-name')
+        new_desc = 'share is now updated'
+        updated = self.admin_shares_v2_client.update_share(
+            share['id'], name=new_name, desc=new_desc, is_public=True)
+        self.assertEqual(new_name, updated['name'])
+        self.assertEqual(new_desc, updated['description'])
+        self.assertTrue(updated['is_public'])
+
+        # this share must now be publicly accessible
+        share = self.alt_shares_v2_client.get_share(share['id'])
+        self.assertEqual(new_name, share['name'])
+        self.assertEqual(new_desc, share['description'])
+        self.assertTrue(share['is_public'])
diff --git a/manila_tempest_tests/tests/api/test_public_shares_negative.py b/manila_tempest_tests/tests/api/test_public_shares_negative.py
new file mode 100644
index 0000000..6d99f13
--- /dev/null
+++ b/manila_tempest_tests/tests/api/test_public_shares_negative.py
@@ -0,0 +1,84 @@
+#    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 tempest.lib import decorators
+from tempest.lib import exceptions as lib_exc
+from testtools import testcase as tc
+
+from manila_tempest_tests.tests.api import base
+
+
+class PublicSharesNegativeTest(base.BaseSharesMixedTest):
+
+    @classmethod
+    def resource_setup(cls):
+        super(PublicSharesNegativeTest, cls).resource_setup()
+        # create share_type
+        share_type = cls._create_share_type()
+        share_type_id = share_type['id']
+        # create a public share - manila's default RBAC only allows
+        # administrator users operating at system scope to create public shares
+        cls.share = cls.create_share(
+            name='public_share',
+            description='public_share_desc',
+            share_type_id=share_type_id,
+            is_public=True,
+            metadata={'key': 'value'},
+            client=cls.admin_shares_v2_client
+        )
+
+    @decorators.idempotent_id('255011c0-4ed9-4174-bb13-8bbd06a62529')
+    @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
+    def test_update_share_with_wrong_public_value(self):
+        self.assertRaises(lib_exc.BadRequest,
+                          self.admin_shares_v2_client.update_share,
+                          self.share["id"],
+                          is_public="truebar")
+
+    @decorators.idempotent_id('3443493b-f56a-4faa-9968-e7cbb0d2802f')
+    @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
+    def test_update_other_tenants_public_share(self):
+        self.assertRaises(lib_exc.Forbidden,
+                          self.alt_shares_v2_client.update_share,
+                          self.share["id"],
+                          name="new_name")
+
+    @decorators.idempotent_id('68d1f1bc-16e4-4086-8982-7e44ca6bdc4d')
+    @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
+    def test_delete_other_tenants_public_share(self):
+        self.assertRaises(lib_exc.Forbidden,
+                          self.alt_shares_v2_client.delete_share,
+                          self.share['id'])
+
+    @decorators.idempotent_id('1f9e5d84-0885-4a4b-9196-9031a1c01508')
+    @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
+    def test_set_metadata_of_other_tenants_public_share(self):
+        self.assertRaises(lib_exc.Forbidden,
+                          self.alt_shares_v2_client.set_metadata,
+                          self.share['id'],
+                          {'key': 'value'})
+
+    @decorators.idempotent_id('fed7a935-9699-43a1-854e-67b61ba6233e')
+    @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
+    def test_update_metadata_of_other_tenants_public_share(self):
+        self.assertRaises(lib_exc.Forbidden,
+                          self.alt_shares_v2_client.update_all_metadata,
+                          self.share['id'],
+                          {'key': 'value'})
+
+    @decorators.idempotent_id('bd62adeb-73c2-4b04-8812-80b479cd5c3b')
+    @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
+    def test_delete_metadata_of_other_tenants_public_share(self):
+        self.assertRaises(lib_exc.Forbidden,
+                          self.alt_shares_v2_client.delete_metadata,
+                          self.share['id'],
+                          'key')
diff --git a/manila_tempest_tests/tests/api/test_shares_actions.py b/manila_tempest_tests/tests/api/test_shares_actions.py
index f67c901..f0792eb 100644
--- a/manila_tempest_tests/tests/api/test_shares_actions.py
+++ b/manila_tempest_tests/tests/api/test_shares_actions.py
@@ -435,40 +435,6 @@
         shares = self.shares_v2_client.list_shares_with_detail(params)
         self.assertGreater(shares["count"], 0)
 
-    @decorators.idempotent_id('557a0474-9e30-47b4-a766-19e2afb13e66')
-    @tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND)
-    def test_list_shares_public_with_detail(self):
-        public_share = self.create_share(
-            name='public_share',
-            description='public_share_desc',
-            share_type_id=self.share_type_id,
-            is_public=True,
-            cleanup_in_class=False
-        )
-        private_share = self.create_share(
-            name='private_share',
-            description='private_share_desc',
-            share_type_id=self.share_type_id,
-            is_public=False,
-            cleanup_in_class=False
-        )
-
-        params = {"is_public": True}
-        shares = self.alt_shares_client.list_shares_with_detail(params)
-
-        keys = [
-            "status", "description", "links", "availability_zone",
-            "created_at", "export_location", "share_proto",
-            "name", "snapshot_id", "id", "size", "project_id", "is_public",
-        ]
-        [self.assertIn(key, sh.keys()) for sh in shares for key in keys]
-
-        gen = [sid["id"] for sid in shares if sid["id"] == public_share["id"]]
-        msg = "expected id lists %s times in share list" % (len(gen))
-        self.assertEqual(1, len(gen), msg)
-
-        self.assertFalse(any([s["id"] == private_share["id"] for s in shares]))
-
     @decorators.idempotent_id('174829eb-fd3e-46ef-880b-f05c3d44d1fe')
     @tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND)
     @testtools.skipUnless(CONF.share.run_snapshot_tests,
@@ -748,16 +714,15 @@
         new_name = data_utils.rand_name("tempest-new-name")
         new_desc = data_utils.rand_name("tempest-new-description")
         updated = self.shares_client.update_share(
-            share["id"], new_name, new_desc, is_public=True)
+            share["id"], name=new_name, desc=new_desc)
         self.assertEqual(new_name, updated["name"])
         self.assertEqual(new_desc, updated["description"])
-        self.assertTrue(updated["is_public"])
 
         # get share
         share = self.shares_client.get_share(self.share['id'])
         self.assertEqual(new_name, share["name"])
         self.assertEqual(new_desc, share["description"])
-        self.assertTrue(share["is_public"])
+        self.assertFalse(share["is_public"])
 
     @decorators.idempotent_id('20f299f6-2441-4629-b44e-d791d57f413c')
     @tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND)
diff --git a/manila_tempest_tests/tests/api/test_shares_from_snapshot_across_pools.py b/manila_tempest_tests/tests/api/test_shares_from_snapshot_across_pools.py
index f8c5255..c86c56d 100644
--- a/manila_tempest_tests/tests/api/test_shares_from_snapshot_across_pools.py
+++ b/manila_tempest_tests/tests/api/test_shares_from_snapshot_across_pools.py
@@ -147,7 +147,6 @@
             raise self.skipException(msg)
         azs = list(azs)
         share_a = self.create_share(share_type_id=self.share_type_id,
-                                    is_public=True,
                                     availability_zone=azs[0])
 
         # Create snapshot
diff --git a/manila_tempest_tests/tests/api/test_shares_negative.py b/manila_tempest_tests/tests/api/test_shares_negative.py
index 9ee4202..d00216d 100644
--- a/manila_tempest_tests/tests/api/test_shares_negative.py
+++ b/manila_tempest_tests/tests/api/test_shares_negative.py
@@ -34,22 +34,6 @@
         cls.share_type = cls._create_share_type()
         cls.share_type_id = cls.share_type['id']
 
-        # create share
-        cls.share = cls.create_share(
-            name='public_share',
-            description='public_share_desc',
-            share_type_id=cls.share_type_id,
-            is_public=True,
-            metadata={'key': 'value'}
-        )
-
-    @decorators.idempotent_id('255011c0-4ed9-4174-bb13-8bbd06a62529')
-    @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
-    def test_update_share_with_wrong_public_value(self):
-        self.assertRaises(lib_exc.BadRequest,
-                          self.shares_client.update_share, self.share["id"],
-                          is_public="truebar")
-
     @decorators.idempotent_id('b9bb8dee-0c7c-4e51-909c-028335b1a6a0')
     @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
     @testtools.skipUnless(CONF.share.run_snapshot_tests,
@@ -154,45 +138,6 @@
             snapshot_id=snap["id"],
         )
 
-    @decorators.idempotent_id('3443493b-f56a-4faa-9968-e7cbb0d2802f')
-    @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
-    def test_update_other_tenants_public_share(self):
-        self.assertRaises(lib_exc.Forbidden,
-                          self.alt_shares_v2_client.update_share,
-                          self.share["id"],
-                          name="new_name")
-
-    @decorators.idempotent_id('68d1f1bc-16e4-4086-8982-7e44ca6bdc4d')
-    @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
-    def test_delete_other_tenants_public_share(self):
-        self.assertRaises(lib_exc.Forbidden,
-                          self.alt_shares_v2_client.delete_share,
-                          self.share['id'])
-
-    @decorators.idempotent_id('1f9e5d84-0885-4a4b-9196-9031a1c01508')
-    @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
-    def test_set_metadata_of_other_tenants_public_share(self):
-        self.assertRaises(lib_exc.Forbidden,
-                          self.alt_shares_v2_client.set_metadata,
-                          self.share['id'],
-                          {'key': 'value'})
-
-    @decorators.idempotent_id('fed7a935-9699-43a1-854e-67b61ba6233e')
-    @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
-    def test_update_metadata_of_other_tenants_public_share(self):
-        self.assertRaises(lib_exc.Forbidden,
-                          self.alt_shares_v2_client.update_all_metadata,
-                          self.share['id'],
-                          {'key': 'value'})
-
-    @decorators.idempotent_id('bd62adeb-73c2-4b04-8812-80b479cd5c3b')
-    @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
-    def test_delete_metadata_of_other_tenants_public_share(self):
-        self.assertRaises(lib_exc.Forbidden,
-                          self.alt_shares_v2_client.delete_metadata,
-                          self.share['id'],
-                          'key')
-
 
 class SharesAPIOnlyNegativeTest(base.BaseSharesMixedTest):