Merge "Fix typos for custom requirements config options descriptions"
diff --git a/HACKING.rst b/HACKING.rst
index 056c86a..f020063 100644
--- a/HACKING.rst
+++ b/HACKING.rst
@@ -53,3 +53,67 @@
Patrole does not have a hacking check for role overriding, but one may be
added in the future.
+
+Branchless Patrole Considerations
+---------------------------------
+
+Like Tempest, Patrole is branchless. This is to better ensure API and RBAC
+consistency between releases because API and RBAC behavior should not change
+between releases. This means that the stable branches are also gated by the
+Patrole master branch, which also means that proposed commits to Patrole must
+work against both the master and all the currently supported stable branches
+of the projects. As such there are a few special considerations that have to
+be accounted for when pushing new changes to Patrole.
+
+1. New Tests for new features
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Patrole, like Tempest, *implicitly* tests new features because new policies
+oftentimes accompany new features. The same `Tempest philosophy`_ regarding
+feature flags and new features also applies to Patrole.
+
+.. _Tempest philosophy: https://docs.openstack.org/tempest/latest/HACKING.html#new-tests-for-new-features
+
+2. New Tests for new policies
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+When adding tests for new policies that were not in previous releases of the
+projects, the new test must be properly skipped with a feature flag. This
+involves using the ``testtools.skip(Unless|If)`` decorator above the test
+to check if the required policy is enabled. Similarly, a feature flag must
+be used whenever an OpenStack service covered by Patrole changes one of its
+policies in a backwards-incompatible way. If there isn't a method of selecting
+the new policy from the config file then there won't be a mechanism to disable
+the test with older stable releases and the new test won't be able to merge.
+
+Introduction of a new feature flag requires specifying a default value for the
+corresponding config option that is appropriate in the latest OpenStack
+release. Because Patrole is branchless, the feature flag's default value will
+need to be overridden to a value that is appropriate in earlier releases in
+which the feature isn't available. In DevStack, this can be accomplished by
+modifying Patrole's lib installation script for previous branches (because
+DevStack is branched).
+
+3. Bug fix on core project needing Patrole changes
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+When trying to land a bug fix which changes a tested API you'll have to use the
+following procedure:
+
+ #. Propose change to the project, get a +2 on the change even with the
+ test failing Patrole side.
+ #. Propose skip to the relevant Patrole test which will only be approved
+ after the corresponding change in the project has a +2.
+ #. Land project change in master and all open stable branches
+ (if required).
+ #. Land changed test in Patrole.
+
+Otherwise the bug fix won't be able to land in the project.
+
+4. New Tests for existing features or policies
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The same `Tempest logic`_ regarding new tests for existing features or
+policies also applies to Patrole.
+
+.. _Tempest logic: https://docs.openstack.org/tempest/latest/HACKING.html#new-tests-for-existing-features
diff --git a/README.rst b/README.rst
index f4ab65c..0c786b9 100644
--- a/README.rst
+++ b/README.rst
@@ -9,9 +9,9 @@
=========================================
Patrole is a security validation tool for verifying that Role-Based Access
-Control is correctly configured and enforced in a system. It runs Tempest-based
-API tests using specified RBAC roles, thus allowing deployments to verify that
-only intended roles have access to those APIs.
+Control is correctly configured and enforced in a system. It runs
+`Tempest`_-based API tests using specified RBAC roles, thus allowing
+deployments to verify that only intended roles have access to those APIs.
Patrole currently offers testing for the following OpenStack services: Nova,
Neutron, Glance, Cinder and Keystone.
@@ -20,6 +20,8 @@
toward policy in code, Patrole will align its testing with the appropriate
documentation.
+.. _Tempest: https://docs.openstack.org/tempest/latest/
+
Design Principles
-----------------
diff --git a/doc/source/configuration.rst b/doc/source/configuration.rst
index ce799ad..f6aaf04 100644
--- a/doc/source/configuration.rst
+++ b/doc/source/configuration.rst
@@ -21,7 +21,7 @@
``self.os_admin.servers_client``.
Similarly, setting ``rbac_test_role`` to a non-admin role results in Tempest's
-primary credentials being overriden by the role specified by
+primary credentials being overridden by the role specified by
``rbac_test_role``.
.. note::
@@ -55,3 +55,46 @@
Patrole currently does not support policy files located on a host different
than the one on which it is running.
+
+Policy Feature Flags
+--------------------
+
+Patrole's ``[policy-feature-enabled]`` configuration group includes one option
+per supported policy feature flag. These feature flags are introduced when an
+OpenStack service introduces a new policy or changes a policy in a
+backwards-incompatible way. Since Patrole is branchless, it copes with the
+unexpected policy change by making the relevant policy change as well, but
+also introduces a new policy feature flag so that the test won't break N-1/N-2
+releases where N is the currently supported release.
+
+The default value for the feature flag is enabled for N and disabled for any
+releases prior to N in which the feature is not available. This is done by
+overriding the default value of the feature flag in DevStack's ``lib/patrole``
+installation script. The change is made in Tempest's DevStack script because
+Patrole's DevStack plugin is hosted in-repo, which is branch-less (whereas
+the former is branched).
+
+After the backwards-incompatible change no longer affects any supported
+release, then the corresponding policy feature flag is removed.
+
+For more information on feature flags, reference the relevant
+`Tempest documentation`_.
+
+.. _Tempest documentation: https://docs.openstack.org/tempest/latest/HACKING.html#1-new-tests-for-new-features
+
+Sample Configuration File
+-------------------------
+
+The following is a sample Patrole configuration for adaptation and use. It is
+auto-generated from Patrole when this documentation is built, so
+if you are having issues with an option, please compare your version of
+Patrole with the version of this documentation.
+
+Note that the Patrole configuration options actually live inside the Tempest
+configuration file; at runtime, Tempest populates its own configuration
+file with Patrole groups and options, assuming that Patrole is correctly
+installed and recognized as a plugin.
+
+The sample configuration can also be viewed in `file form <_static/patrole.conf.sample>`_.
+
+.. literalinclude:: _static/patrole.conf.sample
diff --git a/doc/source/testing.rst b/doc/source/field_guide/index.rst
similarity index 82%
rename from doc/source/testing.rst
rename to doc/source/field_guide/index.rst
index d61c78d..ba06c42 100644
--- a/doc/source/testing.rst
+++ b/doc/source/field_guide/index.rst
@@ -1,8 +1,8 @@
-.. _patrole-testing:
+.. _patrole-field-guide:
-===============
-Patrole Testing
-===============
+============================
+Patrole Field Guide Overview
+============================
Testing Scope
=============
@@ -14,6 +14,15 @@
In other words, all tests in Patrole are RBAC tests.
+:ref:`rbac_field_guide`
+=======================
+
+RBAC tests are `Tempest`_-like API tests plus Patrole's
+:ref:`rbac-validation`. All Patrole tests are RBAC validation tests for the
+OpenStack API.
+
+.. _Tempest: https://docs.openstack.org/tempest/latest/
+
Stable Tests
============
diff --git a/doc/source/field_guide/rbac.rst b/doc/source/field_guide/rbac.rst
new file mode 100644
index 0000000..2654d31
--- /dev/null
+++ b/doc/source/field_guide/rbac.rst
@@ -0,0 +1,71 @@
+.. _rbac_field_guide:
+
+Patrole Field Guide to RBAC Tests
+=================================
+
+
+What are these tests?
+---------------------
+
+Patrole's primary responsibility is to ensure that your OpenStack cloud
+has properly configured Role-Based Access Control (RBAC). All Patrole
+tests cases are devoted to this responsibility. Tempest API clients
+and utility functions are leveraged to accomplish this goal, but such
+functionality is secondary to RBAC validation.
+
+Like Tempest, Patrole not only tests expected positive paths for RBAC
+validation, but also -- and more importantly -- negative paths. While
+Patrole could be thought of as validating RBAC, it more importantly
+verifies that your OpenStack cloud is secure from the perspective of
+RBAC (there are many gotchas when it comes to security, not just RBAC).
+
+Negative paths are arguably more important than positive paths when it
+comes to RBAC and by extension security, because it is essential that
+your cloud be secure from unauthorized access. For example, while it is
+important to verify that the admin role has access to admin-level
+functionality, it is of critical importance to verify that non-admin roles
+*do not* have access to such functionality.
+
+Unlike Tempest, Patrole accomplishes negative testing implicitly -- by
+abstracting it away in the background. Patrole dynamically determines
+whether a role should have access to an API depending on your cloud's
+policy configuration and then confirms whether that is true or false.
+
+
+Why are these tests in Patrole?
+-------------------------------
+
+These tests constitute the core mission in Patrole: to verify RBAC. These
+tests are mainly intended to validate RBAC, but can also *unofficially*
+be used to discover the policy-to-API mapping for an OpenStack component.
+
+It could be argued that some of these tests could be implemented in
+the projects themselves, but that approach has the following shortcomings:
+
+* The projects do not validate RBAC from an integration testing perspective.
+* By extension, RBAC across cross-service communication is not usually
+ validated.
+* The projects' tests do not pass all the metadata to ``oslo.policy`` that is
+ in reality passed by the deployed server to that library to determine
+ whether a given user is authorized to perform an API action.
+* The projects do not exhaustively do RBAC testing for all positive and
+ negative paths.
+* Patrole is designed to work with any role via configuration settings, but
+ on the other hand the projects handpick which roles to test.
+
+
+Scope of these tests
+--------------------
+
+RBAC tests should always use the Tempest implementation of the
+OpenStack API, to take advantage of Tempest's stable library.
+
+Each test should test a specific API endpoint and the related policy.
+
+Each policy should be tested in isolation of one another -- or at least
+as close to this rule as possible -- to ensure proper validation of RBAC.
+
+Each test should be able to work for positive and negative paths.
+
+All tests should be able to be run on their own, not depending on the
+state created by a previous test.
diff --git a/doc/source/index.rst b/doc/source/index.rst
index be3264e..d964845 100644
--- a/doc/source/index.rst
+++ b/doc/source/index.rst
@@ -11,11 +11,25 @@
.. toctree::
:maxdepth: 2
- installation
configuration
usage
- testing
- sampleconf
+
+Patrole Installation Guide
+--------------------------
+
+.. toctree::
+ :maxdepth: 2
+
+ installation
+
+Field Guides
+============
+
+.. toctree::
+ :maxdepth: 1
+
+ field_guide/index
+ field_guide/rbac
Developer's Guide
=================
diff --git a/doc/source/sampleconf.rst b/doc/source/sampleconf.rst
deleted file mode 100644
index ee848b5..0000000
--- a/doc/source/sampleconf.rst
+++ /dev/null
@@ -1,18 +0,0 @@
-.. _patrole-sampleconf:
-
-Sample Configuration File
-==========================
-
-The following is a sample Patrole configuration for adaptation and use. It is
-auto-generated from Patrole when this documentation is built, so
-if you are having issues with an option, please compare your version of
-Patrole with the version of this documentation.
-
-Note that the Patrole configuration options actually live inside the Tempest
-configuration file; at runtime, Tempest populates its own configuration
-file with Patrole groups and options, assuming that Patrole is correctly
-installed and recognized as a plugin.
-
-The sample configuration can also be viewed in `file form <_static/patrole.conf.sample>`_.
-
-.. literalinclude:: _static/patrole.conf.sample
diff --git a/patrole_tempest_plugin/policy_authority.py b/patrole_tempest_plugin/policy_authority.py
index 99348b9..6794afe 100644
--- a/patrole_tempest_plugin/policy_authority.py
+++ b/patrole_tempest_plugin/policy_authority.py
@@ -274,9 +274,9 @@
access_data['is_admin'] = is_admin
# TODO(felipemonteiro): Dynamically calculate is_admin_project rather
# than hard-coding it to True. is_admin_project cannot be determined
- # from the role, but rather from project and domain names. See
- # _populate_is_admin_project in keystone.token.providers.common
- # for more information.
+ # from the role, but rather from project and domain names. For more
+ # information, see:
+ # https://github.com/openstack/keystone/blob/37ce5417418f8acbd27f3dacb70c605b0fe48301/keystone/token/providers/common.py#L150
access_data['is_admin_project'] = True
class Object(object):
diff --git a/patrole_tempest_plugin/rbac_utils.py b/patrole_tempest_plugin/rbac_utils.py
index b3cb271..347f77f 100644
--- a/patrole_tempest_plugin/rbac_utils.py
+++ b/patrole_tempest_plugin/rbac_utils.py
@@ -88,7 +88,7 @@
def test_foo(self):
# Allocate test-level resources here.
with self.rbac_utils.override_role(self):
- # The role for `os_primary` has now been overriden. Within
+ # The role for `os_primary` has now been overridden. Within
# this block, call the API endpoint that enforces the
# expected policy specified by "rule" in the decorator.
self.foo_service.bar_api_call()
diff --git a/patrole_tempest_plugin/tests/api/compute/test_server_misc_policy_actions_rbac.py b/patrole_tempest_plugin/tests/api/compute/test_server_misc_policy_actions_rbac.py
index d393a4c..63dee85 100644
--- a/patrole_tempest_plugin/tests/api/compute/test_server_misc_policy_actions_rbac.py
+++ b/patrole_tempest_plugin/tests/api/compute/test_server_misc_policy_actions_rbac.py
@@ -614,6 +614,9 @@
self.interfaces_client, self.server['id'],
interface['port_id'], 'ACTIVE')
self.addCleanup(
+ waiters.wait_for_interface_detach, self.interfaces_client,
+ self.server['id'], interface['port_id'])
+ self.addCleanup(
test_utils.call_and_ignore_notfound_exc,
self.interfaces_client.delete_interface,
self.server['id'], interface['port_id'])
diff --git a/patrole_tempest_plugin/tests/api/volume/test_volume_basic_crud_rbac.py b/patrole_tempest_plugin/tests/api/volume/test_volume_basic_crud_rbac.py
index 1bd87d2..61532c6 100644
--- a/patrole_tempest_plugin/tests/api/volume/test_volume_basic_crud_rbac.py
+++ b/patrole_tempest_plugin/tests/api/volume/test_volume_basic_crud_rbac.py
@@ -62,11 +62,3 @@
with self.rbac_utils.override_role(self):
self.volumes_client.update_volume(self.volume['id'],
name=update_name)
-
- @rbac_rule_validation.action(
- service="cinder",
- rule="volume_extension:volume_image_metadata")
- @decorators.idempotent_id('3d48ca91-f02b-4616-a69d-4a8b296c8529')
- def test_volume_list_image_metadata(self):
- with self.rbac_utils.override_role(self):
- self.volumes_client.list_volumes(detail=True)