From 7105a891c23ca066d87f68e07a7098ea692ca112 Mon Sep 17 00:00:00 2001 From: Telles Nobrega Date: Thu, 6 Dec 2018 16:42:55 -0300 Subject: [PATCH] APIv2 - Fix 500 on malformed query string on In order to improve return information to clients we are adding a check to verify parameters before each call and return a more appropriate message to the users. Change-Id: I9923601d0903e415a3fe30bec9bdc8fc34b91ff6 Story: #2004506 Task: #28228 --- sahara/api/v2/cluster_templates.py | 6 ++++++ sahara/api/v2/clusters.py | 6 ++++++ sahara/api/v2/data_sources.py | 5 +++++ sahara/api/v2/images.py | 7 +++++++ sahara/api/v2/job_binaries.py | 6 ++++++ sahara/api/v2/job_templates.py | 6 ++++++ sahara/api/v2/job_types.py | 2 ++ sahara/api/v2/jobs.py | 5 +++++ sahara/api/v2/node_group_templates.py | 6 ++++++ sahara/api/v2/plugins.py | 4 ++++ sahara/service/validation.py | 19 +++++++++++++++++++ sahara/utils/api.py | 12 ++++++++++++ 12 files changed, 84 insertions(+) diff --git a/sahara/api/v2/cluster_templates.py b/sahara/api/v2/cluster_templates.py index fd9f7d4fd7..449a2cca97 100644 --- a/sahara/api/v2/cluster_templates.py +++ b/sahara/api/v2/cluster_templates.py @@ -29,6 +29,7 @@ rest = u.RestV2('cluster-templates', __name__) @v.check_exists(api.get_cluster_template, 'marker') @v.validate(None, v.validate_pagination_limit, v.validate_sorting_cluster_templates) +@v.validate_request_params(['plugin_name', 'hadoop_version', 'name']) def cluster_templates_list(): result = api.get_cluster_templates(**u.get_request_args().to_dict()) for ct in result: @@ -40,6 +41,7 @@ def cluster_templates_list(): @acl.enforce("data-processing:cluster-templates:create") @v.validate(ct_schema.CLUSTER_TEMPLATE_SCHEMA_V2, v_ct.check_cluster_template_create) +@v.validate_request_params([]) def cluster_templates_create(data): # renaming hadoop_version -> plugin_version # this can be removed once APIv1 is deprecated @@ -53,6 +55,7 @@ def cluster_templates_create(data): @rest.get('/cluster-templates/') @acl.enforce("data-processing:cluster-templates:get") @v.check_exists(api.get_cluster_template, 'cluster_template_id') +@v.validate_request_params([]) def cluster_templates_get(cluster_template_id): result = u.to_wrapped_dict_no_render( api.get_cluster_template, cluster_template_id) @@ -65,6 +68,7 @@ def cluster_templates_get(cluster_template_id): @v.check_exists(api.get_cluster_template, 'cluster_template_id') @v.validate(ct_schema.CLUSTER_TEMPLATE_UPDATE_SCHEMA_V2, v_ct.check_cluster_template_update) +@v.validate_request_params([]) def cluster_templates_update(cluster_template_id, data): if data.get('plugin_version', None): data['hadoop_version'] = data['plugin_version'] @@ -79,6 +83,7 @@ def cluster_templates_update(cluster_template_id, data): @acl.enforce("data-processing:cluster-templates:delete") @v.check_exists(api.get_cluster_template, 'cluster_template_id') @v.validate(None, v_ct.check_cluster_template_usage) +@v.validate_request_params([]) def cluster_templates_delete(cluster_template_id): api.terminate_cluster_template(cluster_template_id) return u.render() @@ -97,6 +102,7 @@ def _cluster_template_export_helper(template): @rest.get('/cluster-templates//export') @acl.enforce("data-processing:cluster-templates:get") @v.check_exists(api.get_cluster_template, 'cluster_template_id') +@v.validate_request_params([]) def cluster_template_export(cluster_template_id): content = u.to_wrapped_dict_no_render( api.export_cluster_template, cluster_template_id) diff --git a/sahara/api/v2/clusters.py b/sahara/api/v2/clusters.py index 3d6381747f..9e38595652 100644 --- a/sahara/api/v2/clusters.py +++ b/sahara/api/v2/clusters.py @@ -31,6 +31,7 @@ rest = u.RestV2('clusters', __name__) @acl.enforce("data-processing:clusters:get_all") @v.check_exists(api.get_cluster, 'marker') @v.validate(None, v.validate_pagination_limit) +@v.validate_request_params(['plugin_name', 'hadoop_version', 'name']) def clusters_list(): result = api.get_clusters(**u.get_request_args().to_dict()) for c in result: @@ -42,6 +43,7 @@ def clusters_list(): @acl.enforce("data-processing:clusters:create") @v.validate(v_c_schema.CLUSTER_SCHEMA_V2, v_c.check_one_or_multiple_clusters_create) +@v.validate_request_params([]) def clusters_create(data): # renaming hadoop_version -> plugin_version # this can be removed once APIv1 is deprecated @@ -62,6 +64,7 @@ def clusters_create(data): @acl.enforce("data-processing:clusters:scale") @v.check_exists(api.get_cluster, 'cluster_id') @v.validate(v_c_schema.CLUSTER_SCALING_SCHEMA_V2, v_c_s.check_cluster_scaling) +@v.validate_request_params([]) def clusters_scale(cluster_id, data): result = u.to_wrapped_dict_no_render( api.scale_cluster, cluster_id, data) @@ -72,6 +75,7 @@ def clusters_scale(cluster_id, data): @rest.get('/clusters/') @acl.enforce("data-processing:clusters:get") @v.check_exists(api.get_cluster, 'cluster_id') +@v.validate_request_params([]) def clusters_get(cluster_id): data = u.get_request_args() show_events = six.text_type( @@ -86,6 +90,7 @@ def clusters_get(cluster_id): @acl.enforce("data-processing:clusters:modify") @v.check_exists(api.get_cluster, 'cluster_id') @v.validate(v_c_schema.CLUSTER_UPDATE_SCHEMA, v_c.check_cluster_update) +@v.validate_request_params([]) def clusters_update(cluster_id, data): result = u.to_wrapped_dict_no_render( api.update_cluster, cluster_id, data) @@ -97,6 +102,7 @@ def clusters_update(cluster_id, data): @acl.enforce("data-processing:clusters:delete") @v.check_exists(api.get_cluster, 'cluster_id') @v.validate(v_c_schema.CLUSTER_DELETE_SCHEMA_V2, v_c.check_cluster_delete) +@v.validate_request_params([]) def clusters_delete(cluster_id): data = u.request_data() force = data.get('force', False) diff --git a/sahara/api/v2/data_sources.py b/sahara/api/v2/data_sources.py index 2781bc3cc4..028458481c 100644 --- a/sahara/api/v2/data_sources.py +++ b/sahara/api/v2/data_sources.py @@ -29,6 +29,7 @@ rest = u.RestV2('data-sources', __name__) @v.check_exists(api.get_data_source, 'marker') @v.validate(None, v.validate_pagination_limit, v.validate_sorting_data_sources) +@v.validate_request_params(['type']) def data_sources_list(): result = api.get_data_sources(**u.get_request_args().to_dict()) return u.render(res=result, name='data_sources') @@ -37,6 +38,7 @@ def data_sources_list(): @rest.post('/data-sources') @acl.enforce("data-processing:data-sources:register") @v.validate(v_d_s_schema.DATA_SOURCE_SCHEMA, v_d_s.check_data_source_create) +@v.validate_request_params([]) def data_source_register(data): return u.render(api.register_data_source(data).to_wrapped_dict()) @@ -44,6 +46,7 @@ def data_source_register(data): @rest.get('/data-sources/') @acl.enforce("data-processing:data-sources:get") @v.check_exists(api.get_data_source, 'data_source_id') +@v.validate_request_params([]) def data_source_get(data_source_id): return u.to_wrapped_dict(api.get_data_source, data_source_id) @@ -51,6 +54,7 @@ def data_source_get(data_source_id): @rest.delete('/data-sources/') @acl.enforce("data-processing:data-sources:delete") @v.check_exists(api.get_data_source, 'data_source_id') +@v.validate_request_params([]) def data_source_delete(data_source_id): api.delete_data_source(data_source_id) return u.render() @@ -60,5 +64,6 @@ def data_source_delete(data_source_id): @acl.enforce("data-processing:data-sources:modify") @v.check_exists(api.get_data_source, 'data_source_id') @v.validate(v_d_s_schema.DATA_SOURCE_UPDATE_SCHEMA) +@v.validate_request_params([]) def data_source_update(data_source_id, data): return u.to_wrapped_dict(api.data_source_update, data_source_id, data) diff --git a/sahara/api/v2/images.py b/sahara/api/v2/images.py index 1d3d6f9fc3..1cced90d19 100644 --- a/sahara/api/v2/images.py +++ b/sahara/api/v2/images.py @@ -25,6 +25,7 @@ rest = u.RestV2('images', __name__) @rest.get('/images') @acl.enforce("data-processing:images:get_all") +@v.validate_request_params(['name', 'tags', 'username']) def images_list(): tags = u.get_request_args().getlist('tags') name = u.get_request_args().get('name', None) @@ -34,6 +35,7 @@ def images_list(): @rest.get('/images/') @acl.enforce("data-processing:images:get") @v.check_exists(api.get_image, id='image_id') +@v.validate_request_params([]) def images_get(image_id): return u.render(api.get_registered_image(id=image_id).wrapped_dict) @@ -42,6 +44,7 @@ def images_get(image_id): @acl.enforce("data-processing:images:register") @v.check_exists(api.get_image, id='image_id') @v.validate(v_images.image_register_schema, v_images.check_image_register) +@v.validate_request_params([]) def images_set(image_id, data): return u.render(api.register_image(image_id, **data).wrapped_dict) @@ -49,6 +52,7 @@ def images_set(image_id, data): @rest.delete('/images/') @acl.enforce("data-processing:images:unregister") @v.check_exists(api.get_image, id='image_id') +@v.validate_request_params([]) def images_unset(image_id): api.unregister_image(image_id) return u.render() @@ -57,6 +61,7 @@ def images_unset(image_id): @rest.get('/images//tags') @acl.enforce("data-processing:images:get_tags") @v.check_exists(api.get_image, id='image_id') +@v.validate_request_params([]) def image_tags_get(image_id): return u.render(api.get_image_tags(image_id)) @@ -65,6 +70,7 @@ def image_tags_get(image_id): @acl.enforce("data-processing:images:set_tags") @v.check_exists(api.get_image, id='image_id') @v.validate(v_images.image_tags_schema, v_images.check_tags) +@v.validate_request_params([]) def image_tags_update(image_id, data): return u.render(api.set_image_tags(image_id, **data).wrapped_dict) @@ -72,6 +78,7 @@ def image_tags_update(image_id, data): @rest.delete('/images//tags') @acl.enforce("data-processing:images:remove_tags") @v.check_exists(api.get_image, id='image_id') +@v.validate_request_params([]) def image_tags_delete(image_id): api.remove_image_tags(image_id) return u.render() diff --git a/sahara/api/v2/job_binaries.py b/sahara/api/v2/job_binaries.py index ac201fc95c..3e9e4865e7 100644 --- a/sahara/api/v2/job_binaries.py +++ b/sahara/api/v2/job_binaries.py @@ -27,6 +27,7 @@ rest = u.RestV2('job-binaries', __name__) @rest.post('/job-binaries') @acl.enforce("data-processing:job-binaries:create") @v.validate(v_j_b_schema.JOB_BINARY_SCHEMA, v_j_b.check_job_binary) +@v.validate_request_params([]) def job_binary_create(data): return u.render(api.create_job_binary(data).to_wrapped_dict()) @@ -36,6 +37,7 @@ def job_binary_create(data): @v.check_exists(api.get_job_binary, 'marker') @v.validate(None, v.validate_pagination_limit, v.validate_sorting_job_binaries) +@v.validate_request_params(['name']) def job_binary_list(): result = api.get_job_binaries(**u.get_request_args().to_dict()) return u.render(res=result, name='binaries') @@ -44,6 +46,7 @@ def job_binary_list(): @rest.get('/job-binaries/') @acl.enforce("data-processing:job-binaries:get") @v.check_exists(api.get_job_binary, 'job_binary_id') +@v.validate_request_params([]) def job_binary_get(job_binary_id): return u.to_wrapped_dict(api.get_job_binary, job_binary_id) @@ -51,6 +54,7 @@ def job_binary_get(job_binary_id): @rest.delete('/job-binaries/') @acl.enforce("data-processing:job-binaries:delete") @v.check_exists(api.get_job_binary, id='job_binary_id') +@v.validate_request_params([]) def job_binary_delete(job_binary_id): api.delete_job_binary(job_binary_id) return u.render() @@ -59,6 +63,7 @@ def job_binary_delete(job_binary_id): @rest.get('/job-binaries//data') @acl.enforce("data-processing:job-binaries:get_data") @v.check_exists(api.get_job_binary, 'job_binary_id') +@v.validate_request_params([]) def job_binary_data(job_binary_id): data = api.get_job_binary_data(job_binary_id) if type(data) == dict: @@ -69,6 +74,7 @@ def job_binary_data(job_binary_id): @rest.patch('/job-binaries/') @acl.enforce("data-processing:job-binaries:modify") @v.validate(v_j_b_schema.JOB_BINARY_UPDATE_SCHEMA, v_j_b.check_job_binary) +@v.validate_request_params([]) def job_binary_update(job_binary_id, data): return u.render(api.update_job_binary(job_binary_id, data).to_wrapped_dict()) diff --git a/sahara/api/v2/job_templates.py b/sahara/api/v2/job_templates.py index c1e268eb03..dcdf9c59d7 100644 --- a/sahara/api/v2/job_templates.py +++ b/sahara/api/v2/job_templates.py @@ -29,6 +29,7 @@ rest = u.RestV2('job-templates', __name__) @v.check_exists(api.get_job_templates, 'marker') @v.validate(None, v.validate_pagination_limit, v.validate_sorting_jobs) +@v.validate_request_params(['type', 'name']) def job_templates_list(): result = api.get_job_templates(**u.get_request_args().to_dict()) return u.render(res=result, name='job_templates') @@ -37,6 +38,7 @@ def job_templates_list(): @rest.post('/job-templates') @acl.enforce("data-processing:job-templates:create") @v.validate(v_j_schema.JOB_SCHEMA, v_j.check_mains_libs, v_j.check_interface) +@v.validate_request_params([]) def job_templates_create(data): return u.render({'job_template': api.create_job_template(data).to_dict()}) @@ -44,6 +46,7 @@ def job_templates_create(data): @rest.get('/job-templates/') @acl.enforce("data-processing:job-templates:get") @v.check_exists(api.get_job_templates, id='job_templates_id') +@v.validate_request_params([]) def job_templates_get(job_templates_id): return u.render({'job_template': api.get_job_template( job_templates_id).to_dict()}) @@ -53,6 +56,7 @@ def job_templates_get(job_templates_id): @acl.enforce("data-processing:jobs:modify") @v.check_exists(api.get_job_templates, id='job_templates_id') @v.validate(v_j_schema.JOB_UPDATE_SCHEMA) +@v.validate_request_params([]) def job_templates_update(job_templates_id, data): return u.render({'job_template': api.update_job_template( job_templates_id, data).to_dict()}) @@ -61,6 +65,7 @@ def job_templates_update(job_templates_id, data): @rest.delete('/job-templates/') @acl.enforce("data-processing:jobs:delete") @v.check_exists(api.get_job_templates, id='job_templates_id') +@v.validate_request_params([]) def job_templates_delete(job_templates_id): api.delete_job_template(job_templates_id) return u.render() @@ -69,5 +74,6 @@ def job_templates_delete(job_templates_id): @rest.get('/job-templates/config-hints/') @acl.enforce("data-processing:jobs:get_config_hints") @v.check_exists(api.get_job_config_hints, job_type='job_type') +@v.validate_request_params([]) def job_config_hints_get(job_type): return u.render(api.get_job_config_hints(job_type)) diff --git a/sahara/api/v2/job_types.py b/sahara/api/v2/job_types.py index aa9ef9eaef..5c8141636d 100644 --- a/sahara/api/v2/job_types.py +++ b/sahara/api/v2/job_types.py @@ -15,6 +15,7 @@ from sahara.api import acl from sahara.service.api.v2 import job_types as api +from sahara.service import validation as v import sahara.utils.api as u @@ -23,6 +24,7 @@ rest = u.RestV2('job-types', __name__) @rest.get('/job-types') @acl.enforce("data-processing:job-types:get_all") +@v.validate_request_params(['type', 'plugin_name', 'hadoop_version']) def job_types_get(): # We want to use flat=False with to_dict() so that # the value of each arg is given as a list. This supports diff --git a/sahara/api/v2/jobs.py b/sahara/api/v2/jobs.py index 3c4dcfc8cd..a9e41d4cd9 100644 --- a/sahara/api/v2/jobs.py +++ b/sahara/api/v2/jobs.py @@ -31,6 +31,7 @@ rest = u.RestV2('jobs', __name__) @v.check_exists(api.get_job_execution, 'marker') @v.validate(None, v.validate_pagination_limit, v.validate_sorting_job_executions) +@v.validate_request_params(['status']) def jobs_list(): result = api.job_execution_list(**u.get_request_args().to_dict()) # APIv2: renaming oozie_job_id -> engine_job_id @@ -44,6 +45,7 @@ def jobs_list(): @rest.post('/jobs') @acl.enforce("data-processing:jobs:execute") @v.validate(v_j_e_schema.JOB_EXEC_SCHEMA_V2, v_j_e.check_job_execution) +@v.validate_request_params([]) def jobs_execute(data): result = {'job': api.execute_job(data)} dict.update(result['job'], @@ -55,6 +57,7 @@ def jobs_execute(data): @rest.get('/jobs/') @acl.enforce("data-processing:job-executions:get") @v.check_exists(api.get_job_execution, id='job_id') +@v.validate_request_params([]) def jobs_get(job_id): data = u.get_request_args() refresh_status = six.text_type( @@ -69,6 +72,7 @@ def jobs_get(job_id): @v.check_exists(api.get_job_execution, id='job_id') @v.validate( v_j_e_schema.JOB_EXEC_UPDATE_SCHEMA, v_j_e.check_job_execution_update) +@v.validate_request_params([]) def jobs_update(job_id, data): result = {'job': api.update_job_execution(job_id, data)} result['job'].pop('oozie_job_id', force=True) @@ -79,6 +83,7 @@ def jobs_update(job_id, data): @acl.enforce("data-processing:job-executions:delete") @v.check_exists(api.get_job_execution, id='job_id') @v.validate(None, v_j_e.check_job_execution_delete) +@v.validate_request_params([]) def jobs_delete(job_id): api.delete_job_execution(job_id) return u.render() diff --git a/sahara/api/v2/node_group_templates.py b/sahara/api/v2/node_group_templates.py index c0b20d0b4a..17e0c838a2 100644 --- a/sahara/api/v2/node_group_templates.py +++ b/sahara/api/v2/node_group_templates.py @@ -30,6 +30,7 @@ rest = u.RestV2('node-group-templates', __name__) @v.check_exists(api.get_node_group_template, 'marker') @v.validate(None, v.validate_pagination_limit, v.validate_sorting_node_group_templates) +@v.validate_request_params(['plugin_name', 'hadoop_version', 'name']) def node_group_templates_list(): result = api.get_node_group_templates(**u.get_request_args().to_dict()) for ngt in result: @@ -41,6 +42,7 @@ def node_group_templates_list(): @acl.enforce("data-processing:node-group-templates:create") @v.validate(ngt_schema.NODE_GROUP_TEMPLATE_SCHEMA_V2, v_ngt.check_node_group_template_create) +@v.validate_request_params([]) def node_group_templates_create(data): # renaming hadoop_version -> plugin_version # this can be removed once APIv1 is deprecated @@ -54,6 +56,7 @@ def node_group_templates_create(data): @rest.get('/node-group-templates/') @acl.enforce("data-processing:node-group-templates:get") @v.check_exists(api.get_node_group_template, 'node_group_template_id') +@v.validate_request_params([]) def node_group_templates_get(node_group_template_id): result = u.to_wrapped_dict_no_render( api.get_node_group_template, node_group_template_id) @@ -66,6 +69,7 @@ def node_group_templates_get(node_group_template_id): @v.check_exists(api.get_node_group_template, 'node_group_template_id') @v.validate(ngt_schema.NODE_GROUP_TEMPLATE_UPDATE_SCHEMA_V2, v_ngt.check_node_group_template_update) +@v.validate_request_params([]) def node_group_templates_update(node_group_template_id, data): if data.get('plugin_version', None): data['hadoop_version'] = data['plugin_version'] @@ -80,6 +84,7 @@ def node_group_templates_update(node_group_template_id, data): @acl.enforce("data-processing:node-group-templates:delete") @v.check_exists(api.get_node_group_template, 'node_group_template_id') @v.validate(None, v_ngt.check_node_group_template_usage) +@v.validate_request_params([]) def node_group_templates_delete(node_group_template_id): api.terminate_node_group_template(node_group_template_id) return u.render() @@ -100,6 +105,7 @@ def _node_group_template_export_helper(template): @rest.get('/node-group-templates//export') @acl.enforce("data-processing:node-group-templates:get") @v.check_exists(api.get_node_group_template, 'node_group_template_id') +@v.validate_request_params([]) def node_group_template_export(node_group_template_id): content = u.to_wrapped_dict_no_render( api.export_node_group_template, node_group_template_id) diff --git a/sahara/api/v2/plugins.py b/sahara/api/v2/plugins.py index f6f9434ca9..e9ef4d8414 100644 --- a/sahara/api/v2/plugins.py +++ b/sahara/api/v2/plugins.py @@ -25,6 +25,7 @@ rest = u.RestV2('plugins', __name__) @rest.get('/plugins') @acl.enforce("data-processing:plugins:get_all") +@v.validate_request_params([]) def plugins_list(): return u.render(plugins=[p.dict for p in api.get_plugins()]) @@ -32,6 +33,7 @@ def plugins_list(): @rest.get('/plugins/') @acl.enforce("data-processing:plugins:get") @v.check_exists(api.get_plugin, plugin_name='plugin_name') +@v.validate_request_params([]) def plugins_get(plugin_name): return u.render(api.get_plugin(plugin_name).wrapped_dict) @@ -39,6 +41,7 @@ def plugins_get(plugin_name): @rest.get('/plugins//') @acl.enforce("data-processing:plugins:get_version") @v.check_exists(api.get_plugin, plugin_name='plugin_name', version='version') +@v.validate_request_params([]) def plugins_get_version(plugin_name, version): return u.render(api.get_plugin(plugin_name, version).wrapped_dict) @@ -47,5 +50,6 @@ def plugins_get_version(plugin_name, version): @acl.enforce("data-processing:plugins:patch") @v.check_exists(api.get_plugin, plugin_name='plugin_name') @v.validate(v_p.plugin_update_validation_jsonschema(), v_p.check_plugin_update) +@v.validate_request_params([]) def plugins_update(plugin_name, data): return u.render(api.update_plugin(plugin_name, data).wrapped_dict) diff --git a/sahara/service/validation.py b/sahara/service/validation.py index d0e9a10ffe..038d60a37b 100644 --- a/sahara/service/validation.py +++ b/sahara/service/validation.py @@ -198,3 +198,22 @@ def check_exists(get_func, *id_prop, **get_args): return handler return decorator + + +def validate_request_params(supported_params): + def decorator(func): + @functools.wraps(func) + def handler(*args, **kwargs): + pagination_params = ['marker', 'limit', 'sort_by'] + func_name = func.__name__ + params = u.get_request_args() + for param in params.keys(): + if (param not in supported_params and + param not in pagination_params): + return u.invalid_param_error( + 400, + 'The only valid params for %s are %s and %s' % ( + func_name, supported_params, pagination_params)) + return func(*args, **kwargs) + return handler + return decorator diff --git a/sahara/utils/api.py b/sahara/utils/api.py index 847264fceb..5be0d98d72 100644 --- a/sahara/utils/api.py +++ b/sahara/utils/api.py @@ -288,6 +288,18 @@ def render_error_message(error_code, error_message, error_name): return resp +def invalid_param_error(status_code, descr, exc=None): + LOG.error("Request aborted with status code {code} and " + "message '{message}'".format(code=status_code, message=descr)) + + if exc is not None: + LOG.error(traceback.format_exc()) + + error_code = "INVALID_PARAMS_ON_REQUEST" + + return render_error_message(status_code, descr, error_code) + + def internal_error(status_code, descr, exc=None): LOG.error("Request aborted with status code {code} and " "message '{message}'".format(code=status_code, message=descr))