From 3f2391c79e86cbfed18ad446a11d355c2a40bea1 Mon Sep 17 00:00:00 2001
From: Benjamin Bertrand <benjamin.bertrand@ess.eu>
Date: Sat, 7 Mar 2020 21:41:28 +0100
Subject: [PATCH] Add DELETE method on all API endpoints
This is for admin only.
Note that deleting a network with existing hosts is not allowed.
Same for domain, network scope...
Will make testing csentry-api easier by allowing to reset the db during
tests.
JIRA INFRA-1853 #action In Progress
---
app/api/inventory.py | 74 +++++++++++++++++-
app/api/network.py | 60 +++++++++++++++
app/api/utils.py | 2 +-
tests/functional/factories.py | 1 +
tests/functional/test_api.py | 138 +++++++++++++++++++++++++++++++---
tests/functional/test_web.py | 2 +-
6 files changed, 263 insertions(+), 14 deletions(-)
diff --git a/app/api/inventory.py b/app/api/inventory.py
index 914fcb5..aef68cc 100644
--- a/app/api/inventory.py
+++ b/app/api/inventory.py
@@ -13,7 +13,7 @@ from flask import Blueprint, jsonify, request, current_app
from flask_login import login_required
from .. import utils, models
from ..decorators import login_groups_accepted
-from .utils import commit, create_generic_model, get_generic_model
+from .utils import commit, create_generic_model, get_generic_model, delete_generic_model
bp = Blueprint("inventory_api", __name__)
@@ -140,6 +140,18 @@ def patch_item(id_):
return jsonify(item.to_dict())
+@bp.route("/items/<int:item_id>", methods=["DELETE"])
+@login_groups_accepted("admin")
+def delete_item(item_id):
+ """Delete an item
+
+ .. :quickref: Inventory; Delete an item
+
+ :param item_id: item primary key
+ """
+ return delete_generic_model(models.Item, item_id)
+
+
@bp.route("/items/<id_>/comments")
@login_required
def get_item_comments(id_):
@@ -169,6 +181,18 @@ def create_item_comment(id_):
)
+@bp.route("/items/comments/<int:comment_id>", methods=["DELETE"])
+@login_groups_accepted("admin")
+def delete_comment(comment_id):
+ """Delete an item comment
+
+ .. :quickref: Inventory; Delete an item comment
+
+ :param comment_id: comment primary key
+ """
+ return delete_generic_model(models.ItemComment, comment_id)
+
+
@bp.route("/actions")
@login_required
def get_actions():
@@ -202,6 +226,18 @@ def create_manufacturer():
return create_generic_model(models.Manufacturer)
+@bp.route("/manufacturers/<int:manufacturer_id>", methods=["DELETE"])
+@login_groups_accepted("admin")
+def delete_manufacturer(manufacturer_id):
+ """Delete a manufacturer
+
+ .. :quickref: Inventory; Delete a manufacturer
+
+ :param manufacturer_id: manufacturer primary key
+ """
+ return delete_generic_model(models.Manufacturer, manufacturer_id)
+
+
@bp.route("/models")
@login_required
def get_models():
@@ -225,6 +261,18 @@ def create_model():
return create_generic_model(models.Model)
+@bp.route("/models/<int:model_id>", methods=["DELETE"])
+@login_groups_accepted("admin")
+def delete_model(model_id):
+ """Delete a model
+
+ .. :quickref: Inventory; Delete a model
+
+ :param model_id: model primary key
+ """
+ return delete_generic_model(models.Model, model_id)
+
+
@bp.route("/locations")
@login_required
def get_locations():
@@ -248,6 +296,18 @@ def create_locations():
return create_generic_model(models.Location)
+@bp.route("/locations/<int:location_id>", methods=["DELETE"])
+@login_groups_accepted("admin")
+def delete_location(location_id):
+ """Delete a location
+
+ .. :quickref: Inventory; Delete a location
+
+ :param location_id: location primary key
+ """
+ return delete_generic_model(models.Location, location_id)
+
+
@bp.route("/statuses")
@login_required
def get_status():
@@ -271,6 +331,18 @@ def create_status():
return create_generic_model(models.Status)
+@bp.route("/statuses/<int:status_id>", methods=["DELETE"])
+@login_groups_accepted("admin")
+def delete_status(status_id):
+ """Delete a status
+
+ .. :quickref: Inventory; Delete a status
+
+ :param status_id: status primary key
+ """
+ return delete_generic_model(models.Status, status_id)
+
+
@bp.route("/macs")
@login_required
def get_macs():
diff --git a/app/api/network.py b/app/api/network.py
index a293254..be24d54 100644
--- a/app/api/network.py
+++ b/app/api/network.py
@@ -52,6 +52,18 @@ def create_scope():
)
+@bp.route("/scopes/<int:scope_id>", methods=["DELETE"])
+@login_groups_accepted("admin")
+def delete_scope(scope_id):
+ """Delete a network scope
+
+ .. :quickref: Network; Delete a network scope
+
+ :param scope_id: network scope primary key
+ """
+ return utils.delete_generic_model(models.NetworkScope, scope_id)
+
+
@bp.route("/networks")
@login_groups_accepted("admin")
def get_networks():
@@ -133,6 +145,18 @@ def patch_network(network_id):
return utils.update_generic_model(models.Network, network_id, allowed_fields)
+@bp.route("/networks/<int:network_id>", methods=["DELETE"])
+@login_groups_accepted("admin")
+def delete_network(network_id):
+ """Delete a network
+
+ .. :quickref: Network; Delete a network
+
+ :param network_id: network primary key
+ """
+ return utils.delete_generic_model(models.Network, network_id)
+
+
@bp.route("/interfaces")
@login_required
def get_interfaces():
@@ -274,6 +298,18 @@ def create_ansible_groups():
return utils.create_generic_model(models.AnsibleGroup, mandatory_fields=("name",))
+@bp.route("/groups/<int:group_id>", methods=["DELETE"])
+@login_groups_accepted("admin")
+def delete_ansible_group(group_id):
+ """Delete an Ansible group
+
+ .. :quickref: Network; Delete an Ansible group
+
+ :param group_id: Ansible group primary key
+ """
+ return utils.delete_generic_model(models.AnsibleGroup, group_id)
+
+
@bp.route("/hosts")
@login_required
def get_hosts():
@@ -393,6 +429,18 @@ def create_domain():
return utils.create_generic_model(models.Domain, mandatory_fields=("name",))
+@bp.route("/domains/<int:domain_id>", methods=["DELETE"])
+@login_groups_accepted("admin")
+def delete_domain(domain_id):
+ """Delete a domain
+
+ .. :quickref: Network; Delete a domain
+
+ :param domain_id: domain primary key
+ """
+ return utils.delete_generic_model(models.Domain, domain_id)
+
+
@bp.route("/cnames")
@login_required
def get_cnames():
@@ -427,3 +475,15 @@ def create_cname():
return utils.create_generic_model(
models.Cname, mandatory_fields=("name", "interface_id")
)
+
+
+@bp.route("/cnames/<int:cname_id>", methods=["DELETE"])
+@login_groups_accepted("admin")
+def delete_cname(cname_id):
+ """Delete a cname
+
+ .. :quickref: Network; Delete a cname
+
+ :param cname_id: cname primary key
+ """
+ return utils.delete_generic_model(models.Cname, cname_id)
diff --git a/app/api/utils.py b/app/api/utils.py
index 8468eb8..8091518 100644
--- a/app/api/utils.py
+++ b/app/api/utils.py
@@ -165,7 +165,7 @@ def delete_generic_model(model, primary_key):
"""
instance = model.query.get_or_404(primary_key)
db.session.delete(instance)
- db.session.commit()
+ commit()
return jsonify(), 204
diff --git a/tests/functional/factories.py b/tests/functional/factories.py
index 60bbb8c..33722af 100644
--- a/tests/functional/factories.py
+++ b/tests/functional/factories.py
@@ -93,6 +93,7 @@ class ItemCommentFactory(factory.alchemy.SQLAlchemyModelFactory):
body = factory.Sequence(lambda n: f"comment{n}")
user = factory.SubFactory(UserFactory)
+ item = factory.SubFactory(ItemFactory)
class DomainFactory(factory.alchemy.SQLAlchemyModelFactory):
diff --git a/tests/functional/test_api.py b/tests/functional/test_api.py
index 57b6d07..719e33e 100644
--- a/tests/functional/test_api.py
+++ b/tests/functional/test_api.py
@@ -222,6 +222,12 @@ def check_input_is_subset_of_response(response, inputs):
assert d2[key] == value
+def check_delete_success(clt, token, instance, endpoint, model):
+ response = delete(clt, f"{API_URL}/{endpoint}/{instance.id}", token=token)
+ assert response.status_code == 204
+ assert len(model.query.all()) == 0
+
+
def test_login(client):
response = client.post(f"{API_URL}/user/login")
check_response_message(response, "Body should be a JSON object")
@@ -1355,13 +1361,10 @@ def test_delete_interface_invalid_credentials(client, interface_factory, user_to
assert len(models.Interface.query.all()) == 1
-def test_delete_interface_success(client, interface_factory, admin_token):
- interface1 = interface_factory()
- response = delete(
- client, f"{API_URL}/network/interfaces/{interface1.id}", token=admin_token
+def test_delete_interface_success(client, admin_token, interface):
+ check_delete_success(
+ client, admin_token, interface, "network/interfaces", models.Interface,
)
- assert response.status_code == 204
- assert len(models.Interface.query.all()) == 0
def test_delete_interface_invalid_id(client, interface_factory, admin_token):
@@ -1745,11 +1748,10 @@ def test_delete_host_invalid_credentials(client, host_factory, user_token):
assert len(models.Host.query.all()) == 1
-def test_delete_host_success(client, host_factory, admin_token):
- host1 = host_factory()
- response = delete(client, f"{API_URL}/network/hosts/{host1.id}", token=admin_token)
- assert response.status_code == 204
- assert len(models.Host.query.all()) == 0
+def test_delete_host_success(client, admin_token, host):
+ check_delete_success(
+ client, admin_token, host, "network/hosts", models.Host,
+ )
def test_delete_host_invalid_id(client, host_factory, admin_token):
@@ -2553,3 +2555,117 @@ def test_patch_network_invalid_domain(client, network_factory, admin_token):
client, f"{API_URL}/network/networks/{network.id}", data=data, token=admin_token
)
check_response_message(response, f"foo is not a valid domain", 400)
+
+
+def test_delete_item_success(client, admin_token, item):
+ check_delete_success(
+ client, admin_token, item, "inventory/items", models.Item,
+ )
+
+
+def test_delete_item_comment_success(client, admin_token, item_comment):
+ check_delete_success(
+ client,
+ admin_token,
+ item_comment,
+ "inventory/items/comments",
+ models.ItemComment,
+ )
+
+
+def test_delete_manufacturer_success(client, admin_token, manufacturer):
+ check_delete_success(
+ client,
+ admin_token,
+ manufacturer,
+ "inventory/manufacturers",
+ models.Manufacturer,
+ )
+
+
+def test_delete_model_success(client, admin_token, model):
+ check_delete_success(
+ client, admin_token, model, "inventory/models", models.Model,
+ )
+
+
+def test_delete_location_success(client, admin_token, location):
+ check_delete_success(
+ client, admin_token, location, "inventory/locations", models.Location,
+ )
+
+
+def test_delete_status_success(client, admin_token, status):
+ check_delete_success(
+ client, admin_token, status, "inventory/statuses", models.Status,
+ )
+
+
+def test_delete_network_scope_success(client, admin_token, network_scope):
+ check_delete_success(
+ client, admin_token, network_scope, "network/scopes", models.NetworkScope,
+ )
+
+
+def test_delete_network_scope_with_network_fail(
+ client, network_scope, network_factory, admin_token
+):
+ network_factory(scope=network_scope)
+ response = delete(
+ client, f"{API_URL}/network/scopes/{network_scope.id}", token=admin_token
+ )
+ check_response_message(
+ response,
+ '(psycopg2.IntegrityError) null value in column "scope_id" violates not-null constraint',
+ 422,
+ )
+
+
+def test_delete_network_success(client, admin_token, network):
+ check_delete_success(
+ client, admin_token, network, "network/networks", models.Network,
+ )
+
+
+def test_delete_network_with_host_fail(
+ client, network_192_168_1, interface_factory, admin_token
+):
+ interface_factory(network=network_192_168_1)
+ response = delete(
+ client, f"{API_URL}/network/networks/{network_192_168_1.id}", token=admin_token
+ )
+ check_response_message(
+ response,
+ '(psycopg2.IntegrityError) null value in column "network_id" violates not-null constraint',
+ 422,
+ )
+
+
+def test_delete_ansible_group_success(client, admin_token, ansible_group):
+ check_delete_success(
+ client, admin_token, ansible_group, "network/groups", models.AnsibleGroup,
+ )
+
+
+def test_delete_domain_success(client, admin_token, domain):
+ check_delete_success(
+ client, admin_token, domain, "network/domains", models.Domain,
+ )
+
+
+def test_delete_domain_with_network_fail(client, domain, network_factory, admin_token):
+ network_factory(domain=domain)
+ response = delete(
+ client, f"{API_URL}/network/domains/{domain.id}", token=admin_token
+ )
+ check_response_message(
+ response,
+ '(psycopg2.IntegrityError) null value in column "domain_id" violates not-null constraint',
+ 422,
+ )
+
+
+def test_delete_cname_success(client, admin_token, cname):
+ check_delete_success(
+ client, admin_token, cname, "network/cnames", models.Cname,
+ )
diff --git a/tests/functional/test_web.py b/tests/functional/test_web.py
index 9fa25e8..eed25c3 100644
--- a/tests/functional/test_web.py
+++ b/tests/functional/test_web.py
@@ -336,7 +336,7 @@ def test_edit_item_comment_in_index(
logged_rw_client, item_factory, item_comment_factory
):
item1 = item_factory(ics_id="AAA001")
- comment = item_comment_factory(body="Hello", item_id=item1.id)
+ comment = item_comment_factory(body="Hello", item=item1)
assert item1.comments == [comment]
# Edit the comment
body = "Hello world!"
--
GitLab