Merge "Adds meaningful exceptions for missing attributes"
diff --git a/.testr.conf b/.testr.conf
index 6d83b3c..87d049d 100644
--- a/.testr.conf
+++ b/.testr.conf
@@ -2,6 +2,6 @@
 test_command=OS_STDOUT_CAPTURE=${OS_STDOUT_CAPTURE:-1} \
              OS_STDERR_CAPTURE=${OS_STDERR_CAPTURE:-1} \
              OS_TEST_TIMEOUT=${OS_TEST_TIMEOUT:-60} \
-             ${PYTHON:-python} -m subunit.run discover -t ./ . $LISTOPT $IDOPTION
+             ${PYTHON:-python} -m subunit.run discover -t ./ ${OS_TEST_PATH:-./patrole_tempest_plugin/tests/unit} $LISTOPT $IDOPTION
 test_id_option=--load-list $IDFILE
 test_list_option=--list
diff --git a/HACKING.rst b/HACKING.rst
index 5281b53..f89910b 100644
--- a/HACKING.rst
+++ b/HACKING.rst
@@ -1,15 +1,15 @@
 Patrole Style Commandments
 ==========================
 
-- Step 1: Read the OpenStack Style Commandments: `<https://docs.openstack.org/developer/hacking/>`__
-- Step 2: Review Tempest's Style Commandments: `<https://docs.openstack.org/developer/tempest/HACKING.html>`__
+- Step 1: Read the OpenStack Style Commandments: `<https://docs.openstack.org/hacking/latest/>`__
+- Step 2: Review Tempest's Style Commandments: `<https://docs.openstack.org/tempest/latest/HACKING.html>`__
 - Step 3: Read on
 
 Patrole Specific Commandments
 ------------------------------
 
 Patrole borrows the following commandments from Tempest; refer to
-`Tempest's Commandments <https://docs.openstack.org/developer/tempest/HACKING.html>`__
+`Tempest's Commandments <https://docs.openstack.org/tempest/latest/HACKING.html>`__
 for more information:
 
 .. note::
diff --git a/doc/source/sampleconf.rst b/doc/source/sampleconf.rst
index c262f2d..43933db 100644
--- a/doc/source/sampleconf.rst
+++ b/doc/source/sampleconf.rst
@@ -36,7 +36,7 @@
     # are.
     test_custom_requirements = False
 
-    File path of the yaml file that defines your RBAC requirements. This
+    # File path of the yaml file that defines your RBAC requirements. This
     # file must be located on the same host that Patrole runs on. The yaml
     # file should be written as follows:
     custom_requirements_file = patrole/requirements.txt
diff --git a/patrole_tempest_plugin/rbac_policy_parser.py b/patrole_tempest_plugin/rbac_policy_parser.py
index 254bb18..88e9faa 100644
--- a/patrole_tempest_plugin/rbac_policy_parser.py
+++ b/patrole_tempest_plugin/rbac_policy_parser.py
@@ -20,7 +20,7 @@
 from oslo_log import log as logging
 from oslo_policy import policy
 import stevedore
-
+from tempest import clients
 from tempest.common import credentials_factory as credentials
 from tempest import config
 
@@ -93,7 +93,8 @@
         # Cache the list of available services in memory to avoid needlessly
         # doing an API call every time.
         if not hasattr(cls, 'available_services'):
-            admin_mgr = credentials.AdminManager()
+            admin_mgr = clients.Manager(
+                credentials.get_configured_admin_credentials())
             services_client = (admin_mgr.identity_services_v3_client
                                if CONF.identity_feature_enabled.api_v3
                                else admin_mgr.identity_services_client)
diff --git a/patrole_tempest_plugin/rbac_rule_validation.py b/patrole_tempest_plugin/rbac_rule_validation.py
index 75c4c11..c7bd38b 100644
--- a/patrole_tempest_plugin/rbac_rule_validation.py
+++ b/patrole_tempest_plugin/rbac_rule_validation.py
@@ -25,6 +25,7 @@
 
 from patrole_tempest_plugin import rbac_exceptions
 from patrole_tempest_plugin import rbac_policy_parser
