diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d2e807c..8fd14f0 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -52,6 +52,8 @@ jobs: with: working_directory: tests/fixtures service_name: smoke-svc + default_build_context: '.' + default_dockerfile_path: Dockerfile - name: Verify outputs run: | @@ -66,12 +68,14 @@ jobs: test "${{ steps.cfg.outputs.dockerfile_path }}" = "services/smoke/Dockerfile" echo "smoke-svc outputs OK" - - name: Run action against fixture (buildContext absent, no caller default) + - name: Run action against fixture (buildContext absent -> default_build_context applied) id: cfg2 uses: ./ with: working_directory: tests/fixtures service_name: smoke-default-context + default_build_context: '.' + default_dockerfile_path: Dockerfile - name: Verify outputs when YAML lacks buildContext and no caller default run: | @@ -80,40 +84,23 @@ jobs: # default_build_context the caller passed (here: nothing → empty). test "${{ steps.cfg2.outputs.build_context }}" = "" test "${{ steps.cfg2.outputs.dockerfile_path }}" = "services/default/Dockerfile" - echo "default-context outputs OK (empty fallback)" + echo "default-context outputs OK" - - name: Run action with caller-supplied default_* fallbacks + - name: Run action against fixture (service missing, defaults flow through) id: cfg3 uses: ./ with: working_directory: tests/fixtures - service_name: smoke-default-context - default_build_context: 'services/default' - default_dockerfile_path: 'services/default/Custom.Dockerfile' - - - name: Verify caller defaults fill in YAML-absent fields - run: | - set -e - # buildContext is absent in YAML for smoke-default-context → fall back to caller-supplied default - test "${{ steps.cfg3.outputs.build_context }}" = "services/default" - # dockerfilePath IS in YAML → YAML wins, caller default is ignored - test "${{ steps.cfg3.outputs.dockerfile_path }}" = "services/default/Dockerfile" - echo "caller-default fallback OK" - - - name: Run action against missing service with caller defaults - id: cfg4 - uses: ./ - with: - working_directory: tests/fixtures - service_name: not-a-real-service + service_name: does-not-exist default_build_context: '.' - default_dockerfile_path: 'Dockerfile' + default_dockerfile_path: Dockerfile - - name: Verify caller defaults applied on missing service + - name: Verify missing-service fallback outputs run: | set -e - test "${{ steps.cfg4.outputs.config_found }}" = "true" - test "${{ steps.cfg4.outputs.service_found }}" = "false" - test "${{ steps.cfg4.outputs.build_context }}" = "." - test "${{ steps.cfg4.outputs.dockerfile_path }}" = "Dockerfile" - echo "missing-service caller-default OK" + test "${{ steps.cfg3.outputs.config_found }}" = "true" + test "${{ steps.cfg3.outputs.service_found }}" = "false" + test "${{ steps.cfg3.outputs.name }}" = "" + test "${{ steps.cfg3.outputs.build_context }}" = "." + test "${{ steps.cfg3.outputs.dockerfile_path }}" = "Dockerfile" + echo "missing-service fallback outputs OK" diff --git a/README.md b/README.md index eb2859f..36d2afe 100644 --- a/README.md +++ b/README.md @@ -20,11 +20,13 @@ This action parses the Skyhook configuration file and extracts service-specific with: working_directory: code service_name: my-service + default_build_context: '.' + default_dockerfile_path: 'Dockerfile' - name: Build Docker image run: | docker build \ - -f ${{ steps.config.outputs.dockerfile_path || 'Dockerfile' }} \ + -f ${{ steps.config.outputs.dockerfile_path }} \ ${{ steps.config.outputs.build_context }} ``` @@ -35,8 +37,8 @@ This action parses the Skyhook configuration file and extracts service-specific | `working_directory` | Path to the repository root containing `.skyhook/skyhook.yaml` | No | `.` | | `service_name` | Name of the service to look up in the config | Yes | - | | `config_path` | Path to the skyhook config file relative to working_directory | No | `.skyhook/skyhook.yaml` | -| `default_dockerfile_path` | Fallback for `dockerfile_path` when the YAML doesn't supply one (config unreadable, service missing, or `buildTool.docker.dockerfilePath` unset/empty). | No | `""` | -| `default_build_context` | Fallback for `build_context` when the YAML doesn't supply one. | No | `""` | +| `default_build_context` | Value emitted as `build_context` whenever it would otherwise be empty: YAML field absent, config file missing, or service missing. The action does not pick a value for you - callers must pass a non-empty default, otherwise the action fails loudly. | Yes | - | +| `default_dockerfile_path` | Value emitted as `dockerfile_path` whenever it would otherwise be empty (same triggers as `default_build_context`). Must be non-empty. | Yes | - | ## Outputs @@ -46,11 +48,13 @@ This action parses the Skyhook configuration file and extracts service-specific | `path` | Service path relative to repo root | | `deployment_repo` | Separate deployment repository (if configured) | | `deployment_repo_path` | Path within deployment repository | -| `build_context` | Docker build context relative to repo root (defaults to `.` when absent in config) | -| `dockerfile_path` | Dockerfile path relative to repo root | +| `build_context` | Docker build context relative to repo root. Falls back to `default_build_context` when absent/empty in config or when config/service is not found. | +| `dockerfile_path` | Dockerfile path relative to repo root. Falls back to `default_dockerfile_path` when absent/empty in config or when config/service is not found. | | `config_found` | Whether the config file was found (`true`/`false`) | | `service_found` | Whether the service was found in config (`true`/`false`) | +> When a default is applied because a YAML field was absent, the action emits a `::notice::` line. When the entire config file or service is missing, the action emits a `::warning::` (more prominent in the run UI) — so the source of every emitted value is visible at a glance. + ## Config File Format The action expects a `.skyhook/skyhook.yaml` file with the following structure: @@ -70,7 +74,7 @@ services: path: services/another buildTool: docker: - # buildContext omitted - defaults to "." + # buildContext omitted - falls back to default_build_context input dockerfilePath: services/another/Dockerfile environments: @@ -96,44 +100,44 @@ jobs: with: working_directory: code service_name: ${{ env.SERVICE_NAME }} + default_build_context: '.' + default_dockerfile_path: Dockerfile - name: Build and push Docker image uses: skyhook-io/docker-build-push-action@v1 with: context: code/${{ steps.config.outputs.build_context }} - dockerfile: code/${{ steps.config.outputs.dockerfile_path || format('{0}/Dockerfile', steps.config.outputs.build_context) }} + dockerfile: code/${{ steps.config.outputs.dockerfile_path }} image: ${{ inputs.image }} ``` ## Behavior matrix -The two build-tool outputs (`build_context`, `dockerfile_path`) follow a single rule: - -> **YAML wins when the config is readable AND the service exists AND the field is non-empty. In every other case, the caller-supplied `default_*` input is emitted.** +Let `BC` = `default_build_context` input, `DF` = `default_dockerfile_path` input. -`name`, `path`, `deployment_repo`, and `deployment_repo_path` are always sourced from the YAML and emit empty when the config/service can't be read — there are no `default_*` fallbacks for them. +**Uniform rule:** if the action would emit an empty value for `build_context` or `dockerfile_path`, it exits 1 instead. The corresponding default input must be non-empty whenever the YAML doesn't supply the value. -| Scenario | `config_found` | `service_found` | `build_context` | `dockerfile_path` | -|---|---|---|---|---| -| Config file missing | `false` | `false` | `default_build_context` | `default_dockerfile_path` | -| Config found, service missing | `true` | `false` | `default_build_context` | `default_dockerfile_path` | -| Service found, both fields set | `true` | `true` | from YAML | from YAML | -| Service found, only `buildContext` set | `true` | `true` | from YAML | `default_dockerfile_path` | -| Service found, neither field set | `true` | `true` | `default_build_context` | `default_dockerfile_path` | -| Duplicate service names | n/a | n/a | n/a | action exits 1 | +| Scenario | `config_found` | `service_found` | `name` / `path` / `deployment_*` | `build_context` | `dockerfile_path` | +|---|---|---|---|---|---| +| Service found, both YAML fields set | `true` | `true` | from config | from config | from config | +| Service found, only `buildContext` absent/`""`/`null`, `BC` non-empty | `true` | `true` | from config | `BC` | from config | +| Service found, only `dockerfilePath` absent/`""`/`null`, `DF` non-empty | `true` | `true` | from config | from config | `DF` | +| Service found, both YAML fields absent, both defaults non-empty | `true` | `true` | from config | `BC` | `DF` | +| Service missing, both defaults non-empty | `true` | `false` | `""` | `BC` | `DF` | +| Config file missing, both defaults non-empty | `false` | `false` | `""` | `BC` | `DF` | +| Any of the above where the relevant default is empty | n/a | n/a | n/a | n/a | **action exits 1** | +| Duplicate service names in config | n/a | n/a | n/a | n/a | action exits 1 | -Without `default_*` inputs the fallback is empty — meaning a workflow that wants a guaranteed-non-empty value should pass them at the call site: +The action **always** emits a non-empty `build_context` and `dockerfile_path` on success, so the consuming workflow can drop `||` fallbacks: ```yaml -- uses: skyhook-io/read-config@v1 - with: - service_name: my-svc - # Computed sensible fallbacks the caller controls: - default_dockerfile_path: services/my-svc/Dockerfile - default_build_context: . +context: code/${{ steps.config.outputs.build_context }} +dockerfile: code/${{ steps.config.outputs.dockerfile_path }} ``` -`config_found` and `service_found` remain available so the caller can distinguish between a YAML-sourced value and a fallback if needed. +Logging: +- A `::notice::` is emitted when a default is applied because a YAML field was absent for an otherwise-found service. +- A `::warning::` is emitted when the entire config file or service is missing and defaults flow through (more prominent than a notice — the not-found state is usually a misconfiguration). ## Runner requirements diff --git a/action.yml b/action.yml index 0ae0199..3a8ba1a 100644 --- a/action.yml +++ b/action.yml @@ -14,14 +14,18 @@ inputs: description: 'Path to the skyhook config file relative to working_directory' required: false default: '.skyhook/skyhook.yaml' - default_dockerfile_path: - description: 'Fallback for the dockerfile_path output. Used when the YAML config is unreadable, the service is missing, or buildTool.docker.dockerfilePath is unset/empty. Empty by default — the caller decides whether to fall back ("Dockerfile", "/Dockerfile", etc.).' - required: false - default: '' default_build_context: - description: 'Fallback for the build_context output. Used when the YAML config is unreadable, the service is missing, or buildTool.docker.buildContext is unset/empty. Empty by default — the caller decides (".", the source path, etc.).' - required: false - default: '' + description: | + Value emitted as `build_context` whenever it would otherwise be empty: YAML field absent, config + file missing, or service missing. Must be non-empty - if a default would be applied and this input + is empty, the action exits 1 (refuses to emit an empty `build_context` that would break a downstream + `docker build`). + required: true + default_dockerfile_path: + description: | + Value emitted as `dockerfile_path` whenever it would otherwise be empty (same triggers as + `default_build_context`). Must be non-empty. + required: true outputs: # Service configuration @@ -40,10 +44,10 @@ outputs: # Build tool configuration build_context: - description: 'Docker build context relative to repo root. Sourced from buildTool.docker.buildContext in the YAML when available; falls back to the default_build_context input otherwise.' + description: 'Docker build context relative to repo root. Falls back to the default_build_context input when absent/empty in config or when config/service is not found.' value: ${{ steps.parse.outputs.build_context }} dockerfile_path: - description: 'Dockerfile path relative to repo root. Sourced from buildTool.docker.dockerfilePath in the YAML when available; falls back to the default_dockerfile_path input otherwise.' + description: 'Dockerfile path relative to repo root. Falls back to the default_dockerfile_path input when absent/empty in config or when config/service is not found.' value: ${{ steps.parse.outputs.dockerfile_path }} # Status @@ -118,8 +122,8 @@ runs: WORKING_DIR: ${{ inputs.working_directory }} SERVICE_NAME: ${{ inputs.service_name }} CONFIG_PATH: ${{ inputs.config_path }} - DEFAULT_DOCKERFILE_PATH: ${{ inputs.default_dockerfile_path }} DEFAULT_BUILD_CONTEXT: ${{ inputs.default_build_context }} + DEFAULT_DOCKERFILE_PATH: ${{ inputs.default_dockerfile_path }} run: bash "$GITHUB_ACTION_PATH/scripts/parse.sh" branding: diff --git a/scripts/parse.sh b/scripts/parse.sh index f8a0ad6..c317ada 100755 --- a/scripts/parse.sh +++ b/scripts/parse.sh @@ -2,27 +2,32 @@ # Parse a Skyhook config file and emit GitHub Actions-style outputs. # # Inputs (env): -# WORKING_DIR - repo root containing the config (default ".") -# SERVICE_NAME - service to look up (required, non-empty) -# CONFIG_PATH - path to config relative to WORKING_DIR (default ".skyhook/skyhook.yaml") -# DEFAULT_DOCKERFILE_PATH - fallback when YAML doesn't supply dockerfile_path -# DEFAULT_BUILD_CONTEXT - fallback when YAML doesn't supply build_context -# GITHUB_OUTPUT - file to append outputs to (required by GitHub Actions; tests pass a tempfile) -# -# Fallback rule for build_context / dockerfile_path: the YAML value wins when -# the config is readable AND the service exists AND the field is non-empty. -# In every other case (config missing, service missing, field absent or -# explicitly null/empty) the corresponding DEFAULT_* input is emitted — -# empty by default, so the caller decides what their fallback should be. +# WORKING_DIR - repo root containing the config (default ".") +# SERVICE_NAME - service to look up (required, non-empty) +# CONFIG_PATH - path to config relative to WORKING_DIR (default ".skyhook/skyhook.yaml") +# DEFAULT_BUILD_CONTEXT - REQUIRED. Emitted as build_context when YAML omits it OR when +# config/service is not found. The action does not pick a default +# for the caller; this is intentional so callers must opt in. +# DEFAULT_DOCKERFILE_PATH - REQUIRED. Same contract as DEFAULT_BUILD_CONTEXT, for dockerfile_path. +# GITHUB_OUTPUT - file to append outputs to (required by GitHub Actions; tests pass a tempfile) # # Exits non-zero with ::error:: on: # - empty SERVICE_NAME +# - DEFAULT_BUILD_CONTEXT or DEFAULT_DOCKERFILE_PATH unset (note: unset != empty; +# unset catches "the input was never wired in action.yml") +# - any time a default would be applied (YAML absent OR config/service not found) +# and the corresponding default input is empty - the action refuses to emit +# empty build_context / dockerfile_path because downstream `docker build` would +# fail with a cryptic error. Uniform rule: defaults must be non-empty whenever +# they're needed. # - yq parse failure # - duplicate service names # - unexpected SERVICE_INDEX shape set -euo pipefail : "${GITHUB_OUTPUT:?GITHUB_OUTPUT must be set}" +: "${DEFAULT_BUILD_CONTEXT?DEFAULT_BUILD_CONTEXT must be set (wire the default_build_context input)}" +: "${DEFAULT_DOCKERFILE_PATH?DEFAULT_DOCKERFILE_PATH must be set (wire the default_dockerfile_path input)}" WORKING_DIR="${WORKING_DIR:-.}" [ -z "$WORKING_DIR" ] && WORKING_DIR="." @@ -58,10 +63,12 @@ write_output() { } >> "$GITHUB_OUTPUT" } -# Emit empty/identity outputs for the no-data path. build_context and -# dockerfile_path still fall back to the caller-supplied defaults so a -# missing config / service yields the same shape as a present-but-empty one. -write_empty_outputs() { +# Used when no service was matched (config missing OR service missing). +# Identity fields stay empty (no service to describe), but build_context and +# dockerfile_path are populated from the caller-supplied defaults so downstream +# steps can still construct a path. Consumers who care about the difference +# should branch on config_found / service_found. +write_unmatched_outputs() { write_output name "" write_output path "" write_output deployment_repo "" @@ -70,12 +77,42 @@ write_empty_outputs() { write_output dockerfile_path "$DEFAULT_DOCKERFILE_PATH" } +# Refuse to emit an empty value for a SINGLE field. Used in the per-field +# YAML-absent path where we already know which field needs a default. +# +# `which`: human-readable label for the field whose default is about to be used. +# One of "build_context" or "dockerfile_path". +require_nonempty_default() { + local which="$1" reason="$2" value="$3" + if [ -z "$value" ]; then + echo "::error::${reason}, but default_${which} input is empty. Provide a non-empty value so the action can emit a usable ${which}." + exit 1 + fi +} + +# Refuse to emit empty values when BOTH defaults are about to be applied (config +# missing or service missing - we know up front we'll need both). Lists every +# empty input in a single error so the caller can fix them all at once instead +# of fail-then-fail-again. +require_nonempty_defaults_for_unmatched() { + local reason="$1" + local missing=() + [ -z "$DEFAULT_BUILD_CONTEXT" ] && missing+=("default_build_context") + [ -z "$DEFAULT_DOCKERFILE_PATH" ] && missing+=("default_dockerfile_path") + if [ "${#missing[@]}" -gt 0 ]; then + echo "::error::${reason}, but the following required input(s) are empty: ${missing[*]}. Provide non-empty values so the action can emit usable build_context / dockerfile_path." + exit 1 + fi +} + # Config file missing if [ ! -f "$CONFIG_FILE" ]; then echo "Config file not found: $CONFIG_FILE" + require_nonempty_defaults_for_unmatched "Config file not found at '${CONFIG_FILE}'" + echo "::warning::Config file not found - emitting build_context='${DEFAULT_BUILD_CONTEXT}' and dockerfile_path='${DEFAULT_DOCKERFILE_PATH}' from default_build_context / default_dockerfile_path inputs" write_output config_found false write_output service_found false - write_empty_outputs + write_unmatched_outputs exit 0 fi @@ -94,8 +131,10 @@ fi if [ -z "$SERVICE_INDEX" ]; then echo "Service '$SERVICE_NAME' not found in config" + require_nonempty_defaults_for_unmatched "Service '${SERVICE_NAME}' not found in '${CONFIG_FILE}'" + echo "::warning::Service '${SERVICE_NAME}' not found - emitting build_context='${DEFAULT_BUILD_CONTEXT}' and dockerfile_path='${DEFAULT_DOCKERFILE_PATH}' from default_build_context / default_dockerfile_path inputs" write_output service_found false - write_empty_outputs + write_unmatched_outputs exit 0 fi @@ -133,11 +172,20 @@ DOCKERFILE_PATH=$(yq e "${SERVICE_PATH}.buildTool.docker.dockerfilePath // \"\"" [ "$BUILD_CONTEXT" = "null" ] && BUILD_CONTEXT="" [ "$DOCKERFILE_PATH" = "null" ] && DOCKERFILE_PATH="" -# Apply caller-supplied fallbacks for the build-tool fields when YAML didn't -# provide a value. No hardcoded action-side defaults — the caller owns the -# fallback policy via the default_* inputs. -[ -z "$BUILD_CONTEXT" ] && BUILD_CONTEXT="$DEFAULT_BUILD_CONTEXT" -[ -z "$DOCKERFILE_PATH" ] && DOCKERFILE_PATH="$DEFAULT_DOCKERFILE_PATH" +# Apply caller-supplied defaults when YAML omitted the value (or set it to ""/null). +# Uniform rule: if a default is needed and the input is empty, fail. We log the +# default-applied case as a notice so users can see in the action output that +# the value came from the input default and NOT from their config file. +if [ -z "$BUILD_CONTEXT" ]; then + require_nonempty_default build_context "buildTool.docker.buildContext absent for service '${SERVICE_NAME}'" "$DEFAULT_BUILD_CONTEXT" + echo "::notice::buildTool.docker.buildContext absent for '${SERVICE_NAME}' - using default_build_context='${DEFAULT_BUILD_CONTEXT}'" + BUILD_CONTEXT="$DEFAULT_BUILD_CONTEXT" +fi +if [ -z "$DOCKERFILE_PATH" ]; then + require_nonempty_default dockerfile_path "buildTool.docker.dockerfilePath absent for service '${SERVICE_NAME}'" "$DEFAULT_DOCKERFILE_PATH" + echo "::notice::buildTool.docker.dockerfilePath absent for '${SERVICE_NAME}' - using default_dockerfile_path='${DEFAULT_DOCKERFILE_PATH}'" + DOCKERFILE_PATH="$DEFAULT_DOCKERFILE_PATH" +fi write_output name "$NAME" write_output path "$PATH_VALUE" diff --git a/tests/parse_test.sh b/tests/parse_test.sh index ce67821..8fb9ec1 100755 --- a/tests/parse_test.sh +++ b/tests/parse_test.sh @@ -92,6 +92,11 @@ read_output() { ' "$file" } +# Sentinel defaults used by most tests. Distinctive strings so a regression +# (e.g. defaults silently leaking when YAML had a real value) is obvious. +DEFAULT_BC="DEFAULT_BC_SENTINEL" +DEFAULT_DF="DEFAULT_DF_SENTINEL" + run_parse() { local working_dir="$1" svc="$2" cfg_path="${3:-.skyhook/skyhook.yaml}" local out_file @@ -100,24 +105,8 @@ run_parse() { WORKING_DIR="$working_dir" \ SERVICE_NAME="$svc" \ CONFIG_PATH="$cfg_path" \ - DEFAULT_DOCKERFILE_PATH="${DEFAULT_DOCKERFILE_PATH:-}" \ - DEFAULT_BUILD_CONTEXT="${DEFAULT_BUILD_CONTEXT:-}" \ - GITHUB_OUTPUT="$out_file" \ - bash "$PARSE_SCRIPT" >/dev/null - echo "$out_file" -} - -# Run parse.sh with explicit caller-supplied default_* fallback inputs. -run_parse_with_defaults() { - local working_dir="$1" svc="$2" default_df="$3" default_ctx="$4" - local out_file - out_file="$WORK/gh_output.$RANDOM" - : >"$out_file" - WORKING_DIR="$working_dir" \ - SERVICE_NAME="$svc" \ - CONFIG_PATH=".skyhook/skyhook.yaml" \ - DEFAULT_DOCKERFILE_PATH="$default_df" \ - DEFAULT_BUILD_CONTEXT="$default_ctx" \ + DEFAULT_BUILD_CONTEXT="$DEFAULT_BC" \ + DEFAULT_DOCKERFILE_PATH="$DEFAULT_DF" \ GITHUB_OUTPUT="$out_file" \ bash "$PARSE_SCRIPT" >/dev/null echo "$out_file" @@ -132,6 +121,8 @@ run_parse_expect_fail() { if WORKING_DIR="$working_dir" \ SERVICE_NAME="$svc" \ CONFIG_PATH="$cfg_path" \ + DEFAULT_BUILD_CONTEXT="$DEFAULT_BC" \ + DEFAULT_DOCKERFILE_PATH="$DEFAULT_DF" \ GITHUB_OUTPUT="$out_file" \ bash "$PARSE_SCRIPT" >"$err_file" 2>&1; then echo "EXPECTED-FAIL-DID-NOT-FAIL" @@ -173,53 +164,184 @@ assert_eq "$(read_output "$out" deployment_repo_path)" "nbjkgj" "with-context: d assert_eq "$(read_output "$out" build_context)" "java-web-project/src" "with-context: build_context" assert_eq "$(read_output "$out" dockerfile_path)" "java-web-project/src/Dockerfile" "with-context: dockerfile_path" -# --- buildContext absent + no DEFAULT_BUILD_CONTEXT => empty --- -# The action no longer hardcodes "." — the caller owns the fallback via the -# default_build_context input. Without one supplied, the output is empty. +# --- buildContext absent => caller-supplied default --- out=$(run_parse "$WORK" no-context) -assert_eq "$(read_output "$out" build_context)" "" "no-context: build_context empty when no default supplied" -assert_eq "$(read_output "$out" dockerfile_path)" "java-multi-modules/Dockerfile" "no-context: dockerfile_path" +assert_eq "$(read_output "$out" build_context)" "$DEFAULT_BC" "no-context: build_context falls back to default_build_context" +assert_eq "$(read_output "$out" dockerfile_path)" "java-multi-modules/Dockerfile" "no-context: dockerfile_path comes from YAML (not default)" assert_eq "$(read_output "$out" deployment_repo)" "skyhook-dev/deployment" "no-context: deployment_repo" -# --- buildContext: null + no DEFAULT_BUILD_CONTEXT => empty --- +# --- buildContext: null => caller-supplied default --- out=$(run_parse "$WORK" explicit-null) -assert_eq "$(read_output "$out" build_context)" "" "explicit-null: build_context empty when no default supplied" +assert_eq "$(read_output "$out" build_context)" "$DEFAULT_BC" "explicit-null: build_context falls back to default_build_context" -# --- buildContext: "" + no DEFAULT_BUILD_CONTEXT => empty --- +# --- buildContext: "" => caller-supplied default --- out=$(run_parse "$WORK" empty-string) -assert_eq "$(read_output "$out" build_context)" "" "empty-string: build_context empty when no default supplied" +assert_eq "$(read_output "$out" build_context)" "$DEFAULT_BC" "empty-string: build_context falls back to default_build_context" -# --- DEFAULT_BUILD_CONTEXT fills in when YAML field is absent --- -out=$(run_parse_with_defaults "$WORK" no-context "" ".") -assert_eq "$(read_output "$out" build_context)" "." "no-context + default_build_context='.': output is '.'" +# --- defaults must NOT leak when YAML provides a real value (regression guard) --- +out=$(run_parse "$WORK" with-context) +got_bc=$(read_output "$out" build_context) +got_df=$(read_output "$out" dockerfile_path) +if [ "$got_bc" = "$DEFAULT_BC" ] || [ "$got_df" = "$DEFAULT_DF" ]; then + echo "FAIL: defaults leaked through despite YAML having values (build_context='$got_bc', dockerfile_path='$got_df')" + exit 1 +fi +echo "PASS: with-context: defaults do not override YAML-provided values" -out=$(run_parse_with_defaults "$WORK" explicit-null "" "java-multi-modules") -assert_eq "$(read_output "$out" build_context)" "java-multi-modules" "explicit-null + custom default: output is the default" +# --- script must fail clearly if DEFAULT_BUILD_CONTEXT is unset --- +out_file="$WORK/gh_output.unset_bc" +err_file="$WORK/gh_err.unset_bc" +: >"$out_file" +set +e +env -u DEFAULT_BUILD_CONTEXT \ + WORKING_DIR="$WORK" \ + SERVICE_NAME="with-context" \ + CONFIG_PATH=".skyhook/skyhook.yaml" \ + DEFAULT_DOCKERFILE_PATH="$DEFAULT_DF" \ + GITHUB_OUTPUT="$out_file" \ + bash "$PARSE_SCRIPT" >"$err_file" 2>&1 +rc=$? +set -e +if [ "$rc" -eq 0 ]; then + echo "FAIL: parse.sh succeeded when DEFAULT_BUILD_CONTEXT was unset" + cat "$err_file" + exit 1 +fi +assert_contains "$(cat "$err_file")" "DEFAULT_BUILD_CONTEXT must be set" "unset DEFAULT_BUILD_CONTEXT errors" -out=$(run_parse_with_defaults "$WORK" empty-string "" "svc-empty") -assert_eq "$(read_output "$out" build_context)" "svc-empty" "empty-string + custom default: output is the default" +# --- script must fail clearly if DEFAULT_DOCKERFILE_PATH is unset --- +out_file="$WORK/gh_output.unset_df" +err_file="$WORK/gh_err.unset_df" +: >"$out_file" +set +e +env -u DEFAULT_DOCKERFILE_PATH \ + WORKING_DIR="$WORK" \ + SERVICE_NAME="with-context" \ + CONFIG_PATH=".skyhook/skyhook.yaml" \ + DEFAULT_BUILD_CONTEXT="$DEFAULT_BC" \ + GITHUB_OUTPUT="$out_file" \ + bash "$PARSE_SCRIPT" >"$err_file" 2>&1 +rc=$? +set -e +if [ "$rc" -eq 0 ]; then + echo "FAIL: parse.sh succeeded when DEFAULT_DOCKERFILE_PATH was unset" + cat "$err_file" + exit 1 +fi +assert_contains "$(cat "$err_file")" "DEFAULT_DOCKERFILE_PATH must be set" "unset DEFAULT_DOCKERFILE_PATH errors" -# --- DEFAULT_BUILD_CONTEXT NOT used when YAML provides a value --- -out=$(run_parse_with_defaults "$WORK" with-context "" "should-not-be-used") -assert_eq "$(read_output "$out" build_context)" "java-web-project/src" "yaml value wins over default_build_context" +# Invoke parse.sh with explicit default values (including empty). Captures +# combined output and exit code without aborting the test on non-zero exit. +# Args: +# Sets globals: RC, COMBINED (the captured stdout+stderr). +run_parse_with_defaults() { + local working_dir="$1" svc="$2" def_bc="$3" def_df="$4" + local out_file err_file + out_file="$WORK/gh_output.$RANDOM" + err_file="$WORK/gh_err.$RANDOM" + : >"$out_file" + set +e + WORKING_DIR="$working_dir" \ + SERVICE_NAME="$svc" \ + CONFIG_PATH=".skyhook/skyhook.yaml" \ + DEFAULT_BUILD_CONTEXT="$def_bc" \ + DEFAULT_DOCKERFILE_PATH="$def_df" \ + GITHUB_OUTPUT="$out_file" \ + bash "$PARSE_SCRIPT" >"$err_file" 2>&1 + RC=$? + set -e + COMBINED="$(cat "$err_file")" +} -# --- DEFAULT_DOCKERFILE_PATH fills in when YAML field is absent --- -# (Add a fixture without dockerfilePath to exercise this.) +# --- config file missing + empty default_build_context => fail --- +run_parse_with_defaults "$WORK/no-such-dir" "any-svc" "" "$DEFAULT_DF" +[ "$RC" -ne 0 ] || { echo "FAIL: missing config + empty default_build_context should fail"; echo "$COMBINED"; exit 1; } +assert_contains "$COMBINED" "Config file not found" "missing config + empty BC: error mentions config" +assert_contains "$COMBINED" "default_build_context" "missing config + empty BC: error names default_build_context" + +# --- config file missing + empty default_dockerfile_path => fail --- +run_parse_with_defaults "$WORK/no-such-dir" "any-svc" "$DEFAULT_BC" "" +[ "$RC" -ne 0 ] || { echo "FAIL: missing config + empty default_dockerfile_path should fail"; echo "$COMBINED"; exit 1; } +assert_contains "$COMBINED" "default_dockerfile_path" "missing config + empty DF: error names default_dockerfile_path" + +# --- config file missing + both defaults empty => fail, listing both --- +run_parse_with_defaults "$WORK/no-such-dir" "any-svc" "" "" +[ "$RC" -ne 0 ] || { echo "FAIL: missing config + both defaults empty should fail"; echo "$COMBINED"; exit 1; } +assert_contains "$COMBINED" "default_build_context" "missing config + both empty: lists default_build_context" +assert_contains "$COMBINED" "default_dockerfile_path" "missing config + both empty: lists default_dockerfile_path" + +# --- service not found + empty default_build_context => fail --- +run_parse_with_defaults "$WORK" "nonexistent-service" "" "$DEFAULT_DF" +[ "$RC" -ne 0 ] || { echo "FAIL: missing service + empty default_build_context should fail"; echo "$COMBINED"; exit 1; } +assert_contains "$COMBINED" "Service 'nonexistent-service' not found" "missing service + empty BC: error mentions service" +assert_contains "$COMBINED" "default_build_context" "missing service + empty BC: error names default_build_context" + +# --- service not found + empty default_dockerfile_path => fail --- +run_parse_with_defaults "$WORK" "nonexistent-service" "$DEFAULT_BC" "" +[ "$RC" -ne 0 ] || { echo "FAIL: missing service + empty default_dockerfile_path should fail"; echo "$COMBINED"; exit 1; } +assert_contains "$COMBINED" "default_dockerfile_path" "missing service + empty DF: error names default_dockerfile_path" + +# --- service not found + both defaults non-empty => still succeeds --- +run_parse_with_defaults "$WORK" "nonexistent-service" "$DEFAULT_BC" "$DEFAULT_DF" +[ "$RC" -eq 0 ] || { echo "FAIL: missing service + non-empty defaults should succeed"; echo "$COMBINED"; exit 1; } +echo "PASS: missing service with non-empty defaults still succeeds" + +# --- UNIFORM RULE: service FOUND but YAML field absent + empty default => fail --- +# The fail-fast rule is uniform: if a default is ever applied and is empty, fail. +run_parse_with_defaults "$WORK" "no-context" "" "$DEFAULT_DF" +[ "$RC" -ne 0 ] || { echo "FAIL: service-found + empty default_build_context must now fail (uniform rule)"; echo "$COMBINED"; exit 1; } +assert_contains "$COMBINED" "buildTool.docker.buildContext absent" "uniform: YAML-absent + empty BC: error mentions which YAML field" +assert_contains "$COMBINED" "default_build_context input is empty" "uniform: YAML-absent + empty BC: error names the input" + +# Same for dockerfile_path: use a service whose YAML omits dockerfilePath. +# Add a fixture for that purpose. NO_DF_DIR="$WORK/no-df" mkdir -p "$NO_DF_DIR/.skyhook" cat >"$NO_DF_DIR/.skyhook/skyhook.yaml" <<'YAML' services: - - name: bare - path: services/bare + - name: no-df + path: svc-no-df + buildTool: + docker: + buildContext: svc-no-df YAML -out=$(run_parse_with_defaults "$NO_DF_DIR" bare "services/bare/Dockerfile" "services/bare") -assert_eq "$(read_output "$out" dockerfile_path)" "services/bare/Dockerfile" "bare service + default_dockerfile_path: output is the default" -assert_eq "$(read_output "$out" build_context)" "services/bare" "bare service + default_build_context: output is the default" -assert_eq "$(read_output "$out" service_found)" "true" "bare service still reports service_found=true" - -# --- DEFAULT_DOCKERFILE_PATH NOT used when YAML provides a value --- -out=$(run_parse_with_defaults "$WORK" with-context "should-not-be-used" "") -assert_eq "$(read_output "$out" dockerfile_path)" "java-web-project/src/Dockerfile" "yaml value wins over default_dockerfile_path" +run_parse_with_defaults "$NO_DF_DIR" "no-df" "$DEFAULT_BC" "" +[ "$RC" -ne 0 ] || { echo "FAIL: service-found + empty default_dockerfile_path must now fail (uniform rule)"; echo "$COMBINED"; exit 1; } +assert_contains "$COMBINED" "buildTool.docker.dockerfilePath absent" "uniform: YAML-absent + empty DF: error mentions which YAML field" +assert_contains "$COMBINED" "default_dockerfile_path input is empty" "uniform: YAML-absent + empty DF: error names the input" + +# --- "::notice::" is emitted when a non-empty default is applied (informational) --- +notice_log="$WORK/notice.log" +WORKING_DIR="$WORK" \ +SERVICE_NAME="no-context" \ +CONFIG_PATH=".skyhook/skyhook.yaml" \ +DEFAULT_BUILD_CONTEXT="$DEFAULT_BC" \ +DEFAULT_DOCKERFILE_PATH="$DEFAULT_DF" \ +GITHUB_OUTPUT="$WORK/gh_output.notice" \ + bash "$PARSE_SCRIPT" >"$notice_log" 2>&1 +assert_contains "$(cat "$notice_log")" "::notice::buildTool.docker.buildContext absent" "notice emitted when build_context default applied" +assert_contains "$(cat "$notice_log")" "$DEFAULT_BC" "notice includes the default value" + +# --- not-found paths emit ::warning:: (more prominent than the per-field ::notice::) --- +warn_log="$WORK/warn.log" +WORKING_DIR="$WORK" \ +SERVICE_NAME="nonexistent-service" \ +CONFIG_PATH=".skyhook/skyhook.yaml" \ +DEFAULT_BUILD_CONTEXT="$DEFAULT_BC" \ +DEFAULT_DOCKERFILE_PATH="$DEFAULT_DF" \ +GITHUB_OUTPUT="$WORK/gh_output.warn" \ + bash "$PARSE_SCRIPT" >"$warn_log" 2>&1 +assert_contains "$(cat "$warn_log")" "::warning::Service 'nonexistent-service' not found" "service-not-found emits ::warning::" + +warn_log2="$WORK/warn2.log" +WORKING_DIR="$WORK/no-such-dir" \ +SERVICE_NAME="anything" \ +CONFIG_PATH=".skyhook/skyhook.yaml" \ +DEFAULT_BUILD_CONTEXT="$DEFAULT_BC" \ +DEFAULT_DOCKERFILE_PATH="$DEFAULT_DF" \ +GITHUB_OUTPUT="$WORK/gh_output.warn2" \ + bash "$PARSE_SCRIPT" >"$warn_log2" 2>&1 +assert_contains "$(cat "$warn_log2")" "::warning::Config file not found" "config-not-found emits ::warning::" # --- value with spaces survives heredoc round-trip --- out=$(run_parse "$WORK" with-spaces) @@ -231,32 +353,21 @@ out=$(run_parse "$WORK" 'name-with-"-quote') assert_eq "$(read_output "$out" service_found)" "true" "quoted service name resolves" assert_eq "$(read_output "$out" path)" "svc-quote" "quoted service name: path" -# --- service not found, no defaults => empty --- +# --- service not found => identity fields empty, build/dockerfile fall back to defaults --- out=$(run_parse "$WORK" nonexistent-service) assert_eq "$(read_output "$out" config_found)" "true" "missing service: config_found=true" assert_eq "$(read_output "$out" service_found)" "false" "missing service: service_found=false" -assert_eq "$(read_output "$out" build_context)" "" "missing service: build_context empty when no default" -assert_eq "$(read_output "$out" dockerfile_path)" "" "missing service: dockerfile_path empty when no default" +assert_eq "$(read_output "$out" name)" "" "missing service: name stays empty" +assert_eq "$(read_output "$out" path)" "" "missing service: path stays empty" +assert_eq "$(read_output "$out" build_context)" "$DEFAULT_BC" "missing service: build_context = default_build_context" +assert_eq "$(read_output "$out" dockerfile_path)" "$DEFAULT_DF" "missing service: dockerfile_path = default_dockerfile_path" -# --- service not found, WITH defaults => defaults are emitted --- -out=$(run_parse_with_defaults "$WORK" nonexistent-service "Dockerfile" ".") -assert_eq "$(read_output "$out" config_found)" "true" "missing service + defaults: config_found=true" -assert_eq "$(read_output "$out" service_found)" "false" "missing service + defaults: service_found=false" -assert_eq "$(read_output "$out" build_context)" "." "missing service + defaults: build_context fallback applied" -assert_eq "$(read_output "$out" dockerfile_path)" "Dockerfile" "missing service + defaults: dockerfile_path fallback applied" - -# --- config file missing, no defaults => empty --- +# --- config file missing => same fallback contract as missing service --- out=$(run_parse "$WORK/no-such-dir" any-service) assert_eq "$(read_output "$out" config_found)" "false" "missing config: config_found=false" assert_eq "$(read_output "$out" service_found)" "false" "missing config: service_found=false" -assert_eq "$(read_output "$out" build_context)" "" "missing config: build_context empty when no default" -assert_eq "$(read_output "$out" dockerfile_path)" "" "missing config: dockerfile_path empty when no default" - -# --- config file missing, WITH defaults => defaults are emitted --- -out=$(run_parse_with_defaults "$WORK/no-such-dir" any-service "fallback/Dockerfile" "fallback") -assert_eq "$(read_output "$out" config_found)" "false" "missing config + defaults: config_found=false" -assert_eq "$(read_output "$out" build_context)" "fallback" "missing config + defaults: build_context fallback applied" -assert_eq "$(read_output "$out" dockerfile_path)" "fallback/Dockerfile" "missing config + defaults: dockerfile_path fallback applied" +assert_eq "$(read_output "$out" build_context)" "$DEFAULT_BC" "missing config: build_context = default_build_context" +assert_eq "$(read_output "$out" dockerfile_path)" "$DEFAULT_DF" "missing config: dockerfile_path = default_dockerfile_path" # --- empty service_name --- err=$(run_parse_expect_fail "$WORK" "") @@ -296,7 +407,7 @@ services: YAML out_file="$WORK/gh_output.cwd" : >"$out_file" -( cd "$TMP_CWD" && WORKING_DIR="" SERVICE_NAME="cwd-svc" CONFIG_PATH=".skyhook/skyhook.yaml" GITHUB_OUTPUT="$out_file" bash "$PARSE_SCRIPT" >/dev/null ) +( cd "$TMP_CWD" && WORKING_DIR="" SERVICE_NAME="cwd-svc" CONFIG_PATH=".skyhook/skyhook.yaml" DEFAULT_BUILD_CONTEXT="$DEFAULT_BC" DEFAULT_DOCKERFILE_PATH="$DEFAULT_DF" GITHUB_OUTPUT="$out_file" bash "$PARSE_SCRIPT" >/dev/null ) assert_eq "$(read_output "$out_file" service_found)" "true" "empty WORKING_DIR collapses to cwd" # --- trailing slash on WORKING_DIR is stripped --- @@ -331,7 +442,16 @@ grep -q 'version v?4\\.' "$ACTION_FILE" || { echo "FAIL: action.yml does not val grep -q 'v4.47.1' "$ACTION_FILE" || { echo "FAIL: action.yml does not pin yq v4.47.1"; exit 1; } grep -q "context_path" "$ACTION_FILE" && { echo "FAIL: action.yml still references old context_path"; exit 1; } grep -q "build_context:" "$ACTION_FILE" || { echo "FAIL: action.yml does not declare build_context output"; exit 1; } -echo "PASS: action.yml delegates to scripts/parse.sh, validates yq v4, pins v4.47.1, no context_path residue" +grep -q "default_build_context:" "$ACTION_FILE" || { echo "FAIL: action.yml does not declare default_build_context input"; exit 1; } +grep -q "default_dockerfile_path:" "$ACTION_FILE" || { echo "FAIL: action.yml does not declare default_dockerfile_path input"; exit 1; } +grep -q "DEFAULT_BUILD_CONTEXT:" "$ACTION_FILE" || { echo "FAIL: action.yml does not wire DEFAULT_BUILD_CONTEXT env"; exit 1; } +grep -q "DEFAULT_DOCKERFILE_PATH:" "$ACTION_FILE" || { echo "FAIL: action.yml does not wire DEFAULT_DOCKERFILE_PATH env"; exit 1; } +# Both default inputs must be required:true (no implicit defaults) +awk '/^ default_build_context:/{f=1} f && /required: true/{print "OK"; exit} f && /^ [a-z]/ && !/^ default_build_context:/{exit}' "$ACTION_FILE" | grep -q OK \ + || { echo "FAIL: default_build_context is not required: true"; exit 1; } +awk '/^ default_dockerfile_path:/{f=1} f && /required: true/{print "OK"; exit} f && /^ [a-z]/ && !/^ default_dockerfile_path:/{exit}' "$ACTION_FILE" | grep -q OK \ + || { echo "FAIL: default_dockerfile_path is not required: true"; exit 1; } +echo "PASS: action.yml delegates to scripts/parse.sh, validates yq v4, pins v4.47.1, declares required default inputs, no context_path residue" # --- parse.sh structure sanity --- grep -q "strenv(SERVICE_NAME)" "$PARSE_SCRIPT" || { echo "FAIL: parse.sh does not use strenv() for service_name"; exit 1; } @@ -340,11 +460,6 @@ grep -q "service_name input is required" "$PARSE_SCRIPT" || { echo "FAIL: parse. grep -q 'Failed to parse' "$PARSE_SCRIPT" || { echo "FAIL: parse.sh does not surface yq parse errors"; exit 1; } grep -q 'DEFAULT_BUILD_CONTEXT' "$PARSE_SCRIPT" || { echo "FAIL: parse.sh does not consume DEFAULT_BUILD_CONTEXT"; exit 1; } grep -q 'DEFAULT_DOCKERFILE_PATH' "$PARSE_SCRIPT" || { echo "FAIL: parse.sh does not consume DEFAULT_DOCKERFILE_PATH"; exit 1; } -echo "PASS: parse.sh has strenv, dup-detection, empty-name validation, parse-error surfacing, caller-driven defaults" - -# --- action.yml exposes the new default_* inputs --- -grep -q 'default_dockerfile_path:' "$ACTION_FILE" || { echo "FAIL: action.yml does not declare default_dockerfile_path input"; exit 1; } -grep -q 'default_build_context:' "$ACTION_FILE" || { echo "FAIL: action.yml does not declare default_build_context input"; exit 1; } -grep -q 'DEFAULT_DOCKERFILE_PATH: ${{ inputs.default_dockerfile_path }}' "$ACTION_FILE" || { echo "FAIL: action.yml does not pipe default_dockerfile_path to parse.sh"; exit 1; } -grep -q 'DEFAULT_BUILD_CONTEXT: ${{ inputs.default_build_context }}' "$ACTION_FILE" || { echo "FAIL: action.yml does not pipe default_build_context to parse.sh"; exit 1; } -echo "PASS: action.yml exposes default_dockerfile_path + default_build_context inputs piped to parse.sh" +# The script must NOT silently default build_context to "." anymore. +grep -q 'BUILD_CONTEXT="\."' "$PARSE_SCRIPT" && { echo "FAIL: parse.sh still hard-codes BUILD_CONTEXT=\".\" - defaults must come from caller input"; exit 1; } +echo "PASS: parse.sh has strenv, dup-detection, empty-name validation, parse-error surfacing, caller-supplied defaults"