Fix chartbuilder get_files raising UnicodeDecodeError

This adds better exception handling ang logging to
_append_file_to_result helper in get_files. When reading
arbitrary file data and attempting to encode to utf-8
this can cause UnicodeDecodeError to be raised.

However, Armada will not skip over such files; it will
raise an exception with appropriate details instead.

Closes #195
Closes #196

Change-Id: Id7c32c17e351d1ffe042e3755c116c36b6380223
This commit is contained in:
Felipe Monteiro 2018-03-20 15:49:13 +00:00
parent 6339efcfd8
commit 950c6012ab
6 changed files with 121 additions and 10 deletions

View File

@ -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

View File

@ -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

View File

@ -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()

View File

@ -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-

View File

@ -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:

View File

@ -15,6 +15,7 @@ install_command = pip install {opts} {packages}
whitelist_externals =
find
flake8
rm
commands =
find . -type f -name "*.pyc" -delete
python -V