Merge pull request 'Remove ServiceInstance soft-delete' (#174) from 167-drop-soft-delete into main
Some checks failed
Build and Deploy Staging / deploy (push) Blocked by required conditions
Build and Deploy Staging / build (push) Successful in 49s
Tests / test (push) Has been cancelled

Reviewed-on: #174
This commit is contained in:
Tobias Brunner 2025-09-05 13:37:24 +00:00
commit 36dbc2a1dc
13 changed files with 64 additions and 105 deletions

View file

@ -9,6 +9,7 @@ dependencies = [
"cryptography>=45.0.7", "cryptography>=45.0.7",
"django==5.2.6", "django==5.2.6",
"django-allauth>=65.10.0", "django-allauth>=65.10.0",
"django-auditlog>=3.2.1",
"django-fernet-encrypted-fields>=0.3.0", "django-fernet-encrypted-fields>=0.3.0",
"django-jsonform>=2.23.2", "django-jsonform>=2.23.2",
"django-scopes>=2.0.0", "django-scopes>=2.0.0",

View file

@ -272,8 +272,8 @@ class ControlPlaneCRDAdmin(admin.ModelAdmin):
@admin.register(ServiceInstance) @admin.register(ServiceInstance)
class ServiceInstanceAdmin(admin.ModelAdmin): class ServiceInstanceAdmin(admin.ModelAdmin):
list_display = ("name", "organization", "context", "created_by", "is_deleted") list_display = ("name", "organization", "context", "created_by")
list_filter = ("organization", "context", "is_deleted") list_filter = ("organization", "context")
search_fields = ( search_fields = (
"name", "name",
"organization__name", "organization__name",
@ -296,9 +296,6 @@ class ServiceInstanceAdmin(admin.ModelAdmin):
"organization", "organization",
"context", "context",
"created_by", "created_by",
"is_deleted",
"deleted_at",
"deleted_by",
) )
}, },
), ),

View file

@ -0,0 +1,25 @@
# Generated by Django 5.2.4 on 2025-09-03 23:08
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [
("core", "0005_organization_sale_order_fields"),
]
operations = [
migrations.RemoveField(
model_name="serviceinstance",
name="deleted_at",
),
migrations.RemoveField(
model_name="serviceinstance",
name="deleted_by",
),
migrations.RemoveField(
model_name="serviceinstance",
name="is_deleted",
),
]

View file

