Merge pull request #6632 from dokku/remove-disable-chown

Simplify file ownership in the container by setting the process ownership during the build process

docs/appendices/0.34.0-migration-guide.md

@@ -0,0 +1,5 @@
+# 0.34.0 Migration Guide
+
+## Removals
+
+- The `disable-chown` property of the `scheduler-docker-local` plugin has been removed. Mounted paths will no longer have file permissions changed during the pre-deploy phase. Files baked into the container image for herokuish builds will always be owned by the correct user.

docs/deployment/schedulers/docker-local.md

@@ -32,20 +32,6 @@ dokku scheduler:set node-js-app selected
 
 Many builders only produce amd64-compatible images. The docker-local scheduler will automatically detect these and run them via the `--platform=linux/amd64` on arm64 deploy targets.
 
-### Disabling chown of persistent storage
-
-The `scheduler-docker-local` plugin will ensure your storage mounts are owned by either `herokuishuser` or the overridden value you have set in `DOKKU_APP_USER`. You may disable this by running the following `scheduler-docker-local:set` command for your application:
-
-```shell
-dokku scheduler-docker-local:set node-js-app disable-chown true
-```
-
-Once set, you may re-enable it by setting a blank value for `disable-chown`:
-
-```shell
-dokku scheduler-docker-local:set node-js-app disable-chown
-```
-
 ### Disabling the init process
 
 The `scheduler-docker-local` injects an init process by default via the `--init`. For some apps - such as those where the built docker image uses S6 as the init - this may be undesirable and cause issues with process starts. You may disable this by running the following `scheduler-docker-local:set` command for your application:

docs/getting-started/upgrading/index.md

@@ -18,6 +18,7 @@ Docker releases updates periodically to their engine. We recommend reading their
 
 Before upgrading, check the migration guides to get comfortable with new features and prepare your deployment to be upgraded.
 
+- [Upgrading to 0.34](/docs/appendices/0.34.0-migration-guide.md)
 - [Upgrading to 0.33](/docs/appendices/0.33.0-migration-guide.md)
 - [Upgrading to 0.32](/docs/appendices/0.32.0-migration-guide.md)
 - [Upgrading to 0.31](/docs/appendices/0.31.0-migration-guide.md)

plugins/builder-herokuish/builder-release

