Action delete API rework - support for action file deletion

st2api/st2api/controllers/v1/actions.py

@@ -206,7 +206,7 @@ def put(self, action, ref_or_id, requester_user):
 
         return action_api
 
-    def delete(self, ref_or_id, requester_user):
+    def delete(self, files_remove_request, ref_or_id, requester_user):
         """
         Delete an action.
 
@@ -240,6 +240,7 @@ def delete(self, ref_or_id, requester_user):
         pack_name = action_db["pack"]
         entry_point = action_db["entry_point"]
         metadata_file = action_db["metadata_file"]
+        remove_files = files_remove_request.remove_files
 
         try:
             Action.delete(action_db)
@@ -252,30 +253,30 @@ def delete(self, ref_or_id, requester_user):
             )
             abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e))
             return
-        try:
-            delete_action_files_from_pack(
-                pack_name=pack_name,
-                entry_point=entry_point,
-                metadata_file=metadata_file,
-            )
-
-        except PermissionError as e:
-            LOG.error("No permission to delete resource files from disk.")
-            action_db.id = None
-            Action.add_or_update(action_db)
-            abort(http_client.FORBIDDEN, six.text_type(e))
-            return
 
-        except Exception as e:
-            LOG.error(
-                "Exception encountered during deleting resource files from disk. "
-                "Exception was %s",
-                e,
-            )
-            action_db.id = None
-            Action.add_or_update(action_db)
-            abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e))
-            return
+        if remove_files:
+            try:
+                delete_action_files_from_pack(
+                    pack_name=pack_name,
+                    entry_point=entry_point,
+                    metadata_file=metadata_file,
+                )
+            except PermissionError as e:
+                LOG.error("No permission to delete resource files from disk.")
+                action_db.id = None
+                Action.add_or_update(action_db)
+                abort(http_client.FORBIDDEN, six.text_type(e))
+                return
+            except Exception as e:
+                LOG.error(
+                    "Exception encountered during deleting resource files from disk. "
+                    "Exception was %s",
+                    e,
+                )
+                action_db.id = None
+                Action.add_or_update(action_db)
+                abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e))
+                return
 
         extra = {"action_db": action_db}
         LOG.audit("Action deleted. Action.id=%s" % (action_db.id), extra=extra)

st2api/tests/unit/controllers/v1/test_actions.py

@@ -637,8 +637,9 @@ def test_delete_permission_error_and_action_reregistered_to_database(self):
         ) as mock_remove_files:
             msg = "No permission to delete action files from disk"
             mock_remove_files.side_effect = PermissionError(msg)
-            del_resp = self.__do_delete(
-                self.__get_action_id(post_resp), expect_errors=True
+            payload = {"remove_files": True}
+            del_resp = self.__do_delete_action_with_files(
+                payload, self.__get_action_id(post_resp), expect_errors=True
             )
             self.assertEqual(del_resp.status_int, 403)
             self.assertEqual(del_resp.json["faultstring"], msg)
@@ -663,8 +664,9 @@ def test_delete_exception_and_action_reregistered_to_database(self):
         ) as mock_remove_files:
             msg = "Exception encountered during removing files from disk"
             mock_remove_files.side_effect = Exception(msg)
-            del_resp = self.__do_delete(
-                self.__get_action_id(post_resp), expect_errors=True
+            payload = {"remove_files": True}
+            del_resp = self.__do_delete_action_with_files(
+                payload, self.__get_action_id(post_resp), expect_errors=True
             )
             self.assertEqual(del_resp.status_int, 500)
             self.assertEqual(del_resp.json["faultstring"], msg)
@@ -854,3 +856,12 @@ def __do_delete(self, action_id, expect_errors=False):
         return self.app.delete(
             "/v1/actions/%s" % action_id, expect_errors=expect_errors
         )
+
+    def __do_delete_action_with_files(
+        self, files_remove_request, action_id, expect_errors=False
+    ):
+        return self.app.delete_json(
+            "/v1/actions/%s" % action_id,
+            files_remove_request,
+            expect_errors=expect_errors,
+        )

st2client/st2client/commands/action.py

@@ -282,31 +282,24 @@ def __init__(self, resource, *args, **kwargs):
         super(ActionDeleteCommand, self).__init__(resource, *args, **kwargs)
 
         self.parser.add_argument(
-            "-f",
-            "--force",
+            "-rf",
+            "--remove-files",
             action="store_true",
-            dest="force",
-            help="Auto yes flag to delete action files from disk.",
+            dest="remove_files",
+            help="Remove action files from disk.",
         )
 
     @add_auth_token_to_kwargs_from_cli
     def run(self, args, **kwargs):
         resource_id = getattr(args, self.pk_argument_name, None)
         instance = self.get_resource(resource_id, **kwargs)
-        msg = (
-            'Resource with id "%s" has been successfully deleted from database and disk.'
-            % (resource_id)
-        )
-        user_input = ""
-        if not args.force:
-            user_input = input(
-                "The resource files on disk will be deleted. Do you want to continue? (y/n): "
-            )
-        if args.force or user_input.lower() == "y" or user_input.lower() == "yes":
-            self.manager.delete(instance, **kwargs)
-            print(msg)
+        if args.remove_files:
+            remove_files = True
         else:
-            print("Action is not deleted.")
+            remove_files = False
+
+        self.manager.delete_action(instance, remove_files, **kwargs)
+        print('Resource with id "%s" has been successfully deleted.' % (resource_id))
 
     def run_and_print(self, args, **kwargs):
         resource_id = getattr(args, self.pk_argument_name)

st2client/st2client/models/core.py

@@ -376,6 +376,21 @@ def delete(self, instance, **kwargs):
 
         return True
 
+    @add_auth_token_to_kwargs_from_env
+    def delete_action(self, instance, remove_files, **kwargs):
+        url = "/%s/%s" % (self.resource.get_url_path_name(), instance.id)
+        payload = {"remove_files": remove_files}
+        response = self.client.delete(url, data=orjson.dumps(payload), **kwargs)
+        if response.status_code not in [
+            http_client.OK,
+            http_client.NO_CONTENT,
+            http_client.NOT_FOUND,
+        ]:
+            self.handle_error(response)
+            return False
+
+        return True
+
     @add_auth_token_to_kwargs_from_env
     def delete_by_id(self, instance_id, **kwargs):
         url = "/%s/%s" % (self.resource.get_url_path_name(), instance_id)

st2client/tests/unit/test_models.py

@@ -355,6 +355,56 @@ def test_resource_delete_failed(self):
         instance = mgr.get_by_name("abc")
         self.assertRaises(Exception, mgr.delete, instance)
 
+    @mock.patch.object(
+        httpclient.HTTPClient,
+        "get",
+        mock.MagicMock(
+            return_value=base.FakeResponse(
+                json.dumps([base.RESOURCES[0]]), 200, "OK", {}
+            )
+        ),
+    )
+    @mock.patch.object(
+        httpclient.HTTPClient,
+        "delete",
+        mock.MagicMock(return_value=base.FakeResponse("", 204, "NO CONTENT")),
+    )
+    def test_resource_delete_action(self):
+        mgr = models.ResourceManager(base.FakeResource, base.FAKE_ENDPOINT)
+        instance = mgr.get_by_name("abc")
+        mgr.delete_action(instance, True)
+
+    @mock.patch.object(
+        httpclient.HTTPClient,
+        "delete",
+        mock.MagicMock(return_value=base.FakeResponse("", 404, "NOT FOUND")),
+    )
+    def test_resource_delete_action_404(self):
+        mgr = models.ResourceManager(base.FakeResource, base.FAKE_ENDPOINT)
+        instance = base.FakeResource.deserialize(base.RESOURCES[0])
+        mgr.delete_action(instance, False)
+
+    @mock.patch.object(
+        httpclient.HTTPClient,
+        "get",
+        mock.MagicMock(
+            return_value=base.FakeResponse(
+                json.dumps([base.RESOURCES[0]]), 200, "OK", {}
+            )
+        ),
+    )
+    @mock.patch.object(
+        httpclient.HTTPClient,
+        "delete",
+        mock.MagicMock(
+            return_value=base.FakeResponse("", 500, "INTERNAL SERVER ERROR")
+        ),
+    )
+    def test_resource_delete_action_failed(self):
+        mgr = models.ResourceManager(base.FakeResource, base.FAKE_ENDPOINT)
+        instance = mgr.get_by_name("abc")
+        self.assertRaises(Exception, mgr.delete_action, instance, True)
+
     @mock.patch("requests.get")
     @mock.patch("sseclient.SSEClient")
     def test_stream_resource_listen(self, mock_sseclient, mock_requests):

st2common/st2common/openapi.yaml

@@ -413,6 +413,11 @@ paths:
           description: Entity reference or id
           type: string
           required: true
+        - name: files_remove_request
+          in: body
+          description: Flag to remove action files from disk
+          schema:
+            $ref: '#/definitions/ActionDeleteRequest'
       x-parameters:
         - name: user
           in: context
@@ -4684,6 +4689,9 @@ definitions:
     allOf:
       - $ref: '#/definitions/Action'
       - $ref: '#/definitions/DataFilesSubSchema'
+  ActionDeleteRequest:
+    allOf:
+      - $ref: '#/definitions/ActionDeleteSchema'
   ActionParameters:
     type: object
     properties:
@@ -5384,6 +5392,14 @@ definitions:
         items:
           type: string
     additionalProperties: false
+  ActionDeleteSchema:
+    type: object
+    properties:
+      remove_files:
+        type: boolean
+        description: Force delete action files from disk
+        default: false
+    additionalProperties: false
   TokenRequest:
     type: object
     properties:

st2common/st2common/openapi.yaml.j2

@@ -409,6 +409,11 @@ paths:
           description: Entity reference or id
           type: string
           required: true
+        - name: files_remove_request
+          in: body
+          description: Flag to remove action files from disk
+          schema:
+            $ref: '#/definitions/ActionDeleteRequest'
       x-parameters:
         - name: user
           in: context
@@ -4680,6 +4685,9 @@ definitions:
     allOf:
       - $ref: '#/definitions/Action'
       - $ref: '#/definitions/DataFilesSubSchema'
+  ActionDeleteRequest:
+    allOf:
+      - $ref: '#/definitions/ActionDeleteSchema'
   ActionParameters:
     type: object
     properties:
@@ -5380,6 +5388,14 @@ definitions:
         items:
           type: string
     additionalProperties: false
+  ActionDeleteSchema:
+    type: object
+    properties:
+      remove_files:
+        type: boolean
+        description: Force delete action files from disk
+        default: false
+    additionalProperties: false
   TokenRequest:
     type: object
     properties: