Merge pull request #3628 from dokku/3611-windows-newlines

Handle file copying in a secure and reliable fashion

Makefile

@@ -119,12 +119,15 @@ plugin-dependencies: plugn procfile-util
 plugins: plugn procfile-util docker
 	sudo -E dokku plugin:install --core
 
-dependencies: apt-update sshcommand plugn procfile-util docker help2man man-db sigil
+dependencies: apt-update sshcommand plugn procfile-util docker help2man man-db sigil dos2unix
 	$(MAKE) -e stack
 
 apt-update:
 	apt-get update -qq
 
+dos2unix:
+	apt-get install -qq -y dos2unix
+
 help2man:
 	apt-get install -qq -y help2man
 

debian/control

@@ -3,7 +3,7 @@ Version: 0.17.9
 Section: web
 Priority: optional
 Architecture: amd64
-Depends: locales, git, curl, man-db, netcat, sshcommand (>= 0.6.0), gliderlabs-sigil, docker-engine-cs (>= 1.7.1) | docker-engine (>= 1.7.1) | docker-io (>= 1.7.1)  | docker.io (>= 1.7.1) | docker-ce | docker-ee, net-tools, software-properties-common, procfile-util, python-software-properties | python3-software-properties, rsyslog
+Depends: locales, git, curl, man-db, netcat, sshcommand (>= 0.6.0), gliderlabs-sigil, docker-engine-cs (>= 1.7.1) | docker-engine (>= 1.7.1) | docker-io (>= 1.7.1)  | docker.io (>= 1.7.1) | docker-ce | docker-ee, net-tools, software-properties-common, procfile-util, python-software-properties | python3-software-properties, rsyslog, dos2unix
 Recommends: herokuish (>= 0.3.4), parallel, dokku-update
 Pre-Depends: nginx (>= 1.8.0) | openresty, dnsutils, cgroupfs-mount | cgroup-lite, plugn (>= 0.3.0), sudo, python2.7, debconf
 Maintainer: Jose Diaz-Gonzalez <dokku@josediazgonzalez.com>

docs/development/plugin-creation.md

