From ad5537c0864a3fff9c6afdab700f928a010dfa4d Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 4 Apr 2024 02:27:43 -0400 Subject: [PATCH 1/5] refactor: directly trigger pre-deploy hook from post-release-builder script This removes the extra indirection in the deploy process for triggering the hooks. --- plugins/app-json/Makefile | 2 +- plugins/app-json/src/triggers/triggers.go | 9 +++++---- plugins/app-json/triggers.go | 7 ++++--- plugins/ps/Makefile | 2 +- plugins/ps/src/triggers/triggers.go | 9 +++++---- plugins/ps/triggers.go | 4 ++-- .../{pre-deploy => post-release-builder} | 16 ++++++++-------- 7 files changed, 26 insertions(+), 23 deletions(-) rename plugins/scheduler-docker-local/{pre-deploy => post-release-builder} (72%) diff --git a/plugins/app-json/Makefile b/plugins/app-json/Makefile index bff87b1a8..d59ac111a 100644 --- a/plugins/app-json/Makefile +++ b/plugins/app-json/Makefile @@ -1,5 +1,5 @@ SUBCOMMANDS = subcommands/report subcommands/set -TRIGGERS = triggers/app-json-process-deploy-parallelism triggers/app-json-get-content triggers/core-post-deploy triggers/core-post-extract triggers/install triggers/post-app-clone-setup triggers/post-app-rename triggers/post-app-rename-setup triggers/post-create triggers/post-delete triggers/post-deploy triggers/pre-deploy triggers/report +TRIGGERS = triggers/app-json-process-deploy-parallelism triggers/app-json-get-content triggers/core-post-deploy triggers/core-post-extract triggers/install triggers/post-app-clone-setup triggers/post-app-rename triggers/post-app-rename-setup triggers/post-create triggers/post-delete triggers/post-deploy triggers/post-release-builder triggers/report BUILD = commands subcommands triggers PLUGIN_NAME = app-json diff --git a/plugins/app-json/src/triggers/triggers.go b/plugins/app-json/src/triggers/triggers.go index 04580d2e2..828bcc5e6 100644 --- a/plugins/app-json/src/triggers/triggers.go +++ b/plugins/app-json/src/triggers/triggers.go @@ -56,10 +56,11 @@ func main() { appName := flag.Arg(0) imageTag := flag.Arg(3) err = appjson.TriggerPostDeploy(appName, imageTag) - case "pre-deploy": - appName := flag.Arg(0) - imageTag := flag.Arg(1) - err = appjson.TriggerPreDeploy(appName, imageTag) + case "post-release-builder": + builderType := flag.Arg(0) + appName := flag.Arg(1) + image := flag.Arg(2) + err = appjson.TriggerPostReleaseBuilder(builderType, appName, image) case "report": appName := flag.Arg(0) err = appjson.ReportSingleApp(appName, "", "") diff --git a/plugins/app-json/triggers.go b/plugins/app-json/triggers.go index b556c0d49..82cd95dce 100644 --- a/plugins/app-json/triggers.go +++ b/plugins/app-json/triggers.go @@ -212,9 +212,10 @@ func TriggerPostDeploy(appName string, imageTag string) error { return executeScript(appName, image, imageTag, "postdeploy") } -// TriggerPreDeploy is a trigger to execute predeploy and release deployment tasks -func TriggerPreDeploy(appName string, imageTag string) error { - image := common.GetAppImageName(appName, imageTag, "") +// TriggerPostReleaseBuilder is a trigger to execute predeploy and release deployment tasks +func TriggerPostReleaseBuilder(builderType string, appName string, image string) error { + parts := strings.Split(image, ":") + imageTag := parts[len(parts)-1] if err := executeScript(appName, image, imageTag, "predeploy"); err != nil { return err } diff --git a/plugins/ps/Makefile b/plugins/ps/Makefile index fa9285662..8e8457fc4 100644 --- a/plugins/ps/Makefile +++ b/plugins/ps/Makefile @@ -1,5 +1,5 @@ SUBCOMMANDS = subcommands/inspect subcommands/rebuild subcommands/report subcommands/restart subcommands/restore subcommands/retire subcommands/scale subcommands/set subcommands/start subcommands/stop -TRIGGERS = triggers/app-restart triggers/core-post-deploy triggers/core-post-extract triggers/install triggers/post-app-clone triggers/post-app-clone-setup triggers/post-app-rename triggers/post-app-rename-setup triggers/post-create triggers/post-delete triggers/post-stop triggers/pre-deploy triggers/procfile-get-command triggers/procfile-exists triggers/ps-can-scale triggers/ps-current-scale triggers/ps-set-scale triggers/report +TRIGGERS = triggers/app-restart triggers/core-post-deploy triggers/core-post-extract triggers/install triggers/post-app-clone triggers/post-app-clone-setup triggers/post-app-rename triggers/post-app-rename-setup triggers/post-create triggers/post-delete triggers/post-release-builder triggers/post-stop triggers/procfile-get-command triggers/procfile-exists triggers/ps-can-scale triggers/ps-current-scale triggers/ps-set-scale triggers/report BUILD = commands subcommands triggers PLUGIN_NAME = ps diff --git a/plugins/ps/src/triggers/triggers.go b/plugins/ps/src/triggers/triggers.go index d0acb17b2..4d2821dad 100644 --- a/plugins/ps/src/triggers/triggers.go +++ b/plugins/ps/src/triggers/triggers.go @@ -55,10 +55,11 @@ func main() { case "post-stop": appName := flag.Arg(0) err = ps.TriggerPostStop(appName) - case "pre-deploy": - appName := flag.Arg(0) - imageTag := flag.Arg(1) - err = ps.TriggerPreDeploy(appName, imageTag) + case "post-release-builder": + builderType := flag.Arg(0) + appName := flag.Arg(1) + image := flag.Arg(2) + err = ps.TriggerPostReleaseBuilder(builderType, appName, image) case "procfile-get-command": appName := flag.Arg(0) processType := flag.Arg(1) diff --git a/plugins/ps/triggers.go b/plugins/ps/triggers.go index c21e68a89..674369db5 100644 --- a/plugins/ps/triggers.go +++ b/plugins/ps/triggers.go @@ -257,8 +257,8 @@ func TriggerPostStop(appName string) error { }) } -// TriggerPreDeploy ensures an app has an up to date scale parameters -func TriggerPreDeploy(appName string, imageTag string) error { +// TriggerPostReleaseBuilder ensures an app has an up to date scale parameters +func TriggerPostReleaseBuilder(builderType string, appName string, image string) error { if err := updateScale(appName, false, FormationSlice{}); err != nil { common.LogDebug(fmt.Sprintf("Error generating scale file: %s", err.Error())) return err diff --git a/plugins/scheduler-docker-local/pre-deploy b/plugins/scheduler-docker-local/post-release-builder similarity index 72% rename from plugins/scheduler-docker-local/pre-deploy rename to plugins/scheduler-docker-local/post-release-builder index 04abd3d92..e5be08240 100755 --- a/plugins/scheduler-docker-local/pre-deploy +++ b/plugins/scheduler-docker-local/post-release-builder @@ -6,20 +6,20 @@ source "$PLUGIN_CORE_AVAILABLE_PATH/common/property-functions" source "$PLUGIN_AVAILABLE_PATH/config/functions" source "$PLUGIN_AVAILABLE_PATH/scheduler-docker-local/internal-functions" -trigger-scheduler-docker-local-pre-deploy() { - declare desc="scheduler-docker-local pre-deploy plugin trigger" - declare trigger="pre-deploy" - declare APP="$1" IMAGE_TAG="$2" +trigger-scheduler-docker-local-post-release-builder() { + declare desc="scheduler-docker-local post-release-builder plugin trigger" + declare trigger="post-release-builder" + declare BUILDER_TYPE="$1" APP="$2" IMAGE="$3" - local DOKKU_SCHEDULER=$(get_app_scheduler "$APP") + local DOKKU_SCHEDULER="$(get_app_scheduler "$APP")" if [[ "$DOKKU_SCHEDULER" != "docker-local" ]]; then return fi - scheduler-docker-local-pre-deploy-precheck "$APP" + scheduler-docker-local-post-release-builder-precheck "$APP" } -scheduler-docker-local-pre-deploy-precheck() { +scheduler-docker-local-post-release-builder-precheck() { declare desc="Outputs the checks messages if necessary" declare APP="$1" @@ -36,4 +36,4 @@ scheduler-docker-local-pre-deploy-precheck() { fi } -trigger-scheduler-docker-local-pre-deploy "$@" +trigger-scheduler-docker-local-post-release-builder "$@" From 2937ffb880154dcefe7e7f40fee8306efeca9922 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 4 Apr 2024 02:28:32 -0400 Subject: [PATCH 2/5] fix: make the env vars available to containers created during the pre-release process We still re-add them in the builder-release phase to ensure users don't set incorrect ones in a custom trigger. --- plugins/builder-herokuish/builder-release | 17 +++++++++++------ .../dockerfiles/builder-pre-release.Dockerfile | 5 +++++ 2 files changed, 16 insertions(+), 6 deletions(-) create mode 100644 plugins/builder-herokuish/dockerfiles/builder-pre-release.Dockerfile diff --git a/plugins/builder-herokuish/builder-release b/plugins/builder-herokuish/builder-release index 23e7d4ed5..f0a72f890 100755 --- a/plugins/builder-herokuish/builder-release +++ b/plugins/builder-herokuish/builder-release @@ -14,12 +14,6 @@ trigger-builder-herokuish-builder-release() { fi local IMAGE=$(get_app_image_name "$APP" "$IMAGE_TAG") - if fn-plugn-trigger-exists "pre-release-buildpack"; then - dokku_log_warn "Deprecated: please upgrade plugin to use 'pre-release-builder' plugin trigger instead of pre-release-buildpack" - plugn trigger pre-release-buildpack "$APP" "$IMAGE_TAG" - fi - plugn trigger pre-release-builder "$BUILDER_TYPE" "$APP" "$IMAGE" - TMP_WORK_DIR="$(mktemp -d "/tmp/dokku-${DOKKU_PID}-${FUNCNAME[0]}.XXXXXX")" trap "rm -rf '$TMP_WORK_DIR' >/dev/null" RETURN INT TERM EXIT @@ -36,6 +30,17 @@ trigger-builder-herokuish-builder-release() { DOKKU_APP_USER=$(config_get "$APP" DOKKU_APP_USER || true) DOKKU_APP_USER=${DOKKU_APP_USER:="herokuishuser"} + if ! suppress_output "$DOCKER_BIN" image build "${DOCKER_BUILD_ARGS[@]}" $DOKKU_GLOBAL_BUILD_ARGS -f "$PLUGIN_AVAILABLE_PATH/builder-herokuish/dockerfiles/builder-pre-release.Dockerfile" --build-arg APP_IMAGE="$IMAGE" --build-arg "DOKKU_APP_USER=$DOKKU_APP_USER" -t "$IMAGE" "$TMP_WORK_DIR"; then + dokku_log_warn "Failure injecting environment variables" + return 1 + fi + + if fn-plugn-trigger-exists "pre-release-buildpack"; then + dokku_log_warn "Deprecated: please upgrade plugin to use 'pre-release-builder' plugin trigger instead of pre-release-buildpack" + plugn trigger pre-release-buildpack "$APP" "$IMAGE_TAG" + fi + plugn trigger pre-release-builder "$BUILDER_TYPE" "$APP" "$IMAGE" + if ! suppress_output "$DOCKER_BIN" image build "${DOCKER_BUILD_ARGS[@]}" $DOKKU_GLOBAL_BUILD_ARGS -f "$PLUGIN_AVAILABLE_PATH/builder-herokuish/dockerfiles/builder-release.Dockerfile" --build-arg APP_IMAGE="$IMAGE" --build-arg "DOKKU_APP_USER=$DOKKU_APP_USER" -t "$IMAGE" "$TMP_WORK_DIR"; then dokku_log_warn "Failure injecting environment variables" return 1 diff --git a/plugins/builder-herokuish/dockerfiles/builder-pre-release.Dockerfile b/plugins/builder-herokuish/dockerfiles/builder-pre-release.Dockerfile new file mode 100644 index 000000000..bef15548b --- /dev/null +++ b/plugins/builder-herokuish/dockerfiles/builder-pre-release.Dockerfile @@ -0,0 +1,5 @@ +ARG APP_IMAGE +FROM $APP_IMAGE + +ARG DOKKU_APP_USER herokuishuser +COPY --chown=$DOKKU_APP_USER 00-global-env.sh 01-app-env.sh /app/.profile.d/ From 104a5b4a93878a221f81cc6ec3abc066090834d3 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 4 Apr 2024 02:34:09 -0400 Subject: [PATCH 3/5] refactor: execute the predeploy script during the `pre-release-builder` plugin trigger This fixes an issue where the post-release-builder trigger inadvertently modifies the image. We take advantage of the fact that env vars are available at the `pre-release-builder` phase to ensure any image modifications occur as appropriate. --- plugins/app-json/Makefile | 2 +- plugins/app-json/src/triggers/triggers.go | 5 +++++ plugins/app-json/triggers.go | 10 ++++++---- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/plugins/app-json/Makefile b/plugins/app-json/Makefile index d59ac111a..229186118 100644 --- a/plugins/app-json/Makefile +++ b/plugins/app-json/Makefile @@ -1,5 +1,5 @@ SUBCOMMANDS = subcommands/report subcommands/set -TRIGGERS = triggers/app-json-process-deploy-parallelism triggers/app-json-get-content triggers/core-post-deploy triggers/core-post-extract triggers/install triggers/post-app-clone-setup triggers/post-app-rename triggers/post-app-rename-setup triggers/post-create triggers/post-delete triggers/post-deploy triggers/post-release-builder triggers/report +TRIGGERS = triggers/app-json-process-deploy-parallelism triggers/app-json-get-content triggers/core-post-deploy triggers/core-post-extract triggers/install triggers/post-app-clone-setup triggers/post-app-rename triggers/post-app-rename-setup triggers/post-create triggers/post-delete triggers/post-deploy triggers/post-release-builder triggers/pre-release-builder triggers/report BUILD = commands subcommands triggers PLUGIN_NAME = app-json diff --git a/plugins/app-json/src/triggers/triggers.go b/plugins/app-json/src/triggers/triggers.go index 828bcc5e6..862982ac5 100644 --- a/plugins/app-json/src/triggers/triggers.go +++ b/plugins/app-json/src/triggers/triggers.go @@ -61,6 +61,11 @@ func main() { appName := flag.Arg(1) image := flag.Arg(2) err = appjson.TriggerPostReleaseBuilder(builderType, appName, image) + case "pre-release-builder": + builderType := flag.Arg(0) + appName := flag.Arg(1) + image := flag.Arg(2) + err = appjson.TriggerPreReleaseBuilder(builderType, appName, image) case "report": appName := flag.Arg(0) err = appjson.ReportSingleApp(appName, "", "") diff --git a/plugins/app-json/triggers.go b/plugins/app-json/triggers.go index 82cd95dce..0eedd16a0 100644 --- a/plugins/app-json/triggers.go +++ b/plugins/app-json/triggers.go @@ -212,14 +212,16 @@ func TriggerPostDeploy(appName string, imageTag string) error { return executeScript(appName, image, imageTag, "postdeploy") } +func TriggerPreReleaseBuilder(builderType string, appName string, image string) error { + parts := strings.Split(image, ":") + imageTag := parts[len(parts)-1] + return executeScript(appName, image, imageTag, "predeploy") +} + // TriggerPostReleaseBuilder is a trigger to execute predeploy and release deployment tasks func TriggerPostReleaseBuilder(builderType string, appName string, image string) error { parts := strings.Split(image, ":") imageTag := parts[len(parts)-1] - if err := executeScript(appName, image, imageTag, "predeploy"); err != nil { - return err - } - if err := executeScript(appName, image, imageTag, "release"); err != nil { return err } From 1a07025e990ca70099bb73ff98aeb6466417ac88 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 4 Apr 2024 02:34:29 -0400 Subject: [PATCH 4/5] chore: deprecate the pre-deploy trigger in favor of the pre-release-builder trigger --- docs/appendices/0.34.0-migration-guide.md | 4 ++++ docs/development/plugin-triggers.md | 3 +++ plugins/registry/triggers.go | 17 ++++++++++------- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/docs/appendices/0.34.0-migration-guide.md b/docs/appendices/0.34.0-migration-guide.md index 1d99c9df5..d6d157843 100644 --- a/docs/appendices/0.34.0-migration-guide.md +++ b/docs/appendices/0.34.0-migration-guide.md @@ -17,3 +17,7 @@ ```shell dokku scheduler-k3s:set --global ingress-class traefik ``` + +## Deprecations + +The `pre-deploy` plugin trigger is deprecated as of `0.34.4`. It is currently invoked during the `post-release-builder` plugin trigger, where image mutation is heavily discouraged. Users should instead move any trigger usage to the `pre-release-builder` plugin trigger. The `pre-deploy` plugin trigger will be removed in a future release. diff --git a/docs/development/plugin-triggers.md b/docs/development/plugin-triggers.md index 2a66d0527..9ca89d4ca 100644 --- a/docs/development/plugin-triggers.md +++ b/docs/development/plugin-triggers.md @@ -1934,6 +1934,9 @@ fi ### `pre-deploy` +> [!WARNING] +> Deprecated, please use `pre-release-builder` instead + - Description: Allows the running of code before the app's processes are scaled up and after the docker images are prepared. - Invoked by: `dokku deploy` - Arguments: `$APP $IMAGE_TAG` diff --git a/plugins/registry/triggers.go b/plugins/registry/triggers.go index 5579a8b74..44ec3d729 100644 --- a/plugins/registry/triggers.go +++ b/plugins/registry/triggers.go @@ -90,13 +90,16 @@ func TriggerPostDelete(appName string) error { func TriggerPostReleaseBuilder(appName string, image string) error { parts := strings.Split(image, ":") imageTag := parts[len(parts)-1] - _, err := common.CallPlugnTrigger(common.PlugnTriggerInput{ - Trigger: "pre-deploy", - Args: []string{appName, imageTag}, - StreamStdio: true, - }) - if err != nil { - return err + if common.PlugnTriggerExists("pre-deploy") { + common.LogWarn("Deprecated: please upgrade plugin to use 'pre-release-builder' plugin trigger instead of pre-deploy") + _, err := common.CallPlugnTrigger(common.PlugnTriggerInput{ + Trigger: "pre-deploy", + Args: []string{appName, imageTag}, + StreamStdio: true, + }) + if err != nil { + return err + } } imageID, _ := common.DockerInspect(image, "{{ .Id }}") From 9d6201985e311fe946c7c5def3e92384ea8cafc0 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 4 Apr 2024 02:58:48 -0400 Subject: [PATCH 5/5] fix: ensure everything in /app is correctly chowned --- plugins/builder-herokuish/dockerfiles/builder-release.Dockerfile | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/builder-herokuish/dockerfiles/builder-release.Dockerfile b/plugins/builder-herokuish/dockerfiles/builder-release.Dockerfile index aecc0872a..12637e825 100644 --- a/plugins/builder-herokuish/dockerfiles/builder-release.Dockerfile +++ b/plugins/builder-herokuish/dockerfiles/builder-release.Dockerfile @@ -3,5 +3,6 @@ FROM $APP_IMAGE ARG DOKKU_APP_USER herokuishuser COPY --chown=$DOKKU_APP_USER 00-global-env.sh 01-app-env.sh /app/.profile.d/ +RUN TRACE=$TRACE USER=$DOKKU_APP_USER HEROKUISH_DISABLE_CHOWN=false /exec true USER $DOKKU_APP_USER ENV HEROKUISH_SETUIDGUID false