Skip to content

Commit 5f87a24

Browse files
kmalakmala
authored
Merge pull request #1105 from kmala/proctype
fix(procfile): validate the proctypes during build itself
2 parents 4ede878 + 53e118e commit 5f87a24

3 files changed

Lines changed: 65 additions & 27 deletions

File tree

‎rootfs/api/serializers.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,16 @@ class Meta:
189189
fields = ['owner', 'app', 'image', 'sha', 'procfile', 'dockerfile', 'created',
190190
'updated', 'uuid']
191191

192+
def validate_procfile(self, data):
193+
for key, value in data.items():
194+
if value is None or value == "":
195+
raise serializers.ValidationError("Command can't be empty for process type")
196+
197+
if not re.match(PROCTYPE_MATCH, key):
198+
raise serializers.ValidationError("Process types can only contain alphanumeric")
199+
200+
return data
201+
192202

193203
class ConfigSerializer(serializers.ModelSerializer):
194204
"""Serialize a :class:`~api.models.Config` model."""

‎rootfs/api/tests/test_build.py‎

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -201,10 +201,10 @@ def test_build_default_containers(self, mock_requests):
201201
body = {
202202
'image': 'autotest/example',
203203
'sha': 'a'*40,
204-
'procfile': json.dumps({
204+
'procfile': {
205205
'web': 'node server.js',
206206
'worker': 'node worker.js'
207-
})
207+
}
208208
}
209209
response = self.client.post(url, body)
210210
self.assertEqual(response.status_code, 201, response.data)
@@ -227,10 +227,10 @@ def test_build_default_containers(self, mock_requests):
227227
body = {
228228
'image': 'autotest/example',
229229
'sha': 'a'*40,
230-
'procfile': json.dumps({
230+
'procfile': {
231231
'rake': 'node server.js',
232232
'worker': 'node worker.js'
233-
})
233+
}
234234
}
235235
response = self.client.post(url, body)
236236
self.assertEqual(response.status_code, 201, response.data)
@@ -299,10 +299,10 @@ def test_new_build_does_not_scale_up_automatically(self, mock_requests):
299299
body = {
300300
'image': 'autotest/example',
301301
'sha': 'a'*40,
302-
'procfile': json.dumps({
302+
'procfile': {
303303
'web': 'node server.js',
304304
'worker': 'node worker.js'
305-
})
305+
}
306306
}
307307
response = self.client.post(url, body)
308308
self.assertEqual(response.status_code, 201, response.data)
@@ -323,10 +323,10 @@ def test_new_build_does_not_scale_up_automatically(self, mock_requests):
323323
body = {
324324
'image': 'autotest/example',
325325
'sha': 'a'*40,
326-
'procfile': json.dumps({
326+
'procfile': {
327327
'web': 'node server.js',
328328
'worker': 'node worker.js'
329-
})
329+
}
330330
}
331331
response = self.client.post(url, body)
332332
self.assertEqual(response.status_code, 201, response.data)
@@ -487,3 +487,31 @@ def test_build_failures(self, mock_requests):
487487
self.assertEqual(app.release_set.latest().version, 4)
488488
self.assertEqual(app.release_set.latest().build, success_build)
489489
self.assertEqual(app.build_set.count(), 2)
490+
491+
def test_build_validate_procfile(self, mock_requests):
492+
app_id = self.create_app()
493+
494+
# deploy app with incorrect proctype
495+
url = "/v2/apps/{app_id}/builds".format(**locals())
496+
body = {
497+
'image': 'autotest/example',
498+
'sha': 'a'*40,
499+
'procfile': {
500+
'web': 'node server.js',
501+
'worker_test': 'node worker.js'
502+
}
503+
}
504+
response = self.client.post(url, body)
505+
self.assertEqual(response.status_code, 400, response.data)
506+
# deploy app with empty command
507+
url = "/v2/apps/{app_id}/builds".format(**locals())
508+
body = {
509+
'image': 'autotest/example',
510+
'sha': 'a'*40,
511+
'procfile': {
512+
'web': 'node server.js',
513+
'worker': ''
514+
}
515+
}
516+
response = self.client.post(url, body)
517+
self.assertEqual(response.status_code, 400, response.data)