+from patrole_tempest_plugin import rbac_utils
 from patrole_tempest_plugin import requirements_authority
 
 CONF = config.CONF
@@ -85,7 +86,7 @@
                 LOG.info("As admin_only is True, only admin role should be "
                          "allowed to perform the API. Skipping oslo.policy "
                          "check for policy action {0}.".format(rule))
-                allowed = test_obj.rbac_utils.is_admin
+                allowed = rbac_utils.is_admin()
             else:
                 allowed = _is_authorized(test_obj, service, rule,
                                          extra_target_data)
diff --git a/patrole_tempest_plugin/rbac_utils.py b/patrole_tempest_plugin/rbac_utils.py
index 9d7a807..29d41d3 100644
--- a/patrole_tempest_plugin/rbac_utils.py
+++ b/patrole_tempest_plugin/rbac_utils.py
@@ -195,13 +195,13 @@
         self.admin_role_id = admin_role_id
         self.rbac_role_id = rbac_role_id
 
-    @property
-    def is_admin(self):
-        """Verifies whether the current test role equals the admin role.
 
-        :returns: True if ``rbac_test_role`` is the admin role.
-        """
-        return CONF.rbac.rbac_test_role == CONF.identity.admin_role
+def is_admin():
+    """Verifies whether the current test role equals the admin role.
+
+    :returns: True if ``rbac_test_role`` is the admin role.
+    """
+    return CONF.rbac.rbac_test_role == CONF.identity.admin_role
 
 
 @six.add_metaclass(abc.ABCMeta)
diff --git a/patrole_tempest_plugin/tests/api/compute/test_agents_rbac.py b/patrole_tempest_plugin/tests/api/compute/test_agents_rbac.py
index f355358..4712ed0 100644
--- a/patrole_tempest_plugin/tests/api/compute/test_agents_rbac.py
+++ b/patrole_tempest_plugin/tests/api/compute/test_agents_rbac.py
@@ -13,6 +13,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from tempest.lib.common.utils import data_utils
+from tempest.lib.common.utils import test_utils
 from tempest.lib import decorators
 from tempest import test
 
@@ -29,6 +31,16 @@
             raise cls.skipException(
                 '%s skipped as os-agents not enabled' % cls.__name__)
 
+    def _param_helper(self, **kwargs):
+        rand_key = 'architecture'
+        if rand_key in kwargs:
+            # NOTE: The rand_name is for avoiding agent conflicts.
+            # If you try to create an agent with the same hypervisor,
+            # os and architecture as an existing agent, Nova will return
+            # an HTTPConflict or HTTPServerError.
+            kwargs[rand_key] = data_utils.rand_name(kwargs[rand_key])
+        return kwargs
+
     @rbac_rule_validation.action(
         service="nova", rule="os_compute_api:os-agents")
     @decorators.idempotent_id('d1bc6d97-07f5-4f45-ac29-1c619a6a7e27')
@@ -48,3 +60,41 @@
         body = self.agents_client.create_agent(**params)['agent']
         self.addCleanup(self.agents_client.delete_agent,
                         body['agent_id'])
