Merge pull request 'Service Instance Deletion' (#64) from 30-instance-deletion into main

Reviewed-on: #64

src/servala/core/models/service.py

@@ -1,3 +1,4 @@
+import copy
 import json
 
 import kubernetes
@@ -6,7 +7,8 @@ import urlman
 from django.conf import settings
 from django.core.cache import cache
 from django.core.exceptions import ValidationError
-from django.db import IntegrityError, models
+from django.db import IntegrityError, models, transaction
+from django.utils import timezone
 from django.utils.functional import cached_property
 from django.utils.translation import gettext_lazy as _
 from encrypted_fields.fields import EncryptedJSONField
@@ -373,7 +375,15 @@ class ControlPlaneCRD(ServalaModelMixin, models.Model):
 
     @cached_property
     def kind_plural(self):
-        return self.resource_definition.status.accepted_names.plural
+        if (
+            hasattr(self.resource_definition, "status")
+            and hasattr(self.resource_definition.status, "accepted_names")
+            and self.resource_definition.status.accepted_names
+        ):
+            return self.resource_definition.status.accepted_names.plural
+        if self.kind.endswith("s"):
+            return self.kind.lower()
+        return f"{self.kind.lower()}s"
 
     @cached_property
     def resource_definition(self):
@@ -399,8 +409,13 @@ class ControlPlaneCRD(ServalaModelMixin, models.Model):
         if result := cache.get(cache_key):
             return result
 
+        if not self.resource_definition:
+            return
+
         for v in self.resource_definition.spec.versions:
             if v.name == self.version:
+                if not v.schema or not v.schema.open_apiv3_schema:
+                    return
                 result = v.schema.open_apiv3_schema.to_dict()
                 timeout_seconds = 60 * 60 * 24
                 cache.set(cache_key, result, timeout=timeout_seconds)
@@ -410,6 +425,9 @@ class ControlPlaneCRD(ServalaModelMixin, models.Model):
     def django_model(self):
         from servala.core.crd import generate_django_model
 
+        if not self.resource_schema:
+            return
+
         kwargs = {
             "group": self.group,
             "version": self.version,
@@ -421,6 +439,8 @@ class ControlPlaneCRD(ServalaModelMixin, models.Model):
     def model_form_class(self):
         from servala.core.crd import generate_model_form_class
 
+        if not self.django_model:
+            return
         return generate_model_form_class(self.django_model)
 
 
@@ -514,6 +534,7 @@ class ServiceInstance(ServalaModelMixin, models.Model):
     class urls(urlman.Urls):
         base = "{self.organization.urls.instances}{self.name}/"
         update = "{base}update/"
+        delete = "{base}delete/"
 
     def _clear_kubernetes_caches(self):
         """Clears cached properties that depend on Kubernetes state."""
@@ -618,9 +639,51 @@ class ServiceInstance(ServalaModelMixin, models.Model):
                 _("Error updating instance: {error}").format(error=str(e))
             )
 
+    @transaction.atomic
+    def delete_instance(self, user):
+        """
+        Soft deletes the instance in Django and initiates deletion of the
+        corresponding Kubernetes custom resource.
+        """
+        if self.is_deleted:
+            return
+
+        if (
+            self.spec.get("parameters", {})
+            .get("security", {})
+            .get("deletionProtection")
+        ):
+            spec = copy.copy(self.spec)
+            spec["parameters"]["security"]["deletionProtection"] = False
+            self.update_spec(spec, user)
+
+        try:
+            api_instance = self.context.control_plane.custom_objects_api
+            api_instance.delete_namespaced_custom_object(
+                group=self.context.group,
+                version=self.context.version,
+                namespace=self.organization.namespace,
+                plural=self.context.kind_plural,
+                name=self.name,
+                body=client.V1DeleteOptions(),
+            )
+        except ApiException as e:
+            if e.status != 404:
+                # 404 is fine, the object was deleted already.
+                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()
+
     @cached_property
     def kubernetes_object(self):
         """Fetch the Kubernetes custom resource object"""
+        if self.is_deleted:
+            return
         try:
             api_instance = client.CustomObjectsApi(
                 self.context.control_plane.get_kubernetes_client()
@@ -654,6 +717,8 @@ class ServiceInstance(ServalaModelMixin, models.Model):
     @cached_property
     def spec_object(self):
         """Dynamically generated CRD object."""
+        if not self.context.django_model:
+            return
         return self.context.django_model(
             name=self.name,
             organization=self.organization,

src/servala/frontend/forms/service.py

@@ -6,6 +6,7 @@ from servala.core.models import (
     ControlPlane,
     Service,
     ServiceCategory,
+    ServiceInstance,
     ServiceOffering,
 )
 
@@ -37,6 +38,8 @@ class ControlPlaneSelectForm(forms.Form):
     def __init__(self, *args, planes=None, **kwargs):
         super().__init__(*args, **kwargs)
         self.fields["control_plane"].queryset = planes
+        if planes and planes.count() == 1:
+            self.fields["control_plane"].initial = planes.first()
 
 
 class ServiceInstanceFilterForm(forms.Form):
@@ -68,21 +71,50 @@ class ServiceInstanceFilterForm(forms.Form):
     def filter_queryset(self, queryset):
         if self.is_valid():
             data = self.cleaned_data
-            if data["name"]:
+            if data.get("name"):
                 queryset = queryset.filter(name__icontains=data["name"])
-            if data["service"]:
+            if data.get("service"):
                 queryset = queryset.filter(
                     context__service_definition__service=data["service"]
                 )
-            if data["provider"]:
+            if data.get("provider"):
                 queryset = queryset.filter(
                     context__service_offering__provider=data["provider"]
                 )
-            if data["control_plane"]:
+            if data.get("control_plane"):
                 queryset = queryset.filter(context__control_plane=data["control_plane"])
-            if data["status"]:
+            status = data.get("status")
-                if data["status"] == "active":
+            if status == "active":
-                    queryset = queryset.filter(is_deleted=False)
+                queryset = queryset.filter(is_deleted=False)
-                else:
+            elif status == "deleted":
-                    queryset = queryset.filter(is_deleted=True)
+                queryset = queryset.filter(is_deleted=True)
         return queryset
+
+
+class ServiceInstanceDeleteForm(forms.ModelForm):
+    name = forms.CharField(
+        label=_("Instance Name"),
+        max_length=63,
+        widget=forms.TextInput(attrs={"class": "form-control"}),
+    )
+
+    def __init__(self, *args, **kwargs):
+        kwargs["initial"] = {"name": ""}
+        super().__init__(*args, **kwargs)
+        self.fields["name"].help_text = _(
+            "To confirm deletion, please type the instance name: <strong>{instance_name}</strong>"
+        ).format(instance_name=self.instance.name)
+
+    def clean_name(self):
+        entered_name = self.cleaned_data.get("name")
+        if entered_name != self.instance.name:
+            raise forms.ValidationError(
+                _(
+                    "The entered name does not match the instance name. Deletion not confirmed."
+                )
+            )
+        return entered_name
+
+    class Meta:
+        model = ServiceInstance
+        fields = ("name",)

src/servala/frontend/templates/frontend/forms/form.html

@@ -1,6 +1,6 @@
 {% load i18n %}
 {% if form.non_field_errors or form.errors %}
-    <div class="alert alert-danger" role="alert">
+    <div class="alert alert-danger form-errors" role="alert">
         <div>
             {% if form.non_field_errors %}
                 {% if form.non_field_errors|length > 1 %}

src/servala/frontend/templates/frontend/organizations/service_instance_delete_form.html

@@ -0,0 +1,25 @@
+{% load i18n %}
+<form method="post"
+      action="{{ view.request.path }}"
+      hx-post="{{ view.request.path }}"
+      hx-target="this"
+      hx-swap="outerHTML">
+    {% csrf_token %}
+    <div class="modal-header">
+        <h5 class="modal-title" id="deleteInstanceModalLabel">
+            {% blocktranslate with instance_name=instance.name %}Confirm Deletion of {{ instance_name }}{% endblocktranslate %}
+        </h5>
+        <button type="button"
+                class="btn-close"
+                data-bs-dismiss="modal"
+                aria-label="Close"></button>
+    </div>
+    <div class="modal-body hide-form-errors">
+        <p>{% translate "Do you really want to delete this service instance? This action cannot be undone." %}</p>
+        {{ form }}
+    </div>
+    <div class="modal-footer">
+        <button type="button" class="btn btn-secondary" data-bs-dismiss="modal">{% translate "Cancel" %}</button>
+        <button type="submit" class="btn btn-danger">{% translate "Confirm and Delete Instance" %}</button>
+    </div>
+</form>

src/servala/frontend/templates/frontend/organizations/service_instance_detail.html

@@ -1,14 +1,24 @@
 {% extends "frontend/base.html" %}
-{% load i18n static %}
+{% load i18n static pprint_filters %}
 {% block html_title %}
     {% block page_title %}
         {{ instance.name }}
     {% endblock page_title %}
 {% endblock html_title %}
 {% block page_title_extra %}
-    {% if has_change_permission %}
+    <div>
-        <a href="{{ instance.urls.update }}" class="btn btn-primary me-1 mb-1">{% translate "Edit" %}</a>
+        {% if has_change_permission and not instance.is_deleted %}
-    {% endif %}
+            <a href="{{ instance.urls.update }}" class="btn btn-primary me-1 mb-1">{% translate "Edit" %}</a>
+        {% endif %}
+        {% if has_delete_permission and not instance.is_deleted %}
+            <button type="button"
+                    class="btn btn-danger me-1 mb-1"
+                    hx-get="{{ instance.urls.delete }}"
+                    hx-target="#deleteInstanceModalContent"
+                    data-bs-toggle="modal"
+                    data-bs-target="#deleteInstanceModal">{% translate "Delete" %}</button>
+        {% endif %}
+    </div>
 {% endblock page_title_extra %}
 {% block content %}
     <section class="section">
@@ -34,7 +44,7 @@
                             </dd>
                             <dt class="col-sm-4">{% translate "Created By" %}</dt>
                             <dd class="col-sm-8">
-                                {{ instance.created_by }}
+                                {{ instance.created_by|default:"-" }}
                             </dd>
                             <dt class="col-sm-4">{% translate "Created At" %}</dt>
                             <dd class="col-sm-8">
@@ -47,7 +57,14 @@
                             <dt class="col-sm-4">{% translate "Status" %}</dt>
                             <dd class="col-sm-8">
                                 {% if instance.is_deleted %}
-                                    <span class="badge text-bg-secondary">{% translate "Deleted" %}</span>
+                                    <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 %}
@@ -56,7 +73,7 @@
                     </div>
                 </div>
             </div>
-            {% if instance.status_conditions %}
+            {% if not instance.is_deleted and instance.status_conditions %}
                 <div class="col-12 col-md-7">
                     <div class="card">
                         <div class="card-header">
@@ -88,9 +105,9 @@
                                                             <span class="badge text-bg-secondary">{{ condition.status }}</span>
                                                         {% endif %}
                                                     </td>
-                                                    <td>{{ condition.lastTransitionTime }}</td>
+                                                    <td>{{ condition.lastTransitionTime|date:"SHORT_DATETIME_FORMAT" }}</td>
-                                                    <td>{{ condition.reason }}</td>
+                                                    <td>{{ condition.reason|default:"-" }}</td>
-                                                    <td>{{ condition.message }}</td>
+                                                    <td>{{ condition.message|truncatewords:20|default:"-" }}</td>
                                                 </tr>
                                             {% endfor %}
                                         </tbody>
@@ -101,7 +118,7 @@
                     </div>
                 </div>
             {% endif %}
-            {% if instance.spec %}
+            {% if not instance.is_deleted and instance.spec and spec_fieldsets %}
                 <div class="col-12">
                     <div class="card">
                         <div class="card-header">
@@ -119,6 +136,8 @@
                                                    data-bs-toggle="tab"
                                                    role="tab">{{ fieldset.title }}</a>
                                             </li>
+                                        {% empty %}
+                                            <li class="nav-item ms-2">{% translate "No specification details available." %}</li>
                                         {% endfor %}
                                     </ul>
                                     <!-- Tab Content -->
@@ -135,7 +154,7 @@
                                                             {% if field.value|default:""|stringformat:"s"|slice:":1" == "{" or field.value|default:""|stringformat:"s"|slice:":1" == "[" %}
                                                                 <pre>{{ field.value|pprint }}</pre>
                                                             {% else %}
-                                                                {{ field.value }}
+                                                                {{ field.value|default:"-" }}
                                                             {% endif %}
                                                         </dd>
                                                     {% endfor %}
@@ -150,13 +169,15 @@
                                                                 {% if field.value|default:""|stringformat:"s"|slice:":1" == "{" or field.value|default:""|stringformat:"s"|slice:":1" == "[" %}
                                                                     <pre>{{ field.value|pprint }}</pre>
                                                                 {% else %}
-                                                                    {{ field.value }}
+                                                                    {{ field.value|default:"-" }}
                                                                 {% endif %}
                                                             </dd>
                                                         {% endfor %}
                                                     </dl>
                                                 {% endfor %}
                                             </div>
+                                        {% empty %}
+                                            <p>{% translate "No specification details to display." %}</p>
                                         {% endfor %}
                                     </div>
                                 </div>
@@ -200,4 +221,33 @@
             {% endif %}
         </div>
     </section>
+    <!-- Delete Confirmation Modal -->
+    <div class="modal fade"
+         id="deleteInstanceModal"
+         tabindex="-1"
+         aria-labelledby="deleteInstanceModalLabel"
+         aria-hidden="true">
+        <div class="modal-dialog modal-dialog-centered">
+            <div class="modal-content" id="deleteInstanceModalContent">
+                {# Content will be loaded here by HTMX #}
+                <div class="modal-header">
+                    <h5 class="modal-title" id="deleteInstanceModalLabel">{% translate "Confirm Deletion" %}</h5>
+                    <button type="button"
+                            class="btn-close"
+                            data-bs-dismiss="modal"
+                            aria-label="Close"></button>
+                </div>
+                <div class="modal-body">
+                    <div class="d-flex justify-content-center">
+                        <div class="spinner-border" role="status">
+                            <span class="visually-hidden">{% translate "Loading..." %}</span>
+                        </div>
+                    </div>
+                </div>
+                <div class="modal-footer">
+                    <button type="button" class="btn btn-secondary" data-bs-dismiss="modal">{% translate "Cancel" %}</button>
+                </div>
+            </div>
+        </div>
+    </div>
 {% endblock content %}

src/servala/frontend/urls.py

@@ -55,6 +55,11 @@ urlpatterns = [
                     views.ServiceInstanceUpdateView.as_view(),
                     name="organization.instance.update",
                 ),
+                path(
+                    "instances/<slug:slug>/delete/",
+                    views.ServiceInstanceDeleteView.as_view(),
+                    name="organization.instance.delete",
+                ),
             ]
         ),
     ),

