From fea790c6d91c663a546cdbc41c5ea10ad93bad56 Mon Sep 17 00:00:00 2001 From: Chaoyi Yuan Date: Fri, 13 Mar 2020 13:49:49 +0800 Subject: [PATCH 1/5] Fix bug that validation not fail when only schema error exists --- iotedgedev/deploymentmanifest.py | 7 +- ...oyment.manifest_invalid_createoptions.json | 102 ++++++++++++++++++ .../deployment.manifest_invalid_schema.json | 69 ++++++++++++ tests/test_iotedgedev.py | 19 ++-- 4 files changed, 187 insertions(+), 10 deletions(-) create mode 100644 tests/assets/deployment.manifest_invalid_createoptions.json create mode 100644 tests/assets/deployment.manifest_invalid_schema.json diff --git a/iotedgedev/deploymentmanifest.py b/iotedgedev/deploymentmanifest.py index f6059293..52e8c8ca 100644 --- a/iotedgedev/deploymentmanifest.py +++ b/iotedgedev/deploymentmanifest.py @@ -146,7 +146,7 @@ def validate_deployment_template(self): validation_success = True try: template_schema = json.loads(urlopen(Constants.deployment_template_schema_url).read().decode()) - self._validate_json_schema(template_schema, self.json, "Deployment template") + validation_success = self._validate_json_schema(template_schema, self.json, "Deployment template") except Exception as ex: # Ignore other non shcema validation errors self.output.info("Unexpected error during deployment template schema validation, skip schema validation. Error:%s" % ex) @@ -190,6 +190,9 @@ def _validate_json_schema(self, schema_object, json_object, schema_type): self.output.warning("%s schema validation failed. Please see previous logs for more details" % schema_type) else: self.output.info("%s schema validation passed." % schema_type) + + if error_detected: + validation_success = False except jsonschema.exceptions.SchemaError as schemaErr: self.output.info("Errors found in %s schema, skip schema validation. Error:%s" % (schema_type, schemaErr.message)) except Exception as ex: # Ignore other non schema validation errors @@ -201,7 +204,7 @@ def _validate_deployment_manifest_schema(self): validation_success = True try: deployment_schema = json.loads(urlopen(Constants.deployment_manifest_schema_url).read()) - self._validate_json_schema(deployment_schema, self.json, "Deployment manifest") + validation_success = self._validate_json_schema(deployment_schema, self.json, "Deployment manifest") except Exception as ex: # Ignore other non schema validation errors self.output.info("Unexpected error during deployment manifest schema validation, skip schema validation. Error:%s" % ex) diff --git a/tests/assets/deployment.manifest_invalid_createoptions.json b/tests/assets/deployment.manifest_invalid_createoptions.json new file mode 100644 index 00000000..9fc8f00b --- /dev/null +++ b/tests/assets/deployment.manifest_invalid_createoptions.json @@ -0,0 +1,102 @@ +{ + "modulesContent": { + "$edgeAgent": { + "properties.desired": { + "schemaVersion": "1.0", + "runtime": { + "type": "docker", + "settings": { + "minDockerVersion": "v1.25", + "loggingOptions": "", + "registryCredentials": { + "test": { + "username": "$USERNAME", + "password": "$PASSWORD", + "address": "docker.io" + } + } + } + }, + "systemModules": { + "edgeAgent": { + "type": "docker", + "settings": { + "image": "mcr.microsoft.com/azureiotedge-agent:1.0" + } + }, + "edgeHub": { + "type": "docker", + "status": "running", + "restartPolicy": "always", + "settings": { + "image": "mcr.microsoft.com/azureiotedge-hub:1.0", + "createOptions": "{\"HostConfig\":{\"PortBindings\":{\"5671/tcp\":[{\"HostPort\":\"5671\"}],\"8883/tcp\":[{\"HostPort\":\"8883\"}],\"443/tcp\":[{\"HostPort\":\"443\"}]}}}" + } + } + }, + "modules": { + "tempSensor": { + "version": "1.0", + "type": "docker", + "status": "running", + "restartPolicy": "always", + "settings": { + "image": "mcr.microsoft.com/azureiotedge-simulated-temperature-sensor:1.0", + "createOptions": "{\"Env\":[\"abcdefghij0=00\",\"abcdefghij1=01\",\"abcdefghij2=02\",\"abcdefghij3=03\",\"abcdefghij4=04\",\"abcdefghij5=05\",\"abcdefghij6=06\",\"abcdefghij7=07\",\"abcdefghij8=08\",\"abcdefghij9=09\",\"abcdefghij10=10\",\"abcdefghij11=11\",\"abcdefghij12=12\",\"abcdefghij13=13\",\"abcdefghij14=14\",\"abcdefghij15=15\",\"abcdefghij16=16\",\"abcdefghij17=17\",\"abcdefghij18=18\",\"abcdefghij19=19\",\"abcdefghij20=20\",\"abcdefghij22=21\",\"abcdefghij22=22\",\"abcdefghij23=23\",\"abcdefghij24=24\",\"abcdefghij25=25\",\"abcdefghij26=26\",\"abcdefghij27=27\",\"abcdefghi", + "createOptions01": "j28=28\",\"abcdefghij29=29\",\"abcdefghij30=30\",\"abcdefghij31=31\",\"abcdefghij32=32\",\"abcdefghij33=33\",\"abcdefghij34=34\",\"abcdefghij35=35\",\"abcdefghij36=36\",\"abcdefghij37=37\",\"abcdefghij38=38\",\"abcdefghij39=39\",\"abcdefghij40=40\",\"abcdefghij41=41\",\"abcdefghij42=42\",\"abcdefghij43=43\",\"abcdefghij44=44\",\"abcdefghij45=45\",\"abcdefghij46=46\",\"abcdefghij47=47\",\"abcdefghij48=48\",\"abcdefghij49=49\",\"abcdefghij50=50\",\"abcdefghij51=51\",\"abcdefghij52=52\",\"abcdefghij53=53\",\"abcdefghij54=54\",\"abcdefghij55=55\",\"abcdefghij56abcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefgh=56\",", + "createOptions02": "\"abcdefghij56abcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefghabcdefgh=57\",\"abcdefghij58=58\",\"abcdefghij59=59\"]}" + } + }, + "csharpmodule": { + "version": "1.0", + "type": "docker", + "status": "running", + "restartPolicy": "always", + "settings": { + "image": "${MODULES.csharpmodule.amd64}", + "createOptions": "{\"Env\":[\"abcdefghij0=00\",\"abcdefghij1=01\",\"abcdefghij2=02\",\"abcdefghij3=03\",\"abcdefghij4=04\",\"abcdefghij5=05\",\"abcdefghij6=06\",\"abcdefghij7=07\",\"abcdefghij8=08\",\"abcdefghij9=09\",\"abcdefghij10=10\",\"abcdefghij11=11\",\"abcdefghij12=12\",\"abcdefghij13=13\",\"abcdefghij14=14\",\"abcdefghij15=15\",\"abcdefghij16=16\",\"abcdefghij17=17\",\"abcdefghij18=18\",\"abcdefghij19=19\",\"abcdefghij20=20\",\"abcdefghij22=21\",\"abcdefghij22=22\",\"abcdefghij23=23\",\"abcdefghij24=24\",\"abcdefghij25=25\",\"abcdefghij26=26\",\"abcdefghij27=27\",\"abcdefghi", + "createOptions01": "j28=28\",\"abcdefghij29=29\",\"abcdefghij30=30\",\"abcdefghij31=31\",\"abcdefghij32=32\",\"abcdefghij33=33\",\"abcdefghij34=34\",\"abcdefghij35=35\",\"abcdefghij36=36\",\"abcdefghij37=37\",\"abcdefghij38=38\",\"abcdefghij39=39\",\"abcdefghij40=40\",\"abcdefghij41=41\",\"abcdefghij42=42\",\"abcdefghij43=43\",\"abcdefghij44=44\",\"abcdefghij45=45\",\"abcdefghij46=46\",\"abcdefghij47=47\",\"abcdefghij48=48\",\"abcdefghij49=49\",\"abcdefghij50=50\",\"abcdefghij51=51\",\"abcdefghij52=52\",\"abcdefghij53=53\",\"abcdefghij54=54\",\"abcdefghij55=55\",\"abcdefghij56=56\",", + "createOptions03": "\"abcdefghij57=57\",\"abcdefghij58=58\",\"abcdefghij59=59\"]}" + } + }, + "csharpfunction": { + "version": "1.0", + "type": "docker", + "status": "running", + "restartPolicy": "always", + "settings": { + "image": "${MODULES.csharpfunction.amd64.debug}", + "createOptions": "[1,2,3]" + } + }, + "csharpfunction2": { + "version": "1.0", + "type": "docker", + "status": "running", + "restartPolicy": "always", + "settings": { + "image": "${MODULES.csharpfunction2.amd64.debug}", + "createOptions": "{\"Env\":[\"abcdefghij0=00\",\"abcdefghij1=01\",\"abcdefghij2=02\",\"abcdefghij3=03\",\"abcdefghij4=04\",\"abcdefghij5=05\",\"abcdefghij6=06\",\"abcdefghij7=07\",\"abcdefghij8=08\",\"abcdefghij9=09\",\"abcdefghij10=10\",\"abcdefghij11=11\",\"abcdefghij12=12\",\"abcdefghij13=13\",\"abcdefghij14=14\",\"abcdefghij15=15\",\"abcdefghij16=16\",\"abcdefghij17=17\",\"abcdefghij18=18\",\"abcdefghij19=19\",\"abcdefghij20=20\",\"abcdefghij22=21\",\"abcdefghij22=22\",\"abcdefghij23=23\",\"abcdefghij24=24\",\"abcdefghij25=25\",\"abcdefghij26=26\",\"abcdefghij27=27\",\"abcdefghi", + "createOptions01": "j28=28\",\"abcdefghij29=29\",\"abcdefghij30=30\",\"abcdefghij31=31\",\"abcdefghij32=32\",\"abcdefghij33=33\",\"abcdefghij34=34\",\"abcdefghij35=35\",\"abcdefghij36=36\",\"abcdefghij37=37\",\"abcdefghij38=38\",\"abcdefghij39=39\",\"abcdefghij40=40\",\"abcdefghij41=41\",\"abcdefghij42=42\",\"abcdefghij43=43\",\"abcdefghij44=44\",\"abcdefghij45=45\",\"abcdefghij46=46\",\"abcdefghij47=47\",\"abcdefghij48=48\",\"abcdefghij49=49\",\"abcdefghij50=50\",\"abcdefghij51=51\",\"abcdefghij52=52\",\"abcdefghij53=53\",\"abcdefghij54=54\",\"abcdefghij55=55\",\"abcdefghij56=56\",", + "createOptions03": "\"abcdefghij57=57\",\"abcdefghij58=58\",\"abcdefghij59=59\"]}", + "createOptions04": {"test": "value"} + } + } + } + } + }, + "$edgeHub": { + "properties.desired": { + "schemaVersion": "1.0", + "routes": { + "sensorTocsharpmodule": "FROM /messages/modules/tempSensor/outputs/temperatureOutput INTO BrokeredEndpoint(\"/modules/csharpmodule/inputs/input1\")", + "csharpmoduleToIoTHub": "FROM /messages/modules/csharpmodule/outputs/* INTO $upstream", + "csharpfunctionToIoTHub": "FROM /messages/modules/csharpfunction/outputs/* INTO $upstream" + }, + "storeAndForwardConfiguration": { + "timeToLiveSecs": 7200 + } + } + } + } + } \ No newline at end of file diff --git a/tests/assets/deployment.manifest_invalid_schema.json b/tests/assets/deployment.manifest_invalid_schema.json new file mode 100644 index 00000000..5f4a8ad4 --- /dev/null +++ b/tests/assets/deployment.manifest_invalid_schema.json @@ -0,0 +1,69 @@ +{ + "modulesContent": { + "$edgeAgent": { + "properties.desired": { + "schemaVersion": "1.0", + "runtime": { + "type": "docker", + "settings": { + "minDockerVersion": "v1.25", + "loggingOptions": "", + "registryCredentials": { + "test": { + "username": 1, + "password": "pwd" + }, + "test2": { + "username": "$USERNAME", + "password": "$PASSWORD", + "address": "" + } + } + } + }, + "systemModules": { + "edgeAgent": { + "type": "docker", + "settings": { + "image": "mcr.microsoft.com/azureiotedge-agent:1.0" + } + }, + "edgeHub": { + "type": "docker", + "status": "running", + "restartPolicy": "always", + "settings": { + "image": "mcr.microsoft.com/azureiotedge-hub:1.0", + "createOptions": "{\"HostConfig\":{\"PortBindings\":{\"5671/tcp\":[{\"HostPort\":\"5671\"}],\"8883/tcp\":[{\"HostPort\":\"8883\"}],\"443/tcp\":[{\"HostPort\":\"443\"}]}}}" + } + } + }, + "modules": { + "tempSensor": { + "version": "1.0", + "type": "docker", + "status": "running", + "restartPolicy": "always", + "settings": { + "image": "mcr.microsoft.com/azureiotedge-simulated-temperature-sensor:1.0", + "createOptions": "{}" + } + } + } + } + }, + "$edgeHub": { + "properties.desired": { + "schemaVersion": "1.0", + "routes": { + "sensorTocsharpmodule": "FROM /messages/modules/tempSensor/outputs/temperatureOutput INTO BrokeredEndpoint(\"/modules/csharpmodule/inputs/input1\")", + "csharpmoduleToIoTHub": "FROM /messages/modules/csharpmodule/outputs/* INTO $upstream", + "csharpfunctionToIoTHub": "FROM /messages/modules/csharpfunction/outputs/* INTO $upstream" + }, + "storeAndForwardConfiguration": { + "timeToLiveSecs": 7200 + } + } + } + } + } \ No newline at end of file diff --git a/tests/test_iotedgedev.py b/tests/test_iotedgedev.py index ad84075c..0fe309f0 100644 --- a/tests/test_iotedgedev.py +++ b/tests/test_iotedgedev.py @@ -549,14 +549,17 @@ def test_validate_create_options_failed(): def test_fail_gen_config_on_validation_error(): os.chdir(tests_assets_dir) - deployment_file_name = "deployment.template_invalidresult.json" - try: - if get_docker_os_type() == "windows": - result = runner_invoke(['genconfig', '-P', get_platform_type(), '-f', deployment_file_name, '--fail-on-validation-error']) - else: - result = runner_invoke(['genconfig', '-f', deployment_file_name, '--fail-on-validation-error']) - except Exception as err: - assert "ERROR: Deployment manifest validation failed. Please see previous logs for more details." in "%s" % err + test_files = ["deployment.manifest_invalid.json", "deployment.manifest_invalid_schema.json", "deployment.manifest_invalid_createoptions.json"] + for deployment_file_name in test_files: + try: + if get_docker_os_type() == "windows": + result = runner_invoke(['genconfig', '-P', get_platform_type(), '-f', deployment_file_name, '--fail-on-validation-error']) + else: + result = runner_invoke(['genconfig', '-f', deployment_file_name, '--fail-on-validation-error']) + raise Exception("genconfig command should fail in %s" % deployment_file_name) + except Exception as err: + assert "ERROR: Deployment manifest validation failed. Please see previous logs for more details." in "%s" % err + assert "genconfig command should fail" not in "%s" % err if get_docker_os_type() == "windows": result = runner_invoke(['genconfig', '-P', get_platform_type(), '-f', deployment_file_name]) From 38fde7eeedef5a8738b175c47f8c7c36f4e979eb Mon Sep 17 00:00:00 2001 From: Chaoyi Yuan Date: Fri, 13 Mar 2020 13:57:51 +0800 Subject: [PATCH 2/5] Add changelog and bump version --- CHANGELOG.md | 4 ++++ iotedgedev/__init__.py | 2 +- setup.cfg | 2 +- setup.py | 2 +- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index af50d754..e2b82bce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,10 @@ # Changelog All notable changes to this project since 0.82.0 will be documented in this file. +## [2.1.3] +### Changed +- Fix genconfig does not fail when schema validation failed. [[#424](https://github.com/Azure/iotedgedev/issues/424)] + ## [2.1.2] - 2020-01-14 ### Changed - Fix error when install on Azure Pipelines agent diff --git a/iotedgedev/__init__.py b/iotedgedev/__init__.py index 1369ebde..26d4fb86 100644 --- a/iotedgedev/__init__.py +++ b/iotedgedev/__init__.py @@ -4,5 +4,5 @@ __author__ = 'Microsoft Corporation' __email__ = 'vsciet@microsoft.com' -__version__ = '2.1.2' +__version__ = '2.1.3' __AIkey__ = '95b20d64-f54f-4de3-8ad5-165a75a6c6fe' diff --git a/setup.cfg b/setup.cfg index 235c1662..2caf0f88 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 2.1.2 +current_version = 2.1.3 commit = True tag = True diff --git a/setup.py b/setup.py index 10e36c7c..29f561b1 100644 --- a/setup.py +++ b/setup.py @@ -60,7 +60,7 @@ def run(self): setup( name='iotedgedev', - version='2.1.2', + version='2.1.3', description='The Azure IoT Edge Dev Tool greatly simplifies the IoT Edge development process by automating many routine manual tasks, such as building, deploying, pushing modules and configuring the IoT Edge Runtime.', long_description='See https://github.com/azure/iotedgedev for usage instructions.', author='Microsoft Corporation', From 60cac6655901b22fad8de6336616c1e28e042373 Mon Sep 17 00:00:00 2001 From: Chaoyi Yuan Date: Fri, 13 Mar 2020 15:28:48 +0800 Subject: [PATCH 3/5] resolve comment --- iotedgedev/deploymentmanifest.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/iotedgedev/deploymentmanifest.py b/iotedgedev/deploymentmanifest.py index 52e8c8ca..6c1f19b6 100644 --- a/iotedgedev/deploymentmanifest.py +++ b/iotedgedev/deploymentmanifest.py @@ -182,17 +182,14 @@ def _validate_json_schema(self, schema_object, json_object, schema_type): validator_class = jsonschema.validators.validator_for(schema_object) validator = validator_class(schema_object) validation_errors = validator.iter_errors(self.json) - error_detected = False for error in validation_errors: - error_detected = True + validation_success = False self.output.warning("%s schema error: %s. Property path:%s" % (schema_type, error.message, "->".join(error.path))) - if error_detected: - self.output.warning("%s schema validation failed. Please see previous logs for more details" % schema_type) - else: + if validation_success: self.output.info("%s schema validation passed." % schema_type) + else: + self.output.warning("%s schema validation failed. Please see previous logs for more details" % schema_type) - if error_detected: - validation_success = False except jsonschema.exceptions.SchemaError as schemaErr: self.output.info("Errors found in %s schema, skip schema validation. Error:%s" % (schema_type, schemaErr.message)) except Exception as ex: # Ignore other non schema validation errors From 92ead9041b91d250e0e95f179a0012efc1543fb1 Mon Sep 17 00:00:00 2001 From: Chaoyi Yuan Date: Mon, 23 Mar 2020 22:49:09 +0800 Subject: [PATCH 4/5] Remove postinstall step which installs az cli --- setup.py | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/setup.py b/setup.py index 29f561b1..626a3f8a 100644 --- a/setup.py +++ b/setup.py @@ -9,22 +9,6 @@ from setuptools.command.install import install -def _execute(): - check_call('pip install azure-cli --no-deps'.split()) - - -class PostInstall(install): - def run(self): - atexit.register(_execute) - install.run(self) - - -class PostDevelop(develop): - def run(self): - atexit.register(_execute) - develop.run(self) - - with open('CHANGELOG.md') as history_file: history = history_file.read() @@ -91,6 +75,5 @@ def run(self): ], test_suite='tests', tests_require=test_requirements, - setup_requires=setup_requirements, - cmdclass={'install': PostInstall, 'develop': PostDevelop} + setup_requires=setup_requirements ) From 25e2f3f41eb65c4b61f28b6b0dacbbad35964b7f Mon Sep 17 00:00:00 2001 From: Chaoyi Yuan Date: Fri, 27 Mar 2020 11:15:33 +0800 Subject: [PATCH 5/5] Update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e2b82bce..cf5a3f20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ # Changelog All notable changes to this project since 0.82.0 will be documented in this file. -## [2.1.3] +## [2.1.3] - 2020-03-27 ### Changed - Fix genconfig does not fail when schema validation failed. [[#424](https://github.com/Azure/iotedgedev/issues/424)]