Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 16 additions & 29 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ jobs:
with:
working_directory: tests/fixtures
service_name: smoke-svc
default_build_context: '.'
default_dockerfile_path: Dockerfile

- name: Verify outputs
run: |
Expand All @@ -66,12 +68,14 @@ jobs:
test "${{ steps.cfg.outputs.dockerfile_path }}" = "services/smoke/Dockerfile"
echo "smoke-svc outputs OK"

- name: Run action against fixture (buildContext absent, no caller default)
- name: Run action against fixture (buildContext absent -> default_build_context applied)
id: cfg2
uses: ./
with:
working_directory: tests/fixtures
service_name: smoke-default-context
default_build_context: '.'
default_dockerfile_path: Dockerfile

- name: Verify outputs when YAML lacks buildContext and no caller default
run: |
Expand All @@ -80,40 +84,23 @@ jobs:
# default_build_context the caller passed (here: nothing → empty).
test "${{ steps.cfg2.outputs.build_context }}" = ""
test "${{ steps.cfg2.outputs.dockerfile_path }}" = "services/default/Dockerfile"
echo "default-context outputs OK (empty fallback)"
echo "default-context outputs OK"

- name: Run action with caller-supplied default_* fallbacks

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI test asserts empty but default will be applied

High Severity

The cfg2 step passes default_build_context: '.' and the fixture smoke-default-context has no buildContext in YAML, so parse.sh will apply the default and emit build_context = ".". But the assertion still expects build_context to be empty (""). The step name was updated to reflect the new behavior ("default_build_context applied"), but the assertion and comments were left unchanged from the old behavior. This will cause the smoke CI job to fail on every PR.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4bd920f. Configure here.

- name: Run action against fixture (service missing, defaults flow through)
id: cfg3
uses: ./
with:
working_directory: tests/fixtures
service_name: smoke-default-context
default_build_context: 'services/default'
default_dockerfile_path: 'services/default/Custom.Dockerfile'

- name: Verify caller defaults fill in YAML-absent fields
run: |
set -e
# buildContext is absent in YAML for smoke-default-context → fall back to caller-supplied default
test "${{ steps.cfg3.outputs.build_context }}" = "services/default"
# dockerfilePath IS in YAML → YAML wins, caller default is ignored
test "${{ steps.cfg3.outputs.dockerfile_path }}" = "services/default/Dockerfile"
echo "caller-default fallback OK"

- name: Run action against missing service with caller defaults
id: cfg4
uses: ./
with:
working_directory: tests/fixtures
service_name: not-a-real-service
service_name: does-not-exist
default_build_context: '.'
default_dockerfile_path: 'Dockerfile'
default_dockerfile_path: Dockerfile

