From 1e58b56ac7d6a0a0b794d93d949a256a7ede71fe Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sun, 18 Dec 2022 17:49:47 -0500 Subject: [PATCH 1/4] feat: add git-get-property trigger --- docs/development/plugin-triggers.md | 16 ++++++++++++++++ plugins/20_events/git-get-property | 1 + plugins/git/git-get-property | 19 +++++++++++++++++++ 3 files changed, 36 insertions(+) create mode 120000 plugins/20_events/git-get-property create mode 100755 plugins/git/git-get-property diff --git a/docs/development/plugin-triggers.md b/docs/development/plugin-triggers.md index a001ab3fa..57dc999b3 100644 --- a/docs/development/plugin-triggers.md +++ b/docs/development/plugin-triggers.md @@ -803,6 +803,22 @@ set -eo pipefail; [[ $DOKKU_TRACE ]] && set -x # TODO ``` +### `git-get-property` + +- Description: Return the value for an app's git property +- Invoked by: +- Arguments: `$APP $KEY` +- Example: + +```shell +#!/usr/bin/env bash + +set -eo pipefail; [[ $DOKKU_TRACE ]] && set -x +APP="$1"; PROPERTY="$2" + +# TODO +``` + ### `git-from-archive` - Description: Updates an app's git repository from an archive and then triggers a build diff --git a/plugins/20_events/git-get-property b/plugins/20_events/git-get-property new file mode 120000 index 000000000..5178a749f --- /dev/null +++ b/plugins/20_events/git-get-property @@ -0,0 +1 @@ +hook \ No newline at end of file diff --git a/plugins/git/git-get-property b/plugins/git/git-get-property new file mode 100755 index 000000000..182e9cd8e --- /dev/null +++ b/plugins/git/git-get-property @@ -0,0 +1,19 @@ +#!/usr/bin/env bash +set -eo pipefail +source "$PLUGIN_AVAILABLE_PATH/git/internal-functions" +[[ $DOKKU_TRACE ]] && set -x + +trigger-git-git-get-property() { + declare desc="return the value for an app's git property" + declare trigger="git-get-property" + declare APP="$1" KEY="$2" + + if [[ "$KEY" == "source-image" ]]; then + fn-git-source-image "$APP" + return + fi + + return 1 +} + +trigger-git-git-get-property "$@" From ca4be83e64a0c2c1c69e2fffe31bbe1b54079c8f Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sun, 18 Dec 2022 17:49:55 -0500 Subject: [PATCH 2/4] fix: allow nginx-conf-sigil-path to be set globally --- plugins/nginx-vhosts/subcommands/set | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/nginx-vhosts/subcommands/set b/plugins/nginx-vhosts/subcommands/set index 503f4ac0a..b3ae03c93 100755 --- a/plugins/nginx-vhosts/subcommands/set +++ b/plugins/nginx-vhosts/subcommands/set @@ -10,7 +10,7 @@ cmd-nginx-set() { [[ "$1" == "$cmd" ]] && shift 1 declare APP="$1" KEY="$2" VALUE="$3" local VALID_KEYS=("access-log-format" "access-log-path" "bind-address-ipv4" "bind-address-ipv6" "client-max-body-size" "disable-custom-config" "error-log-path" "hsts" "hsts-include-subdomains" "hsts-preload" "hsts-max-age" "nginx-conf-sigil-path" "proxy-read-timeout" "proxy-buffer-size" "proxy-buffering" "proxy-buffers" "proxy-busy-buffers-size" "x-forwarded-for-value" "x-forwarded-port-value" "x-forwarded-proto-value" "x-forwarded-ssl") - local GLOBAL_KEYS=("hsts") + local GLOBAL_KEYS=("hsts" "nginx-conf-sigil-path") [[ -z "$KEY" ]] && dokku_log_fail "No key specified" From 654939afa7e0422f86f12e8766525e0cdec066f3 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sun, 18 Dec 2022 17:52:32 -0500 Subject: [PATCH 3/4] refactor: do not pull image if it already exists --- docs/deployment/methods/git.md | 2 ++ plugins/git/git-from-image | 8 ++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/deployment/methods/git.md b/docs/deployment/methods/git.md index ad34a5098..444b181e6 100644 --- a/docs/deployment/methods/git.md +++ b/docs/deployment/methods/git.md @@ -136,6 +136,8 @@ In the above example, Dokku will build the app as if the repository contained _o FROM dokku/node-js-getting-started:latest ``` +If the specified image already exists on the Dokku host, it will not be pulled again, though this behavior may be changed using [build phase docker-options](/docs/advanced-usage/docker-options.md). + Triggering a build with the same arguments multiple times will result in Dokku exiting `0` early as there will be no changes detected. If the image tag is reused but the underlying image is different, it is recommended to use the image digest instead of the tag. This can be retrieved via the following command: ```shell diff --git a/plugins/git/git-from-image b/plugins/git/git-from-image index 6300ee02d..009f3feec 100755 --- a/plugins/git/git-from-image +++ b/plugins/git/git-from-image @@ -26,8 +26,12 @@ trigger-git-git-from-image() { echo "FROM $DOCKER_IMAGE" >>"$TMP_WORK_DIR/Dockerfile" echo "LABEL com.dokku.docker-image-labeler/alternate-tags=[\\\"$DOCKER_IMAGE\\\"]" >>"$TMP_WORK_DIR/Dockerfile" - dokku_log_info1 "Pulling image" - "$DOCKER_BIN" image pull "$DOCKER_IMAGE" + if [[ "$(docker images -q "$DOCKER_IMAGE" 2>/dev/null)" == "" ]]; then + dokku_log_info1 "Pulling image" + "$DOCKER_BIN" image pull "$DOCKER_IMAGE" + else + dokku_log_info1 "Image exists on host, skipping pull" + fi export DOKKU_BUILDING_FROM_IMAGE="$DOCKER_IMAGE" plugn trigger git-from-directory "$APP" "$TMP_WORK_DIR" "$USER_NAME" "$USER_EMAIL" From c3c6e2174a4e9ad607cc371812c832ae36106998 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sun, 18 Dec 2022 17:57:24 -0500 Subject: [PATCH 4/4] fix: keep track of the source image when building an app via git:from-image This change fixes issues with ps:rebuild not working as expected - essentially ignoring files in the repo - as well as adds tests. Closes #5518 --- docs/appendices/0.29.0-migration-guide.md | 4 +- docs/deployment/methods/git.md | 12 ++ plugins/git/git-from-image | 3 +- plugins/git/internal-functions | 8 ++ plugins/git/subcommands/set | 4 +- plugins/nginx-vhosts/core-post-extract | 6 +- plugins/ps/triggers.go | 10 +- tests/apps/python/alt.Dockerfile | 5 + tests/unit/nginx-vhosts_12.bats | 129 ++++++++++++++++++++++ 9 files changed, 171 insertions(+), 10 deletions(-) create mode 100644 tests/apps/python/alt.Dockerfile create mode 100644 tests/unit/nginx-vhosts_12.bats diff --git a/docs/appendices/0.29.0-migration-guide.md b/docs/appendices/0.29.0-migration-guide.md index 3aced033b..bd1e08ea6 100644 --- a/docs/appendices/0.29.0-migration-guide.md +++ b/docs/appendices/0.29.0-migration-guide.md @@ -5,13 +5,15 @@ - The output of `run:detached` now uses the container name - eg. `node-js-app.run.1` - vs the container id. - The ID of `cron` tasks is now base36-encoded instead of base64-encoded. - The `nginx.conf.sigil` is now extracted when source code is extracted for a build and not from the built image. Users can specify alternative paths via the `nginx-conf-sigil-path` property of the `nginx` plugin. See the [nginx documentation](/docs/networking/proxies/nginx.md#customizing-the-nginx-configuration) for more information on how to configure the `nginx.conf.sigil` path for your application. + - For deploys via `git:from-image`, the `nginx.conf.sigil` file will be extracted from the source image, respecting the value of `nginx-conf-sigil-path`. +- The `Procfile` is now extracted when source code is extracted for a build and not from the built image. Users can specify alternative paths via the `procfile-path` property of the `ps` plugin. See the [process management documentation](/docs/processes/process-management.md#changing-the-procfile-location) for more information on how to configure the `Procfile` path for your application. + - For deploys via `git:from-image`, the `Procfile` file will be extracted from the source image, respecting the value of `procfile-path`. - The existing `pre-restore` hook has been renamed to `scheduler-pre-restore`. There is a new `pre-restore` hook that is triggered within the `ps:restore` command prior to restoring any apps. - Nginx init commands are now performed via systemctl on Ubuntu systems when `/usr/bin/systemctl` is available. ## Removals - The `DOKKU_WAIT_TO_RETIRE` environment variable has been migrated to a `checks` property named `wait-to-retire` and will be ignored if set as an environment variable. -- The `Procfile` is now extracted when source code is extracted for a build and not from the built image. Users can specify alternative paths via the `procfile-path` property of the `ps` plugin. See the [process management documentation](/docs/processes/process-management.md#changing-the-procfile-location) for more information on how to configure the `Procfile` path for your application. - The `domains-setup` trigger has been removed. Initial app domains will now be automatically setup during app creation. - The `URLS` file containing generated urls for an app is no longer generated or referenced. Users should retrieve app urls via the new `domains-urls` plugin trigger. - The common function `get_app_urls` has been removed. Users should retrieve app urls via the new `domains-urls` plugin trigger. diff --git a/docs/deployment/methods/git.md b/docs/deployment/methods/git.md index 444b181e6..6508f8f31 100644 --- a/docs/deployment/methods/git.md +++ b/docs/deployment/methods/git.md @@ -159,6 +159,18 @@ dokku git:from-image node-js-app dokku/node-js-getting-started:latest "Camila" " If the image is a private image that requires a docker login to access, the `registry:login` command should be used to log into the registry. See the [registry documentation](/docs/advanced-usage/registry-management.md#logging-into-a-registry) for more details on this process. +Building an app from an image will result in the following files being extracted from the source image (with all custom paths specified for each file being respected): + +- nginx.conf.sigil +- Procfile + +In the case where the repository is later modified to manually add any of the above files and deployed via `git push`, the files will still be extracted from the initial source image. To avoid this, please clear the `source-image` git property. It will be set back to the original source image on any subsequent `git:from-image` calls. + +```shell +# sets an empty value +dokku git:set node-js-app source-image +``` + Finally, certain images may require a custom build context in order for `ONBUILD ADD` and `ONBUILD COPY` statements to succeed. A custom build context can be specified via the `--build-dir` flag. All files in the specified `build-dir` will be copied into the repository for use within the `docker build` process. The build context _must_ be specified on each deploy, and is not otherwise persisted between builds. ```shell diff --git a/plugins/git/git-from-image b/plugins/git/git-from-image index 009f3feec..3da05f3a1 100755 --- a/plugins/git/git-from-image +++ b/plugins/git/git-from-image @@ -1,5 +1,6 @@ #!/usr/bin/env bash source "$PLUGIN_CORE_AVAILABLE_PATH/common/functions" +source "$PLUGIN_CORE_AVAILABLE_PATH/common/property-functions" set -eo pipefail [[ $DOKKU_TRACE ]] && set -x @@ -33,7 +34,7 @@ trigger-git-git-from-image() { dokku_log_info1 "Image exists on host, skipping pull" fi - export DOKKU_BUILDING_FROM_IMAGE="$DOCKER_IMAGE" + fn-plugin-property-write "git" "$APP" "source-image" "$DOCKER_IMAGE" plugn trigger git-from-directory "$APP" "$TMP_WORK_DIR" "$USER_NAME" "$USER_EMAIL" } diff --git a/plugins/git/internal-functions b/plugins/git/internal-functions index d2ae423c0..5813d512b 100755 --- a/plugins/git/internal-functions +++ b/plugins/git/internal-functions @@ -256,6 +256,7 @@ cmd-git-report-single() { "--git-keep-git-dir: $(fn-plugin-property-get "git" "$APP" "keep-git-dir" "false")" "--git-rev-env-var: $(fn-plugin-property-get "git" "$APP" "rev-env-var" "GIT_REV")" "--git-sha: $(fn-git-cmd "$APP_ROOT" rev-parse --short HEAD 2>/dev/null || false)" + "--git-source-image: $(fn-git-source-image "$APP")" "--git-last-updated-at: $(fn-git-last-updated-at "$APP")" ) @@ -544,6 +545,13 @@ fn-git-setup-build-dir-submodules() { fi } +fn-git-source-image() { + declare desc="retrieve the source-image for a given application" + declare APP="$1" + + fn-plugin-property-get "git" "$APP" "source-image" "" +} + fn-git-use-worktree() { declare desc="detects whether to use git worktree" local GIT_VERSION MAJOR_VERSION MINOR_VERSION diff --git a/plugins/git/subcommands/set b/plugins/git/subcommands/set index 394c9e3d5..50bb3d180 100755 --- a/plugins/git/subcommands/set +++ b/plugins/git/subcommands/set @@ -9,13 +9,13 @@ cmd-git-set() { declare cmd="git:set" [[ "$1" == "$cmd" ]] && shift 1 declare APP="$1" KEY="$2" VALUE="$3" - local VALID_KEYS=("deploy-branch" "keep-git-dir" "rev-env-var") + local VALID_KEYS=("deploy-branch" "keep-git-dir" "rev-env-var" "source-image") [[ "$APP" == "--global" ]] || verify_app_name "$APP" [[ -z "$KEY" ]] && dokku_log_fail "No key specified" if ! fn-in-array "$KEY" "${VALID_KEYS[@]}"; then - dokku_log_fail "Invalid key specified, valid keys include: deploy-branch, keep-git-dir, rev-env-var" + dokku_log_fail "Invalid key specified, valid keys include: deploy-branch, keep-git-dir, rev-env-var, source-image" fi if [[ -n "$VALUE" ]]; then diff --git a/plugins/nginx-vhosts/core-post-extract b/plugins/nginx-vhosts/core-post-extract index c2d4c74be..70b7d6a09 100755 --- a/plugins/nginx-vhosts/core-post-extract +++ b/plugins/nginx-vhosts/core-post-extract @@ -41,9 +41,11 @@ trigger-nginx-vhosts-core-post-extract() { declare trigger="post-extract" declare APP="$1" SOURCECODE_WORK_DIR="$2" local CONF_SIGIL_PATH="$(fn-nginx-computed-nginx-conf-sigil-path "$APP")" + local app_source_image - if [[ -n "$DOKKU_BUILDING_FROM_IMAGE" ]]; then - fn-nginx-vhosts-copy-from-image "$APP" "$DOKKU_BUILDING_FROM_IMAGE" "$CONF_SIGIL_PATH" + app_source_image="$(plugn trigger git-get-property "$APP" "source-image")" + if [[ -n "$app_source_image" ]]; then + fn-nginx-vhosts-copy-from-image "$APP" "$app_source_image" "$CONF_SIGIL_PATH" else fn-nginx-vhosts-copy-from-directory "$APP" "$SOURCECODE_WORK_DIR" "$CONF_SIGIL_PATH" fi diff --git a/plugins/ps/triggers.go b/plugins/ps/triggers.go index e020c1f4a..ee3f133c8 100644 --- a/plugins/ps/triggers.go +++ b/plugins/ps/triggers.go @@ -74,7 +74,10 @@ func TriggerCorePostExtract(appName string, sourceWorkDir string) error { } processSpecificProcfile := fmt.Sprintf("%s.%s", existingProcfile, os.Getenv("DOKKU_PID")) - if os.Getenv("DOKKU_BUILDING_FROM_IMAGE") == "" { + b, _ := common.PlugnTriggerOutput("git-get-property", []string{appName, "source-image"}...) + appSourceImage := strings.TrimSpace(string(b[:])) + + if appSourceImage == "" { repoProcfilePath := path.Join(sourceWorkDir, procfilePath) if !common.FileExists(repoProcfilePath) { return common.TouchFile(fmt.Sprintf("%s.missing", processSpecificProcfile)) @@ -84,13 +87,12 @@ func TriggerCorePostExtract(appName string, sourceWorkDir string) error { return fmt.Errorf("Unable to extract Procfile: %v", err.Error()) } } else { - if err := common.CopyFromImage(appName, os.Getenv("DOKKU_BUILDING_FROM_IMAGE"), procfilePath, processSpecificProcfile); err != nil { + if err := common.CopyFromImage(appName, appSourceImage, procfilePath, processSpecificProcfile); err != nil { return common.TouchFile(fmt.Sprintf("%s.missing", processSpecificProcfile)) } } - b, err := sh.Command("procfile-util", "check", "-P", processSpecificProcfile).CombinedOutput() - if err != nil { + if b, err := sh.Command("procfile-util", "check", "-P", processSpecificProcfile).CombinedOutput(); err != nil { return fmt.Errorf(strings.TrimSpace(string(b[:]))) } return nil diff --git a/tests/apps/python/alt.Dockerfile b/tests/apps/python/alt.Dockerfile new file mode 100644 index 000000000..289b02f6a --- /dev/null +++ b/tests/apps/python/alt.Dockerfile @@ -0,0 +1,5 @@ +FROM python:3.11.0-buster + +WORKDIR /app + +COPY . /app diff --git a/tests/unit/nginx-vhosts_12.bats b/tests/unit/nginx-vhosts_12.bats new file mode 100644 index 000000000..d9d1dcda3 --- /dev/null +++ b/tests/unit/nginx-vhosts_12.bats @@ -0,0 +1,129 @@ +#!/usr/bin/env bats + +load test_helper + +setup() { + global_setup + [[ -f "$DOKKU_ROOT/VHOST" ]] && cp -fp "$DOKKU_ROOT/VHOST" "$DOKKU_ROOT/VHOST.bak" + create_app +} + +teardown() { + destroy_app 0 $TEST_APP + [[ -f "$DOKKU_ROOT/VHOST.bak" ]] && mv "$DOKKU_ROOT/VHOST.bak" "$DOKKU_ROOT/VHOST" && chown dokku:dokku "$DOKKU_ROOT/VHOST" + global_teardown +} + +@test "(nginx-vhosts) git:from-image nginx.conf.sigil" { + local CUSTOM_TMP=$(mktemp -d "/tmp/dokku.me.XXXXX") + trap 'popd &>/dev/null || true; rm -rf "$CUSTOM_TMP"' INT TERM + rmdir "$CUSTOM_TMP" && cp -r "${BATS_TEST_DIRNAME}/../../tests/apps/python" "$CUSTOM_TMP" + + pushd $CUSTOM_TMP + custom_nginx_template "$TEST_APP" "$CUSTOM_TMP" + + run /bin/bash -c "dokku nginx:set --global nginx-conf-sigil-path" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "git init" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "git add nginx.conf.sigil" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "git commit -m 'Add custom nginx.conf.sigil'" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "docker image build -t dokku-test/$TEST_APP:latest -f $CUSTOM_TMP/alt.Dockerfile $CUSTOM_TMP" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku git:from-image $TEST_APP dokku-test/$TEST_APP:latest" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "Overriding default nginx.conf with detected nginx.conf.sigil" + + run /bin/bash -c "dokku nginx:set --global nginx-conf-sigil-path dokku/nginx.conf.sigil" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku proxy:report $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "Overriding default nginx.conf with detected nginx.conf.sigil" 0 + + run /bin/bash -c "mkdir -p $CUSTOM_TMP/dokku" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "mv $CUSTOM_TMP/nginx.conf.sigil $CUSTOM_TMP/dokku/nginx.conf.sigil" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "git add ." + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "git commit -m 'Move the nginx.conf.sigil'" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "docker image build -t dokku-test/$TEST_APP:v2 -f $CUSTOM_TMP/alt.Dockerfile $CUSTOM_TMP" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku git:from-image $TEST_APP dokku-test/$TEST_APP:v2" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "Overriding default nginx.conf with detected nginx.conf.sigil" + + run /bin/bash -c "dokku nginx:set --global nginx-conf-sigil-path" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "Overriding default nginx.conf with detected nginx.conf.sigil" 0 + + run /bin/bash -c "dokku nginx:set $TEST_APP nginx-conf-sigil-path dokku/nginx.conf.sigil" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "Overriding default nginx.conf with detected nginx.conf.sigil" + + run /bin/bash -c "docker image rm dokku-test/$TEST_APP:latest dokku-test/$TEST_APP:v2" + echo "output: $output" + echo "status: $status" + assert_success +}