Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix genconfig does not fail when schema validation failed #425

Merged
merged 5 commits into from
Mar 27, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion iotedgedev/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
7 changes: 5 additions & 2 deletions iotedgedev/deploymentmanifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exception happens and the validation is still success?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only schema failures should be treated as validation failure. Other exceptions such as network error should not fail the validation. iotedgedev will only print related error and skip validation.


Expand Down Expand Up @@ -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:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line 189 has if error_detected?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

besides it already has error_detected, do we still need validation_success?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the error_detected and reuse validation_success. We need to declare validation_success outside of try to handle situations with exception.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed error_detected? we have two blocks of error_detected (189 and 194)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the commit that removes error_detected, I removed both of them and use validation_success instead: 60cac66

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
Expand All @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[bumpversion]
current_version = 2.1.2
current_version = 2.1.3
commit = True
tag = True

Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
102 changes: 102 additions & 0 deletions tests/assets/deployment.manifest_invalid_createoptions.json
Original file line number Diff line number Diff line change
@@ -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
}
}
}
}
}
69 changes: 69 additions & 0 deletions tests/assets/deployment.manifest_invalid_schema.json
Original file line number Diff line number Diff line change
@@ -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
}
}
}
}
}
19 changes: 11 additions & 8 deletions tests/test_iotedgedev.py
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down