From 59594937f9dde4f3f352f444d171735c1638ff9c Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Fri, 29 May 2026 11:38:29 -0400 Subject: [PATCH] feat: split stack property into per-builder properties The `stack` property lived on the `buildpacks` plugin but was shared by both the herokuish and pack builders, so a value valid for one builder broke the other. Each builder now owns its own `stack` property managed via `builder-herokuish:set` and `builder-pack:set`, while `buildpacks:set-property stack` is deprecated and forwards the value to the builder detected from it; existing values migrate automatically on upgrade and changing a builder's stack clears that builder's cache. --- docs/README.md | 1 + docs/appendices/0.39.0-migration-guide.md | 21 ++++++ .../builders/buildpack-management.md | 19 +---- .../builders/cloud-native-buildpacks.md | 37 ++++++++-- .../builders/herokuish-buildpacks.md | 29 ++++++-- plugins/builder-herokuish/builder-build | 4 +- plugins/builder-herokuish/internal-functions | 24 ++++++ plugins/builder-herokuish/report.go | 30 ++++++++ plugins/builder-herokuish/subcommands/set | 12 ++- plugins/builder-pack/builder-build | 8 +- plugins/builder-pack/internal-functions | 26 +++++++ plugins/builder-pack/report.go | 30 ++++++++ plugins/builder-pack/subcommands/set | 12 ++- plugins/buildpacks/.gitignore | 1 - plugins/buildpacks/Makefile | 2 +- plugins/buildpacks/buildpacks.go | 23 ++++-- plugins/buildpacks/report.go | 39 +--------- plugins/buildpacks/src/triggers/triggers.go | 3 - plugins/buildpacks/subcommands.go | 73 ++++++++++++------ plugins/buildpacks/triggers.go | 61 ++++++++------- tests/unit/builder-herokuish.bats | 44 +++++++++++ tests/unit/builder-pack.bats | 62 +++++++++++++++- tests/unit/buildpacks.bats | 74 +++++++------------ 23 files changed, 453 insertions(+), 182 deletions(-) create mode 100644 docs/appendices/0.39.0-migration-guide.md diff --git a/docs/README.md b/docs/README.md index e0e518706..8178c6227 100644 --- a/docs/README.md +++ b/docs/README.md @@ -123,6 +123,7 @@ This documentation covers the installation, configuration, and usage of Dokku - ### Migration Guides +- [0.39.0](appendices/0.39.0-migration-guide.md) - [0.38.0](appendices/0.38.0-migration-guide.md) - [0.37.0](appendices/0.37.0-migration-guide.md) - [0.36.0](appendices/0.36.0-migration-guide.md) diff --git a/docs/appendices/0.39.0-migration-guide.md b/docs/appendices/0.39.0-migration-guide.md new file mode 100644 index 000000000..420e5f7b4 --- /dev/null +++ b/docs/appendices/0.39.0-migration-guide.md @@ -0,0 +1,21 @@ +# 0.39.0 Migration Guide + +## Changes + +- The buildpack `stack` property has been split out of the `buildpacks` plugin and into the two builder plugins that consume it. Previously a single `stack` value set via `buildpacks:set-property` was shared by both the herokuish and CNB (`pack`) builders, even though a herokuish stack image is not a valid CNB builder and vice versa. The herokuish stack is now managed via `builder-herokuish:set stack ` (default `gliderlabs/herokuish:latest-24`), and the CNB builder is managed via `builder-pack:set stack ` (default `heroku/builder:24`). Both support the `--global` scope and expose `stack`, `global-stack`, and `computed-stack` keys via their respective `:report` commands. +- The `buildpacks:set-property stack` command is deprecated but continues to work. It emits a deprecation warning and forwards the value to the appropriate builder: values referencing a herokuish image (the value contains `herokuish`) are written to `builder-herokuish`, and all other values are written to `builder-pack`. Unsetting (`buildpacks:set-property stack`) clears the value on both builders. +- The `--buildpacks-stack`, `--buildpacks-global-stack`, and `--buildpacks-computed-stack` report flags (and the corresponding `stack`, `global-stack`, `computed-stack` JSON keys) have been removed from `buildpacks:report`. Use `builder-herokuish:report` and `builder-pack:report` instead. The `buildpacks:report` command continues to report the buildpacks `list`. +- The internal `buildpack-stack-name` plugin trigger has been removed. Each builder now reads its own `stack` property directly during the build. +- Changing a builder's `stack` now clears that builder's build cache so the next deploy starts fresh. For herokuish this removes the `cache-` Docker volume; for pack the next `pack build` runs with `--clear-cache`. + +### Stack property migrated to builder plugins + +Existing `stack` values stored via `buildpacks:set-property` are migrated automatically the first time `dokku` runs the buildpacks plugin install step after the upgrade. Each per-app and global value is moved to `builder-herokuish` or `builder-pack` based on the same herokuish detection used by the deprecated command, and the old `buildpacks` value is removed. No manual action is required. + +Going forward, configure the stack using the builder commands: + +| Deprecated command | Replacement command | +|---|---| +| `dokku buildpacks:set-property stack ` | `dokku builder-herokuish:set stack ` | +| `dokku buildpacks:set-property stack ` | `dokku builder-pack:set stack ` | +| `dokku buildpacks:set-property --global stack ` | `dokku builder-herokuish:set --global stack ` / `dokku builder-pack:set --global stack ` | diff --git a/docs/deployment/builders/buildpack-management.md b/docs/deployment/builders/buildpack-management.md index 4255960d2..c3cddf540 100644 --- a/docs/deployment/builders/buildpack-management.md +++ b/docs/deployment/builders/buildpack-management.md @@ -178,20 +178,11 @@ dokku buildpacks:report ``` =====> node-js-app buildpacks information - Buildpacks computed stack: gliderlabs/herokuish:v0.7.0-22 - Buildpacks global stack: gliderlabs/herokuish:latest-24 Buildpacks list: https://github.com/heroku/heroku-buildpack-nodejs.git - Buildpacks stack: gliderlabs/herokuish:v0.7.0-20 =====> python-sample buildpacks information - Buildpacks computed stack: gliderlabs/herokuish:latest-24 - Buildpacks global stack: gliderlabs/herokuish:latest-24 Buildpacks list: https://github.com/heroku/heroku-buildpack-nodejs.git,https://github.com/heroku/heroku-buildpack-python.git - Buildpacks stack: =====> ruby-sample buildpacks information - Buildpacks computed stack: gliderlabs/herokuish:latest-24 - Buildpacks global stack: gliderlabs/herokuish:latest-24 Buildpacks list: - Buildpacks stack: ``` You can run the command for a specific app also. @@ -215,11 +206,7 @@ dokku buildpacks:report node-js-app --buildpacks-list ### Settable properties -These properties are managed via `buildpacks:set-property` (the legacy command name for this plugin). +The `buildpacks` plugin has no settable properties of its own. The list of buildpack URLs is managed via the `buildpacks:add`, `buildpacks:set`, `buildpacks:remove`, and `buildpacks:clear` commands. -> [!NOTE] -> The `Report flags` column lists the CLI argument names accepted by `buildpacks:report`. The JSON keys emitted by `buildpacks:report --format json` are the same names with the leading `--buildpacks-` stripped (e.g. `stack`, `global-stack`, `computed-stack`). Legacy keys with the `buildpacks-` prefix are also emitted during the 0.38.x deprecation window and will be removed in a future major release. - -| Property | Scope | Default | Report flags | Description | -|---|---|---|---|---| -| `stack` | app + global | none | `--buildpacks-stack`, `--buildpacks-global-stack`, `--buildpacks-computed-stack` | Herokuish stack image used to compile the app (e.g. `heroku/heroku:24`) | +> [!WARNING] +> The `buildpacks:set-property stack` command is deprecated as of 0.39.0. The `stack` property moved to the `builder-herokuish` and `builder-pack` plugins. Set it via `builder-herokuish:set stack ` (herokuish) or `builder-pack:set stack ` (CNB). The deprecated command still works but emits a warning and forwards the value to the appropriate builder based on whether the value references a herokuish image. diff --git a/docs/deployment/builders/cloud-native-buildpacks.md b/docs/deployment/builders/cloud-native-buildpacks.md index 35caf287d..5b7277551 100644 --- a/docs/deployment/builders/cloud-native-buildpacks.md +++ b/docs/deployment/builders/cloud-native-buildpacks.md @@ -100,14 +100,23 @@ dokku builder-pack:report Builder pack computed projecttoml path: project2.toml Builder pack global projecttoml path: Builder pack projecttoml path: project2.toml + Builder pack computed stack: heroku/builder:24 + Builder pack global stack: + Builder pack stack: =====> python-sample builder-pack information Builder pack computed projecttoml path: project.toml Builder pack global projecttoml path: Builder pack projecttoml path: + Builder pack computed stack: heroku/builder:24 + Builder pack global stack: + Builder pack stack: =====> ruby-sample builder-pack information Builder pack computed projecttoml path: project.toml Builder pack global projecttoml path: Builder pack projecttoml path: + Builder pack computed stack: heroku/builder:24 + Builder pack global stack: + Builder pack stack: ``` The `projecttoml-path` and `global-projecttoml-path` keys hold the raw per-app and global value respectively, and are empty when nothing has been set. The `computed-projecttoml-path` key holds the effective value used at build time, falling back to the global value (where one has been set) and then to the built-in default of `project.toml`. @@ -138,32 +147,35 @@ project2.toml ### Customizing the Buildpack stack builder > [!IMPORTANT] -> New as of 0.23.0 +> New as of 0.23.0. The `stack` property moved from the `buildpacks` plugin to the `builder-pack` plugin in 0.39.0. -The default stack builder in use by CNB buildpacks in Dokku is based on `heroku/builder:24`. Users may desire to switch the stack builder to a custom version, either to update the operating system or to customize packages included with the stack builder. This can be performed via the `buildpacks:set-property` command. +The default stack builder in use by CNB buildpacks in Dokku is based on `heroku/builder:24`. Users may desire to switch the stack builder to a custom version, either to update the operating system or to customize packages included with the stack builder. This can be performed via the `builder-pack:set` command. ```shell -dokku buildpacks:set-property node-js-app stack paketobuildpacks/build:base-cnb +dokku builder-pack:set node-js-app stack paketobuildpacks/build:base-cnb ``` -The specified stack builder can also be unset by omitting the name of the stack builder when calling `buildpacks:set-property`. +The specified stack builder can also be unset by omitting the name of the stack builder when calling `builder-pack:set`. ```shell -dokku buildpacks:set-property node-js-app stack +dokku builder-pack:set node-js-app stack ``` -A change in the stack builder value will execute the `post-stack-set` trigger. +A change in the stack builder value flags the next build to run with `--clear-cache` so a stale pack cache is not reused. Finally, stack builders can be set or unset globally as a fallback. This will take precedence over a globally set `DOKKU_CNB_BUILDER` environment variable (`heroku/builder:24` by default). ```shell # set globally -dokku buildpacks:set-property --global stack paketobuildpacks/build:base-cnb +dokku builder-pack:set --global stack paketobuildpacks/build:base-cnb # unset globally -dokku buildpacks:set-property --global stack +dokku builder-pack:set --global stack ``` +> [!WARNING] +> The `buildpacks:set-property stack` command is deprecated. It still works but emits a deprecation warning and forwards the value to `builder-pack` (when the value is a CNB builder) or `builder-herokuish` (when the value references a herokuish image). + ### Specifying commands via Procfile See the [Procfile documentation](/docs/processes/process-management.md#procfile) for more information on how to specify different processes for your app. @@ -175,3 +187,12 @@ See the [Procfile documentation](/docs/processes/process-management.md#procfile) | Property | Scope | Default | Report flags | Description | |---|---|---|---|---| | `projecttoml-path` | app + global | `project.toml` | `--builder-pack-projecttoml-path`, `--builder-pack-global-projecttoml-path`, `--builder-pack-computed-projecttoml-path` | Path within the app to the CNB `project.toml` manifest, relative to the build root | +| `stack` | app + global | `heroku/builder:24` | `--builder-pack-stack`, `--builder-pack-global-stack`, `--builder-pack-computed-stack` | CNB builder image used to build the app (e.g. `paketobuildpacks/build:base-cnb`) | + +### Internal properties + +The following properties are recorded internally by the builder-pack plugin and are not exposed via `builder-pack:report`: + +| Property | Scope | Description | Source | +|---|---|---|---| +| `clear-cache` | per-app | Transient flag set when the pack `stack` changes. The next `pack build` runs with `--clear-cache` and then the flag is deleted, ensuring a stack change does not reuse a stale pack cache | `plugins/builder-pack/subcommands/set` sets it on stack change; `plugins/builder-pack/builder-build` consumes and deletes it | diff --git a/docs/deployment/builders/herokuish-buildpacks.md b/docs/deployment/builders/herokuish-buildpacks.md index 59c6832b7..944bfc670 100644 --- a/docs/deployment/builders/herokuish-buildpacks.md +++ b/docs/deployment/builders/herokuish-buildpacks.md @@ -46,32 +46,35 @@ dokku builder:set node-js-app selected herokuish ### Customizing the Buildpack stack builder > [!IMPORTANT] -> New as of 0.23.0 +> New as of 0.23.0. The `stack` property moved from the `buildpacks` plugin to the `builder-herokuish` plugin in 0.39.0. -The default stack builder in use by Herokuish buildpacks in Dokku is based on `gliderlabs/herokuish:latest`. Typically, this is installed via an OS package which pulls the requisite Docker image. Users may desire to switch the stack builder to a custom version, either to update the operating system or to customize packages included with the stack builder. This can be performed via the `buildpacks:set-property` command. +The default stack builder in use by Herokuish buildpacks in Dokku is based on `gliderlabs/herokuish:latest`. Typically, this is installed via an OS package which pulls the requisite Docker image. Users may desire to switch the stack builder to a custom version, either to update the operating system or to customize packages included with the stack builder. This can be performed via the `builder-herokuish:set` command. ```shell -dokku buildpacks:set-property node-js-app stack gliderlabs/herokuish:latest +dokku builder-herokuish:set node-js-app stack gliderlabs/herokuish:latest ``` -The specified stack builder can also be unset by omitting the name of the stack builder when calling `buildpacks:set-property`. +The specified stack builder can also be unset by omitting the name of the stack builder when calling `builder-herokuish:set`. ```shell -dokku buildpacks:set-property node-js-app stack +dokku builder-herokuish:set node-js-app stack ``` -A change in the stack builder value will execute the `post-stack-set` trigger. +A change in the stack builder value clears the herokuish build cache so the next deploy starts fresh. Finally, stack builders can be set or unset globally as a fallback. This will take precedence over a globally set `DOKKU_IMAGE` environment variable (`gliderlabs/herokuish:latest-24` by default). ```shell # set globally -dokku buildpacks:set-property --global stack gliderlabs/herokuish:latest +dokku builder-herokuish:set --global stack gliderlabs/herokuish:latest # unset globally -dokku buildpacks:set-property --global stack +dokku builder-herokuish:set --global stack ``` +> [!WARNING] +> The `buildpacks:set-property stack` command is deprecated. It still works but emits a deprecation warning and forwards the value to `builder-herokuish` (when the value references a herokuish image) or `builder-pack` (otherwise). + ### Allowing herokuish for non-amd64 platforms > [!IMPORTANT] @@ -119,14 +122,23 @@ dokku builder-herokuish:report Builder herokuish computed allowed: false Builder herokuish global allowed: Builder herokuish allowed: false + Builder herokuish computed stack: gliderlabs/herokuish:latest-24 + Builder herokuish global stack: + Builder herokuish stack: =====> python-sample builder-herokuish information Builder herokuish computed allowed: true Builder herokuish global allowed: Builder herokuish allowed: + Builder herokuish computed stack: gliderlabs/herokuish:latest-24 + Builder herokuish global stack: + Builder herokuish stack: =====> ruby-sample builder-herokuish information Builder herokuish computed allowed: true Builder herokuish global allowed: Builder herokuish allowed: + Builder herokuish computed stack: gliderlabs/herokuish:latest-24 + Builder herokuish global stack: + Builder herokuish stack: ``` The `allowed` and `global-allowed` keys hold the raw per-app and global value respectively, and are empty when nothing has been set. The `computed-allowed` key holds the effective value used at build time, falling back to the global value (where one has been set) and then to the built-in default (`true` on amd64, `false` otherwise). @@ -218,3 +230,4 @@ See the [buildpack management documentation](/docs/deployment/builders/buildpack | Property | Scope | Default | Report flags | Description | |---|---|---|---|---| | `allowed` | app + global | `true` | `--builder-herokuish-allowed`, `--builder-herokuish-global-allowed`, `--builder-herokuish-computed-allowed` | When `false`, the herokuish builder is skipped during builder detection for this app | +| `stack` | app + global | `gliderlabs/herokuish:latest-24` | `--builder-herokuish-stack`, `--builder-herokuish-global-stack`, `--builder-herokuish-computed-stack` | Herokuish stack image used to compile the app (e.g. `gliderlabs/herokuish:latest`) | diff --git a/plugins/builder-herokuish/builder-build b/plugins/builder-herokuish/builder-build index 736688249..ba5f2da1e 100755 --- a/plugins/builder-herokuish/builder-build +++ b/plugins/builder-herokuish/builder-build @@ -2,6 +2,7 @@ set -eo pipefail [[ $DOKKU_TRACE ]] && set -x source "$PLUGIN_CORE_AVAILABLE_PATH/common/functions" +source "$PLUGIN_AVAILABLE_PATH/builder-herokuish/internal-functions" source "$PLUGIN_AVAILABLE_PATH/config/functions" fn-builder-herokuish-ensure-cache() { @@ -23,7 +24,8 @@ trigger-builder-herokuish-builder-build() { return fi - local stack="$(plugn trigger buildpack-stack-name "$APP")" + local stack="$(fn-builder-herokuish-stack "$APP")" + [[ -z "$stack" ]] && stack="$(fn-builder-herokuish-global-stack "$APP")" if [[ -n "$stack" ]]; then dokku_log_info1 "Building $APP from $stack" DOKKU_IMAGE="$stack" diff --git a/plugins/builder-herokuish/internal-functions b/plugins/builder-herokuish/internal-functions index 395f146fd..1fb64ad70 100644 --- a/plugins/builder-herokuish/internal-functions +++ b/plugins/builder-herokuish/internal-functions @@ -28,3 +28,27 @@ fn-builder-herokuish-allowed() { fn-plugin-property-get-default "builder-herokuish" "$APP" "allowed" "" } + +fn-builder-herokuish-computed-stack() { + declare APP="$1" + + stack="$(fn-builder-herokuish-stack "$APP")" + if [[ -z "$stack" ]]; then + stack="$(fn-builder-herokuish-global-stack "$APP")" + fi + if [[ -z "$stack" ]]; then + stack="${DOKKU_IMAGE:-gliderlabs/herokuish:latest-24}" + fi + + echo "$stack" +} + +fn-builder-herokuish-global-stack() { + fn-plugin-property-get-default "builder-herokuish" "--global" "stack" "" +} + +fn-builder-herokuish-stack() { + declare APP="$1" + + fn-plugin-property-get-default "builder-herokuish" "$APP" "stack" "" +} diff --git a/plugins/builder-herokuish/report.go b/plugins/builder-herokuish/report.go index b5ab0b347..a2f6e809f 100644 --- a/plugins/builder-herokuish/report.go +++ b/plugins/builder-herokuish/report.go @@ -1,6 +1,8 @@ package builderherokuish import ( + "os" + "github.com/dokku/dokku/plugins/common" ) @@ -17,12 +19,17 @@ func ReportSingleApp(appName string, format string, infoFlag string) error { flags = map[string]common.ReportFunc{ "--builder-herokuish-computed-allowed": reportComputedAllowed, "--builder-herokuish-global-allowed": reportGlobalAllowed, + "--builder-herokuish-computed-stack": reportComputedStack, + "--builder-herokuish-global-stack": reportGlobalStack, } } else { flags = map[string]common.ReportFunc{ "--builder-herokuish-computed-allowed": reportComputedAllowed, "--builder-herokuish-global-allowed": reportGlobalAllowed, "--builder-herokuish-allowed": reportAllowed, + "--builder-herokuish-computed-stack": reportComputedStack, + "--builder-herokuish-global-stack": reportGlobalStack, + "--builder-herokuish-stack": reportStack, } } @@ -68,6 +75,29 @@ func reportComputedAllowed(appName string) string { return allowed } +func reportStack(appName string) string { + return common.PropertyGet("builder-herokuish", appName, "stack") +} + +func reportGlobalStack(appName string) string { + return common.PropertyGet("builder-herokuish", "--global", "stack") +} + +func reportComputedStack(appName string) string { + stack := reportStack(appName) + if stack == "" { + stack = reportGlobalStack(appName) + } + if stack == "" { + stack = os.Getenv("DOKKU_IMAGE") + } + if stack == "" { + stack = "gliderlabs/herokuish:latest-24" + } + + return stack +} + // hostArchitecture returns the dpkg architecture of the host, or an empty string // when it cannot be determined (matching the bash `dpkg --print-architecture` check) func hostArchitecture() string { diff --git a/plugins/builder-herokuish/subcommands/set b/plugins/builder-herokuish/subcommands/set index 777fbb07f..e157a5f57 100755 --- a/plugins/builder-herokuish/subcommands/set +++ b/plugins/builder-herokuish/subcommands/set @@ -9,13 +9,13 @@ cmd-builder-herokuish-set() { declare cmd="builder-herokuish:set" [[ "$1" == "$cmd" ]] && shift 1 declare APP="$1" KEY="$2" VALUE="$3" - local VALID_KEYS=("allowed") + local VALID_KEYS=("allowed" "stack") [[ "$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: allowed" + dokku_log_fail "Invalid key specified, valid keys include: allowed, stack" fi if [[ -n "$VALUE" ]]; then @@ -25,6 +25,14 @@ cmd-builder-herokuish-set() { dokku_log_info2_quiet "Unsetting ${KEY}" fn-plugin-property-delete "builder-herokuish" "$APP" "$KEY" fi + + if [[ "$KEY" == "stack" ]]; then + if [[ "$APP" == "--global" ]]; then + for app in $(dokku_apps 2>/dev/null || true); do plugn trigger post-stack-set "$app" "$VALUE"; done + else + plugn trigger post-stack-set "$APP" "$VALUE" + fi + fi } cmd-builder-herokuish-set "$@" diff --git a/plugins/builder-pack/builder-build b/plugins/builder-pack/builder-build index ead8c515e..2f7e060c0 100755 --- a/plugins/builder-pack/builder-build +++ b/plugins/builder-pack/builder-build @@ -1,5 +1,6 @@ #!/usr/bin/env bash source "$PLUGIN_CORE_AVAILABLE_PATH/common/functions" +source "$PLUGIN_AVAILABLE_PATH/builder-pack/internal-functions" source "$PLUGIN_AVAILABLE_PATH/config/functions" set -eo pipefail [[ $DOKKU_TRACE ]] && set -x @@ -13,7 +14,8 @@ trigger-builder-pack-builder-build() { return fi - local stack="$(plugn trigger buildpack-stack-name "$APP")" + local stack="$(fn-builder-pack-stack "$APP")" + [[ -z "$stack" ]] && stack="$(fn-builder-pack-global-stack "$APP")" if [[ -n "$stack" ]]; then DOKKU_CNB_BUILDER="$stack" fi @@ -303,6 +305,10 @@ trigger-builder-pack-builder-build() { local DOCKER_CONFIG DOCKER_CONFIG="$(fn-registry-docker-config-dir "$APP")" [[ -n "$DOCKER_CONFIG" ]] && export DOCKER_CONFIG + if [[ "$(fn-plugin-property-get-default "builder-pack" "$APP" "clear-cache" "false")" == "true" ]]; then + fn-in-array "--clear-cache" "${PACK_ARGS[@]}" || PACK_ARGS+=("--clear-cache") + fn-plugin-property-delete "builder-pack" "$APP" "clear-cache" + fi pack build "$IMAGE" --builder "$DOKKU_CNB_BUILDER" --path "$SOURCECODE_WORK_DIR" --default-process web "${PACK_ARGS[@]}" "${ENV_ARGS[@]}" docker-image-labeler relabel --label=dokku --label=org.label-schema.schema-version=1.0 --label=org.label-schema.vendor=dokku --label=com.dokku.image-stage=build --label=com.dokku.builder-type=pack --label=com.dokku.app-name=$APP "$IMAGE" diff --git a/plugins/builder-pack/internal-functions b/plugins/builder-pack/internal-functions index f81f131ea..56359a371 100644 --- a/plugins/builder-pack/internal-functions +++ b/plugins/builder-pack/internal-functions @@ -29,3 +29,29 @@ fn-builder-pack-projecttoml-path() { fn-plugin-property-get-default "builder-pack" "$APP" "projecttoml-path" "" } + +fn-builder-pack-computed-stack() { + declare APP="$1" + + stack="$(fn-builder-pack-stack "$APP")" + if [[ "$stack" == "" ]]; then + stack="$(fn-builder-pack-global-stack "$APP")" + fi + if [[ "$stack" == "" ]]; then + stack="${DOKKU_CNB_BUILDER:-heroku/builder:24}" + fi + + echo "$stack" +} + +fn-builder-pack-global-stack() { + declare APP="$1" + + fn-plugin-property-get-default "builder-pack" "--global" "stack" "" +} + +fn-builder-pack-stack() { + declare APP="$1" + + fn-plugin-property-get-default "builder-pack" "$APP" "stack" "" +} diff --git a/plugins/builder-pack/report.go b/plugins/builder-pack/report.go index c1949c457..c87245477 100644 --- a/plugins/builder-pack/report.go +++ b/plugins/builder-pack/report.go @@ -1,6 +1,8 @@ package builderpack import ( + "os" + "github.com/dokku/dokku/plugins/common" ) @@ -17,12 +19,17 @@ func ReportSingleApp(appName string, format string, infoFlag string) error { flags = map[string]common.ReportFunc{ "--builder-pack-computed-projecttoml-path": reportComputedProjecttomlPath, "--builder-pack-global-projecttoml-path": reportGlobalProjecttomlPath, + "--builder-pack-computed-stack": reportComputedStack, + "--builder-pack-global-stack": reportGlobalStack, } } else { flags = map[string]common.ReportFunc{ "--builder-pack-computed-projecttoml-path": reportComputedProjecttomlPath, "--builder-pack-global-projecttoml-path": reportGlobalProjecttomlPath, "--builder-pack-projecttoml-path": reportProjecttomlPath, + "--builder-pack-computed-stack": reportComputedStack, + "--builder-pack-global-stack": reportGlobalStack, + "--builder-pack-stack": reportStack, } } @@ -64,3 +71,26 @@ func reportComputedProjecttomlPath(appName string) string { return value } + +func reportStack(appName string) string { + return common.PropertyGet("builder-pack", appName, "stack") +} + +func reportGlobalStack(appName string) string { + return common.PropertyGet("builder-pack", "--global", "stack") +} + +func reportComputedStack(appName string) string { + value := reportStack(appName) + if value == "" { + value = reportGlobalStack(appName) + } + if value == "" { + value = os.Getenv("DOKKU_CNB_BUILDER") + } + if value == "" { + value = "heroku/builder:24" + } + + return value +} diff --git a/plugins/builder-pack/subcommands/set b/plugins/builder-pack/subcommands/set index 8642d829c..b8021b911 100755 --- a/plugins/builder-pack/subcommands/set +++ b/plugins/builder-pack/subcommands/set @@ -9,13 +9,13 @@ cmd-builder-pack-set() { declare cmd="builder-pack:set" [[ "$1" == "$cmd" ]] && shift 1 declare APP="$1" KEY="$2" VALUE="$3" - local VALID_KEYS=("projecttoml-path") + local VALID_KEYS=("projecttoml-path" "stack") [[ "$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: projecttoml-path" + dokku_log_fail "Invalid key specified, valid keys include: projecttoml-path, stack" fi if [[ -n "$VALUE" ]]; then @@ -25,6 +25,14 @@ cmd-builder-pack-set() { dokku_log_info2_quiet "Unsetting ${KEY}" fn-plugin-property-delete "builder-pack" "$APP" "$KEY" fi + + if [[ "$KEY" == "stack" ]]; then + if [[ "$APP" == "--global" ]]; then + for app in $(dokku_apps 2>/dev/null || true); do fn-plugin-property-write "builder-pack" "$app" "clear-cache" "true"; done + else + fn-plugin-property-write "builder-pack" "$APP" "clear-cache" "true" + fi + fi } cmd-builder-pack-set "$@" diff --git a/plugins/buildpacks/.gitignore b/plugins/buildpacks/.gitignore index 9929fd526..291036bd4 100644 --- a/plugins/buildpacks/.gitignore +++ b/plugins/buildpacks/.gitignore @@ -2,7 +2,6 @@ /subcommands/* /triggers/* /triggers -/buildpack-stack-name /install /post-* /report diff --git a/plugins/buildpacks/Makefile b/plugins/buildpacks/Makefile index 70ed37a5f..f6fe7c51d 100644 --- a/plugins/buildpacks/Makefile +++ b/plugins/buildpacks/Makefile @@ -1,5 +1,5 @@ SUBCOMMANDS = subcommands/add subcommands/clear subcommands/detect subcommands/list subcommands/remove subcommands/report subcommands/set subcommands/set-property -TRIGGERS = triggers/buildpack-stack-name triggers/install triggers/post-app-clone-setup triggers/post-app-rename-setup triggers/post-delete triggers/post-extract triggers/report +TRIGGERS = triggers/install triggers/post-app-clone-setup triggers/post-app-rename-setup triggers/post-delete triggers/post-extract triggers/report BUILD = commands subcommands triggers PLUGIN_NAME = buildpacks diff --git a/plugins/buildpacks/buildpacks.go b/plugins/buildpacks/buildpacks.go index 8f4e3b956..078f7c3f1 100644 --- a/plugins/buildpacks/buildpacks.go +++ b/plugins/buildpacks/buildpacks.go @@ -1,13 +1,24 @@ package buildpacks +import "strings" + var ( // DefaultProperties is a map of all valid buildpacks properties with corresponding default property values - DefaultProperties = map[string]string{ - "stack": "", - } + DefaultProperties = map[string]string{} // GlobalProperties is a map of all valid global buildpacks properties - GlobalProperties = map[string]bool{ - "stack": true, - } + GlobalProperties = map[string]bool{} ) + +// isHerokuishStack reports whether a stack value targets the herokuish builder +func isHerokuishStack(value string) bool { + return strings.Contains(value, "herokuish") +} + +// builderForStack returns the builder plugin name a stack value should be written to +func builderForStack(value string) string { + if isHerokuishStack(value) { + return "builder-herokuish" + } + return "builder-pack" +} diff --git a/plugins/buildpacks/report.go b/plugins/buildpacks/report.go index a2a2221a6..31510db78 100644 --- a/plugins/buildpacks/report.go +++ b/plugins/buildpacks/report.go @@ -1,7 +1,6 @@ package buildpacks import ( - "os" "strings" "github.com/dokku/dokku/plugins/common" @@ -17,15 +16,10 @@ func ReportSingleApp(appName string, format string, infoFlag string) error { var flags map[string]common.ReportFunc if appName == "--global" { - flags = map[string]common.ReportFunc{ - "--buildpacks-global-stack": reportGlobalStack, - } + flags = map[string]common.ReportFunc{} } else { flags = map[string]common.ReportFunc{ - "--buildpacks-computed-stack": reportComputedStack, - "--buildpacks-global-stack": reportGlobalStack, - "--buildpacks-list": reportList, - "--buildpacks-stack": reportStack, + "--buildpacks-list": reportList, } } @@ -48,31 +42,6 @@ func ReportSingleApp(appName string, format string, infoFlag string) error { }) } -func reportComputedStack(appName string) string { - if stack := common.PropertyGetDefault("buildpacks", appName, "stack", ""); stack != "" { - return stack - } - - if stack := common.PropertyGetDefault("buildpacks", "--global", "stack", ""); stack != "" { - return stack - } - - results, _ := common.CallPlugnTrigger(common.PlugnTriggerInput{ - Trigger: "config-get", - Args: []string{appName, "DOKKU_IMAGE"}, - }) - if dokkuImage := results.StdoutContents(); dokkuImage != "" { - common.LogWarn("Deprecated: use buildpacks:set-property instead of specifying DOKKU_IMAGE environment variable") - return dokkuImage - } - - return os.Getenv("DOKKU_IMAGE") -} - -func reportGlobalStack(appName string) string { - return common.PropertyGetDefault("buildpacks", "--global", "stack", "") -} - func reportList(appName string) string { buildpacks, err := common.PropertyListGet("buildpacks", appName, "buildpacks") if err != nil { @@ -81,7 +50,3 @@ func reportList(appName string) string { return strings.Join(buildpacks, ",") } - -func reportStack(appName string) string { - return common.PropertyGetDefault("buildpacks", appName, "stack", "") -} diff --git a/plugins/buildpacks/src/triggers/triggers.go b/plugins/buildpacks/src/triggers/triggers.go index cf33fd006..b6df0961f 100644 --- a/plugins/buildpacks/src/triggers/triggers.go +++ b/plugins/buildpacks/src/triggers/triggers.go @@ -18,9 +18,6 @@ func main() { var err error switch trigger { - case "buildpack-stack-name": - appName := flag.Arg(0) - err = buildpacks.TriggerBuildpackStackName(appName) case "install": err = buildpacks.TriggerInstall() case "post-app-clone-setup": diff --git a/plugins/buildpacks/subcommands.go b/plugins/buildpacks/subcommands.go index fb9e7fa93..ef579429d 100644 --- a/plugins/buildpacks/subcommands.go +++ b/plugins/buildpacks/subcommands.go @@ -215,35 +215,64 @@ func setBuildpackList(appName string, buildpacks []string, index int) error { // CommandSetProperty implements buildpacks:set-property func CommandSetProperty(appName string, property string, value string) error { - oldStack := "" - if property == "stack" { - oldStack = common.PropertyGet("buildpacks", appName, "stack") + if property != "stack" { + common.CommandPropertySet("buildpacks", appName, property, value, DefaultProperties, GlobalProperties) + return nil } - common.CommandPropertySet("buildpacks", appName, property, value, DefaultProperties, GlobalProperties) - if property == "stack" && oldStack != value { - if appName != "--global" { - _, err := common.CallPlugnTrigger(common.PlugnTriggerInput{ - Trigger: "post-stack-set", - Args: []string{appName, value}, - StreamStdio: true, - }) + if value != "" { + builder := builderForStack(value) + common.LogWarn(fmt.Sprintf("Deprecated: buildpacks:set-property stack is deprecated, use %s:set stack instead", builder)) + if err := common.PropertyWrite(builder, appName, "stack", value); err != nil { return err } + return clearBuilderCache(builder, appName) + } - apps, err := common.DokkuApps() - if err != nil && !errors.Is(err, common.NoAppsExist) { + common.LogWarn("Deprecated: buildpacks:set-property stack is deprecated, use builder-herokuish:set stack or builder-pack:set stack instead") + for _, builder := range []string{"builder-herokuish", "builder-pack"} { + if err := common.PropertyDelete(builder, appName, "stack"); err != nil { return err } - for _, app := range apps { - _, err := common.CallPlugnTrigger(common.PlugnTriggerInput{ - Trigger: "post-stack-set", - Args: []string{app, value}, - StreamStdio: true, - }) - if err != nil { - return err - } + if err := clearBuilderCache(builder, appName); err != nil { + return err + } + } + + return nil +} + +// clearBuilderCache clears the build cache for the given builder after a stack +// change. The herokuish cache is the cache-$APP volume removed by repo via the +// post-stack-set trigger, while the pack cache lives in pack-managed volumes and +// is cleared by flagging the next build to run with --clear-cache. +func clearBuilderCache(builder string, appName string) error { + apps := []string{appName} + if appName == "--global" { + dokkuApps, err := common.DokkuApps() + if err != nil { + if errors.Is(err, common.NoAppsExist) { + return nil + } + return err + } + apps = dokkuApps + } + + for _, app := range apps { + if builder == "builder-herokuish" { + if _, err := common.CallPlugnTrigger(common.PlugnTriggerInput{ + Trigger: "post-stack-set", + Args: []string{app, ""}, + StreamStdio: true, + }); err != nil { + return err + } + continue + } + + if err := common.PropertyWrite("builder-pack", app, "clear-cache", "true"); err != nil { + return err } } diff --git a/plugins/buildpacks/triggers.go b/plugins/buildpacks/triggers.go index fce9ecd5d..a107961b0 100644 --- a/plugins/buildpacks/triggers.go +++ b/plugins/buildpacks/triggers.go @@ -2,6 +2,7 @@ package buildpacks import ( "bufio" + "errors" "fmt" "os" "path/filepath" @@ -9,38 +10,46 @@ import ( "github.com/dokku/dokku/plugins/common" ) -// TriggerBuildpackStackName echos the stack name for the app -func TriggerBuildpackStackName(appName string) error { - if stack := common.PropertyGetDefault("buildpacks", appName, "stack", ""); stack != "" { - fmt.Println(stack) - return nil - } - - if stack := common.PropertyGetDefault("buildpacks", "--global", "stack", ""); stack != "" { - fmt.Println(stack) - return nil - } - - results, _ := common.CallPlugnTrigger(common.PlugnTriggerInput{ - Trigger: "config-get", - Args: []string{appName, "DOKKU_IMAGE"}, - }) - dokkuImage := results.StdoutContents() - if dokkuImage != "" { - common.LogWarn("Deprecated: use buildpacks:set-property instead of specifying DOKKU_IMAGE environment variable") - fmt.Println(dokkuImage) - return nil - } - - return nil -} - // TriggerInstall runs the install step for the buildpacks plugin func TriggerInstall() error { if err := common.PropertySetup("buildpacks"); err != nil { return fmt.Errorf("Unable to install the buildpacks plugin: %s", err.Error()) } + return migrateStackProperty() +} + +// migrateStackProperty moves any legacy buildpacks stack values to the +// appropriate builder plugin, detecting herokuish vs pack stacks. It is +// idempotent: once a value is migrated the buildpacks stack key is removed. +func migrateStackProperty() error { + appNames := []string{"--global"} + apps, err := common.DokkuApps() + if err != nil && !errors.Is(err, common.NoAppsExist) { + return err + } + appNames = append(appNames, apps...) + + for _, appName := range appNames { + if !common.PropertyExists("buildpacks", appName, "stack") { + continue + } + + value := common.PropertyGet("buildpacks", appName, "stack") + if value != "" { + builder := builderForStack(value) + if !common.PropertyExists(builder, appName, "stack") { + if err := common.PropertyWrite(builder, appName, "stack", value); err != nil { + return err + } + } + } + + if err := common.PropertyDelete("buildpacks", appName, "stack"); err != nil { + return err + } + } + return nil } diff --git a/tests/unit/builder-herokuish.bats b/tests/unit/builder-herokuish.bats index 1acf41778..451fb04d7 100644 --- a/tests/unit/builder-herokuish.bats +++ b/tests/unit/builder-herokuish.bats @@ -104,6 +104,50 @@ teardown() { assert_success } +@test "(builder-herokuish:report) stack raw vs computed vs global" { + local default_stack="gliderlabs/herokuish:latest-24" + + run /bin/bash -c "dokku builder-herokuish:report $TEST_APP --builder-herokuish-stack" + assert_success + assert_output_not_exists + + run /bin/bash -c "dokku builder-herokuish:report $TEST_APP --builder-herokuish-global-stack" + assert_success + assert_output_not_exists + + run /bin/bash -c "dokku builder-herokuish:report $TEST_APP --builder-herokuish-computed-stack" + assert_success + assert_output "$default_stack" + + run /bin/bash -c "dokku builder-herokuish:set --global stack gliderlabs/herokuish:global" + assert_success + + run /bin/bash -c "dokku builder-herokuish:report $TEST_APP --builder-herokuish-global-stack" + assert_success + assert_output "gliderlabs/herokuish:global" + + run /bin/bash -c "dokku builder-herokuish:report $TEST_APP --builder-herokuish-computed-stack" + assert_success + assert_output "gliderlabs/herokuish:global" + + run /bin/bash -c "dokku builder-herokuish:set $TEST_APP stack gliderlabs/herokuish:app" + assert_success + + run /bin/bash -c "dokku builder-herokuish:report $TEST_APP --builder-herokuish-stack" + assert_success + assert_output "gliderlabs/herokuish:app" + + run /bin/bash -c "dokku builder-herokuish:report $TEST_APP --builder-herokuish-computed-stack" + assert_success + assert_output "gliderlabs/herokuish:app" + + run /bin/bash -c "dokku builder-herokuish:set $TEST_APP stack" + assert_success + + run /bin/bash -c "dokku builder-herokuish:set --global stack" + assert_success +} + @test "(builder-herouish:build .env)" { run /bin/bash -c "dokku docker-options:add $TEST_APP build '--label=com.dokku.build-test=safe'" echo "output: $output" diff --git a/tests/unit/builder-pack.bats b/tests/unit/builder-pack.bats index 542c65112..48eb23336 100644 --- a/tests/unit/builder-pack.bats +++ b/tests/unit/builder-pack.bats @@ -107,6 +107,55 @@ teardown() { assert_success } +@test "(builder-pack:report) stack raw vs computed vs global" { + run /bin/bash -c "dokku builder-pack:set --global stack" + assert_success + + run /bin/bash -c "dokku builder-pack:report $TEST_APP --builder-pack-stack" + assert_success + assert_output_not_exists + + run /bin/bash -c "dokku builder-pack:report $TEST_APP --builder-pack-global-stack" + assert_success + assert_output_not_exists + + run /bin/bash -c "dokku builder-pack:report $TEST_APP --builder-pack-computed-stack" + assert_success + assert_output "heroku/builder:24" + + run /bin/bash -c "dokku builder-pack:set --global stack paketobuildpacks/builder:global" + assert_success + + run /bin/bash -c "dokku builder-pack:report $TEST_APP --builder-pack-global-stack" + assert_success + assert_output "paketobuildpacks/builder:global" + + run /bin/bash -c "dokku builder-pack:report $TEST_APP --builder-pack-computed-stack" + assert_success + assert_output "paketobuildpacks/builder:global" + + run /bin/bash -c "dokku builder-pack:set $TEST_APP stack paketobuildpacks/builder:app" + assert_success + + run /bin/bash -c "dokku builder-pack:report $TEST_APP --builder-pack-stack" + assert_success + assert_output "paketobuildpacks/builder:app" + + run /bin/bash -c "dokku builder-pack:report $TEST_APP --builder-pack-computed-stack" + assert_success + assert_output "paketobuildpacks/builder:app" + + run /bin/bash -c "cat $DOKKU_LIB_ROOT/config/builder-pack/$TEST_APP/clear-cache" + assert_success + assert_output "true" + + run /bin/bash -c "dokku builder-pack:set $TEST_APP stack" + assert_success + + run /bin/bash -c "dokku builder-pack:set --global stack" + assert_success +} + @test "(builder-pack:set)" { run /bin/bash -c "dokku config:set $TEST_APP SECRET_KEY=fjdkslafjdk" echo "output: $output" @@ -118,11 +167,17 @@ teardown() { echo "status: $status" assert_success - run /bin/bash -c "dokku buildpacks:set-property $TEST_APP stack heroku/builder:24" + run /bin/bash -c "dokku builder-pack:set $TEST_APP stack heroku/builder:24" echo "output: $output" echo "status: $status" assert_success + run /bin/bash -c "cat $DOKKU_LIB_ROOT/config/builder-pack/$TEST_APP/clear-cache" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "true" + run deploy_app python dokku@$DOKKU_DOMAIN:$TEST_APP add_requirements_txt_cnb echo "output: $output" echo "status: $status" @@ -130,6 +185,11 @@ teardown() { assert_output_contains 'Building with buildpack 1' 0 assert_output_contains 'Installing dependencies using pip' + run /bin/bash -c "test -f $DOKKU_LIB_ROOT/config/builder-pack/$TEST_APP/clear-cache" + echo "output: $output" + echo "status: $status" + assert_failure + run /bin/bash -c "dokku builder-pack:set $TEST_APP projecttoml-path nonexistent.toml" echo "output: $output" echo "status: $status" diff --git a/tests/unit/buildpacks.bats b/tests/unit/buildpacks.bats index 732a3be27..d6ecad0af 100644 --- a/tests/unit/buildpacks.bats +++ b/tests/unit/buildpacks.bats @@ -253,71 +253,51 @@ teardown() { assert_output "https://github.com/heroku/heroku-buildpack-nodejs.git" } -@test "(buildpacks) buildpacks:set-property" { - run /bin/bash -c "dokku buildpacks:set-property --global stack gliderlabs/herokuish:latest" +@test "(buildpacks) buildpacks:set-property forwards stack to the detected builder" { + run /bin/bash -c "dokku buildpacks:set-property $TEST_APP stack gliderlabs/herokuish:app" echo "output: $output" echo "status: $status" assert_success -} + assert_output_contains "Deprecated" -@test "(buildpacks:report) stack raw vs computed vs global" { - run /bin/bash -c "dokku buildpacks:set-property --global stack" - assert_success - - run /bin/bash -c "dokku buildpacks:report $TEST_APP --format json | jq -r '.\"buildpacks-stack\"'" - assert_success - assert_output "" - - run /bin/bash -c "dokku buildpacks:report $TEST_APP --format json | jq -r '.\"buildpacks-global-stack\"'" - assert_success - assert_output "" - - run /bin/bash -c "dokku buildpacks:set-property --global stack gliderlabs/herokuish:global" - assert_success - - run /bin/bash -c "dokku buildpacks:report $TEST_APP --format json | jq -r '.\"buildpacks-global-stack\"'" - assert_success - assert_output "gliderlabs/herokuish:global" - - run /bin/bash -c "dokku buildpacks:report $TEST_APP --format json | jq -r '.\"buildpacks-computed-stack\"'" - assert_success - assert_output "gliderlabs/herokuish:global" - - run /bin/bash -c "dokku buildpacks:set-property $TEST_APP stack gliderlabs/herokuish:app" - assert_success - - run /bin/bash -c "dokku buildpacks:report $TEST_APP --format json | jq -r '.\"buildpacks-stack\"'" + run /bin/bash -c "dokku builder-herokuish:report $TEST_APP --builder-herokuish-stack" + echo "output: $output" + echo "status: $status" assert_success assert_output "gliderlabs/herokuish:app" - run /bin/bash -c "dokku buildpacks:report $TEST_APP --format json | jq -r '.\"buildpacks-global-stack\"'" + run /bin/bash -c "dokku builder-pack:report $TEST_APP --builder-pack-stack" + echo "output: $output" + echo "status: $status" assert_success - assert_output "gliderlabs/herokuish:global" + assert_output "" - run /bin/bash -c "dokku buildpacks:report $TEST_APP --format json | jq -r '.\"buildpacks-computed-stack\"'" + run /bin/bash -c "dokku buildpacks:set-property $TEST_APP stack heroku/builder:24" + echo "output: $output" + echo "status: $status" assert_success - assert_output "gliderlabs/herokuish:app" + + run /bin/bash -c "dokku builder-pack:report $TEST_APP --builder-pack-stack" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "heroku/builder:24" run /bin/bash -c "dokku buildpacks:set-property $TEST_APP stack" + echo "output: $output" + echo "status: $status" assert_success - run /bin/bash -c "dokku buildpacks:set-property --global stack" + run /bin/bash -c "dokku builder-herokuish:report $TEST_APP --builder-herokuish-stack" assert_success + assert_output "" + + run /bin/bash -c "dokku builder-pack:report $TEST_APP --builder-pack-stack" + assert_success + assert_output "" } @test "(buildpacks:report) emits new stripped JSON keys alongside legacy" { - run /bin/bash -c "dokku buildpacks:report $TEST_APP --format json | jq -r 'has(\"stack\") and has(\"buildpacks-stack\")'" - assert_success - assert_output "true" - - run /bin/bash -c "dokku buildpacks:report $TEST_APP --format json | jq -r 'has(\"global-stack\") and has(\"buildpacks-global-stack\")'" - assert_success - assert_output "true" - - run /bin/bash -c "dokku buildpacks:report $TEST_APP --format json | jq -r 'has(\"computed-stack\") and has(\"buildpacks-computed-stack\")'" - assert_success - assert_output "true" - run /bin/bash -c "dokku buildpacks:report $TEST_APP --format json | jq -r 'has(\"list\") and has(\"buildpacks-list\")'" assert_success assert_output "true"