From 601f97ce0fb6a0c6fde6d5ecb3480e93a5ca6433 Mon Sep 17 00:00:00 2001 From: eliran-mic Date: Thu, 30 Apr 2026 21:24:25 +0300 Subject: [PATCH 1/3] feat: initial v1 release with buildContext schema Commits the previously-uncommitted action.yml plus tests, with the build-context field renamed from contextPath to buildContext and a default of "." applied when the field is absent. Schema change vs. local draft: - buildTool.docker.contextPath -> buildTool.docker.buildContext - output context_path -> build_context - build_context defaults to "." when buildContext is absent (was "") Why buildContext: matches docker's own term ("build context"), pairs naturally with dockerfilePath, drops the redundant Path suffix. Tests: tests/parse_test.sh exercises both the populated and absent cases, plus checks that action.yml no longer references the old context_path identifier. Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 21 +++--- action.yml | 153 +++++++++++++++++++++++++++++++++++++++++ read-config/README.md | 115 +++++++++++++++++++++++++++++++ tests/metadata_test.sh | 20 ++++++ tests/parse_test.sh | 75 ++++++++++++++++++++ 5 files changed, 373 insertions(+), 11 deletions(-) create mode 100644 action.yml create mode 100644 read-config/README.md create mode 100644 tests/metadata_test.sh create mode 100755 tests/parse_test.sh diff --git a/README.md b/README.md index d62e7bb..6a5636e 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ A GitHub Action that reads service configuration from `.skyhook/skyhook.yaml`. This action parses the Skyhook configuration file and extracts service-specific settings including: - Service path - Deployment repository configuration -- Docker build tool settings (context path, dockerfile path) +- Docker build tool settings (build context, dockerfile path) ## Usage @@ -23,7 +23,7 @@ This action parses the Skyhook configuration file and extracts service-specific run: | docker build \ -f ${{ steps.config.outputs.dockerfile_path || 'Dockerfile' }} \ - ${{ steps.config.outputs.context_path || '.' }} + ${{ steps.config.outputs.build_context }} ``` ## Inputs @@ -42,7 +42,7 @@ 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 | -| `context_path` | Docker build context path relative to repo root | +| `build_context` | Docker build context relative to repo root (defaults to `.` when absent in config) | | `dockerfile_path` | Dockerfile path relative to repo root | | `config_found` | Whether the config file was found (`true`/`false`) | | `service_found` | Whether the service was found in config (`true`/`false`) | @@ -59,15 +59,15 @@ services: deploymentRepoPath: services/my-service buildTool: docker: - contextPath: services/my-service + buildContext: services/my-service dockerfilePath: docker/Dockerfile.my-service - name: another-service path: services/another buildTool: docker: - contextPath: . - # dockerfilePath defaults to {contextPath}/Dockerfile if not specified + # buildContext omitted - defaults to "." + dockerfilePath: services/another/Dockerfile environments: - name: dev @@ -96,18 +96,17 @@ jobs: - name: Build and push Docker image uses: skyhook-io/docker-build-push-action@v1 with: - # Use config values with fallbacks - context: code/${{ steps.config.outputs.context_path || env.SERVICE_DIR }} - dockerfile: code/${{ steps.config.outputs.dockerfile_path || format('{0}/Dockerfile', steps.config.outputs.context_path || env.SERVICE_DIR) }} + context: code/${{ steps.config.outputs.build_context }} + dockerfile: code/${{ steps.config.outputs.dockerfile_path || format('{0}/Dockerfile', steps.config.outputs.build_context) }} image: ${{ inputs.image }} ``` ## Fallback Behavior -If the config file doesn't exist or the service isn't found, all output values will be empty strings. This allows workflows to use fallback values: +If the config file doesn't exist or the service isn't found, all output values will be empty strings (except `build_context`, which always defaults to `.` when the service is found but `buildContext` is absent). When the service is missing entirely, fall back in the workflow: ```yaml -context: ${{ steps.config.outputs.context_path || '.' }} +context: ${{ steps.config.outputs.build_context || '.' }} dockerfile: ${{ steps.config.outputs.dockerfile_path || 'Dockerfile' }} ``` diff --git a/action.yml b/action.yml new file mode 100644 index 0000000..aa0b2b4 --- /dev/null +++ b/action.yml @@ -0,0 +1,153 @@ +name: 'Read Skyhook Config' +description: 'Reads service configuration from .skyhook/skyhook.yaml' +author: 'Skyhook' + +inputs: + working_directory: + description: 'Path to the repository root containing .skyhook/skyhook.yaml' + required: false + default: '.' + service_name: + description: 'Name of the service to look up in the config' + required: true + config_path: + description: 'Path to the skyhook config file relative to working_directory' + required: false + default: '.skyhook/skyhook.yaml' + +outputs: + # Service configuration + name: + description: 'Service name from config' + value: ${{ steps.parse.outputs.name }} + path: + description: 'Service path relative to repo root' + value: ${{ steps.parse.outputs.path }} + deployment_repo: + description: 'Separate deployment repository (if configured)' + value: ${{ steps.parse.outputs.deployment_repo }} + deployment_repo_path: + description: 'Path within deployment repository' + value: ${{ steps.parse.outputs.deployment_repo_path }} + + # Build tool configuration + build_context: + description: 'Docker build context relative to repo root (defaults to "." when absent in config)' + value: ${{ steps.parse.outputs.build_context }} + dockerfile_path: + description: 'Dockerfile path relative to repo root' + value: ${{ steps.parse.outputs.dockerfile_path }} + + # Status + config_found: + description: 'Whether the config file was found (true/false)' + value: ${{ steps.parse.outputs.config_found }} + service_found: + description: 'Whether the service was found in config (true/false)' + value: ${{ steps.parse.outputs.service_found }} + +runs: + using: 'composite' + steps: + - name: Install yq + shell: bash + run: | + if ! command -v yq &> /dev/null; then + echo "Installing yq..." + sudo wget -qO /usr/local/bin/yq https://github.com/mikefarah/yq/releases/latest/download/yq_linux_amd64 + sudo chmod +x /usr/local/bin/yq + fi + + - name: Parse skyhook config + id: parse + shell: bash + env: + WORKING_DIR: ${{ inputs.working_directory }} + SERVICE_NAME: ${{ inputs.service_name }} + CONFIG_PATH: ${{ inputs.config_path }} + run: | + CONFIG_FILE="${WORKING_DIR}/${CONFIG_PATH}" + + # Check if config file exists + if [ ! -f "$CONFIG_FILE" ]; then + echo "Config file not found: $CONFIG_FILE" + echo "config_found=false" >> $GITHUB_OUTPUT + echo "service_found=false" >> $GITHUB_OUTPUT + # Set empty outputs for all fields + echo "name=" >> $GITHUB_OUTPUT + echo "path=" >> $GITHUB_OUTPUT + echo "deployment_repo=" >> $GITHUB_OUTPUT + echo "deployment_repo_path=" >> $GITHUB_OUTPUT + echo "build_context=" >> $GITHUB_OUTPUT + echo "dockerfile_path=" >> $GITHUB_OUTPUT + exit 0 + fi + + echo "config_found=true" >> $GITHUB_OUTPUT + echo "Found config file: $CONFIG_FILE" + + # Find the service by name in the services array + # Use yq to query the YAML + SERVICE_INDEX=$(yq e ".services | to_entries | .[] | select(.value.name == \"${SERVICE_NAME}\") | .key" "$CONFIG_FILE" 2>/dev/null || echo "") + + if [ -z "$SERVICE_INDEX" ]; then + echo "Service '$SERVICE_NAME' not found in config" + echo "service_found=false" >> $GITHUB_OUTPUT + # Set empty outputs + echo "name=" >> $GITHUB_OUTPUT + echo "path=" >> $GITHUB_OUTPUT + echo "deployment_repo=" >> $GITHUB_OUTPUT + echo "deployment_repo_path=" >> $GITHUB_OUTPUT + echo "build_context=" >> $GITHUB_OUTPUT + echo "dockerfile_path=" >> $GITHUB_OUTPUT + exit 0 + fi + + echo "service_found=true" >> $GITHUB_OUTPUT + echo "Found service '$SERVICE_NAME' at index $SERVICE_INDEX" + + # Extract service configuration + SERVICE_PATH=".services[$SERVICE_INDEX]" + + # Get basic service fields + NAME=$(yq e "${SERVICE_PATH}.name // \"\"" "$CONFIG_FILE") + PATH_VALUE=$(yq e "${SERVICE_PATH}.path // \"\"" "$CONFIG_FILE") + DEPLOYMENT_REPO=$(yq e "${SERVICE_PATH}.deploymentRepo // \"\"" "$CONFIG_FILE") + DEPLOYMENT_REPO_PATH=$(yq e "${SERVICE_PATH}.deploymentRepoPath // \"\"" "$CONFIG_FILE") + + # Get buildTool.docker configuration + BUILD_CONTEXT=$(yq e "${SERVICE_PATH}.buildTool.docker.buildContext // \"\"" "$CONFIG_FILE") + DOCKERFILE_PATH=$(yq e "${SERVICE_PATH}.buildTool.docker.dockerfilePath // \"\"" "$CONFIG_FILE") + + # Handle "null" strings from yq + [ "$NAME" = "null" ] && NAME="" + [ "$PATH_VALUE" = "null" ] && PATH_VALUE="" + [ "$DEPLOYMENT_REPO" = "null" ] && DEPLOYMENT_REPO="" + [ "$DEPLOYMENT_REPO_PATH" = "null" ] && DEPLOYMENT_REPO_PATH="" + [ "$BUILD_CONTEXT" = "null" ] && BUILD_CONTEXT="" + [ "$DOCKERFILE_PATH" = "null" ] && DOCKERFILE_PATH="" + + # Default build context to "." when absent + [ -z "$BUILD_CONTEXT" ] && BUILD_CONTEXT="." + + # Output values + echo "name=${NAME}" >> $GITHUB_OUTPUT + echo "path=${PATH_VALUE}" >> $GITHUB_OUTPUT + echo "deployment_repo=${DEPLOYMENT_REPO}" >> $GITHUB_OUTPUT + echo "deployment_repo_path=${DEPLOYMENT_REPO_PATH}" >> $GITHUB_OUTPUT + echo "build_context=${BUILD_CONTEXT}" >> $GITHUB_OUTPUT + echo "dockerfile_path=${DOCKERFILE_PATH}" >> $GITHUB_OUTPUT + + # Log outputs for debugging + echo "Parsed service configuration:" + echo " name: ${NAME}" + echo " path: ${PATH_VALUE}" + echo " deployment_repo: ${DEPLOYMENT_REPO}" + echo " deployment_repo_path: ${DEPLOYMENT_REPO_PATH}" + echo " build_context: ${BUILD_CONTEXT}" + echo " dockerfile_path: ${DOCKERFILE_PATH}" + +branding: + icon: 'file-text' + color: 'blue' + diff --git a/read-config/README.md b/read-config/README.md new file mode 100644 index 0000000..6a5636e --- /dev/null +++ b/read-config/README.md @@ -0,0 +1,115 @@ +# read-config + +A GitHub Action that reads service configuration from `.skyhook/skyhook.yaml`. + +## Description + +This action parses the Skyhook configuration file and extracts service-specific settings including: +- Service path +- Deployment repository configuration +- Docker build tool settings (build context, dockerfile path) + +## Usage + +```yaml +- name: Read service config + id: config + uses: skyhook-io/read-config@v1 + with: + working_directory: code + service_name: my-service + +- name: Build Docker image + run: | + docker build \ + -f ${{ steps.config.outputs.dockerfile_path || 'Dockerfile' }} \ + ${{ steps.config.outputs.build_context }} +``` + +## Inputs + +| Input | Description | Required | Default | +|-------|-------------|----------|---------| +| `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` | + +## Outputs + +| Output | Description | +|--------|-------------| +| `name` | Service name from config | +| `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 | +| `config_found` | Whether the config file was found (`true`/`false`) | +| `service_found` | Whether the service was found in config (`true`/`false`) | + +## Config File Format + +The action expects a `.skyhook/skyhook.yaml` file with the following structure: + +```yaml +services: + - name: my-service + path: services/my-service + deploymentRepo: org/deployment-repo + deploymentRepoPath: services/my-service + buildTool: + docker: + buildContext: services/my-service + dockerfilePath: docker/Dockerfile.my-service + + - name: another-service + path: services/another + buildTool: + docker: + # buildContext omitted - defaults to "." + dockerfilePath: services/another/Dockerfile + +environments: + - name: dev + clusterName: dev-cluster + namespace: dev +``` + +## Example: Build with Dynamic Config + +```yaml +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v5 + with: + path: code + + - name: Read service config + id: config + uses: skyhook-io/read-config@v1 + with: + working_directory: code + service_name: ${{ env.SERVICE_NAME }} + + - 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) }} + image: ${{ inputs.image }} +``` + +## Fallback Behavior + +If the config file doesn't exist or the service isn't found, all output values will be empty strings (except `build_context`, which always defaults to `.` when the service is found but `buildContext` is absent). When the service is missing entirely, fall back in the workflow: + +```yaml +context: ${{ steps.config.outputs.build_context || '.' }} +dockerfile: ${{ steps.config.outputs.dockerfile_path || 'Dockerfile' }} +``` + +## License + +MIT diff --git a/tests/metadata_test.sh b/tests/metadata_test.sh new file mode 100644 index 0000000..4fc727e --- /dev/null +++ b/tests/metadata_test.sh @@ -0,0 +1,20 @@ +#!/usr/bin/env bash +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +ACTION_FILE="${REPO_ROOT}/action.yml" + +if [[ ! -f "${ACTION_FILE}" ]]; then + echo "FAIL: action.yml not found at repository root (${ACTION_FILE})" + exit 1 +fi + +echo "Checking basic action metadata in ${ACTION_FILE}..." + +grep -q "^name:" "${ACTION_FILE}" || { echo "FAIL: missing top-level 'name' field"; exit 1; } +grep -q "^description:" "${ACTION_FILE}" || { echo "FAIL: missing top-level 'description' field"; exit 1; } +grep -q "^runs:" "${ACTION_FILE}" || { echo "FAIL: missing top-level 'runs' section"; exit 1; } +grep -q "using: 'composite'" "${ACTION_FILE}" || { echo "FAIL: expected runs.using to be 'composite'"; exit 1; } + +echo "PASS: action.yml metadata looks valid for GitHub Marketplace." + diff --git a/tests/parse_test.sh b/tests/parse_test.sh new file mode 100755 index 0000000..9d82d70 --- /dev/null +++ b/tests/parse_test.sh @@ -0,0 +1,75 @@ +#!/usr/bin/env bash +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +ACTION_FILE="${REPO_ROOT}/action.yml" + +if ! command -v yq &>/dev/null; then + echo "SKIP: yq not installed; install via 'brew install yq' or download from mikefarah/yq" + exit 0 +fi + +WORK=$(mktemp -d) +trap 'rm -rf "$WORK"' EXIT + +CONFIG_DIR="$WORK/.skyhook" +mkdir -p "$CONFIG_DIR" +CONFIG_FILE="$CONFIG_DIR/skyhook.yaml" + +cat >"$CONFIG_FILE" <<'YAML' +services: + - name: with-context + path: java-web-project + deploymentRepo: KoalaOps/deployment + deploymentRepoPath: nbjkgj + buildTool: + docker: + buildContext: java-web-project/src + dockerfilePath: java-web-project/src/Dockerfile + - name: no-context + path: java-multi-modules + deploymentRepo: skyhook-dev/deployment + deploymentRepoPath: nbjkgj + buildTool: + docker: + dockerfilePath: java-multi-modules/Dockerfile +YAML + +# Inline parse logic mirroring action.yml — kept in sync deliberately +parse() { + local SERVICE_NAME="$1" + local SERVICE_INDEX + SERVICE_INDEX=$(yq e ".services | to_entries | .[] | select(.value.name == \"${SERVICE_NAME}\") | .key" "$CONFIG_FILE" 2>/dev/null || echo "") + [ -z "$SERVICE_INDEX" ] && { echo "MISS"; return; } + + local SERVICE_PATH=".services[$SERVICE_INDEX]" + local BUILD_CONTEXT + BUILD_CONTEXT=$(yq e "${SERVICE_PATH}.buildTool.docker.buildContext // \"\"" "$CONFIG_FILE") + [ "$BUILD_CONTEXT" = "null" ] && BUILD_CONTEXT="" + [ -z "$BUILD_CONTEXT" ] && BUILD_CONTEXT="." + + local DOCKERFILE_PATH + DOCKERFILE_PATH=$(yq e "${SERVICE_PATH}.buildTool.docker.dockerfilePath // \"\"" "$CONFIG_FILE") + [ "$DOCKERFILE_PATH" = "null" ] && DOCKERFILE_PATH="" + + echo "${BUILD_CONTEXT}|${DOCKERFILE_PATH}" +} + +assert_eq() { + local got="$1" want="$2" label="$3" + if [ "$got" != "$want" ]; then + echo "FAIL ($label): got '$got' want '$want'" + exit 1 + fi + echo "PASS: $label" +} + +assert_eq "$(parse with-context)" "java-web-project/src|java-web-project/src/Dockerfile" "buildContext is read" +assert_eq "$(parse no-context)" ".|java-multi-modules/Dockerfile" "buildContext defaults to '.' when absent" + +# Sanity: action.yml references the renamed field and default +grep -q "buildTool.docker.buildContext" "$ACTION_FILE" || { echo "FAIL: action.yml does not read buildContext"; exit 1; } +grep -q 'BUILD_CONTEXT="\."' "$ACTION_FILE" || { echo "FAIL: action.yml does not default build_context to '.'"; exit 1; } +grep -q "build_context:" "$ACTION_FILE" || { echo "FAIL: action.yml does not declare build_context output"; exit 1; } +grep -q "context_path" "$ACTION_FILE" && { echo "FAIL: action.yml still references old context_path"; exit 1; } +echo "PASS: action.yml references buildContext, defaults to '.', exposes build_context output, and drops context_path" From d6fa2b2147d4ec19081fb209a29179e81e2ba309 Mon Sep 17 00:00:00 2001 From: eliran-mic Date: Thu, 30 Apr 2026 21:35:47 +0300 Subject: [PATCH 2/3] fix: address deep-review findings before v1 tag BLOCKERs: - Detect duplicate service names (yq returns multi-line index list); action exits 1 with ::error:: instead of producing garbage outputs. - Correct README owner: skyhook-io -> KoalaOps in usage examples. - Remove stray duplicate read-config/README.md directory. MAJORs: - Cross-platform yq installer: Linux/Darwin x86_64+arm64, curl preferred over wget (wget not on macOS), sudo only when not root, clear error on unsupported platform. - Pin yq to v4.47.1 instead of "latest" (avoids supply-chain surprise). - README "Fallback Behavior" prose -> explicit behavior matrix table that documents the asymmetry (build_context defaults to "." only when service is found). - Use strenv(SERVICE_NAME) in yq query so service names containing literal `"` are handled correctly instead of producing a silent miss. - Assert SERVICE_INDEX is a single integer before interpolating. - Test coverage extended: explicit null buildContext, empty-string buildContext, missing service, service_name with quote, duplicate-name detection, and grep-checks for strenv + dup-detection + pinned yq in action.yml. author field: 'Skyhook' -> 'KoalaOps' to match repo owner. Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 24 +++++++-- action.yml | 46 ++++++++++++++--- read-config/README.md | 115 ------------------------------------------ tests/parse_test.sh | 51 +++++++++++++++++-- 4 files changed, 104 insertions(+), 132 deletions(-) delete mode 100644 read-config/README.md diff --git a/README.md b/README.md index 6a5636e..955d43f 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ This action parses the Skyhook configuration file and extracts service-specific ```yaml - name: Read service config id: config - uses: skyhook-io/read-config@v1 + uses: KoalaOps/read-config@v1 with: working_directory: code service_name: my-service @@ -88,28 +88,42 @@ jobs: - name: Read service config id: config - uses: skyhook-io/read-config@v1 + uses: KoalaOps/read-config@v1 with: working_directory: code service_name: ${{ env.SERVICE_NAME }} - name: Build and push Docker image - uses: skyhook-io/docker-build-push-action@v1 + uses: KoalaOps/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) }} image: ${{ inputs.image }} ``` -## Fallback Behavior +## Behavior matrix -If the config file doesn't exist or the service isn't found, all output values will be empty strings (except `build_context`, which always defaults to `.` when the service is found but `buildContext` is absent). When the service is missing entirely, fall back in the workflow: +| 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 | + +`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: ```yaml context: ${{ steps.config.outputs.build_context || '.' }} dockerfile: ${{ steps.config.outputs.dockerfile_path || 'Dockerfile' }} ``` +## Runner requirements + +- Bash + `yq` v4.x. The action installs yq v4.47.1 if missing. +- Auto-install supports `Linux-x86_64`, `Linux-aarch64`, `Darwin-x86_64`, `Darwin-arm64`. On other platforms (Windows, BSD, etc.), pre-install yq before this step or the action will fail with a clear error. +- The auto-installer uses `curl` (preferred) or `wget`, and `sudo` if not running as root. + ## License MIT diff --git a/action.yml b/action.yml index aa0b2b4..15a15e8 100644 --- a/action.yml +++ b/action.yml @@ -1,6 +1,6 @@ name: 'Read Skyhook Config' description: 'Reads service configuration from .skyhook/skyhook.yaml' -author: 'Skyhook' +author: 'KoalaOps' inputs: working_directory: @@ -52,11 +52,27 @@ runs: - name: Install yq shell: bash run: | - if ! command -v yq &> /dev/null; then - echo "Installing yq..." - sudo wget -qO /usr/local/bin/yq https://github.com/mikefarah/yq/releases/latest/download/yq_linux_amd64 - sudo chmod +x /usr/local/bin/yq + if command -v yq &>/dev/null; then exit 0; fi + case "$(uname -s)-$(uname -m)" in + Linux-x86_64) ASSET=yq_linux_amd64 ;; + Linux-aarch64) ASSET=yq_linux_arm64 ;; + Darwin-x86_64) ASSET=yq_darwin_amd64 ;; + Darwin-arm64) ASSET=yq_darwin_arm64 ;; + *) echo "::error::yq not found and no prebuilt binary for $(uname -s)-$(uname -m). Install yq before using this action."; exit 1 ;; + esac + SUDO="" + [ "$(id -u)" -ne 0 ] && command -v sudo &>/dev/null && SUDO=sudo + DEST=/usr/local/bin/yq + URL="https://github.com/mikefarah/yq/releases/download/v4.47.1/${ASSET}" + echo "Installing yq v4.47.1 from $URL" + if command -v curl &>/dev/null; then + $SUDO curl -fsSL -o "$DEST" "$URL" + elif command -v wget &>/dev/null; then + $SUDO wget -qO "$DEST" "$URL" + else + echo "::error::Neither curl nor wget is available; cannot install yq."; exit 1 fi + $SUDO chmod +x "$DEST" - name: Parse skyhook config id: parse @@ -86,9 +102,10 @@ runs: echo "config_found=true" >> $GITHUB_OUTPUT echo "Found config file: $CONFIG_FILE" - # Find the service by name in the services array - # Use yq to query the YAML - SERVICE_INDEX=$(yq e ".services | to_entries | .[] | select(.value.name == \"${SERVICE_NAME}\") | .key" "$CONFIG_FILE" 2>/dev/null || echo "") + # Find the service by name in the services array. + # Use strenv() to inject SERVICE_NAME via env, avoiding quote-injection + # if the name contains a literal `"`. + SERVICE_INDEX=$(SERVICE_NAME="$SERVICE_NAME" yq e '.services | to_entries | .[] | select(.value.name == strenv(SERVICE_NAME)) | .key' "$CONFIG_FILE" 2>/dev/null || echo "") if [ -z "$SERVICE_INDEX" ]; then echo "Service '$SERVICE_NAME' not found in config" @@ -103,6 +120,19 @@ runs: exit 0 fi + # Detect duplicate service names: yq returns a newline-separated list of + # indices when multiple matches exist. Treat that as a malformed config. + MATCH_COUNT=$(printf '%s\n' "$SERVICE_INDEX" | grep -c .) + if [ "$MATCH_COUNT" -gt 1 ]; then + echo "::error::Multiple services named '$SERVICE_NAME' found in $CONFIG_FILE (indices: $(echo "$SERVICE_INDEX" | tr '\n' ',' | sed 's/,$//'))" + exit 1 + fi + + if ! [[ "$SERVICE_INDEX" =~ ^[0-9]+$ ]]; then + echo "::error::Unexpected service index from yq: '$SERVICE_INDEX'" + exit 1 + fi + echo "service_found=true" >> $GITHUB_OUTPUT echo "Found service '$SERVICE_NAME' at index $SERVICE_INDEX" diff --git a/read-config/README.md b/read-config/README.md deleted file mode 100644 index 6a5636e..0000000 --- a/read-config/README.md +++ /dev/null @@ -1,115 +0,0 @@ -# read-config - -A GitHub Action that reads service configuration from `.skyhook/skyhook.yaml`. - -## Description - -This action parses the Skyhook configuration file and extracts service-specific settings including: -- Service path -- Deployment repository configuration -- Docker build tool settings (build context, dockerfile path) - -## Usage - -```yaml -- name: Read service config - id: config - uses: skyhook-io/read-config@v1 - with: - working_directory: code - service_name: my-service - -- name: Build Docker image - run: | - docker build \ - -f ${{ steps.config.outputs.dockerfile_path || 'Dockerfile' }} \ - ${{ steps.config.outputs.build_context }} -``` - -## Inputs - -| Input | Description | Required | Default | -|-------|-------------|----------|---------| -| `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` | - -## Outputs - -| Output | Description | -|--------|-------------| -| `name` | Service name from config | -| `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 | -| `config_found` | Whether the config file was found (`true`/`false`) | -| `service_found` | Whether the service was found in config (`true`/`false`) | - -## Config File Format - -The action expects a `.skyhook/skyhook.yaml` file with the following structure: - -```yaml -services: - - name: my-service - path: services/my-service - deploymentRepo: org/deployment-repo - deploymentRepoPath: services/my-service - buildTool: - docker: - buildContext: services/my-service - dockerfilePath: docker/Dockerfile.my-service - - - name: another-service - path: services/another - buildTool: - docker: - # buildContext omitted - defaults to "." - dockerfilePath: services/another/Dockerfile - -environments: - - name: dev - clusterName: dev-cluster - namespace: dev -``` - -## Example: Build with Dynamic Config - -```yaml -jobs: - build: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v5 - with: - path: code - - - name: Read service config - id: config - uses: skyhook-io/read-config@v1 - with: - working_directory: code - service_name: ${{ env.SERVICE_NAME }} - - - 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) }} - image: ${{ inputs.image }} -``` - -## Fallback Behavior - -If the config file doesn't exist or the service isn't found, all output values will be empty strings (except `build_context`, which always defaults to `.` when the service is found but `buildContext` is absent). When the service is missing entirely, fall back in the workflow: - -```yaml -context: ${{ steps.config.outputs.build_context || '.' }} -dockerfile: ${{ steps.config.outputs.dockerfile_path || 'Dockerfile' }} -``` - -## License - -MIT diff --git a/tests/parse_test.sh b/tests/parse_test.sh index 9d82d70..81c3fd0 100755 --- a/tests/parse_test.sh +++ b/tests/parse_test.sh @@ -33,15 +33,34 @@ services: buildTool: docker: dockerfilePath: java-multi-modules/Dockerfile + - name: explicit-null + path: svc-null + buildTool: + docker: + buildContext: null + dockerfilePath: svc-null/Dockerfile + - name: empty-string + path: svc-empty + buildTool: + docker: + buildContext: "" + dockerfilePath: svc-empty/Dockerfile YAML -# Inline parse logic mirroring action.yml — kept in sync deliberately +# Inline parse logic mirroring action.yml - kept in sync deliberately parse() { local SERVICE_NAME="$1" local SERVICE_INDEX - SERVICE_INDEX=$(yq e ".services | to_entries | .[] | select(.value.name == \"${SERVICE_NAME}\") | .key" "$CONFIG_FILE" 2>/dev/null || echo "") + SERVICE_INDEX=$(SERVICE_NAME="$SERVICE_NAME" yq e '.services | to_entries | .[] | select(.value.name == strenv(SERVICE_NAME)) | .key' "$CONFIG_FILE" 2>/dev/null || echo "") [ -z "$SERVICE_INDEX" ] && { echo "MISS"; return; } + local MATCH_COUNT + MATCH_COUNT=$(printf '%s\n' "$SERVICE_INDEX" | grep -c .) + if [ "$MATCH_COUNT" -gt 1 ]; then + echo "DUPLICATE" + return + fi + local SERVICE_PATH=".services[$SERVICE_INDEX]" local BUILD_CONTEXT BUILD_CONTEXT=$(yq e "${SERVICE_PATH}.buildTool.docker.buildContext // \"\"" "$CONFIG_FILE") @@ -66,10 +85,34 @@ assert_eq() { assert_eq "$(parse with-context)" "java-web-project/src|java-web-project/src/Dockerfile" "buildContext is read" assert_eq "$(parse no-context)" ".|java-multi-modules/Dockerfile" "buildContext defaults to '.' when absent" +assert_eq "$(parse explicit-null)" ".|svc-null/Dockerfile" "buildContext explicit null defaults to '.'" +assert_eq "$(parse empty-string)" ".|svc-empty/Dockerfile" "buildContext empty string defaults to '.'" +assert_eq "$(parse nonexistent-service)" "MISS" "missing service yields MISS" +assert_eq "$(parse 'name-with-\"-quote')" "MISS" "service_name containing a literal quote is handled (no crash)" + +# Duplicate-name detection +DUP_FILE="$WORK/.skyhook/dup.yaml" +cat >"$DUP_FILE" <<'YAML' +services: + - name: dup + path: a + - name: dup + path: b +YAML +DUP_INDEX=$(SERVICE_NAME="dup" yq e '.services | to_entries | .[] | select(.value.name == strenv(SERVICE_NAME)) | .key' "$DUP_FILE") +DUP_COUNT=$(printf '%s\n' "$DUP_INDEX" | grep -c .) +if [ "$DUP_COUNT" -ne 2 ]; then + echo "FAIL: expected 2 matches for duplicate name, got $DUP_COUNT" + exit 1 +fi +echo "PASS: duplicate service names produce >1 match (action.yml errors on this)" -# Sanity: action.yml references the renamed field and default +# Sanity: action.yml references the renamed field, default, strenv, dup detection, and drops context_path grep -q "buildTool.docker.buildContext" "$ACTION_FILE" || { echo "FAIL: action.yml does not read buildContext"; exit 1; } grep -q 'BUILD_CONTEXT="\."' "$ACTION_FILE" || { echo "FAIL: action.yml does not default build_context to '.'"; exit 1; } grep -q "build_context:" "$ACTION_FILE" || { echo "FAIL: action.yml does not declare build_context output"; exit 1; } +grep -q "strenv(SERVICE_NAME)" "$ACTION_FILE" || { echo "FAIL: action.yml does not use strenv() for service_name"; exit 1; } +grep -q "Multiple services named" "$ACTION_FILE" || { echo "FAIL: action.yml does not detect duplicate service names"; exit 1; } +grep -q "v4.47.1" "$ACTION_FILE" || { echo "FAIL: action.yml does not pin yq to a specific version"; exit 1; } grep -q "context_path" "$ACTION_FILE" && { echo "FAIL: action.yml still references old context_path"; exit 1; } -echo "PASS: action.yml references buildContext, defaults to '.', exposes build_context output, and drops context_path" +echo "PASS: action.yml has expected schema + defaults + strenv + dup-detection + pinned yq + no context_path" From 328175504d486d0b7fc1d432b031e51e71ed736a Mon Sep 17 00:00:00 2001 From: eliran-mic Date: Thu, 30 Apr 2026 21:57:22 +0300 Subject: [PATCH 3/3] fix: address cycle-3 review findings before v1 tag BLOCKER: - Replace 'echo key=val >> $GITHUB_OUTPUT' with heredoc-form writes (write_output helper). Multiline YAML values, leading whitespace, literal '=' all preserved. Tests cover all three. MAJORs: - Validate yq v4.x when preinstalled (v3 has different syntax and would silently produce wrong output). - Validate non-empty service_name (required:true only checks input presence, not value). - Surface yq parse failures as ::error::Failed to parse... instead of swallowing stderr and reporting "service not found". - Extract parse logic to scripts/parse.sh; tests now invoke it directly with a temp $GITHUB_OUTPUT, so future regressions in action.yml's parse step are caught. - Add .github/workflows/test.yml: shell tests on ubuntu+macos and end-to-end smoke test that exercises the action against tests/fixtures/.skyhook/skyhook.yaml. MINORs: - Strip trailing slash from working_directory; collapse empty value to ".". - Retry yq download up to 3 times with backoff (curl --retry 3 + outer loop) for transient network blips. - Marketplace-friendly action description. Test count: 33 passing locally (was 8). Files added: scripts/parse.sh, tests/fixtures/.skyhook/skyhook.yaml, .github/workflows/test.yml. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/test.yml | 72 ++++++++ README.md | 4 +- action.yml | 150 +++++----------- scripts/parse.sh | 133 ++++++++++++++ tests/fixtures/.skyhook/skyhook.yaml | 14 ++ tests/parse_test.sh | 251 ++++++++++++++++++++++----- 6 files changed, 468 insertions(+), 156 deletions(-) create mode 100644 .github/workflows/test.yml create mode 100755 scripts/parse.sh create mode 100644 tests/fixtures/.skyhook/skyhook.yaml diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..158f355 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,72 @@ +name: test + +on: + push: + branches: [main] + pull_request: + +jobs: + shell-tests: + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, macos-latest] + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v4 + + - name: Install yq v4 (linux) + if: runner.os == 'Linux' + run: | + if command -v yq && yq --version | grep -qE 'version v?4\.'; then exit 0; fi + sudo curl -fsSL --retry 3 --retry-delay 2 \ + -o /usr/local/bin/yq \ + https://github.com/mikefarah/yq/releases/download/v4.47.1/yq_linux_amd64 + sudo chmod +x /usr/local/bin/yq + yq --version + + - name: Install yq v4 (macos) + if: runner.os == 'macOS' + run: brew install yq && yq --version + + - run: bash tests/metadata_test.sh + - run: bash tests/parse_test.sh + + smoke: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Run action against fixture (buildContext present) + id: cfg + uses: ./ + with: + working_directory: tests/fixtures + service_name: smoke-svc + + - name: Verify outputs + run: | + set -e + test "${{ steps.cfg.outputs.config_found }}" = "true" + test "${{ steps.cfg.outputs.service_found }}" = "true" + test "${{ steps.cfg.outputs.name }}" = "smoke-svc" + test "${{ steps.cfg.outputs.path }}" = "services/smoke" + test "${{ steps.cfg.outputs.deployment_repo }}" = "KoalaOps/deployment" + test "${{ steps.cfg.outputs.deployment_repo_path }}" = "smoke" + test "${{ steps.cfg.outputs.build_context }}" = "services/smoke" + test "${{ steps.cfg.outputs.dockerfile_path }}" = "services/smoke/Dockerfile" + echo "smoke-svc outputs OK" + + - name: Run action against fixture (buildContext absent -> default '.') + id: cfg2 + uses: ./ + with: + working_directory: tests/fixtures + service_name: smoke-default-context + + - name: Verify default-context outputs + run: | + set -e + test "${{ steps.cfg2.outputs.build_context }}" = "." + test "${{ steps.cfg2.outputs.dockerfile_path }}" = "services/default/Dockerfile" + echo "default-context outputs OK" diff --git a/README.md b/README.md index 955d43f..f6cd0f2 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,8 @@ # read-config -A GitHub Action that reads service configuration from `.skyhook/skyhook.yaml`. +[![test](https://github.com/KoalaOps/read-config/actions/workflows/test.yml/badge.svg)](https://github.com/KoalaOps/read-config/actions/workflows/test.yml) + +A GitHub Action that reads service configuration from `.skyhook/skyhook.yaml` and exposes the matching service's fields as step outputs. ## Description diff --git a/action.yml b/action.yml index 15a15e8..addf911 100644 --- a/action.yml +++ b/action.yml @@ -1,5 +1,5 @@ name: 'Read Skyhook Config' -description: 'Reads service configuration from .skyhook/skyhook.yaml' +description: 'Read service build/deploy config from a Skyhook YAML file (path, dockerfile, build context, deployment repo) by service name' author: 'KoalaOps' inputs: @@ -8,7 +8,7 @@ inputs: required: false default: '.' service_name: - description: 'Name of the service to look up in the config' + description: 'Name of the service to look up in the config (must be non-empty)' required: true config_path: description: 'Path to the skyhook config file relative to working_directory' @@ -49,30 +49,59 @@ outputs: runs: using: 'composite' steps: - - name: Install yq + - name: Install yq (v4) shell: bash run: | - if command -v yq &>/dev/null; then exit 0; fi + set -euo pipefail + # Accept preinstalled yq only if it is v4.x + if command -v yq &>/dev/null; then + if yq --version 2>&1 | grep -qE 'version v?4\.'; then + echo "Found preinstalled yq: $(yq --version)" + exit 0 + fi + echo "::error::Found yq but not v4.x: $(yq --version 2>&1). Uninstall it or upgrade to v4." + exit 1 + fi + case "$(uname -s)-$(uname -m)" in Linux-x86_64) ASSET=yq_linux_amd64 ;; Linux-aarch64) ASSET=yq_linux_arm64 ;; Darwin-x86_64) ASSET=yq_darwin_amd64 ;; Darwin-arm64) ASSET=yq_darwin_arm64 ;; - *) echo "::error::yq not found and no prebuilt binary for $(uname -s)-$(uname -m). Install yq before using this action."; exit 1 ;; + *) echo "::error::yq not found and no prebuilt binary for $(uname -s)-$(uname -m). Install yq v4 before using this action."; exit 1 ;; esac + SUDO="" [ "$(id -u)" -ne 0 ] && command -v sudo &>/dev/null && SUDO=sudo DEST=/usr/local/bin/yq URL="https://github.com/mikefarah/yq/releases/download/v4.47.1/${ASSET}" echo "Installing yq v4.47.1 from $URL" - if command -v curl &>/dev/null; then - $SUDO curl -fsSL -o "$DEST" "$URL" - elif command -v wget &>/dev/null; then - $SUDO wget -qO "$DEST" "$URL" - else - echo "::error::Neither curl nor wget is available; cannot install yq."; exit 1 - fi + + download() { + if command -v curl &>/dev/null; then + $SUDO curl -fsSL --retry 3 --retry-delay 2 -o "$DEST" "$URL" + elif command -v wget &>/dev/null; then + $SUDO wget -qO "$DEST" "$URL" + else + echo "::error::Neither curl nor wget is available; cannot install yq." + return 2 + fi + } + + for attempt in 1 2 3; do + if download; then + break + fi + rc=$? + if [ $rc -eq 2 ] || [ $attempt -eq 3 ]; then + echo "::error::yq download failed (attempt $attempt/3, exit $rc)" + exit 1 + fi + sleep $((attempt * 2)) + done + $SUDO chmod +x "$DEST" + yq --version - name: Parse skyhook config id: parse @@ -81,103 +110,8 @@ runs: WORKING_DIR: ${{ inputs.working_directory }} SERVICE_NAME: ${{ inputs.service_name }} CONFIG_PATH: ${{ inputs.config_path }} - run: | - CONFIG_FILE="${WORKING_DIR}/${CONFIG_PATH}" - - # Check if config file exists - if [ ! -f "$CONFIG_FILE" ]; then - echo "Config file not found: $CONFIG_FILE" - echo "config_found=false" >> $GITHUB_OUTPUT - echo "service_found=false" >> $GITHUB_OUTPUT - # Set empty outputs for all fields - echo "name=" >> $GITHUB_OUTPUT - echo "path=" >> $GITHUB_OUTPUT - echo "deployment_repo=" >> $GITHUB_OUTPUT - echo "deployment_repo_path=" >> $GITHUB_OUTPUT - echo "build_context=" >> $GITHUB_OUTPUT - echo "dockerfile_path=" >> $GITHUB_OUTPUT - exit 0 - fi - - echo "config_found=true" >> $GITHUB_OUTPUT - echo "Found config file: $CONFIG_FILE" - - # Find the service by name in the services array. - # Use strenv() to inject SERVICE_NAME via env, avoiding quote-injection - # if the name contains a literal `"`. - SERVICE_INDEX=$(SERVICE_NAME="$SERVICE_NAME" yq e '.services | to_entries | .[] | select(.value.name == strenv(SERVICE_NAME)) | .key' "$CONFIG_FILE" 2>/dev/null || echo "") - - if [ -z "$SERVICE_INDEX" ]; then - echo "Service '$SERVICE_NAME' not found in config" - echo "service_found=false" >> $GITHUB_OUTPUT - # Set empty outputs - echo "name=" >> $GITHUB_OUTPUT - echo "path=" >> $GITHUB_OUTPUT - echo "deployment_repo=" >> $GITHUB_OUTPUT - echo "deployment_repo_path=" >> $GITHUB_OUTPUT - echo "build_context=" >> $GITHUB_OUTPUT - echo "dockerfile_path=" >> $GITHUB_OUTPUT - exit 0 - fi - - # Detect duplicate service names: yq returns a newline-separated list of - # indices when multiple matches exist. Treat that as a malformed config. - MATCH_COUNT=$(printf '%s\n' "$SERVICE_INDEX" | grep -c .) - if [ "$MATCH_COUNT" -gt 1 ]; then - echo "::error::Multiple services named '$SERVICE_NAME' found in $CONFIG_FILE (indices: $(echo "$SERVICE_INDEX" | tr '\n' ',' | sed 's/,$//'))" - exit 1 - fi - - if ! [[ "$SERVICE_INDEX" =~ ^[0-9]+$ ]]; then - echo "::error::Unexpected service index from yq: '$SERVICE_INDEX'" - exit 1 - fi - - echo "service_found=true" >> $GITHUB_OUTPUT - echo "Found service '$SERVICE_NAME' at index $SERVICE_INDEX" - - # Extract service configuration - SERVICE_PATH=".services[$SERVICE_INDEX]" - - # Get basic service fields - NAME=$(yq e "${SERVICE_PATH}.name // \"\"" "$CONFIG_FILE") - PATH_VALUE=$(yq e "${SERVICE_PATH}.path // \"\"" "$CONFIG_FILE") - DEPLOYMENT_REPO=$(yq e "${SERVICE_PATH}.deploymentRepo // \"\"" "$CONFIG_FILE") - DEPLOYMENT_REPO_PATH=$(yq e "${SERVICE_PATH}.deploymentRepoPath // \"\"" "$CONFIG_FILE") - - # Get buildTool.docker configuration - BUILD_CONTEXT=$(yq e "${SERVICE_PATH}.buildTool.docker.buildContext // \"\"" "$CONFIG_FILE") - DOCKERFILE_PATH=$(yq e "${SERVICE_PATH}.buildTool.docker.dockerfilePath // \"\"" "$CONFIG_FILE") - - # Handle "null" strings from yq - [ "$NAME" = "null" ] && NAME="" - [ "$PATH_VALUE" = "null" ] && PATH_VALUE="" - [ "$DEPLOYMENT_REPO" = "null" ] && DEPLOYMENT_REPO="" - [ "$DEPLOYMENT_REPO_PATH" = "null" ] && DEPLOYMENT_REPO_PATH="" - [ "$BUILD_CONTEXT" = "null" ] && BUILD_CONTEXT="" - [ "$DOCKERFILE_PATH" = "null" ] && DOCKERFILE_PATH="" - - # Default build context to "." when absent - [ -z "$BUILD_CONTEXT" ] && BUILD_CONTEXT="." - - # Output values - echo "name=${NAME}" >> $GITHUB_OUTPUT - echo "path=${PATH_VALUE}" >> $GITHUB_OUTPUT - echo "deployment_repo=${DEPLOYMENT_REPO}" >> $GITHUB_OUTPUT - echo "deployment_repo_path=${DEPLOYMENT_REPO_PATH}" >> $GITHUB_OUTPUT - echo "build_context=${BUILD_CONTEXT}" >> $GITHUB_OUTPUT - echo "dockerfile_path=${DOCKERFILE_PATH}" >> $GITHUB_OUTPUT - - # Log outputs for debugging - echo "Parsed service configuration:" - echo " name: ${NAME}" - echo " path: ${PATH_VALUE}" - echo " deployment_repo: ${DEPLOYMENT_REPO}" - echo " deployment_repo_path: ${DEPLOYMENT_REPO_PATH}" - echo " build_context: ${BUILD_CONTEXT}" - echo " dockerfile_path: ${DOCKERFILE_PATH}" + run: bash "$GITHUB_ACTION_PATH/scripts/parse.sh" branding: icon: 'file-text' color: 'blue' - diff --git a/scripts/parse.sh b/scripts/parse.sh new file mode 100755 index 0000000..812eb4c --- /dev/null +++ b/scripts/parse.sh @@ -0,0 +1,133 @@ +#!/usr/bin/env bash +# 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) +# +# Exits non-zero with ::error:: on: +# - empty SERVICE_NAME +# - yq parse failure +# - duplicate service names +# - unexpected SERVICE_INDEX shape +set -euo pipefail + +: "${GITHUB_OUTPUT:?GITHUB_OUTPUT must be set}" + +WORKING_DIR="${WORKING_DIR:-.}" +[ -z "$WORKING_DIR" ] && WORKING_DIR="." +WORKING_DIR="${WORKING_DIR%/}" +CONFIG_PATH="${CONFIG_PATH:-.skyhook/skyhook.yaml}" +SERVICE_NAME="${SERVICE_NAME:-}" + +if [ -z "$SERVICE_NAME" ]; then + echo "::error::service_name input is required and must be non-empty" + exit 1 +fi + +CONFIG_FILE="${WORKING_DIR}/${CONFIG_PATH}" + +# Heredoc-form output write: safe for multiline values, leading/trailing whitespace, +# and values containing literal "=". Generates a unique delimiter per call. +write_output() { + local key="$1" val="$2" + local delim + delim="ghadelim_$(LC_ALL=C tr -dc 'a-f0-9' /dev/null | head -c 16 || echo "$$_$RANDOM$RANDOM")" + { + printf '%s<<%s\n' "$key" "$delim" + printf '%s\n' "$val" + printf '%s\n' "$delim" + } >> "$GITHUB_OUTPUT" +} + +write_empty_outputs() { + write_output name "" + write_output path "" + write_output deployment_repo "" + write_output deployment_repo_path "" + write_output build_context "" + write_output dockerfile_path "" +} + +# Config file missing +if [ ! -f "$CONFIG_FILE" ]; then + echo "Config file not found: $CONFIG_FILE" + write_output config_found false + write_output service_found false + write_empty_outputs + exit 0 +fi + +write_output config_found true +echo "Found config file: $CONFIG_FILE" + +# Look up the service. Use strenv() to inject SERVICE_NAME safely (handles +# names containing literal `"`). Do NOT suppress yq stderr - a malformed YAML +# should surface as a clear parse error, not a silent "service not found". +if ! SERVICE_INDEX=$(SERVICE_NAME="$SERVICE_NAME" yq e \ + '.services | to_entries | .[] | select(.value.name == strenv(SERVICE_NAME)) | .key' \ + "$CONFIG_FILE"); then + echo "::error::Failed to parse $CONFIG_FILE - check YAML syntax" + exit 1 +fi + +if [ -z "$SERVICE_INDEX" ]; then + echo "Service '$SERVICE_NAME' not found in config" + write_output service_found false + write_empty_outputs + exit 0 +fi + +# Duplicate service names: yq returns newline-separated indices. +MATCH_COUNT=$(printf '%s\n' "$SERVICE_INDEX" | grep -c .) +if [ "$MATCH_COUNT" -gt 1 ]; then + INDICES=$(echo "$SERVICE_INDEX" | tr '\n' ',' | sed 's/,$//') + echo "::error::Multiple services named '$SERVICE_NAME' found in $CONFIG_FILE (indices: $INDICES)" + exit 1 +fi + +if ! [[ "$SERVICE_INDEX" =~ ^[0-9]+$ ]]; then + echo "::error::Unexpected service index from yq: '$SERVICE_INDEX'" + exit 1 +fi + +write_output service_found true +echo "Found service '$SERVICE_NAME' at index $SERVICE_INDEX" + +SERVICE_PATH=".services[$SERVICE_INDEX]" + +NAME=$(yq e "${SERVICE_PATH}.name // \"\"" "$CONFIG_FILE") +PATH_VALUE=$(yq e "${SERVICE_PATH}.path // \"\"" "$CONFIG_FILE") +DEPLOYMENT_REPO=$(yq e "${SERVICE_PATH}.deploymentRepo // \"\"" "$CONFIG_FILE") +DEPLOYMENT_REPO_PATH=$(yq e "${SERVICE_PATH}.deploymentRepoPath // \"\"" "$CONFIG_FILE") +BUILD_CONTEXT=$(yq e "${SERVICE_PATH}.buildTool.docker.buildContext // \"\"" "$CONFIG_FILE") +DOCKERFILE_PATH=$(yq e "${SERVICE_PATH}.buildTool.docker.dockerfilePath // \"\"" "$CONFIG_FILE") + +# yq v4 with `// ""` returns "" for missing/null; the "null" string post-checks +# below are defensive guards in case yq behaviour shifts. +[ "$NAME" = "null" ] && NAME="" +[ "$PATH_VALUE" = "null" ] && PATH_VALUE="" +[ "$DEPLOYMENT_REPO" = "null" ] && DEPLOYMENT_REPO="" +[ "$DEPLOYMENT_REPO_PATH" = "null" ] && DEPLOYMENT_REPO_PATH="" +[ "$BUILD_CONTEXT" = "null" ] && BUILD_CONTEXT="" +[ "$DOCKERFILE_PATH" = "null" ] && DOCKERFILE_PATH="" + +# Default build context to "." when absent +[ -z "$BUILD_CONTEXT" ] && BUILD_CONTEXT="." + +write_output name "$NAME" +write_output path "$PATH_VALUE" +write_output deployment_repo "$DEPLOYMENT_REPO" +write_output deployment_repo_path "$DEPLOYMENT_REPO_PATH" +write_output build_context "$BUILD_CONTEXT" +write_output dockerfile_path "$DOCKERFILE_PATH" + +echo "Parsed service configuration:" +echo " name: ${NAME}" +echo " path: ${PATH_VALUE}" +echo " deployment_repo: ${DEPLOYMENT_REPO}" +echo " deployment_repo_path: ${DEPLOYMENT_REPO_PATH}" +echo " build_context: ${BUILD_CONTEXT}" +echo " dockerfile_path: ${DOCKERFILE_PATH}" diff --git a/tests/fixtures/.skyhook/skyhook.yaml b/tests/fixtures/.skyhook/skyhook.yaml new file mode 100644 index 0000000..2743b8a --- /dev/null +++ b/tests/fixtures/.skyhook/skyhook.yaml @@ -0,0 +1,14 @@ +services: + - name: smoke-svc + path: services/smoke + deploymentRepo: KoalaOps/deployment + deploymentRepoPath: smoke + buildTool: + docker: + buildContext: services/smoke + dockerfilePath: services/smoke/Dockerfile + - name: smoke-default-context + path: services/default + buildTool: + docker: + dockerfilePath: services/default/Dockerfile diff --git a/tests/parse_test.sh b/tests/parse_test.sh index 81c3fd0..6eb209d 100755 --- a/tests/parse_test.sh +++ b/tests/parse_test.sh @@ -1,7 +1,10 @@ #!/usr/bin/env bash +# Exercises scripts/parse.sh directly with a temp $GITHUB_OUTPUT, so the test +# fails if the real action's parsing logic regresses. set -euo pipefail REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +PARSE_SCRIPT="${REPO_ROOT}/scripts/parse.sh" ACTION_FILE="${REPO_ROOT}/action.yml" if ! command -v yq &>/dev/null; then @@ -9,6 +12,17 @@ if ! command -v yq &>/dev/null; then exit 0 fi +# Require yq v4 (parse.sh uses v4 syntax) +if ! yq --version 2>&1 | grep -qE 'version v?4\.'; then + echo "SKIP: yq is not v4.x: $(yq --version 2>&1)" + exit 0 +fi + +if [ ! -x "$PARSE_SCRIPT" ]; then + echo "FAIL: $PARSE_SCRIPT not found or not executable" + exit 1 +fi + WORK=$(mktemp -d) trap 'rm -rf "$WORK"' EXIT @@ -45,33 +59,68 @@ services: docker: buildContext: "" dockerfilePath: svc-empty/Dockerfile + - name: with-spaces + path: "services/with spaces/sub" + buildTool: + docker: + buildContext: "services/with spaces/sub" + dockerfilePath: "services/with spaces/sub/Dockerfile" + - name: 'name-with-"-quote' + path: svc-quote + buildTool: + docker: + dockerfilePath: svc-quote/Dockerfile YAML -# Inline parse logic mirroring action.yml - kept in sync deliberately -parse() { - local SERVICE_NAME="$1" - local SERVICE_INDEX - SERVICE_INDEX=$(SERVICE_NAME="$SERVICE_NAME" yq e '.services | to_entries | .[] | select(.value.name == strenv(SERVICE_NAME)) | .key' "$CONFIG_FILE" 2>/dev/null || echo "") - [ -z "$SERVICE_INDEX" ] && { echo "MISS"; return; } - - local MATCH_COUNT - MATCH_COUNT=$(printf '%s\n' "$SERVICE_INDEX" | grep -c .) - if [ "$MATCH_COUNT" -gt 1 ]; then - echo "DUPLICATE" - return - fi - - local SERVICE_PATH=".services[$SERVICE_INDEX]" - local BUILD_CONTEXT - BUILD_CONTEXT=$(yq e "${SERVICE_PATH}.buildTool.docker.buildContext // \"\"" "$CONFIG_FILE") - [ "$BUILD_CONTEXT" = "null" ] && BUILD_CONTEXT="" - [ -z "$BUILD_CONTEXT" ] && BUILD_CONTEXT="." +# Read a single output value from a $GITHUB_OUTPUT file written in heredoc form. +# The heredoc form is: KEY< 0 && substr($0, 1, idx - 1) == key) { + delim = substr($0, idx + 2) + capturing = 1 + out = "" + } + } + END { print out } + ' "$file" +} - local DOCKERFILE_PATH - DOCKERFILE_PATH=$(yq e "${SERVICE_PATH}.buildTool.docker.dockerfilePath // \"\"" "$CONFIG_FILE") - [ "$DOCKERFILE_PATH" = "null" ] && DOCKERFILE_PATH="" +run_parse() { + local working_dir="$1" svc="$2" cfg_path="${3:-.skyhook/skyhook.yaml}" + local out_file + out_file="$WORK/gh_output.$RANDOM" + : >"$out_file" + WORKING_DIR="$working_dir" \ + SERVICE_NAME="$svc" \ + CONFIG_PATH="$cfg_path" \ + GITHUB_OUTPUT="$out_file" \ + bash "$PARSE_SCRIPT" >/dev/null + echo "$out_file" +} - echo "${BUILD_CONTEXT}|${DOCKERFILE_PATH}" +run_parse_expect_fail() { + local working_dir="$1" svc="$2" cfg_path="${3:-.skyhook/skyhook.yaml}" + local out_file err_file + out_file="$WORK/gh_output.$RANDOM" + err_file="$WORK/gh_err.$RANDOM" + : >"$out_file" + if WORKING_DIR="$working_dir" \ + SERVICE_NAME="$svc" \ + CONFIG_PATH="$cfg_path" \ + GITHUB_OUTPUT="$out_file" \ + bash "$PARSE_SCRIPT" >"$err_file" 2>&1; then + echo "EXPECTED-FAIL-DID-NOT-FAIL" + cat "$err_file" + return + fi + cat "$err_file" } assert_eq() { @@ -83,36 +132,144 @@ assert_eq() { echo "PASS: $label" } -assert_eq "$(parse with-context)" "java-web-project/src|java-web-project/src/Dockerfile" "buildContext is read" -assert_eq "$(parse no-context)" ".|java-multi-modules/Dockerfile" "buildContext defaults to '.' when absent" -assert_eq "$(parse explicit-null)" ".|svc-null/Dockerfile" "buildContext explicit null defaults to '.'" -assert_eq "$(parse empty-string)" ".|svc-empty/Dockerfile" "buildContext empty string defaults to '.'" -assert_eq "$(parse nonexistent-service)" "MISS" "missing service yields MISS" -assert_eq "$(parse 'name-with-\"-quote')" "MISS" "service_name containing a literal quote is handled (no crash)" +assert_contains() { + local haystack="$1" needle="$2" label="$3" + if ! echo "$haystack" | grep -qF "$needle"; then + echo "FAIL ($label): output does not contain '$needle'" + echo "--- output ---" + echo "$haystack" + echo "---" + exit 1 + fi + echo "PASS: $label" +} + +# --- happy path: buildContext present --- +out=$(run_parse "$WORK" with-context) +assert_eq "$(read_output "$out" config_found)" "true" "with-context: config_found=true" +assert_eq "$(read_output "$out" service_found)" "true" "with-context: service_found=true" +assert_eq "$(read_output "$out" name)" "with-context" "with-context: name" +assert_eq "$(read_output "$out" path)" "java-web-project" "with-context: path" +assert_eq "$(read_output "$out" deployment_repo)" "KoalaOps/deployment" "with-context: deployment_repo" +assert_eq "$(read_output "$out" deployment_repo_path)" "nbjkgj" "with-context: deployment_repo_path" +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 "." --- +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" deployment_repo)" "skyhook-dev/deployment" "no-context: deployment_repo" + +# --- buildContext: null => default "." --- +out=$(run_parse "$WORK" explicit-null) +assert_eq "$(read_output "$out" build_context)" "." "explicit-null: build_context defaults to '.'" -# Duplicate-name detection -DUP_FILE="$WORK/.skyhook/dup.yaml" -cat >"$DUP_FILE" <<'YAML' +# --- buildContext: "" => default "." --- +out=$(run_parse "$WORK" empty-string) +assert_eq "$(read_output "$out" build_context)" "." "empty-string: build_context defaults to '.'" + +# --- value with spaces survives heredoc round-trip --- +out=$(run_parse "$WORK" with-spaces) +assert_eq "$(read_output "$out" path)" "services/with spaces/sub" "with-spaces: path preserves spaces" +assert_eq "$(read_output "$out" build_context)" "services/with spaces/sub" "with-spaces: build_context preserves spaces" + +# --- service_name with literal quote: should still resolve via strenv() --- +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 --- +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)" + +# --- config file missing --- +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" + +# --- empty service_name --- +err=$(run_parse_expect_fail "$WORK" "") +assert_contains "$err" "service_name input is required and must be non-empty" "empty service_name errors" + +# --- duplicate service names --- +DUP_DIR="$WORK/dup" +mkdir -p "$DUP_DIR/.skyhook" +cat >"$DUP_DIR/.skyhook/skyhook.yaml" <<'YAML' services: - name: dup path: a - name: dup path: b YAML -DUP_INDEX=$(SERVICE_NAME="dup" yq e '.services | to_entries | .[] | select(.value.name == strenv(SERVICE_NAME)) | .key' "$DUP_FILE") -DUP_COUNT=$(printf '%s\n' "$DUP_INDEX" | grep -c .) -if [ "$DUP_COUNT" -ne 2 ]; then - echo "FAIL: expected 2 matches for duplicate name, got $DUP_COUNT" - exit 1 -fi -echo "PASS: duplicate service names produce >1 match (action.yml errors on this)" +err=$(run_parse_expect_fail "$DUP_DIR" "dup") +assert_contains "$err" "Multiple services named 'dup'" "duplicate service names error" -# Sanity: action.yml references the renamed field, default, strenv, dup detection, and drops context_path -grep -q "buildTool.docker.buildContext" "$ACTION_FILE" || { echo "FAIL: action.yml does not read buildContext"; exit 1; } -grep -q 'BUILD_CONTEXT="\."' "$ACTION_FILE" || { echo "FAIL: action.yml does not default build_context to '.'"; exit 1; } -grep -q "build_context:" "$ACTION_FILE" || { echo "FAIL: action.yml does not declare build_context output"; exit 1; } -grep -q "strenv(SERVICE_NAME)" "$ACTION_FILE" || { echo "FAIL: action.yml does not use strenv() for service_name"; exit 1; } -grep -q "Multiple services named" "$ACTION_FILE" || { echo "FAIL: action.yml does not detect duplicate service names"; exit 1; } -grep -q "v4.47.1" "$ACTION_FILE" || { echo "FAIL: action.yml does not pin yq to a specific version"; exit 1; } +# --- malformed YAML --- +BAD_DIR="$WORK/bad" +mkdir -p "$BAD_DIR/.skyhook" +cat >"$BAD_DIR/.skyhook/skyhook.yaml" <<'YAML' +services: + - name: ok + path: [unterminated +YAML +err=$(run_parse_expect_fail "$BAD_DIR" "ok") +assert_contains "$err" "Failed to parse" "malformed YAML errors loudly" + +# --- empty WORKING_DIR collapses to "." (uses cwd, not "/") --- +TMP_CWD="$WORK/cwd-test" +mkdir -p "$TMP_CWD/.skyhook" +cat >"$TMP_CWD/.skyhook/skyhook.yaml" <<'YAML' +services: + - name: cwd-svc + path: x +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 ) +assert_eq "$(read_output "$out_file" service_found)" "true" "empty WORKING_DIR collapses to cwd" + +# --- trailing slash on WORKING_DIR is stripped --- +out=$(run_parse "$WORK/" with-context) +assert_eq "$(read_output "$out" service_found)" "true" "trailing slash on WORKING_DIR works" + +# --- multiline / "=" / leading space round-trip via heredoc --- +HEREDOC_DIR="$WORK/heredoc" +mkdir -p "$HEREDOC_DIR/.skyhook" +cat >"$HEREDOC_DIR/.skyhook/skyhook.yaml" <<'YAML' +services: + - name: weird + path: " leading-space" + deploymentRepo: "key=value-equals" + deploymentRepoPath: "line1\nline2" + buildTool: + docker: + dockerfilePath: "Dockerfile" +YAML +out=$(run_parse "$HEREDOC_DIR" weird) +assert_eq "$(read_output "$out" path)" " leading-space" "leading whitespace preserved through heredoc" +assert_eq "$(read_output "$out" deployment_repo)" "key=value-equals" "literal '=' preserved through heredoc" +# yq parses double-quoted YAML strings with escape sequences, so "line1\nline2" +# becomes a real two-line value. The heredoc round-trip must preserve the +# embedded newline - this is exactly the multiline output-injection case. +expected_multiline=$'line1\nline2' +assert_eq "$(read_output "$out" deployment_repo_path)" "$expected_multiline" "multiline value round-trips through heredoc" + +# --- action.yml structure sanity --- +grep -q 'bash "\$GITHUB_ACTION_PATH/scripts/parse.sh"' "$ACTION_FILE" || { echo "FAIL: action.yml does not call scripts/parse.sh"; exit 1; } +grep -q 'version v?4\\.' "$ACTION_FILE" || { echo "FAIL: action.yml does not validate yq v4"; exit 1; } +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; } -echo "PASS: action.yml has expected schema + defaults + strenv + dup-detection + pinned yq + no context_path" +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" + +# --- 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"