From cd6ad8485f4a29d88d7c8cd58f5c95d5d11034ed Mon Sep 17 00:00:00 2001 From: Chris MacNaughton Date: Tue, 29 Jan 2019 14:03:47 +0100 Subject: [PATCH 1/8] Allow file assertions to include globs --- .../test_zaza_utilities_file_assertions.py | 63 +++++++++++++++++ zaza/charm_tests/security/tests.py | 33 ++++----- zaza/utilities/file_assertions.py | 68 +++++++++++++++++++ 3 files changed, 143 insertions(+), 21 deletions(-) create mode 100644 unit_tests/utilities/test_zaza_utilities_file_assertions.py create mode 100644 zaza/utilities/file_assertions.py diff --git a/unit_tests/utilities/test_zaza_utilities_file_assertions.py b/unit_tests/utilities/test_zaza_utilities_file_assertions.py new file mode 100644 index 0000000..f0eba1c --- /dev/null +++ b/unit_tests/utilities/test_zaza_utilities_file_assertions.py @@ -0,0 +1,63 @@ +# Copyright 2018 Canonical Ltd. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import mock +import unit_tests.utils as ut_utils +import zaza.utilities.file_assertions as file_assertions + + +class TestFileAssertionUtils(ut_utils.BaseTestCase): + def setUp(self): + super(TestFileAssertionUtils, self).setUp() + # Patch all run_on_unit calls + self.patch( + 'zaza.utilities.file_assertions.model.run_on_unit', + new_callable=mock.MagicMock(), + name='run_on_unit' + ) + self._assert = mock.MagicMock() + self._assert.assertEqual = mock.MagicMock() + + def test_path_glob(self): + self.run_on_unit.return_value = { + 'Stdout': 'file-name root root 600' + } + file_details = {'path': '*'} + file_assertions.assert_path_glob( + self._assert, 'test/0', file_details) + self.run_on_unit.assert_called_once_with( + 'test/0', 'stat -c "%n %U %G %a" *') + + def test_single_path(self): + self.run_on_unit.return_value = { + 'Stdout': 'root root 600' + } + file_details = {'path': 'test'} + file_assertions.assert_single_file( + self._assert, 'test/0', file_details) + self.run_on_unit.assert_called_once_with( + 'test/0', 'stat -c "%U %G %a" test') + + def test_error_message_glob(self): + message = file_assertions._error_message( + "Owner", "test/0", "root", "/path/to/something") + self.assertEqual( + message, + "Owner is incorrect for /path/to/something on test/0: root") + + def test_error_message_single(self): + + message = file_assertions._error_message( + "Owner", "test/0", "root") + self.assertEqual(message, "Owner is incorrect on test/0: root") diff --git a/zaza/charm_tests/security/tests.py b/zaza/charm_tests/security/tests.py index 51c685e..2a414a2 100644 --- a/zaza/charm_tests/security/tests.py +++ b/zaza/charm_tests/security/tests.py @@ -20,31 +20,20 @@ import unittest import zaza.model as model import zaza.charm_lifecycle.utils as utils +from zaza.utilities.file_assertions import ( + assert_path_glob, + assert_single_file, +) -def _make_test_function(application, file_details): +def _make_test_function(application, file_details, paths=[]): def test(self): - expected_owner = file_details.get("owner", "root") - expected_group = file_details.get("group", "root") - expected_mode = file_details.get("mode", "600") for unit in model.get_units(application): unit = unit.entity_id - result = model.run_on_unit( - unit, 'stat -c "%U %G %a" {}'.format(file_details['path'])) - ownership = result['Stdout'] - owner, group, mode = ownership.split() - self.assertEqual(expected_owner, - owner, - "Owner is incorrect for {}: {}" - .format(unit, owner)) - self.assertEqual(expected_group, - group, - "Group is incorrect for {}: {}" - .format(unit, group)) - self.assertEqual(expected_mode, - mode, - "Mode is incorrect for {}: {}" - .format(unit, mode)) + if '*' in file_details['path']: + assert_path_glob(self, unit, file_details, paths) + else: + assert_single_file(self, unit, file_details) return test @@ -57,7 +46,9 @@ def _add_tests(): # Lets make sure to only add tests for deployed applications if name in deployed_applications: for file in attributes['files']: - test_func = _make_test_function(name, file) + paths = [file['path'] for file in attributes['files']] + paths = [path for path in paths if path[-1] is not "*"] + test_func = _make_test_function(name, file, paths=paths) setattr( cls, 'test_{}_{}'.format(name, file['path']), diff --git a/zaza/utilities/file_assertions.py b/zaza/utilities/file_assertions.py new file mode 100644 index 0000000..08375c1 --- /dev/null +++ b/zaza/utilities/file_assertions.py @@ -0,0 +1,68 @@ +# Copyright 2018 Canonical Ltd. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Module of helpers for Zaza file assertions.""" + +import zaza.model as model + + +def assert_path_glob(test_case, unit, file_details, paths=[]): + """Verify all files in a given directory.""" + result = model.run_on_unit( + unit, 'stat -c "%n %U %G %a" {}'.format(file_details['path'])) + files = result['Stdout'] + for file in files.splitlines(): + file, owner, group, mode = file.split() + if file not in paths and file not in ['.', '..']: + verify_file(test_case, + unit, + file_details, + owner, + group, + mode, + path=file) + + +def assert_single_file(test_case, unit, file_details): + """Verify ownership of a single file.""" + result = model.run_on_unit( + unit, 'stat -c "%U %G %a" {}'.format(file_details['path'])) + ownership = result['Stdout'] + owner, group, mode = ownership.split() + verify_file(test_case, unit, file_details, owner, group, mode) + + +def verify_file(test_case, unit, file_details, + actual_owner, actual_group, actual_mode, path=None): + """Assert file has correct permissions.""" + expected_owner = file_details.get("owner", "root") + expected_group = file_details.get("group", "root") + expected_mode = file_details.get("mode", "600") + test_case.assertEqual(expected_owner, + actual_owner, + _error_message("Owner", unit, actual_owner, path)) + test_case.assertEqual(expected_group, + actual_group, + _error_message("Group", unit, actual_group, path)) + test_case.assertEqual(expected_mode, + actual_mode, + _error_message("Mode", unit, actual_mode, path)) + + +def _error_message(thing, unit, value, path=None): + if path: + return "{} is incorrect for {} on {}: {}".format( + thing, path, unit, value) + else: + return "{} is incorrect on {}: {}".format(thing, unit, value) From 774a49faef092f88747bdc9f7bb2ea6e4d078a00 Mon Sep 17 00:00:00 2001 From: Chris MacNaughton Date: Tue, 29 Jan 2019 14:49:03 +0100 Subject: [PATCH 2/8] Support globstar for recursion --- unit_tests/utilities/test_zaza_utilities_file_assertions.py | 3 ++- zaza/utilities/file_assertions.py | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/unit_tests/utilities/test_zaza_utilities_file_assertions.py b/unit_tests/utilities/test_zaza_utilities_file_assertions.py index f0eba1c..b40276c 100644 --- a/unit_tests/utilities/test_zaza_utilities_file_assertions.py +++ b/unit_tests/utilities/test_zaza_utilities_file_assertions.py @@ -37,7 +37,8 @@ class TestFileAssertionUtils(ut_utils.BaseTestCase): file_assertions.assert_path_glob( self._assert, 'test/0', file_details) self.run_on_unit.assert_called_once_with( - 'test/0', 'stat -c "%n %U %G %a" *') + 'test/0', 'bash -c "shopt -s -q globstar;' + ' stat -c "%n %U %G %a" *"') def test_single_path(self): self.run_on_unit.return_value = { diff --git a/zaza/utilities/file_assertions.py b/zaza/utilities/file_assertions.py index 08375c1..470fcd2 100644 --- a/zaza/utilities/file_assertions.py +++ b/zaza/utilities/file_assertions.py @@ -20,7 +20,9 @@ import zaza.model as model def assert_path_glob(test_case, unit, file_details, paths=[]): """Verify all files in a given directory.""" result = model.run_on_unit( - unit, 'stat -c "%n %U %G %a" {}'.format(file_details['path'])) + unit, 'bash -c "' + 'shopt -s -q globstar; ' + 'stat -c "%n %U %G %a" {}"'.format(file_details['path'])) files = result['Stdout'] for file in files.splitlines(): file, owner, group, mode = file.split() From 6ec0eeb04f605fcc97fd15b537e688298a747bcf Mon Sep 17 00:00:00 2001 From: Chris MacNaughton Date: Wed, 30 Jan 2019 11:00:57 +0100 Subject: [PATCH 3/8] allow deployed bundle to use DVR --- zaza/charm_tests/neutron/setup.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/zaza/charm_tests/neutron/setup.py b/zaza/charm_tests/neutron/setup.py index d4fcd63..306b543 100644 --- a/zaza/charm_tests/neutron/setup.py +++ b/zaza/charm_tests/neutron/setup.py @@ -25,6 +25,8 @@ from zaza.utilities import ( juju as juju_utils, openstack as openstack_utils, ) +import zaza.model as model + # The overcloud network configuration settings are declared. # These are the network configuration settings under test. @@ -72,6 +74,10 @@ def basic_overcloud_network(): network_config.update(DEFAULT_UNDERCLOUD_NETWORK_CONFIG) # Environment specific settings network_config.update(generic_utils.get_undercloud_env_vars()) + # Deployed model settings + if (model.get_application_config('neutron-api') + .get('enable-dvr').get('value')): + network_config.update({"dvr_enabled": True}) # Get keystone session keystone_session = openstack_utils.get_overcloud_keystone_session() From a49dfb79c94f51f860e2b76f622f59f18ad16274 Mon Sep 17 00:00:00 2001 From: Chris MacNaughton Date: Wed, 30 Jan 2019 11:50:10 +0100 Subject: [PATCH 4/8] move paths calculation out one loop --- zaza/charm_tests/security/tests.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/zaza/charm_tests/security/tests.py b/zaza/charm_tests/security/tests.py index 2a414a2..a174c60 100644 --- a/zaza/charm_tests/security/tests.py +++ b/zaza/charm_tests/security/tests.py @@ -45,9 +45,12 @@ def _add_tests(): for name, attributes in files.items(): # Lets make sure to only add tests for deployed applications if name in deployed_applications: + paths = [ + file['path'] for + file in attributes['files'] + if "*" not in file["path"] + ] for file in attributes['files']: - paths = [file['path'] for file in attributes['files']] - paths = [path for path in paths if path[-1] is not "*"] test_func = _make_test_function(name, file, paths=paths) setattr( cls, From fdffe20a8a3491d3293bd02cfe05f63b985c8b12 Mon Sep 17 00:00:00 2001 From: Chris MacNaughton Date: Wed, 30 Jan 2019 11:59:44 +0100 Subject: [PATCH 5/8] remove dynamic function default --- zaza/charm_tests/security/tests.py | 2 +- zaza/utilities/file_assertions.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/zaza/charm_tests/security/tests.py b/zaza/charm_tests/security/tests.py index a174c60..fd6db82 100644 --- a/zaza/charm_tests/security/tests.py +++ b/zaza/charm_tests/security/tests.py @@ -26,7 +26,7 @@ from zaza.utilities.file_assertions import ( ) -def _make_test_function(application, file_details, paths=[]): +def _make_test_function(application, file_details, paths=None): def test(self): for unit in model.get_units(application): unit = unit.entity_id diff --git a/zaza/utilities/file_assertions.py b/zaza/utilities/file_assertions.py index 470fcd2..5993f81 100644 --- a/zaza/utilities/file_assertions.py +++ b/zaza/utilities/file_assertions.py @@ -17,8 +17,10 @@ import zaza.model as model -def assert_path_glob(test_case, unit, file_details, paths=[]): +def assert_path_glob(test_case, unit, file_details, paths=None): """Verify all files in a given directory.""" + if not paths: + paths = [] result = model.run_on_unit( unit, 'bash -c "' 'shopt -s -q globstar; ' From 97edfa0433a79b4827fad50ec6f1284795409031 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Wed, 30 Jan 2019 16:49:31 +0000 Subject: [PATCH 6/8] Pass a version to the cinder client. Passing a version to the cinder client is mandatory *1 so supply a version and default to 2 as per existing amulet helper. *1 https://github.com/openstack/python-cinderclient/blob/master/cinderclient/client.py#L795 --- zaza/utilities/openstack.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/zaza/utilities/openstack.py b/zaza/utilities/openstack.py index b3ae064..780c71c 100644 --- a/zaza/utilities/openstack.py +++ b/zaza/utilities/openstack.py @@ -226,15 +226,17 @@ def get_octavia_session_client(session, service_type='load-balancer', endpoint=endpoint.url) -def get_cinder_session_client(session): +def get_cinder_session_client(session, version=2): """Return cinderclient authenticated by keystone session. :param session: Keystone session object :type session: keystoneauth1.session.Session object + :param version: Cinder API version + :type version: int :returns: Authenticated cinderclient :rtype: cinderclient.Client object """ - return cinderclient.Client(session=session) + return cinderclient.Client(session=session, version=version) def get_keystone_scope(): From 0a73120e0f69194a37e7fd137c39b75c93bc447b Mon Sep 17 00:00:00 2001 From: Chris MacNaughton Date: Thu, 31 Jan 2019 08:01:46 +0100 Subject: [PATCH 7/8] Add docstrings to file-assertion code --- zaza/charm_tests/security/tests.py | 12 +++++ zaza/utilities/file_assertions.py | 77 +++++++++++++++++++++++++----- 2 files changed, 76 insertions(+), 13 deletions(-) diff --git a/zaza/charm_tests/security/tests.py b/zaza/charm_tests/security/tests.py index fd6db82..bc203f6 100644 --- a/zaza/charm_tests/security/tests.py +++ b/zaza/charm_tests/security/tests.py @@ -27,6 +27,17 @@ from zaza.utilities.file_assertions import ( def _make_test_function(application, file_details, paths=None): + """Generate a test function given the specified inputs. + + :param application: Application name to assert file ownership on + :type application: str + :param file_details: Dictionary of file details to test + :type file_details: dict + :param paths: List of paths to test in this application + :type paths: Optional[list(str)] + :returns: Test function + :rtype: unittest.TestCase + """ def test(self): for unit in model.get_units(application): unit = unit.entity_id @@ -38,6 +49,7 @@ def _make_test_function(application, file_details, paths=None): def _add_tests(): + """Add tests to the unittest.TestCase.""" def class_decorator(cls): """Add tests based on input yaml to `cls`.""" files = utils.get_charm_config('./file-assertions.yaml') diff --git a/zaza/utilities/file_assertions.py b/zaza/utilities/file_assertions.py index 5993f81..ae8a33e 100644 --- a/zaza/utilities/file_assertions.py +++ b/zaza/utilities/file_assertions.py @@ -18,7 +18,19 @@ import zaza.model as model def assert_path_glob(test_case, unit, file_details, paths=None): - """Verify all files in a given directory.""" + """Verify all files in a given directory. + + :param test_case: Test case that we are asserting in + :type test_case: unittest.TestCase + :param unit: Unit name to operate on + :type unit: str + :param file_details: Dictionary with details of the file + :type file_details: dict + :param paths: list of paths that are explicitly tested + :type paths: list + :returns: Nothing + :rtype: None + """ if not paths: paths = [] result = model.run_on_unit( @@ -29,27 +41,53 @@ def assert_path_glob(test_case, unit, file_details, paths=None): for file in files.splitlines(): file, owner, group, mode = file.split() if file not in paths and file not in ['.', '..']: - verify_file(test_case, - unit, - file_details, - owner, - group, - mode, - path=file) + _verify_file(test_case, + unit, + file_details, + owner, + group, + mode, + path=file) def assert_single_file(test_case, unit, file_details): - """Verify ownership of a single file.""" + """Verify ownership of a single file. + + :param test_case: Test case that we are asserting in + :type test_case: unittest.TestCase + :param unit: Unit name to operate on + :type unit: str + :param file_details: Dictionary with details of the file + :type file_details: dict + :returns: Nothing + :rtype: None + """ result = model.run_on_unit( unit, 'stat -c "%U %G %a" {}'.format(file_details['path'])) ownership = result['Stdout'] owner, group, mode = ownership.split() - verify_file(test_case, unit, file_details, owner, group, mode) + _verify_file(test_case, unit, file_details, owner, group, mode) -def verify_file(test_case, unit, file_details, - actual_owner, actual_group, actual_mode, path=None): - """Assert file has correct permissions.""" +def _verify_file(test_case, unit, file_details, + actual_owner, actual_group, actual_mode, path=None): + """Assert file has correct permissions. + + :param test_case: Test case that we are asserting in + :type test_case: unittest.TestCase + :param unit: Unit name to operate on + :type unit: str + :param file_details: Dictionary with details of the file + :type file_details: dict + :param actual_owner: Owner of the file + :type actual_owner str + :param actual_group: Group of the file + :type actual_group str + :param actual_mode: Mode of the file + :type actual_mode str + :returns: Nothing + :rtype: None + """ expected_owner = file_details.get("owner", "root") expected_group = file_details.get("group", "root") expected_mode = file_details.get("mode", "600") @@ -65,6 +103,19 @@ def verify_file(test_case, unit, file_details, def _error_message(thing, unit, value, path=None): + """Format assertion error based on presence of path. + + :param thing: Ownership type + :type thing: str + :param unit: Unit tested + :type unit: str + :param value: Actual value from test + :type value: str + :param path: Path tested + :type path: Optional[str] + :returns: Erorr Message + :rtype: str + """ if path: return "{} is incorrect for {} on {}: {}".format( thing, path, unit, value) From 60f998be176f4db9db6e76fddb62b07299c9fe03 Mon Sep 17 00:00:00 2001 From: Chris MacNaughton Date: Fri, 1 Feb 2019 13:31:19 +0100 Subject: [PATCH 8/8] match correct PEP8 styling --- zaza/charm_tests/neutron/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zaza/charm_tests/neutron/setup.py b/zaza/charm_tests/neutron/setup.py index 306b543..be9ae72 100644 --- a/zaza/charm_tests/neutron/setup.py +++ b/zaza/charm_tests/neutron/setup.py @@ -76,7 +76,7 @@ def basic_overcloud_network(): network_config.update(generic_utils.get_undercloud_env_vars()) # Deployed model settings if (model.get_application_config('neutron-api') - .get('enable-dvr').get('value')): + .get('enable-dvr').get('value')): network_config.update({"dvr_enabled": True}) # Get keystone session