Fix volume image metadata endpoints returning None

This commit fixes the following volume image metadata
endpoints returning None following policy enforcement
failure:

  * ``os-set_image_metadata``
  * ``os-unset_image_metadata``

The endpoints will now correctly raise a 403 Forbidden
instead.

The kwarg `fatal=False` was dropped from
`context.authorize` for these APIs because the kwarg
is only useful when adding additional information to
the response body (if the user is authorized).

This commit:

  * makes the fix for the two endpoints above
  * adds unit tests for validating the new, correct
    behavior (as a side note, policy overriding
    in tests can be more easily accomplished via
    adoption of something like [0])

Also note that since the default policy rule
for these endpoints is "admin_or_owner" Tempest
doesn't validate this behavior by default.

[0] e599b13e49/nova/tests/unit/policy_fixture.py (L30)

Change-Id: Icc286d529609165e5f14cb506342660d7bc2ae9f
Closes-Bug: #1737609

cinder/api/contrib/volume_image_metadata.py

@@ -85,7 +85,7 @@ class VolumeImageMetadataController(wsgi.Controller):
     @wsgi.action("os-set_image_metadata")
     def create(self, req, id, body):
         context = req.environ['cinder.context']
-        if context.authorize(policy.IMAGE_METADATA_POLICY, fatal=False):
+        if context.authorize(policy.IMAGE_METADATA_POLICY):
             try:
                 metadata = body['os-set_image_metadata']['metadata']
             except (KeyError, TypeError):
@@ -128,7 +128,7 @@ class VolumeImageMetadataController(wsgi.Controller):
     def delete(self, req, id, body):
         """Deletes an existing image metadata."""
         context = req.environ['cinder.context']
-        if context.authorize(policy.IMAGE_METADATA_POLICY, fatal=False):
+        if context.authorize(policy.IMAGE_METADATA_POLICY):
             try:
                 key = body['os-unset_image_metadata']['key']
             except (KeyError, TypeError):

cinder/tests/unit/api/contrib/test_volume_image_metadata.py

@@ -14,6 +14,7 @@
 
 import uuid
 
+from oslo_policy import policy as oslo_policy
 from oslo_serialization import jsonutils
 from oslo_utils import timeutils
 from six.moves import http_client
@@ -25,6 +26,9 @@ from cinder import db
 from cinder import exception
 from cinder import objects
 from cinder.objects import fields
+from cinder.policies import base as base_policy
+from cinder.policies import volume_metadata as metadata_policy
+from cinder import policy
 from cinder import test
 from cinder.tests.unit.api import fakes
 from cinder.tests.unit import fake_constants as fake
@@ -216,6 +220,26 @@ class VolumeImageMetadataTest(test.TestCase):
         self.assertEqual(fake_image_metadata,
                          jsonutils.loads(res.body)["metadata"])
 
+    def test_create_image_metadata_policy_not_authorized(self):
+        rules = {
+            metadata_policy.IMAGE_METADATA_POLICY: base_policy.RULE_ADMIN_API
+        }
+        policy.set_rules(oslo_policy.Rules.from_dict(rules))
+        self.addCleanup(policy.reset)
+
+        req = fakes.HTTPRequest.blank('/v2/%s/volumes/%s/action' % (
+            fake.PROJECT_ID, fake.VOLUME_ID), use_admin_context=False)
+
+        req.method = 'POST'
+        req.content_type = "application/json"
+        body = {"os-set_image_metadata": {
+            "metadata": {"image_name": "fake"}}
+        }
+        req.body = jsonutils.dump_as_bytes(body)
+
+        self.assertRaises(exception.PolicyNotAuthorized,
+                          self.controller.create, req, fake.VOLUME_ID, None)
+
     def test_create_with_keys_case_insensitive(self):
         # If the keys in uppercase_and_lowercase, should return the one
         # which server added
@@ -320,6 +344,26 @@ class VolumeImageMetadataTest(test.TestCase):
             fake_auth_context=self.user_ctxt))
         self.assertEqual(http_client.OK, res.status_int)
 
+    def test_delete_image_metadata_policy_not_authorized(self):
+        rules = {
+            metadata_policy.IMAGE_METADATA_POLICY: base_policy.RULE_ADMIN_API
+        }
+        policy.set_rules(oslo_policy.Rules.from_dict(rules))
+        self.addCleanup(policy.reset)
+
+        req = fakes.HTTPRequest.blank('/v2/%s/volumes/%s/action' % (
+            fake.PROJECT_ID, fake.VOLUME_ID), use_admin_context=False)
+
+        req.method = 'POST'
+        req.content_type = "application/json"
+        body = {"os-unset_image_metadata": {
+            "metadata": {"image_name": "fake"}}
+        }
+        req.body = jsonutils.dump_as_bytes(body)
+
+        self.assertRaises(exception.PolicyNotAuthorized,
+                          self.controller.delete, req, fake.VOLUME_ID, None)
+
     def test_delete_meta_not_found(self):
         data = {"os-unset_image_metadata": {
             "key": "invalid_id"}

releasenotes/notes/fix-vol-image-metadata-endpoints-returning-none-ba0590e6c6757b0c.yaml

@@ -0,0 +1,10 @@
+---
+fixes:
+  - |
+    Fix the following volume image metadata endpoints returning None following
+    policy enforcement failure:
+
+    * ``os-set_image_metadata``
+    * ``os-unset_image_metadata``
+
+    The endpoints will now correctly raise a 403 Forbidden instead.