‎rootfs/api/tests/test_pods.py‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,10 @@ def test_container_api_heroku(self, mock_requests):
4949
body = {
5050
'image': 'autotest/example',
5151
'sha': 'a'*40,
52-
'procfile': json.dumps({
52+
'procfile': {
5353
'web': 'node server.js',
5454
'worker': 'node worker.js'
55-
})
55+
}
5656
}
5757
response = self.client.post(url, body)
5858
self.assertEqual(response.status_code, 201, response.data)
@@ -211,10 +211,10 @@ def test_release(self, mock_requests):
211211
body = {
212212
'image': 'autotest/example',
213213
'sha': 'a'*40,
214-
'procfile': json.dumps({
214+
'procfile': {
215215
'web': 'node server.js',
216216
'worker': 'node worker.js'
217-
})
217+
}
218218
}
219219
response = self.client.post(url, body)
220220
self.assertEqual(response.status_code, 201, response.data)
@@ -298,10 +298,10 @@ def test_container_str(self, mock_requests):
298298
body = {
299299
'image': 'autotest/example',
300300
'sha': 'a'*40,
301-
'procfile': json.dumps({
301+
'procfile': {
302302
'web': 'node server.js',
303303
'worker': 'node worker.js'
304-
})
304+
}
305305
}
306306
response = self.client.post(url, body)
307307
self.assertEqual(response.status_code, 201, response.data)
@@ -333,10 +333,10 @@ def test_pod_command_format(self, mock_requests):
333333
body = {
334334
'image': 'autotest/example',
335335
'sha': 'a'*40,
336-
'procfile': json.dumps({
336+
'procfile': {
337337
'web': 'node server.js',
338338
'worker': 'node worker.js'
339-
})
339+
}
340340
}
341341
response = self.client.post(url, body)
342342
self.assertEqual(response.status_code, 201, response.data)
@@ -377,10 +377,10 @@ def test_scale_errors(self, mock_requests):
377377
body = {
378378
'image': 'autotest/example',
379379
'sha': 'a'*40,
380-
'procfile': json.dumps({
380+
'procfile': {
381381
'web': 'node server.js',
382382
'worker': 'node worker.js'
383-
})
383+
}
384384
}
385385
response = self.client.post(url, body)
386386
self.assertEqual(response.status_code, 201, response.data)
@@ -438,10 +438,10 @@ def test_admin_can_manage_other_pods(self, mock_requests):
438438
body = {
439439
'image': 'autotest/example',
440440
'sha': 'a'*40,
441-
'procfile': json.dumps({
441+
'procfile': {
442442
'web': 'node server.js',
443443
'worker': 'node worker.js'
444-
})
444+
}
445445
}
446446
response = self.client.post(url, body)
447447
self.assertEqual(response.status_code, 201, response.data)
@@ -675,7 +675,7 @@ def test_scale_with_unauthorized_user_returns_403(self, mock_requests):
675675
body = {
676676
'image': 'autotest/example',
677677
'sha': 'a'*40,
678-
'procfile': json.dumps({'web': 'node server.js', 'worker': 'node worker.js'})
678+
'procfile': {'web': 'node server.js', 'worker': 'node worker.js'}
679679
}
680680
response = self.client.post(url, body)
681681
unauthorized_user = User.objects.get(username='autotest2')
@@ -700,10 +700,10 @@ def test_modified_procfile_from_build_removes_pods(self, mock_requests):
700700
body = {
701701
'image': 'autotest/example',
702702
'sha': 'a'*40,
703-
'procfile': json.dumps({
703+
'procfile': {
704704
'web': 'node server.js',
705705
'worker': 'node worker.js'
706-
})
706+
}
707707
}
708708
response = self.client.post(build_url, body)
709709

@@ -715,9 +715,9 @@ def test_modified_procfile_from_build_removes_pods(self, mock_requests):
715715
body = {
716716
'image': 'autotest/example',
717717
'sha': 'a'*40,
718-
'procfile': json.dumps({
718+
'procfile': {
719719
'worker': 'node worker.js'
720-
})
720+
}
721721
}
722722
response = self.client.post(build_url, body)
723723
self.assertEqual(response.status_code, 201, response.data)
@@ -735,10 +735,10 @@ def test_restart_pods(self, mock_requests):
735735
body = {
736736
'image': 'autotest/example',
737737
'sha': 'a'*40,
738-
'procfile': json.dumps({
738+
'procfile': {
739739
'web': 'node server.js',
740740
'worker': 'node worker.js'
741-
})
741+
}
742742
}
743743
response = self.client.post(build_url, body)
744744

0 commit comments

Comments
 (0)