+
+    @rbac_rule_validation.action(
+        service="nova",
+        rule="os_compute_api:os-agents")
+    @decorators.idempotent_id('b22f2681-9ffb-439b-b240-dae503e41020')
+    def test_update_agent(self):
+        params = self._param_helper(
+            hypervisor='common', os='linux',
+            architecture='x86_64', version='7.0',
+            url='xxx://xxxx/xxx/xxx',
+            md5hash='add6bb58e139be103324d04d82d8f545')
+        body = self.agents_client.create_agent(**params)['agent']
+        self.addCleanup(self.agents_client.delete_agent,
+                        body['agent_id'])
+
+        self.rbac_utils.switch_role(self, toggle_rbac_role=True)
+        update_params = self._param_helper(
+            version='8.0',
+            url='xxx://xxxx/xxx/xxx2',
+            md5hash='add6bb58e139be103324d04d82d8f547')
+        self.agents_client.update_agent(body['agent_id'], **update_params)
+
+    @rbac_rule_validation.action(
+        service="nova",
+        rule="os_compute_api:os-agents")
+    @decorators.idempotent_id('c5042af8-0682-43b0-abc4-bf33349e23dd')
+    def test_delete_agent(self):
+        params = self._param_helper(
+            hypervisor='common', os='linux',
+            architecture='x86_64', version='7.0',
+            url='xxx://xxxx/xxx/xxx',
+            md5hash='add6bb58e139be103324d04d82d8f545')
+        body = self.agents_client.create_agent(**params)['agent']
+        self.addCleanup(test_utils.call_and_ignore_notfound_exc,
+                        self.agents_client.delete_agent,
+                        body['agent_id'])
+        self.rbac_utils.switch_role(self, toggle_rbac_role=True)
+        self.agents_client.delete_agent(body['agent_id'])
diff --git a/patrole_tempest_plugin/tests/api/identity/v2/test_projects_rbac.py b/patrole_tempest_plugin/tests/api/identity/v2/test_projects_rbac.py
index f7fced1..db1f6e6 100644
--- a/patrole_tempest_plugin/tests/api/identity/v2/test_projects_rbac.py
+++ b/patrole_tempest_plugin/tests/api/identity/v2/test_projects_rbac.py
@@ -18,6 +18,7 @@
 
 from patrole_tempest_plugin import rbac_exceptions
 from patrole_tempest_plugin import rbac_rule_validation
+from patrole_tempest_plugin import rbac_utils
 from patrole_tempest_plugin.tests.api.identity import rbac_base
 
 CONF = config.CONF