src/servala/frontend/views/__init__.py

@@ -7,6 +7,7 @@ from .organization import (
 )
 from .service import (
     ServiceDetailView,
+    ServiceInstanceDeleteView,
     ServiceInstanceDetailView,
     ServiceInstanceListView,
     ServiceInstanceUpdateView,
@@ -21,6 +22,7 @@ __all__ = [
     "OrganizationDashboardView",
     "OrganizationUpdateView",
     "ServiceDetailView",
+    "ServiceInstanceDeleteView",
     "ServiceInstanceDetailView",
     "ServiceInstanceListView",
     "ServiceInstanceUpdateView",

src/servala/frontend/views/service.py

@@ -1,9 +1,10 @@
 from django.contrib import messages
 from django.core.exceptions import ValidationError
+from django.http import HttpResponse
 from django.shortcuts import redirect
 from django.utils.functional import cached_property
 from django.utils.translation import gettext_lazy as _
-from django.views.generic import DetailView, ListView
+from django.views.generic import DetailView, ListView, UpdateView
 
 from servala.core.crd import deslugify
 from servala.core.models import (
@@ -15,6 +16,7 @@ from servala.core.models import (
 from servala.frontend.forms.service import (
     ControlPlaneSelectForm,
     ServiceFilterForm,
+    ServiceInstanceDeleteForm,
     ServiceInstanceFilterForm,
 )
 from servala.frontend.views.mixins import (
@@ -34,10 +36,14 @@ class ServiceListView(OrganizationViewMixin, ListView):
 
     def get_queryset(self):
         """Return all services."""
-        services = Service.objects.all().select_related("category")
+        services = (
+            Service.objects.all()
+            .select_related("category")
+            .prefetch_related("offerings__provider")
+        )
         if self.filter_form.is_valid():
             services = self.filter_form.filter_queryset(services)
-        return services
+        return services.distinct()
 
     @cached_property
     def filter_form(self):
@@ -92,7 +98,7 @@ class ServiceOfferingDetailView(OrganizationViewMixin, HtmxViewMixin, DetailView
 
     @cached_property
     def selected_plane(self):
-        if self.select_form.data and self.select_form.is_valid():
+        if self.select_form.is_valid() and self.select_form.cleaned_data:
             return self.select_form.cleaned_data["control_plane"]
         field = self.select_form.fields["control_plane"]
         return field.initial or field.queryset.first()
@@ -110,7 +116,7 @@ class ServiceOfferingDetailView(OrganizationViewMixin, HtmxViewMixin, DetailView
         ).first()
 
     def get_instance_form(self):
-        if not self.context_object:
+        if not self.context_object or not self.context_object.model_form_class:
             return None
         return self.context_object.model_form_class(
             data=self.request.POST if self.request.method == "POST" else None,
@@ -144,7 +150,7 @@ class ServiceOfferingDetailView(OrganizationViewMixin, HtmxViewMixin, DetailView
         if form.is_valid():
             try:
                 service_instance = ServiceInstance.create_instance(
-                    organization=self.organization,
+                    organization=self.request.organization,
                     name=form.cleaned_data["name"],
                     context=self.context_object,
                     created_by=request.user,
@@ -185,8 +191,8 @@ class ServiceInstanceMixin:
     def get_object(self, **kwargs):
         instance = super().get_object(**kwargs)
         if (
-            not instance.kubernetes_object
+            not instance.is_deleted
-            and not instance.is_deleted
+            and not instance.kubernetes_object
             and not self._has_warned
         ):
             messages.warning(
@@ -207,11 +213,17 @@ class ServiceInstanceDetailView(
 
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)
-        if self.object.kubernetes_object and self.object.spec:
+        if (
+            not self.object.is_deleted
+            and self.object.kubernetes_object
+            and self.object.spec
+        ):
             context["spec_fieldsets"] = self.get_nested_spec()
-        permission_required = ServiceInstance.get_perm("change")
         context["has_change_permission"] = self.request.user.has_perm(
-            permission_required, self.object
+            ServiceInstance.get_perm("change"), self.object
+        )
+        context["has_delete_permission"] = self.request.user.has_perm(
+            ServiceInstance.get_perm("delete"), self.object
         )
         return context
 
@@ -220,8 +232,9 @@ class ServiceInstanceDetailView(
         Organize spec data into fieldsets similar to how the form does it.
         """
         spec = self.object.spec or {}
+        if not spec:
+            return []
 
-        # Process spec fields
         others = []
         nested_fieldsets = {}
 
@@ -371,7 +384,6 @@ class ServiceInstanceListView(OrganizationViewMixin, ListView):
         """Return all service instances for the current organization with filtering."""
         queryset = ServiceInstance.objects.filter(
             organization=self.request.organization,
-            is_deleted=False,  # Exclude soft-deleted
         ).select_related(
             "context__service_offering__provider",
             "context__control_plane",
@@ -379,10 +391,53 @@ class ServiceInstanceListView(OrganizationViewMixin, ListView):
         )
         if self.filter_form.is_valid():
             queryset = self.filter_form.filter_queryset(queryset)
-        return 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")
 
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)
         context["organization"] = self.request.organization
         context["filter_form"] = self.filter_form
         return context
+
+
+class ServiceInstanceDeleteView(
+    ServiceInstanceMixin, OrganizationViewMixin, HtmxViewMixin, UpdateView
+):
+    template_name = "frontend/organizations/service_instance_delete_form.html"
+    form_class = ServiceInstanceDeleteForm
+    permission_type = "delete"
+
+    def form_valid(self, form):
+        try:
+            self.object.delete_instance(user=self.request.user)
+            messages.success(
+                self.request,
+                _("Service instance '{name}' has been scheduled for deletion.").format(
+                    name=self.object.name
+                ),
+            )
+            response = HttpResponse()
+            response["HX-Redirect"] = self.get_success_url()
+            return response
+        except Exception as e:
+            messages.error(
+                self.request,
+                _(
+                    "An error occurred while trying to delete instance '{name}': {error}"
+                ).format(name=self.object.name, error=str(e)),
+            )
+            response = HttpResponse()
+            response["HX-Redirect"] = str(self.object.urls.base)
+            return response
+
+    def get_success_url(self):
+        return str(self.request.organization.urls.instances)

src/servala/static/css/servala.css

@@ -90,3 +90,6 @@ a.btn-keycloak {
     flex-wrap: wrap;
     justify-content: space-between;
 }
+.hide-form-errors .alert.form-errors {
+    display: none;
+}