diff --git a/armada/exceptions/chartbuilder_exceptions.py b/armada/exceptions/chartbuilder_exceptions.py index 4c0d7674..4a87ae3e 100644 --- a/armada/exceptions/chartbuilder_exceptions.py +++ b/armada/exceptions/chartbuilder_exceptions.py @@ -18,7 +18,7 @@ from armada.exceptions import base_exception class ChartBuilderException(base_exception.ArmadaBaseException): '''Base class for the Chartbuilder handler exception and error handling.''' - message = 'An unknown Armada handler error occured.' + message = 'An unknown Armada handler error occurred.' class DependencyException(ChartBuilderException): @@ -45,14 +45,29 @@ class HelmChartBuildException(ChartBuilderException): *Coming Soon* ''' - def __init__(self, chart_name): + def __init__(self, chart_name, details): self._chart_name = chart_name - self._message = 'Failed to build Helm chart for ' + \ - self._chart_name + '.' + self._message = ('Failed to build Helm chart for {chart_name}. ' + 'Details: {details}'.format( + **{'chart_name': chart_name, + 'details': details})) super(HelmChartBuildException, self).__init__(self._message) +class FilesLoadException(ChartBuilderException): + ''' + Exception that occurs while trying to read a file in the chart directory. + + **Troubleshoot:** + + * Ensure that the file can be encoded to utf-8 or else it cannot be parsed. + ''' + + message = ('A %(clazz)s exception occurred while trying to read ' + 'file: %(file)s. Details:\n%(details)s') + + class IgnoredFilesLoadException(ChartBuilderException): ''' Exception that occurs when there is an error loading files contained in diff --git a/armada/handlers/chartbuilder.py b/armada/handlers/chartbuilder.py index dee3ccd8..0856e5e2 100644 --- a/armada/handlers/chartbuilder.py +++ b/armada/handlers/chartbuilder.py @@ -152,8 +152,33 @@ class ChartBuilder(object): abspath = os.path.abspath(os.path.join(root, file)) relpath = os.path.join(rel_folder_path, file) - with open(abspath, 'r') as f: - file_contents = f.read().encode('utf-8') + encodings = ('utf-8', 'latin1') + unicode_errors = [] + + for encoding in encodings: + try: + with open(abspath, 'r') as f: + file_contents = f.read().encode(encoding) + except OSError as e: + LOG.debug('Failed to open and read file %s in the helm ' + 'chart directory.', abspath) + raise chartbuilder_exceptions.FilesLoadException( + file=abspath, details=e) + except UnicodeError as e: + LOG.debug('Attempting to read %s using encoding %s.', + abspath, encoding) + msg = "(encoding=%s) %s" % (encoding, str(e)) + unicode_errors.append(msg) + else: + break + + if len(unicode_errors) == 2: + LOG.debug('Failed to read file %s in the helm chart directory.' + ' Ensure that it is encoded using utf-8.', abspath) + raise chartbuilder_exceptions.FilesLoadException( + file=abspath, clazz=unicode_errors[0].__class__.__name__, + details='\n'.join(e for e in unicode_errors)) + non_template_files.append( Any(type_url=relpath, value=file_contents)) @@ -242,9 +267,10 @@ class ChartBuilder(object): dependencies=dependencies, values=self.get_values(), files=self.get_files()) - except Exception: + except Exception as e: chart_name = self.chart.chart_name - raise chartbuilder_exceptions.HelmChartBuildException(chart_name) + raise chartbuilder_exceptions.HelmChartBuildException( + chart_name, details=e) self._helm_chart = helm_chart return helm_chart diff --git a/armada/tests/unit/handlers/test_chartbuilder.py b/armada/tests/unit/handlers/test_chartbuilder.py index fc90bbd3..5100784c 100644 --- a/armada/tests/unit/handlers/test_chartbuilder.py +++ b/armada/tests/unit/handlers/test_chartbuilder.py @@ -28,7 +28,7 @@ from armada.handlers.chartbuilder import ChartBuilder from armada.exceptions import chartbuilder_exceptions -class ChartBuilderTestCase(testtools.TestCase): +class BaseChartBuilderTestCase(testtools.TestCase): chart_yaml = """ apiVersion: v1 description: A sample Helm chart for Kubernetes @@ -121,6 +121,9 @@ class ChartBuilderTestCase(testtools.TestCase): self.addCleanup(shutil.rmtree, subdir) return subdir + +class ChartBuilderTestCase(BaseChartBuilderTestCase): + def test_source_clone(self): # Create a temporary directory with Chart.yaml that contains data # from ``self.chart_yaml``. @@ -177,6 +180,17 @@ class ChartBuilderTestCase(testtools.TestCase): key=lambda x: x.type_url) self.assertEqual(expected_files, repr(actual_files).strip()) + def test_get_files_with_unicode_characters(self): + chart_dir = self.useFixture(fixtures.TempDir()) + self.addCleanup(shutil.rmtree, chart_dir.path) + for filename in ['foo', 'bar', 'Chart.yaml', 'values.yaml']: + self._write_temporary_file_contents( + chart_dir.path, filename, "DIRC^@^@^@^B^@^@^@×Z®<86>F.1") + + mock_chart = mock.Mock(source_dir=[chart_dir.path, '']) + chartbuilder = ChartBuilder(mock_chart) + chartbuilder.get_files() + def test_get_basic_helm_chart(self): # Before ChartBuilder is executed the `source_dir` points to a # directory that was either clone or unpacked from a tarball... pretend @@ -427,3 +441,53 @@ class ChartBuilderTestCase(testtools.TestCase): dependency-chart.*Another sample Helm chart for Kubernetes.* """).replace('\n', '').strip() self.assertRegex(repr(chartbuilder.dump()), re) + + +class ChartBuilderNegativeTestCase(BaseChartBuilderTestCase): + + def setUp(self): + super(ChartBuilderNegativeTestCase, self).setUp() + # Create an exception for testing since instantiating one manually + # is tedious. + try: + str(b'\xff', 'utf8') + except UnicodeDecodeError as e: + self.exc_to_raise = e + else: + self.fail('Failed to create an exception needed for testing.') + + def test_get_files_always_fails_to_read_binary_file_raises_exc(self): + chart_dir = self.useFixture(fixtures.TempDir()) + self.addCleanup(shutil.rmtree, chart_dir.path) + for filename in ['foo', 'bar', 'Chart.yaml', 'values.yaml']: + self._write_temporary_file_contents( + chart_dir.path, filename, "DIRC^@^@^@^B^@^@^@×Z®<86>F.1") + + mock_chart = mock.Mock(source_dir=[chart_dir.path, '']) + chartbuilder = ChartBuilder(mock_chart) + + # Confirm it failed for both encodings. + error_re = (r'.*A str exception occurred while trying to read file:' + '.*Details:\n.*\(encoding=utf-8\).*\n\(encoding=latin1\)') + with mock.patch("builtins.open", mock.mock_open(read_data="")) \ + as mock_file: + mock_file.return_value.read.side_effect = self.exc_to_raise + self.assertRaisesRegexp(chartbuilder_exceptions.FilesLoadException, + error_re, chartbuilder.get_files) + + def test_get_files_fails_once_to_read_binary_file_passes(self): + chart_dir = self.useFixture(fixtures.TempDir()) + self.addCleanup(shutil.rmtree, chart_dir.path) + files = ['foo', 'bar'] + for filename in files: + self._write_temporary_file_contents( + chart_dir.path, filename, "DIRC^@^@^@^B^@^@^@×Z®<86>F.1") + + mock_chart = mock.Mock(source_dir=[chart_dir.path, '']) + chartbuilder = ChartBuilder(mock_chart) + + side_effects = [self.exc_to_raise, "", ""] + with mock.patch("builtins.open", mock.mock_open(read_data="")) \ + as mock_file: + mock_file.return_value.read.side_effect = side_effects + chartbuilder.get_files() diff --git a/docs/source/commands/apply.rst b/docs/source/commands/apply.rst index 8be5e0f0..16ca8ac3 100644 --- a/docs/source/commands/apply.rst +++ b/docs/source/commands/apply.rst @@ -9,7 +9,7 @@ Commands Usage: armada apply [OPTIONS] FILENAME - This command install and updates charts defined in armada manifest + This command installs and updates charts defined in armada manifest The apply argument must be relative path to Armada Manifest. Executing apply command once will install all charts defined in manifest. Re- diff --git a/docs/source/operations/exceptions/chartbuilder-exceptions.inc b/docs/source/operations/exceptions/chartbuilder-exceptions.inc index eb81a064..437f55fd 100644 --- a/docs/source/operations/exceptions/chartbuilder-exceptions.inc +++ b/docs/source/operations/exceptions/chartbuilder-exceptions.inc @@ -25,6 +25,11 @@ :members: :show-inheritance: :undoc-members: + * - FilesLoadException + - .. autoexception:: armada.exceptions.chartbuilder_exceptions.FilesLoadException + :members: + :show-inheritance: + :undoc-members: * - HelmChartBuildException - .. autoexception:: armada.exceptions.chartbuilder_exceptions.HelmChartBuildException :members: diff --git a/tox.ini b/tox.ini index f63587ec..d61790e3 100644 --- a/tox.ini +++ b/tox.ini @@ -15,6 +15,7 @@ install_command = pip install {opts} {packages} whitelist_externals = find flake8 + rm commands = find . -type f -name "*.pyc" -delete python -V