From 0405b36ab4ee6456cd6c0b97c0fecc87c76b76d1 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 11 Feb 2021 06:41:11 -0500 Subject: [PATCH 01/10] fix: implement config-export hook This will allow dropping many of the "source config/functions" calls littered over the codebase. Plugin triggers are the way to perform cross-plugin communication :) --- docs/development/plugin-triggers.md | 15 ++++++++++ plugins/20_events/config-export | 1 + plugins/config/Makefile | 2 +- plugins/config/functions.go | 38 +++++++++++++++++++++++++ plugins/config/src/triggers/triggers.go | 6 ++++ plugins/config/subcommands.go | 33 +-------------------- plugins/config/triggers.go | 19 ++++++++++++- 7 files changed, 80 insertions(+), 34 deletions(-) create mode 120000 plugins/20_events/config-export create mode 100644 plugins/config/functions.go diff --git a/docs/development/plugin-triggers.md b/docs/development/plugin-triggers.md index 0f6db0532..37acb4e32 100644 --- a/docs/development/plugin-triggers.md +++ b/docs/development/plugin-triggers.md @@ -261,6 +261,21 @@ help_content esac ``` +### `config-export` + +- Description: Returns the environment variables in a specified format +- Invoked by: app-json plugin +- Arguments: `$APP $GLOBAL $MERGED $FORMAT` +- Example: + +```shell +#!/usr/bin/env bash + +set -eo pipefail; [[ $DOKKU_TRACE ]] && set -x + +# TODO +``` + ### `config-get` - Description: Fetches the app config value for a key diff --git a/plugins/20_events/config-export b/plugins/20_events/config-export new file mode 120000 index 000000000..5178a749f --- /dev/null +++ b/plugins/20_events/config-export @@ -0,0 +1 @@ +hook \ No newline at end of file diff --git a/plugins/config/Makefile b/plugins/config/Makefile index 8fd01f747..7be217943 100644 --- a/plugins/config/Makefile +++ b/plugins/config/Makefile @@ -1,5 +1,5 @@ SUBCOMMANDS = subcommands/bundle subcommands/clear subcommands/export subcommands/get subcommands/keys subcommands/show subcommands/set subcommands/unset -TRIGGERS = triggers/config-get triggers/config-get-global +TRIGGERS = triggers/config-export triggers/config-get triggers/config-get-global BUILD = commands subcommands triggers PLUGIN_NAME = config diff --git a/plugins/config/functions.go b/plugins/config/functions.go new file mode 100644 index 000000000..72b46aa7a --- /dev/null +++ b/plugins/config/functions.go @@ -0,0 +1,38 @@ +package config + +import "fmt" + +func export(appName string, global bool, merged bool, format string) error { + appName, err := getAppNameOrGlobal(appName, global) + if err != nil { + return err + } + + env := getEnvironment(appName, merged) + exportType := ExportFormatExports + suffix := "\n" + + exportTypes := map[string]ExportFormat{ + "exports": ExportFormatExports, + "envfile": ExportFormatEnvfile, + "docker-args": ExportFormatDockerArgs, + "docker-args-keys": ExportFormatDockerArgsKeys, + "shell": ExportFormatShell, + "pretty": ExportFormatPretty, + "json": ExportFormatJSON, + "json-list": ExportFormatJSONList, + } + + exportType, ok := exportTypes[format] + if !ok { + return fmt.Errorf("Unknown export format: %v", format) + } + + if exportType == ExportFormatShell { + suffix = " " + } + + exported := env.Export(exportType) + fmt.Print(exported + suffix) + return nil +} diff --git a/plugins/config/src/triggers/triggers.go b/plugins/config/src/triggers/triggers.go index 0ad9f4446..f55d80786 100644 --- a/plugins/config/src/triggers/triggers.go +++ b/plugins/config/src/triggers/triggers.go @@ -18,6 +18,12 @@ func main() { var err error switch trigger { + case "config-export": + appName := flag.Arg(0) + global := flag.Arg(1) + merged := flag.Arg(2) + format := flag.Arg(3) + config.TriggerConfigExport(appName, global, merged, format) case "config-get": appName := flag.Arg(0) key := flag.Arg(1) diff --git a/plugins/config/subcommands.go b/plugins/config/subcommands.go index fdf30af52..033b45dd2 100644 --- a/plugins/config/subcommands.go +++ b/plugins/config/subcommands.go @@ -33,38 +33,7 @@ func CommandClear(appName string, global bool, noRestart bool) error { // CommandExport implements config:export func CommandExport(appName string, global bool, merged bool, format string) error { - appName, err := getAppNameOrGlobal(appName, global) - if err != nil { - return err - } - - env := getEnvironment(appName, merged) - exportType := ExportFormatExports - suffix := "\n" - - exportTypes := map[string]ExportFormat{ - "exports": ExportFormatExports, - "envfile": ExportFormatEnvfile, - "docker-args": ExportFormatDockerArgs, - "docker-args-keys": ExportFormatDockerArgsKeys, - "shell": ExportFormatShell, - "pretty": ExportFormatPretty, - "json": ExportFormatJSON, - "json-list": ExportFormatJSONList, - } - - exportType, ok := exportTypes[format] - if !ok { - return fmt.Errorf("Unknown export format: %v", format) - } - - if exportType == ExportFormatShell { - suffix = " " - } - - exported := env.Export(exportType) - fmt.Print(exported + suffix) - return nil + return export(appName, global, merged, format) } // CommandGet implements config:get diff --git a/plugins/config/triggers.go b/plugins/config/triggers.go index 6bebf8230..7bcfbb50f 100644 --- a/plugins/config/triggers.go +++ b/plugins/config/triggers.go @@ -1,6 +1,23 @@ package config -import "fmt" +import ( + "fmt" + "strconv" +) + +// TriggerConfigExport returns a global config value by key +func TriggerConfigExport(appName string, global string, merged string, format string) error { + g, err := strconv.ParseBool(global) + if err != nil { + return err + } + + m, err := strconv.ParseBool(merged) + if err != nil { + return err + } + return export(appName, g, m, format) +} // TriggerConfigGet returns an app config value by key func TriggerConfigGet(appName string, key string) error { From 34807028dbebd0ba89163874fe619ece7822f87a Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 11 Feb 2021 06:43:15 -0500 Subject: [PATCH 02/10] feat: ensure we inject environment variables into the docker container create call in app-json This fixes an issue where the values are empty for CNB applications. --- plugins/app-json/functions.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/plugins/app-json/functions.go b/plugins/app-json/functions.go index 750944a29..042600dc9 100644 --- a/plugins/app-json/functions.go +++ b/plugins/app-json/functions.go @@ -366,15 +366,25 @@ func createdContainerID(appName string, dockerArgs []string, image string, comma arguments = append(arguments, image) arguments = append(arguments, command...) + b, err := common.PlugnTriggerOutput("config-export", []string{appName, "false", "true", "json"}...) + if err != nil { + return "", err + } + var env map[string]string + if err := json.Unmarshal(b, &env); err != nil { + return "", err + } + containerCreateCmd := common.NewShellCmdWithArgs( common.DockerBin(), arguments..., ) var stderr bytes.Buffer + containerCreateCmd.Env = env containerCreateCmd.ShowOutput = false containerCreateCmd.Command.Stderr = &stderr - b, err := containerCreateCmd.Output() + b, err = containerCreateCmd.Output() if err != nil { return "", errors.New(stderr.String()) } From b82677068761ac093c0093b6549618c53f13de89 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 11 Feb 2021 06:43:37 -0500 Subject: [PATCH 03/10] fix: add cnb fix to copy_from_image --- plugins/common/docker.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/common/docker.go b/plugins/common/docker.go index 42fe0cb93..492240f3c 100644 --- a/plugins/common/docker.go +++ b/plugins/common/docker.go @@ -68,6 +68,8 @@ func CopyFromImage(appName string, image string, source string, destination stri if !IsAbsPath(source) { if IsImageHerokuishBased(image, appName) { workDir = "/app" + } else if IsImageCnbBased(image) { + workDir = "/workspace" } else { workDir, _ = DockerInspect(image, "{{.Config.WorkingDir}}") } From 5614475521eeee32d8880215bfe952dd2567a0a9 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 11 Feb 2021 06:48:38 -0500 Subject: [PATCH 04/10] refactor: reverse if statements The shell version of IsImageHerokuishBased returns true if it is CNB for legacy reasons... --- plugins/common/docker.go | 19 ++++++++++++++++--- plugins/common/functions | 6 +++--- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/plugins/common/docker.go b/plugins/common/docker.go index 492240f3c..ef759affc 100644 --- a/plugins/common/docker.go +++ b/plugins/common/docker.go @@ -66,10 +66,10 @@ func CopyFromImage(appName string, image string, source string, destination stri workDir := "" if !IsAbsPath(source) { - if IsImageHerokuishBased(image, appName) { - workDir = "/app" - } else if IsImageCnbBased(image) { + if IsImageCnbBased(image) { workDir = "/workspace" + } else if IsImageHerokuishBased(image, appName) { + workDir = "/app" } else { workDir, _ = DockerInspect(image, "{{.Config.WorkingDir}}") } @@ -259,6 +259,19 @@ func DockerInspect(containerOrImageID, format string) (output string, err error) return } +// IsImageCnbBased returns true if app image is based on cnb +func IsImageCnbBased(image string) bool { + if len(image) == 0 { + return false + } + + output, err := DockerInspect(image, "{{index .Config.Labels \"io.buildpacks.stack.id\" }}") + if err != nil { + return false + } + return output != "" +} + // IsImageHerokuishBased returns true if app image is based on herokuish func IsImageHerokuishBased(image string, appName string) bool { output, err := DockerInspect(image, "{{range .Config.Env}}{{if eq . \"USER=herokuishuser\" }}{{println .}}{{end}}{{end}}") diff --git a/plugins/common/functions b/plugins/common/functions index 7fbd0b396..93002186a 100755 --- a/plugins/common/functions +++ b/plugins/common/functions @@ -477,10 +477,10 @@ copy_from_image() { if verify_image "$IMAGE"; then if ! is_abs_path "$SRC_FILE"; then - if is_image_herokuish_based "$IMAGE" "$APP"; then - WORKDIR="/app" - elif is_image_cnb_based "$IMAGE"; then + if is_image_cnb_based "$IMAGE"; then WORKDIR="/workspace" + elif is_image_herokuish_based "$IMAGE" "$APP"; then + WORKDIR="/app" else WORKDIR="$("$DOCKER_BIN" image inspect --format '{{.Config.WorkingDir}}' "$IMAGE")" fi From d48cf24c4d40dca5cb33775da297026fbec2a22a Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 11 Feb 2021 06:49:19 -0500 Subject: [PATCH 05/10] fix: unify IsImageHerokuishBased with shell version Really we should just spit this out into a bin and then have the shell version wrap the golang ones... --- plugins/common/docker.go | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/plugins/common/docker.go b/plugins/common/docker.go index ef759affc..fe9dfa4bd 100644 --- a/plugins/common/docker.go +++ b/plugins/common/docker.go @@ -274,7 +274,27 @@ func IsImageCnbBased(image string) bool { // IsImageHerokuishBased returns true if app image is based on herokuish func IsImageHerokuishBased(image string, appName string) bool { - output, err := DockerInspect(image, "{{range .Config.Env}}{{if eq . \"USER=herokuishuser\" }}{{println .}}{{end}}{{end}}") + if len(image) == 0 { + return false + } + + if IsImageCnbBased(image) { + return true + } + + dokkuAppUser := "" + if len(appName) != 0 { + b, err := PlugnTriggerOutput("config-get", []string{appName, "DOKKU_APP_USER"}...) + if err == nil { + dokkuAppUser = strings.TrimSpace(string(b)) + } + } + + if len(dokkuAppUser) == 0 { + dokkuAppUser = "herokuishuser" + } + + output, err := DockerInspect(image, fmt.Sprintf("{{range .Config.Env}}{{if eq . \"USER=%s\" }}{{println .}}{{end}}{{end}}", dokkuAppUser)) if err != nil { return false } From da1ddee0ba4a908409c73cbae0b8df9030e79faf Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 11 Feb 2021 06:51:01 -0500 Subject: [PATCH 06/10] fix: override the launcher for cnb apps since the default launcher is the process launcher... --- plugins/app-json/functions.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/plugins/app-json/functions.go b/plugins/app-json/functions.go index 042600dc9..95f6d3f72 100644 --- a/plugins/app-json/functions.go +++ b/plugins/app-json/functions.go @@ -164,6 +164,7 @@ func executeScript(appName string, image string, imageTag string, phase string) common.LogInfo1(fmt.Sprintf("Executing %s task from %s: %s", phase, phaseSource, command)) isHerokuishImage := common.IsImageHerokuishBased(image, appName) + isImageCnbBased := common.IsImageCnbBased(image) dockerfileEntrypoint := "" dockerfileCommand := "" if !isHerokuishImage { @@ -224,6 +225,13 @@ func executeScript(appName string, image string, imageTag string, phase string) if os.Getenv("DOKKU_TRACE") != "" { dockerArgs = append(dockerArgs, "--env", "DOKKU_TRACE="+os.Getenv("DOKKU_TRACE")) } + if isImageCnbBased { + // TODO: handle non-linux lifecycles + // Ideally we don't have to override this but `pack` injects the web process + // as the default entrypoint, so we need to specify the launcher so the script + // runs as expected + dockerArgs = append(dockerArgs, "--entrypoint=/cnb/lifecycle/launcher") + } containerID, err := createdContainerID(appName, dockerArgs, image, script, phase) if err != nil { From 8ca713f6bb8984724e2c5a3f90eaa2bdb61a89c8 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 11 Feb 2021 06:53:48 -0500 Subject: [PATCH 07/10] fix: handle empty start command for cnb apps We should instead fetch the command from the Procfile. --- plugins/scheduler-docker-local/scheduler-deploy | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plugins/scheduler-docker-local/scheduler-deploy b/plugins/scheduler-docker-local/scheduler-deploy index 521afc1a8..d7a169695 100755 --- a/plugins/scheduler-docker-local/scheduler-deploy +++ b/plugins/scheduler-docker-local/scheduler-deploy @@ -20,12 +20,15 @@ trigger-scheduler-docker-local-scheduler-deploy() { local DOCKER_RUN_LABEL_ARGS="--label=com.dokku.app-name=$APP --label=com.dokku.container-type=deploy" local DOKKU_DOCKER_STOP_TIMEOUT DOKKU_HEROKUISH DOKKU_NETWORK_BIND_ALL IMAGE DOKKU_HEROKUISH=false + DOKKU_CNB=false IMAGE=$(get_deploying_app_image_name "$APP" "$IMAGE_TAG") plugn trigger pre-deploy "$APP" "$IMAGE_TAG" + is_image_cnb_based "$IMAGE" && DOKKU_CNB=true is_image_herokuish_based "$IMAGE" "$APP" && DOKKU_HEROKUISH=true local IMAGE_SOURCE_TYPE="dockerfile" [[ "$DOKKU_HEROKUISH" == "true" ]] && IMAGE_SOURCE_TYPE="herokuish" + [[ "$DOKKU_CNB" == "true" ]] && IMAGE_SOURCE_TYPE="cnb" local DOKKU_SCALE_FILE="$DOKKU_ROOT/$APP/DOKKU_SCALE" local oldids=$(get_app_container_ids "$APP") @@ -81,6 +84,7 @@ trigger-scheduler-docker-local-scheduler-deploy() { local START_CMD [[ "$DOKKU_HEROKUISH" == "true" ]] && START_CMD="/start $PROC_TYPE" + [[ "$DOKKU_CNB" == "true" ]] && START_CMD="" [[ -n "$DOKKU_START_CMD" ]] && START_CMD="$DOKKU_START_CMD" local DOKKU_PORT="" @@ -205,7 +209,7 @@ trigger-scheduler-docker-local-scheduler-deploy() { fn-scheduler-docker-local-extract-start-cmd() { declare APP="$1" PROC_TYPE="$2" START_CMD="$3" DOKKU_HEROKUISH="$4" PORT="$5" local DOKKU_DOCKERFILE_START_CMD DOKKU_PROCFILE_START_CMD START_CMD - if [[ "$DOKKU_HEROKUISH" != "false" ]]; then + if [[ "$DOKKU_HEROKUISH" != "false" ]] && [[ -n "$START_CMD" ]]; then echo "$START_CMD" return fi From 0673fdff2da59873ba1bf7edf59a874b9004cff6 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 11 Feb 2021 07:16:43 -0500 Subject: [PATCH 08/10] fix: handle new herokuish image detection in deployment tasks --- plugins/app-json/functions.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/plugins/app-json/functions.go b/plugins/app-json/functions.go index 95f6d3f72..c86b78932 100644 --- a/plugins/app-json/functions.go +++ b/plugins/app-json/functions.go @@ -16,7 +16,7 @@ import ( "golang.org/x/sync/errgroup" ) -func constructScript(command string, shell string, isHerokuishImage bool, hasEntrypoint bool) []string { +func constructScript(command string, shell string, isHerokuishImage bool, isCnbImage bool, hasEntrypoint bool) []string { if hasEntrypoint { words, err := shellquote.Split(strings.TrimSpace(command)) if err != nil { @@ -31,7 +31,7 @@ func constructScript(command string, shell string, isHerokuishImage bool, hasEnt script = append(script, "set -x;") } - if isHerokuishImage { + if isHerokuishImage && !isCnbImage { script = append(script, []string{ "if [[ -d '/app' ]]; then", " export HOME=/app;", @@ -60,7 +60,7 @@ func constructScript(command string, shell string, isHerokuishImage bool, hasEnt script = append(script, fmt.Sprintf("%s || exit 1;", command)) - if isHerokuishImage { + if isHerokuishImage && !isCnbImage { script = append(script, []string{ "if [[ -d '/cache' ]]; then", " rm -f /tmp/cache;", @@ -164,7 +164,7 @@ func executeScript(appName string, image string, imageTag string, phase string) common.LogInfo1(fmt.Sprintf("Executing %s task from %s: %s", phase, phaseSource, command)) isHerokuishImage := common.IsImageHerokuishBased(image, appName) - isImageCnbBased := common.IsImageCnbBased(image) + isCnbImage := common.IsImageCnbBased(image) dockerfileEntrypoint := "" dockerfileCommand := "" if !isHerokuishImage { @@ -174,11 +174,13 @@ func executeScript(appName string, image string, imageTag string, phase string) hasEntrypoint := dockerfileEntrypoint != "" dokkuAppShell := getDokkuAppShell(appName) - script := constructScript(command, dokkuAppShell, isHerokuishImage, hasEntrypoint) + script := constructScript(command, dokkuAppShell, isHerokuishImage, isCnbImage, hasEntrypoint) imageSourceType := "dockerfile" if isHerokuishImage { imageSourceType = "herokuish" + } else if isCnbImage { + imageSourceType = "cnb" } cacheDir := fmt.Sprintf("%s/cache", common.AppRoot(appName)) @@ -225,7 +227,7 @@ func executeScript(appName string, image string, imageTag string, phase string) if os.Getenv("DOKKU_TRACE") != "" { dockerArgs = append(dockerArgs, "--env", "DOKKU_TRACE="+os.Getenv("DOKKU_TRACE")) } - if isImageCnbBased { + if isCnbImage { // TODO: handle non-linux lifecycles // Ideally we don't have to override this but `pack` injects the web process // as the default entrypoint, so we need to specify the launcher so the script @@ -254,7 +256,7 @@ func executeScript(appName string, image string, imageTag string, phase string) } commitArgs := []string{"container", "commit"} - if !isHerokuishImage { + if !isHerokuishImage || isCnbImage { if dockerfileEntrypoint != "" { commitArgs = append(commitArgs, "--change", dockerfileEntrypoint) } From 123fbea7c0e94d87a7e7256ccb372e277b6d675d Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 11 Feb 2021 07:16:54 -0500 Subject: [PATCH 09/10] feat: add a deployment task test for cnb --- tests/apps/python/Procfile | 2 +- tests/apps/python/release.py | 10 ++++++++++ tests/apps/python/requirements.txt | 1 + tests/unit/app-json.bats | 20 ++++++++++++++++++++ 4 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 tests/apps/python/release.py create mode 100644 tests/apps/python/requirements.txt diff --git a/tests/apps/python/Procfile b/tests/apps/python/Procfile index def474954..88d4d849f 100644 --- a/tests/apps/python/Procfile +++ b/tests/apps/python/Procfile @@ -7,7 +7,7 @@ cron: python3 worker.py web: python3 web.py # testing inline comment worker: python3 worker.py custom: echo -n -release: touch /app/release.test +release: python3 release.py # Old version with separate processes (use this if you have issues with the threaded version) diff --git a/tests/apps/python/release.py b/tests/apps/python/release.py new file mode 100644 index 000000000..afa731b5f --- /dev/null +++ b/tests/apps/python/release.py @@ -0,0 +1,10 @@ +#!/usr/bin/env python +import os + + +def main(): + print("SECRET_KEY: {0}".format(os.getenv('SECRET_KEY'))) + + +if __name__ == '__main__': + main() diff --git a/tests/apps/python/requirements.txt b/tests/apps/python/requirements.txt new file mode 100644 index 000000000..7e1060246 --- /dev/null +++ b/tests/apps/python/requirements.txt @@ -0,0 +1 @@ +flask diff --git a/tests/unit/app-json.bats b/tests/unit/app-json.bats index 3851afe98..d8c86f832 100644 --- a/tests/unit/app-json.bats +++ b/tests/unit/app-json.bats @@ -103,3 +103,23 @@ teardown() { echo "status: $status" assert_success } + +@test "(app-json) app.json cnb release" { + run /bin/bash -c "dokku config:set --no-restart $TEST_APP DOKKU_CNB_EXPERIMENTAL=1 SECRET_KEY=fjdkslafjdk" + echo "output: $output" + echo "status: $status" + assert_success + + run deploy_app python dokku@dokku.me:$TEST_APP add_requirements_txt + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "Executing release task from Procfile" + assert_output_contains "SECRET_KEY: fjdkslafjdk" + + run /bin/bash -c "curl $(dokku url $TEST_APP)/env" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains '"SECRET_KEY": "fjdkslafjdk"' +} From 5e7ea4217f048db392af4ed17c0a22b0201db6ce Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 11 Feb 2021 07:34:27 -0500 Subject: [PATCH 10/10] tests: install pack for app-json tests --- tests/unit/app-json.bats | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/unit/app-json.bats b/tests/unit/app-json.bats index d8c86f832..4f87bc727 100644 --- a/tests/unit/app-json.bats +++ b/tests/unit/app-json.bats @@ -1,6 +1,12 @@ #!/usr/bin/env bats load test_helper +setup_file() { + add-apt-repository --yes ppa:cncf-buildpacks/pack-cli + apt-get update + apt-get --yes install pack-cli +} + setup() { global_setup create_app