Validate config before writing .yaml or starting project, resolves #1095

 (#1139)

cmd/ddev/cmd/config.go

@@ -55,7 +55,7 @@ var (
 	webserverTypeArg string
 )
 
-var providerName = ddevapp.DefaultProviderName
+var providerName = ddevapp.ProviderDefault
 
 // extraFlagsHandlingFunc does specific handling for additional flags, and is different per provider.
 var extraFlagsHandlingFunc func(cmd *cobra.Command, args []string, app *ddevapp.DdevApp) error
@@ -297,15 +297,12 @@ func handleMainConfigArgs(cmd *cobra.Command, args []string, app *ddevapp.DdevAp
 		app.WebserverType = webserverTypeArg
 	}
 
-	provider, err := app.GetProvider()
-	util.CheckErr(err)
-	err = provider.ValidateField("Name", app.Name)
-	if err != nil {
-		return fmt.Errorf("failed to validate configuration: %v", err)
+	// Ensure the configuration passes validation before writing config file.
+	if err := app.ValidateConfig(); err != nil {
+		return fmt.Errorf("failed to validate config: %v", err)
 	}
 
-	err = app.WriteConfig()
-	if err != nil {
+	if err := app.WriteConfig(); err != nil {
 		return fmt.Errorf("could not write ddev config file %s: %v", app.ConfigPath, err)
 	}
 

cmd/ddev/cmd/config_druds3.go

@@ -2,6 +2,7 @@ package cmd
 
 import (
 	"fmt"
+
 	"github.com/drud/ddev/pkg/ddevapp"
 	"github.com/spf13/cobra"
 )
@@ -20,7 +21,7 @@ var drudS3ConfigCommand *cobra.Command = &cobra.Command{
 	Short:   "Create or modify a ddev project drud-s3 configuration in the current directory",
 	Example: `"ddev config drud-s3" or "ddev config drud-s3 --access-key-id=AKIAISOMETHINGMAGIC --secret-access-key=rweeMAGICSECRET --docroot=. --projectname=d7-kickstart --projecttype=drupal7 --bucket-name=my_aws_s3_bucket --environment=production"`,
 	PreRun: func(cmd *cobra.Command, args []string) {
-		providerName = "drud-s3"
+		providerName = ddevapp.ProviderDrudS3
 		extraFlagsHandlingFunc = handleDrudS3Flags
 	},
 	Run: handleConfigRun,

cmd/ddev/cmd/config_pantheon.go

@@ -2,6 +2,7 @@ package cmd
 
 import (
 	"fmt"
+
 	"github.com/drud/ddev/pkg/ddevapp"
 	"github.com/spf13/cobra"
 )
@@ -15,7 +16,7 @@ var pantheonConfigCommand *cobra.Command = &cobra.Command{
 	Short:   "Create or modify a ddev project pantheon configuration in the current directory",
 	Example: `"ddev config pantheon" or "ddev config pantheon --docroot=. --projectname=myproject --pantheon-environment=dev"`,
 	PreRun: func(cmd *cobra.Command, args []string) {
-		providerName = "pantheon"
+		providerName = ddevapp.ProviderPantheon
 		extraFlagsHandlingFunc = handlePantheonFlags
 	},
 	Run: handleConfigRun,

cmd/ddev/cmd/config_test.go

@@ -104,8 +104,8 @@ func TestConfigSetValues(t *testing.T) {
 
 	// Build config args
 	projectName := "my-project-name"
-	projectType := "typo3"
-	phpVersion := "7.1"
+	projectType := ddevapp.AppTypeTYPO3
+	phpVersion := ddevapp.PHP71
 	httpPort := "81"
 	httpsPort := "444"
 	xdebugEnabled := true
@@ -114,7 +114,7 @@ func TestConfigSetValues(t *testing.T) {
 	additionalFQDNsSlice := []string{"abc.com", "123.pizza", "xyz.co.uk"}
 	additionalFQDNs := strings.Join(additionalFQDNsSlice, ",")
 	uploadDir := filepath.Join("custom", "config", "path")
-	webserverType := "apache-fpm"
+	webserverType := ddevapp.WebserverApacheFPM
 
 	args := []string{
 		"config",

cmd/ddev/cmd/root_test.go

@@ -29,7 +29,7 @@ var (
 		DBTarURL:                      "https://github.com/drud/wordpress/releases/download/v0.4.0/db.tar.gz",
 		HTTPProbeURI:                  "wp-admin/setup-config.php",
 		Docroot:                       "htdocs",
-		Type:                          "wordpress",
+		Type:                          ddevapp.AppTypeWordPress,
 		Safe200URIWithExpectation:     testcommon.URIWithExpect{URI: "/readme.html", Expect: "Welcome. WordPress is a very special project to me."},
 	},
 	}

pkg/ddevapp/apptypes.go

@@ -59,46 +59,28 @@ var appTypeMatrix map[string]AppTypeFuncs
 
 func init() {
 	appTypeMatrix = map[string]AppTypeFuncs{
-		"php": {},
-		"drupal6": {
+		AppTypePHP: {},
+		AppTypeDrupal6: {
 			settingsCreator: createDrupal6SettingsFile, uploadDir: getDrupalUploadDir, hookDefaultComments: getDrupal6Hooks, apptypeSettingsPaths: setDrupalSiteSettingsPaths, appTypeDetect: isDrupal6App, postImportDBAction: nil, configOverrideAction: drupal6ConfigOverrideAction, postConfigAction: nil, postStartAction: drupal6PostStartAction, importFilesAction: drupalImportFilesAction,
 		},
-		"drupal7": {
+		AppTypeDrupal7: {
 			settingsCreator: createDrupal7SettingsFile, uploadDir: getDrupalUploadDir, hookDefaultComments: getDrupal7Hooks, apptypeSettingsPaths: setDrupalSiteSettingsPaths, appTypeDetect: isDrupal7App, postImportDBAction: nil, configOverrideAction: drupal7ConfigOverrideAction, postConfigAction: nil, postStartAction: drupal7PostStartAction, importFilesAction: drupalImportFilesAction,
 		},
-		"drupal8": {
+		AppTypeDrupal8: {
 			settingsCreator: createDrupal8SettingsFile, uploadDir: getDrupalUploadDir, hookDefaultComments: getDrupal8Hooks, apptypeSettingsPaths: setDrupalSiteSettingsPaths, appTypeDetect: isDrupal8App, postImportDBAction: nil, configOverrideAction: nil, postConfigAction: nil, postStartAction: drupal8PostStartAction, importFilesAction: drupalImportFilesAction,
 		},
-		"wordpress": {
+		AppTypeWordPress: {
 			settingsCreator: createWordpressSettingsFile, uploadDir: getWordpressUploadDir, hookDefaultComments: getWordpressHooks, apptypeSettingsPaths: setWordpressSiteSettingsPaths, appTypeDetect: isWordpressApp, postImportDBAction: nil, configOverrideAction: nil, postConfigAction: nil, postStartAction: nil, importFilesAction: wordpressImportFilesAction,
 		},
-		"typo3": {
+		AppTypeTYPO3: {
 			settingsCreator: createTypo3SettingsFile, uploadDir: getTypo3UploadDir, hookDefaultComments: getTypo3Hooks, apptypeSettingsPaths: setTypo3SiteSettingsPaths, appTypeDetect: isTypo3App, postImportDBAction: nil, configOverrideAction: typo3ConfigOverrideAction, postConfigAction: nil, postStartAction: nil, importFilesAction: typo3ImportFilesAction,
 		},
-		"backdrop": {
+		AppTypeBackdrop: {
 			settingsCreator: createBackdropSettingsFile, uploadDir: getBackdropUploadDir, hookDefaultComments: getBackdropHooks, apptypeSettingsPaths: setBackdropSiteSettingsPaths, appTypeDetect: isBackdropApp, postImportDBAction: backdropPostImportDBAction, configOverrideAction: nil, postConfigAction: nil, postStartAction: backdropPostStartAction, importFilesAction: backdropImportFilesAction,
 		},
 	}
 }
 
-// GetValidAppTypes returns the valid apptype keys from the appTypeMatrix
-func GetValidAppTypes() []string {
-	keys := make([]string, 0, len(appTypeMatrix))
-	for k := range appTypeMatrix {
-		keys = append(keys, k)
-	}
-	return keys
-}
-
-// IsValidAppType checks to see if the given apptype string is a valid configured
-// apptype.
-func IsValidAppType(apptype string) bool {
-	if _, ok := appTypeMatrix[apptype]; ok {
-		return true
-	}
-	return false
-}
-
 // CreateSettingsFile creates the settings file (like settings.php) for the
 // provided app is the apptype has a settingsCreator function.
 func (app *DdevApp) CreateSettingsFile() (string, error) {
@@ -181,7 +163,7 @@ func (app *DdevApp) DetectAppType() string {
 			return appName
 		}
 	}
-	return "php"
+	return AppTypePHP
 }
 
 // PostImportDBAction calls each apptype's detector until it finds a match,

pkg/ddevapp/apptypes_test.go

@@ -21,11 +21,11 @@ func TestApptypeDetection(t *testing.T) {
 	assert := asrt.New(t)
 
 	fileLocations := map[string]string{
-		"drupal6":   "misc/ahah.js",
-		"drupal7":   "misc/ajax.js",
-		"drupal8":   "core/scripts/drupal.sh",
-		"wordpress": "wp-settings.php",
-		"backdrop":  "core/scripts/backdrop.sh",
+		ddevapp.AppTypeDrupal6:   "misc/ahah.js",
+		ddevapp.AppTypeDrupal7:   "misc/ajax.js",
+		ddevapp.AppTypeDrupal8:   "core/scripts/drupal.sh",
+		ddevapp.AppTypeWordPress: "wp-settings.php",
+		ddevapp.AppTypeBackdrop:  "core/scripts/backdrop.sh",
 	}
 
 	for expectedType, expectedPath := range fileLocations {
@@ -41,7 +41,7 @@ func TestApptypeDetection(t *testing.T) {
 		_, err = os.OpenFile(filepath.Join(testDir, expectedPath), os.O_RDONLY|os.O_CREATE, 0666)
 		assert.NoError(err)
 
-		app, err := ddevapp.NewApp(testDir, ddevapp.DefaultProviderName)
+		app, err := ddevapp.NewApp(testDir, ddevapp.ProviderDefault)
 		assert.NoError(err)
 
 		foundType := app.DetectAppType()
@@ -55,11 +55,11 @@ func TestPostConfigAction(t *testing.T) {
 	assert := asrt.New(t)
 
 	appTypes := map[string]string{
-		"drupal6":   "5.6",
-		"drupal7":   "7.1",
-		"drupal8":   ddevapp.DdevDefaultPHPVersion,
-		"wordpress": ddevapp.DdevDefaultPHPVersion,
-		"backdrop":  ddevapp.DdevDefaultPHPVersion,
+		ddevapp.AppTypeDrupal6:   ddevapp.PHP56,
+		ddevapp.AppTypeDrupal7:   ddevapp.PHP71,
+		ddevapp.AppTypeDrupal8:   ddevapp.PHPDefault,
+		ddevapp.AppTypeWordPress: ddevapp.PHPDefault,
+		ddevapp.AppTypeBackdrop:  ddevapp.PHPDefault,
 	}
 
 	for appType, expectedPHPVersion := range appTypes {
@@ -69,7 +69,7 @@ func TestPostConfigAction(t *testing.T) {
 		defer testcommon.CleanupDir(testDir)
 		defer testcommon.Chdir(testDir)()
 
-		app, err := ddevapp.NewApp(testDir, ddevapp.DefaultProviderName)
+		app, err := ddevapp.NewApp(testDir, ddevapp.ProviderDefault)
 		assert.NoError(err)
 
 		// Prompt for apptype as a way to get it into the config.

pkg/ddevapp/config.go

@@ -23,22 +23,6 @@ import (
 	yaml "gopkg.in/yaml.v2"
 )
 
-// DefaultProviderName contains the name of the default provider which will be used if one is not otherwise specified.
-const DefaultProviderName = "default"
-
-// DdevDefaultPHPVersion is the default PHP version, overridden by $DDEV_PHP_VERSION
-const DdevDefaultPHPVersion = "7.1"
-
-// DdevDefaultWebserverType is the default webserver type, as nginx-fpm/apache-fpm/apache-cgi,
-// overridden by $DDEV_WEBSERVER_TYPE
-var DdevDefaultWebserverType = "nginx-fpm"
-
-// DdevDefaultRouterHTTPPort is the starting router port, 80
-const DdevDefaultRouterHTTPPort = "80"
-
-// DdevDefaultRouterHTTPSPort is the starting https router port, 443
-const DdevDefaultRouterHTTPSPort = "443"
-
 // 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])$`)
 
@@ -63,7 +47,7 @@ type Provider interface {
 func init() {
 	// This is for automated testing only. It allows us to override the webserver type.
 	if testWebServerType := os.Getenv("DDEV_TEST_WEBSERVER_TYPE"); testWebServerType != "" {
-		DdevDefaultWebserverType = testWebServerType
+		WebserverDefault = testWebServerType
 	}
 }
 
@@ -75,8 +59,8 @@ func NewApp(AppRoot string, provider string) (*DdevApp, error) {
 	app.AppRoot = AppRoot
 	app.ConfigPath = app.GetConfigPath("config.yaml")
 	app.APIVersion = version.DdevVersion
-	app.PHPVersion = DdevDefaultPHPVersion
-	app.WebserverType = DdevDefaultWebserverType
+	app.PHPVersion = PHPDefault
+	app.WebserverType = WebserverDefault
 	app.RouterHTTPPort = DdevDefaultRouterHTTPPort
 	app.RouterHTTPSPort = DdevDefaultRouterHTTPSPort
 
@@ -98,12 +82,12 @@ func NewApp(AppRoot string, provider string) (*DdevApp, error) {
 	// Otherwise we accept whatever might have been in config file if there was anything.
 	if provider == "" && app.Provider != "" {
 		// Do nothing. This is the case where the config has a provider and no override is provided. Config wins.
-	} else if provider == "pantheon" || provider == "drud-s3" || provider == DefaultProviderName {
+	} else if provider == ProviderPantheon || provider == ProviderDrudS3 || provider == ProviderDefault {
 		app.Provider = provider // Use the provider passed-in. Function argument wins.
 	} else if provider == "" && app.Provider == "" {
-		app.Provider = DefaultProviderName // Nothing passed in, nothing configured. Set c.Provider to default
+		app.Provider = ProviderDefault // Nothing passed in, nothing configured. Set c.Provider to default
 	} else {
-		return app, fmt.Errorf("Provider '%s' is not implemented", provider)
+		return app, fmt.Errorf("provider '%s' is not implemented", provider)
 	}
 
 	return app, nil
@@ -194,11 +178,11 @@ func (app *DdevApp) ReadConfig() error {
 		app.Name = filepath.Base(app.AppRoot)
 	}
 	if app.PHPVersion == "" {
-		app.PHPVersion = DdevDefaultPHPVersion
+		app.PHPVersion = PHPDefault
 	}
 
 	if app.WebserverType == "" {
-		app.WebserverType = DdevDefaultWebserverType
+		app.WebserverType = WebserverDefault
 	}
 
 	if app.RouterHTTPPort == "" {
@@ -273,20 +257,36 @@ func (app *DdevApp) PromptForConfig() error {
 
 // ValidateConfig ensures the configuration meets ddev's requirements.
 func (app *DdevApp) ValidateConfig() error {
-	if _, err := os.Stat(app.ConfigPath); os.IsNotExist(err) {
-		return fmt.Errorf("no valid project config.yaml was found at %s", app.ConfigPath)
+	provider, err := app.GetProvider()
+	if err != nil {
+		return err.(invalidProvider)
 	}
 
-	// validate hostname
-	match := hostRegex.MatchString(app.GetHostname())
-	if !match {
-		return fmt.Errorf("%s is not a valid hostname. Please enter a project name in your configuration that will allow for a valid hostname. See https://en.wikipedia.org/wiki/Hostname#Restrictions_on_valid_hostnames for valid hostname requirements", app.GetHostname())
+	// validate project name
+	if err = provider.ValidateField("Name", app.Name); err != nil {
+		return err.(invalidAppName)
+	}
+
+	// validate hostnames
+	for _, hn := range app.GetHostnames() {
+		if !hostRegex.MatchString(hn) {
+			return fmt.Errorf("invalid hostname: %s. See https://en.wikipedia.org/wiki/Hostname#Restrictions_on_valid_hostnames for valid hostname requirements", hn).(invalidHostname)
+		}
 	}
 
 	// validate apptype
-	match = IsValidAppType(app.Type)
-	if !match {
-		return fmt.Errorf("'%s' is not a valid apptype", app.Type)
+	if !IsValidAppType(app.Type) {
+		return fmt.Errorf("invalid app type: %s", app.Type).(invalidAppType)
+	}
+
+	// validate PHP version
+	if !IsValidPHPVersion(app.PHPVersion) {
+		return fmt.Errorf("invalid PHP version: %s, must be one of %v", app.PHPVersion, GetValidPHPVersions()).(invalidPHPVersion)
+	}
+
+	// validate webserver type
+	if !IsValidWebserverType(app.WebserverType) {
+		return fmt.Errorf("invalid webserver type: %s, must be one of %s", app.WebserverType, GetValidWebserverTypes()).(invalidWebserverType)
 	}
 
 	return nil
@@ -372,12 +372,12 @@ func (app *DdevApp) CheckCustomConfig() {
 	ddevDir := filepath.Dir(app.ConfigPath)
 
 	customConfig := false
-	if _, err := os.Stat(filepath.Join(ddevDir, "nginx-site.conf")); err == nil && app.WebserverType == "nginx-fpm" {
+	if _, err := os.Stat(filepath.Join(ddevDir, "nginx-site.conf")); err == nil && app.WebserverType == WebserverNginxFPM {
 		util.Warning("Using custom nginx configuration in nginx-site.conf")
 		customConfig = true
 	}
 
-	if _, err := os.Stat(filepath.Join(ddevDir, "apache", "apache-site.conf")); err == nil && app.WebserverType != "nginx-fpm" {
+	if _, err := os.Stat(filepath.Join(ddevDir, "apache", "apache-site.conf")); err == nil && app.WebserverType != WebserverNginxFPM {
 		util.Warning("Using custom apache configuration in apache/apache-site.conf")
 		customConfig = true
 	}