From 4dc1077a6e0077b6d3bf90793d9b4d3788e2c19d Mon Sep 17 00:00:00 2001 From: Ruslan Aliev Date: Sun, 27 Nov 2016 18:51:13 +0300 Subject: [PATCH] Use UUID as backup_id parameter Current backup_id field may contain unsupported characters for HTTP requests. It causes problems with /v1/backups/{backup_id} requests. Proposed solution: use UUID as backup_id instead of 'container_hostname_backupname_timestamp_level' pattern for define backup_id. Change-Id: I38e2ed52ab375ea0705cdb88f82e5e66cef5cd39 Closes-Bug: #1645110 Closes-Bug: #1618030 Signed-off-by: Ruslan Aliev --- freezer_api/common/db_mappings.py | 3 --- freezer_api/common/utils.py | 25 +++---------------- .../tests/api/test_api_backups.py | 22 +++------------- freezer_api/tests/unit/common.py | 6 ++--- freezer_api/tests/unit/test_elastic.py | 2 +- freezer_api/tests/unit/test_utils.py | 10 +------- install-guide/source/get_started.rst | 8 +----- install-guide/source/metadata_structure.rst | 2 +- specs/Freezer-API-spec.rst | 8 +++--- 9 files changed, 19 insertions(+), 67 deletions(-) diff --git a/freezer_api/common/db_mappings.py b/freezer_api/common/db_mappings.py index 70f2798e..5e3e58b6 100644 --- a/freezer_api/common/db_mappings.py +++ b/freezer_api/common/db_mappings.py @@ -203,9 +203,6 @@ backups_mapping = { } } }, - "backup_uuid": { - "type": "string" - }, "user_id": { "index": "not_analyzed", "type": "string" diff --git a/freezer_api/common/utils.py b/freezer_api/common/utils.py index d2eb19a6..bc04bd6e 100644 --- a/freezer_api/common/utils.py +++ b/freezer_api/common/utils.py @@ -30,43 +30,26 @@ class BackupMetadataDoc: def __init__(self, user_id='', user_name='', data={}): self.user_id = user_id self.user_name = user_name + self.backup_id = uuid.uuid4().hex self.data = data def is_valid(self): try: assert (self.backup_id is not '') assert (self.user_id is not '') + assert (self.data['container'] is not '') + assert (self.data['hostname'] is not '') + assert (self.data['backup_name'] is not '') except Exception: return False return True def serialize(self): return {'backup_id': self.backup_id, - 'backup_uuid': self.backup_uuid, 'user_id': self.user_id, 'user_name': self.user_name, 'backup_metadata': self.data} - @property - def backup_set_id(self): - return '{0}_{1}_{2}'.format( - self.data['container'], - self.data['hostname'], - self.data['backup_name'] - ) - - @property - def backup_id(self): - return '{0}_{1}_{2}'.format( - self.backup_set_id, - self.data['time_stamp'], - self.data['curr_backup_level'] - ) - - @property - def backup_uuid(self): - return uuid.uuid4().hex - class JobDoc: job_doc_validator = jsonschema.Draft4Validator( diff --git a/freezer_api/tests/freezer_api_tempest_plugin/tests/api/test_api_backups.py b/freezer_api/tests/freezer_api_tempest_plugin/tests/api/test_api_backups.py index 9845d6c9..f1ea32d7 100644 --- a/freezer_api/tests/freezer_api_tempest_plugin/tests/api/test_api_backups.py +++ b/freezer_api/tests/freezer_api_tempest_plugin/tests/api/test_api_backups.py @@ -142,10 +142,8 @@ class TestFreezerApiBackups(base.BaseFreezerApiTest): expected = self._build_expected_data(backup_id, metadata) - # TODO(JonasPf): Currently there is no way to know the uuid as the api client - # until this is fixed we'll ignore it. - del(expected['backup_uuid']) - del(response_body['backup_uuid']) + # backup_id is generated automatically, we can't know it + del(response_body['backup_id']) self.assertEqual(200, resp.status) self.assertEqual(expected, response_body) @@ -189,22 +187,12 @@ class TestFreezerApiBackups(base.BaseFreezerApiTest): expected = self._build_expected_data(backup_id, metadata) - # TODO(JonasPf): Currently there is no way to know the uuid as the api client - # until this is fixed we'll ignore it. - del(expected['backup_uuid']) - del(response_body['backup_uuid']) + # backup_id is generated automatically, we can't know it + del(response_body['backup_id']) self.assertEqual(200, resp.status) self.assertEqual(expected, response_body) - @test.attr(type="gate") - def test_api_backups_post_twice(self): - """ Create the same backup twice expecting an error message """ - metadata = self._build_metadata("test_freezer_backups") - self._create_temporary_backup(metadata) - - self.assertRaises(tempest.lib.exceptions.Conflict, self._create_temporary_backup, metadata) - @test.attr(type="gate") def test_api_backups_delete(self): metadata = self._build_metadata("test_freezer_backups") @@ -232,9 +220,7 @@ class TestFreezerApiBackups(base.BaseFreezerApiTest): def _build_expected_data(self, backup_id, metadata): return { 'user_name': self.os.credentials.username, - 'backup_uuid': None, 'user_id': self.os.credentials.user_id, - 'backup_id': backup_id, 'backup_metadata': metadata } diff --git a/freezer_api/tests/unit/common.py b/freezer_api/tests/unit/common.py index 6a22c263..116d41c1 100644 --- a/freezer_api/tests/unit/common.py +++ b/freezer_api/tests/unit/common.py @@ -33,12 +33,12 @@ from freezer_api import policy CONF = cfg.CONF -fake_data_0_backup_id = 'freezer_container_alpha_important_data_backup_8475903425_0' +fake_data_0_backup_id = 'b740ed9ad2b646aba304ef54c21c6774' fake_data_0_user_id = 'qwerty1234' fake_data_0_user_name = 'asdffdsa' fake_data_0_wrapped_backup_metadata = { - 'backup_id': 'freezer_container_alpha_important_data_backup_8475903425_0', + 'backup_id': 'b740ed9ad2b646aba304ef54c21c6774', 'user_id': 'qwerty1234', 'user_name': 'asdffdsa', 'backup_metadata': { @@ -252,7 +252,7 @@ fake_job_0_elasticsearch_found = { fake_data_1_wrapped_backup_metadata = { - 'backup_id': 'freezer_container_alpha_important_data_backup_125235431_1', + 'backup_id': 'b740ed9ad2b646aba304ef54c21c6774', 'user_id': 'qwerty1234', 'user_name': 'asdffdsa', 'backup_metadata': { diff --git a/freezer_api/tests/unit/test_elastic.py b/freezer_api/tests/unit/test_elastic.py index 0f9fd1b8..bd17e698 100644 --- a/freezer_api/tests/unit/test_elastic.py +++ b/freezer_api/tests/unit/test_elastic.py @@ -463,7 +463,7 @@ class TestElasticSearchEngine_backup(unittest.TestCase): res = self.eng.add_backup(fake_data_0_user_id, user_name=fake_data_0_user_name, doc=fake_data_0_backup_metadata) - self.assertEqual(res, fake_data_0_wrapped_backup_metadata['backup_id']) + self.assertTrue(res) def test_add_backup_raises_when_doc_exists(self): self.eng.backup_manager.search.return_value = [fake_data_0_wrapped_backup_metadata] diff --git a/freezer_api/tests/unit/test_utils.py b/freezer_api/tests/unit/test_utils.py index c21249d3..f443ed53 100644 --- a/freezer_api/tests/unit/test_utils.py +++ b/freezer_api/tests/unit/test_utils.py @@ -53,7 +53,7 @@ DATA_user_id = 'AUx6F07NwlhuOVELWtHx' DATA_user_name = 'gegrex55NPlwlhuOVELWtHv' -DATA_backup_id = 'freezer_container_alpha_important_data_backup_12341234_0' +DATA_backup_id = 'd78f820089d64b7b9096d3133ca3162d' DATA_wrapped_backup_metadata = { 'user_id': DATA_user_id, @@ -94,9 +94,6 @@ class TestBackupMetadataDoc(unittest.TestCase): user_name=DATA_user_name, data=DATA_backup_metadata) - def test_backup_id(self): - assert (self.backup_metadata.backup_id == DATA_backup_id) - def test_is_valid_return_True_when_valid(self): self.assertTrue(self.backup_metadata.is_valid()) @@ -104,11 +101,6 @@ class TestBackupMetadataDoc(unittest.TestCase): self.backup_metadata.user_id = '' self.assertFalse(self.backup_metadata.is_valid()) - def test_backup_id_correct(self): - self.assertEqual(self.backup_metadata.backup_id, DATA_backup_id) - self.backup_metadata.data['container'] = 'different' - self.assertNotEqual(self.backup_metadata.backup_id, DATA_backup_id) - class TestJobDoc(unittest.TestCase): diff --git a/install-guide/source/get_started.rst b/install-guide/source/get_started.rst index 7d9e6667..efa2f8aa 100644 --- a/install-guide/source/get_started.rst +++ b/install-guide/source/get_started.rst @@ -18,10 +18,4 @@ Concepts and definitions *hostname* is _probably_ going to be the host fqdn. *backup_id* -defined as `container_hostname_backupname_timestamp_level` uniquely -identifies a backup - -*backup_set* -defined as `container_hostname_backupname` identifies a group of related -backups which share the same container,hostname and backupname - +defined as UUID of a backup diff --git a/install-guide/source/metadata_structure.rst b/install-guide/source/metadata_structure.rst index 867dd9eb..8217b7b3 100644 --- a/install-guide/source/metadata_structure.rst +++ b/install-guide/source/metadata_structure.rst @@ -40,7 +40,7 @@ It stores and returns the information provided in this form .. code-block:: json { - "backup_id": string # container_hostname_backupname_timestamp_level + "backup_id": string # backup UUID "user_id": string, # owner of the backup metadata (OS X-User-Id, keystone provided) "user_name": string # owner of the backup metadata (OS X-User-Name, keystone provided) diff --git a/specs/Freezer-API-spec.rst b/specs/Freezer-API-spec.rst index 09b78046..7ea83dc0 100644 --- a/specs/Freezer-API-spec.rst +++ b/specs/Freezer-API-spec.rst @@ -144,7 +144,7 @@ Example :: { - "backup_id": "freezer_container_alpha_important_data_backup_123444324_1" + "backup_id": "b740ed9ad2b646aba304ef54c21c6774" } 3.2 List backup information @@ -180,7 +180,7 @@ Example "backups": [ { "backup_id": - "freezer_container_alpha_important_data_backup_123444324_1", + "b740ed9ad2b646aba304ef54c21c6774", "backup_metadata": { "backup_name": "important_data_backup", "backup_session": 12344321, @@ -205,7 +205,7 @@ Example }, { "backup_id": - "freezer_container_alpha_important_data_backup_123444311_0", + "b740ed9ad2b646aba304ef54c21c6774", "backup_metadata": { "backup_name": "important_data_backup", "backup_session": 12344311, @@ -253,7 +253,7 @@ Example { - "backup_id": "freezer_container_alpha_important_data_backup_123444311_0", + "backup_id": "b740ed9ad2b646aba304ef54c21c6774", "backup_metadata": { "backup_name": "important_data_backup", "backup_session": 12344311,