Merge pull request 'Service Instance Update' (#61) from 29-service-instance-update into main

Reviewed-on: #61

src/servala/core/crd.py

@@ -9,6 +9,18 @@ from django.utils.translation import gettext_lazy as _
 from servala.core.models import ServiceInstance
 
 
+class CRDModel(models.Model):
+    """Base class for all virtual CRD models"""
+
+    def __init__(self, **kwargs):
+        if spec := kwargs.pop("spec", None):
+            kwargs.update(unnest_data({"spec": spec}))
+        super().__init__(**kwargs)
+
+    class Meta:
+        abstract = True
+
+
 def duplicate_field(field_name, model):
     # Get the field from the model
     field = model._meta.get_field(field_name)
@@ -47,7 +59,7 @@ def generate_django_model(schema, group, version, kind):
 
     # create the model class
     model_name = kind
-    model_class = type(model_name, (models.Model,), model_fields)
+    model_class = type(model_name, (CRDModel,), model_fields)
     return model_class
 
 
@@ -138,6 +150,21 @@ def get_django_field(
     return models.CharField(max_length=255, **kwargs)
 
 
+def unnest_data(data):
+    result = {}
+
+    def _flatten_dict(d, parent_key=""):
+        for key, value in d.items():
+            new_key = f"{parent_key}.{key}" if parent_key else key
+            if isinstance(value, dict):
+                _flatten_dict(value, new_key)
+            else:
+                result[new_key] = value
+
+    _flatten_dict(data)
+    return result
+
+
 class CrdModelFormMixin:
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
@@ -146,6 +173,11 @@ class CrdModelFormMixin:
         for field in ("organization", "context"):
             self.fields[field].widget = forms.HiddenInput()
 
+        if self.instance and self.instance.pk:
+            self.fields["name"].disabled = True
+            self.fields["name"].help_text = _("Name cannot be changed after creation.")
+            self.fields["name"].widget = forms.HiddenInput()
+
     def strip_title(self, field_name, label):
         field = self.fields[field_name]
         if field and field.label.startswith(label):
@@ -156,7 +188,10 @@ class CrdModelFormMixin:
 
         # General fieldset for non-spec fields
         general_fields = [
-            field for field in self.fields if not field.startswith("spec.")
+            field_name
+            for field_name, field in self.fields.items()
+            if not field_name.startswith("spec.")
+            and not isinstance(field.widget, forms.HiddenInput)
         ]
         if general_fields:
             fieldsets.append(

src/servala/core/models/service.py

@@ -188,6 +188,10 @@ class ControlPlane(ServalaModelMixin, models.Model):
     def get_kubernetes_client(self):
         return kubernetes.client.ApiClient(self.kubernetes_config)
 
+    @cached_property
+    def custom_objects_api(self):
+        return client.CustomObjectsApi(self.get_kubernetes_client())
+
     def test_connection(self):
         if not self.api_credentials:
             return False, _("No API credentials provided")
@@ -509,6 +513,19 @@ class ServiceInstance(ServalaModelMixin, models.Model):
 
     class urls(urlman.Urls):
         base = "{self.organization.urls.instances}{self.name}/"
+        update = "{base}update/"
+
+    def _clear_kubernetes_caches(self):
+        """Clears cached properties that depend on Kubernetes state."""
+        attrs = self.__dict__.keys()
+        for key in (
+            "kubernetes_object",
+            "spec",
+            "status_conditions",
+            "connection_credentials",
+        ):
+            if key in attrs:
+                delattr(self, key)
 
     @classmethod
     def create_instance(cls, name, organization, context, created_by, spec_data):
@@ -540,9 +557,7 @@ class ServiceInstance(ServalaModelMixin, models.Model):
             }
             if label := context.control_plane.required_label:
                 create_data["metadata"]["labels"] = {settings.DEFAULT_LABEL_KEY: label}
-            api_instance = client.CustomObjectsApi(
-                context.control_plane.get_kubernetes_client()
-            )
+            api_instance = context.control_plane.custom_objects_api
             api_instance.create_namespaced_custom_object(
                 group=context.group,
                 version=context.version,
@@ -562,6 +577,47 @@ class ServiceInstance(ServalaModelMixin, models.Model):
             raise ValidationError(_("Error creating instance: {}").format(str(e)))
         return instance
 
+    def update_spec(self, spec_data, updated_by):
+        try:
+            api_instance = self.context.control_plane.custom_objects_api
+            patch_body = {"spec": spec_data}
+
+            api_instance.patch_namespaced_custom_object(
+                group=self.context.group,
+                version=self.context.version,
+                namespace=self.organization.namespace,
+                plural=self.context.kind_plural,
+                name=self.name,
+                body=patch_body,
+            )
+            self._clear_kubernetes_caches()
+            self.save()  # Updates updated_at timestamp
+        except ApiException as e:
+            if e.status == 404:
+                raise ValidationError(
+                    _(
+                        "Service instance not found in Kubernetes. It may have been deleted externally."
+                    )
+                )
+            try:
+                error_body = json.loads(e.body)
+                reason = error_body.get("message", str(e))
+                raise ValidationError(
+                    _("Kubernetes API error updating instance: {error}").format(
+                        error=reason
+                    )
+                )
+            except (ValueError, TypeError):
+                raise ValidationError(
+                    _("Kubernetes API error updating instance: {error}").format(
+                        error=str(e)
+                    )
+                )
+        except Exception as e:
+            raise ValidationError(
+                _("Error updating instance: {error}").format(error=str(e))
+            )
+
     @cached_property
     def kubernetes_object(self):
         """Fetch the Kubernetes custom resource object"""
@@ -595,6 +651,20 @@ class ServiceInstance(ServalaModelMixin, models.Model):
         spec.pop("writeConnectionSecretToRef", None)
         return spec
 
+    @cached_property
+    def spec_object(self):
+        """Dynamically generated CRD object."""
+        return self.context.django_model(
+            name=self.name,
+            organization=self.organization,
+            context=self.context,
+            spec=self.spec,
+            # We pass -1 as ID in order to make it clear that a) this object exists (remotely),
+            # and b) it’s not a normal database object. This allows us to treat e.g. update
+            # forms differently from create forms.
+            pk=-1,
+        )
+
     @cached_property
     def status_conditions(self):
         if not self.kubernetes_object:

src/servala/frontend/templates/frontend/base.html

@@ -25,9 +25,13 @@
             <div class="content-wrapper container">
                 <div class="page-heading">
                     <h3>
-                        {% block page_title %}
-                            Dashboard
-                        {% endblock page_title %}
+                        <span>
+                            {% block page_title %}
+                                Dashboard
+                            {% endblock page_title %}
+                        </span>
+                        {% block page_title_extra %}
+                        {% endblock page_title_extra %}
                     </h3>
                 </div>
                 <div class="page-content">

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

@@ -5,6 +5,11 @@
         {{ instance.name }}
     {% endblock page_title %}
 {% endblock html_title %}
+{% block page_title_extra %}
+    {% if has_change_permission %}
+        <a href="{{ instance.urls.update }}" class="btn btn-primary me-1 mb-1">{% translate "Edit" %}</a>
+    {% endif %}
+{% endblock page_title_extra %}
 {% block content %}
     <section class="section">
         <div class="row">

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

@@ -0,0 +1,43 @@
+{% extends "frontend/base.html" %}
+{% load i18n %}
+{% load static %}
+{% load partials %}
+{% block html_title %}
+    {% block page_title %}
+        {% block title %}
+            {% blocktranslate with instance_name=instance.name organization_name=request.organization.name %}Update {{ instance_name }} in {{ organization_name }}{% endblocktranslate %}
+        {% endblock %}
+    {% endblock page_title %}
+{% endblock html_title %}
+{% block page_title_extra %}
+    <a href="{{ instance.urls.base }}" class="btn btn-secondary me-1 mb-1">{% translate "Back" %}</a>
+{% endblock page_title_extra %}
+{% partialdef service-form %}
+{% if form %}
+    <div class="card">
+        <div class="card-header d-flex align-items-center"></div>
+        <div class="card-body">
+            {% if form_error %}
+                <div class="alert alert-danger">
+                    {% translate "Oops! Something went wrong with the service form generation. Please try again later." %}
+                </div>
+            {% else %}
+                {% include "includes/tabbed_fieldset_form.html" with form=form %}
+            {% endif %}
+        </div>
+    </div>
+{% endif %}
+{% endpartialdef %}
+{% block content %}
+    <section class="section">
+        <div class="card">
+            {% if not form %}
+                <div class="alert alert-warning" role="alert">
+                    {% translate "Cannot update this service instance because its details could not be retrieved from the underlying system. It might have been deleted externally." %}
+                </div>
+            {% else %}
+                <div id="service-form">{% partial service-form %}</div>
+            {% endif %}
+        </div>
+    </section>
+{% endblock content %}

src/servala/frontend/urls.py

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

src/servala/frontend/views/__init__.py

@@ -9,6 +9,7 @@ from .service import (
     ServiceDetailView,
     ServiceInstanceDetailView,
     ServiceInstanceListView,
+    ServiceInstanceUpdateView,
     ServiceListView,
     ServiceOfferingDetailView,
 )
@@ -22,6 +23,7 @@ __all__ = [
     "ServiceDetailView",
     "ServiceInstanceDetailView",
     "ServiceInstanceListView",
+    "ServiceInstanceUpdateView",
     "ServiceListView",
     "ServiceOfferingDetailView",
     "ProfileView",

src/servala/frontend/views/service.py

@@ -2,6 +2,7 @@ from django.contrib import messages
 from django.core.exceptions import ValidationError
 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 servala.core.crd import deslugify
@@ -16,7 +17,11 @@ from servala.frontend.forms.service import (
     ServiceFilterForm,
     ServiceInstanceFilterForm,
 )
-from servala.frontend.views.mixins import HtmxViewMixin, OrganizationViewMixin
+from servala.frontend.views.mixins import (
+    HtmxUpdateView,
+    HtmxViewMixin,
+    OrganizationViewMixin,
+)
 
 
 class ServiceListView(OrganizationViewMixin, ListView):
@@ -105,6 +110,8 @@ class ServiceOfferingDetailView(OrganizationViewMixin, HtmxViewMixin, DetailView
         ).first()
 
     def get_instance_form(self):
+        if not self.context_object:
+            return None
         return self.context_object.model_form_class(
             data=self.request.POST if self.request.method == "POST" else None,
             initial={
@@ -130,6 +137,10 @@ class ServiceOfferingDetailView(OrganizationViewMixin, HtmxViewMixin, DetailView
             return self.render_to_response(context)
 
         form = self.get_instance_form()
+        if not form:  # Should not happen if context_object is valid, but as a safeguard
+            messages.error(self.request, _("Could not initialize service form."))
+            return self.render_to_response(context)
+
         if form.is_valid():
             try:
                 service_instance = ServiceInstance.create_instance(
@@ -143,22 +154,24 @@ class ServiceOfferingDetailView(OrganizationViewMixin, HtmxViewMixin, DetailView
             except ValidationError as e:
                 messages.error(self.request, e.message or str(e))
             except Exception as e:
-                messages.error(self.request, str(e))
+                messages.error(
+                    self.request, _("Error creating instance: {}").format(str(e))
+                )
 
         # If the form is not valid or if the service creation failed, we render it again
         context["service_form"] = form
         return self.render_to_response(context)
 
 
-class ServiceInstanceDetailView(OrganizationViewMixin, DetailView):
-    """View to display details of a specific service instance."""
-
-    template_name = "frontend/organizations/service_instance_detail.html"
-    context_object_name = "instance"
+class ServiceInstanceMixin:
     model = ServiceInstance
-    permission_type = "view"
+    context_object_name = "instance"
     slug_field = "name"
 
+    def dispatch(self, *args, **kwargs):
+        self._has_warned = False
+        return super().dispatch(*args, **kwargs)
+
     def get_queryset(self):
         """Return service instance for the current organization."""
         return ServiceInstance.objects.filter(
@@ -169,6 +182,39 @@ class ServiceInstanceDetailView(OrganizationViewMixin, DetailView):
             "context__service_definition__service",
         )
 
+    def get_object(self, **kwargs):
+        instance = super().get_object(**kwargs)
+        if (
+            not instance.kubernetes_object
+            and not instance.is_deleted
+            and not self._has_warned
+        ):
+            messages.warning(
+                self.request,
+                _(
+                    "Could not retrieve instance details from Kubernetes. It might have been deleted externally."
+                ),
+            )
+            self._has_warned = True
+        return instance
+
+
+class ServiceInstanceDetailView(
+    ServiceInstanceMixin, OrganizationViewMixin, DetailView
+):
+    template_name = "frontend/organizations/service_instance_detail.html"
+    permission_type = "view"
+
+    def get_context_data(self, **kwargs):
+        context = super().get_context_data(**kwargs)
+        if 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
+        )
+        return context
+
     def get_nested_spec(self):
         """
         Organize spec data into fieldsets similar to how the form does it.
@@ -272,12 +318,43 @@ class ServiceInstanceDetailView(OrganizationViewMixin, DetailView):
 
         return fieldsets
 
-    def get_context_data(self, **kwargs):
-        """Return service instance for the current organization."""
-        context = super().get_context_data(**kwargs)
-        if self.object.spec:
-            context["spec_fieldsets"] = self.get_nested_spec()
-        return context
+
+class ServiceInstanceUpdateView(
+    ServiceInstanceMixin, OrganizationViewMixin, HtmxUpdateView
+):
+    template_name = "frontend/organizations/service_instance_update.html"
+    permission_type = "change"
+
+    def get_form_class(self):
+        return self.object.context.model_form_class
+
+    def get_form_kwargs(self):
+        kwargs = super().get_form_kwargs()
+        kwargs["instance"] = self.object.spec_object
+        return kwargs
+
+    def form_valid(self, form):
+        try:
+            spec_data = form.get_nested_data().get("spec")
+            self.object.update_spec(spec_data=spec_data, updated_by=self.request.user)
+            messages.success(
+                self.request,
+                _("Service instance '{name}' updated successfully.").format(
+                    name=self.object.name
+                ),
+            )
+            return redirect(self.object.urls.base)
+        except ValidationError as e:
+            messages.error(self.request, e.message or str(e))
+            return self.form_invalid(form)
+        except Exception as e:
+            messages.error(
+                self.request, _("Error updating instance: {error}").format(error=str(e))
+            )
+            return self.form_invalid(form)
+
+    def get_success_url(self):
+        return self.object.urls.base
 
 
 class ServiceInstanceListView(OrganizationViewMixin, ListView):
@@ -293,7 +370,12 @@ class ServiceInstanceListView(OrganizationViewMixin, ListView):
     def get_queryset(self):
         """Return all service instances for the current organization with filtering."""
         queryset = ServiceInstance.objects.filter(
-            organization=self.request.organization
+            organization=self.request.organization,
+            is_deleted=False,  # Exclude soft-deleted
+        ).select_related(
+            "context__service_offering__provider",
+            "context__control_plane",
+            "context__service_definition__service",
         )
         if self.filter_form.is_valid():
             queryset = self.filter_form.filter_queryset(queryset)

src/servala/static/css/servala.css

@@ -85,3 +85,8 @@ html[data-bs-theme="dark"] .btn-outline-primary, .btn-outline-primary {
 a.btn-keycloak {
     display: inline-flex;
 }
+.page-heading h3 {
+    display: flex;
+    flex-wrap: wrap;
+    justify-content: space-between;
+}