Repository: incubator-airflow Updated Branches: refs/heads/master fa84d4999 -> 0bc248fc7
[AIRFLOW-940] Handle error on variable decrypt Invalid variables could break the variable view by unhandled InvalidToken exception from Fernet. This commit converts the fernet error into an AirflowException, given that fernet is loaded dynamically. Also, the exception is handled in the VariableView by showing the token "INVALID" in the UI render. Closes #2510 from edgarRd/erod-error-handling-var- decrypt Project: http://git-wip-us.apache.org/repos/asf/incubator-airflow/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-airflow/commit/0bc248fc Tree: http://git-wip-us.apache.org/repos/asf/incubator-airflow/tree/0bc248fc Diff: http://git-wip-us.apache.org/repos/asf/incubator-airflow/diff/0bc248fc Branch: refs/heads/master Commit: 0bc248fc7aa51f5890e550a63fcb28eb427f55a3 Parents: fa84d49 Author: Edgar Rodriguez <edgar.rodrig...@airbnb.com> Authored: Thu Aug 10 17:26:05 2017 -0700 Committer: Alex Guziel <alex.guz...@airbnb.com> Committed: Thu Aug 10 17:26:05 2017 -0700 ---------------------------------------------------------------------- airflow/models.py | 7 +++++- airflow/www/views.py | 5 +++- tests/www/test_views.py | 58 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/0bc248fc/airflow/models.py ---------------------------------------------------------------------- diff --git a/airflow/models.py b/airflow/models.py index 35a09ef..c15491c 100755 --- a/airflow/models.py +++ b/airflow/models.py @@ -3831,7 +3831,12 @@ class Variable(Base): raise AirflowException( "Can't decrypt _val for key={}, FERNET_KEY configuration \ missing".format(self.key)) - return fernet.decrypt(bytes(self._val, 'utf-8')).decode() + try: + return fernet.decrypt(bytes(self._val, 'utf-8')).decode() + except: + raise AirflowException( + "Can't decrypt _val for key={}, invalid token or value" + .format(self.key)) else: return self._val http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/0bc248fc/airflow/www/views.py ---------------------------------------------------------------------- diff --git a/airflow/www/views.py b/airflow/www/views.py index 07e0e18..f8dffb9 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -2283,7 +2283,10 @@ class VariableView(wwwutils.DataProfilingMixin, AirflowModelView): def hidden_field_formatter(view, context, model, name): if wwwutils.should_hide_value_for_key(model.key): return Markup('*' * 8) - return getattr(model, name) + try: + return getattr(model, name) + except AirflowException: + return Markup('<span class="label label-danger">Invalid</span>') form_columns = ( 'key', http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/0bc248fc/tests/www/test_views.py ---------------------------------------------------------------------- diff --git a/tests/www/test_views.py b/tests/www/test_views.py index 5c0eefa..f20dca1 100644 --- a/tests/www/test_views.py +++ b/tests/www/test_views.py @@ -82,6 +82,64 @@ class TestChartModelView(unittest.TestCase): self.assertIn('Sort by Owner', response.data.decode('utf-8')) +class TestVariableView(unittest.TestCase): + + CREATE_ENDPOINT = '/admin/variable/new/?url=/admin/variable/' + + @classmethod + def setUpClass(cls): + super(TestVariableView, cls).setUpClass() + session = Session() + session.query(models.Variable).delete() + session.commit() + session.close() + + def setUp(self): + super(TestVariableView, self).setUp() + configuration.load_test_config() + app = application.create_app(testing=True) + app.config['WTF_CSRF_METHODS'] = [] + self.app = app.test_client() + self.session = Session() + self.variable = { + 'key': 'test_key', + 'val': 'text_val', + 'is_encrypted': True + } + + def tearDown(self): + self.session.query(models.Variable).delete() + self.session.commit() + self.session.close() + super(TestVariableView, self).tearDown() + + def test_can_handle_error_on_decrypt(self): + # create valid variable + response = self.app.post( + self.CREATE_ENDPOINT, + data=self.variable, + follow_redirects=True, + ) + self.assertEqual(response.status_code, 200) + + # update the variable with a wrong value, given that is encrypted + Var = models.Variable + (self.session.query(Var) + .filter(Var.key == self.variable['key']) + .update({ + 'val': 'failed_value_not_encrypted' + }, synchronize_session=False)) + self.session.commit() + + # retrieve Variables page, should not fail and contain the Invalid + # label for the variable + response = self.app.get('/admin/variable', follow_redirects=True) + self.assertEqual(response.status_code, 200) + self.assertEqual(self.session.query(models.Variable).count(), 1) + self.assertIn('<span class="label label-danger">Invalid</span>', + response.data.decode('utf-8')) + + class TestKnownEventView(unittest.TestCase): CREATE_ENDPOINT = '/admin/knownevent/new/?url=/admin/knownevent/'