@ -6,11 +6,11 @@ import re
import kubernetes import kubernetes
import rules import rules
import urlman import urlman
from auditlog.registry import auditlog
from django.conf import settings from django.conf import settings
from django.core.cache import cache from django.core.cache import cache
from django.core.exceptions import ValidationError from django.core.exceptions import ValidationError
from django.db import IntegrityError, models, transaction from django.db import IntegrityError, models, transaction
from django.utils import timezone
from django.utils.functional import cached_property from django.utils.functional import cached_property
from django.utils.safestring import mark_safe from django.utils.safestring import mark_safe
from django.utils.translation import gettext_lazy as _ from django.utils.translation import gettext_lazy as _
@ -571,16 +571,6 @@ class ServiceInstance(ServalaModelMixin, models.Model):
on_delete=models.PROTECT, on_delete=models.PROTECT,
) )
is_deleted = models.BooleanField(default=False)
deleted_at = models.DateTimeField(null=True, blank=True)
deleted_by = models.ForeignKey(
to="core.User",
on_delete=models.SET_NULL,
null=True,
blank=True,
related_name="+",
)
class Meta: class Meta:
verbose_name = _("Service instance") verbose_name = _("Service instance")
verbose_name_plural = _("Service instances") verbose_name_plural = _("Service instances")
@ -794,14 +784,10 @@ class ServiceInstance(ServalaModelMixin, models.Model):
raise ValidationError(self.organization.add_support_message(message)) raise ValidationError(self.organization.add_support_message(message))
@transaction.atomic @transaction.atomic
def delete_instance(self, user): def delete(self, using=None, keep_parents=False, user=None):
""" """
Soft deletes the instance in Django and initiates deletion of the Deletes the Django instance and the corresponding Kubernetes custom resource.
corresponding Kubernetes custom resource.
""" """
if self.is_deleted:
return
if ( if (
self.spec.get("parameters", {}) self.spec.get("parameters", {})
.get("security", {}) .get("security", {})
@ -825,19 +811,13 @@ class ServiceInstance(ServalaModelMixin, models.Model):
if e.status != 404: if e.status != 404:
# 404 is fine, the object was deleted already. # 404 is fine, the object was deleted already.
raise raise
self.is_deleted = True
self.deleted_at = timezone.now()
self.deleted_by = user
self.save(
update_fields=["is_deleted", "deleted_at", "deleted_by", "updated_at"]
)
self._clear_kubernetes_caches() self._clear_kubernetes_caches()
return super().delete(using=using, keep_parents=keep_parents)
@cached_property @cached_property
def kubernetes_object(self): def kubernetes_object(self):
"""Fetch the Kubernetes custom resource object""" """Fetch the Kubernetes custom resource object"""
if self.is_deleted:
return
try: try:
api_instance = client.CustomObjectsApi( api_instance = client.CustomObjectsApi(
self.context.control_plane.get_kubernetes_client() self.context.control_plane.get_kubernetes_client()
@ -942,3 +922,6 @@ class ServiceInstance(ServalaModelMixin, models.Model):
return {"error": str(e)} return {"error": str(e)}
except Exception as e: except Exception as e:
return {"error": str(e)} return {"error": str(e)}
auditlog.register(ServiceInstance, exclude_fields=["updated_at"], serialize_data=True)

View file

@ -64,14 +64,6 @@ class ServiceInstanceFilterForm(forms.Form):
required=False, required=False,
label=_("Service Provider Zone"), label=_("Service Provider Zone"),
) )
status = forms.ChoiceField(
choices=(
("active", _("Active")),
("deleted", _("Deleted")),
),
required=False,
label=_("Status"),
)
def filter_queryset(self, queryset): def filter_queryset(self, queryset):
if self.is_valid(): if self.is_valid():
@ -88,11 +80,6 @@ class ServiceInstanceFilterForm(forms.Form):
) )
if data.get("control_plane"): if data.get("control_plane"):
queryset = queryset.filter(context__control_plane=data["control_plane"]) queryset = queryset.filter(context__control_plane=data["control_plane"])
status = data.get("status")
if status == "active":
queryset = queryset.filter(is_deleted=False)
elif status == "deleted":
queryset = queryset.filter(is_deleted=True)
return queryset return queryset

View file

@ -112,13 +112,6 @@
<span class="small text-muted">{{ instance.context.service_offering.service.name }}</span> <span class="small text-muted">{{ instance.context.service_offering.service.name }}</span>
</div> </div>
</td> </td>
<td>
{% if instance.is_deleted %}
<span class="badge bg-danger">{% translate "Deleted" %}</span>
{% else %}
<span class="badge bg-success">{% translate "Active" %}</span>
{% endif %}
</td>
<td> <td>
<span class="small text-muted">{{ instance.created_at|date:"M d, Y" }}</span> <span class="small text-muted">{{ instance.created_at|date:"M d, Y" }}</span>
</td> </td>

View file

