From 61bd6692721d371ac73eab0ab47e88e1e8aae3b7 Mon Sep 17 00:00:00 2001 From: eliran-mic Date: Mon, 4 May 2026 17:35:30 +0300 Subject: [PATCH] feat: add required default_build_context / default_dockerfile_path inputs The action requires callers to pass two non-empty defaults for build_context and dockerfile_path. Whenever a default would be applied (YAML field absent, config file missing, or service missing), the corresponding default input is emitted. If a default is needed and the input is empty, the action exits 1 with a clear error pointing at the offending input(s). Logging: - ::notice:: when a default is applied for an otherwise-found service (the YAML simply omitted the field; informational) - ::warning:: when the entire config or service is missing and defaults flow through (more prominent - usually a misconfiguration) Tests cover per-field fail-fast, both-empty collect-all error, ::warning:: prefix on not-found paths, and a regression guard that defaults do not leak when the YAML provides a value (57 PASS). Co-authored-by: Cursor --- .github/workflows/test.yml | 25 ++++- README.md | 49 ++++++--- action.yml | 18 ++- scripts/parse.sh | 84 ++++++++++++-- tests/parse_test.sh | 220 ++++++++++++++++++++++++++++++++++--- 5 files changed, 352 insertions(+), 44 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 158f355..96680ad 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -43,6 +43,8 @@ jobs: with: working_directory: tests/fixtures service_name: smoke-svc + default_build_context: '.' + default_dockerfile_path: Dockerfile - name: Verify outputs run: | @@ -57,12 +59,14 @@ jobs: test "${{ steps.cfg.outputs.dockerfile_path }}" = "services/smoke/Dockerfile" echo "smoke-svc outputs OK" - - name: Run action against fixture (buildContext absent -> 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 default-context outputs run: | @@ -70,3 +74,22 @@ jobs: test "${{ steps.cfg2.outputs.build_context }}" = "." test "${{ steps.cfg2.outputs.dockerfile_path }}" = "services/default/Dockerfile" echo "default-context outputs OK" + + - name: Run action against fixture (service missing, defaults flow through) + id: cfg3 + uses: ./ + with: + working_directory: tests/fixtures + service_name: does-not-exist + default_build_context: '.' + default_dockerfile_path: Dockerfile + + - name: Verify missing-service fallback outputs + run: | + set -e + 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 a4fba51..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,6 +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_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 @@ -44,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: @@ -68,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: @@ -94,32 +100,45 @@ 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 -| Scenario | `config_found` | `service_found` | `build_context` | Other outputs | -|---|---|---|---|---| -| Config file missing | `false` | `false` | `""` | `""` | -| Config found, service missing | `true` | `false` | `""` | `""` | -| Service found, `buildContext` set | `true` | `true` | from config | from config | -| Service found, `buildContext` absent | `true` | `true` | `"."` | from config | -| Duplicate service names in config | n/a | n/a | n/a | action exits 1 | +Let `BC` = `default_build_context` input, `DF` = `default_dockerfile_path` input. + +**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. -`build_context` defaults to `"."` only when the service is found and the field is absent. When the service or config itself is missing, `build_context` is empty - the workflow should decide whether to fall back or fail loudly: +| 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 | + +The action **always** emits a non-empty `build_context` and `dockerfile_path` on success, so the consuming workflow can drop `||` fallbacks: ```yaml -context: ${{ steps.config.outputs.build_context || '.' }} -dockerfile: ${{ steps.config.outputs.dockerfile_path || 'Dockerfile' }} +context: code/${{ steps.config.outputs.build_context }} +dockerfile: code/${{ steps.config.outputs.dockerfile_path }} ``` +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 - Bash + `yq` v4.x. The action installs yq v4.47.1 if missing. diff --git a/action.yml b/action.yml index 173e9fb..3a8ba1a 100644 --- a/action.yml +++ b/action.yml @@ -14,6 +14,18 @@ inputs: description: 'Path to the skyhook config file relative to working_directory' required: false default: '.skyhook/skyhook.yaml' + default_build_context: + 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 @@ -32,10 +44,10 @@ outputs: # Build tool configuration build_context: - description: 'Docker build context relative to repo root (defaults to "." when absent in config)' + 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' + 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 @@ -110,6 +122,8 @@ runs: WORKING_DIR: ${{ inputs.working_directory }} SERVICE_NAME: ${{ inputs.service_name }} CONFIG_PATH: ${{ inputs.config_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 812eb4c..7f32e78 100755 --- a/scripts/parse.sh +++ b/scripts/parse.sh @@ -2,19 +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") -# GITHUB_OUTPUT - file to append outputs to (required by GitHub Actions; tests pass a tempfile) +# 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="." @@ -42,21 +55,56 @@ write_output() { } >> "$GITHUB_OUTPUT" } -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 "" write_output deployment_repo_path "" - write_output build_context "" - write_output dockerfile_path "" + write_output build_context "$DEFAULT_BUILD_CONTEXT" + 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 @@ -75,8 +123,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 @@ -114,8 +164,20 @@ DOCKERFILE_PATH=$(yq e "${SERVICE_PATH}.buildTool.docker.dockerfilePath // \"\"" [ "$BUILD_CONTEXT" = "null" ] && BUILD_CONTEXT="" [ "$DOCKERFILE_PATH" = "null" ] && DOCKERFILE_PATH="" -# Default build context to "." when absent -[ -z "$BUILD_CONTEXT" ] && BUILD_CONTEXT="." +# 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 6eb209d..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,6 +105,8 @@ run_parse() { 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" >/dev/null echo "$out_file" @@ -114,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" @@ -155,19 +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 => default "." --- +# --- buildContext absent => caller-supplied default --- out=$(run_parse "$WORK" no-context) -assert_eq "$(read_output "$out" build_context)" "." "no-context: build_context defaults to '.'" -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 => default "." --- +# --- buildContext: null => caller-supplied default --- out=$(run_parse "$WORK" explicit-null) -assert_eq "$(read_output "$out" build_context)" "." "explicit-null: build_context defaults to '.'" +assert_eq "$(read_output "$out" build_context)" "$DEFAULT_BC" "explicit-null: build_context falls back to default_build_context" -# --- buildContext: "" => default "." --- +# --- buildContext: "" => caller-supplied default --- out=$(run_parse "$WORK" empty-string) -assert_eq "$(read_output "$out" build_context)" "." "empty-string: build_context defaults to '.'" +assert_eq "$(read_output "$out" build_context)" "$DEFAULT_BC" "empty-string: build_context falls back to default_build_context" + +# --- 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" + +# --- 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" + +# --- 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" + +# 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")" +} + +# --- 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: no-df + path: svc-no-df + buildTool: + docker: + buildContext: svc-no-df +YAML +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) @@ -179,17 +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 --- +# --- 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 (asymmetry)" +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" -# --- config file missing --- +# --- 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" +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" "") @@ -229,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 --- @@ -264,12 +442,24 @@ 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; } grep -q "Multiple services named" "$PARSE_SCRIPT" || { echo "FAIL: parse.sh does not detect duplicate names"; exit 1; } grep -q "service_name input is required" "$PARSE_SCRIPT" || { echo "FAIL: parse.sh does not validate non-empty service_name"; exit 1; } grep -q 'Failed to parse' "$PARSE_SCRIPT" || { echo "FAIL: parse.sh does not surface yq parse errors"; exit 1; } -grep -q 'BUILD_CONTEXT="\."' "$PARSE_SCRIPT" || { echo "FAIL: parse.sh does not default build_context to '.'"; exit 1; } -echo "PASS: parse.sh has strenv, dup-detection, empty-name validation, parse-error surfacing, build_context default" +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; } +# 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"