Stop using container versions in config.yaml, version the APIVersion, f…

…ixes #762, fixes #713 (#820)

cmd/ddev/cmd/describe_test.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"strings"
 	"testing"
 
 	"encoding/json"
@@ -76,16 +77,22 @@ func TestDescribe(t *testing.T) {
 		args = []string{"describe", "-j"}
 		out, err = exec.RunCommand(DdevBin, args)
 		assert.NoError(err)
-		// Unmarshall the json results results. The describe function only has 4 fields to output
-		data := make(log.Fields, 4)
-		err = json.Unmarshal([]byte(out), &data)
+		logItems, err := unmarshallJSONLogs(out)
 		assert.NoError(err)
+
+		// The description log should be the last item; there may be a warning
+		// or other info before that.
+		data := logItems[len(logItems)-1]
+		assert.EqualValues(data["level"], "info")
 		raw, ok := data["raw"].(map[string]interface{})
 		assert.True(ok)
 		assert.EqualValues(raw["status"], "running")
 		assert.EqualValues(raw["name"], v.Name)
 		assert.EqualValues(raw["shortroot"].(string), ddevapp.RenderHomeRootedDir(v.Dir))
 		assert.EqualValues(raw["approot"].(string), v.Dir)
+
+		assert.NotEmpty(data["msg"])
+
 		cleanup()
 	}
 }
@@ -176,3 +183,22 @@ func TestDescribeAppWithInvalidParams(t *testing.T) {
 	assert.Error(err)
 	cleanup()
 }
+
+// unmarshallJSONLogs takes a string buffer and splits it into lines,
+// discards empty lines, and unmarshalls into an array of logs
+func unmarshallJSONLogs(in string) ([]log.Fields, error) {
+	logData := make([]log.Fields, 0)
+	logStrings := strings.Split(in, "\n")
+	data := make(log.Fields, 4)
+
+	for _, logLine := range logStrings {
+		if logLine != "" {
+			err := json.Unmarshal([]byte(logLine), &data)
+			if err != nil {
+				return []log.Fields{}, err
+			}
+			logData = append(logData, data)
+		}
+	}
+	return logData, nil
+}

cmd/ddev/cmd/list_test.go

@@ -5,13 +5,11 @@ import (
 	"runtime"
 	"testing"
 
-	"encoding/json"
 	oexec "os/exec"
 	"time"
 
 	"github.com/drud/ddev/pkg/ddevapp"
 	"github.com/drud/ddev/pkg/exec"
-	log "github.com/sirupsen/logrus"
 	asrt "github.com/stretchr/testify/assert"
 )
 
@@ -29,10 +27,14 @@ func TestDevList(t *testing.T) {
 	jsonOut, err := exec.RunCommand(DdevBin, args)
 	assert.NoError(err)
 
-	// Unmarshall the json results. The list function has 4 fields to output
-	data := make(log.Fields, 4)
-	err = json.Unmarshal([]byte(jsonOut), &data)
+	logItems, err := unmarshallJSONLogs(jsonOut)
 	assert.NoError(err)
+
+	// The list should be the last item; there may be a warning
+	// or other info before that.
+	data := logItems[len(logItems)-1]
+	assert.EqualValues(data["level"], "info")
+
 	raw, ok := data["raw"].([]interface{})
 	assert.True(ok)
 

cmd/ddev/cmd/restart_test.go

@@ -3,13 +3,10 @@ package cmd
 import (
 	"testing"
 
-	"encoding/json"
-
 	"strings"
 
 	"github.com/drud/ddev/pkg/ddevapp"
 	"github.com/drud/ddev/pkg/exec"
-	log "github.com/sirupsen/logrus"
 	asrt "github.com/stretchr/testify/assert"
 )
 
@@ -48,28 +45,16 @@ func TestDevRestartJSON(t *testing.T) {
 		args := []string{"restart", "-j"}
 		out, err := exec.RunCommand(DdevBin, args)
 		assert.NoError(err)
-		logStrings := strings.Split(out, "\n")
-		// We expect 4 lines of json in result, and a blank (Restarting,
-		// need-add-hosts, successfully restarted, can be reached at,
-		// blank at end)
-		assert.True(len(logStrings) >= 3)
 
-		// Wander through the json output lines making sure they're reasonable json.
-		for _, entry := range logStrings {
-			if entry != "" { // Ignore empty line.
-				// Unmarshall the json results. Normal log entries have 3 fields
-				data := make(log.Fields, 3)
-				err = json.Unmarshal([]byte(entry), &data)
-				assert.NoError(err)
-				if !strings.Contains(data["msg"].(string), "You must manually add the following") && !strings.Contains(data["msg"].(string), "Warning: containers will run as root") {
-					assert.EqualValues(data["level"], "info")
-				}
-				assert.NotEmpty(data["msg"])
-			}
-		}
+		logItems, err := unmarshallJSONLogs(out)
+		assert.NoError(err)
+
+		// The key item should be the last item; there may be a warning
+		// or other info before that.
+		data := logItems[len(logItems)-1]
+		assert.EqualValues(data["level"], "info")
+		assert.Contains(data["msg"], "Your project can be reached at "+strings.Join(app.GetAllURLs(), ", "))
 
-		// Go ahead and look for normal strings within the json output.
-		assert.Contains(string(out), strings.Join(app.GetAllURLs(), ", "))
 		cleanup()
 	}
 }

