Merge pull request #5532 from dokku/5518-from-image

Fix issue where git:from-image deploys may result in extracted files not being respected

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.

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
@@ -157,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

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

plugins/20_events/git-get-property

@@ -0,0 +1 @@
+hook

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
 
@@ -26,10 +27,14 @@ 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"
+  fn-plugin-property-write "git" "$APP" "source-image" "$DOCKER_IMAGE"
   plugn trigger git-from-directory "$APP" "$TMP_WORK_DIR" "$USER_NAME" "$USER_EMAIL"
 }
 

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 "$@"

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

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

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

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"
 

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

tests/apps/python/alt.Dockerfile

@@ -0,0 +1,5 @@
+FROM python:3.11.0-buster
+
+WORKDIR /app
+
+COPY . /app

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
+}