Merge pull request 'Include link to support in error message' (#149) from error-with-help into main

Reviewed-on: #149

src/servala/core/models/organization.py

@@ -4,6 +4,7 @@ from django.conf import settings
 from django.db import models, transaction
 from django.utils.functional import cached_property
 from django.utils.text import slugify
+from django.utils.safestring import mark_safe
 from django.utils.translation import gettext_lazy as _
 from django_scopes import ScopedManager, scopes_disabled
 
@@ -74,6 +75,14 @@ class Organization(ServalaModelMixin, models.Model):
                 user=user, organization=self, role=OrganizationRole.OWNER
             )
 
+    def add_support_message(self, message):
+        support_message = _(
+            "Need help? We're happy to help via the <a href='{support_url}'>support form</a>."
+        ).format(support_url=self.urls.support)
+        return mark_safe(
+            f'{message} <i class="bi bi-person-raised-hand"></i> {support_message}'
+        )
+
     @classmethod
     @transaction.atomic
     def create_organization(cls, instance, owner):

src/servala/core/models/service.py

@@ -1,5 +1,7 @@
 import copy
+import html
 import json
+import re
 
 import kubernetes
 import rules
@@ -10,6 +12,7 @@ from django.core.exceptions import ValidationError
 from django.db import IntegrityError, models, transaction
 from django.utils import timezone
 from django.utils.functional import cached_property
+from django.utils.safestring import mark_safe
 from django.utils.translation import gettext_lazy as _
 from encrypted_fields.fields import EncryptedJSONField
 from kubernetes import client, config
@@ -603,6 +606,58 @@ class ServiceInstance(ServalaModelMixin, models.Model):
         spec_data = prune_empty_data(spec_data)
         return spec_data
 
+    @classmethod
+    def _format_kubernetes_error(cls, error_message):
+        if not error_message:
+            return {"message": "", "errors": None, "has_list": False}
+
+        error_message = str(error_message).strip()
+
+        # Pattern to match validation errors in brackets like [error1, error2, error3]
+        pattern = r"\[([^\]]+)\]"
+        match = re.search(pattern, error_message)
+
+        if not match:
+            return {"message": error_message, "errors": None, "has_list": False}
+
+        errors_text = match.group(1).strip()
+
+        if "," not in errors_text:
+            return {"message": error_message, "errors": None, "has_list": False}
+
+        errors = [error.strip().strip("\"'") for error in errors_text.split(",")]
+        errors = [error for error in errors if error]
+
+        if len(errors) <= 1:
+            return {"message": error_message, "errors": None, "has_list": False}
+
+        base_message = re.sub(pattern, "", error_message).strip()
+        base_message = base_message.rstrip(":").strip()
+
+        return {"message": base_message, "errors": errors, "has_list": True}
+
+    @classmethod
+    def _safe_format_error(cls, error_data):
+        if not isinstance(error_data, dict):
+            return html.escape(str(error_data))
+
+        if not error_data.get("has_list", False):
+            return html.escape(error_data.get("message", ""))
+
+        message = html.escape(error_data.get("message", ""))
+        errors = error_data.get("errors", [])
+
+        if not errors:
+            return message
+
+        escaped_errors = [html.escape(str(error)) for error in errors]
+        error_items = "".join(f"<li>{error}</li>" for error in escaped_errors)
+
+        if message:
+            return mark_safe(f"{message}<ul>{error_items}</ul>")
+        else:
+            return mark_safe(f"<ul>{error_items}</ul>")
+
     @classmethod
     def create_instance(cls, name, organization, context, created_by, spec_data):
         # Ensure the namespace exists
@@ -615,11 +670,10 @@ class ServiceInstance(ServalaModelMixin, models.Model):
                 context=context,
             )
         except IntegrityError:
-            raise ValidationError(
-                _(
+            message = _(
                 "An instance with this name already exists in this organization. Please choose a different name."
             )
-            )
+            raise ValidationError(organization.add_support_message(message))
 
         try:
             spec_data = cls._prepare_spec_data(spec_data)
@@ -657,10 +711,25 @@ class ServiceInstance(ServalaModelMixin, models.Model):
                 try:
                     error_body = json.loads(e.body)
                     reason = error_body.get("message", str(e))
-                    raise ValidationError(_("Kubernetes API error: {}").format(reason))
+                    error_data = cls._format_kubernetes_error(reason)
+                    formatted_reason = cls._safe_format_error(error_data)
+                    message = _("Error reported by control plane: {reason}").format(
+                        reason=formatted_reason
+                    )
+                    raise ValidationError(organization.add_support_message(message))
                 except (ValueError, TypeError):
-                    raise ValidationError(_("Kubernetes API error: {}").format(str(e)))
-            raise ValidationError(_("Error creating instance: {}").format(str(e)))
+                    error_data = cls._format_kubernetes_error(str(e))
+                    formatted_error = cls._safe_format_error(error_data)
+                    message = _("Error reported by control plane: {error}").format(
+                        error=formatted_error
+                    )
+                    raise ValidationError(organization.add_support_message(message))
+            error_data = cls._format_kubernetes_error(str(e))
+            formatted_error = cls._safe_format_error(error_data)
+            message = _("Error creating instance: {error}").format(
+                error=formatted_error
+            )
+            raise ValidationError(organization.add_support_message(message))
         return instance
 
     def update_spec(self, spec_data, updated_by):
@@ -681,29 +750,33 @@ class ServiceInstance(ServalaModelMixin, models.Model):
             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."
-                    )
+                message = _(
+                    "Service instance not found in control plane. It may have been deleted externally."
                 )