- name: Verify caller defaults applied on missing service
- name: Verify missing-service fallback outputs
run: |
set -e
test "${{ steps.cfg4.outputs.config_found }}" = "true"
test "${{ steps.cfg4.outputs.service_found }}" = "false"
test "${{ steps.cfg4.outputs.build_context }}" = "."
test "${{ steps.cfg4.outputs.dockerfile_path }}" = "Dockerfile"
echo "missing-service caller-default OK"
test "${{ steps.cfg3.outputs.config_found }}" = "true"
test "${{ steps.cfg3.outputs.service_found }}" = "false"
test "${{ steps.cfg3.outputs.name }}" = ""
test "${{ steps.cfg3.outputs.build_context }}" = "."
test "${{ steps.cfg3.outputs.dockerfile_path }}" = "Dockerfile"
echo "missing-service fallback outputs OK"
58 changes: 31 additions & 27 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
```

Expand All @@ -35,8 +37,8 @@ This action parses the Skyhook configuration file and extracts service-specific
| `working_directory` | Path to the repository root containing `.skyhook/skyhook.yaml` | No | `.` |
| `service_name` | Name of the service to look up in the config | Yes | - |
| `config_path` | Path to the skyhook config file relative to working_directory | No | `.skyhook/skyhook.yaml` |
| `default_dockerfile_path` | Fallback for `dockerfile_path` when the YAML doesn't supply one (config unreadable, service missing, or `buildTool.docker.dockerfilePath` unset/empty). | No | `""` |
| `default_build_context` | Fallback for `build_context` when the YAML doesn't supply one. | No | `""` |
| `default_build_context` | Value emitted as `build_context` whenever it would otherwise be empty: YAML field absent, config file missing, or service missing. The action does not pick a value for you - callers must pass a non-empty default, otherwise the action fails loudly. | Yes | - |
| `default_dockerfile_path` | Value emitted as `dockerfile_path` whenever it would otherwise be empty (same triggers as `default_build_context`). Must be non-empty. | Yes | - |

## Outputs

Expand All @@ -46,11 +48,13 @@ This action parses the Skyhook configuration file and extracts service-specific
| `path` | Service path relative to repo root |
| `deployment_repo` | Separate deployment repository (if configured) |
| `deployment_repo_path` | Path within deployment repository |
| `build_context` | Docker build context relative to repo root (defaults to `.` when absent in config) |
| `dockerfile_path` | Dockerfile path relative to repo root |
| `build_context` | Docker build context relative to repo root. Falls back to `default_build_context` when absent/empty in config or when config/service is not found. |
| `dockerfile_path` | Dockerfile path relative to repo root. Falls back to `default_dockerfile_path` when absent/empty in config or when config/service is not found. |
| `config_found` | Whether the config file was found (`true`/`false`) |
| `service_found` | Whether the service was found in config (`true`/`false`) |

> When a default is applied because a YAML field was absent, the action emits a `::notice::` line. When the entire config file or service is missing, the action emits a `::warning::` (more prominent in the run UI) — so the source of every emitted value is visible at a glance.

## Config File Format

The action expects a `.skyhook/skyhook.yaml` file with the following structure:
Expand All @@ -70,7 +74,7 @@ services:
path: services/another
buildTool:
docker:
# buildContext omitted - defaults to "."
# buildContext omitted - falls back to default_build_context input
dockerfilePath: services/another/Dockerfile

environments:
Expand All @@ -96,44 +100,44 @@ jobs:
with:
working_directory: code
service_name: ${{ env.SERVICE_NAME }}
default_build_context: '.'
default_dockerfile_path: Dockerfile

- name: Build and push Docker image
uses: skyhook-io/docker-build-push-action@v1
with:
context: code/${{ steps.config.outputs.build_context }}
dockerfile: code/${{ steps.config.outputs.dockerfile_path || format('{0}/Dockerfile', steps.config.outputs.build_context) }}
dockerfile: code/${{ steps.config.outputs.dockerfile_path }}
image: ${{ inputs.image }}
```

## Behavior matrix

The two build-tool outputs (`build_context`, `dockerfile_path`) follow a single rule:

> **YAML wins when the config is readable AND the service exists AND the field is non-empty. In every other case, the caller-supplied `default_*` input is emitted.**
Let `BC` = `default_build_context` input, `DF` = `default_dockerfile_path` input.

`name`, `path`, `deployment_repo`, and `deployment_repo_path` are always sourced from the YAML and emit empty when the config/service can't be read — there are no `default_*` fallbacks for them.
**Uniform rule:** if the action would emit an empty value for `build_context` or `dockerfile_path`, it exits 1 instead. The corresponding default input must be non-empty whenever the YAML doesn't supply the value.

| Scenario | `config_found` | `service_found` | `build_context` | `dockerfile_path` |
|---|---|---|---|---|
| Config file missing | `false` | `false` | `default_build_context` | `default_dockerfile_path` |
| Config found, service missing | `true` | `false` | `default_build_context` | `default_dockerfile_path` |
| Service found, both fields set | `true` | `true` | from YAML | from YAML |
| Service found, only `buildContext` set | `true` | `true` | from YAML | `default_dockerfile_path` |
| Service found, neither field set | `true` | `true` | `default_build_context` | `default_dockerfile_path` |
| Duplicate service names | n/a | n/a | n/a | action exits 1 |
| Scenario | `config_found` | `service_found` | `name` / `path` / `deployment_*` | `build_context` | `dockerfile_path` |
|---|---|---|---|---|---|
| Service found, both YAML fields set | `true` | `true` | from config | from config | from config |
| Service found, only `buildContext` absent/`""`/`null`, `BC` non-empty | `true` | `true` | from config | `BC` | from config |
| Service found, only `dockerfilePath` absent/`""`/`null`, `DF` non-empty | `true` | `true` | from config | from config | `DF` |
| Service found, both YAML fields absent, both defaults non-empty | `true` | `true` | from config | `BC` | `DF` |
| Service missing, both defaults non-empty | `true` | `false` | `""` | `BC` | `DF` |
| Config file missing, both defaults non-empty | `false` | `false` | `""` | `BC` | `DF` |
| Any of the above where the relevant default is empty | n/a | n/a | n/a | n/a | **action exits 1** |
| Duplicate service names in config | n/a | n/a | n/a | n/a | action exits 1 |

