Skip to content

Commit 7392eb3

Browse files
committed
fix(config): proc type names validation should allow alphanumeric
And the way the regex was written it would also match as many valid chars as it could find, so even one a-z char at the start was saying the name was valid https://docs-v2.readthedocs.io/en/latest/using-workflow/process-types-and-the-procfile/#declaring-process-types
1 parent de79dcd commit 7392eb3

3 files changed

Lines changed: 35 additions & 10 deletions

File tree

rootfs/api/serializers.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@
1111

1212
from api import models
1313

14-
PROCTYPE_MATCH = re.compile(r'^(?P<type>[a-z]+)')
14+
# proc type name is alphanumeric
15+
# https://docs-v2.readthedocs.io/en/latest/using-workflow/process-types-and-the-procfile/#declaring-process-types
16+
PROCTYPE_MATCH = re.compile(r'^(?P<type>[a-z0-9]+)$')
1517
MEMLIMIT_MATCH = re.compile(r'^(?P<mem>[0-9]+(MB|KB|GB|[BKMG]))$', re.IGNORECASE)
1618
CPUSHARE_MATCH = re.compile(r'^(?P<cpu>[0-9.]+[m]{0,1})$')
1719
TAGVAL_MATCH = re.compile(r'^(?:[a-zA-Z\d][-\.\w]{0,61})?[a-zA-Z\d]$')
@@ -145,7 +147,7 @@ def validate_memory(self, data):
145147
continue
146148

147149
if not re.match(PROCTYPE_MATCH, key):
148-
raise serializers.ValidationError("Process types can only contain [a-z]")
150+
raise serializers.ValidationError("Process types can only contain alphanumeric")
149151

150152
if not re.match(MEMLIMIT_MATCH, str(value)):
151153
raise serializers.ValidationError(
@@ -159,7 +161,7 @@ def validate_cpu(self, data):
159161
continue
160162

161163
if not re.match(PROCTYPE_MATCH, key):
162-
raise serializers.ValidationError("Process types can only contain [a-z]")
164+
raise serializers.ValidationError("Process types can only contain alphanumeric")
163165

164166
shares = re.match(CPUSHARE_MATCH, str(value))
165167
if not shares:

rootfs/api/tests/test_config.py

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44
55
Run the tests with "./manage.py test api"
66
"""
7-
8-
97
import json
108

119
from django.contrib.auth.models import User
@@ -355,6 +353,17 @@ def test_limit_memory(self, mock_requests):
355353
self.assertNotEqual(limit3['uuid'], limit4['uuid'])
356354
self.assertNotIn('worker', json.dumps(response.data['memory']))
357355

356+
# bad memory values
357+
mem = {'web': '1Z'}
358+
body = {'memory': json.dumps(mem)}
359+
response = self.client.post(url, body)
360+
self.assertEqual(response.status_code, 400)
361+
362+
mem = {'w3&b': '1G'}
363+
body = {'memory': json.dumps(mem)}
364+
response = self.client.post(url, body)
365+
self.assertEqual(response.status_code, 400)
366+
358367
# disallow put/patch/delete
359368
response = self.client.put(url)
360369
self.assertEqual(response.status_code, 405)
@@ -397,14 +406,14 @@ def test_limit_cpu(self, mock_requests):
397406
self.assertEqual(cpu['web'], '1024')
398407

399408
# set an additional value
400-
body = {'cpu': json.dumps({'worker': '512'})}
409+
body = {'cpu': json.dumps({'worker': '512m'})}
401410
response = self.client.post(url, body)
402411
self.assertEqual(response.status_code, 201)
403412
limit2 = response.data
404413
self.assertNotEqual(limit1['uuid'], limit2['uuid'])
405414
cpu = response.data['cpu']
406415
self.assertIn('worker', cpu)
407-
self.assertEqual(cpu['worker'], '512')
416+
self.assertEqual(cpu['worker'], '512m')
408417
self.assertIn('web', cpu)
409418
self.assertEqual(cpu['web'], '1024')
410419

@@ -415,17 +424,28 @@ def test_limit_cpu(self, mock_requests):
415424
self.assertEqual(limit2, limit3)
416425
cpu = response.data['cpu']
417426
self.assertIn('worker', cpu)
418-
self.assertEqual(cpu['worker'], '512')
427+
self.assertEqual(cpu['worker'], '512m')
419428
self.assertIn('web', cpu)
420429
self.assertEqual(cpu['web'], '1024')
421430

422431
# unset a value
423-
body = {'memory': json.dumps({'worker': None})}
432+
body = {'cpu': json.dumps({'worker': None})}
424433
response = self.client.post(url, body)
425434
self.assertEqual(response.status_code, 201)
426435
limit4 = response.data
427436
self.assertNotEqual(limit3['uuid'], limit4['uuid'])
428-
self.assertNotIn('worker', json.dumps(response.data['memory']))
437+
self.assertNotIn('worker', json.dumps(response.data['cpu']))
438+
439+
# bad cpu values
440+
mem = {'web': '1G'}
441+
body = {'cpu': json.dumps(mem)}
442+
response = self.client.post(url, body)
443+
self.assertEqual(response.status_code, 400)
444+
445+
mem = {'w3&b': '1G'}
446+
body = {'cpu': json.dumps(mem)}
447+
response = self.client.post(url, body)
448+
self.assertEqual(response.status_code, 400)
429449

430450
# disallow put/patch/delete
431451
response = self.client.put(url)

rootfs/scheduler/mock.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,9 @@ def delete_pods(url, pods, current, desired):
9898
if len(removed) == delta:
9999
break
100100

101+
if not pods:
102+
break
103+
101104
item = pods.pop()
102105
pod = cache.get(item)
103106
if 'deletionTimestamp' in pod['metadata']:

0 commit comments

Comments
 (0)