@ -7,10 +7,10 @@
{% endblock html_title %} {% endblock html_title %}
{% block page_title_extra %} {% block page_title_extra %}
<div> <div>
{% if has_change_permission and not instance.is_deleted %} {% if has_change_permission %}
<a href="{{ instance.urls.update }}" class="btn btn-primary me-1 mb-1">{% translate "Edit" %}</a> <a href="{{ instance.urls.update }}" class="btn btn-primary me-1 mb-1">{% translate "Edit" %}</a>
{% endif %} {% endif %}
{% if has_delete_permission and not instance.is_deleted %} {% if has_delete_permission %}
<button type="button" <button type="button"
class="btn btn-danger me-1 mb-1" class="btn btn-danger me-1 mb-1"
hx-get="{{ instance.urls.delete }}" hx-get="{{ instance.urls.delete }}"
@ -54,26 +54,11 @@
<dd class="col-sm-8"> <dd class="col-sm-8">
{{ instance.updated_at|date:"SHORT_DATETIME_FORMAT" }} {{ instance.updated_at|date:"SHORT_DATETIME_FORMAT" }}
</dd> </dd>
<dt class="col-sm-4">{% translate "Status" %}</dt>
<dd class="col-sm-8">
{% if instance.is_deleted %}
<span class="badge text-bg-danger">{% translate "Deleted" %}</span>
{% if instance.deleted_at %}
<small class="text-muted d-block mt-1">
{% blocktranslate with date=instance.deleted_at|date:"SHORT_DATETIME_FORMAT" user=instance.deleted_by|default:_("system") %}
On {{ date }} by {{ user }}
{% endblocktranslate %}
</small>
{% endif %}
{% else %}
<span class="badge text-bg-success">{% translate "Active" %}</span>
{% endif %}
</dd>
</dl> </dl>
</div> </div>
</div> </div>
</div> </div>
{% if not instance.is_deleted and instance.status_conditions %} {% if instance.status_conditions %}
<div class="col-12 col-md-7"> <div class="col-12 col-md-7">
<div class="card"> <div class="card">
<div class="card-header"> <div class="card-header">
@ -118,7 +103,7 @@
</div> </div>
</div> </div>
{% endif %} {% endif %}
{% if not instance.is_deleted and instance.spec and spec_fieldsets %} {% if instance.spec and spec_fieldsets %}
<div class="col-12"> <div class="col-12">
<div class="card"> <div class="card">
<div class="card-header"> <div class="card-header">

View file

@ -27,7 +27,6 @@
<th>{% translate "Service Provider" %}</th> <th>{% translate "Service Provider" %}</th>
<th>{% translate "Service Provider Zone" %}</th> <th>{% translate "Service Provider Zone" %}</th>
<th>{% translate "Created At" %}</th> <th>{% translate "Created At" %}</th>
<th>{% translate "Status" %}</th>
</tr> </tr>
</thead> </thead>
<tbody> <tbody>
@ -40,17 +39,10 @@
<td>{{ instance.context.service_offering.provider.name }}</td> <td>{{ instance.context.service_offering.provider.name }}</td>
<td>{{ instance.context.control_plane.name }}</td> <td>{{ instance.context.control_plane.name }}</td>
<td>{{ instance.created_at|date:"SHORT_DATETIME_FORMAT" }}</td> <td>{{ instance.created_at|date:"SHORT_DATETIME_FORMAT" }}</td>
<td>
{% if instance.is_deleted %}
<span class="badge text-bg-secondary">{% translate "Deleted" %}</span>
{% else %}
<span class="badge text-bg-success">{% translate "Active" %}</span>
{% endif %}
</td>
</tr> </tr>
{% empty %} {% empty %}
<tr> <tr>
<td colspan="6">{% translate "No service instances found." %}</td> <td colspan="5">{% translate "No service instances found." %}</td>
</tr> </tr>
{% endfor %} {% endfor %}
</tbody> </tbody>

View file

@ -1,6 +1,6 @@
from django.conf import settings from django.conf import settings
from django.contrib.auth.mixins import LoginRequiredMixin from django.contrib.auth.mixins import LoginRequiredMixin
from django.db.models import Count, Q from django.db.models import Count
from django.shortcuts import redirect, render from django.shortcuts import redirect, render
from django.urls import reverse_lazy from django.urls import reverse_lazy
from django.utils.functional import cached_property from django.utils.functional import cached_property
@ -32,9 +32,7 @@ class OrganizationSelectionView(LoginRequiredMixin, TemplateView):
def get_context_data(self, **kwargs): def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs) context = super().get_context_data(**kwargs)
context["user_organizations"] = self.user_organizations.annotate( context["user_organizations"] = self.user_organizations.annotate(
instance_count=Count( instance_count=Count("service_instances")
"service_instances", filter=Q(service_instances__is_deleted=False)
)
) )
return context return context

