Merge pull request #4270 from dokku/4267-fix-app-name-verification

Allow renaming old applications to new format

plugins/apps/functions.go

@@ -83,6 +83,17 @@ func destroyApp(appName string) error {
 
 	forceCleanup := true
 	common.DockerCleanup(appName, forceCleanup)
+
+	// remove contents for apps that are symlinks to other folders
+	if err := os.RemoveAll(fmt.Sprintf("%v/", common.AppRoot(appName))); err != nil {
+		common.LogWarn(err.Error())
+	}
+
+	// then remove the folder and/or the symlink
+	if err := os.RemoveAll(common.AppRoot(appName)); err != nil {
+		common.LogWarn(err.Error())
+	}
+
 	return nil
 }
 

plugins/apps/subcommands.go

@@ -18,7 +18,7 @@ func CommandClone(oldAppName string, newAppName string, skipDeploy bool, ignoreE
 		return errors.New("Please specify an new app name")
 	}
 
-	if err := common.IsValidAppName(oldAppName); err != nil {
+	if err := common.VerifyAppName(oldAppName); err != nil {
 		return err
 	}
 
@@ -26,10 +26,6 @@ func CommandClone(oldAppName string, newAppName string, skipDeploy bool, ignoreE
 		return err
 	}
 
-	if err := appExists(oldAppName); err != nil {
-		return err
-	}
-
 	if err := appExists(newAppName); err == nil {
 		if ignoreExisting {
 			common.LogWarn("Name is already taken")
@@ -61,8 +57,8 @@ func CommandClone(oldAppName string, newAppName string, skipDeploy bool, ignoreE
 
 // CommandCreate creates app via command line
 func CommandCreate(appName string) error {
-	if appName == "" {
-		return errors.New("Please specify an app to run the command on")
+	if err := common.IsValidAppName(appName); err != nil {
+		return err
 	}
 
 	return createApp(appName)
@@ -70,8 +66,8 @@ func CommandCreate(appName string) error {
 
 // CommandDestroy destroys an app
 func CommandDestroy(appName string, force bool) error {
-	if appName == "" {
-		return errors.New("Please specify an app to run the command on")
+	if err := common.VerifyAppName(appName); err != nil {
+		return err
 	}
 
 	if force {
@@ -153,7 +149,7 @@ func CommandRename(oldAppName string, newAppName string, skipDeploy bool) error
 		return errors.New("Please specify an new app name")
 	}
 
-	if err := common.IsValidAppName(oldAppName); err != nil {
+	if err := common.VerifyAppName(oldAppName); err != nil {
 		return err
 	}
 
@@ -161,10 +157,6 @@ func CommandRename(oldAppName string, newAppName string, skipDeploy bool) error
 		return err
 	}
 
-	if err := appExists(oldAppName); err != nil {
-		return err
-	}
-
 	if err := appExists(newAppName); err == nil {
 		return errors.New("Name is already taken")
 	}

plugins/apps/triggers.go

@@ -1,9 +1,6 @@
 package apps
 
 import (
-	"fmt"
-	"os"
-
 	"github.com/dokku/dokku/plugins/common"
 )
 
@@ -31,16 +28,6 @@ func TriggerAppMaybeCreate(appName string) error {
 func TriggerPostDelete(appName string) error {
 	imageRepo := common.GetAppImageRepo(appName)
 
-	// remove contents for apps that are symlinks to other folders
-	if err := os.RemoveAll(fmt.Sprintf("%v/", common.AppRoot(appName))); err != nil {
-		common.LogWarn(err.Error())
-	}
-
-	// then remove the folder and/or the symlink
-	if err := os.RemoveAll(common.AppRoot(appName)); err != nil {
-		common.LogWarn(err.Error())
-	}
-
 	imagesByAppLabel, err := listImagesByAppLabel(appName)
 	if err != nil {
 		common.LogWarn(err.Error())

plugins/common/common.go

@@ -416,10 +416,10 @@ func UcFirst(str string) string {
 	return ""
 }
 
-// IsValidAppName verifies app name format
+// IsValidAppName verifies that the app name matches naming restrictions
 func IsValidAppName(appName string) error {
 	if appName == "" {
-		return fmt.Errorf("APP must not be null")
+		return errors.New("Please specify an app to run the command on")
 	}
 
 	r, _ := regexp.Compile("^[a-z0-9][^/:_A-Z]*$")
@@ -430,10 +430,27 @@ func IsValidAppName(appName string) error {
 	return errors.New("App name must begin with lowercase alphanumeric character, and cannot include uppercase characters, colons, or underscores")
 }
 
-// VerifyAppName verifies app name format and app existence"
+// isValidAppNameOld verifies that the app name matches the old naming restrictions
+func isValidAppNameOld(appName string) error {
+	if appName == "" {
+		return errors.New("Please specify an app to run the command on")
+	}
+
+	r, _ := regexp.Compile("^[a-z0-9][^/:A-Z]*$")
+	if r.MatchString(appName) {
+		return nil
+	}
+
+	return errors.New("App name must begin with lowercase alphanumeric character, and cannot include uppercase characters, or colons")
+}
+
+// VerifyAppName checks if an app conforming to either the old or new
+// naming conventions exists
 func VerifyAppName(appName string) error {
-	if err := IsValidAppName(appName); err != nil {
-		return err
+	newErr := IsValidAppName(appName)
+	oldErr := isValidAppNameOld(appName)
+	if newErr != nil && oldErr != nil {
+		return newErr
 	}
 
 	appRoot := AppRoot(appName)

plugins/common/functions

@@ -181,9 +181,9 @@ dokku_container_log_verbose_quiet() {
 }
 
 is_valid_app_name() {
-  declare desc="verify app name format"
+  declare desc="verify that the app name matches naming restrictions"
   local APP="$1"
-  [[ -z "$APP" ]] && dokku_log_fail "APP must not be null"
+  [[ -z "$APP" ]] && dokku_log_fail "Please specify an app to run the command on"
   if [[ "$APP" =~ ^[a-z].* ]] || [[ "$APP" =~ ^[0-9].* ]]; then
     if [[ ! $APP =~ [A-Z] ]] && [[ ! $APP =~ [:] ]] && [[ ! $APP =~ [_] ]]; then
       return 0
@@ -193,10 +193,31 @@ is_valid_app_name() {
   dokku_log_fail "App name must begin with lowercase alphanumeric character, and cannot include uppercase characters, colons, or underscores"
 }
 
+is_valid_app_name_old() {
+  declare desc="verify that the app name matches the old naming restrictions"
+  local APP="$1"
+  [[ -z "$APP" ]] && dokku_log_fail "Please specify an app to run the command on"
+  if [[ "$APP" =~ ^[a-z].* ]] || [[ "$APP" =~ ^[0-9].* ]]; then
+    if [[ ! $APP =~ [A-Z] ]] && [[ ! $APP =~ [:] ]]; then
+      return 0
+    fi
+  fi
+
+  dokku_log_fail "App name must begin with lowercase alphanumeric character, and cannot include uppercase characters, or colons"
+}
+
 verify_app_name() {
   declare desc="verify app name format and app existence"
-  local APP="$1"
-  is_valid_app_name "$APP"
+  declare APP="$1"
+  local VALID_NEW=false
+  local VALID_OLD=false
+  is_valid_app_name "$APP" 2>/dev/null && VALID_NEW=true
+  is_valid_app_name_old "$APP" 2>/dev/null && VALID_OLD=true
+
+  if [[ "$VALID_NEW" == "false" ]] && [[ "$VALID_OLD" == "false" ]]; then
+    dokku_log_fail "App name must begin with lowercase alphanumeric character, and cannot include uppercase characters, colons, or underscores"
+  fi
+
   [[ ! -d "$DOKKU_ROOT/$APP" ]] && dokku_log_fail "App $APP does not exist"
 
   return 0
@@ -216,7 +237,6 @@ get_app_image_repo() {
   declare desc="central definition of image repo pattern"
   local APP="$1"
   local IMAGE_REPO="dokku/$APP"
-  is_valid_app_name "$APP"
   echo "$IMAGE_REPO"
 }
 
@@ -225,7 +245,6 @@ get_deploying_app_image_name() {
   local APP="$1"
   local IMAGE_TAG="$2"
   IMAGE_REPO="$3"
-  is_valid_app_name "$APP"
 
   local IMAGE_REMOTE_REPOSITORY=$(plugn trigger deployed-app-repository "$APP")
   local NEW_IMAGE_TAG=$(plugn trigger deployed-app-image-tag "$APP")
@@ -246,7 +265,6 @@ get_app_image_name() {
   local APP="$1"
   local IMAGE_TAG="$2"
   IMAGE_REPO="$3"
-  is_valid_app_name "$APP"
 
   if [[ -z "$IMAGE_REPO" ]]; then
     IMAGE_REPO=$(get_app_image_repo "$APP")

plugins/config/config.go

@@ -1,7 +1,6 @@
 package config
 
 import (
-	"errors"
 	"fmt"
 	"os"
 	"regexp"
@@ -145,9 +144,14 @@ func getEnvironment(appName string, merged bool) (env *Env) {
 }
 
 func getAppNameOrGlobal(appName string, global bool) (string, error) {
-	if appName == "" && !global {
-		return appName, errors.New("Please specify an app to run the command on or --global")
+	if global {
+		return appName, nil
 	}
+
+	if err := common.VerifyAppName(appName); err != nil {
+		return appName, err
+	}
+
 	return appName, nil
 }
 

plugins/config/environment.go

@@ -56,6 +56,10 @@ func newEnvFromString(rep string) (env *Env, err error) {
 
 //LoadAppEnv loads an environment for the given app
 func LoadAppEnv(appName string) (env *Env, err error) {
+	err = common.VerifyAppName(appName)
+	if err != nil {
+		return
+	}
 	appfile, err := getAppFile(appName)
 	if err != nil {
 		return

plugins/network/go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/codeskyblue/go-sh v0.0.0-20190412065543-76bd3d59ff27
 	github.com/dokku/dokku/plugins/common v0.0.0-00010101000000-000000000000
 	github.com/dokku/dokku/plugins/config v0.0.0-00010101000000-000000000000
-  github.com/spf13/pflag v1.0.5 // indirect
+	github.com/spf13/pflag v1.0.5 // indirect
 )
 
 replace github.com/dokku/dokku/plugins/common => ../common

tests/unit/apps_1.bats

@@ -179,7 +179,7 @@ teardown() {
   echo "status: $status"
   assert_success
 
-  run /bin/bash -c "dokku --trace apps:rename $TEST_APP great-test-name"
+  run /bin/bash -c "dokku apps:rename $TEST_APP great-test-name"
   echo "output: $output"
   echo "status: $status"
   assert_success

tests/unit/test_helper.bash

@@ -21,14 +21,26 @@ global_setup() {
   [[ ! -f "${BATS_PARENT_TMPNAME}.skip" ]] || skip "$SKIPPED_TEST_ERR_MSG"
 
   free -m
+  cleanup_apps
+  cleanup_containers
 }
 
 global_teardown() {
   [[ -n "$BATS_TEST_COMPLETED" ]] || touch "${BATS_PARENT_TMPNAME}.skip"
+  cleanup_apps
+  cleanup_containers
+}
+
+cleanup_apps() {
+  rm -rf $DOKKU_ROOT/*/nginx.conf
+
   apps=$(dokku --quiet apps:list)
   if [[ -n "${apps}" ]]; then
     dokku --quiet apps:list | xargs -n1 dokku --force apps:destroy
   fi
+}
+
+cleanup_containers() {
   containers=$(docker container ls --quiet)
   if [[ -n "$containers" ]]; then
     docker container ls --quiet | xargs -n1 docker container rm -f || true
@@ -132,7 +144,7 @@ assert_output_contains() {
   local found=0
   until [ "${input/$expected/}" = "$input" ]; do
     input="${input/$expected/}"
-    let found+=1
+    found=$((found + 1))
   done
   assert_equal "$count" "$found"
 }
@@ -208,7 +220,7 @@ add_domain() {
 # shellcheck disable=SC2119
 check_urls() {
   local PATTERN="$1"
-  run /bin/bash -c "dokku --quiet urls $TEST_APP | grep -E \"${1}\""
+  run /bin/bash -c "dokku --quiet urls $TEST_APP | grep -E \"${PATTERN}\""
   echo "output: $output"
   echo "status: $status"
   assert_success
@@ -238,7 +250,7 @@ assert_nonssl_domain() {
 assert_app_domain() {
   local domain=$1
   run /bin/bash -c "dokku domains:report $TEST_APP --domains-app-vhosts | tr \" \" \"\n\" | grep -xF ${domain}"
-  echo "app domains: $(dokku domains:report $TEST_APP --domains-app-vhosts | tr \" \" \"\n\")"
+  echo "app domains: $(dokku domains:report "$TEST_APP" --domains-app-vhosts | tr \" \" \"\n\")"
   echo "output: $output"
   echo "status: $status"
   assert_output "${domain}"
@@ -315,7 +327,8 @@ deploy_app() {
 }
 
 setup_client_repo() {
-  local TMP=$(mktemp -d "/tmp/dokku.me.XXXXX")
+  local TMP
+  TMP=$(mktemp -d "/tmp/dokku.me.XXXXX")
   rmdir "$TMP" && cp -r "${BATS_TEST_DIRNAME}/../../tests/apps/nodejs-express" "$TMP"
   cd "$TMP" || exit 1
   git init