@@ -33,7 +33,10 @@ trigger-builder-herokuish-builder-release() {
     DOCKER_BUILD_ARGS+=("--platform=linux/amd64")
   fi
 
-  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" -t "$IMAGE" "$TMP_WORK_DIR"; then
+  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-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

plugins/builder-herokuish/dockerfiles/builder-release.Dockerfile

@@ -1,4 +1,8 @@
 ARG APP_IMAGE
 FROM $APP_IMAGE
 
-COPY 00-global-env.sh 01-app-env.sh /app/.profile.d/
+ARG DOKKU_APP_USER herokuishuser
+COPY --chown=$DOKKU_APP_USER 00-global-env.sh 01-app-env.sh /app/.profile.d/
+RUN chown -R "$DOKKU_APP_USER:$DOKKU_APP_USER" /app
+USER $DOKKU_APP_USER
+ENV HEROKUISH_SETUIDGUID false

plugins/builder-herokuish/dockerfiles/copy-source.Dockerfile

@@ -4,4 +4,5 @@ FROM $APP_IMAGE
 ARG DOKKU_APP_USER herokuishuser
 RUN USER=$DOKKU_APP_USER /exec true
 COPY --chown=$DOKKU_APP_USER . /app
-WORKDIR /app
+WORKDIR /app
+ENV HEROKUISH_DISABLE_CHOWN true

plugins/builder-herokuish/dockerfiles/pre-build.Dockerfile

@@ -1,5 +1,5 @@
 ARG APP_IMAGE
 FROM $APP_IMAGE
 
-COPY .env.d /tmp/env
-COPY .env /app/.env
+COPY --chown=$DOKKU_APP_USER .env.d /tmp/env
+COPY --chown=$DOKKU_APP_USER .env /app/.env

plugins/scheduler-docker-local/internal-functions

@@ -36,7 +36,6 @@ cmd-scheduler-docker-local-report-single() {
   fi
   verify_app_name "$APP"
   local flag_map=(
-    "--scheduler-docker-local-disable-chown: $(fn-plugin-property-get "scheduler-docker-local" "$APP" "disable-chown" "")"
     "--scheduler-docker-local-init-process: $(fn-plugin-property-get "scheduler-docker-local" "$APP" "init-process" "true")"
     "--scheduler-docker-local-parallel-schedule-count: $(fn-plugin-property-get "scheduler-docker-local" "$APP" "parallel-schedule-count" "")"
   )

plugins/scheduler-docker-local/pre-deploy

@@ -16,52 +16,9 @@ trigger-scheduler-docker-local-pre-deploy() {
     return
   fi
 
-  scheduler-docker-local-pre-deploy-chown-app "$APP" "$IMAGE_TAG"
   scheduler-docker-local-pre-deploy-precheck "$APP"
 }
 
-scheduler-docker-local-pre-deploy-chown-app() {
-  declare desc="Runs chown against the /app directory for herokuish images"
-  declare APP="$1" IMAGE_TAG="$2"
-  local DOCKER_RUN_LABEL_ARGS="--label=com.dokku.app-name=$APP"
-  local IMAGE DISABLE_CHOWN DOCKER_ARGS DOKKU_APP_TYPE DOKKU_APP_USER APP_PATHS CONTAINER_PATHS
-  declare -a ARG_ARRAY
-
-  IMAGE=$(get_app_image_name "$APP" "$IMAGE_TAG")
-
-  DOKKU_APP_TYPE=$(config_get "$APP" DOKKU_APP_TYPE || true)
-  DOKKU_APP_USER=$(config_get "$APP" DOKKU_APP_USER || true)
-  DOKKU_APP_USER=${DOKKU_APP_USER:="herokuishuser"}
-  APP_PATHS=$(plugn trigger storage-list "$APP" "deploy")
-
-  if [[ -n "$APP_PATHS" ]]; then
-    CONTAINER_PATHS=$(echo "$APP_PATHS" | awk -F ':' '{ print $2 }' | xargs)
-    DOCKER_ARGS=$(: | plugn trigger docker-args-deploy "$APP" "$IMAGE_TAG")
-
-    filterdArgs=("--cpus" "--gpus" "--memory" "--memory-reservation" "--memory-swap" "--publish" "--publish-all" "-p" "-P" "--restart")
-    for filteredArg in "${filterdArgs[@]}"; do
-      # shellcheck disable=SC2001
-      DOCKER_ARGS=$(sed -e "s/$filteredArg=[[:graph:]]\+[[:blank:]]\?//g" <<<"$DOCKER_ARGS")
-
-      # shellcheck disable=SC2001
-      DOCKER_ARGS=$(sed -e "s/$filteredArg\+[[:blank:]][[:graph:]]\+[[:blank:]]\?//g" <<<"$DOCKER_ARGS")
-    done
-
-    eval "ARG_ARRAY=($DOCKER_ARGS)"
-  fi
-
-  if [[ "$DOKKU_APP_TYPE" != "herokuish" ]] || [[ -z "$CONTAINER_PATHS" ]]; then
-    return
-  fi
-
-  DISABLE_CHOWN="$(fn-plugin-property-get "scheduler-docker-local" "$APP" "disable-chown" "")"
-  if [[ "$DISABLE_CHOWN" == "true" ]]; then
-    return
-  fi
-
-  "$DOCKER_BIN" container run --rm "${DOCKER_RUN_LABEL_ARGS[@]}" $DOKKU_GLOBAL_RUN_ARGS "${ARG_ARRAY[@]}" $IMAGE /bin/bash -c "find $CONTAINER_PATHS -not -user $DOKKU_APP_USER -print0 | xargs -0 -r chown -R $DOKKU_APP_USER" || true
-}
-
 scheduler-docker-local-pre-deploy-precheck() {
   declare desc="Outputs the checks messages if necessary"
   declare APP="$1"

plugins/scheduler-docker-local/subcommands/set

@@ -9,13 +9,13 @@ cmd-scheduler-docker-local-set() {
   declare cmd="scheduler-docker-local:set"
   [[ "$1" == "$cmd" ]] && shift 1
   declare APP="$1" KEY="$2" VALUE="$3"
-  local VALID_KEYS=("disable-chown" "init-process" "parallel-schedule-count")
+  local VALID_KEYS=("init-process" "parallel-schedule-count")
   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: disable-chown, init-process, parallel-schedule-count"
+    dokku_log_fail "Invalid key specified, valid keys include: init-process, parallel-schedule-count"
   fi
 
   if [[ -n "$VALUE" ]]; then

tests/unit/apps_1.bats

@@ -194,10 +194,6 @@ teardown() {
   echo "output: $output"
   echo "status: $status"
   assert_success
-  run /bin/bash -c "dokku scheduler-docker-local:set  $TEST_APP disable-chown true"
-  echo "output: $output"
-  echo "status: $status"
-  assert_success
 
   run /bin/bash -c "dokku apps:rename $TEST_APP great-test-name"
   echo "output: $output"
@@ -228,11 +224,6 @@ teardown() {
   echo "status: $status"
   assert_success
   assert_output "100"
-  run /bin/bash -c "dokku scheduler-docker-local:report great-test-name --scheduler-docker-local-disable-chown"
-  echo "output: $output"
-  echo "status: $status"
-  assert_success
-  assert_output "true"
 
   run /bin/bash -c "dokku --force apps:destroy great-test-name"
   echo "output: $output"

tests/unit/apps_2.bats

@@ -59,10 +59,6 @@ teardown() {
   echo "output: $output"
   echo "status: $status"
   assert_success
-  run /bin/bash -c "dokku scheduler-docker-local:set  $TEST_APP disable-chown true"
-  echo "output: $output"
-  echo "status: $status"
-  assert_success
 
   run /bin/bash -c "dokku apps:clone $TEST_APP great-test-name"
   echo "output: $output"
@@ -103,11 +99,6 @@ teardown() {
   echo "status: $status"
   assert_success
   assert_output "100"
-  run /bin/bash -c "dokku scheduler-docker-local:report great-test-name --scheduler-docker-local-disable-chown"
-  echo "output: $output"
-  echo "status: $status"
-  assert_success
-  assert_output "true"
 
   run /bin/bash -c "dokku --force apps:destroy great-test-name"
   echo "output: $output"