Address sigprof review: stricter token validation

Per sigprof's feedback, the previous validation (depth >= token_count)
allowed a questionable case where token_count > 1 but < depth, which
would lose position information for some levels.

New validation: token_count must be either 1 (full path expansion) or
exactly match the tree depth (level-by-level substitution).

Updated test T2 to verify this mismatched case is now rejected.
This commit is contained in:
Mark Coleman
2026-01-19 16:03:12 +01:00
parent 8c4ba36319
commit 233e623783
2 changed files with 25 additions and 16 deletions

View File

@@ -127,8 +127,9 @@ class ModuleCommonForm(forms.Form):
)
token_count = template.name.count(MODULE_TOKEN)
# Validate: depth must be >= token_count (can't expand tokens without context)
if len(module_bays) < token_count:
# A single token which gets expanded to the full path is always
# allowed; otherwise the number of tokens needs to match the path length.
if token_count != 1 and token_count != len(module_bays):
raise forms.ValidationError(
_(
"Cannot install module with placeholder values in a module bay tree {level} in tree "

View File

@@ -1144,11 +1144,14 @@ class ModuleBayTestCase(TestCase):
interface = sfp_module.interfaces.first()
self.assertEqual(interface.name, 'SFP 1/2')
def test_multi_token_deeper_tree_only_consumes_tokens(self):
def test_mismatched_multi_token_fails_validation(self):
"""
T2: Multi-token with deeper tree only consumes tokens (depth=3, tokens=2).
2 tokens → 2 levels, even if tree is deeper.
T2: Multi-token with mismatched depth fails validation (depth=3, tokens=2).
Per sigprof's feedback: allowing this would lose position info for level 3.
Only single-token (full path) or exact-match multi-token should be allowed.
"""
from dcim.forms import ModuleForm
site = Site.objects.create(name='T2 Site', slug='t2-site')
manufacturer = Manufacturer.objects.create(name='T2 Manufacturer', slug='t2-manufacturer')
device_role = DeviceRole.objects.create(name='T2 Role', slug='t2-role')
@@ -1187,7 +1190,7 @@ class ModuleBayTestCase(TestCase):
position='1'
)
# Create leaf module type with 2-token interface template
# Create leaf module type with 2-token interface template (mismatched for depth 3)
leaf_type = ModuleType.objects.create(
manufacturer=manufacturer,
model='T2 Leaf'
@@ -1198,7 +1201,7 @@ class ModuleBayTestCase(TestCase):
type=InterfaceTypeChoices.TYPE_10GE_SFP_PLUS
)
# Create device and install 3 levels of modules
# Create device and install first 2 levels of modules
device = Device.objects.create(
name='T2 Device',
device_type=device_type,
@@ -1222,17 +1225,22 @@ class ModuleBayTestCase(TestCase):
module_type=level3_type
)
# Level 3 (leaf)
# Attempt to install leaf module at depth=3 with 2 tokens - should fail
bay3 = module2.modulebays.get(name='Level3 Bay')
leaf_module = Module.objects.create(
device=device,
module_bay=bay3,
module_type=leaf_type
)
# Verify: 2 tokens → consumes first 2 levels only: "1/1" (not "1/1/1")
interface = leaf_module.interfaces.first()
self.assertEqual(interface.name, 'SFP 1/1')
form = ModuleForm(data={
'device': device.pk,
'module_bay': bay3.pk,
'module_type': leaf_type.pk,
'status': 'active',
'replicate_components': True,
'adopt_components': False,
})
# Validation should fail: 2 tokens != 1 and 2 tokens != 3 depth
self.assertFalse(form.is_valid())
self.assertIn('2', str(form.errors))
self.assertIn('3', str(form.errors))
def test_too_many_tokens_fails_validation(self):
"""