From 69d74c1a66d1eab260caaa1017ecdbd446cf5263 Mon Sep 17 00:00:00 2001 From: Jeremy Freudberg Date: Wed, 9 Jan 2019 19:24:34 -0500 Subject: [PATCH] Some polish for APIv2 - update_keypair now only in v2 schema - tenant_id->project_id in cluster provision steps - tenant_id->project_id in referenced job binaries in job templates - proper check for job template existence, to fail early (as intended) - hadoop_version->plugin_version for query string filter - unbreak some data source stuff (related to tenant_id->project_id) - fix omission of show_progress from cluster GET query string whitelist - job_id->job_template_id for v2 jobs - add missing release note info for strict query string checking - release notes for all the rest Change-Id: Idea117c406b5ab9b8d85ccf8adb175053416d6ff Story: 2004505 Task: 28822 --- .../some-polish-api-v2-2d2e390a74b088f9.yaml | 12 ++++++++++ ...idation-query-string-a6cadbf2f9c57d06.yaml | 5 ++++ sahara/api/v2/cluster_templates.py | 8 +++++-- sahara/api/v2/clusters.py | 23 +++++++++++++++---- sahara/api/v2/data_sources.py | 8 +++---- sahara/api/v2/job_templates.py | 22 ++++++++++++++---- sahara/api/v2/job_types.py | 9 +++++--- sahara/api/v2/jobs.py | 9 ++++++++ sahara/api/v2/node_group_templates.py | 8 +++++-- sahara/service/validations/clusters_schema.py | 8 ++++--- 10 files changed, 90 insertions(+), 22 deletions(-) create mode 100644 releasenotes/notes/some-polish-api-v2-2d2e390a74b088f9.yaml create mode 100644 releasenotes/notes/strict-validation-query-string-a6cadbf2f9c57d06.yaml diff --git a/releasenotes/notes/some-polish-api-v2-2d2e390a74b088f9.yaml b/releasenotes/notes/some-polish-api-v2-2d2e390a74b088f9.yaml new file mode 100644 index 0000000000..b6f8a49f20 --- /dev/null +++ b/releasenotes/notes/some-polish-api-v2-2d2e390a74b088f9.yaml @@ -0,0 +1,12 @@ +--- +other: + - Some polishings to APIv2 have been made in an effort to bring it from + experimental (and therefore, evolving and unpredictable) to stable. More + instances of `tenant_id` have been changed to `project_id`, in the + cluster and job template APIs. `job_id` was changed to `job_template_id` + in the job API. The newly-minted query string validation feature has been + fixed to allow `show_progress` as a parameter on cluster GET; on a similar + note some APIv2 endpoints which previously could be filtered by + `hadoop_version` are now filtered by `plugin_version` instead. Also, the + schema for cluster PATCH in APIv1.1 now no longer includes the key + `update_keypair`; its prior inclusion was a mistake. diff --git a/releasenotes/notes/strict-validation-query-string-a6cadbf2f9c57d06.yaml b/releasenotes/notes/strict-validation-query-string-a6cadbf2f9c57d06.yaml new file mode 100644 index 0000000000..64eac63efb --- /dev/null +++ b/releasenotes/notes/strict-validation-query-string-a6cadbf2f9c57d06.yaml @@ -0,0 +1,5 @@ +--- +other: + - In APIv2 there is now strict checking of parameters in the query string. + This means that unexpected values in the query string will give a 400 + error (as opposed to previously being ignored, or causing a 500 error). diff --git a/sahara/api/v2/cluster_templates.py b/sahara/api/v2/cluster_templates.py index bfcc8955a0..b7ca523022 100644 --- a/sahara/api/v2/cluster_templates.py +++ b/sahara/api/v2/cluster_templates.py @@ -29,9 +29,13 @@ 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']) +@v.validate_request_params(['plugin_name', 'plugin_version', 'name']) def cluster_templates_list(): - result = api.get_cluster_templates(**u.get_request_args().to_dict()) + request_args = u.get_request_args().to_dict() + if 'plugin_version' in request_args: + request_args['hadoop_version'] = request_args['plugin_version'] + del request_args['plugin_version'] + result = api.get_cluster_templates(**request_args) for ct in result: u._replace_hadoop_version_plugin_version(ct) u._replace_tenant_id_project_id(ct) diff --git a/sahara/api/v2/clusters.py b/sahara/api/v2/clusters.py index 007e441da3..649dd63902 100644 --- a/sahara/api/v2/clusters.py +++ b/sahara/api/v2/clusters.py @@ -27,16 +27,28 @@ import sahara.utils.api as u rest = u.RestV2('clusters', __name__) +def _replace_tenant_id_project_id_provision_steps(c): + if 'provision_progress' in c: + for step in c['provision_progress']: + dict.update(step, {'project_id': step['tenant_id']}) + dict.pop(step, 'tenant_id') + + @rest.get('/clusters') @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']) +@v.validate_request_params(['plugin_name', 'plugin_version', 'name']) def clusters_list(): - result = api.get_clusters(**u.get_request_args().to_dict()) + request_args = u.get_request_args().to_dict() + if 'plugin_version' in request_args: + request_args['hadoop_version'] = request_args['plugin_version'] + del request_args['plugin_version'] + result = api.get_clusters(**request_args) for c in result: u._replace_hadoop_version_plugin_version(c) u._replace_tenant_id_project_id(c) + _replace_tenant_id_project_id_provision_steps(c) return u.render(res=result, name='clusters') @@ -73,13 +85,14 @@ def clusters_scale(cluster_id, data): api.scale_cluster, cluster_id, data) u._replace_hadoop_version_plugin_version(result['cluster']) u._replace_tenant_id_project_id(result['cluster']) + _replace_tenant_id_project_id_provision_steps(result['cluster']) return u.render(result) @rest.get('/clusters/') @acl.enforce("data-processing:clusters:get") @v.check_exists(api.get_cluster, 'cluster_id') -@v.validate_request_params([]) +@v.validate_request_params(['show_progress']) def clusters_get(cluster_id): data = u.get_request_args() show_events = six.text_type( @@ -88,19 +101,21 @@ def clusters_get(cluster_id): api.get_cluster, cluster_id, show_events) u._replace_hadoop_version_plugin_version(result['cluster']) u._replace_tenant_id_project_id(result['cluster']) + _replace_tenant_id_project_id_provision_steps(result['cluster']) return u.render(result) @rest.patch('/clusters/') @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(v_c_schema.CLUSTER_UPDATE_SCHEMA_V2, 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) u._replace_hadoop_version_plugin_version(result['cluster']) u._replace_tenant_id_project_id(result['cluster']) + _replace_tenant_id_project_id_provision_steps(result['cluster']) return u.render(result) diff --git a/sahara/api/v2/data_sources.py b/sahara/api/v2/data_sources.py index 476254aa3b..4d56402540 100644 --- a/sahara/api/v2/data_sources.py +++ b/sahara/api/v2/data_sources.py @@ -52,9 +52,9 @@ def data_source_register(data): @v.check_exists(api.get_data_source, 'data_source_id') @v.validate_request_params([]) def data_source_get(data_source_id): - result = u.to_wrapped_dict(api.get_data_source, data_source_id) + result = api.get_data_source(data_source_id).to_wrapped_dict() u._replace_tenant_id_project_id(result['data_source']) - return result + return u.render(result) @rest.delete('/data-sources/') @@ -72,6 +72,6 @@ def data_source_delete(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): - result = u.to_wrapped_dict(api.data_source_update, data_source_id, data) + result = api.data_source_update(data_source_id, data).to_wrapped_dict() u._replace_tenant_id_project_id(result['data_source']) - return result + return u.render(result) diff --git a/sahara/api/v2/job_templates.py b/sahara/api/v2/job_templates.py index 6ac5ebc91d..f9d5b1a82f 100644 --- a/sahara/api/v2/job_templates.py +++ b/sahara/api/v2/job_templates.py @@ -24,9 +24,15 @@ import sahara.utils.api as u rest = u.RestV2('job-templates', __name__) +def _replace_tenant_id_project_id_job_binary(jb_list): + for jb_obj in jb_list: + dict.update(jb_obj, {'project_id': jb_obj['tenant_id']}) + dict.pop(jb_obj, 'tenant_id') + + @rest.get('/job-templates') @acl.enforce("data-processing:job-templates:get_all") -@v.check_exists(api.get_job_templates, 'marker') +@v.check_exists(api.get_job_template, 'marker') @v.validate(None, v.validate_pagination_limit, v.validate_sorting_jobs) @v.validate_request_params(['type', 'name']) @@ -34,6 +40,8 @@ def job_templates_list(): result = api.get_job_templates(**u.get_request_args().to_dict()) for jt in result: u._replace_tenant_id_project_id(jt) + _replace_tenant_id_project_id_job_binary(jt['mains']) + _replace_tenant_id_project_id_job_binary(jt['libs']) return u.render(res=result, name='job_templates') @@ -44,35 +52,41 @@ def job_templates_list(): def job_templates_create(data): result = {'job_template': api.create_job_template(data).to_dict()} u._replace_tenant_id_project_id(result['job_template']) + _replace_tenant_id_project_id_job_binary(result['job_template']['mains']) + _replace_tenant_id_project_id_job_binary(result['job_template']['libs']) return u.render(result) @rest.get('/job-templates/') @acl.enforce("data-processing:job-templates:get") -@v.check_exists(api.get_job_templates, id='job_templates_id') +@v.check_exists(api.get_job_template, id='job_templates_id') @v.validate_request_params([]) def job_templates_get(job_templates_id): result = {'job_template': api.get_job_template( job_templates_id).to_dict()} u._replace_tenant_id_project_id(result['job_template']) + _replace_tenant_id_project_id_job_binary(result['job_template']['mains']) + _replace_tenant_id_project_id_job_binary(result['job_template']['libs']) return u.render(result) @rest.patch('/job-templates/') @acl.enforce("data-processing:jobs:modify") -@v.check_exists(api.get_job_templates, id='job_templates_id') +@v.check_exists(api.get_job_template, id='job_templates_id') @v.validate(v_j_schema.JOB_UPDATE_SCHEMA) @v.validate_request_params([]) def job_templates_update(job_templates_id, data): result = {'job_template': api.update_job_template( job_templates_id, data).to_dict()} u._replace_tenant_id_project_id(result['job_template']) + _replace_tenant_id_project_id_job_binary(result['job_template']['mains']) + _replace_tenant_id_project_id_job_binary(result['job_template']['libs']) return u.render(result) @rest.delete('/job-templates/') @acl.enforce("data-processing:jobs:delete") -@v.check_exists(api.get_job_templates, id='job_templates_id') +@v.check_exists(api.get_job_template, id='job_templates_id') @v.validate_request_params([]) def job_templates_delete(job_templates_id): api.delete_job_template(job_templates_id) diff --git a/sahara/api/v2/job_types.py b/sahara/api/v2/job_types.py index 5c8141636d..c0cf914e5a 100644 --- a/sahara/api/v2/job_types.py +++ b/sahara/api/v2/job_types.py @@ -24,10 +24,13 @@ 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']) +@v.validate_request_params(['type', 'plugin_name', 'plugin_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 # filters of the form ?type=Pig&type=Java, etc. - return u.render(job_types=api.get_job_types( - **u.get_request_args().to_dict(flat=False))) + request_args = u.get_request_args().to_dict(flat=False) + if 'plugin_version' in request_args: + request_args['hadoop_version'] = request_args['plugin_version'] + del request_args['plugin_version'] + return u.render(job_types=api.get_job_types(**request_args)) diff --git a/sahara/api/v2/jobs.py b/sahara/api/v2/jobs.py index 7db59c1db8..1ba1ad9763 100644 --- a/sahara/api/v2/jobs.py +++ b/sahara/api/v2/jobs.py @@ -26,6 +26,11 @@ import sahara.utils.api as u rest = u.RestV2('jobs', __name__) +def _replace_job_id_job_template_id(job_obj): + dict.update(job_obj, {'job_template_id': job_obj['job_id']}) + dict.pop(job_obj, 'job_id') + + @rest.get('/jobs') @acl.enforce("data-processing:job-executions:get_all") @v.check_exists(api.get_job_execution, 'marker') @@ -40,6 +45,7 @@ def jobs_list(): for je in result: je.pop('oozie_job_id', force=True) u._replace_tenant_id_project_id(je) + _replace_job_id_job_template_id(je) return u.render(res=result, name='jobs') @@ -53,6 +59,7 @@ def jobs_execute(data): {'engine_job_id': result['job']['oozie_job_id']}) dict.pop(result['job'], 'oozie_job_id') u._replace_tenant_id_project_id(result['job']) + _replace_job_id_job_template_id(result['job']) return u.render(result) @@ -67,6 +74,7 @@ def jobs_get(job_id): result = {'job': api.get_job_execution(job_id, refresh_status)} result['job'].pop('oozie_job_id', force=True) u._replace_tenant_id_project_id(result['job']) + _replace_job_id_job_template_id(result['job']) return u.render(result) @@ -80,6 +88,7 @@ def jobs_update(job_id, data): result = {'job': api.update_job_execution(job_id, data)} result['job'].pop('oozie_job_id', force=True) u._replace_tenant_id_project_id(result['job']) + _replace_job_id_job_template_id(result['job']) return u.render(result) diff --git a/sahara/api/v2/node_group_templates.py b/sahara/api/v2/node_group_templates.py index 00cc4354a8..42498ff156 100644 --- a/sahara/api/v2/node_group_templates.py +++ b/sahara/api/v2/node_group_templates.py @@ -30,9 +30,13 @@ 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']) +@v.validate_request_params(['plugin_name', 'plugin_version', 'name']) def node_group_templates_list(): - result = api.get_node_group_templates(**u.get_request_args().to_dict()) + request_args = u.get_request_args().to_dict() + if 'plugin_version' in request_args: + request_args['hadoop_version'] = request_args['plugin_version'] + del request_args['plugin_version'] + result = api.get_node_group_templates(**request_args) for ngt in result: u._replace_hadoop_version_plugin_version(ngt) u._replace_tenant_id_project_id(ngt) diff --git a/sahara/service/validations/clusters_schema.py b/sahara/service/validations/clusters_schema.py index 77e0419c54..904bb3158f 100644 --- a/sahara/service/validations/clusters_schema.py +++ b/sahara/service/validations/clusters_schema.py @@ -71,9 +71,6 @@ CLUSTER_UPDATE_SCHEMA = { "description": { "type": ["string", "null"] }, - "update_keypair": { - "type": ["boolean", "null"] - }, "name": { "type": "string", "minLength": 1, @@ -99,6 +96,11 @@ CLUSTER_UPDATE_SCHEMA = { "additionalProperties": False, "required": [] } +CLUSTER_UPDATE_SCHEMA_V2 = copy.deepcopy(CLUSTER_UPDATE_SCHEMA) +CLUSTER_UPDATE_SCHEMA_V2['properties'].update({ + "update_keypair": { + "type": ["boolean", "null"] + }}) CLUSTER_SCALING_SCHEMA = { "type": "object",