From 228ab9bc0d8fa1642d038c781d1f74008b13ced4 Mon Sep 17 00:00:00 2001 From: Tobias Kunze Date: Mon, 10 Nov 2025 13:36:09 +0100 Subject: [PATCH 1/4] Make fqdn generator fail silently --- src/servala/static/js/fqdn.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/servala/static/js/fqdn.js b/src/servala/static/js/fqdn.js index 178c586..ec70dad 100644 --- a/src/servala/static/js/fqdn.js +++ b/src/servala/static/js/fqdn.js @@ -1,7 +1,9 @@ const initializeFqdnGeneration = (prefix) => { const nameField = document.querySelector(`input#id_${prefix}-name`); - const fqdnField = document.getElementById(`${prefix}-spec.parameters.service.fqdn_container`).querySelector('input.array-item-input'); + const fqdnFieldContainer = document.getElementById(`${prefix}-spec.parameters.service.fqdn_container`) + if (!nameField || !fqdnFieldContainer) return + const fqdnField = fqdnFieldContainer.querySelector('input.array-item-input'); if (nameField && fqdnField) { const generateFqdn = (instanceName) => { From c7c22aa265b7e234e0799816d98c78f79e86911b Mon Sep 17 00:00:00 2001 From: Tobias Kunze Date: Mon, 10 Nov 2025 14:02:34 +0100 Subject: [PATCH 2/4] Add default config --- src/servala/core/crd/forms.py | 33 ++++++- src/servala/core/forms.py | 36 +++++-- src/tests/test_form_config.py | 180 ++++++++++++++++++++++++++++++++++ 3 files changed, 238 insertions(+), 11 deletions(-) diff --git a/src/servala/core/crd/forms.py b/src/servala/core/crd/forms.py index 99e2737..bdb8ed6 100644 --- a/src/servala/core/crd/forms.py +++ b/src/servala/core/crd/forms.py @@ -6,6 +6,27 @@ from servala.core.crd.utils import deslugify from servala.core.models import ControlPlaneCRD from servala.frontend.forms.widgets import DynamicArrayWidget +# Fields that must be present in every form +MANDATORY_FIELDS = ["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": { + "type": "text", + "label": "Instance Name", + "help_text": "Unique name for the new instance", + "required": True, + "max_length": 63, + }, + "spec.parameters.service.fqdn": { + "type": "array", + "label": "FQDNs", + "help_text": "Domain names for accessing this service", + "required": False, + }, +} + class FormGeneratorMixin: """Shared base class for ModelForm classes based on our generated CRD models. @@ -278,12 +299,20 @@ class CustomFormMixin(FormGeneratorMixin): def _apply_field_config(self): for fieldset in self.form_config.get("fieldsets", []): - for field_config in fieldset.get("fields", []): - field_name = field_config.get("controlplane_field_mapping") + for fc in fieldset.get("fields", []): + field_name = fc.get("controlplane_field_mapping") if field_name not in self.fields: continue + field_config = fc.copy() + # Merge with defaults if field has default config + if field_name in DEFAULT_FIELD_CONFIGS: + field_config = DEFAULT_FIELD_CONFIGS[field_name].copy() + for key, value in fc.items(): + if value or (value is False): + field_config[key] = value + field = self.fields[field_name] field_type = field_config.get("type") diff --git a/src/servala/core/forms.py b/src/servala/core/forms.py index 076a9d9..69132fc 100644 --- a/src/servala/core/forms.py +++ b/src/servala/core/forms.py @@ -6,6 +6,7 @@ from django import forms from django.utils.translation import gettext_lazy as _ from django_jsonform.widgets import JSONFormWidget +from servala.core.crd.forms import DEFAULT_FIELD_CONFIGS, MANDATORY_FIELDS from servala.core.models import ControlPlane, ServiceDefinition CONTROL_PLANE_USER_INFO_SCHEMA = { @@ -244,8 +245,26 @@ class ServiceDefinitionAdminForm(forms.ModelForm): for field in fieldset.get("fields", []): mapping = field.get("controlplane_field_mapping") included_mappings.add(mapping) + + # Validate that fields without defaults have required properties + if mapping not in DEFAULT_FIELD_CONFIGS: + if not field.get("label"): + errors.append( + _( + "Field with mapping '{}' must have a 'label' property " + "(or use a mapping with default config)" + ).format(mapping) + ) + if not field.get("type"): + errors.append( + _( + "Field with mapping '{}' must have a 'type' property " + "(or use a mapping with default config)" + ).format(mapping) + ) + if mapping and mapping not in valid_paths: - field_name = field.get("name", mapping) + field_name = field.get("label", field.get("name", mapping)) errors.append( _( "Field '{}' has invalid mapping '{}'. Valid paths are: {}" @@ -262,14 +281,13 @@ class ServiceDefinitionAdminForm(forms.ModelForm): field, mapping, spec_schema, "spec", errors ) - if "name" not in included_mappings: - raise forms.ValidationError( - { - "form_config": _( - "You must include a `name` field in the custom form config." - ) - } - ) + for mandatory_field in MANDATORY_FIELDS: + if mandatory_field not in included_mappings: + errors.append( + _( + "Required field '{}' must be included in the form configuration" + ).format(mandatory_field) + ) if errors: raise forms.ValidationError({"form_config": errors}) diff --git a/src/tests/test_form_config.py b/src/tests/test_form_config.py index ba44d72..7188d61 100644 --- a/src/tests/test_form_config.py +++ b/src/tests/test_form_config.py @@ -5,6 +5,7 @@ from django.core.validators import MaxValueValidator, MinValueValidator from django.db import models from servala.core.crd import generate_custom_form_class +from servala.core.crd.forms import DEFAULT_FIELD_CONFIGS, MANDATORY_FIELDS from servala.core.forms import ServiceDefinitionAdminForm from servala.core.models import ControlPlaneCRD @@ -904,3 +905,182 @@ def test_three_plus_element_choices_fail_validation(): assert len(errors) > 0 assert "must have 1 or 2 elements" in str(errors[0]) + + +def test_field_with_default_config_only_needs_mapping(): + + class TestModel(models.Model): + name = models.CharField(max_length=100) + + class Meta: + app_label = "test" + + minimal_config = { + "fieldsets": [ + { + "fields": [ + { + "controlplane_field_mapping": "name", + }, + ] + } + ] + } + + 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"] + + +def test_field_with_default_config_can_override_defaults(): + + class TestModel(models.Model): + name = models.CharField(max_length=100) + + class Meta: + app_label = "test" + + override_config = { + "fieldsets": [ + { + "fields": [ + { + "controlplane_field_mapping": "name", + "label": "Custom Name Label", + "required": False, + }, + ] + } + ] + } + + form_class = generate_custom_form_class(override_config, TestModel) + form = form_class() + + name_field = form.fields["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"] + + +def test_admin_form_validates_mandatory_fields_present(): + + mock_crd = Mock() + mock_crd.resource_schema = { + "properties": { + "spec": { + "properties": { + "environment": { + "type": "string", + "enum": ["dev", "prod"], + } + } + } + } + } + + config_without_name = { + "fieldsets": [ + { + "fields": [ + { + "type": "choice", + "label": "Environment", + "controlplane_field_mapping": "spec.environment", + "choices": [["dev", "Development"]], + }, + ] + } + ] + } + + errors = [] + included_mappings = set() + for fieldset in config_without_name.get("fieldsets", []): + for field in fieldset.get("fields", []): + mapping = field.get("controlplane_field_mapping") + included_mappings.add(mapping) + + for mandatory_field in MANDATORY_FIELDS: + if mandatory_field not in included_mappings: + errors.append(f"Required field '{mandatory_field}' must be included") + + assert len(errors) > 0 + assert "name" in str(errors[0]).lower() + + +def test_admin_form_validates_fields_without_defaults_need_label_and_type(): + config_with_incomplete_field = { + "fieldsets": [ + { + "fields": [ + {"controlplane_field_mapping": "name"}, # Has defaults - OK + { + "controlplane_field_mapping": "spec.unknown", # No defaults + # Missing label and type + }, + ] + } + ] + } + + errors = [] + + for fieldset in config_with_incomplete_field.get("fieldsets", []): + for field in fieldset.get("fields", []): + mapping = field.get("controlplane_field_mapping") + + if mapping not in DEFAULT_FIELD_CONFIGS: + if not field.get("label"): + errors.append( + f"Field with mapping '{mapping}' must have a 'label' property" + ) + if not field.get("type"): + errors.append( + f"Field with mapping '{mapping}' must have a 'type' property" + ) + + assert len(errors) == 2 + assert any("label" in str(e) for e in errors) + assert any("type" in str(e) for e in errors) + + +def test_empty_values_dont_override_default_configs(): + + class TestModel(models.Model): + name = models.CharField(max_length=100) + + class Meta: + app_label = "test" + + admin_form_config = { + "fieldsets": [ + { + "fields": [ + { + "controlplane_field_mapping": "name", + "type": "", + "label": "", + "help_text": None, + "max_length": None, + "required": False, + }, + ] + } + ] + } + + form_class = generate_custom_form_class(admin_form_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.max_length == DEFAULT_FIELD_CONFIGS["name"]["max_length"] + + assert name_field.required is False # Was overridden by explicit False From 7a8dc91afe48e0d7efe966975409713503cf8fe3 Mon Sep 17 00:00:00 2001 From: Tobias Kunze Date: Mon, 10 Nov 2025 14:02:40 +0100 Subject: [PATCH 3/4] Make sure max length applies to textarea --- src/servala/core/crd/forms.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/servala/core/crd/forms.py b/src/servala/core/crd/forms.py index bdb8ed6..659684e 100644 --- a/src/servala/core/crd/forms.py +++ b/src/servala/core/crd/forms.py @@ -350,6 +350,13 @@ class CustomFormMixin(FormGeneratorMixin): if "default_value" in field_config and field.initial is None: field.initial = field_config["default_value"] + if field_type in ("text", "textarea") and field_config.get( + "max_length" + ): + field.max_length = field_config.get("max_length") + if hasattr(field.widget, "attrs"): + field.widget.attrs["maxlength"] = field_config.get("max_length") + field.controlplane_field_mapping = field_name def get_fieldsets(self): From 9ac9f5e1c944f32e8d3962f41594118d137d9fd5 Mon Sep 17 00:00:00 2001 From: Tobias Kunze Date: Mon, 10 Nov 2025 14:04:42 +0100 Subject: [PATCH 4/4] Add print debugging to tab setup --- src/servala/static/js/bootstrap-tabs.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/servala/static/js/bootstrap-tabs.js b/src/servala/static/js/bootstrap-tabs.js index d382475..1d9eb89 100644 --- a/src/servala/static/js/bootstrap-tabs.js +++ b/src/servala/static/js/bootstrap-tabs.js @@ -3,15 +3,22 @@ (function() { 'use strict'; + console.log("Loading bootstrap tab setup") + const initBootstrapTabs = () => { + console.log("setting up bootstrap tabs") const customTabList = document.querySelectorAll('#myTab button[data-bs-toggle="tab"]'); + console.log("found custom tabs", customTabList) customTabList.forEach(function(tabButton) { new bootstrap.Tab(tabButton); + console.log("setting up custom tab", tabButton) }); const expertTabList = document.querySelectorAll('#expertTab button[data-bs-toggle="tab"]'); + console.log("found expert tabs", expertTabList) expertTabList.forEach(function(tabButton) { new bootstrap.Tab(tabButton); + console.log("setting up expert tab", tabButton) }); }