ddev rm, describe, and stop are now reliable even when a site directo…

…ry is missing; fixes #459 (#460)

Thanks @nmccrory  for all your painstaking work on this. Sorry it took so long to get it in.

cmd/ddev/cmd/describe_test.go

@@ -26,7 +26,7 @@ func TestDescribeBadArgs(t *testing.T) {
 	args := []string{"describe"}
 	out, err := exec.RunCommand(DdevBin, args)
 	assert.Error(err)
-	assert.Contains(string(out), "unable to determine the application for this command")
+	assert.Contains(string(out), "Please specify a site name or change directories")
 
 	// Ensure we get a failure if we run a describe on a named application which does not exist.
 	args = []string{"describe", util.RandString(16)}

cmd/ddev/cmd/logs_test.go

@@ -27,7 +27,7 @@ func TestDevLogsBadArgs(t *testing.T) {
 	args := []string{"logs"}
 	out, err := exec.RunCommand(DdevBin, args)
 	assert.Error(err)
-	assert.Contains(string(out), "unable to determine the application for this command")
+	assert.Contains(string(out), "Please specify a site name or change directories")
 }
 
 // TestDevLogs tests that the Dev logs functionality is working.

cmd/ddev/cmd/remove.go

@@ -15,7 +15,7 @@ var LocalDevRMCmd = &cobra.Command{
 	Short:   "Remove the local development environment for a site.",
 	Long: `Remove the local development environment for a site. You can run 'ddev remove'
 from a site directory to remove that site, or you can specify a site to remove
-by running 'ddev rm <sitename>'. By default, remove is a non-destructive operation and will
+by running 'ddev remove <sitename>'. By default, remove is a non-destructive operation and will
 leave database contents intact.
 
 To remove database contents, you may use the --remove-data flag with remove.`,

cmd/ddev/cmd/root_test.go

@@ -72,7 +72,7 @@ func TestGetActiveAppRoot(t *testing.T) {
 	assert := asrt.New(t)
 
 	_, err := platform.GetActiveAppRoot("")
-	assert.Contains(err.Error(), "unable to determine the application for this command")
+	assert.Contains(err.Error(), "Please specify a site name or change directories")
 
 	_, err = platform.GetActiveAppRoot("potato")
 	assert.Error(err)

pkg/plugins/platform/local.go

@@ -42,7 +42,8 @@ func (l *LocalApp) GetType() string {
 	return strings.ToLower(l.AppConfig.AppType)
 }
 
-// Init populates LocalApp settings based on the current working directory.
+// Init populates LocalApp config based on the current working directory.
+// It does not start the containers.
 func (l *LocalApp) Init(basePath string) error {
 	config, err := ddevapp.NewConfig(basePath, "")
 
@@ -674,6 +675,10 @@ func (l *LocalApp) Stop() error {
 		return fmt.Errorf("no site to remove")
 	}
 
+	if strings.Contains(l.SiteStatus(), SiteDirMissing) || strings.Contains(l.SiteStatus(), SiteConfigMissing) {
+		return fmt.Errorf("ddev can no longer find your application files at %s. If you would like to continue using ddev to manage this site please restore your files to that directory. If you would like to remove this site from ddev, you may run 'ddev remove %s'", l.AppRoot(), l.GetName())
+	}
+
 	_, _, err := dockerutil.ComposeCmd(l.ComposeFiles(), "stop")
 
 	if err != nil {
@@ -801,20 +806,18 @@ func (l *LocalApp) CreateSettingsFile() error {
 	return nil
 }
 
-// Down stops the docker containers for the local project.
+// Down stops the docker containers for the project in current directory.
 func (l *LocalApp) Down(removeData bool) error {
 	l.DockerEnv()
 	settingsFilePath := l.AppConfig.SiteSettingsPath
 
-	_, _, err := dockerutil.ComposeCmd(l.ComposeFiles(), "down", "-v")
+	// Remove all the containers and volumes for app.
+	err := Cleanup(l)
 	if err != nil {
-		util.Warning("Could not stop site with docker-compose. Attempting manual cleanup.")
-		err = Cleanup(l)
-		if err != nil {
-			util.Warning("Received error from Cleanup, err=", err)
-		}
+		return fmt.Errorf("Failed to remove %s: %s", l.GetName(), err)
 	}
 
+	// Remove data/database if we need to.
 	if removeData {
 		if fileutil.FileExists(settingsFilePath) {
 			signatureFound, err := fileutil.FgrepStringInFile(settingsFilePath, model.DdevSettingsFileSignature)
@@ -830,26 +833,31 @@ func (l *LocalApp) Down(removeData bool) error {
 				}
 			}
 		}
-		dir := filepath.Dir(l.AppConfig.DataDir)
+		// Check that l.AppConfig.DataDir is a directory that is safe to remove.
+		err = validateDataDirRemoval(l.AppConfig)
+		if err != nil {
+			return fmt.Errorf("failed to remove data directories: %v", err)
+		}
 		// mysql data can be set to read-only on linux hosts. PurgeDirectory ensures files
 		// are writable before we attempt to remove them.
-		if !fileutil.FileExists(dir) {
+		if !fileutil.FileExists(l.AppConfig.DataDir) {
 			util.Warning("No application data to remove")
 		} else {
-			err := fileutil.PurgeDirectory(dir)
+			err := fileutil.PurgeDirectory(l.AppConfig.DataDir)
 			if err != nil {
 				return fmt.Errorf("failed to remove data directories: %v", err)
 			}
 			// PurgeDirectory leaves the directory itself in place, so we remove it here.
-			err = os.RemoveAll(dir)
+			err = os.RemoveAll(l.AppConfig.DataDir)
 			if err != nil {
-				return fmt.Errorf("failed to remove data directories: %v", err)
+				return fmt.Errorf("failed to remove data directory %s: %v", l.AppConfig.DataDir, err)
 			}
 			util.Success("Application data removed")
 		}
 	}
 
-	return StopRouter()
+	err = StopRouter()
+	return err
 }
 
 // URL returns the URL for a given application.
@@ -935,6 +943,10 @@ func GetActiveAppRoot(siteName string) (string, error) {
 		if err != nil {
 			return "", fmt.Errorf("error determining the current directory: %s", err)
 		}
+		_, err = CheckForConf(siteDir)
+		if err != nil {
+			return "", fmt.Errorf("Could not find a site in %s. Have you run 'ddev config'? Please specify a site name or change directories: %s", siteDir, err)
+		}
 	} else {
 		var ok bool
 
@@ -953,10 +965,10 @@ func GetActiveAppRoot(siteName string) (string, error) {
 			return "", fmt.Errorf("could not determine the location of %s from container: %s", siteName, dockerutil.ContainerName(webContainer))
 		}
 	}
-
 	appRoot, err := CheckForConf(siteDir)
 	if err != nil {
-		return "", fmt.Errorf("unable to determine the application for this command. Have you run 'ddev config'? Error: %s", err)
+		// In the case of a missing .ddev/config.yml just return the site directory.
+		return siteDir, nil
 	}
 
 	return appRoot, nil
@@ -974,6 +986,60 @@ func GetActiveApp(siteName string) (App, error) {
 		return app, err
 	}
 
-	err = app.Init(activeAppRoot)
-	return app, err
+	// Ignore app.Init() error, since app.Init() fails if no directory found.
+	// We already were successful with *finding* the app, and if we get an
+	// incomplete one we have to add to it.
+	_ = app.Init(activeAppRoot)
+	// Check to see if there are any missing AppConfig values that still need to be restored.
+	localApp, _ := app.(*LocalApp)
+
+	if localApp.AppConfig.Name == "" || localApp.AppConfig.DataDir == "" {
+		err = restoreApp(app, siteName)
+		if err != nil {
+			return app, err
+		}
+	}
+
+	return app, nil
+}
+
+// restoreApp recreates an AppConfig's Name and/or DataDir and returns an error
+// if it cannot restore them.
+func restoreApp(app App, siteName string) error {
+	localApp, _ := app.(*LocalApp)
+	if siteName == "" {
+		return fmt.Errorf("error restoring AppConfig: no siteName given")
+	}
+	localApp.AppConfig.Name = siteName
+	// Ensure that AppConfig.DataDir is set so that site data can be removed if necessary.
+	dataDir := fmt.Sprintf("%s/%s", util.GetGlobalDdevDir(), localApp.AppConfig.Name)
+	localApp.AppConfig.DataDir = dataDir
+
+	return nil
+}
+
+// validateDataDirRemoval validates that dataDir is a safe filepath to be removed by ddev.
+func validateDataDirRemoval(config *ddevapp.Config) error {
+	dataDir := config.DataDir
+	unsafeFilePathErr := fmt.Errorf("filepath: %s unsafe for removal", dataDir)
+	// Check for an empty filepath
+	if dataDir == "" {
+		return unsafeFilePathErr
+	}
+	// Get the current working directory.
+	currDir, err := os.Getwd()
+	if err != nil {
+		return err
+	}
+	// Check that dataDir is not the current directory.
+	if dataDir == currDir {
+		return unsafeFilePathErr
+	}
+	// Get the last element of dataDir and use it to check that there is something after GlobalDdevDir.
+	lastPathElem := filepath.Base(dataDir)
+	nextLastPathElem := filepath.Base(filepath.Dir(dataDir))
+	if lastPathElem == ".ddev" || nextLastPathElem != config.Name || lastPathElem == "" {
+		return unsafeFilePathErr
+	}
+	return nil
 }

pkg/plugins/platform/local_test.go

@@ -2,12 +2,12 @@ package platform_test
 
 import (
 	"fmt"
+	"os"
 	"path/filepath"
+	"runtime"
 	"testing"
 	"time"
 
-	"os"
-
 	"strings"
 
 	"github.com/drud/ddev/pkg/ddevapp"
@@ -123,9 +123,11 @@ func TestMain(m *testing.M) {
 			log.Fatalf("TestMain shutdown: app.Init() failed on site %s in dir %s, err=%v", TestSites[i].Name, TestSites[i].Dir, err)
 		}
 
-		err = app.Down(true)
-		if err != nil {
-			log.Fatalf("TestMain startup: app.Down() failed on site %s, err=%v", TestSites[i].Name, err)
+		if app.SiteStatus() != platform.SiteNotFound {
+			err = app.Down(true)
+			if err != nil {
+				log.Fatalf("TestMain shutdown: app.Down() failed on site %s, err=%v", TestSites[i].Name, err)
+			}
 		}
 
 		runTime()
@@ -196,7 +198,7 @@ func TestStartWithoutDdevConfig(t *testing.T) {
 
 	_, err = platform.GetActiveApp("")
 	assert.Error(err)
-	assert.Contains(err.Error(), "unable to determine")
+	assert.Contains(err.Error(), "Could not find a site")
 }
 
 // TestGetApps tests the GetApps function to ensure it accurately returns a list of running applications.
@@ -472,7 +474,41 @@ func TestLocalStop(t *testing.T) {
 	}
 }
 
-// TestDescribe tests that the describe command works properly on a started or stopped site.
+// TestLocalStopMissingDirectory tests that the 'ddev stop' command works properly on sites with missing directories or ddev configs.
+func TestLocalStopMissingDirectory(t *testing.T) {
+	assert := asrt.New(t)
+
+	site := TestSites[0]
+	testcommon.ClearDockerEnv()
+	app, err := platform.GetPluginApp("local")
+	assert.NoError(err)
+	err = app.Init(site.Dir)
+	assert.NoError(err)
+
+	// Restart the site since it was stopped in the previous test.
+	if app.SiteStatus() != platform.SiteRunning {
+		err = app.Start()
+		assert.NoError(err)
+	}
+
+	tempPath := testcommon.CreateTmpDir("site-copy")
+	siteCopyDest := filepath.Join(tempPath, "site")
+	defer removeAllErrCheck(tempPath, assert)
+
+	// Move the site directory to a temp location to mimic a missing directory.
+	err = os.Rename(site.Dir, siteCopyDest)
+	assert.NoError(err)
+
+	out := app.Stop()
+	assert.Error(out)
+	assert.Contains(out.Error(), "If you would like to continue using ddev to manage this site please restore your files to that directory.")
+	// Move the site directory back to its original location.
+	err = os.Rename(siteCopyDest, site.Dir)
+	assert.NoError(err)
+
+}
+
+// TestDescribe tests that the describe command works properly on a stopped site.
 func TestDescribe(t *testing.T) {
 	assert := asrt.New(t)
 	app, err := platform.GetPluginApp("local")
@@ -505,6 +541,31 @@ func TestDescribe(t *testing.T) {
 	}
 }
 
+// TestDescribeMissingDirectory tests that the describe command works properly on sites with missing directories or ddev configs.
+func TestDescribeMissingDirectory(t *testing.T) {
+	assert := asrt.New(t)
+	site := TestSites[0]
+	tempPath := testcommon.CreateTmpDir("site-copy")
+	siteCopyDest := filepath.Join(tempPath, "site")
+	defer removeAllErrCheck(tempPath, assert)
+
+	app, err := platform.GetPluginApp("local")
+	assert.NoError(err)
+	err = app.Init(site.Dir)
+	assert.NoError(err)
+
+	// Move the site directory to a temp location to mimick a missing directory.
+	err = os.Rename(site.Dir, siteCopyDest)
+	assert.NoError(err)
+
+	desc, err := app.Describe()
+	assert.NoError(err)
+	assert.Contains(desc["status"], platform.SiteDirMissing, "Status did not include the phrase 'app directory missing' when describing a site with missing directories.")
+	// Move the site directory back to its original location.
+	err = os.Rename(siteCopyDest, site.Dir)
+	assert.NoError(err)
+}
+
 // TestRouterPortsCheck makes sure that we can detect if the ports are available before starting the router.
 func TestRouterPortsCheck(t *testing.T) {
 	assert := asrt.New(t)
@@ -565,6 +626,12 @@ func TestRouterPortsCheck(t *testing.T) {
 // TestCleanupWithoutCompose ensures app containers can be properly cleaned up without a docker-compose config file present.
 func TestCleanupWithoutCompose(t *testing.T) {
 	assert := asrt.New(t)
+	// Skip test because we can't rename folders while they're in use if running on Windows.
+	if runtime.GOOS == "windows" {
+		log.Println("Skipping test TestCleanupWithoutCompose on Windows")
+		t.Skip()
+	}
+
 	site := TestSites[0]
 
 	revertDir := site.Chdir()
@@ -578,9 +645,20 @@ func TestCleanupWithoutCompose(t *testing.T) {
 	// Ensure we have a site started so we have something to cleanup
 	err = app.Start()
 	assert.NoError(err)
+	// Setup by creating temp directory and nesting a folder for our site.
+	tempPath := testcommon.CreateTmpDir("site-copy")
+	siteCopyDest := filepath.Join(tempPath, "site")
+	defer removeAllErrCheck(tempPath, assert)
 
-	// Call the Cleanup command()
-	err = platform.Cleanup(app)
+	// Move site directory to a temp directory to mimick a missing directory.
+	err = os.Rename(site.Dir, siteCopyDest)
+	assert.NoError(err)
+
+	// Call the Down command()
+	// Notice that we set the removeData parameter to true.
+	// This gives us added test coverage over sites with missing directories
+	// by ensuring any associated database files get cleaned up as well.
+	err = app.Down(true)
 	assert.NoError(err)
 
 	for _, containerType := range [3]string{"web", "db", "dba"} {
@@ -603,6 +681,9 @@ func TestCleanupWithoutCompose(t *testing.T) {
 	assert.NoError(err)
 
 	revertDir()
+	// Move the site directory back to its original location.
+	err = os.Rename(siteCopyDest, site.Dir)
+	assert.NoError(err)
 }
 
 // TestGetappsEmpty ensures that GetApps returns an empty list when no applications are running.
@@ -620,9 +701,10 @@ func TestGetAppsEmpty(t *testing.T) {
 		err = app.Init(site.Dir)
 		assert.NoError(err)
 
-		err = app.Down(true)
-		assert.NoError(err)
-
+		if app.SiteStatus() != platform.SiteNotFound {
+			err = app.Down(true)
+			assert.NoError(err)
+		}
 		switchDir()
 	}
 
@@ -719,3 +801,8 @@ func constructContainerName(containerType string, app platform.App) (string, err
 	name := dockerutil.ContainerName(container)
 	return name, nil
 }
+
+func removeAllErrCheck(path string, assert *asrt.Assertions) {
+	err := os.RemoveAll(path)
+	assert.NoError(err)
+}

pkg/plugins/platform/types.go

@@ -17,7 +17,7 @@ const SiteNotFound = "not found"
 const SiteDirMissing = "app directory missing"
 
 // SiteConfigMissing defines the string used to denote when a site is missing its .ddev/config.yml file.
-const SiteConfigMissing = ".ddev/config.yml missing"
+const SiteConfigMissing = ".ddev/config.yaml missing"
 
 // SiteStopped defines the string used to denote when a site is in the stopped state.
 const SiteStopped = "stopped"