View file

@ -70,9 +70,7 @@ class OrganizationDashboardView(
context = super().get_context_data(**kwargs) context = super().get_context_data(**kwargs)
organization = self.get_object() organization = self.get_object()
service_instances = ServiceInstance.objects.filter( service_instances = ServiceInstance.objects.filter(organization=organization)
organization=organization, is_deleted=False
)
recent_instances = service_instances.order_by("-created_at")[:5] recent_instances = service_instances.order_by("-created_at")[:5]
for instance in recent_instances: for instance in recent_instances:

View file

@ -198,11 +198,7 @@ class ServiceInstanceMixin:
def get_object(self, **kwargs): def get_object(self, **kwargs):
instance = super().get_object(**kwargs) instance = super().get_object(**kwargs)
if ( if not instance.kubernetes_object and not self._has_warned:
not instance.is_deleted
and not instance.kubernetes_object
and not self._has_warned
):
messages.warning( messages.warning(
self.request, self.request,
_( _(
@ -221,11 +217,7 @@ class ServiceInstanceDetailView(
def get_context_data(self, **kwargs): def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs) context = super().get_context_data(**kwargs)
if ( if self.object.kubernetes_object and self.object.spec:
not self.object.is_deleted
and self.object.kubernetes_object
and self.object.spec
):
context["spec_fieldsets"] = self.get_nested_spec() context["spec_fieldsets"] = self.get_nested_spec()
context["has_change_permission"] = self.request.user.has_perm( context["has_change_permission"] = self.request.user.has_perm(
ServiceInstance.get_perm("change"), self.object ServiceInstance.get_perm("change"), self.object
@ -406,15 +398,6 @@ class ServiceInstanceListView(OrganizationViewMixin, ListView):
) )
if self.filter_form.is_valid(): if self.filter_form.is_valid():
queryset = self.filter_form.filter_queryset(queryset) queryset = self.filter_form.filter_queryset(queryset)
status_filter = (
self.filter_form.cleaned_data.get("status")
if self.filter_form.is_valid()
else "active"
)
if status_filter == "active":
queryset = queryset.filter(is_deleted=False)
elif status_filter == "deleted":
queryset = queryset.filter(is_deleted=True)
return queryset.order_by("-created_at") return queryset.order_by("-created_at")
def get_context_data(self, **kwargs): def get_context_data(self, **kwargs):
@ -433,7 +416,7 @@ class ServiceInstanceDeleteView(
def form_valid(self, form): def form_valid(self, form):
try: try:
self.object.delete_instance(user=self.request.user) self.object.delete(user=self.request.user)
messages.success( messages.success(
self.request, self.request,
_("Service instance '{name}' has been scheduled for deletion.").format( _("Service instance '{name}' has been scheduled for deletion.").format(

View file

@ -157,6 +157,7 @@ INSTALLED_APPS = [
"allauth.account", "allauth.account",
"allauth.socialaccount", "allauth.socialaccount",
"allauth.socialaccount.providers.openid_connect", "allauth.socialaccount.providers.openid_connect",
"auditlog",
"servala.core", "servala.core",
] ]
@ -170,6 +171,7 @@ MIDDLEWARE = [
"django.middleware.clickjacking.XFrameOptionsMiddleware", "django.middleware.clickjacking.XFrameOptionsMiddleware",
"allauth.account.middleware.AccountMiddleware", "allauth.account.middleware.AccountMiddleware",
"django.contrib.auth.middleware.LoginRequiredMiddleware", "django.contrib.auth.middleware.LoginRequiredMiddleware",
"auditlog.middleware.AuditlogMiddleware",
"servala.core.middleware.OrganizationMiddleware", "servala.core.middleware.OrganizationMiddleware",
] ]
LOGIN_URL = "account_login" LOGIN_URL = "account_login"

15
uv.lock generated
View file

@ -376,6 +376,19 @@ dependencies = [
] ]
sdist = { url = "https://files.pythonhosted.org/packages/ac/82/e6f607b0bad524d227f6e5aaffdb5e2b286f6ab1b4b3151134ae2303c2d6/django_allauth-65.11.1.tar.gz", hash = "sha256:e95d5234cccaf92273d315e1393cc4626cb88a19d66a1bf0e81f89f7958cfa06", size = 1915592, upload-time = "2025-08-27T18:05:05.581Z" } sdist = { url = "https://files.pythonhosted.org/packages/ac/82/e6f607b0bad524d227f6e5aaffdb5e2b286f6ab1b4b3151134ae2303c2d6/django_allauth-65.11.1.tar.gz", hash = "sha256:e95d5234cccaf92273d315e1393cc4626cb88a19d66a1bf0e81f89f7958cfa06", size = 1915592, upload-time = "2025-08-27T18:05:05.581Z" }
[[package]]
name = "django-auditlog"
version = "3.2.1"
source = { registry = "https://pypi.org/simple" }
dependencies = [
{ name = "django" },
{ name = "python-dateutil" },
]
sdist = { url = "https://files.pythonhosted.org/packages/e1/46/9da1d94493832fa18d2f6324a76d387fa232001593866987a96047709f4e/django_auditlog-3.2.1.tar.gz", hash = "sha256:63a4c9f7793e94eed804bc31a04d9b0b58244b1d280e2ed273c8b406bff1f779", size = 72926, upload-time = "2025-07-03T20:08:17.734Z" }
wheels = [
{ url = "https://files.pythonhosted.org/packages/a7/06/67296d050a72dcd76f57f220df621cb27e5b9282ba7ad0f5f74870dce241/django_auditlog-3.2.1-py3-none-any.whl", hash = "sha256:99603ca9d015f7e9b062b1c34f3e0826a3ce6ae6e5950c81bb7e663f7802a899", size = 38330, upload-time = "2025-07-03T20:07:51.735Z" },
]
[[package]] [[package]]
name = "django-fernet-encrypted-fields" name = "django-fernet-encrypted-fields"
version = "0.3.0" version = "0.3.0"
@ -1129,6 +1142,7 @@ dependencies = [
{ name = "cryptography" }, { name = "cryptography" },
{ name = "django" }, { name = "django" },
{ name = "django-allauth" }, { name = "django-allauth" },
{ name = "django-auditlog" },
{ name = "django-fernet-encrypted-fields" }, { name = "django-fernet-encrypted-fields" },
{ name = "django-jsonform" }, { name = "django-jsonform" },
{ name = "django-scopes" }, { name = "django-scopes" },
@ -1166,6 +1180,7 @@ requires-dist = [
{ name = "cryptography", specifier = ">=45.0.7" }, { name = "cryptography", specifier = ">=45.0.7" },
{ name = "django", specifier = "==5.2.6" }, { name = "django", specifier = "==5.2.6" },
{ name = "django-allauth", specifier = ">=65.10.0" }, { name = "django-allauth", specifier = ">=65.10.0" },
{ name = "django-auditlog", specifier = ">=3.2.1" },
{ name = "django-fernet-encrypted-fields", specifier = ">=0.3.0" }, { name = "django-fernet-encrypted-fields", specifier = ">=0.3.0" },
{ name = "django-jsonform", specifier = ">=2.23.2" }, { name = "django-jsonform", specifier = ">=2.23.2" },
{ name = "django-scopes", specifier = ">=2.0.0" }, { name = "django-scopes", specifier = ">=2.0.0" },