disallow editing of infra type on instances in api, check in model save as well

Author: dogboat

dojo/cicd_infrastructure/api/serializers.py

@@ -7,3 +7,10 @@ class CICDInfrastructureSerializer(serializers.ModelSerializer):
     class Meta:
         model = CICDInfrastructure
         fields = "__all__"
+
+    def get_fields(self):
+        fields = super().get_fields()
+        if self.instance is not None:
+            # Disallow editing of infra type on an instance; see the matching comment on CICDInfrastructure#save()
+            fields["infrastructure_type"].read_only = True
+        return fields

dojo/cicd_infrastructure/models.py

@@ -1,3 +1,4 @@
+from django.core.exceptions import ValidationError
 from django.db import models
 from django.utils.translation import gettext as _
 
@@ -20,3 +21,15 @@ class Meta:
 
     def __str__(self):
         return self.name
+
+    def save(self, *args, **kwargs):
+        if self.pk:
+            # Disallow editing of the infra type on an instance; engagement CICD FKs are scoped by infrastructure_type
+            # via limit_choices_to (build_server/scm_server/orchestration), so changing the type would create a
+            # semantic conflict between an engagement and this object.
+            current_type = type(self).objects.filter(pk=self.pk).values_list("infrastructure_type", flat=True).first()
+            if current_type is not None and current_type != self.infrastructure_type:
+                raise ValidationError(
+                    {"infrastructure_type": _("infrastructure_type cannot be changed once set.")},
+                )
+        super().save(*args, **kwargs)

dojo/cicd_infrastructure/ui/forms.py

@@ -11,7 +11,5 @@ class Meta:
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         if self.instance.pk:
-            # Disallow editing of the infra type on an instance; engagement CICD FKs are scoped by infrastructure_type
-            # via limit_choices_to (build_server/scm_server/orchestration), so changing the type would create a
-            # semantic conflict between an engagement and this object.
+            # Disallow editing of infra type on an instance; see the matching comment on CICDInfrastructure#save()
             self.fields["infrastructure_type"].disabled = True

unittests/test_cicd_infrastructure.py

@@ -5,6 +5,7 @@
 ``test_cicd_infrastructure_migration.py``.
 """
 
+from django.core.exceptions import ValidationError
 from django.db import IntegrityError, transaction
 from django.urls import reverse
 from rest_framework.authtoken.models import Token
@@ -45,6 +46,36 @@ def test_description_and_url_default_to_empty_string(self):
         self.assertEqual(infra.description, "")
         self.assertEqual(infra.url, "")
 
+    def test_save_rejects_infrastructure_type_change_on_existing(self):
+        """
+        Model save() guard: infrastructure_type is immutable once an instance exists.
+        Engagement CICD FKs are scoped by infrastructure_type via limit_choices_to,
+        so flipping the type would leave any referencing engagement pointing at a row
+        whose type contradicts the FK's role.
+        """
+        infra = CICDInfrastructure.objects.create(name="Locked", infrastructure_type="build_server")
+        infra.infrastructure_type = "scm_server"
+        with self.assertRaises(ValidationError) as ctx:
+            infra.save()
+        self.assertIn("infrastructure_type", ctx.exception.message_dict)
+        # The DB row is untouched.
+        self.assertEqual(
+            CICDInfrastructure.objects.get(pk=infra.pk).infrastructure_type,
+            "build_server",
+        )
+
+    def test_save_allows_changing_other_fields_on_existing(self):
+        """name/description/url remain editable post-creation."""
+        infra = CICDInfrastructure.objects.create(name="Editable", infrastructure_type="build_server")
+        infra.name = "Renamed"
+        infra.description = "now has a description"
+        infra.url = "https://renamed.example.com"
+        infra.save()
+        infra.refresh_from_db()
+        self.assertEqual(infra.name, "Renamed")
+        self.assertEqual(infra.description, "now has a description")
+        self.assertEqual(infra.url, "https://renamed.example.com")
+
 
 # ---------------------------------------------------------------------------
 # Form
@@ -167,3 +198,23 @@ def test_non_superuser_cannot_create(self):
         )
         self.assertEqual(r.status_code, 403, r.content[:1000])
         self.assertFalse(CICDInfrastructure.objects.filter(name="NonSuperCreate").exists())
+
+    def test_patch_silently_ignores_infrastructure_type_change(self):
+        """
+        Serializer marks infrastructure_type read_only on update, so DRF drops the
+        incoming value from validated_data. The PATCH succeeds (200) but the type is
+        unchanged — same defense the form provides for the legacy UI.
+        """
+        existing = CICDInfrastructure.objects.create(
+            name="ApiLocked", infrastructure_type="build_server",
+        )
+        c = self._client_for("admin")
+        r = c.patch(
+            reverse("cicd_infrastructure-detail", args=[existing.pk]),
+            {"infrastructure_type": "scm_server", "description": "still build"},
+            format="json",
+        )
+        self.assertEqual(r.status_code, 200, r.content[:1000])
+        existing.refresh_from_db()
+        self.assertEqual(existing.infrastructure_type, "build_server")
+        self.assertEqual(existing.description, "still build")