feat: add preflight manifest check to initialize and update actions#10
feat: add preflight manifest check to initialize and update actions#10nicomiguelino wants to merge 7 commits into
Conversation
Adds a local `preflight` composite action that installs yq and checks the manifest file before proceeding. For `update`, skips if the manifest is missing or has no `id`. For `initialize`, skips if the manifest already has an `id` (already initialized). Closes #9.
There was a problem hiding this comment.
Pull request overview
Adds a reusable “preflight” composite action to validate an Edge App manifest before running initialize/update, and gates the remaining steps to skip gracefully when the manifest state indicates the action should not proceed (missing/invalid id for update, already-initialized for initialize).
Changes:
- Added
preflightcomposite action that installsyqand evaluates manifest eligibility. - Updated
updateaction to run preflight first and skip all subsequent steps whenshould_skipis true. - Updated
initializeaction to run preflight first and skip all subsequent steps whenshould_skipis true.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
preflight/action.yml |
New composite action to install yq and decide whether downstream steps should be skipped based on manifest state. |
update/action.yml |
Adds preflight step and guards subsequent steps to avoid deploying when manifest is missing/invalid. |
initialize/action.yml |
Adds preflight step and guards subsequent steps to avoid re-initializing an already initialized app. |
Comments suppressed due to low confidence (2)
preflight/action.yml:24
- This downloads and executes a prebuilt binary without any integrity verification. Consider pinning by commit SHA and verifying a published checksum (or using a package manager / dedicated setup action) to reduce supply-chain risk.
wget https://github.com/mikefarah/yq/releases/download/v4.53.2/yq_linux_amd64 \
-O /usr/local/bin/yq && chmod +x /usr/local/bin/yq
preflight/action.yml:49
- The id check can incorrectly treat an empty string id (e.g.,
id: "") as present becauseyq '.id'outputs quoted strings. Prefer a raw/evaluable query (e.g.,yq -r '.id // ""'with a single evaluation) so the guard truly enforces non-empty id values.
elif [ -z "$(yq '.id' "$MANIFEST_FILE")" ] || [ "$(yq '.id' "$MANIFEST_FILE")" = "null" ]; then
echo "Manifest file '$MANIFEST_FILE' has no 'id' field. Skipping deployment."
SHOULD_SKIP=true
fi
elif [ "${{ inputs.mode }}" = "initialize" ]; then
if [ -f "$MANIFEST_FILE" ] && \
[ -n "$(yq '.id' "$MANIFEST_FILE")" ] && \
[ "$(yq '.id' "$MANIFEST_FILE")" != "null" ]; then
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
preflight/action.yml:51
yq '.id'is executed multiple times; capture the value once (or useyq -eand rely on exit status) to avoid redundant work and potential inconsistencies if the file changes between calls.
elif [ -z "$(yq '.id' "$MANIFEST_FILE")" ] || [ "$(yq '.id' "$MANIFEST_FILE")" = "null" ]; then
echo "Manifest file '$MANIFEST_FILE' has no 'id' field. Skipping deployment."
SHOULD_SKIP=true
fi
elif [ "${{ inputs.mode }}" = "initialize" ]; then
if [ -f "$MANIFEST_FILE" ] && \
[ -n "$(yq '.id' "$MANIFEST_FILE")" ] && \
[ "$(yq '.id' "$MANIFEST_FILE")" != "null" ]; then
preflight/action.yml:49
- If
inputs.modeis not exactlyupdateorinitialize, the script silently falls through withSHOULD_SKIP=false, which could lead to unintended deployments. Consider validatingmodeand failing fast (or explicitly settingshould_skip=true) on unknown values.
if [ "${{ inputs.mode }}" = "update" ]; then
if [ ! -f "$MANIFEST_FILE" ]; then
echo "Manifest file '$MANIFEST_FILE' not found. Skipping deployment."
SHOULD_SKIP=true
elif [ -z "$(yq '.id' "$MANIFEST_FILE")" ] || [ "$(yq '.id' "$MANIFEST_FILE")" = "null" ]; then
echo "Manifest file '$MANIFEST_FILE' has no 'id' field. Skipping deployment."
SHOULD_SKIP=true
fi
elif [ "${{ inputs.mode }}" = "initialize" ]; then
if [ -f "$MANIFEST_FILE" ] && \
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
../preflight is invalid; uses: paths resolve from the repo root, so ./preflight is correct for both initialize and update.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
preflight/action.yml:51
yq '.id'is executed multiple times in the same conditional, which is unnecessary work and makes the logic harder to read/maintain. Capture the id value once into a variable (or use a singleyq -epredicate) and reuse it for the null/empty checks.
elif [ -z "$(yq '.id' "$MANIFEST_FILE")" ] || [ "$(yq '.id' "$MANIFEST_FILE")" = "null" ]; then
echo "Manifest file '$MANIFEST_FILE' has no 'id' field. Skipping deployment."
SHOULD_SKIP=true
fi
elif [ "${{ inputs.mode }}" = "initialize" ]; then
if [ -f "$MANIFEST_FILE" ] && \
[ -n "$(yq '.id' "$MANIFEST_FILE")" ] && \
[ "$(yq '.id' "$MANIFEST_FILE")" != "null" ]; then
| mkdir -p "$RUNNER_TEMP/bin" | ||
| wget https://github.com/mikefarah/yq/releases/download/v4.53.2/yq_linux_amd64 \ | ||
| -O "$RUNNER_TEMP/bin/yq" && chmod +x "$RUNNER_TEMP/bin/yq" |
| SHOULD_SKIP=false | ||
|
|
||
| if [ "${{ inputs.mode }}" = "update" ]; then | ||
| if [ ! -f "$MANIFEST_FILE" ]; then | ||
| echo "Manifest file '$MANIFEST_FILE' not found. Skipping deployment." | ||
| SHOULD_SKIP=true | ||
| elif [ -z "$(yq '.id' "$MANIFEST_FILE")" ] || [ "$(yq '.id' "$MANIFEST_FILE")" = "null" ]; then | ||
| echo "Manifest file '$MANIFEST_FILE' has no 'id' field. Skipping deployment." | ||
| SHOULD_SKIP=true | ||
| fi | ||
| elif [ "${{ inputs.mode }}" = "initialize" ]; then | ||
| if [ -f "$MANIFEST_FILE" ] && \ | ||
| [ -n "$(yq '.id' "$MANIFEST_FILE")" ] && \ | ||
| [ "$(yq '.id' "$MANIFEST_FILE")" != "null" ]; then |
| if [ "${{ inputs.environment }}" = "stage" ]; then | ||
| MANIFEST_FILE="screenly_qc.yml" | ||
| else | ||
| MANIFEST_FILE="screenly.yml" | ||
| fi |
| if [ "${{ inputs.mode }}" = "update" ]; then | ||
| if [ ! -f "$MANIFEST_FILE" ]; then | ||
| echo "Manifest file '$MANIFEST_FILE' not found. Skipping deployment." | ||
| SHOULD_SKIP=true | ||
| elif [ -z "$(yq '.id' "$MANIFEST_FILE")" ] || [ "$(yq '.id' "$MANIFEST_FILE")" = "null" ]; then | ||
| echo "Manifest file '$MANIFEST_FILE' has no 'id' field. Skipping deployment." | ||
| SHOULD_SKIP=true | ||
| fi | ||
| elif [ "${{ inputs.mode }}" = "initialize" ]; then | ||
| if [ -f "$MANIFEST_FILE" ] && \ | ||
| [ -n "$(yq '.id' "$MANIFEST_FILE")" ] && \ | ||
| [ "$(yq '.id' "$MANIFEST_FILE")" != "null" ]; then | ||
| echo "Manifest file '$MANIFEST_FILE' already has an 'id'. App is already initialized. Skipping." | ||
| SHOULD_SKIP=true | ||
| fi | ||
| fi |
Summary
preflightcomposite action that installsyq(v4.53.2) and checks the manifest file before proceedingupdate: skips remaining steps if the manifest file is missing or has noidfieldinitialize: skips remaining steps if the manifest already has anid(app already initialized)Closes #9.