From f5dd4b80e7cd52285ffce2aa4fedab0fad2d8ff3 Mon Sep 17 00:00:00 2001 From: Ganesh Nalawade Date: Mon, 15 Mar 2021 14:30:44 +0530 Subject: [PATCH] Fix jsonschma input data validation (#50) Fix jsonschma input data validation Reviewed-by: https://github.com/apps/ansible-zuul --- .../jsonschema_data_validation_fix.yaml | 3 +++ plugins/sub_plugins/validate/jsonschema.py | 25 ++++++------------- .../files/criteria/check_list_data.json | 17 +++++++++++++ .../validate/files/data/test_list_data.json | 1 + .../validate/tasks/include/filter.yaml | 4 +-- .../validate/tasks/include/lookup.yaml | 4 +-- .../validate/tasks/include/module.yaml | 15 +++++++++-- .../targets/validate/tasks/include/test.yaml | 4 +-- tests/unit/plugins/action/test_validate.py | 8 ++---- tests/unit/plugins/filter/test_validate.py | 5 ++-- 10 files changed, 51 insertions(+), 35 deletions(-) create mode 100644 changelogs/fragments/jsonschema_data_validation_fix.yaml create mode 100644 tests/integration/targets/validate/files/criteria/check_list_data.json create mode 100644 tests/integration/targets/validate/files/data/test_list_data.json diff --git a/changelogs/fragments/jsonschema_data_validation_fix.yaml b/changelogs/fragments/jsonschema_data_validation_fix.yaml new file mode 100644 index 0000000..3c22a52 --- /dev/null +++ b/changelogs/fragments/jsonschema_data_validation_fix.yaml @@ -0,0 +1,3 @@ +--- +bugfixes: + - Fix jsonschema input data format checking (https://github.com/ansible-collections/ansible.utils/pull/50). diff --git a/plugins/sub_plugins/validate/jsonschema.py b/plugins/sub_plugins/validate/jsonschema.py index 291e61e..98697b7 100644 --- a/plugins/sub_plugins/validate/jsonschema.py +++ b/plugins/sub_plugins/validate/jsonschema.py @@ -33,8 +33,7 @@ DOCUMENTATION = """ vars: - name: ansible_validate_jsonschema_draft notes: - - The value of I(data) option should be either of type B(dict) or B(strings) which should be - a valid B(dict) when read in python. + - The value of I(data) option should be either a valid B(JSON) object or a B(JSON) string. - The value of I(criteria) should be B(list) of B(dict) or B(list) of B(strings) and each B(string) within the B(list) entry should be a valid B(dict) when read in python. """ @@ -98,19 +97,14 @@ class Validate(ValidateBase): :return: None: In case all arguments passed are valid """ try: - if isinstance(self._data, dict): - self._data = json.loads(json.dumps(self._data)) - elif isinstance(self._data, string_types): + if isinstance(self._data, string_types): self._data = json.loads(self._data) else: - msg = "Expected value of 'data' option is either dict or str, received type '{data_type}'".format( - data_type=type(self._data) - ) - raise AnsibleError(msg) + self._data = json.loads(json.dumps(self._data)) except (TypeError, JSONDecodeError) as exe: msg = ( - "'data' option value is invalid, value should of type dict or str format of dict." + "'data' option value is invalid, value should a valid JSON." " Failed to read with error '{err}'".format( err=to_text(exe, errors="surrogate_then_replace") ) @@ -120,20 +114,15 @@ class Validate(ValidateBase): try: criteria = [] for item in to_list(self._criteria): - if isinstance(item, dict): - criteria.append(json.loads(json.dumps(item))) - elif isinstance(self._criteria, string_types): + if isinstance(self._criteria, string_types): criteria.append(json.loads(item)) else: - msg = "Expected value of 'criteria' option is either list of dict/str or dict or str, received type '{criteria_type}'".format( - criteria_type=type(criteria) - ) - raise AnsibleError(msg) + criteria.append(json.loads(json.dumps(item))) self._criteria = criteria except (TypeError, JSONDecodeError) as exe: msg = ( - "'criteria' option value is invalid, value should of type dict or str format of dict." + "'criteria' option value is invalid, value should a valid JSON." " Failed to read with error '{err}'".format( err=to_text(exe, errors="surrogate_then_replace") ) diff --git a/tests/integration/targets/validate/files/criteria/check_list_data.json b/tests/integration/targets/validate/files/criteria/check_list_data.json new file mode 100644 index 0000000..d45e16c --- /dev/null +++ b/tests/integration/targets/validate/files/criteria/check_list_data.json @@ -0,0 +1,17 @@ +{ + "definitions": {}, + "$schema": "http://json-schema.org/draft-07/schema#", + "$id": "https://example.com/object1615264346.json", + "title": "Root", + "type": "array", + "default": [], + "items":{ + "$id": "#root/items", + "title": "Items", + "type": "integer", + "examples": [ + 1 + ], + "default": 0 + } +} \ No newline at end of file diff --git a/tests/integration/targets/validate/files/data/test_list_data.json b/tests/integration/targets/validate/files/data/test_list_data.json new file mode 100644 index 0000000..7660873 --- /dev/null +++ b/tests/integration/targets/validate/files/data/test_list_data.json @@ -0,0 +1 @@ +[1] diff --git a/tests/integration/targets/validate/tasks/include/filter.yaml b/tests/integration/targets/validate/tasks/include/filter.yaml index a6b636a..92d4707 100644 --- a/tests/integration/targets/validate/tasks/include/filter.yaml +++ b/tests/integration/targets/validate/tasks/include/filter.yaml @@ -48,7 +48,7 @@ - assert: that: - "result['failed'] == true" - - "'\\'data\\' option value is invalid, value should of type dict or str format of dict' in result.msg" + - "'\\'data\\' option value is invalid' in result.msg" - name: invalid criteria value ansible.builtin.set_fact: @@ -59,7 +59,7 @@ - assert: that: - "result['failed'] == true" - - "'\\'criteria\\' option value is invalid, value should of type dict or str format of dict' in result.msg" + - "'\\'criteria\\' option value is invalid' in result.msg" - name: read data and criteria from file ansible.builtin.set_fact: diff --git a/tests/integration/targets/validate/tasks/include/lookup.yaml b/tests/integration/targets/validate/tasks/include/lookup.yaml index 5f6893e..14cc377 100644 --- a/tests/integration/targets/validate/tasks/include/lookup.yaml +++ b/tests/integration/targets/validate/tasks/include/lookup.yaml @@ -66,7 +66,7 @@ - assert: that: - "result['failed'] == true" - - "'\\'data\\' option value is invalid, value should of type dict or str format of dict' in result.msg" + - "'\\'data\\' option value is invalid' in result.msg" - name: invalid criteria value ansible.builtin.set_fact: @@ -77,7 +77,7 @@ - assert: that: - "result['failed'] == true" - - "'\\'criteria\\' option value is invalid, value should of type dict or str format of dict' in result.msg" + - "'\\'criteria\\' option value is invalid' in result.msg" - name: read data and criteria from file ansible.builtin.set_fact: diff --git a/tests/integration/targets/validate/tasks/include/module.yaml b/tests/integration/targets/validate/tasks/include/module.yaml index d0d3cc3..e1b878e 100644 --- a/tests/integration/targets/validate/tasks/include/module.yaml +++ b/tests/integration/targets/validate/tasks/include/module.yaml @@ -79,7 +79,7 @@ - assert: that: - "result['failed'] == true" - - "'\\'data\\' option value is invalid, value should of type dict or str format of dict' in result.msg" + - "'\\'data\\' option value is invalid' in result.msg" - name: invalid criteria value ansible.utils.validate: @@ -92,7 +92,7 @@ - assert: that: - "result['failed'] == true" - - "'\\'criteria\\' option value is invalid, value should of type dict or str format of dict' in result.msg" + - "'\\'criteria\\' option value is invalid' in result.msg" - name: validate data using jsonschema engine (invalid data read from file) ansible.utils.validate: @@ -132,3 +132,14 @@ that: - "'errors' not in result" - "'all checks passed' in result.msg" + +- name: validate list data using jsonschema + ansible.utils.validate: + data: "{{ lookup('ansible.builtin.file', 'data/test_list_data.json') }}" + criteria: "{{ lookup('ansible.builtin.file', 'criteria/check_list_data.json') }}" + engine: ansible.utils.jsonschema + +- assert: + that: + - "'errors' not in result" + - "'all checks passed' in result.msg" diff --git a/tests/integration/targets/validate/tasks/include/test.yaml b/tests/integration/targets/validate/tasks/include/test.yaml index 5f77b45..ff1fce2 100644 --- a/tests/integration/targets/validate/tasks/include/test.yaml +++ b/tests/integration/targets/validate/tasks/include/test.yaml @@ -46,7 +46,7 @@ - assert: that: - "result['failed'] == true" - - "'\\'data\\' option value is invalid, value should of type dict or str format of dict' in result.msg" + - "'\\'data\\' option value is invalid' in result.msg" - name: invalid criteria value ansible.builtin.set_fact: @@ -57,7 +57,7 @@ - assert: that: - "result['failed'] == true" - - "'\\'criteria\\' option value is invalid, value should of type dict or str format of dict' in result.msg" + - "'\\'criteria\\' option value is invalid' in result.msg" - name: read data and criteria from file ansible.builtin.set_fact: diff --git a/tests/unit/plugins/action/test_validate.py b/tests/unit/plugins/action/test_validate.py index 98fc25e..df4103f 100644 --- a/tests/unit/plugins/action/test_validate.py +++ b/tests/unit/plugins/action/test_validate.py @@ -158,10 +158,7 @@ class TestValidate(unittest.TestCase): with self.assertRaises(AnsibleActionFail) as error: self._plugin.run(task_vars=None) - self.assertIn( - "'data' option value is invalid, value should of type dict or str format of dict", - str(error.exception), - ) + self.assertIn("'data' option value is invalid", str(error.exception)) # invalid criteria option value self._plugin._task.args = { @@ -173,8 +170,7 @@ class TestValidate(unittest.TestCase): with self.assertRaises(AnsibleActionFail) as error: self._plugin.run(task_vars=None) self.assertIn( - "'criteria' option value is invalid, value should of type dict or str format of dict", - str(error.exception), + "'criteria' option value is invalid", str(error.exception) ) def test_invalid_validate_plugin_config_options(self): diff --git a/tests/unit/plugins/filter/test_validate.py b/tests/unit/plugins/filter/test_validate.py index f9251f7..34ed9da 100644 --- a/tests/unit/plugins/filter/test_validate.py +++ b/tests/unit/plugins/filter/test_validate.py @@ -118,10 +118,9 @@ class TestValidate(unittest.TestCase): # missing required arguments with self.assertRaises(AnsibleFilterError) as error: - validate([DATA], {}) + validate([DATA]) self.assertIn( - "Expected value of 'data' option is either dict or str, received type", - str(error.exception), + "Missing either 'data' or 'criteria' value", str(error.exception) ) args = [DATA, [CRITERIA_IN_RATE_CHECK]]