+                raise ValidationError(self.organization.add_support_message(message))
             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
-                    )
-                )
+                error_data = self._format_kubernetes_error(reason)
+                formatted_reason = self._safe_format_error(error_data)
+                message = _(
+                    "Error reported by control plane while updating instance: {reason}"
+                ).format(reason=formatted_reason)
+                raise ValidationError(self.organization.add_support_message(message))
             except (ValueError, TypeError):
-                raise ValidationError(
-                    _("Kubernetes API error updating instance: {error}").format(
-                        error=str(e)
-                    )
-                )
+                error_data = self._format_kubernetes_error(str(e))
+                formatted_error = self._safe_format_error(error_data)
+                message = _(
+                    "Error reported by control plane while updating instance: {error}"
+                ).format(error=formatted_error)
+                raise ValidationError(self.organization.add_support_message(message))
         except Exception as e:
-            raise ValidationError(
-                _("Error updating instance: {error}").format(error=str(e))
+            error_data = self._format_kubernetes_error(str(e))
+            formatted_error = self._safe_format_error(error_data)
+            message = _("Error updating instance: {error}").format(
+                error=formatted_error
             )
+            raise ValidationError(self.organization.add_support_message(message))
 
     @transaction.atomic
     def delete_instance(self, user):

src/servala/frontend/templates/includes/k8s_error.html

@@ -0,0 +1,14 @@
+{% if show_error %}
+<div class="{{ css_class }}">
+    {% if has_list %}
+        {% if message %}{{ message }}{% endif %}
+        <ul>
+        {% for error in errors %}
+            <li>{{ error }}</li>
+        {% endfor %}
+        </ul>
+    {% else %}
+        {{ message }}
+    {% endif %}
+</div>
+{% endif %}

src/servala/frontend/templates/includes/message.html

@@ -9,7 +9,7 @@
 <script>
 document.addEventListener('DOMContentLoaded', function() {
     const alert = document.getElementById('auto-dismiss-alert-{{ forloop.counter0|default:'0' }}');
-    if (alert) {
+    if (alert && !alert.classList.contains('alert-danger')) {
         setTimeout(function() {
             let opacity = 1;
             const fadeOutInterval = setInterval(function() {

src/servala/frontend/templatetags/error_filters.py

@@ -0,0 +1,43 @@
+"""
+Template filters for safe error formatting.
+"""
+
+import html
+from django import template
+from django.utils.safestring import mark_safe
+
+register = template.Library()
+
+
+@register.filter
+def format_k8s_error(error_data):
+    """
+    Template filter to safely format Kubernetes error data.
+    Usage: {{ error_data|format_k8s_error }}
+
+    Args:
+        error_data: Dictionary with structure from _format_kubernetes_error method
+                   or a simple string
+
+    Returns:
+        Safely formatted HTML string
+    """
+    if not error_data:
+        return ""
+
+    if not error_data.get("has_list", False):
+        return html.escape(error_data.get("message", ""))
+
+    message = html.escape(error_data.get("message", ""))
+    errors = error_data.get("errors", [])
+
+    if not errors:
+        return message
+
+    escaped_errors = [html.escape(str(error)) for error in errors]
+    error_items = "".join(f"<li>{error}</li>" for error in escaped_errors)
+
+    if message:
+        return mark_safe(f"{message}<ul>{error_items}</ul>")
+    else:
+        return mark_safe(f"<ul>{error_items}</ul>")

src/servala/frontend/views/service.py

@@ -144,7 +144,12 @@ class ServiceOfferingDetailView(OrganizationViewMixin, HtmxViewMixin, DetailView
 
         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."))
+            messages.error(
+                self.request,
+                self.organization.add_support_message(
+                    _("Could not initialize service form.")
+                ),
+            )
             return self.render_to_response(context)
 
         if form.is_valid():
@@ -161,7 +166,10 @@ class ServiceOfferingDetailView(OrganizationViewMixin, HtmxViewMixin, DetailView
                 messages.error(self.request, e.message or str(e))
             except Exception as e:
                 messages.error(
-                    self.request, _("Error creating instance: {}").format(str(e))
+                    self.request,
+                    self.organization.add_support_message(
+                        _(f"Error creating instance: {str(e)}.")
+                    ),
                 )
 
         # If the form is not valid or if the service creation failed, we render it again
@@ -366,7 +374,10 @@ class ServiceInstanceUpdateView(
             return self.form_invalid(form)
         except Exception as e:
             messages.error(
-                self.request, _("Error updating instance: {error}").format(error=str(e))
+                self.request,
+                self.organization.add_support_message(
+                    _(f"Error updating instance: {str(e)}.")
+                ),
             )
             return self.form_invalid(form)
 
@@ -435,9 +446,11 @@ class ServiceInstanceDeleteView(
         except Exception as e:
             messages.error(
                 self.request,
+                self.organization.add_support_message(
                     _(
-                    "An error occurred while trying to delete instance '{name}': {error}"
-                ).format(name=self.object.name, error=str(e)),
+                        f"An error occurred while trying to delete instance '{self.object.name}': {str(e)}."
+                    )
+                ),
             )
             response = HttpResponse()
             response["HX-Redirect"] = str(self.object.urls.base)