From a925cc7ddb94224209d349c144e574bd501dad12 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Wed, 8 Aug 2018 15:41:40 -0400 Subject: [PATCH] Add error IDs for when an unknown blueprint is requested. This adds some fairly redundant code to the beginning of all the blueprint routes to attempt reading a commit from git for the blueprint's recipe. If it succeeds, the blueprint exists and the route can continue. Otherwise, return an error. Hopefully this doesn't slow things down too much. --- src/pylorax/api/errors.py | 7 +++++++ src/pylorax/api/v0.py | 36 +++++++++++++++++++++++++----------- tests/pylorax/test_server.py | 2 +- 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/src/pylorax/api/errors.py b/src/pylorax/api/errors.py index 823d8532..e2be52c6 100644 --- a/src/pylorax/api/errors.py +++ b/src/pylorax/api/errors.py @@ -25,5 +25,12 @@ BAD_LIMIT_OR_OFFSET = "BadLimitOrOffset" # a build that is not yet done. BUILD_IN_WRONG_STATE = "BuildInWrongState" +# Returned from the API when a blueprint name or other similar identifier is +# given that contains invalid characters. +INVALID_NAME = "InvalidName" + +# Returned from the API when a blueprint that was requested does not exist. +UNKNOWN_BLUEPRINT = "UnknownBlueprint" + # Returned from the API when a UUID that was requested does not exist. UNKNOWN_UUID = "UnknownUUID" diff --git a/src/pylorax/api/v0.py b/src/pylorax/api/v0.py index 50f34f79..6ac0e0e8 100644 --- a/src/pylorax/api/v0.py +++ b/src/pylorax/api/v0.py @@ -989,7 +989,7 @@ from pylorax.api.projects import modules_list, modules_info, ProjectsError, repo from pylorax.api.projects import get_repo_sources, delete_repo_source, source_to_repo, yum_repo_to_file_repo from pylorax.api.queue import queue_status, build_status, uuid_delete, uuid_status, uuid_info from pylorax.api.queue import uuid_tar, uuid_image, uuid_cancel, uuid_log -from pylorax.api.recipes import list_branch_files, read_recipe_commit, recipe_filename, list_commits +from pylorax.api.recipes import RecipeError, list_branch_files, read_recipe_commit, recipe_filename, list_commits from pylorax.api.recipes import recipe_from_dict, recipe_from_toml, commit_recipe, delete_recipe, revert_recipe from pylorax.api.recipes import tag_recipe_commit, recipe_diff from pylorax.api.regexes import VALID_API_STRING @@ -1012,6 +1012,15 @@ def take_limits(iterable, offset, limit): """ return iterable[offset:][:limit] +def blueprint_exists(api, branch, blueprint_name): + try: + with api.config["GITLOCK"].lock: + read_recipe_commit(api.config["GITLOCK"].repo, branch, blueprint_name) + + return True + except RecipeError: + return False + def v0_api(api): # Note that Sphinx will not generate documentations for any of these. @api.route("/api/v0/blueprints/list") @@ -1074,7 +1083,7 @@ def v0_api(api): if not ws_blueprint and not git_blueprint: # Neither blueprint, return an error - errors.append("%s: %s" % (blueprint_name, ", ".join(exceptions))) + errors.append({"id": UNKNOWN_BLUEPRINT, "msg": "%s: %s" % (blueprint_name, ", ".join(exceptions))}) elif ws_blueprint and not git_blueprint: # No git blueprint, return the workspace blueprint changes.append({"name":blueprint_name, "changed":True}) @@ -1121,6 +1130,11 @@ def v0_api(api): errors = [] for blueprint_name in [n.strip() for n in blueprint_names.split(",")]: filename = recipe_filename(blueprint_name) + + if not blueprint_exists(api, branch, blueprint_name): + errors.append({"id": UNKNOWN_BLUEPRINT, "msg": "Unknown blueprint name: %s" % blueprint_name}) + continue + try: with api.config["GITLOCK"].lock: commits = take_limits(list_commits(api.config["GITLOCK"].repo, branch, filename), offset, limit) @@ -1174,7 +1188,7 @@ def v0_api(api): branch = request.args.get("branch", "master") if VALID_API_STRING.match(branch) is None: - return jsonify(status=False, errors=["Invalid characters in branch argument"]), 400 + return jsonify(status=False, errors=[{"id": INVALID_NAME, "msg": "Invalid characters in branch argument"}]), 400 try: with api.config["GITLOCK"].lock: @@ -1221,14 +1235,14 @@ def v0_api(api): branch = request.args.get("branch", "master") if VALID_API_STRING.match(branch) is None: - return jsonify(status=False, errors=["Invalid characters in branch argument"]), 400 + return jsonify(status=False, errors=[{"id": INVALID_NAME, "msg": "Invalid characters in branch argument"}]), 400 try: with api.config["GITLOCK"].lock: workspace_delete(api.config["GITLOCK"].repo, branch, blueprint_name) except Exception as e: log.error("(v0_blueprints_delete_workspace) %s", str(e)) - return jsonify(status=False, error=[str(e)]), 400 + return jsonify(status=False, errors=[str(e)]), 400 else: return jsonify(status=True) @@ -1245,7 +1259,7 @@ def v0_api(api): branch = request.args.get("branch", "master") if VALID_API_STRING.match(branch) is None: - return jsonify(status=False, errors=["Invalid characters in branch argument"]), 400 + return jsonify(status=False, errors=[{"id": INVALID_NAME, "msg": "Invalid characters in branch argument"}]), 400 try: with api.config["GITLOCK"].lock: @@ -1271,7 +1285,7 @@ def v0_api(api): branch = request.args.get("branch", "master") if VALID_API_STRING.match(branch) is None: - return jsonify(status=False, errors=["Invalid characters in branch argument"]), 400 + return jsonify(status=False, errors=[{"id": INVALID_NAME, "msg": "Invalid characters in branch argument"}]), 400 try: with api.config["GITLOCK"].lock: @@ -1298,7 +1312,7 @@ def v0_api(api): branch = request.args.get("branch", "master") if VALID_API_STRING.match(branch) is None: - return jsonify(status=False, errors=["Invalid characters in branch argument"]), 400 + return jsonify(status=False, errors=[{"id": INVALID_NAME, "msg": "Invalid characters in branch argument"}]), 400 try: if from_commit == "NEWEST": @@ -1372,7 +1386,7 @@ def v0_api(api): # No blueprint found, skip it. if not blueprint: - errors.append("%s: blueprint_not_found" % (blueprint_name)) + errors.append({"id": UNKNOWN_BLUEPRINT, "msg": "%s: blueprint_not_found" % blueprint_name}) continue # Combine modules and packages and depsolve the list @@ -1432,7 +1446,7 @@ def v0_api(api): # No blueprint found, skip it. if not blueprint: - errors.append("%s: blueprint not found" % blueprint_name) + errors.append({"id": UNKNOWN_BLUEPRINT, "msg": "%s: blueprint not found" % blueprint_name}) continue # Combine modules and packages and depsolve the list @@ -1732,7 +1746,7 @@ def v0_api(api): return jsonify(status=False, errors=["Missing POST body"]), 400 if "blueprint_name" not in compose: - errors.append("No 'blueprint_name' in the JSON request") + errors.append({"id": UNKNOWN_BLUEPRINT,"msg": "No 'blueprint_name' in the JSON request"}) else: blueprint_name = compose["blueprint_name"] diff --git a/tests/pylorax/test_server.py b/tests/pylorax/test_server.py index 950b39c4..49b9bba6 100644 --- a/tests/pylorax/test_server.py +++ b/tests/pylorax/test_server.py @@ -185,7 +185,7 @@ class ServerTestCase(unittest.TestCase): self.assertEqual(data, info_dict_2) info_dict_3 = {"changes":[], - "errors":["missing-blueprint: No commits for missing-blueprint.toml on the master branch."], + "errors":[{"id": UNKNOWN_BLUEPRINT, "msg": "missing-blueprint: No commits for missing-blueprint.toml on the master branch."}], "blueprints":[] } resp = self.server.get("/api/v0/blueprints/info/missing-blueprint")