diff --git a/plugins/apps/functions.go b/plugins/apps/functions.go index 867c3536b..437706328 100644 --- a/plugins/apps/functions.go +++ b/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 } diff --git a/plugins/apps/subcommands.go b/plugins/apps/subcommands.go index 2c73a234b..fe23dce64 100644 --- a/plugins/apps/subcommands.go +++ b/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") } diff --git a/plugins/apps/triggers.go b/plugins/apps/triggers.go index 45426436f..49f53d9c9 100644 --- a/plugins/apps/triggers.go +++ b/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()) diff --git a/plugins/common/common.go b/plugins/common/common.go index 5ef60c4ed..4fc019cea 100644 --- a/plugins/common/common.go +++ b/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) diff --git a/plugins/common/functions b/plugins/common/functions index 88146de85..dc721a5de 100755 --- a/plugins/common/functions +++ b/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") diff --git a/plugins/config/config.go b/plugins/config/config.go index 37734261a..71feb4788 100644 --- a/plugins/config/config.go +++ b/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 } diff --git a/plugins/config/environment.go b/plugins/config/environment.go index a4bb1063f..23e639eda 100644 --- a/plugins/config/environment.go +++ b/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 diff --git a/plugins/network/go.mod b/plugins/network/go.mod index 9208e00ad..8858202ab 100644 --- a/plugins/network/go.mod +++ b/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 diff --git a/tests/unit/apps_1.bats b/tests/unit/apps_1.bats index 28ababc7b..79cea4238 100644 --- a/tests/unit/apps_1.bats +++ b/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 diff --git a/tests/unit/test_helper.bash b/tests/unit/test_helper.bash index 1cdf008ab..379bb184f 100644 --- a/tests/unit/test_helper.bash +++ b/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