@@ -187,6 +187,23 @@ local DOKKU_COMMIT_ARGS=("--change" "LABEL org.label-schema.schema-version=1.0"
 "$DOCKER_BIN" run "--label=com.dokku.app-name=${APP}" $DOKKU_GLOBAL_RUN_ARGS ...
 ```
 
+## Copy files from the built image using `copy_from_image`
+
+Avoid copying files from running containers as these files may change over time. Instead copy files from the image built during the deploy process. This can be done via the `copy_from_image` helper function. This will correctly handle various corner cases in copying files from an image.
+
+```shell
+source "$PLUGIN_CORE_AVAILABLE_PATH/common/functions"
+
+local TMP_FILE=$(mktemp "/tmp/dokku-${FUNCNAME[0]}.XXXX")
+trap "rm -rf '$TMP_FILE' >/dev/null" RETURN INT TERM
+
+local IMAGE_TAG="$(get_running_image_tag "$APP")"
+local IMAGE=$(get_deploying_app_image_name "$APP" "$IMAGE_TAG")
+copy_from_image "$IMAGE" "file-being-copied" "$TMP_FILE" 2>/dev/null
+```
+
+Files are copied from the `/app` directory - for images built via buildpacks - or `WORKDIR` - for images built via Dockerfile.
+
 ## Avoid calling the `dokku` binary directly
 
 > New as of 0.6.0

plugins/app-json/internal-functions

@@ -12,7 +12,7 @@ get_phase_script() {
   local APP_JSON_FILE="$GET_PHASE_SCRIPT_TMP_WORK_DIR/app.json"
   trap "rm -rf '$GET_PHASE_SCRIPT_TMP_WORK_DIR' >/dev/null" RETURN INT TERM
 
-  copy_from_image "$IMAGE" "app.json" "$GET_PHASE_SCRIPT_TMP_WORK_DIR" 2>/dev/null || true
+  copy_from_image "$IMAGE" "app.json" "$APP_JSON_FILE" 2>/dev/null || true
 
   if [[ ! -f "$APP_JSON_FILE" ]]; then
     return 0

plugins/common/functions

@@ -371,7 +371,7 @@ parse_args() {
 
 copy_from_image() {
   declare desc="copy file from named image to destination"
-  declare IMAGE="$1" SRC_FILE="$2" DST_DIR="$3"
+  declare IMAGE="$1" SRC_FILE="$2" DST_FILE="$3"
   local WORK_DIR=""
   verify_app_name "$APP"
 
@@ -389,12 +389,28 @@ copy_from_image() {
         SRC_FILE="${WORKDIR}/${SRC_FILE}"
       fi
     fi
+
+    TMP_FILE_COMMAND_OUTPUT=$(mktemp "/tmp/dokku-${FUNCNAME[0]}.XXXX")
+    trap "rm -rf '$TMP_FILE_COMMAND_OUTPUT' >/dev/null || true" RETURN
+
     local CID=$("$DOCKER_BIN" create "${DOCKER_CREATE_LABEL_ARGS[@]}" $DOKKU_GLOBAL_RUN_ARGS "$IMAGE")
-    if ! "$DOCKER_BIN" cp "$CID:$SRC_FILE" "$DST_DIR"; then
-      "$DOCKER_BIN" rm -f "$CID" &>/dev/null
+    "$DOCKER_BIN" cp "$CID:$SRC_FILE" "$TMP_FILE_COMMAND_OUTPUT" 2>/dev/null || true
+    "$DOCKER_BIN" rm -f "$CID" &>/dev/null
+
+    # docker cp exits with status 1 when run as non-root user when it tries to chown the file
+    # after successfully copying the file. Thus, we suppress stderr.
+    # ref: https://github.com/dotcloud/docker/issues/3986
+    if [[ ! -s "$TMP_FILE_COMMAND_OUTPUT" ]]; then
       return 1
     fi
-    "$DOCKER_BIN" rm -f "$CID" &>/dev/null
+
+    # workaround for CHECKS file when owner is root. seems to only happen when running inside docker
+    dos2unix -l <"$TMP_FILE_COMMAND_OUTPUT" >"$DST_FILE"
+
+    # add trailing newline for certain places where file parsing depends on it
+    if [[ "$(tail -c1 "$DST_FILE")" != "" ]]; then
+      echo "" >>"$DST_FILE"
+    fi
   else
     return 1
   fi

plugins/nginx-vhosts/functions

@@ -185,12 +185,12 @@ get_custom_nginx_template() {
   declare desc="attempts to copy custom nginx template from app image"
   local APP="$1"
   verify_app_name "$APP"
-  local DESTINATION="$2"
+  local DESTINATION_FILE="$2"
   local IMAGE_TAG="$(get_running_image_tag "$APP")"
   local IMAGE=$(get_deploying_app_image_name "$APP" "$IMAGE_TAG")
   local NGINX_TEMPLATE_NAME="nginx.conf.sigil"
 
-  copy_from_image "$IMAGE" "$NGINX_TEMPLATE_NAME" "$DESTINATION" 2>/dev/null || true
+  copy_from_image "$IMAGE" "$NGINX_TEMPLATE_NAME" "$DESTINATION_FILE" 2>/dev/null || true
 }
 
 is_spdy_enabled() {

plugins/ps/functions

@@ -62,7 +62,7 @@ generate_scale_file() {
   local DOKKU_PROCFILE="$DOKKU_ROOT/$APP/DOKKU_PROCFILE"
   verify_app_name "$APP"
 
-  if copy_from_image "$IMAGE" "DOKKU_SCALE" "$DOKKU_ROOT/$APP" >/dev/null 2>&1; then
+  if copy_from_image "$IMAGE" "DOKKU_SCALE" "$DOKKU_SCALE_FILE" 2>/dev/null; then
     cp "$DOKKU_SCALE_FILE" "${DOKKU_SCALE_FILE}.extracted"
   else
     rm -f "${DOKKU_SCALE_FILE}.extracted"

plugins/scheduler-docker-local/check-deploy

@@ -80,14 +80,11 @@ scheduler-docker-local-check-deploy() {
   # use this number of retries for checks
   local ATTEMPTS="${DOKKU_CHECKS_ATTEMPTS:-5}"
 
-  # try to copy CHECKS from container if not in APP dir & quit gracefully if it doesn't exist
-  # docker cp exits with status 1 when run as non-root user when it tries to chown the file
-  # after successfully copying the file. Thus, we suppress stderr.
-  # ref: https://github.com/dotcloud/docker/issues/3986
   local CHECK_DEPLOY_TMP_WORK_DIR=$(mktemp -d "/tmp/dokku-${FUNCNAME[0]}.XXXX")
-  "$DOCKER_BIN" cp "$DOKKU_APP_CONTAINER_ID:/app/CHECKS" "$CHECK_DEPLOY_TMP_WORK_DIR" 2>/dev/null || true
-
   local CHECKS_FILENAME=${CHECK_DEPLOY_TMP_WORK_DIR}/CHECKS
+  local IMAGE_TAG="$(get_running_image_tag "$APP")"
+  local IMAGE=$(get_deploying_app_image_name "$APP" "$IMAGE_TAG")
+  copy_from_image "$IMAGE" "CHECKS" "$CHECKS_FILENAME" 2>/dev/null || true
 
   checks_check_deploy_cleanup() {
     declare desc="cleans up CHECK_DEPLOY_TMP_WORK_DIR and print container output"

rpm.mk

@@ -63,6 +63,7 @@ endif
 		--depends '/usr/bin/docker' \
 		--depends 'bind-utils' \
 		--depends 'curl' \
+		--depends 'dos2unix' \
 		--depends 'git' \
 		--depends 'gliderlabs-sigil' \
 		--depends 'man-db' \

tests/ci/setup.sh

@@ -29,7 +29,7 @@ install_dependencies() {
 
   sudo add-apt-repository -y ppa:nginx/stable
   sudo apt-get update
-  sudo apt-get -qq -y install cgroupfs-mount nginx
+  sudo apt-get -qq -y install cgroupfs-mount dos2unix nginx
   sudo cp "${ROOT_DIR}/tests/dhparam.pem" /etc/nginx/dhparam.pem
 
   sudo dpkg -i "${ROOT_DIR}/build/$HEROKUISH_PACKAGE_NAME" \