@@ -112,7 +113,7 @@
         admin-scoped tenants, raise ``RbacActionFailed`` exception otherwise.
         """
         tenants_client = self.os_admin.tenants_client if \
-            self.rbac_utils.is_admin else self.os_primary.tenants_client
+            rbac_utils.is_admin() else self.os_primary.tenants_client
         admin_tenant_id = self.os_admin.credentials.project_id
         non_admin_tenant_id = self.os_primary.credentials.project_id
 
diff --git a/patrole_tempest_plugin/tests/api/identity/v3/test_tokens_negative_rbac.py b/patrole_tempest_plugin/tests/api/identity/v3/test_tokens_negative_rbac.py
index 90952a8..18e5bf1 100644
--- a/patrole_tempest_plugin/tests/api/identity/v3/test_tokens_negative_rbac.py
+++ b/patrole_tempest_plugin/tests/api/identity/v3/test_tokens_negative_rbac.py
@@ -18,6 +18,7 @@
 from tempest.lib import exceptions as lib_exc
 
 from patrole_tempest_plugin import rbac_rule_validation
+from patrole_tempest_plugin import rbac_utils
 from patrole_tempest_plugin.tests.api.identity import rbac_base
 
 CONF = config.CONF
@@ -31,8 +32,8 @@
     def skip_checks(cls):
         super(IdentityTokenV3RbacTest, cls).skip_checks()
         # In case of admin, the positive testcase would be used, hence
-        # skipping negative testcase
-        if CONF.rbac.rbac_test_role == CONF.identity.admin_role:
+        # skipping negative testcase.
+        if rbac_utils.is_admin():
             raise cls.skipException(
                 "Skipped as admin role doesn't require negative testing")
 
diff --git a/patrole_tempest_plugin/tests/unit/test_rbac_policy_parser.py b/patrole_tempest_plugin/tests/unit/test_rbac_policy_parser.py
index 7ce925a..36fa045 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_policy_parser.py
+++ b/patrole_tempest_plugin/tests/unit/test_rbac_policy_parser.py
@@ -40,11 +40,11 @@
 
     def setUp(self):
         super(RbacPolicyTest, self).setUp()
-
-        m_creds = self.patchobject(rbac_policy_parser, 'credentials')
-        m_creds.AdminManager().identity_services_client.list_services.\
+        self.patchobject(rbac_policy_parser, 'credentials')
+        m_creds = self.patchobject(rbac_policy_parser, 'clients')
+        m_creds.Manager().identity_services_client.list_services.\
             return_value = self.services
-        m_creds.AdminManager().identity_services_v3_client.list_services.\
+        m_creds.Manager().identity_services_v3_client.list_services.\
             return_value = self.services
 
         current_directory = os.path.dirname(os.path.realpath(__file__))
@@ -458,7 +458,7 @@
     @mock.patch.object(rbac_policy_parser, 'policy', autospec=True)
     @mock.patch.object(rbac_policy_parser.RbacPolicyParser, '_get_policy_data',
                        autospec=True)
-    @mock.patch.object(rbac_policy_parser, 'credentials', autospec=True)
+    @mock.patch.object(rbac_policy_parser, 'clients', autospec=True)
     @mock.patch.object(rbac_policy_parser, 'os', autospec=True)
     def test_discover_policy_files_with_many_invalid_one_valid(self, m_os,
                                                                m_creds, *args):
@@ -466,7 +466,7 @@
         m_os.path.isfile.side_effect = [False, False, True, False]
 
         # Ensure the outer for loop runs only once in `discover_policy_files`.
-        m_creds.AdminManager().identity_services_v3_client.\
+        m_creds.Manager().identity_services_v3_client.\
             list_services.return_value = {
                 'services': [{'name': 'test_service'}]}
 
@@ -504,10 +504,10 @@
 
     def _test_validate_service(self, v2_services, v3_services,
                                expected_failure=False, expected_services=None):
-        with mock.patch.object(rbac_policy_parser, 'credentials') as m_creds:
-            m_creds.AdminManager().identity_services_client.list_services.\
+        with mock.patch.object(rbac_policy_parser, 'clients') as m_creds:
+            m_creds.Manager().identity_services_client.list_services.\
                 return_value = v2_services
-            m_creds.AdminManager().identity_services_v3_client.list_services.\
+            m_creds.Manager().identity_services_v3_client.list_services.\
                 return_value = v3_services
 
         test_tenant_id = mock.sentinel.tenant_id
diff --git a/releasenotes/notes/agents-ca4a5e232ce242a5.yaml b/releasenotes/notes/agents-ca4a5e232ce242a5.yaml
new file mode 100644
index 0000000..3cd9646
--- /dev/null
+++ b/releasenotes/notes/agents-ca4a5e232ce242a5.yaml
@@ -0,0 +1,5 @@
+---
+features:
+  - |
+    Added tests to test_agents_rbac.py for PUT and
+    DELETE endpoints.
diff --git a/test-whitelist.txt b/test-whitelist.txt
deleted file mode 100644
index 162992a..0000000
--- a/test-whitelist.txt
+++ /dev/null
@@ -1 +0,0 @@
-patrole_tempest_plugin.tests.unit.test*
diff --git a/tox.ini b/tox.ini
index d2e83e9..41e893c 100644
--- a/tox.ini
+++ b/tox.ini
@@ -8,6 +8,9 @@
 install_command = pip install -c{env:UPPER_CONSTRAINTS_FILE:https://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints.txt} {opts} {packages}
 setenv =
    VIRTUAL_ENV={envdir}
+   OS_TEST_PATH=./patrole_tempest_plugin/tests/unit
+   LANGUAGE=en_US
+   LC_ALL=en_US.utf-8
    PYTHONWARNINGS=default::DeprecationWarning
 passenv = OS_STDOUT_CAPTURE OS_STDERR_CAPTURE OS_TEST_TIMEOUT OS_TEST_LOCK_PATH OS_TEST_PATH http_proxy HTTP_PROXY https_proxy HTTPS_PROXY no_proxy NO_PROXY
 whitelist_externals = find
@@ -15,7 +18,7 @@
        -r{toxinidir}/test-requirements.txt
 commands =
     find . -type f -name "*.pyc" -delete
-    ostestr {posargs} --whitelist-file test-whitelist.txt
+    ostestr {posargs}
 
 [testenv:pep8]
 commands = flake8 {posargs}