Without `default_*` inputs the fallback is empty — meaning a workflow that wants a guaranteed-non-empty value should pass them at the call site:
The action **always** emits a non-empty `build_context` and `dockerfile_path` on success, so the consuming workflow can drop `||` fallbacks:

```yaml
- uses: skyhook-io/read-config@v1
with:
service_name: my-svc
# Computed sensible fallbacks the caller controls:
default_dockerfile_path: services/my-svc/Dockerfile
default_build_context: .
context: code/${{ steps.config.outputs.build_context }}
dockerfile: code/${{ steps.config.outputs.dockerfile_path }}
```

`config_found` and `service_found` remain available so the caller can distinguish between a YAML-sourced value and a fallback if needed.
Logging:
- A `::notice::` is emitted when a default is applied because a YAML field was absent for an otherwise-found service.
- A `::warning::` is emitted when the entire config file or service is missing and defaults flow through (more prominent than a notice — the not-found state is usually a misconfiguration).

## Runner requirements

Expand Down
24 changes: 14 additions & 10 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,18 @@ inputs:
description: 'Path to the skyhook config file relative to working_directory'
required: false
default: '.skyhook/skyhook.yaml'
default_dockerfile_path:
description: 'Fallback for the dockerfile_path output. Used when the YAML config is unreadable, the service is missing, or buildTool.docker.dockerfilePath is unset/empty. Empty by default — the caller decides whether to fall back ("Dockerfile", "<service-path>/Dockerfile", etc.).'
required: false
default: ''
default_build_context:
description: 'Fallback for the build_context output. Used when the YAML config is unreadable, the service is missing, or buildTool.docker.buildContext is unset/empty. Empty by default — the caller decides (".", the source path, etc.).'
required: false
default: ''
description: |
Value emitted as `build_context` whenever it would otherwise be empty: YAML field absent, config
file missing, or service missing. Must be non-empty - if a default would be applied and this input
is empty, the action exits 1 (refuses to emit an empty `build_context` that would break a downstream
`docker build`).
required: true
default_dockerfile_path:
description: |
Value emitted as `dockerfile_path` whenever it would otherwise be empty (same triggers as
`default_build_context`). Must be non-empty.
required: true

outputs:
# Service configuration
Expand All @@ -40,10 +44,10 @@ outputs:

# Build tool configuration
build_context:
description: 'Docker build context relative to repo root. Sourced from buildTool.docker.buildContext in the YAML when available; falls back to the default_build_context input otherwise.'
description: 'Docker build context relative to repo root. Falls back to the default_build_context input when absent/empty in config or when config/service is not found.'
value: ${{ steps.parse.outputs.build_context }}
dockerfile_path:
description: 'Dockerfile path relative to repo root. Sourced from buildTool.docker.dockerfilePath in the YAML when available; falls back to the default_dockerfile_path input otherwise.'
description: 'Dockerfile path relative to repo root. Falls back to the default_dockerfile_path input when absent/empty in config or when config/service is not found.'
value: ${{ steps.parse.outputs.dockerfile_path }}

# Status
Expand Down Expand Up @@ -118,8 +122,8 @@ runs:
WORKING_DIR: ${{ inputs.working_directory }}
SERVICE_NAME: ${{ inputs.service_name }}
CONFIG_PATH: ${{ inputs.config_path }}
DEFAULT_DOCKERFILE_PATH: ${{ inputs.default_dockerfile_path }}
DEFAULT_BUILD_CONTEXT: ${{ inputs.default_build_context }}
DEFAULT_DOCKERFILE_PATH: ${{ inputs.default_dockerfile_path }}
run: bash "$GITHUB_ACTION_PATH/scripts/parse.sh"

branding:
Expand Down
Loading
Loading