Merge pull request #2673 from bacongobbler/no-bash-wrapping

fix(controller): revert to `start proctype` for buildpack images

Author: Matthew Fisher

builder/bin/generate-buildhook.go

@@ -4,7 +4,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
-	"strconv"
 
 	"github.com/deis/deis/builder"
 )
@@ -29,8 +28,10 @@ func main() {
 	var procfile builder.ProcessType
 	assert(json.Unmarshal([]byte(os.Args[5]), &procfile))
 
-	dockerfile, err := strconv.ParseBool(os.Args[6])
-	assert(err)
+	var dockerfile string = os.Args[6]
+	if dockerfile == "false" {
+		dockerfile = ""
+	}
 
 	buildHook := builder.BuildHook{
 		Sha:         os.Args[1],

builder/image/templates/builder

@@ -149,9 +149,12 @@ else
     RELEASE_INFO="{}"
 fi
 
-if [ -f $TMP_DIR/Procfile ]; then
-    # update release info with data from the Procfile
-    RELEASE_INFO=$(cat $TMP_DIR/Procfile | /app/bin/yaml2json-procfile)
+if [ "$RELEASE_INFO" == "{}" ]; then
+    if [ -f $TMP_DIR/Procfile ]; then
+        # fall back to Procfile if there's no default process types
+        # FIXME: refactor into slugbuilder
+        RELEASE_INFO=$(cat $TMP_DIR/Procfile | /app/bin/yaml2json-procfile)
+    fi
 fi
 
 puts-step "Launching... "

builder/types.go

@@ -27,7 +27,7 @@ type BuildHook struct {
 	ReceiveRepo string      `json:"receive_repo"`
 	Image       string      `json:"image"`
 	Procfile    ProcessType `json:"procfile"`
-	Dockerfile  bool        `json:"dockerfile"`
+	Dockerfile  string      `json:"dockerfile"`
 }
 
 // BuildHookResponse represents a controller's build-hook response object.

builder/utils.go

@@ -71,7 +71,7 @@ func ParseReleaseVersion(bytes []byte) (int, error) {
 
 func GetDefaultType(bytes []byte) (string, error) {
 	type YamlTypeMap struct {
-		DefaultProcessTypes ProcessType
+		DefaultProcessTypes ProcessType `default_process_types`
 	}
 
 	var p YamlTypeMap
@@ -80,7 +80,7 @@ func GetDefaultType(bytes []byte) (string, error) {
 		return "", err
 	}
 
-	retVal, err := json.Marshal(&p)
+	retVal, err := json.Marshal(&p.DefaultProcessTypes)
 
 	if err != nil {
 		return "", err

builder/utils_test.go

@@ -121,8 +121,11 @@ default_process_types:
 		if err != nil {
 			t.Error(err)
 		}
-		if defaultType == "" {
-			t.Error("default type cannot be empty")
+		if defaultType != `{"web":"while true; do echo hello; sleep 1; done"}` && string(data) != "" {
+			t.Errorf("incorrect default type, got %s", defaultType)
+		}
+		if string(data) == "" && defaultType != "{}" {
+			t.Errorf("incorrect default type, got %s", defaultType)
 		}
 	}
 }

controller/api/models.py

@@ -407,8 +407,13 @@ def _get_command(self):
         if self.type == 'cmd':
             return ''
         try:
-            # ensure they cannot break out and run commands on the host
-            return "bash -c '{}'".format(self.release.build.procfile[self.type])
+            # if this is not procfile-based app, ensure they cannot break out
+            # and run arbitrary commands on the host
+            # FIXME: remove slugrunner's hardcoded entrypoint
+            if self.release.build.dockerfile or not self.release.build.sha:
+                return "bash -c '{}'".format(self.release.build.procfile[self.type])
+            else:
+                return 'start {}'.format(self.type)
         # if the key is not present or if a parent attribute is None
         except (KeyError, TypeError):
             return 'start {}'.format(self.type)

controller/api/tests/test_container.py

@@ -456,11 +456,14 @@ def test_command_good(self):
         app_id = response.data['id']
         app = App.objects.get(id=app_id)
         user = User.objects.get(username='autotest')
+        # Heroku Buildpack app
         build = Build.objects.create(owner=user,
                                      app=app,
                                      image="qwerty",
                                      procfile={'web': 'node server.js',
-                                               'worker': 'node worker.js'})
+                                               'worker': 'node worker.js'},
+                                     sha='african-swallow',
+                                     dockerfile='')
         # create an initial release
         release = Release.objects.create(version=2,
                                          owner=user,
@@ -473,10 +476,22 @@ def test_command_good(self):
                                      release=release,
                                      type='web',
                                      num=1)
+        # use `start web` for backwards compatibility with slugrunner
+        self.assertEqual(c._command, 'start web')
+        c.type = 'worker'
+        self.assertEqual(c._command, 'start worker')
+        # switch to docker image app
+        build.sha = None
+        c.type = 'web'
+        self.assertEqual(c._command, "bash -c 'node server.js'")
+        # switch to dockerfile app
+        build.sha = 'european-swallow'
+        build.dockerfile = 'dockerdockerdocker'
         self.assertEqual(c._command, "bash -c 'node server.js'")
         c.type = 'worker'
         self.assertEqual(c._command, "bash -c 'node worker.js'")
         c.release.build.procfile = None
+        # for backwards compatibility if no Procfile is supplied
         self.assertEqual(c._command, 'start worker')
         c.type = 'cmd'
         self.assertEqual(c._command, '')