From 9a76c20c6b9605bd031796411af8eb8a7bc2fe68 Mon Sep 17 00:00:00 2001
From: "Brian C. Lane" <bcl@redhat.com>
Date: Thu, 28 May 2020 11:52:32 -0700
Subject: [PATCH] lorax-composer: deleting an unknown workspace should return
 an error

This changes the workspace delete behavior to match osbuild-composer's,
returning an error if the workspace doesn't exist.
---
 src/pylorax/api/v0.py        |  5 ++++-
 src/pylorax/api/workspace.py | 34 ++++++++++++++++++++++++++++++++--
 tests/pylorax/test_server.py | 14 ++++++++++++++
 3 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/src/pylorax/api/v0.py b/src/pylorax/api/v0.py
index 21c2812b..c977d517 100644
--- a/src/pylorax/api/v0.py
+++ b/src/pylorax/api/v0.py
@@ -71,7 +71,7 @@ from pylorax.api.recipes import tag_recipe_commit, recipe_diff, RecipeFileError
 from pylorax.api.regexes import VALID_API_STRING, VALID_BLUEPRINT_NAME
 import pylorax.api.toml as toml
 from pylorax.api.utils import take_limits, blueprint_exists
-from pylorax.api.workspace import workspace_read, workspace_write, workspace_delete
+from pylorax.api.workspace import workspace_read, workspace_write, workspace_delete, workspace_exists
 
 # The API functions don't actually get called by any code here
 # pylint: disable=unused-variable
@@ -481,6 +481,9 @@ def v0_blueprints_delete_workspace(blueprint_name):
 
     try:
         with api.config["GITLOCK"].lock:
+            if not workspace_exists(api.config["GITLOCK"].repo, branch, blueprint_name):
+                raise Exception("Unknown blueprint: %s" % blueprint_name)
+
             workspace_delete(api.config["GITLOCK"].repo, branch, blueprint_name)
     except Exception as e:
         log.error("(v0_blueprints_delete_workspace) %s", str(e))
diff --git a/src/pylorax/api/workspace.py b/src/pylorax/api/workspace.py
index 44952c07..02b3e56c 100644
--- a/src/pylorax/api/workspace.py
+++ b/src/pylorax/api/workspace.py
@@ -81,6 +81,37 @@ def workspace_write(repo, branch, recipe):
     open(filename, 'wb').write(recipe.toml().encode("UTF-8"))
 
 
+def workspace_filename(repo, branch, recipe_name):
+    """Return the path and filename of the workspace recipe
+
+    :param repo: Open repository
+    :type repo: Git.Repository
+    :param branch: Branch name
+    :type branch: str
+    :param recipe_name: The name of the recipe
+    :type recipe_name: str
+    :returns: workspace recipe path and filename
+    :rtype: str
+    """
+    ws_dir = workspace_dir(repo, branch)
+    return joinpaths(ws_dir, recipe_filename(recipe_name))
+
+
+def workspace_exists(repo, branch, recipe_name):
+    """Return true of the workspace recipe exists
+
+    :param repo: Open repository
+    :type repo: Git.Repository
+    :param branch: Branch name
+    :type branch: str
+    :param recipe_name: The name of the recipe
+    :type recipe_name: str
+    :returns: True if the file exists
+    :rtype: bool
+    """
+    return os.path.exists(workspace_filename(repo, branch, recipe_name))
+
+
 def workspace_delete(repo, branch, recipe_name):
     """Delete the recipe from the workspace
 
@@ -93,7 +124,6 @@ def workspace_delete(repo, branch, recipe_name):
     :returns: None
     :raises: IO related errors
     """
-    ws_dir = workspace_dir(repo, branch)
-    filename = joinpaths(ws_dir, recipe_filename(recipe_name))
+    filename = workspace_filename(repo, branch, recipe_name)
     if os.path.exists(filename):
         os.unlink(filename)
diff --git a/tests/pylorax/test_server.py b/tests/pylorax/test_server.py
index 839faff1..2a86acc7 100644
--- a/tests/pylorax/test_server.py
+++ b/tests/pylorax/test_server.py
@@ -401,6 +401,13 @@ class ServerAPIV0TestCase(unittest.TestCase):
         self.assertEqual(len(changes), 1)
         self.assertEqual(changes[0], {"name":"example-glusterfs", "changed":True})
 
+    def test_09_blueprints_unknown_ws_delete(self):
+        """Test DELETE /api/v0/blueprints/workspace/missing-blueprint"""
+        resp = self.server.delete("/api/v0/blueprints/workspace/missing-blueprint")
+        self.assertEqual(resp.status_code, 400)
+        data = json.loads(resp.data)
+        self.assertEqual(data["status"], False)
+
     def test_09_blueprints_ws_delete(self):
         """Test DELETE /api/v0/blueprints/workspace/<blueprint_name>"""
         # Write to the workspace first, just use the test_blueprints_ws_json test for this
@@ -2042,6 +2049,13 @@ class ServerAPIV1TestCase(unittest.TestCase):
         self.assertEqual(len(changes), 1)
         self.assertEqual(changes[0], {"name":"example-glusterfs", "changed":True})
 
+    def test_09_blueprints_unknown_ws_delete(self):
+        """Test DELETE /api/v1/blueprints/workspace/missing-blueprint"""
+        resp = self.server.delete("/api/v1/blueprints/workspace/missing-blueprint")
+        self.assertEqual(resp.status_code, 400)
+        data = json.loads(resp.data)
+        self.assertEqual(data["status"], False)
+
     def test_09_blueprints_ws_delete(self):
         """Test DELETE /api/v1/blueprints/workspace/<blueprint_name>"""
         # Write to the workspace first, just use the test_blueprints_ws_json test for this