Merge "Fix glance metadata properties filtering"

cinder/image/image_utils.py

@@ -1290,6 +1290,16 @@ class TemporaryImages(object):
         return self.temporary_images[user].get(image_id)
 
 
+def _filter_out_metadata(metadata, filter_keys):
+    new_metadata = {}
+    for k, v in metadata.items():
+        if any(k.startswith(filter_key)
+               for filter_key in filter_keys):
+            continue
+        new_metadata[k] = v
+    return new_metadata
+
+
 def filter_out_reserved_namespaces_metadata(
         metadata: Optional[dict[str, str]]) -> dict[str, str]:
 
@@ -1303,12 +1313,12 @@ def filter_out_reserved_namespaces_metadata(
         LOG.debug("No metadata to be filtered.")
         return {}
 
-    new_metadata = {}
-    for k, v in metadata.items():
-        if any(k.startswith(reserved_name_space)
-               for reserved_name_space in reserved_name_spaces):
-            continue
-        new_metadata[k] = v
+    new_metadata = _filter_out_metadata(metadata, reserved_name_spaces)
+    # NOTE(ganso): handle adjustment of metadata structure performed by
+    # the cinder.volume.api.API._merge_volume_image_meta() method
+    if 'properties' in new_metadata:
+        new_metadata['properties'] = _filter_out_metadata(
+            metadata['properties'], reserved_name_spaces)
 
     LOG.debug("The metadata set [%s] was filtered using the reserved name "
               "spaces [%s], and the result is [%s].", metadata,

cinder/tests/unit/test_image_utils.py

@@ -2521,3 +2521,69 @@ class TestFilterReservedNamespaces(test.TestCase):
                       "name spaces [%s], and the result is [%s].",
                       metadata_for_test, keys_to_pop, expected_result)
         ])
+
+    @ddt.data(  # remove default keys
+        ({"some_key": 13, "other_key": "test",
+          "os_glance_key": "this should be removed",
+          "os_glance_key2": "this should also be removed",
+          "properties": {"os_glance_key3": "this should be removed",
+                         "os_glance_key4": "this should also be removed",
+                         "another_key": "foobar"}
+          },
+         None,
+         []),
+        # remove nothing
+        ({"some_key": 13, "other_key": "test",
+          "properties": {"another_key": "foobar"}},
+         None,
+         []),
+        # custom config empty
+        ({"some_key": 13, "other_key": "test",
+          "os_glance_key": "this should be removed",
+          "os_glance_key2": "this should also be removed",
+          "properties": {"os_glance_key3": "this should be removed",
+                         "os_glance_key4": "this should also be removed",
+                         "another_key": "foobar"}
+          },
+         [],
+         []),
+        # custom config
+        ({"some_key": 13, "other_key": "test",
+          "os_glance_key": "this should be removed",
+          "os_glance_key2": "this should also be removed",
+          "properties": {"os_glance_key3": "this should be removed",
+                         "os_glance_key4": "this should also be removed",
+                         "custom_key": "this should be removed",
+                         "another_custom_key": "this should also be removed",
+                         "another_key": "foobar"},
+          },
+         ['custom_key', 'another_custom_key'],
+         ['custom_key', 'another_custom_key']))
+    @ddt.unpack
+    def test_filter_out_reserved_namespaces_metadata_properties(
+            self, metadata_for_test, config, keys_to_pop):
+        hardcoded_keys = ['os_glance', "img_signature"]
+
+        keys_to_pop = hardcoded_keys + keys_to_pop
+
+        if config:
+            self.override_config('reserved_image_namespaces', config)
+
+        expected_result = {
+            "some_key": 13,
+            "other_key": "test",
+            "properties": {
+                "another_key": "foobar"
+            }
+        }
+
+        method_return = image_utils.filter_out_reserved_namespaces_metadata(
+            metadata_for_test)
+
+        self.assertEqual(expected_result, method_return)
+
+        image_utils.LOG.debug.assert_has_calls([
+            mock.call("The metadata set [%s] was filtered using the reserved "
+                      "name spaces [%s], and the result is [%s].",
+                      metadata_for_test, keys_to_pop, expected_result)
+        ])

releasenotes/notes/bug1945500-e4df056b8be2e0ef.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    `Bug #1945500 <https://bugs.launchpad.net/cinder/+bug/1945500>`_: The
+    original attempt at fixing this bug did not account for differences in
+    how glance and cinder store image metadata, and as a result some image
+    properties were not filtered out. This new improved fix addresses those
+    differences and makes the filtering more thorough.