pkg/ddevapp/config.go

@@ -32,10 +32,6 @@ const DdevDefaultRouterHTTPPort = "80"
 // DdevDefaultRouterHTTPSPort is the starting https router port, 443
 const DdevDefaultRouterHTTPSPort = "443"
 
-// CurrentAppVersion sets the current YAML config file version.
-// We're not doing anything with AppVersion, so just default it to 1 for now.
-const CurrentAppVersion = "1"
-
 // Regexp pattern to determine if a hostname is valid per RFC 1123.
 var hostRegex = regexp.MustCompile(`^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$`)
 
@@ -64,7 +60,7 @@ func NewApp(AppRoot string, provider string) (*DdevApp, error) {
 
 	app.AppRoot = AppRoot
 	app.ConfigPath = app.GetConfigPath("config.yaml")
-	app.APIVersion = CurrentAppVersion
+	app.APIVersion = version.DdevVersion
 	app.PHPVersion = DdevDefaultPHPVersion
 	app.RouterHTTPPort = DdevDefaultRouterHTTPPort
 	app.RouterHTTPSPort = DdevDefaultRouterHTTPSPort
@@ -106,37 +102,50 @@ func (app *DdevApp) GetConfigPath(filename string) string {
 // WriteConfig writes the app configuration into the .ddev folder.
 func (app *DdevApp) WriteConfig() error {
 
-	err := PrepDdevDirectory(filepath.Dir(app.ConfigPath))
+	// Work against a copy of the DdevApp, since we don't want to actually change it.
+	appcopy := *app
+	// Update the "APIVersion" to be the ddev version.
+	appcopy.APIVersion = version.DdevVersion
+
+	// We don't want to even set the images on write, even though we'll respect them on read.
+	appcopy.DBAImage = ""
+	appcopy.DBImage = ""
+	appcopy.WebImage = ""
+
+	err := PrepDdevDirectory(filepath.Dir(appcopy.ConfigPath))
 	if err != nil {
 		return err
 	}
 
-	cfgbytes, err := yaml.Marshal(app)
+	cfgbytes, err := yaml.Marshal(appcopy)
 	if err != nil {
 		return err
 	}
 
+	// Append current image information
+	cfgbytes = append(cfgbytes, []byte(fmt.Sprintf("\n\n# This config.yaml was created with ddev version %s \n# webimage: %s:%s\n# dbimage: %s:%s\n# dbaimage: %s:%s\n# However we do not recommend explicitly wiring these images into the\n# config.yaml as they may break future versions of ddev.\n# You can update this config.yaml using 'ddev config'.\n", version.DdevVersion, version.WebImg, version.WebTag, version.DBImg, version.DBTag, version.DBAImg, version.DBATag))...)
+
 	// Append hook information and sample hook suggestions.
 	cfgbytes = append(cfgbytes, []byte(ConfigInstructions)...)
-	cfgbytes = append(cfgbytes, app.GetHookDefaultComments()...)
+	cfgbytes = append(cfgbytes, appcopy.GetHookDefaultComments()...)
 
-	err = ioutil.WriteFile(app.ConfigPath, cfgbytes, 0644)
+	err = ioutil.WriteFile(appcopy.ConfigPath, cfgbytes, 0644)
 	if err != nil {
 		return err
 	}
 
-	provider, err := app.GetProvider()
+	provider, err := appcopy.GetProvider()
 	if err != nil {
 		return err
 	}
 
-	err = provider.Write(app.GetConfigPath("import.yaml"))
+	err = provider.Write(appcopy.GetConfigPath("import.yaml"))
 	if err != nil {
 		return err
 	}
 
-	// Allow app-specific post-config action
-	err = app.PostConfigAction()
+	// Allow project-specific post-config action
+	err = appcopy.PostConfigAction()
 	if err != nil {
 		return err
 	}
@@ -165,6 +174,9 @@ func (app *DdevApp) ReadConfig() error {
 		return err
 	}
 
+	if app.APIVersion != version.DdevVersion {
+		util.Warning("Your .ddev/config.yaml version is %s, but ddev is version %s. \nPlease run 'ddev config' to update your config.yaml. \nddev may not operate correctly until you do.", app.APIVersion, version.DdevVersion)
+	}
 	// If any of these values aren't defined in the config file, set them to defaults.
 	if app.Name == "" {
 		app.Name = filepath.Base(app.AppRoot)

pkg/ddevapp/config_test.go

@@ -31,7 +31,7 @@ func TestNewConfig(t *testing.T) {
 	assert.NoError(err)
 
 	// Ensure the config uses specified defaults.
-	assert.Equal(app.APIVersion, CurrentAppVersion)
+	assert.Equal(app.APIVersion, version.DdevVersion)
 	assert.Equal(app.DBImage, version.DBImg+":"+version.DBTag)
 	assert.Equal(app.WebImage, version.WebImg+":"+version.WebTag)
 	assert.Equal(app.DBAImage, version.DBAImg+":"+version.DBATag)
@@ -304,13 +304,10 @@ func TestReadConfig(t *testing.T) {
 
 	// This closely resembles the values one would have from NewApp()
 	app := &DdevApp{
+		APIVersion: version.DdevVersion,
 		ConfigPath: filepath.Join("testdata", "config.yaml"),
 		AppRoot:    "testdata",
-		APIVersion: CurrentAppVersion,
 		Name:       "TestRead",
-		WebImage:   version.WebImg + ":" + version.WebTag,
-		DBImage:    version.DBImg + ":" + version.DBTag,
-		DBAImage:   version.DBAImg + ":" + version.DBATag,
 		Provider:   DefaultProviderName,
 	}
 
@@ -321,10 +318,11 @@ func TestReadConfig(t *testing.T) {
 
 	// Values not defined in file, we should still have default values
 	assert.Equal(app.Name, "TestRead")
-	assert.Equal(app.DBImage, version.DBImg+":"+version.DBTag)
+	assert.Equal(app.APIVersion, version.DdevVersion)
 
 	// Values defined in file, we should have values from file
 	assert.Equal(app.Type, "drupal8")
+	assert.Equal(app.Docroot, "test")
 	assert.Equal(app.WebImage, "test/testimage:latest")
 }
 
@@ -371,7 +369,7 @@ func TestWriteConfig(t *testing.T) {
 	app := &DdevApp{
 		ConfigPath: filepath.Join(testDir, "config.yaml"),
 		AppRoot:    testDir,
-		APIVersion: CurrentAppVersion,
+		APIVersion: version.DdevVersion,
 		Name:       "TestWrite",
 		WebImage:   version.WebImg + ":" + version.WebTag,
 		DBImage:    version.DBImg + ":" + version.DBTag,

pkg/ddevapp/ddevapp.go

@@ -56,9 +56,9 @@ type DdevApp struct {
 	Type                  string               `yaml:"type"`
 	Docroot               string               `yaml:"docroot"`
 	PHPVersion            string               `yaml:"php_version"`
-	WebImage              string               `yaml:"webimage"`
-	DBImage               string               `yaml:"dbimage"`
-	DBAImage              string               `yaml:"dbaimage"`
+	WebImage              string               `yaml:"webimage,omitempty"`
+	DBImage               string               `yaml:"dbimage,omitempty"`
+	DBAImage              string               `yaml:"dbaimage,omitempty"`
 	RouterHTTPPort        string               `yaml:"router_http_port"`
 	RouterHTTPSPort       string               `yaml:"router_https_port"`
 	XdebugEnabled         bool                 `yaml:"xdebug_enabled"`

pkg/ddevapp/templates.go

@@ -109,8 +109,9 @@ const ConfigInstructions = `
 
 # php_version: "7.1"  # PHP version to use, "5.6", "7.0", "7.1", "7.2"
 
-# You can delete the webimage, dbimage, dbaimage lines to use the defaults
-# in ddev.
+# You can explicitly specify the webimage, dbimage, dbaimage lines but this 
+# is not recommended, as the images are often closely tied to ddev's' behavior,
+# so this can break upgrades.
 
 # webimage: <docker_image>  # nginx/php docker image. 
 # dbimage: <docker_image>  # mariadb docker image. 
@@ -119,6 +120,8 @@ const ConfigInstructions = `
 # router_http_port: <port>  # Port to be used for http (defaults to port 80)
 # router_https_port: <port> # Port for https (defaults to 443)
 
+# xdebug_enabled: false  # Set to true to enable xdebug and "ddev start" or "ddev restart"
+
 #additional_hostnames:
 # - somename
 # - someothername

pkg/ddevapp/testdata/config.yaml

@@ -1,4 +1,3 @@
-APIVersion: "1"
 type: drupal8
 docroot: test
 webimage: test/testimage:latest

pkg/version/version.go

@@ -6,7 +6,7 @@ var VERSION = ""
 // IMPORTANT: These versions are overridden by version ldflags specifications VERSION_VARIABLES in the Makefile
 
 // DdevVersion is the current version of ddev, by default the git committish (should be current git tag)
-var DdevVersion = "v0.3.0-dev" // Note that this is overridden by make
+var DdevVersion = "v0.0.0-overridden-by-make" // Note that this is overridden by make
 
 // DockerVersionConstraint is the current minimum version of docker required for ddev.
 // See https://godoc.org/github.com/Masterminds/semver#hdr-Checking_Version_Constraints