diff --git a/.env.example b/.env.example index 63df700..9e31fbd 100644 --- a/.env.example +++ b/.env.example @@ -77,3 +77,7 @@ SERVALA_ODOO_HELPDESK_TEAM_ID='5' # OSB API authentication settings SERVALA_OSB_USERNAME='' SERVALA_OSB_PASSWORD='' + +# Prefix for auto-generated Kubernetes resource names for service instances. +# Format: {prefix}-{hash}. Defaults to 'si' (service instance). +SERVALA_INSTANCE_NAME_PREFIX='si' diff --git a/README.md b/README.md index eaa1cdd..f435a90 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,32 @@ Then access it with http://localhost:8080/ and the Django admin with http://loca Deployment files are in the `deployment/kustomize` folder and makes use of [Kustomize](https://kustomize.io/) to account for differences between the deployment stages. Stages are configured with overlays in `deployment/kustomize/overlays/$environment`. +### Resource Requests and Limits + +Resources are configured to comply with APPUiO Cloud's memory-to-CPU ratio of 4096 MiB/core. +See [APPUiO Cloud documentation](https://docs.appuio.cloud/user/how-to/check-cpu-requests.html) for details. + +**Production:** + +| Container | CPU Request | Memory Request | CPU Limit | Memory Limit | +|-----------|-------------|----------------|-----------|--------------| +| servala | 500m | 2Gi | 2 | 4Gi | + +**Staging:** + +| Container | CPU Request | Memory Request | CPU Limit | Memory Limit | +|------------------|-------------|----------------|-----------|--------------| +| servala | 250m | 1Gi | 1 | 2Gi | +| ssh-tunnel-dev | 50m | 204Mi | 100m | 256Mi | +| ssh-tunnel-talos | 50m | 204Mi | 100m | 256Mi | + +**Ratio Calculation:** + +The ratio is calculated as: `Sum of Memory Requests / Sum of CPU Requests` + +- Production: 2048 MiB / 0.5 cores = **4096 MiB/core** ✓ +- Staging: (1024 + 204 + 204) MiB / (0.25 + 0.05 + 0.05) cores = 1432 MiB / 0.35 cores = **4091 MiB/core** ✓ + ### Staging The code is automatically built and deployed on a push to the main branch. diff --git a/deployment/kustomize/base/portal/deployment.yaml b/deployment/kustomize/base/portal/deployment.yaml index 03deb98..1b33aae 100644 --- a/deployment/kustomize/base/portal/deployment.yaml +++ b/deployment/kustomize/base/portal/deployment.yaml @@ -21,6 +21,13 @@ spec: - name: servala image: servala.app.codey.ch/servala/servala-portal:latest imagePullPolicy: Always + resources: + requests: + cpu: 500m + memory: 2Gi + limits: + cpu: 2 + memory: 4Gi ports: - name: http containerPort: 8080 diff --git a/deployment/kustomize/overlays/production/portal-deployment.yaml b/deployment/kustomize/overlays/production/portal-deployment.yaml index 47574f4..bc6050f 100644 --- a/deployment/kustomize/overlays/production/portal-deployment.yaml +++ b/deployment/kustomize/overlays/production/portal-deployment.yaml @@ -32,10 +32,3 @@ spec: secretKeyRef: name: portal-storage-creds key: AWS_SECRET_ACCESS_KEY - resources: - limits: - cpu: 2 - memory: 2Gi - requests: - cpu: 500m - memory: 512Mi diff --git a/deployment/kustomize/overlays/staging/portal-deployment.yaml b/deployment/kustomize/overlays/staging/portal-deployment.yaml index 0c13f2d..e978ccd 100644 --- a/deployment/kustomize/overlays/staging/portal-deployment.yaml +++ b/deployment/kustomize/overlays/staging/portal-deployment.yaml @@ -7,6 +7,13 @@ spec: spec: containers: - name: servala + resources: + requests: + cpu: 250m + memory: 1Gi + limits: + cpu: 1 + memory: 2Gi env: - name: SERVALA_ENVIRONMENT value: staging @@ -32,8 +39,15 @@ spec: secretKeyRef: name: portal-storage-creds key: AWS_SECRET_ACCESS_KEY - - name: ssh-tunnel + - name: ssh-tunnel-dev image: servala.app.codey.ch/servala/servala-portal:latest + resources: + requests: + cpu: 50m + memory: 204Mi + limits: + cpu: 100m + memory: 256Mi command: - "/bin/bash" - "-c" @@ -41,13 +55,40 @@ spec: mkdir -p /app/.ssh && chmod 700 /app/.ssh echo "$SSH_PRIVATE_KEY" > /app/.ssh/id chmod 600 /app/.ssh/id - ssh $SSH_HOST -l $SSH_USER -o StrictHostKeyChecking=no -L 8443:127.0.0.1:8443 -N -i /app/.ssh/id -v + ssh $SSH_HOST -l $SSH_USER -o StrictHostKeyChecking=no -L 6443:127.0.0.1:6443 -N -i /app/.ssh/id -v env: - name: SSH_HOST - valueFrom: - secretKeyRef: - name: servala-sshclient - key: ssh-host + value: "78.47.176.209" + - name: SSH_USER + valueFrom: + secretKeyRef: + name: servala-sshclient + key: ssh-user + - name: SSH_PRIVATE_KEY + valueFrom: + secretKeyRef: + name: servala-sshclient + key: ssh-private-key + - name: ssh-tunnel-talos + image: servala.app.codey.ch/servala/servala-portal:latest + resources: + requests: + cpu: 50m + memory: 204Mi + limits: + cpu: 100m + memory: 256Mi + command: + - "/bin/bash" + - "-c" + - | + mkdir -p /app/.ssh && chmod 700 /app/.ssh + echo "$SSH_PRIVATE_KEY" > /app/.ssh/id + chmod 600 /app/.ssh/id + ssh $SSH_HOST -l $SSH_USER -o StrictHostKeyChecking=no -L 6444:172.18.200.10:6443 -N -i /app/.ssh/id -v + env: + - name: SSH_HOST + value: mgmt.cls-rma1-9c02.servala.com - name: SSH_USER valueFrom: secretKeyRef: diff --git a/pyproject.toml b/pyproject.toml index 0e895cc..bb254a9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,15 +28,15 @@ dependencies = [ [dependency-groups] dev = [ - "black>=25.11.0", + "black>=25.12.0", "bumpver>=2025.1131", - "coverage>=7.12.0", + "coverage>=7.13.0", "djlint>=1.36.4", "flake8>=7.3.0", "flake8-bugbear>=25.11.29", "flake8-pyproject>=1.2.4", "isort>=7.0.0", - "pytest>=9.0.1", + "pytest>=9.0.2", "pytest-cov>=7.0.0", "pytest-django>=4.11.1", "pytest-mock>=3.15.1", diff --git a/src/servala/api/views.py b/src/servala/api/views.py index 9b0082d..0bca7a7 100644 --- a/src/servala/api/views.py +++ b/src/servala/api/views.py @@ -301,7 +301,7 @@ The Servala Team""" "core_serviceinstance_change", service_instance.pk ) description_parts.append( - f"Instance: {service_instance.name} - {instance_url}" + f"Instance: {service_instance.display_name} ({service_instance.name}) - {instance_url}" ) else: description_parts.append(f"Instance: {instance_id}") diff --git a/src/servala/core/crd/forms.py b/src/servala/core/crd/forms.py index 18df8bc..6bde2c9 100644 --- a/src/servala/core/crd/forms.py +++ b/src/servala/core/crd/forms.py @@ -9,17 +9,17 @@ from servala.core.models import ControlPlaneCRD from servala.frontend.forms.widgets import DynamicArrayWidget, NumberInputWithAddon # Fields that must be present in every form -MANDATORY_FIELDS = ["name"] +MANDATORY_FIELDS = ["display_name"] # Default field configurations - fields that can be included with just a mapping # to avoid administrators having to duplicate common information DEFAULT_FIELD_CONFIGS = { - "name": { + "display_name": { "type": "text", "label": "Instance Name", - "help_text": "Unique name for the new instance", + "help_text": "", "required": True, - "max_length": 63, + "max_length": 100, }, "spec.parameters.service.fqdn": { "type": "array", @@ -51,11 +51,6 @@ class FormGeneratorMixin: crd = getattr(crd, "pk", crd) # can be int or object self.fields["context"].queryset = ControlPlaneCRD.objects.filter(pk=crd) - if self.instance and hasattr(self.instance, "name") and self.instance.name: - if "name" in self.fields: - self.fields["name"].disabled = True - self.fields["name"].widget = forms.HiddenInput() - def has_mandatory_fields(self, field_list): for field_name in field_list: if field_name in self.fields and self.fields[field_name].required: @@ -307,15 +302,6 @@ class CustomFormMixin(FormGeneratorMixin): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self._apply_field_config() - if ( - self.instance - and hasattr(self.instance, "name") - and self.instance.name - and "name" in self.fields - ): - self.fields["name"].widget = forms.HiddenInput() - self.fields["name"].disabled = True - self.fields.pop("context", None) def _apply_field_config(self): for fieldset in self.form_config.get("fieldsets", []): @@ -472,7 +458,7 @@ def generate_custom_form_class(form_config, model): """ Generate a custom (user-friendly) form class from form_config JSON. """ - field_list = ["context", "name"] + field_list = ["context", "display_name"] for fieldset in form_config.get("fieldsets", []): for field_config in fieldset.get("fields", []): diff --git a/src/servala/core/crd/models.py b/src/servala/core/crd/models.py index 86df97f..a4fcc28 100644 --- a/src/servala/core/crd/models.py +++ b/src/servala/core/crd/models.py @@ -23,9 +23,9 @@ def generate_django_model(schema, group, version, kind): """ Generates a virtual Django model from a Kubernetes CRD's OpenAPI v3 schema. """ - # We always need these three fields to know our own name and our full namespace + # We always need these fields to know our display name and our full namespace model_fields = {"__module__": "crd_models"} - for field_name in ("name", "context"): + for field_name in ("display_name", "context"): model_fields[field_name] = duplicate_field(field_name, ServiceInstance) # All other fields are generated from the schema, except for the diff --git a/src/servala/core/forms.py b/src/servala/core/forms.py index 090abba..3bad850 100644 --- a/src/servala/core/forms.py +++ b/src/servala/core/forms.py @@ -254,7 +254,7 @@ class ServiceDefinitionAdminForm(forms.ModelForm): if not schema or not (spec_schema := schema.get("properties", {}).get("spec")): return - valid_paths = self._extract_field_paths(spec_schema, "spec") | {"name"} + valid_paths = self._extract_field_paths(spec_schema, "spec") | {"display_name"} included_mappings = set() errors = [] for fieldset in form_config.get("fieldsets", []): diff --git a/src/servala/core/management/commands/sync_billing_metadata.py b/src/servala/core/management/commands/sync_billing_metadata.py index 2093948..ad1d74b 100644 --- a/src/servala/core/management/commands/sync_billing_metadata.py +++ b/src/servala/core/management/commands/sync_billing_metadata.py @@ -195,6 +195,7 @@ class Command(BaseCommand): compute_plan_assignment=instance.compute_plan_assignment, control_plane=instance.context.control_plane, instance_name=instance.name, + display_name=instance.display_name, organization=instance.organization, service=instance.context.service_offering.service, ) diff --git a/src/servala/core/migrations/0019_add_display_name.py b/src/servala/core/migrations/0019_add_display_name.py new file mode 100644 index 0000000..4ff3b92 --- /dev/null +++ b/src/servala/core/migrations/0019_add_display_name.py @@ -0,0 +1,46 @@ +from django.db import migrations, models + +import servala.core.validators + + +def populate_display_name(apps, schema_editor): + """ + For existing instances, copy name to display_name. + Existing instances already have their name matching the k8s resource, + so we cannot add a prefix. + """ + ServiceInstance = apps.get_model("core", "ServiceInstance") + for instance in ServiceInstance.objects.all(): + instance.display_name = instance.name + instance.save(update_fields=["display_name"]) + + +class Migration(migrations.Migration): + dependencies = [ + ("core", "0018_add_invoice_grouping_to_organization_origin"), + ] + + operations = [ + migrations.AlterField( + model_name="serviceinstance", + name="name", + field=models.CharField( + max_length=63, + validators=[servala.core.validators.kubernetes_name_validator], + verbose_name="Instance ID", + ), + ), + # Add display_name field with a temporary default + migrations.AddField( + model_name="serviceinstance", + name="display_name", + field=models.CharField( + default="", + help_text="Display name for this instance", + max_length=100, + verbose_name="Name", + ), + preserve_default=False, + ), + migrations.RunPython(populate_display_name, migrations.RunPython.noop), + ] diff --git a/src/servala/core/models/service.py b/src/servala/core/models/service.py index e973235..31527c5 100644 --- a/src/servala/core/models/service.py +++ b/src/servala/core/models/service.py @@ -1,4 +1,5 @@ import copy +import hashlib import html import json import re @@ -615,8 +616,16 @@ class ServiceInstance(ServalaModelMixin, models.Model): on the fly. """ + # The Kubernetes resource name (metadata.name). This field is immutable after + # creation and is auto-generated for new instances. Do not modify directly! name = models.CharField( - max_length=63, verbose_name=_("Name"), validators=[kubernetes_name_validator] + max_length=63, + verbose_name=_("Instance ID"), + validators=[kubernetes_name_validator], + ) + display_name = models.CharField( + max_length=100, + verbose_name=_("Name"), ) organization = models.ForeignKey( to="core.Organization", @@ -686,6 +695,24 @@ class ServiceInstance(ServalaModelMixin, models.Model): spec_data = prune_empty_data(spec_data) return spec_data + @staticmethod + def generate_resource_name(organization, display_name, service, attempt=0): + """ + Generate a unique Kubernetes-compatible resource name. + + Format: {prefix}-{sha256[:8]} + + The hash input is: org_slug:display_name:service_slug[:attempt if > 0] + On collision, we retry with an incremented attempt number included in hash. + """ + hash_input = ( + f"{organization.slug}:{display_name.lower().strip()}:{service.slug}" + ) + if attempt > 0: + hash_input += f":{attempt}" + hash_value = hashlib.sha256(hash_input.encode("utf-8")).hexdigest()[:8] + return f"{settings.SERVALA_INSTANCE_NAME_PREFIX}-{hash_value}" + @staticmethod def _apply_compute_plan_to_spec(spec_data, compute_plan_assignment): """ @@ -719,16 +746,20 @@ class ServiceInstance(ServalaModelMixin, models.Model): compute_plan_assignment, control_plane, instance_name=None, + display_name=None, organization=None, service=None, ): """ - Build Kubernetes annotations for billing integration. + Build Kubernetes annotations for billing integration and display name. """ from servala.core.models.organization import InvoiceGroupingChoice annotations = {} + if display_name: + annotations["servala.com/displayName"] = display_name + if compute_plan_assignment: annotations["servala.com/erp_product_id_resource"] = str( compute_plan_assignment.odoo_product_id @@ -830,18 +861,35 @@ class ServiceInstance(ServalaModelMixin, models.Model): @transaction.atomic def create_instance( cls, - name, + display_name, organization, context, created_by, spec_data, compute_plan_assignment=None, ): + service = context.service_offering.service + name = None + for attempt in range(10): + name = cls.generate_resource_name( + organization, display_name, service, attempt + ) + if not cls.objects.filter( + name=name, organization=organization, context=context + ).exists(): + break + else: + message = _( + "Could not generate a unique resource name. Please try a different display name." + ) + raise ValidationError(organization.add_support_message(message)) + # Ensure the namespace exists context.control_plane.get_or_create_namespace(organization) try: instance = cls.objects.create( name=name, + display_name=display_name, organization=organization, created_by=created_by, context=context, @@ -883,8 +931,9 @@ class ServiceInstance(ServalaModelMixin, models.Model): compute_plan_assignment=compute_plan_assignment, control_plane=context.control_plane, instance_name=name, + display_name=display_name, organization=organization, - service=context.service_offering.service, + service=service, ) if annotations: create_data["metadata"]["annotations"] = annotations @@ -941,6 +990,7 @@ class ServiceInstance(ServalaModelMixin, models.Model): compute_plan_assignment=plan_to_use, control_plane=self.context.control_plane, instance_name=self.name, + display_name=self.display_name, organization=self.organization, service=self.context.service_offering.service, ) @@ -1065,7 +1115,7 @@ class ServiceInstance(ServalaModelMixin, models.Model): if not self.context.django_model: return return self.context.django_model( - name=self.name, + display_name=self.display_name, context=self.context, spec=self.spec, # We pass -1 as ID in order to make it clear that a) this object exists (remotely), diff --git a/src/servala/frontend/forms/service.py b/src/servala/frontend/forms/service.py index 169d6ea..26c9c70 100644 --- a/src/servala/frontend/forms/service.py +++ b/src/servala/frontend/forms/service.py @@ -123,7 +123,7 @@ class ServiceInstanceFilterForm(forms.Form): class ServiceInstanceDeleteForm(forms.ModelForm): name = forms.CharField( - label=_("Instance Name"), + label=_("Instance ID"), max_length=63, widget=forms.TextInput(attrs={"class": "form-control"}), ) @@ -132,7 +132,7 @@ class ServiceInstanceDeleteForm(forms.ModelForm): kwargs["initial"] = {"name": ""} super().__init__(*args, **kwargs) self.fields["name"].help_text = _( - "To confirm deletion, please type the instance name: {instance_name}" + "To confirm deletion, please type the instance ID: {instance_name}" ).format(instance_name=self.instance.name) def clean_name(self): @@ -140,7 +140,7 @@ class ServiceInstanceDeleteForm(forms.ModelForm): if entered_name != self.instance.name: raise forms.ValidationError( _( - "The entered name does not match the instance name. Deletion not confirmed." + "The entered name does not match the instance ID. Deletion not confirmed." ) ) return entered_name diff --git a/src/servala/frontend/templates/frontend/forms/errors.html b/src/servala/frontend/templates/frontend/forms/errors.html index 964687d..1d31737 100644 --- a/src/servala/frontend/templates/frontend/forms/errors.html +++ b/src/servala/frontend/templates/frontend/forms/errors.html @@ -11,7 +11,7 @@ {{ form.non_field_errors.0 }} {% endif %} {% else %} - {% translate "We could not save your changes." %} + {% translate "Please review and correct the errors highlighted in the form below." %} {% endif %} diff --git a/src/servala/frontend/templates/frontend/organizations/dashboard.html b/src/servala/frontend/templates/frontend/organizations/dashboard.html index 441d0be..9a309f1 100644 --- a/src/servala/frontend/templates/frontend/organizations/dashboard.html +++ b/src/servala/frontend/templates/frontend/organizations/dashboard.html @@ -96,7 +96,7 @@ {{ instance.name }} + class="fw-semibold text-decoration-none">{{ instance.display_name }}
diff --git a/src/servala/frontend/templates/frontend/organizations/service_offering_detail.html b/src/servala/frontend/templates/frontend/organizations/service_instance_create.html similarity index 98% rename from src/servala/frontend/templates/frontend/organizations/service_offering_detail.html rename to src/servala/frontend/templates/frontend/organizations/service_instance_create.html index 39b69a8..3c41ddf 100644 --- a/src/servala/frontend/templates/frontend/organizations/service_offering_detail.html +++ b/src/servala/frontend/templates/frontend/organizations/service_instance_create.html @@ -26,7 +26,8 @@ {% translate "Oops! Something went wrong with the service form generation. Please try again later." %}
{% else %} - {% include "includes/tabbed_fieldset_form.html" with form=custom_service_form expert_form=service_form %} + {% translate "Create" as create_label %} + {% include "includes/tabbed_fieldset_form.html" with form=custom_service_form expert_form=service_form form_submit_label=create_label hide_form_errors=True %} {% endif %} diff --git a/src/servala/frontend/templates/frontend/organizations/service_instance_delete_form.html b/src/servala/frontend/templates/frontend/organizations/service_instance_delete_form.html index 5296c56..3129814 100644 --- a/src/servala/frontend/templates/frontend/organizations/service_instance_delete_form.html +++ b/src/servala/frontend/templates/frontend/organizations/service_instance_delete_form.html @@ -7,7 +7,7 @@ {% csrf_token %}
diff --git a/src/servala/frontend/templates/frontend/organizations/service_instances.html b/src/servala/frontend/templates/frontend/organizations/service_instances.html index c443e6e..20c337e 100644 --- a/src/servala/frontend/templates/frontend/organizations/service_instances.html +++ b/src/servala/frontend/templates/frontend/organizations/service_instances.html @@ -23,6 +23,7 @@ {% translate "Name" %} + {% translate "Instance ID" %} {% translate "Service" %} {% translate "Service Provider" %} {% translate "Service Provider Zone" %} @@ -33,7 +34,10 @@ {% for instance in instances %} - {{ instance.name }} + {{ instance.display_name }} + + + {{ instance.name }} {{ instance.context.service_definition.service.name }} {{ instance.context.service_offering.provider.name }} @@ -42,7 +46,7 @@ {% empty %} - {% translate "No service instances found." %} + {% translate "No service instances found." %} {% endfor %} diff --git a/src/servala/frontend/templates/includes/tabbed_fieldset_form.html b/src/servala/frontend/templates/includes/tabbed_fieldset_form.html index f54b1ae..c72976a 100644 --- a/src/servala/frontend/templates/includes/tabbed_fieldset_form.html +++ b/src/servala/frontend/templates/includes/tabbed_fieldset_form.html @@ -1,7 +1,9 @@ {% load i18n %} {% load get_field %} {% load static %} -{% include "frontend/forms/errors.html" %} +{% if not hide_form_errors %} + {% include "frontend/forms/errors.html" %} +{% endif %} {% if form and expert_form and not hide_expert_mode %}
/offering//", - views.ServiceOfferingDetailView.as_view(), - name="organization.offering", + views.ServiceInstanceCreateView.as_view(), + name="organization.instance.create", ), path( "", diff --git a/src/servala/frontend/views/__init__.py b/src/servala/frontend/views/__init__.py index 33b0560..3307754 100644 --- a/src/servala/frontend/views/__init__.py +++ b/src/servala/frontend/views/__init__.py @@ -16,12 +16,12 @@ from .organization import ( ) from .service import ( ServiceDetailView, + ServiceInstanceCreateView, ServiceInstanceDeleteView, ServiceInstanceDetailView, ServiceInstanceListView, ServiceInstanceUpdateView, ServiceListView, - ServiceOfferingDetailView, ) from .support import SupportView @@ -35,12 +35,12 @@ __all__ = [ "OrganizationSelectionView", "OrganizationUpdateView", "ServiceDetailView", + "ServiceInstanceCreateView", "ServiceInstanceDeleteView", "ServiceInstanceDetailView", "ServiceInstanceListView", "ServiceInstanceUpdateView", "ServiceListView", - "ServiceOfferingDetailView", "ProfileView", "SupportView", "custom_404", diff --git a/src/servala/frontend/views/service.py b/src/servala/frontend/views/service.py index fe439c6..2a858dc 100644 --- a/src/servala/frontend/views/service.py +++ b/src/servala/frontend/views/service.py @@ -83,7 +83,7 @@ class ServiceDetailView(OrganizationViewMixin, DetailView): if self.visible_offerings.count() == 1: offering = self.visible_offerings.first() return redirect( - "frontend:organization.offering", + "frontend:organization.instance.create", organization=self.request.organization.slug, slug=self.object.slug, pk=offering.pk, @@ -97,8 +97,8 @@ class ServiceDetailView(OrganizationViewMixin, DetailView): return context -class ServiceOfferingDetailView(OrganizationViewMixin, HtmxViewMixin, DetailView): - template_name = "frontend/organizations/service_offering_detail.html" +class ServiceInstanceCreateView(OrganizationViewMixin, HtmxViewMixin, DetailView): + template_name = "frontend/organizations/service_instance_create.html" context_object_name = "offering" model = ServiceOffering permission_type = "view" @@ -276,7 +276,7 @@ class ServiceOfferingDetailView(OrganizationViewMixin, HtmxViewMixin, DetailView try: service_instance = ServiceInstance.create_instance( organization=self.request.organization, - name=form.cleaned_data["name"], + display_name=form.cleaned_data["display_name"], context=self.context_object, created_by=request.user, spec_data=form.get_nested_data().get("spec"), @@ -762,6 +762,9 @@ class ServiceInstanceUpdateView( current_spec = dict(self.object.spec) if self.object.spec else {} spec_data = self._deep_merge(current_spec, spec_data) + if display_name := form_data.get("display_name"): + self.object.display_name = display_name + compute_plan_assignment = None if self.plan_form.is_valid(): compute_plan_assignment = self.plan_form.cleaned_data.get( @@ -776,7 +779,7 @@ class ServiceInstanceUpdateView( messages.success( self.request, _("Service instance '{name}' updated successfully.").format( - name=self.object.name + name=self.object.display_name ), ) return redirect(self.object.urls.base) @@ -837,7 +840,7 @@ class ServiceInstanceDeleteView( messages.success( self.request, _("Service instance '{name}' has been scheduled for deletion.").format( - name=self.object.name + name=self.object.display_name ), ) response = HttpResponse() @@ -848,7 +851,7 @@ class ServiceInstanceDeleteView( self.request, self.organization.add_support_message( _( - f"An error occurred while trying to delete instance '{self.object.name}': {str(e)}." + f"An error occurred while trying to delete instance '{self.object.display_name}': {str(e)}." ) ), ) diff --git a/src/servala/settings.py b/src/servala/settings.py index f57ac0d..1319394 100644 --- a/src/servala/settings.py +++ b/src/servala/settings.py @@ -270,6 +270,9 @@ SESSION_COOKIE_SECURE = not DEBUG DEFAULT_LABEL_KEY = "appcat.vshn.io/provider-config" DEFAULT_AUTO_FIELD = "django.db.models.BigAutoField" +# Prefix for auto-generated Kubernetes resource names for service instances +SERVALA_INSTANCE_NAME_PREFIX = os.environ.get("SERVALA_INSTANCE_NAME_PREFIX", "si") + # TODO TIME_ZONE = "UTC" diff --git a/src/servala/static/js/fqdn.js b/src/servala/static/js/fqdn.js index e775ebc..86964c4 100644 --- a/src/servala/static/js/fqdn.js +++ b/src/servala/static/js/fqdn.js @@ -1,6 +1,6 @@ const initializeFqdnGeneration = (prefix) => { - const nameField = document.querySelector(`input#id_${prefix}-name`); + const nameField = document.querySelector(`input#id_${prefix}-display_name`); if (!nameField) return // Try to find array input first (DynamicArrayWidget), then fallback to regular text input @@ -23,9 +23,19 @@ const initializeFqdnGeneration = (prefix) => { if (!fqdnField) return if (nameField && fqdnField) { + const sanitizeForFqdn = (name) => { + return name + .toLowerCase() + .replace(/[^a-z0-9-]/g, '-') // Replace invalid chars with hyphens + .replace(/-+/g, '-') // Collapse multiple hyphens + .replace(/^-|-$/g, ''); // Trim hyphens from start/end + } + const generateFqdn = (instanceName) => { if (!instanceName) return ''; - return `${instanceName}-${fqdnConfig.namespace}.${fqdnConfig.wildcardDns}`; + const sanitized = sanitizeForFqdn(instanceName); + if (!sanitized) return '' + return `${sanitized}-${fqdnConfig.namespace}.${fqdnConfig.wildcardDns}`; } nameField.addEventListener('input', function() { diff --git a/src/tests/conftest.py b/src/tests/conftest.py index 859b4ca..298eb42 100644 --- a/src/tests/conftest.py +++ b/src/tests/conftest.py @@ -18,6 +18,7 @@ from servala.core.models.service import ( Service, ServiceCategory, ServiceDefinition, + ServiceInstance, ServiceOffering, ) @@ -44,15 +45,38 @@ def other_organization(origin): @pytest.fixture def org_owner(organization): - user = User.objects.create(email="user@example.org", password="example") + owner = User.objects.create(email="owner@example.org", password="example") OrganizationMembership.objects.create( - organization=organization, user=user, role="owner" + organization=organization, user=owner, role="owner" ) - return user + return owner @pytest.fixture -def test_service_category(): +def org_admin(organization): + admin = User.objects.create(email="admin@example.org", password="example") + OrganizationMembership.objects.create( + organization=organization, user=admin, role="admin" + ) + return admin + + +@pytest.fixture +def org_member(organization): + member = User.objects.create(email="member@example.org", password="example") + OrganizationMembership.objects.create( + organization=organization, user=member, role="member" + ) + return member + + +@pytest.fixture +def user(): + return User.objects.create(email="generic-user@example.org", password="example") + + +@pytest.fixture +def service_category(): return ServiceCategory.objects.create( name="Databases", description="Database services", @@ -60,18 +84,18 @@ def test_service_category(): @pytest.fixture -def test_service(test_service_category): +def service(service_category): return Service.objects.create( name="Redis", slug="redis", - category=test_service_category, + category=service_category, description="Redis database service", osb_service_id="test-service-123", ) @pytest.fixture -def test_cloud_provider(): +def cloud_provider(): return CloudProvider.objects.create( name="Exoscale", description="Exoscale cloud provider", @@ -79,10 +103,10 @@ def test_cloud_provider(): @pytest.fixture -def test_service_offering(test_service, test_cloud_provider): +def service_offering(service, cloud_provider): return ServiceOffering.objects.create( - service=test_service, - provider=test_cloud_provider, + service=service, + provider=cloud_provider, description="Redis on Exoscale", osb_plan_id="test-plan-123", ) @@ -125,11 +149,11 @@ def mock_odoo_failure(mocker): @pytest.fixture -def test_control_plane(test_cloud_provider): +def control_plane(cloud_provider): return ControlPlane.objects.create( name="Geneva (CH-GVA-2)", description="Geneva control plane", - cloud_provider=test_cloud_provider, + cloud_provider=cloud_provider, api_credentials={ "server": "https://k8s.example.com", "token": "test-token", @@ -139,10 +163,10 @@ def test_control_plane(test_cloud_provider): @pytest.fixture -def test_service_definition(test_service): +def service_definition(service): return ServiceDefinition.objects.create( name="Redis Standard", - service=test_service, + service=service, api_definition={ "group": "vshn.appcat.vshn.io", "version": "v1", @@ -152,13 +176,11 @@ def test_service_definition(test_service): @pytest.fixture -def test_control_plane_crd( - test_service_offering, test_control_plane, test_service_definition -): +def control_plane_crd(service_offering, control_plane, service_definition): return ControlPlaneCRD.objects.create( - service_offering=test_service_offering, - control_plane=test_control_plane, - service_definition=test_service_definition, + service_offering=service_offering, + control_plane=control_plane, + service_definition=service_definition, ) @@ -176,10 +198,10 @@ def compute_plan(): @pytest.fixture -def compute_plan_assignment(compute_plan, test_control_plane_crd): +def compute_plan_assignment(compute_plan, control_plane_crd): return ComputePlanAssignment.objects.create( compute_plan=compute_plan, - control_plane_crd=test_control_plane_crd, + control_plane_crd=control_plane_crd, sla="besteffort", odoo_product_id="test-product-id", odoo_unit_id="test-unit-id", @@ -187,3 +209,30 @@ def compute_plan_assignment(compute_plan, test_control_plane_crd): unit="hour", is_active=True, ) + + +@pytest.fixture +def service_instance(organization, control_plane_crd): + return ServiceInstance.objects.create( + name="test-abc12345", + display_name="My Test Instance", + organization=organization, + context=control_plane_crd, + ) + + +@pytest.fixture +def control_plane_with_storage(cloud_provider): + return ControlPlane.objects.create( + name="Storage Zone", + description="Zone with storage billing", + cloud_provider=cloud_provider, + api_credentials={ + "server": "https://k8s.example.com", + "token": "test-token", + "certificate-authority-data": "test-ca-data", + }, + storage_plan_odoo_product_id="storage-product-123", + storage_plan_odoo_unit_id="storage-unit-456", + storage_plan_price_per_gib="0.10", + ) diff --git a/src/tests/test_api_exoscale.py b/src/tests/test_api_exoscale.py index 37542e5..4362748 100644 --- a/src/tests/test_api_exoscale.py +++ b/src/tests/test_api_exoscale.py @@ -55,14 +55,14 @@ def valid_osb_payload(): def test_successful_onboarding_new_organization( mock_odoo_success, osb_client, - test_service, - test_service_offering, + service, + service_offering, valid_osb_payload, exoscale_origin, instance_id, ): - valid_osb_payload["service_id"] = test_service.osb_service_id - valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id + valid_osb_payload["service_id"] = service.osb_service_id + valid_osb_payload["plan_id"] = service_offering.osb_plan_id response = osb_client.put( f"/api/osb/v2/service_instances/{instance_id}", @@ -107,15 +107,15 @@ def test_successful_onboarding_new_organization( @pytest.mark.django_db def test_new_organization_inherits_origin( osb_client, - test_service, - test_service_offering, + service, + service_offering, valid_osb_payload, exoscale_origin, instance_id, billing_entity, ): - valid_osb_payload["service_id"] = test_service.osb_service_id - valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id + valid_osb_payload["service_id"] = service.osb_service_id + valid_osb_payload["plan_id"] = service_offering.osb_plan_id exoscale_origin.billing_entity = billing_entity exoscale_origin.save() @@ -137,8 +137,8 @@ def test_new_organization_inherits_origin( @pytest.mark.django_db def test_duplicate_organization_returns_existing( osb_client, - test_service, - test_service_offering, + service, + service_offering, valid_osb_payload, exoscale_origin, instance_id, @@ -148,10 +148,10 @@ def test_duplicate_organization_returns_existing( osb_guid="test-org-guid-123", origin=exoscale_origin, ) - org.limit_osb_services.add(test_service) + org.limit_osb_services.add(service) - valid_osb_payload["service_id"] = test_service.osb_service_id - valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id + valid_osb_payload["service_id"] = service.osb_service_id + valid_osb_payload["plan_id"] = service_offering.osb_plan_id response = osb_client.put( f"/api/osb/v2/service_instances/{instance_id}", @@ -169,13 +169,13 @@ def test_duplicate_organization_returns_existing( @pytest.mark.django_db def test_unauthenticated_osb_api_request_fails( client, - test_service, - test_service_offering, + service, + service_offering, valid_osb_payload, instance_id, ): - valid_osb_payload["service_id"] = test_service.osb_service_id - valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id + valid_osb_payload["service_id"] = service.osb_service_id + valid_osb_payload["plan_id"] = service_offering.osb_plan_id response = client.put( f"/api/osb/v2/service_instances/{instance_id}", @@ -205,15 +205,15 @@ def test_unauthenticated_osb_api_request_fails( ) def test_missing_required_fields_error( osb_client, - test_service, - test_service_offering, + service, + service_offering, valid_osb_payload, field_to_remove, expected_error, instance_id, ): - valid_osb_payload["service_id"] = test_service.osb_service_id - valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id + valid_osb_payload["service_id"] = service.osb_service_id + valid_osb_payload["plan_id"] = service_offering.osb_plan_id if isinstance(field_to_remove, tuple): if field_to_remove[0] == "context": @@ -251,10 +251,8 @@ def test_invalid_service_id_error(osb_client, valid_osb_payload, instance_id): @pytest.mark.django_db -def test_invalid_plan_id_error( - osb_client, test_service, valid_osb_payload, instance_id -): - valid_osb_payload["service_id"] = test_service.osb_service_id +def test_invalid_plan_id_error(osb_client, service, valid_osb_payload, instance_id): + valid_osb_payload["service_id"] = service.osb_service_id valid_osb_payload["plan_id"] = 99999 response = osb_client.put( @@ -266,17 +264,17 @@ def test_invalid_plan_id_error( assert response.status_code == 400 response_data = json.loads(response.content) assert ( - f"Unknown plan_id: 99999 for service_id: {test_service.osb_service_id}" + f"Unknown plan_id: 99999 for service_id: {service.osb_service_id}" in response_data["error"] ) @pytest.mark.django_db def test_empty_users_array_error( - osb_client, test_service, test_service_offering, valid_osb_payload, instance_id + osb_client, service, service_offering, valid_osb_payload, instance_id ): - valid_osb_payload["service_id"] = test_service.osb_service_id - valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id + valid_osb_payload["service_id"] = service.osb_service_id + valid_osb_payload["plan_id"] = service_offering.osb_plan_id valid_osb_payload["parameters"]["users"] = [] response = osb_client.put( @@ -292,10 +290,10 @@ def test_empty_users_array_error( @pytest.mark.django_db def test_multiple_users_error( - osb_client, test_service, test_service_offering, valid_osb_payload, instance_id + osb_client, service, service_offering, valid_osb_payload, instance_id ): - valid_osb_payload["service_id"] = test_service.osb_service_id - valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id + valid_osb_payload["service_id"] = service.osb_service_id + valid_osb_payload["plan_id"] = service_offering.osb_plan_id valid_osb_payload["parameters"]["users"] = [ {"email": "user1@example.com", "full_name": "User One"}, {"email": "user2@example.com", "full_name": "User Two"}, @@ -314,10 +312,10 @@ def test_multiple_users_error( @pytest.mark.django_db def test_empty_email_address_error( - osb_client, test_service, test_service_offering, valid_osb_payload, instance_id + osb_client, service, service_offering, valid_osb_payload, instance_id ): - valid_osb_payload["service_id"] = test_service.osb_service_id - valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id + valid_osb_payload["service_id"] = service.osb_service_id + valid_osb_payload["plan_id"] = service_offering.osb_plan_id valid_osb_payload["parameters"]["users"] = [ {"email": "", "full_name": "User With No Email"}, ] @@ -350,14 +348,14 @@ def test_invalid_json_error(osb_client, instance_id): def test_user_creation_with_name_parsing( mock_odoo_success, osb_client, - test_service, - test_service_offering, + service, + service_offering, valid_osb_payload, exoscale_origin, instance_id, ): - valid_osb_payload["service_id"] = test_service.osb_service_id - valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id + valid_osb_payload["service_id"] = service.osb_service_id + valid_osb_payload["plan_id"] = service_offering.osb_plan_id valid_osb_payload["parameters"]["users"][0]["full_name"] = "John Doe Smith" response = osb_client.put( @@ -376,14 +374,14 @@ def test_user_creation_with_name_parsing( def test_email_normalization( mock_odoo_success, osb_client, - test_service, - test_service_offering, + service, + service_offering, valid_osb_payload, exoscale_origin, instance_id, ): - valid_osb_payload["service_id"] = test_service.osb_service_id - valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id + valid_osb_payload["service_id"] = service.osb_service_id + valid_osb_payload["plan_id"] = service_offering.osb_plan_id valid_osb_payload["parameters"]["users"][0]["email"] = " TEST@EXAMPLE.COM " response = osb_client.put( @@ -401,14 +399,14 @@ def test_email_normalization( def test_odoo_integration_failure_handling( mock_odoo_failure, osb_client, - test_service, - test_service_offering, + service, + service_offering, valid_osb_payload, exoscale_origin, instance_id, ): - valid_osb_payload["service_id"] = test_service.osb_service_id - valid_osb_payload["plan_id"] = test_service_offering.osb_plan_id + valid_osb_payload["service_id"] = service.osb_service_id + valid_osb_payload["plan_id"] = service_offering.osb_plan_id response = osb_client.put( f"/api/osb/v2/service_instances/{instance_id}", @@ -425,14 +423,14 @@ def test_odoo_integration_failure_handling( def test_organization_creation_with_context_only( mock_odoo_success, osb_client, - test_service, - test_service_offering, + service, + service_offering, exoscale_origin, instance_id, ): payload = { - "service_id": test_service.osb_service_id, - "plan_id": test_service_offering.osb_plan_id, + "service_id": service.osb_service_id, + "plan_id": service_offering.osb_plan_id, "context": { "organization_guid": "fallback-org-guid", "organization_name": "Fallback Organization", @@ -462,13 +460,13 @@ def test_organization_creation_with_context_only( def test_delete_offboarding_success( mock_odoo_success, osb_client, - test_service, - test_service_offering, + service, + service_offering, instance_id, ): response = osb_client.delete( f"/api/osb/v2/service_instances/{instance_id}" - f"?service_id={test_service.osb_service_id}&plan_id={test_service_offering.osb_plan_id}" + f"?service_id={service.osb_service_id}&plan_id={service_offering.osb_plan_id}" ) assert response.status_code == 200 @@ -476,9 +474,9 @@ def test_delete_offboarding_success( @pytest.mark.django_db -def test_delete_missing_service_id(osb_client, test_service_offering, instance_id): +def test_delete_missing_service_id(osb_client, service_offering, instance_id): response = osb_client.delete( - f"/api/osb/v2/service_instances/{instance_id}?plan_id={test_service_offering.osb_plan_id}" + f"/api/osb/v2/service_instances/{instance_id}?plan_id={service_offering.osb_plan_id}" ) assert response.status_code == 400 @@ -487,9 +485,9 @@ def test_delete_missing_service_id(osb_client, test_service_offering, instance_i @pytest.mark.django_db -def test_delete_missing_plan_id(osb_client, test_service, instance_id): +def test_delete_missing_plan_id(osb_client, service, instance_id): response = osb_client.delete( - f"/api/osb/v2/service_instances/{instance_id}?service_id={test_service.osb_service_id}" + f"/api/osb/v2/service_instances/{instance_id}?service_id={service.osb_service_id}" ) assert response.status_code == 400 @@ -509,16 +507,16 @@ def test_delete_invalid_service_id(osb_client, instance_id): @pytest.mark.django_db -def test_delete_invalid_plan_id(osb_client, test_service, instance_id): +def test_delete_invalid_plan_id(osb_client, service, instance_id): response = osb_client.delete( f"/api/osb/v2/service_instances/{instance_id}" - f"?service_id={test_service.osb_service_id}&plan_id=invalid" + f"?service_id={service.osb_service_id}&plan_id=invalid" ) assert response.status_code == 400 response_data = json.loads(response.content) assert ( - f"Unknown plan_id: invalid for service_id: {test_service.osb_service_id}" + f"Unknown plan_id: invalid for service_id: {service.osb_service_id}" in response_data["error"] ) @@ -527,13 +525,13 @@ def test_delete_invalid_plan_id(osb_client, test_service, instance_id): def test_patch_suspension_success( mock_odoo_success, osb_client, - test_service, - test_service_offering, + service, + service_offering, instance_id, ): payload = { - "service_id": test_service.osb_service_id, - "plan_id": test_service_offering.osb_plan_id, + "service_id": service.osb_service_id, + "plan_id": service_offering.osb_plan_id, "parameters": { "users": [ { @@ -556,9 +554,9 @@ def test_patch_suspension_success( @pytest.mark.django_db -def test_patch_missing_service_id(osb_client, test_service_offering, instance_id): +def test_patch_missing_service_id(osb_client, service_offering, instance_id): payload = { - "plan_id": test_service_offering.osb_plan_id, + "plan_id": service_offering.osb_plan_id, "parameters": {"users": []}, } @@ -574,9 +572,9 @@ def test_patch_missing_service_id(osb_client, test_service_offering, instance_id @pytest.mark.django_db -def test_patch_missing_plan_id(osb_client, test_service, instance_id): +def test_patch_missing_plan_id(osb_client, service, instance_id): payload = { - "service_id": test_service.osb_service_id, + "service_id": service.osb_service_id, "parameters": {"users": []}, } @@ -609,8 +607,8 @@ def test_delete_creates_ticket_with_admin_links( mocker, mock_odoo_success, osb_client, - test_service, - test_service_offering, + service, + service_offering, instance_id, ): # Mock the create_helpdesk_ticket function @@ -618,7 +616,7 @@ def test_delete_creates_ticket_with_admin_links( response = osb_client.delete( f"/api/osb/v2/service_instances/{instance_id}" - f"?service_id={test_service.osb_service_id}&plan_id={test_service_offering.osb_plan_id}" + f"?service_id={service.osb_service_id}&plan_id={service_offering.osb_plan_id}" ) assert response.status_code == 200 @@ -629,10 +627,10 @@ def test_delete_creates_ticket_with_admin_links( # Check that the description contains an admin URL assert "admin/core/serviceoffering" in call_kwargs["description"] - assert f"/{test_service_offering.pk}/" in call_kwargs["description"] + assert f"/{service_offering.pk}/" in call_kwargs["description"] assert ( call_kwargs["title"] - == f"Exoscale OSB Offboard - {test_service.name} - {instance_id}" + == f"Exoscale OSB Offboard - {service.name} - {instance_id}" ) @@ -641,8 +639,8 @@ def test_patch_creates_ticket_with_user_admin_links( mocker, mock_odoo_success, osb_client, - test_service, - test_service_offering, + service, + service_offering, instance_id, org_owner, ): @@ -650,8 +648,8 @@ def test_patch_creates_ticket_with_user_admin_links( mock_create_ticket = mocker.patch("servala.api.views.create_helpdesk_ticket") payload = { - "service_id": test_service.osb_service_id, - "plan_id": test_service_offering.osb_plan_id, + "service_id": service.osb_service_id, + "plan_id": service_offering.osb_plan_id, "parameters": { "users": [ { @@ -680,8 +678,7 @@ def test_patch_creates_ticket_with_user_admin_links( assert "admin/core/user" in call_kwargs["description"] assert f"/{org_owner.pk}/" in call_kwargs["description"] assert ( - call_kwargs["title"] - == f"Exoscale OSB Suspend - {test_service.name} - {instance_id}" + call_kwargs["title"] == f"Exoscale OSB Suspend - {service.name} - {instance_id}" ) @@ -690,8 +687,8 @@ def test_ticket_includes_organization_and_instance_when_found( mocker, mock_odoo_success, osb_client, - test_service, - test_service_offering, + service, + service_offering, organization, ): # Mock the create_helpdesk_ticket function @@ -699,12 +696,12 @@ def test_ticket_includes_organization_and_instance_when_found( service_definition = ServiceDefinition.objects.create( name="Test Definition", - service=test_service, + service=service, api_definition={"group": "test.example.com", "version": "v1", "kind": "Test"}, ) control_plane = ControlPlane.objects.create( name="Test Control Plane", - cloud_provider=test_service_offering.provider, + cloud_provider=service_offering.provider, api_credentials={ "certificate-authority-data": "test", "server": "https://test", @@ -712,20 +709,22 @@ def test_ticket_includes_organization_and_instance_when_found( }, ) crd = ControlPlaneCRD.objects.create( - service_offering=test_service_offering, + service_offering=service_offering, control_plane=control_plane, service_definition=service_definition, ) instance_name = "test-instance-123" + instance_display_name = "Test Instance 123" service_instance = ServiceInstance.objects.create( name=instance_name, + display_name=instance_display_name, organization=organization, context=crd, ) response = osb_client.delete( f"/api/osb/v2/service_instances/{instance_name}" - f"?service_id={test_service.osb_service_id}&plan_id={test_service_offering.osb_plan_id}" + f"?service_id={service.osb_service_id}&plan_id={service_offering.osb_plan_id}" ) assert response.status_code == 200 @@ -739,7 +738,10 @@ def test_ticket_includes_organization_and_instance_when_found( assert "admin/core/organization" in call_kwargs["description"] assert f"/{organization.pk}/" in call_kwargs["description"] - # Check instance is included - assert f"Instance: {service_instance.name}" in call_kwargs["description"] + # Check instance is included (format: "display_name (resource_name)") + assert ( + f"Instance: {service_instance.display_name} ({service_instance.name})" + in call_kwargs["description"] + ) assert "admin/core/serviceinstance" in call_kwargs["description"] assert f"/{service_instance.pk}/" in call_kwargs["description"] diff --git a/src/tests/test_form_config.py b/src/tests/test_form_config.py index 014f1f2..2a010ed 100644 --- a/src/tests/test_form_config.py +++ b/src/tests/test_form_config.py @@ -22,7 +22,7 @@ def test_custom_model_form_class_returns_class_when_form_config_exists(): { "type": "text", "label": "Name", - "controlplane_field_mapping": "name", + "controlplane_field_mapping": "display_name", "required": True, } ], @@ -32,7 +32,7 @@ def test_custom_model_form_class_returns_class_when_form_config_exists(): crd.service_definition = service_def class TestModel(models.Model): - name = models.CharField(max_length=100) + display_name = models.CharField(max_length=100) class Meta: app_label = "test" @@ -193,7 +193,7 @@ def test_choice_field_uses_custom_choices_from_form_config(): """Test that choice fields use custom choices when provided in form_config""" class TestModel(models.Model): - name = models.CharField(max_length=100) + display_name = models.CharField(max_length=100) environment = models.CharField( max_length=20, choices=[ @@ -215,7 +215,7 @@ def test_choice_field_uses_custom_choices_from_form_config(): { "type": "text", "label": "Name", - "controlplane_field_mapping": "name", + "controlplane_field_mapping": "display_name", "required": True, }, { @@ -246,7 +246,7 @@ def test_choice_field_uses_custom_choices_from_form_config(): def test_choice_field_uses_control_plane_choices_when_no_custom_choices(): class TestModel(models.Model): - name = models.CharField(max_length=100) + display_name = models.CharField(max_length=100) environment = models.CharField( max_length=20, choices=[ @@ -267,7 +267,7 @@ def test_choice_field_uses_control_plane_choices_when_no_custom_choices(): { "type": "text", "label": "Name", - "controlplane_field_mapping": "name", + "controlplane_field_mapping": "display_name", "required": True, }, { @@ -292,7 +292,7 @@ def test_choice_field_uses_control_plane_choices_when_no_custom_choices(): def test_choice_field_validates_against_control_plane_choices(): class TestModel(models.Model): - name = models.CharField(max_length=100) + display_name = models.CharField(max_length=100) environment = models.CharField( max_length=20, choices=[ @@ -313,7 +313,7 @@ def test_choice_field_validates_against_control_plane_choices(): { "type": "text", "label": "Name", - "controlplane_field_mapping": "name", + "controlplane_field_mapping": "display_name", "required": True, }, { @@ -330,15 +330,15 @@ def test_choice_field_validates_against_control_plane_choices(): form_class = generate_custom_form_class(form_config, TestModel) - form = form_class(data={"name": "test-service", "environment": "dev"}) + form = form_class(data={"display_name": "test-service", "environment": "dev"}) form.fields["context"].required = False # Skip context validation assert form.is_valid(), f"Form should be valid but has errors: {form.errors}" - form = form_class(data={"name": "test-service", "environment": "prod"}) + form = form_class(data={"display_name": "test-service", "environment": "prod"}) form.fields["context"].required = False # Skip context validation assert form.is_valid(), f"Form should be valid but has errors: {form.errors}" - form = form_class(data={"name": "test-service", "environment": "invalid"}) + form = form_class(data={"display_name": "test-service", "environment": "invalid"}) form.fields["context"].required = False # Skip context validation assert not form.is_valid() assert "environment" in form.errors @@ -368,7 +368,7 @@ def test_admin_form_validates_choice_values_against_schema(): { "type": "text", "label": "Name", - "controlplane_field_mapping": "name", + "controlplane_field_mapping": "display_name", }, { "type": "choice", @@ -399,7 +399,7 @@ def test_admin_form_validates_choice_values_against_schema(): { "type": "text", "label": "Name", - "controlplane_field_mapping": "name", + "controlplane_field_mapping": "display_name", }, { "type": "choice", @@ -431,7 +431,7 @@ def test_admin_form_validates_choice_values_against_schema(): def test_number_field_min_max_sets_widget_attributes(): class TestModel(models.Model): - name = models.CharField(max_length=100) + display_name = models.CharField(max_length=100) port = models.IntegerField() replica_count = models.IntegerField() @@ -446,7 +446,7 @@ def test_number_field_min_max_sets_widget_attributes(): { "type": "text", "label": "Name", - "controlplane_field_mapping": "name", + "controlplane_field_mapping": "display_name", "required": True, }, { @@ -494,7 +494,7 @@ def test_number_field_min_max_sets_widget_attributes(): def test_default_value_for_all_field_types(): class TestModel(models.Model): - name = models.CharField(max_length=100) + display_name = models.CharField(max_length=100) description = models.TextField() port = models.IntegerField() environment = models.CharField( @@ -518,7 +518,7 @@ def test_default_value_for_all_field_types(): { "type": "text", "label": "Name", - "controlplane_field_mapping": "name", + "controlplane_field_mapping": "display_name", "default_value": "default-name", }, { @@ -559,7 +559,7 @@ def test_default_value_for_all_field_types(): form_class = generate_custom_form_class(form_config, TestModel) form = form_class() - assert form.fields["name"].initial == "default-name" + assert form.fields["display_name"].initial == "default-name" assert form.fields["description"].initial == "Default description text" assert form.fields["port"].initial == "8080" assert form.fields["environment"].initial == "dev" @@ -570,7 +570,7 @@ def test_default_value_for_all_field_types(): def test_default_value_not_override_existing_instance(): class TestModel(models.Model): - name = models.CharField(max_length=100) + display_name = models.CharField(max_length=100) port = models.IntegerField() class Meta: @@ -583,7 +583,7 @@ def test_default_value_not_override_existing_instance(): { "type": "text", "label": "Name", - "controlplane_field_mapping": "name", + "controlplane_field_mapping": "display_name", "default_value": "default-name", }, { @@ -597,11 +597,11 @@ def test_default_value_not_override_existing_instance(): ] } - instance = TestModel(name="existing-name", port=3000) + instance = TestModel(display_name="existing-name", port=3000) form_class = generate_custom_form_class(form_config, TestModel) form = form_class(instance=instance) - assert form.initial["name"] == "existing-name" + assert form.initial["display_name"] == "existing-name" assert form.initial["port"] == 3000 @@ -708,7 +708,7 @@ def test_form_config_handles_empty_string_as_none(): { "type": "text", "label": "Name", - "controlplane_field_mapping": "name", + "controlplane_field_mapping": "display_name", "max_length": "", # Empty string }, ] @@ -744,7 +744,7 @@ def test_single_element_choices_are_normalized(): { "type": "text", "label": "Name", - "controlplane_field_mapping": "name", + "controlplane_field_mapping": "display_name", }, { "type": "choice", @@ -879,7 +879,7 @@ def test_three_plus_element_choices_fail_validation(): def test_field_with_default_config_only_needs_mapping(): class TestModel(models.Model): - name = models.CharField(max_length=100) + display_name = models.CharField(max_length=100) class Meta: app_label = "test" @@ -889,7 +889,7 @@ def test_field_with_default_config_only_needs_mapping(): { "fields": [ { - "controlplane_field_mapping": "name", + "controlplane_field_mapping": "display_name", }, ] } @@ -899,16 +899,16 @@ def test_field_with_default_config_only_needs_mapping(): form_class = generate_custom_form_class(minimal_config, TestModel) form = form_class() - name_field = form.fields["name"] - assert name_field.label == DEFAULT_FIELD_CONFIGS["name"]["label"] - assert name_field.help_text == DEFAULT_FIELD_CONFIGS["name"]["help_text"] - assert name_field.required == DEFAULT_FIELD_CONFIGS["name"]["required"] + name_field = form.fields["display_name"] + assert name_field.label == DEFAULT_FIELD_CONFIGS["display_name"]["label"] + assert name_field.help_text == DEFAULT_FIELD_CONFIGS["display_name"]["help_text"] + assert name_field.required == DEFAULT_FIELD_CONFIGS["display_name"]["required"] def test_field_with_default_config_can_override_defaults(): class TestModel(models.Model): - name = models.CharField(max_length=100) + display_name = models.CharField(max_length=100) class Meta: app_label = "test" @@ -918,7 +918,7 @@ def test_field_with_default_config_can_override_defaults(): { "fields": [ { - "controlplane_field_mapping": "name", + "controlplane_field_mapping": "display_name", "label": "Custom Name Label", "required": False, }, @@ -930,16 +930,16 @@ def test_field_with_default_config_can_override_defaults(): form_class = generate_custom_form_class(override_config, TestModel) form = form_class() - name_field = form.fields["name"] + name_field = form.fields["display_name"] assert name_field.label == "Custom Name Label" assert name_field.required is False - assert name_field.help_text == DEFAULT_FIELD_CONFIGS["name"]["help_text"] + assert name_field.help_text == DEFAULT_FIELD_CONFIGS["display_name"]["help_text"] def test_empty_values_dont_override_default_configs(): class TestModel(models.Model): - name = models.CharField(max_length=100) + display_name = models.CharField(max_length=100) class Meta: app_label = "test" @@ -949,7 +949,7 @@ def test_empty_values_dont_override_default_configs(): { "fields": [ { - "controlplane_field_mapping": "name", + "controlplane_field_mapping": "display_name", "type": "", "label": "", "help_text": None, @@ -964,11 +964,11 @@ def test_empty_values_dont_override_default_configs(): form_class = generate_custom_form_class(admin_form_config, TestModel) form = form_class() - name_field = form.fields["name"] + name_field = form.fields["display_name"] - assert name_field.label == DEFAULT_FIELD_CONFIGS["name"]["label"] - assert name_field.help_text == DEFAULT_FIELD_CONFIGS["name"]["help_text"] - assert name_field.max_length == DEFAULT_FIELD_CONFIGS["name"]["max_length"] + assert name_field.label == DEFAULT_FIELD_CONFIGS["display_name"]["label"] + assert name_field.help_text == DEFAULT_FIELD_CONFIGS["display_name"]["help_text"] + assert name_field.max_length == DEFAULT_FIELD_CONFIGS["display_name"]["max_length"] assert name_field.required is False # Was overridden by explicit False @@ -976,7 +976,7 @@ def test_empty_values_dont_override_default_configs(): def test_number_field_validates_min_max_values(): class TestModel(models.Model): - name = models.CharField(max_length=100) + display_name = models.CharField(max_length=100) port = models.IntegerField() class Meta: @@ -990,7 +990,7 @@ def test_number_field_validates_min_max_values(): { "type": "text", "label": "Name", - "controlplane_field_mapping": "name", + "controlplane_field_mapping": "display_name", "required": True, }, { @@ -1009,26 +1009,26 @@ def test_number_field_validates_min_max_values(): form_class = generate_custom_form_class(form_config, TestModel) # Test value below minimum fails validation - form = form_class(data={"name": "test-service", "port": 0}) + form = form_class(data={"display_name": "test-service", "port": 0}) form.fields["context"].required = False assert not form.is_valid() assert "port" in form.errors # Test value above maximum fails validation - form = form_class(data={"name": "test-service", "port": 65536}) + form = form_class(data={"display_name": "test-service", "port": 65536}) form.fields["context"].required = False assert not form.is_valid() assert "port" in form.errors # Test valid value passes validation - form = form_class(data={"name": "test-service", "port": 8080}) + form = form_class(data={"display_name": "test-service", "port": 8080}) form.fields["context"].required = False assert form.is_valid(), f"Form should be valid but has errors: {form.errors}" def test_number_field_with_addon_text_roundtrip(): class TestModel(models.Model): - name = models.CharField(max_length=100) + display_name = models.CharField(max_length=100) disk_size = models.IntegerField() class Meta: @@ -1041,7 +1041,7 @@ def test_number_field_with_addon_text_roundtrip(): { "type": "text", "label": "Name", - "controlplane_field_mapping": "name", + "controlplane_field_mapping": "display_name", "required": True, }, { @@ -1059,7 +1059,7 @@ def test_number_field_with_addon_text_roundtrip(): form = form_class(initial={"name": "test-instance", "disk_size": "25Gi"}) assert form.initial["disk_size"] == 25 - form = form_class(data={"name": "test-instance", "disk_size": "25"}) + form = form_class(data={"display_name": "test-instance", "disk_size": "25"}) form.fields["context"].required = False assert form.is_valid(), f"Form should be valid but has errors: {form.errors}" nested_data = form.get_nested_data() diff --git a/src/tests/test_management_commands.py b/src/tests/test_management_commands.py index 714d29a..f0f0277 100644 --- a/src/tests/test_management_commands.py +++ b/src/tests/test_management_commands.py @@ -109,7 +109,7 @@ class TestReencryptFieldsCommand: assert "Starting re-encryption" in output assert "Re-encrypted 0 ControlPlane objects" in output - def test_reencrypt_fields_with_control_plane(self, test_control_plane): + def test_reencrypt_fields_with_control_plane(self, control_plane): out = StringIO() call_command("reencrypt_fields", stdout=out) @@ -147,11 +147,11 @@ class TestSyncBillingMetadataCommand: assert "No control planes found with the specified IDs" in out.getvalue() - def test_sync_billing_metadata_dry_run_with_control_plane(self, test_control_plane): + def test_sync_billing_metadata_dry_run_with_control_plane(self, control_plane): out = StringIO() call_command("sync_billing_metadata", "--dry-run", stdout=out) output = out.getvalue() assert "DRY RUN" in output assert "Syncing billing metadata on 1 control plane(s)" in output - assert test_control_plane.name in output + assert control_plane.name in output diff --git a/src/tests/test_rules.py b/src/tests/test_rules.py new file mode 100644 index 0000000..9a83163 --- /dev/null +++ b/src/tests/test_rules.py @@ -0,0 +1,151 @@ +import pytest +from django_scopes import scope + +from servala.core.models.organization import OrganizationRole +from servala.core.rules import ( + has_organization_role, + is_organization_admin, + is_organization_member, + is_organization_owner, +) + +pytestmark = pytest.mark.django_db + + +def test_has_organization_role_returns_false_for_non_organization_object(user): + assert has_organization_role(user, "not an organization", None) is False + + +def test_has_organization_role_returns_false_for_non_member(user, organization): + with scope(organization=organization): + assert has_organization_role(user, organization, None) is False + + +@pytest.mark.parametrize("user_fixture", ["org_owner", "org_admin", "org_member"]) +def test_has_organization_role_returns_true_for_any_member( + user_fixture, organization, request +): + user = request.getfixturevalue(user_fixture) + with scope(organization=organization): + assert has_organization_role(user, organization, None) is True + + +@pytest.mark.parametrize( + "user_fixture,roles,expected", + [ + ("org_owner", [OrganizationRole.OWNER], True), + ("org_owner", [OrganizationRole.ADMIN], False), + ("org_owner", [OrganizationRole.MEMBER], False), + ("org_owner", [OrganizationRole.OWNER, OrganizationRole.ADMIN], True), + ("org_admin", [OrganizationRole.ADMIN], True), + ("org_admin", [OrganizationRole.OWNER], False), + ("org_admin", [OrganizationRole.OWNER, OrganizationRole.ADMIN], True), + ("org_member", [OrganizationRole.MEMBER], True), + ("org_member", [OrganizationRole.OWNER], False), + ("org_member", [OrganizationRole.ADMIN], False), + ], +) +def test_has_organization_role_filters_by_roles( + user_fixture, roles, expected, organization, request +): + user = request.getfixturevalue(user_fixture) + with scope(organization=organization): + assert has_organization_role(user, organization, roles) is expected + + +@pytest.mark.parametrize( + "user_fixture,expected", + [ + ("org_owner", True), + ("org_admin", False), + ("org_member", False), + ], +) +def test_is_organization_owner(user_fixture, expected, organization, request): + user = request.getfixturevalue(user_fixture) + with scope(organization=organization): + assert is_organization_owner(user, organization) is expected + + +@pytest.mark.parametrize( + "user_fixture,expected", + [ + ("org_owner", True), + ("org_admin", False), + ("org_member", False), + ], +) +def test_is_organization_owner_with_related_object( + user_fixture, expected, organization, mocker, request +): + user = request.getfixturevalue(user_fixture) + obj = mocker.MagicMock() + obj.organization = organization + with scope(organization=organization): + assert is_organization_owner(user, obj) is expected + + +@pytest.mark.parametrize( + "user_fixture,expected", + [ + ("org_owner", True), + ("org_admin", True), + ("org_member", False), + ], +) +def test_is_organization_admin(user_fixture, expected, organization, request): + user = request.getfixturevalue(user_fixture) + with scope(organization=organization): + assert is_organization_admin(user, organization) is expected + + +@pytest.mark.parametrize( + "user_fixture,expected", + [ + ("org_owner", True), + ("org_admin", True), + ("org_member", False), + ], +) +def test_is_organization_admin_with_related_object( + user_fixture, expected, organization, mocker, request +): + user = request.getfixturevalue(user_fixture) + obj = mocker.MagicMock() + obj.organization = organization + with scope(organization=organization): + assert is_organization_admin(user, obj) is expected + + +@pytest.mark.parametrize("user_fixture", ["org_owner", "org_admin", "org_member"]) +def test_is_organization_member_returns_true_for_members( + user_fixture, organization, request +): + user = request.getfixturevalue(user_fixture) + with scope(organization=organization): + assert is_organization_member(user, organization) is True + + +def test_is_organization_member_returns_false_for_non_member(user, organization): + with scope(organization=organization): + assert is_organization_member(user, organization) is False + + +@pytest.mark.parametrize("user_fixture", ["org_owner", "org_admin", "org_member"]) +def test_is_organization_member_with_related_object( + user_fixture, organization, mocker, request +): + user = request.getfixturevalue(user_fixture) + obj = mocker.MagicMock() + obj.organization = organization + with scope(organization=organization): + assert is_organization_member(user, obj) is True + + +def test_is_organization_member_with_related_object_returns_false_for_non_member( + user, organization, mocker +): + obj = mocker.MagicMock() + obj.organization = organization + with scope(organization=organization): + assert is_organization_member(user, obj) is False diff --git a/src/tests/test_service_models.py b/src/tests/test_service_models.py new file mode 100644 index 0000000..794fad2 --- /dev/null +++ b/src/tests/test_service_models.py @@ -0,0 +1,349 @@ +"""Tests for servala.core.models.service module.""" + +from unittest.mock import MagicMock + +import pytest +from django.core.exceptions import ValidationError + +from servala.core.models.service import ( + ControlPlane, + Service, + ServiceInstance, + prune_empty_data, + validate_api_credentials, + validate_dict, +) + +pytestmark = pytest.mark.django_db + + +@pytest.fixture +def service_with_external_links(service_category): + return Service.objects.create( + name="PostgreSQL", + slug="postgresql", + category=service_category, + description="PostgreSQL database", + external_links=[ + {"url": "https://docs.example.com", "title": "Docs", "featured": True}, + {"url": "https://github.com/example", "title": "GitHub", "featured": False}, + {"url": "https://api.example.com", "title": "API", "featured": True}, + ], + ) + + +@pytest.mark.parametrize("data", [None, {}]) +def test_validate_dict_allows_empty_by_default(data): + validate_dict(data) + + +@pytest.mark.parametrize("data", [None, {}]) +def test_validate_dict_raises_when_empty_not_allowed(data): + with pytest.raises(ValidationError, match="Data may not be empty"): + validate_dict(data, allow_empty=False) + + +def test_validate_dict_missing_required_fields_raises(): + with pytest.raises(ValidationError, match="Missing required fields"): + validate_dict({"field1": "v"}, required_fields={"field1", "field2", "field3"}) + + +def test_validate_dict_all_required_fields_present_passes(): + validate_dict({"a": 1, "b": 2, "extra": 3}, required_fields={"a", "b"}) + + +@pytest.mark.parametrize("data", [None, {}]) +def test_validate_api_credentials_allows_empty(data): + validate_api_credentials(data) + + +@pytest.mark.parametrize( + "input_data,expected", + [ + ({"a": 1, "b": None, "c": 3}, {"a": 1, "c": 3}), + ({"a": "hello", "b": "", "c": "world"}, {"a": "hello", "c": "world"}), + ({"a": [1, 2], "b": [], "c": [3]}, {"a": [1, 2], "c": [3]}), + ( + {"a": {"nested": 1}, "b": {}, "c": {"x": 2}}, + {"a": {"nested": 1}, "c": {"x": 2}}, + ), + ({"outer": {"inner": {"empty": None}}}, {}), + ( + {"false_val": False, "zero": 0, "none": None}, + {"false_val": False, "zero": 0}, + ), + ("string", "string"), + (42, 42), + ], +) +def test_prune_empty_data(input_data, expected): + assert prune_empty_data(input_data) == expected + + +def test_prune_empty_data_nested_dicts(): + data = {"level1": {"level2": {"keep": "value", "remove": None, "empty": ""}}} + assert prune_empty_data(data) == {"level1": {"level2": {"keep": "value"}}} + + +def test_prune_empty_data_in_lists(): + data = {"items": [{"keep": 1}, {"remove": None}, {"also_keep": 2}]} + assert prune_empty_data(data) == {"items": [{"keep": 1}, {"also_keep": 2}]} + + +def test_service_featured_links_filters_correctly(service_with_external_links): + featured = service_with_external_links.featured_links + assert len(featured) == 2 + assert all(link["featured"] for link in featured) + assert {link["title"] for link in featured} == {"Docs", "API"} + + +@pytest.mark.parametrize( + "external_links,expected_count", + [ + (None, 0), + ([], 0), + ([{"url": "https://x.com", "title": "X", "featured": False}], 0), + ], +) +def test_service_featured_links_empty_cases( + service_category, external_links, expected_count +): + svc = Service.objects.create( + name="Test", + slug="test-svc", + category=service_category, + external_links=external_links, + ) + assert len(svc.featured_links) == expected_count + + +def test_service_str(service): + assert str(service) == "Redis" + + +def test_service_category_str(service_category): + assert str(service_category) == "Databases" + + +def test_cloud_provider_str(cloud_provider): + assert str(cloud_provider) == "Exoscale" + + +def test_control_plane_str(control_plane): + assert str(control_plane) == "Geneva (CH-GVA-2)" + + +def test_control_plane_test_connection_no_credentials(cloud_provider): + plane = ControlPlane.objects.create( + name="No Creds", cloud_provider=cloud_provider, api_credentials={} + ) + success, message = plane.test_connection() + assert success is False + assert "No API credentials" in str(message) + + +def test_service_definition_str(service_definition): + assert str(service_definition) == "Redis Standard" + + +def test_service_definition_control_planes(service_definition, control_plane_crd): + assert control_plane_crd.control_plane in service_definition.control_planes + + +def test_control_plane_crd_str(control_plane_crd): + result = str(control_plane_crd) + assert "Redis" in result and "Exoscale" in result and "Geneva" in result + + +@pytest.mark.parametrize( + "prop,expected", + [("group", "vshn.appcat.vshn.io"), ("version", "v1"), ("kind", "VSHNRedis")], +) +def test_control_plane_crd_api_properties(control_plane_crd, prop, expected): + assert getattr(control_plane_crd, prop) == expected + + +def test_service_offering_str(service_offering): + result = str(service_offering) + assert "Redis" in result and "Exoscale" in result + + +def test_service_offering_control_planes(service_offering, control_plane_crd): + assert control_plane_crd.control_plane in service_offering.control_planes + + +def test_generate_resource_name_consistent(organization, service): + name1 = ServiceInstance.generate_resource_name(organization, "My Instance", service) + name2 = ServiceInstance.generate_resource_name(organization, "My Instance", service) + assert name1 == name2 + + +def test_generate_resource_name_different_inputs(organization, service): + name_a = ServiceInstance.generate_resource_name(organization, "Instance A", service) + name_b = ServiceInstance.generate_resource_name(organization, "Instance B", service) + assert name_a != name_b + + +def test_generate_resource_name_attempt_changes_hash(organization, service): + name0 = ServiceInstance.generate_resource_name( + organization, "X", service, attempt=0 + ) + name1 = ServiceInstance.generate_resource_name( + organization, "X", service, attempt=1 + ) + assert name0 != name1 + + +def test_generate_resource_name_format(organization, service, settings): + name = ServiceInstance.generate_resource_name(organization, "Test", service) + assert name.startswith(f"{settings.SERVALA_INSTANCE_NAME_PREFIX}-") + assert len(name.split("-")[-1]) == 8 + + +@pytest.mark.parametrize( + "display_name", ["My Instance", "MY INSTANCE", " My Instance "] +) +def test_generate_resource_name_normalizes_display_name( + organization, service, display_name +): + canonical = ServiceInstance.generate_resource_name( + organization, "my instance", service + ) + assert ( + ServiceInstance.generate_resource_name(organization, display_name, service) + == canonical + ) + + +@pytest.mark.parametrize("spec_data", [None, {}]) +def test_prepare_spec_data_handles_empty(spec_data): + assert ServiceInstance._prepare_spec_data(spec_data) == {} + + +def test_prepare_spec_data_prunes_empty_values(): + assert ServiceInstance._prepare_spec_data({"keep": "v", "rm": None}) == { + "keep": "v" + } + + +def test_apply_compute_plan_to_spec(compute_plan_assignment): + result = ServiceInstance._apply_compute_plan_to_spec({}, compute_plan_assignment) + assert result["parameters"]["size"]["memory"] == "2Gi" + assert result["parameters"]["size"]["cpu"] == "1000m" + assert result["parameters"]["size"]["requests"]["memory"] == "1Gi" + assert result["parameters"]["size"]["requests"]["cpu"] == "500m" + assert result["parameters"]["service"]["serviceLevel"] == "besteffort" + + +def test_apply_compute_plan_to_spec_none_assignment(): + spec = {"existing": "value"} + assert ServiceInstance._apply_compute_plan_to_spec(spec, None) == { + "existing": "value" + } + + +def test_apply_compute_plan_preserves_existing(compute_plan_assignment): + spec = {"parameters": {"custom": "setting", "service": {"other": "config"}}} + result = ServiceInstance._apply_compute_plan_to_spec(spec, compute_plan_assignment) + assert result["parameters"]["custom"] == "setting" + assert result["parameters"]["service"]["other"] == "config" + + +def test_build_billing_annotations_display_name(): + cp = MagicMock(storage_plan_odoo_product_id=None, storage_plan_odoo_unit_id=None) + annotations = ServiceInstance._build_billing_annotations( + compute_plan_assignment=None, control_plane=cp, display_name="My Service" + ) + assert annotations["servala.com/displayName"] == "My Service" + + +def test_build_billing_annotations_no_display_name(): + cp = MagicMock(storage_plan_odoo_product_id=None, storage_plan_odoo_unit_id=None) + annotations = ServiceInstance._build_billing_annotations( + compute_plan_assignment=None, control_plane=cp, display_name=None + ) + assert "servala.com/displayName" not in annotations + + +def test_build_billing_annotations_compute_plan(compute_plan_assignment): + cp = MagicMock(storage_plan_odoo_product_id=None, storage_plan_odoo_unit_id=None) + annotations = ServiceInstance._build_billing_annotations( + compute_plan_assignment=compute_plan_assignment, control_plane=cp + ) + assert annotations["servala.com/erp_product_id_resource"] == "test-product-id" + assert annotations["servala.com/erp_unit_id_resource"] == "test-unit-id" + + +def test_build_billing_annotations_storage_plan(control_plane_with_storage): + annotations = ServiceInstance._build_billing_annotations( + compute_plan_assignment=None, control_plane=control_plane_with_storage + ) + assert annotations["servala.com/erp_product_id_storage"] == "storage-product-123" + assert annotations["servala.com/erp_unit_id_storage"] == "storage-unit-456" + + +@pytest.mark.parametrize( + "error_msg,expected_has_list,expected_errors", + [ + ("", False, None), + (None, False, None), + ("Something went wrong", False, None), + ("Error: [single error]", False, None), + ("Validation failed: [e1, e2, e3]", True, ["e1", "e2", "e3"]), + ], +) +def test_format_kubernetes_error(error_msg, expected_has_list, expected_errors): + result = ServiceInstance._format_kubernetes_error(error_msg) + assert result["has_list"] == expected_has_list + assert result["errors"] == expected_errors + + +def test_format_kubernetes_error_strips_quotes(): + result = ServiceInstance._format_kubernetes_error("Errors: [\"quoted\", 'single']") + assert "quoted" in result["errors"] + assert "single" in result["errors"] + + +def test_safe_format_error_non_dict(): + assert ServiceInstance._safe_format_error("plain string") == "plain string" + + +def test_safe_format_error_escapes_html(): + error_data = {"message": "", "has_list": False} + result = ServiceInstance._safe_format_error(error_data) + assert "