diff --git a/api-ref/source/v2/metadefs-namespaces-tags.inc b/api-ref/source/v2/metadefs-namespaces-tags.inc index 6d52fad549..176cbb4159 100644 --- a/api-ref/source/v2/metadefs-namespaces-tags.inc +++ b/api-ref/source/v2/metadefs-namespaces-tags.inc @@ -191,6 +191,7 @@ Request .. rest_parameters:: metadefs-parameters.yaml + - X-Openstack-Append: append - namespace_name: namespace_name - tags: tags diff --git a/api-ref/source/v2/metadefs-parameters.yaml b/api-ref/source/v2/metadefs-parameters.yaml index 9a58aa8391..5f6b6fdb9a 100644 --- a/api-ref/source/v2/metadefs-parameters.yaml +++ b/api-ref/source/v2/metadefs-parameters.yaml @@ -1,4 +1,11 @@ # variables in header +append: + description: | + If present and set to True, new metadefs tags are appended to the existing ones. + Otherwise, existing tags are overwritten. + in: header + required: false + type: string Content-Type-json: description: | The media type descriptor for the request body. Use diff --git a/glance/api/v2/metadef_tags.py b/glance/api/v2/metadef_tags.py index 190a7af816..3c684ee029 100644 --- a/glance/api/v2/metadef_tags.py +++ b/glance/api/v2/metadef_tags.py @@ -18,6 +18,7 @@ import http.client as http from oslo_log import log as logging from oslo_serialization import jsonutils from oslo_utils import encodeutils +from oslo_utils import strutils import webob.exc from wsme.rest import json @@ -120,11 +121,14 @@ class TagsController(object): md_resource=namespace_obj, enforcer=self.policy).add_metadef_tags() + can_append = strutils.bool_from_string(req.headers.get( + 'X-Openstack-Append')) + tag_list = [] for metadata_tag in metadata_tags.tags: tag_list.append(tag_factory.new_tag( namespace=namespace, **metadata_tag.to_dict())) - tag_repo.add_tags(tag_list) + tag_repo.add_tags(tag_list, can_append) tag_list_out = [MetadefTag(**{'name': db_metatag.name}) for db_metatag in tag_list] metadef_tags = MetadefTags() diff --git a/glance/db/__init__.py b/glance/db/__init__.py index 182832a89f..d5d7b92102 100644 --- a/glance/db/__init__.py +++ b/glance/db/__init__.py @@ -846,7 +846,7 @@ class MetadefTagRepo(object): self._format_metadef_tag_to_db(metadata_tag) ) - def add_tags(self, metadata_tags): + def add_tags(self, metadata_tags, can_append=False): tag_list = [] namespace = None for metadata_tag in metadata_tags: @@ -855,7 +855,7 @@ class MetadefTagRepo(object): namespace = metadata_tag.namespace self.db_api.metadef_tag_create_tags( - self.context, namespace, tag_list) + self.context, namespace, tag_list, can_append) def get(self, namespace, name): try: diff --git a/glance/db/simple/api.py b/glance/db/simple/api.py index 12f0b0b90b..14ac17670e 100644 --- a/glance/db/simple/api.py +++ b/glance/db/simple/api.py @@ -1910,7 +1910,8 @@ def metadef_tag_create(context, namespace_name, values): @log_call -def metadef_tag_create_tags(context, namespace_name, tag_list): +def metadef_tag_create_tags(context, namespace_name, tag_list, + can_append=False): """Create a metadef tag""" global DATA @@ -1921,6 +1922,12 @@ def metadef_tag_create_tags(context, namespace_name, tag_list): allowed_attributes = ['name'] data_tag_list = [] tag_name_list = [] + if can_append: + # NOTE(mrjoshi): We need to fetch existing tags here for duplicate + # check while adding new one + tag_name_list = [tag['name'] + for tag in metadef_tag_get_all(context, + namespace_name)] for tag_value in tag_list: tag_values = copy.deepcopy(tag_value) tag_name = tag_values['name'] @@ -1945,8 +1952,8 @@ def metadef_tag_create_tags(context, namespace_name, tag_list): tag_values['namespace_id'] = namespace['id'] data_tag_list.append(_format_tag(tag_values)) - - DATA['metadef_tags'] = [] + if not can_append: + DATA['metadef_tags'] = [] for tag in data_tag_list: DATA['metadef_tags'].append(tag) diff --git a/glance/db/sqlalchemy/api.py b/glance/db/sqlalchemy/api.py index 2099d378eb..b8d40c05e8 100644 --- a/glance/db/sqlalchemy/api.py +++ b/glance/db/sqlalchemy/api.py @@ -2180,11 +2180,11 @@ def metadef_tag_create(context, namespace_name, tag_dict, def metadef_tag_create_tags(context, namespace_name, tag_list, - session=None): + can_append=False, session=None): """Create a metadata-schema tag or raise if it already exists.""" session = get_session() return metadef_tag_api.create_tags( - context, namespace_name, tag_list, session) + context, namespace_name, tag_list, can_append, session) @utils.no_4byte_params diff --git a/glance/db/sqlalchemy/metadef_api/tag.py b/glance/db/sqlalchemy/metadef_api/tag.py index fac0728b6a..d3f200ca1d 100644 --- a/glance/db/sqlalchemy/metadef_api/tag.py +++ b/glance/db/sqlalchemy/metadef_api/tag.py @@ -111,7 +111,7 @@ def create(context, namespace_name, values, session): return metadef_tag.to_dict() -def create_tags(context, namespace_name, tag_list, session): +def create_tags(context, namespace_name, tag_list, can_append, session): metadef_tags_list = [] if tag_list: @@ -119,10 +119,10 @@ def create_tags(context, namespace_name, tag_list, session): try: with session.begin(): - query = (session.query(models.MetadefTag).filter_by( - namespace_id=namespace['id'])) - query.delete(synchronize_session='fetch') - + if not can_append: + query = (session.query(models.MetadefTag).filter_by( + namespace_id=namespace['id'])) + query.delete(synchronize_session='fetch') for value in tag_list: value.update({'namespace_id': namespace['id']}) metadef_utils.drop_protected_attrs( diff --git a/glance/domain/proxy.py b/glance/domain/proxy.py index 617f9c4ee2..72551bec81 100644 --- a/glance/domain/proxy.py +++ b/glance/domain/proxy.py @@ -547,11 +547,11 @@ class MetadefTagRepo(object): def add(self, meta_tag): self.base.add(self.tag_proxy_helper.unproxy(meta_tag)) - def add_tags(self, meta_tags): + def add_tags(self, meta_tags, can_append=False): tags_list = [] for meta_tag in meta_tags: tags_list.append(self.tag_proxy_helper.unproxy(meta_tag)) - self.base.add_tags(tags_list) + self.base.add_tags(tags_list, can_append) def list(self, *args, **kwargs): tags = self.base.list(*args, **kwargs) diff --git a/glance/notifier.py b/glance/notifier.py index ea25be1914..cb83d13a8d 100644 --- a/glance/notifier.py +++ b/glance/notifier.py @@ -913,8 +913,9 @@ class MetadefTagRepoProxy(NotificationRepoProxy, domain_proxy.MetadefTagRepo): self.send_notification('metadef_tag.create', metadef_tag) return result - def add_tags(self, metadef_tags): - result = super(MetadefTagRepoProxy, self).add_tags(metadef_tags) + def add_tags(self, metadef_tags, can_append=False): + result = super(MetadefTagRepoProxy, self).add_tags(metadef_tags, + can_append) for metadef_tag in metadef_tags: self.send_notification('metadef_tag.create', metadef_tag) diff --git a/glance/tests/functional/db/base_metadef.py b/glance/tests/functional/db/base_metadef.py index a2af098024..9762c02aff 100644 --- a/glance/tests/functional/db/base_metadef.py +++ b/glance/tests/functional/db/base_metadef.py @@ -590,6 +590,34 @@ class MetadefTagTests(object): expected = set(['Tag1', 'Tag2', 'Tag3']) self.assertEqual(expected, actual) + def test_tag_create_tags_with_append(self): + fixture = build_namespace_fixture() + created_ns = self.db_api.metadef_namespace_create(self.context, + fixture) + self.assertIsNotNone(created_ns) + self._assert_saved_fields(fixture, created_ns) + + tags = build_tags_fixture(['Tag1', 'Tag2', 'Tag3']) + created_tags = self.db_api.metadef_tag_create_tags( + self.context, created_ns['namespace'], tags) + actual = set([tag['name'] for tag in created_tags]) + expected = set(['Tag1', 'Tag2', 'Tag3']) + self.assertEqual(expected, actual) + + new_tags = build_tags_fixture(['Tag4', 'Tag5', 'Tag6']) + new_created_tags = self.db_api.metadef_tag_create_tags( + self.context, created_ns['namespace'], new_tags, can_append=True) + actual = set([tag['name'] for tag in new_created_tags]) + expected = set(['Tag4', 'Tag5', 'Tag6']) + self.assertEqual(expected, actual) + + tags = self.db_api.metadef_tag_get_all(self.context, + created_ns['namespace'], + sort_key='created_at') + actual = set([tag['name'] for tag in tags]) + expected = set(['Tag1', 'Tag2', 'Tag3', 'Tag4', 'Tag5', 'Tag6']) + self.assertEqual(expected, actual) + def test_tag_create_duplicate_tags_1(self): fixture = build_namespace_fixture() created_ns = self.db_api.metadef_namespace_create(self.context, @@ -619,6 +647,22 @@ class MetadefTagTests(object): self.db_api.metadef_tag_create, self.context, created_ns['namespace'], dup_tag) + def test_tag_create_duplicate_tags_3(self): + fixture = build_namespace_fixture() + created_ns = self.db_api.metadef_namespace_create(self.context, + fixture) + self.assertIsNotNone(created_ns) + self._assert_saved_fields(fixture, created_ns) + + tags = build_tags_fixture(['Tag1', 'Tag2', 'Tag3']) + self.db_api.metadef_tag_create_tags(self.context, + created_ns['namespace'], tags) + dup_tags = build_tags_fixture(['Tag3', 'Tag4', 'Tag5']) + self.assertRaises(exception.Duplicate, + self.db_api.metadef_tag_create_tags, + self.context, created_ns['namespace'], + dup_tags, can_append=True) + def test_tag_get(self): fixture_ns = build_namespace_fixture() created_ns = self.db_api.metadef_namespace_create(self.context, diff --git a/glance/tests/functional/v2/test_metadef_tags.py b/glance/tests/functional/v2/test_metadef_tags.py index 3c91ebbc27..6548db7a51 100644 --- a/glance/tests/functional/v2/test_metadef_tags.py +++ b/glance/tests/functional/v2/test_metadef_tags.py @@ -160,6 +160,36 @@ class TestMetadefTags(metadef_base.MetadefFunctionalTestBase): tags = jsonutils.loads(response.text)['tags'] self.assertEqual(3, len(tags)) + # Create new tags and append to existing tags. + path = self._url('/v2/metadefs/namespaces/%s/tags' % + (namespace_name)) + headers = self._headers({'content-type': 'application/json', + 'X-Openstack-Append': 'True'}) + data = jsonutils.dumps( + {"tags": [{"name": "tag4"}, {"name": "tag5"}, {"name": "tag6"}]} + ) + response = requests.post(path, headers=headers, data=data) + self.assertEqual(http.CREATED, response.status_code) + + # List out all six tags. + response = requests.get(path, headers=self._headers()) + self.assertEqual(http.OK, response.status_code) + tags = jsonutils.loads(response.text)['tags'] + self.assertEqual(6, len(tags)) + + # Attempt to create duplicate existing tag6 + data = jsonutils.dumps( + {"tags": [{"name": "tag6"}, {"name": "tag7"}, {"name": "tag8"}]} + ) + response = requests.post(path, headers=headers, data=data) + self.assertEqual(http.CONFLICT, response.status_code) + + # Verify the previous 6 still exist + response = requests.get(path, headers=self._headers()) + self.assertEqual(http.OK, response.status_code) + tags = jsonutils.loads(response.text)['tags'] + self.assertEqual(6, len(tags)) + def _create_tags(self, namespaces): tags = [] for namespace in namespaces: diff --git a/glance/tests/unit/test_db_metadef.py b/glance/tests/unit/test_db_metadef.py index 27188f201a..3042ef4310 100644 --- a/glance/tests/unit/test_db_metadef.py +++ b/glance/tests/unit/test_db_metadef.py @@ -515,6 +515,18 @@ class TestMetadefRepo(test_utils.BaseTestCase): tag_names = set([t.name for t in tags]) self.assertEqual(set([TAG3, TAG4, TAG5]), tag_names) + def test_add_tags_with_append_true(self): + tags = self.tag_repo.list(filters={'namespace': NAMESPACE1}) + tag_names = set([t.name for t in tags]) + self.assertEqual(set([TAG1, TAG2, TAG3]), tag_names) + + tags = _db_tags_fixture([TAG4, TAG5]) + self.db.metadef_tag_create_tags(self.context, NAMESPACE1, tags, + can_append=True) + tags = self.tag_repo.list(filters={'namespace': NAMESPACE1}) + tag_names = set([t.name for t in tags]) + self.assertEqual(set([TAG1, TAG2, TAG3, TAG4, TAG5]), tag_names) + def test_add_duplicate_tags_with_pre_existing_tags(self): tags = self.tag_repo.list(filters={'namespace': NAMESPACE1}) tag_names = set([t.name for t in tags]) diff --git a/glance/tests/unit/test_policy.py b/glance/tests/unit/test_policy.py index 5c52eedf8a..c2cbbbd7d5 100644 --- a/glance/tests/unit/test_policy.py +++ b/glance/tests/unit/test_policy.py @@ -239,7 +239,7 @@ class MdTagRepoStub(object): def add(self, tag): return 'mdtag_add' - def add_tags(self, tags): + def add_tags(self, tags, can_append=False): return ['mdtag_add_tags'] def get(self, ns, tag_name): diff --git a/glance/tests/unit/utils.py b/glance/tests/unit/utils.py index 8362a42462..ccf5107b33 100644 --- a/glance/tests/unit/utils.py +++ b/glance/tests/unit/utils.py @@ -64,7 +64,7 @@ def sort_url_by_qs_keys(url): def get_fake_request(path='', method='POST', is_admin=False, user=USER1, - roles=None, tenant=TENANT1): + roles=None, headers=None, tenant=TENANT1): if roles is None: roles = ['member', 'reader'] @@ -72,6 +72,9 @@ def get_fake_request(path='', method='POST', is_admin=False, user=USER1, req.method = method req.headers = {'x-openstack-request-id': 'my-req'} + if headers is not None: + req.headers.update(headers) + kwargs = { 'user': user, 'tenant': tenant, diff --git a/glance/tests/unit/v2/test_metadef_resources.py b/glance/tests/unit/v2/test_metadef_resources.py index 134ec86763..781bce0fb5 100644 --- a/glance/tests/unit/v2/test_metadef_resources.py +++ b/glance/tests/unit/v2/test_metadef_resources.py @@ -2002,6 +2002,94 @@ class TestMetadefsControllers(base.IsolatedUnitTest): ] ) + def test_tag_create_tags_with_append_true(self): + request = unit_test_utils.get_fake_request( + headers={'X-Openstack-Append': 'True'}, roles=['admin']) + + metadef_tags = tags.MetadefTags() + # As TAG1 is already created in setup, just creating other two tags. + metadef_tags.tags = _db_tags_fixture([TAG2, TAG3]) + output = self.tag_controller.create_tags( + request, metadef_tags, NAMESPACE1) + output = output.to_dict() + self.assertEqual(2, len(output['tags'])) + actual = set([tag.name for tag in output['tags']]) + expected = set([TAG2, TAG3]) + self.assertEqual(expected, actual) + self.assertNotificationLog( + "metadef_tag.create", [ + {'name': TAG2, 'namespace': NAMESPACE1}, + {'name': TAG3, 'namespace': NAMESPACE1}, + ] + ) + + metadef_tags = tags.MetadefTags() + metadef_tags.tags = _db_tags_fixture([TAG4, TAG5]) + output = self.tag_controller.create_tags( + request, metadef_tags, NAMESPACE1) + output = output.to_dict() + self.assertEqual(2, len(output['tags'])) + actual = set([tag.name for tag in output['tags']]) + expected = set([TAG4, TAG5]) + self.assertEqual(expected, actual) + self.assertNotificationLog( + "metadef_tag.create", [ + {'name': TAG4, 'namespace': NAMESPACE1}, + {'name': TAG5, 'namespace': NAMESPACE1}, + ] + ) + + output = self.tag_controller.index(request, NAMESPACE1) + output = output.to_dict() + self.assertEqual(5, len(output['tags'])) + actual = set([tag.name for tag in output['tags']]) + expected = set([TAG1, TAG2, TAG3, TAG4, TAG5]) + self.assertEqual(expected, actual) + + def test_tag_create_tags_with_append_false(self): + request = unit_test_utils.get_fake_request( + headers={'X-Openstack-Append': 'False'}, roles=['admin']) + + metadef_tags = tags.MetadefTags() + # As TAG1 is already created in setup, just creating other two tags. + metadef_tags.tags = _db_tags_fixture([TAG2, TAG3]) + output = self.tag_controller.create_tags( + request, metadef_tags, NAMESPACE1) + output = output.to_dict() + self.assertEqual(2, len(output['tags'])) + actual = set([tag.name for tag in output['tags']]) + expected = set([TAG2, TAG3]) + self.assertEqual(expected, actual) + self.assertNotificationLog( + "metadef_tag.create", [ + {'name': TAG2, 'namespace': NAMESPACE1}, + {'name': TAG3, 'namespace': NAMESPACE1}, + ] + ) + + metadef_tags = tags.MetadefTags() + metadef_tags.tags = _db_tags_fixture([TAG4, TAG5]) + output = self.tag_controller.create_tags( + request, metadef_tags, NAMESPACE1) + output = output.to_dict() + self.assertEqual(2, len(output['tags'])) + actual = set([tag.name for tag in output['tags']]) + expected = set([TAG4, TAG5]) + self.assertEqual(expected, actual) + self.assertNotificationLog( + "metadef_tag.create", [ + {'name': TAG4, 'namespace': NAMESPACE1}, + {'name': TAG5, 'namespace': NAMESPACE1}, + ] + ) + + output = self.tag_controller.index(request, NAMESPACE1) + output = output.to_dict() + self.assertEqual(2, len(output['tags'])) + actual = set([tag.name for tag in output['tags']]) + expected = set([TAG4, TAG5]) + self.assertEqual(expected, actual) + def test_tag_create_duplicate_tags(self): request = unit_test_utils.get_fake_request(roles=['admin']) @@ -2048,6 +2136,42 @@ class TestMetadefsControllers(base.IsolatedUnitTest): expected = set([TAG1, TAG2, TAG3]) self.assertEqual(expected, actual) + def test_tag_create_duplicate_with_pre_existing_tags_with_append(self): + request = unit_test_utils.get_fake_request( + headers={'X-Openstack-Append': 'True'}, roles=['admin']) + + metadef_tags = tags.MetadefTags() + # As TAG1 is already created in setup, just creating other two tags. + metadef_tags.tags = _db_tags_fixture([TAG2, TAG3]) + output = self.tag_controller.create_tags( + request, metadef_tags, NAMESPACE1) + output = output.to_dict() + self.assertEqual(2, len(output['tags'])) + actual = set([tag.name for tag in output['tags']]) + expected = set([TAG2, TAG3]) + self.assertEqual(expected, actual) + self.assertNotificationLog( + "metadef_tag.create", [ + {'name': TAG2, 'namespace': NAMESPACE1}, + {'name': TAG3, 'namespace': NAMESPACE1}, + ] + ) + + metadef_tags = tags.MetadefTags() + metadef_tags.tags = _db_tags_fixture([TAG4, TAG5, TAG4]) + self.assertRaises( + webob.exc.HTTPConflict, + self.tag_controller.create_tags, + request, metadef_tags, NAMESPACE1) + self.assertNotificationsLog([]) + + output = self.tag_controller.index(request, NAMESPACE1) + output = output.to_dict() + self.assertEqual(3, len(output['tags'])) + actual = set([tag.name for tag in output['tags']]) + expected = set([TAG1, TAG2, TAG3]) + self.assertEqual(expected, actual) + def test_tag_create_conflict(self): request = unit_test_utils.get_fake_request(roles=['admin']) self.assertRaises(webob.exc.HTTPConflict, diff --git a/releasenotes/notes/fix-md-tag-create-multiple-c04756cf5155983d.yaml b/releasenotes/notes/fix-md-tag-create-multiple-c04756cf5155983d.yaml new file mode 100644 index 0000000000..775c8a13ef --- /dev/null +++ b/releasenotes/notes/fix-md-tag-create-multiple-c04756cf5155983d.yaml @@ -0,0 +1,13 @@ +--- +features: + - | + A new optional header ``X-Openstack-Append`` has been added to append the + new metadef tags to the existing tags. If the header is present it will + append the new tags to the existing one, if not then it will default to the + old behaviour i.e. overwriting the existing tags with the new one. +Fixes: + - | + * Bug 1939169_: glance md-tag-create-multiple overwrites existing tags + + .. _1939169: https://bugs.launchpad.net/glance/+bug/1939169 +