diff --git a/.editorconfig b/.editorconfig index 58de178..d3c791b 100644 --- a/.editorconfig +++ b/.editorconfig @@ -5,3 +5,8 @@ root = true end_of_line = lf charset = utf-8 insert_final_newline = true + +[*.sh] +indent_style = space +indent_size = 2 +switch_case_indent = true diff --git a/.gitea/workflows/validate-workflows.yml b/.gitea/workflows/validate-workflows.yml index e31f372..68defb6 100644 --- a/.gitea/workflows/validate-workflows.yml +++ b/.gitea/workflows/validate-workflows.yml @@ -26,3 +26,9 @@ jobs: - name: Check the coverage of the tools run: .venv/bin/python -m coverage report + + - name: Lint the shell scripts of the actions + env: + SHELLCHECK: .venv/bin/shellcheck + SHFMT: .venv/bin/shfmt + run: bash tools/lint-action-scripts.sh actions diff --git a/actions/connect-tailscale/action.yml b/actions/connect-tailscale/action.yml index 9952188..b8510a1 100644 --- a/actions/connect-tailscale/action.yml +++ b/actions/connect-tailscale/action.yml @@ -28,9 +28,13 @@ inputs: required: false default: "" version: - description: "Tailscale version to install when tailscale is not on the runner, e.g. 1.80.2. Pin it; latest is allowed" + description: "Exact Tailscale version to install when tailscale is not on the runner, e.g. 1.80.2. Empty uses the installed tailscale and fails when there is none" required: false - default: "latest" + default: "" + sha256: + description: "Optional SHA-256 of the downloaded archive tailscale__.tgz; the install fails when it differs" + required: false + default: "" userspace-networking: description: >- true runs tailscaled without a TUN device (no NET_ADMIN needed); tailnet @@ -70,6 +74,7 @@ runs: TS_TAGS: ${{ inputs.tags }} TS_HOSTNAME: ${{ inputs.hostname }} TS_VERSION: ${{ inputs.version }} + TS_SHA256: ${{ inputs.sha256 }} TS_USERSPACE: ${{ inputs.userspace-networking }} TS_ARGS: ${{ inputs.args }} TS_TIMEOUT: ${{ inputs.timeout }} diff --git a/actions/connect-tailscale/connect-tailscale.sh b/actions/connect-tailscale/connect-tailscale.sh index f673e84..917f292 100644 --- a/actions/connect-tailscale/connect-tailscale.sh +++ b/actions/connect-tailscale/connect-tailscale.sh @@ -7,14 +7,24 @@ # # Settings come from the environment, never from interpolated expressions: # TS_AUTHKEY, TS_OAUTH_SECRET, TS_TAGS, TS_HOSTNAME, TS_VERSION, TS_USERSPACE, -# TS_ARGS, TS_TIMEOUT, TS_HEALTH_CHECK_HOST. TS_TAILSCALE, TS_TAILSCALED, -# TS_SUDO and TS_STATE_DIR name the commands and the work folder, for the tests. +# TS_SHA256, TS_ARGS, TS_TIMEOUT, TS_HEALTH_CHECK_HOST. TS_TAILSCALE, +# TS_TAILSCALED, TS_SUDO, TS_STATE_DIR, TS_START_WAIT (tries) and +# TS_WAIT_INTERVAL (seconds) name the commands, the work folder and the waits, +# for the tests. # The auth key is passed to tailscale in a file of mode 600, never on a command # line. Every failure is printed as ::error::[category] message. TS_TAILSCALE="${TS_TAILSCALE:-tailscale}" TS_TAILSCALED="${TS_TAILSCALED:-tailscaled}" -SUDO="${TS_SUDO-sudo}" +# sudo is used only when the job is not root: a job container runs as root and +# often has no sudo. TS_SUDO overrides it (empty for none). +if [ -n "${TS_SUDO+x}" ]; then + SUDO="$TS_SUDO" +elif [ "$(id -u)" = "0" ]; then + SUDO="" +else + SUDO="sudo" +fi STATE_DIR="${TS_STATE_DIR:-${RUNNER_TEMP:-/tmp}/tailscale}" fail() { @@ -46,11 +56,13 @@ validate_inputs() { fi [[ "$TS_TIMEOUT" =~ ^[0-9]+$ ]] || fail configuration "timeout must be a number of seconds." - [[ "$TS_VERSION" =~ ^(latest|[0-9]+\.[0-9]+\.[0-9]+)$ ]] || - fail configuration "version must be latest or like 1.80.2." + [[ "$TS_VERSION" =~ ^([0-9]+\.[0-9]+\.[0-9]+)?$ ]] || + fail configuration "version must be an exact version like 1.80.2." + [[ "${TS_SHA256:-}" =~ ^([0-9a-fA-F]{64})?$ ]] || + fail configuration "sha256 must be 64 hexadecimal digits." case "$TS_USERSPACE" in - true | false) ;; - *) fail configuration "userspace-networking must be true or false." ;; + true | false) ;; + *) fail configuration "userspace-networking must be true or false." ;; esac [[ "${TS_HOSTNAME:-}" =~ ^[a-zA-Z0-9-]*$ ]] || fail configuration "hostname may hold letters, digits and hyphens only." @@ -58,14 +70,16 @@ validate_inputs() { machine_arch() { case "$(uname -m)" in - x86_64 | amd64) echo amd64 ;; - aarch64 | arm64) echo arm64 ;; - *) fail environment "no Tailscale build for $(uname -m)." ;; + x86_64 | amd64) echo amd64 ;; + aarch64 | arm64) echo arm64 ;; + *) fail environment "no Tailscale build for $(uname -m)." ;; esac } install_tools() { command -v "$TS_TAILSCALE" >/dev/null 2>&1 && command -v "$TS_TAILSCALED" >/dev/null 2>&1 && return 0 + [ -n "$TS_VERSION" ] || + fail configuration "tailscale is not on the runner; set version (for example 1.80.2) so it can be installed." local arch url tmp arch="$(machine_arch)" url="https://pkgs.tailscale.com/stable/tailscale_${TS_VERSION}_${arch}.tgz" @@ -73,6 +87,10 @@ install_tools() { tmp="$(mktemp -d)" curl -fsSL "$url" -o "$tmp/tailscale.tgz" || fail environment "could not download $url." + if [ -n "${TS_SHA256:-}" ]; then + echo "${TS_SHA256} $tmp/tailscale.tgz" | sha256sum -c --quiet - || + fail environment "the downloaded archive does not match sha256." + fi tar -xzf "$tmp/tailscale.tgz" -C "$tmp" --strip-components=1 || fail environment "could not unpack the Tailscale archive." $SUDO install -m 755 "$tmp/tailscale" "$tmp/tailscaled" /usr/local/bin/ || @@ -80,13 +98,34 @@ install_tools() { rm -rf "$tmp" } +daemon_answers() { + "$TS_TAILSCALE" version --daemon >/dev/null 2>&1 +} + start_daemon() { + # a daemon that already runs belongs to the host; this action would log it out + if daemon_answers; then + fail environment "a tailscaled already runs on this runner; the action starts its own and must not take over the host's. Use it on a job container." + fi local args=(--state=mem:) if [ "$TS_USERSPACE" = "true" ]; then args+=(--tun=userspace-networking --socks5-server=localhost:1055) fi mkdir -p "$STATE_DIR" $SUDO nohup "$TS_TAILSCALED" "${args[@]}" >"$STATE_DIR/tailscaled.log" 2>&1 & + : >"$STATE_DIR/started-by-action" + wait_for_daemon +} + +# wait_for_daemon: tailscaled needs a moment to create its socket +wait_for_daemon() { + local tries="${TS_START_WAIT:-30}" i + for ((i = 0; i < tries; i++)); do + daemon_answers && return 0 + sleep "${TS_WAIT_INTERVAL:-1}" + done + fail connection "tailscaled did not answer in $tries tries; the last lines of its log: $(tail -n 5 "$STATE_DIR/tailscaled.log" 2>/dev/null | tr ' +' ' ')" } # write_key_file: the auth key in a file of mode 600; prints its path @@ -127,6 +166,11 @@ tailnet_up() { } tailnet_down() { + # only a daemon this action started is logged out and stopped + if [ ! -f "$STATE_DIR/started-by-action" ]; then + echo "This action did not start tailscaled on this runner; nothing to do." + return 0 + fi $SUDO "$TS_TAILSCALE" logout 2>/dev/null || echo "The node was not logged in." $SUDO "$TS_TAILSCALE" down 2>/dev/null || true $SUDO pkill -x "$(basename "$TS_TAILSCALED")" 2>/dev/null || true @@ -138,12 +182,12 @@ main() { set -uo pipefail TS_USERSPACE="${TS_USERSPACE:-false}" TS_TIMEOUT="${TS_TIMEOUT:-60}" - TS_VERSION="${TS_VERSION:-latest}" + TS_VERSION="${TS_VERSION:-}" case "${1:-}" in - mask) mask_secrets ;; - up) tailnet_up ;; - down) tailnet_down ;; - *) fail configuration "unknown command '${1:-}'; use mask, up or down." ;; + mask) mask_secrets ;; + up) tailnet_up ;; + down) tailnet_down ;; + *) fail configuration "unknown command '${1:-}'; use mask, up or down." ;; esac } diff --git a/actions/connect-wireguard/connect-wireguard.sh b/actions/connect-wireguard/connect-wireguard.sh index 1579fa9..97768a1 100644 --- a/actions/connect-wireguard/connect-wireguard.sh +++ b/actions/connect-wireguard/connect-wireguard.sh @@ -15,7 +15,17 @@ WG_DIR="${WG_DIR:-/etc/wireguard}" WG_QUICK="${WG_QUICK:-wg-quick}" WG_PING="${WG_PING:-ping}" -SUDO="${WG_SUDO-sudo}" +# sudo is used only when the job is not root: a job container runs as root and +# often has no sudo. WG_SUDO overrides it (empty for none). +if [ -n "${WG_SUDO+x}" ]; then + SUDO="$WG_SUDO" +elif [ "$(id -u)" = "0" ]; then + SUDO="" +else + SUDO="sudo" +fi +# variables that must reach wg-quick through sudo, which resets the environment +WG_QUICK_ENV=() fail() { echo "::error::[$1] $2" @@ -23,14 +33,24 @@ fail() { } mask_secrets() { - local value line + local value line key for value in "${WG_PRIVATE_KEY:-}" "${WG_PRESHARED_KEY:-}"; do [ -n "$value" ] && echo "::add-mask::$value" done - # every line of a full configuration is masked, so a key cannot leak in pieces + # a full configuration: mask the value of every PrivateKey and PresharedKey + # line, and nothing else, so the rest of the log stays readable while IFS= read -r line; do - line="${line#"${line%%[![:space:]]*}"}" - [ -n "$line" ] && echo "::add-mask::$line" + line="${line//$' '/}" + key="${line%%=*}" + key="${key//[[:space:]]/}" + case "${key,,}" in + privatekey | presharedkey) + value="${line#*=}" + value="${value#"${value%%[![:space:]]*}"}" + value="${value%"${value##*[![:space:]]}"}" + [ -n "$value" ] && echo "::add-mask::$value" + ;; + esac done <<<"${WG_CONFIG:-}" return 0 } @@ -51,6 +71,11 @@ render_config() { [ -n "${!name:-}" ] || fail configuration "set config, or all of private-key, address, peer-public-key, endpoint and allowed-ips ($name is empty)." done + for name in WG_PRIVATE_KEY WG_ADDRESS WG_PEER_PUBLIC_KEY WG_ENDPOINT WG_ALLOWED_IPS WG_PRESHARED_KEY WG_DNS; do + # a line break would add directives (PostUp runs as root) to the file + [[ "${!name:-}" != *[[:cntrl:]]* ]] || + fail configuration "$name must be one line without control characters." + done [[ "$WG_KEEPALIVE" =~ ^[0-9]+$ ]] || fail configuration "persistent-keepalive must be a number of seconds." printf '[Interface]\nPrivateKey = %s\nAddress = %s\n' "$WG_PRIVATE_KEY" "$WG_ADDRESS" @@ -77,7 +102,7 @@ choose_implementation() { return 0 fi if command -v wireguard-go >/dev/null 2>&1 || $SUDO apt-get install -y -qq wireguard-go 2>/dev/null; then - export WG_QUICK_USERSPACE_IMPLEMENTATION=wireguard-go + WG_QUICK_ENV=(WG_QUICK_USERSPACE_IMPLEMENTATION=wireguard-go) echo "No kernel WireGuard module; using wireguard-go." else fail environment "the runner has no WireGuard kernel module and wireguard-go could not be installed; the runner container needs NET_ADMIN and /dev/net/tun (see guides/vpn-wireguard.md)." @@ -97,8 +122,12 @@ tunnel_up() { $SUDO install -d -m 700 "$WG_DIR" (umask 077 && printf '%s\n' "$rendered" | $SUDO tee "$conf" >/dev/null) $SUDO chmod 600 "$conf" - $SUDO "$WG_QUICK" up "$WG_INTERFACE" || + # env keeps the userspace variable through sudo; the file holds the key, so + # it is removed when the interface does not come up + if ! $SUDO env ${WG_QUICK_ENV[@]+"${WG_QUICK_ENV[@]}"} "$WG_QUICK" up "$WG_INTERFACE"; then + $SUDO rm -f "$conf" fail connection "wg-quick could not bring $WG_INTERFACE up." + fi if [ -n "${WG_HEALTH_CHECK_HOST:-}" ]; then "$WG_PING" -c 3 -W 5 "$WG_HEALTH_CHECK_HOST" >/dev/null 2>&1 || fail connection "health-check-host '$WG_HEALTH_CHECK_HOST' does not answer through the tunnel." @@ -119,10 +148,10 @@ main() { WG_INTERFACE="${WG_INTERFACE:-wg0}" WG_KEEPALIVE="${WG_KEEPALIVE:-25}" case "${1:-}" in - mask) mask_secrets ;; - up) tunnel_up ;; - down) tunnel_down ;; - *) fail configuration "unknown command '${1:-}'; use mask, up or down." ;; + mask) mask_secrets ;; + up) tunnel_up ;; + down) tunnel_down ;; + *) fail configuration "unknown command '${1:-}'; use mask, up or down." ;; esac } diff --git a/docs/adr/adr-0011-vpn-composite-actions.md b/docs/adr/adr-0011-vpn-composite-actions.md new file mode 100644 index 0000000..ae7f351 --- /dev/null +++ b/docs/adr/adr-0011-vpn-composite-actions.md @@ -0,0 +1,68 @@ +# ADR-0011: A VPN for deployment is delivered as a composite action + +## Metadata +| Key | Value | +| --- | --- | +| ID | ADR-0011 | +| CrossReference | [MIL-012], [ADR-0005], [ADR-0006] | + +## Version History +| Date | Status | Author | Reviewer | Change | Commit | +| --- | --- | --- | --- | --- | --- | +| 2026-10-19 | Proposed | Jens Tirsvad Nielsen | S01 | Initial version | [3325bea] | + +Allowed Status values: `Proposed`, `Accepted`, `Rejected`, `Deprecated`, `Superseded by ADR-NNNN`. + +--- + +## Context + +A deployment job often has to reach hosts that are not on the internet. [MIL-012] adds a way to bring a VPN up in the job, for WireGuard and for Tailscale. The GitHub marketplace has `tailscale/github-action` for Tailscale; it is written for GitHub Actions, has not been verified on Gitea Actions, and WireGuard has no such action. Every other shared automation of this repository is a reusable workflow ([ADR-0005], [ADR-0009], [ADR-0010]), so the question is whether a VPN is one too. + +Options: + +1. **A reusable workflow.** Consistent with the rest of the repository. But a called workflow runs as a separate job, on its own runner and network; a tunnel brought up there is gone when that job ends and is never seen by the caller's deployment job. It cannot work. +2. **The marketplace action `tailscale/github-action`.** No code to maintain. Written for GitHub, not verified on Gitea, nothing for WireGuard, and an action from outside the instance that receives the VPN secret. +3. **A composite action of this repository**, one for each VPN, in `actions//`, called as a step of the caller's own job with `uses:` and `gitea.action_path` for its script. It runs in the job, so the tunnel is there for the steps after it. + +Facts: a composite action in a subfolder of this repository resolves on the instance in both the short form (`TirSystem/github-action/actions/@`) and with the full address, and `gitea.action_path` points at its folder (verified in [MIL-012] task 1, runs 417 and 418 of the instance). A key must not reach a log or a command line. A tunnel needs a network interface, so the runner container needs `NET_ADMIN` and `/dev/net/tun`, which this repository cannot give. + +## Decision + +A VPN for deployment is delivered as **a composite action of this repository**, one per VPN: `actions/connect-wireguard` and `actions/connect-tailscale`. + +- Each action has a script file next to its `action.yml` that does the work. The `run:` lines only call it, and every input reaches the script as an environment variable, never as an interpolated expression ([ADR-0005]). +- Each action has an `up` and a `down` mode. The caller runs `down` in a last step with `if: always()`. +- Secrets are masked first, written only to a file of mode 600 where a file is needed, never printed and never put on a command line. +- The scripts are linted (shellcheck and shfmt) and tested offline with stub commands on every pull request. +- The actions are versioned with the repository ([ADR-0006]); a new action is a minor release, and callers pin the tag. +- The runner is outside the scope of the action: the operator (S03) provides a runner whose container has `NET_ADMIN` and `/dev/net/tun`. The actions state the requirement and fail with a message that names it. + +## Consequences + +**Positive:** + +- The tunnel lives in the job that deploys, which a reusable workflow cannot do. +- One implementation for Gitea, covered by offline tests; no GitHub-only action receives the VPN secret. +- WireGuard and Tailscale share one contract (`up`, `down`, a health check, masked secrets, categorised errors). + +**Negative:** + +- Two scripts to maintain, and behaviour that stubs cannot show (daemon start-up, `sudo`, the runner's capabilities) is verified only by runs on a real runner ([MIL-012] tasks 3 and 4). +- A composite action cannot declare a post step, so the caller must remember the `down` step; a cancelled job can leave a node or a key file behind until the runner cleans up. +- The action needs a privileged runner, which widens the attack surface of that runner; the guides tell the operator to keep it to a dedicated label. + +## Affected Artifacts + +- [MIL-012] — the decision behind its deliverable and its tasks 3 and 4. +- [ADR-0005] — the same contract style (script file, environment variables, categorised failures) is used for the actions. +- [ADR-0006] — a new action is a minor release. + +--- + +[MIL-012]: ../milestones/mil-012-vpn-actions-for-deployment.md +[ADR-0005]: ./adr-0005-quality-check-contract.md +[ADR-0006]: ./adr-0006-release-process-and-version-pinning.md +[ADR-0009]: ./adr-0009-wiki-publishing-reusable-workflow.md +[ADR-0010]: ./adr-0010-code-metrics-badges-contract.md +[3325bea]: https://git.tirsystem.com/TirSystem/github-action/commit/3325bea772304b54e8dfffad592b4afadc3ec2b8 diff --git a/docs/artifact-registry.md b/docs/artifact-registry.md index 94df088..92460e4 100644 --- a/docs/artifact-registry.md +++ b/docs/artifact-registry.md @@ -11,9 +11,9 @@ each document lives in *this* project and the next version to use. | SA | Stakeholder Analysis | docs/stakeholder-analysis.md | 002 | | PP | Project Plan | docs/project-plan.md | 002 | | MIL | Milestone / Gateway | docs/milestones/*.md | 013 | -| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 082 | +| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 083 | | TM | Traceability Matrix | docs/sqa/traceability-matrix.md | 002 | -| ADR | Architecture Decision Record | docs/adr/adr-NNNN-*.md | 0011 | +| ADR | Architecture Decision Record | docs/adr/adr-NNNN-*.md | 0012 | | UCD | Use Case Diagram | docs/use-case-diagram.md | 002 | | US | User Story | docs/user-stories.md | 002 | | UC | Use Case | docs/uc-NNN/uc.md | 009 | diff --git a/docs/milestones/mil-012-vpn-actions-for-deployment.md b/docs/milestones/mil-012-vpn-actions-for-deployment.md index 97ff53b..a82c913 100644 --- a/docs/milestones/mil-012-vpn-actions-for-deployment.md +++ b/docs/milestones/mil-012-vpn-actions-for-deployment.md @@ -4,12 +4,13 @@ | Key | Value | | --- | --- | | ID | MIL-012 | -| CrossReference | [BC-001], [US-001], [MIL-004], [MIL-011], [ADR-0006] | +| CrossReference | [BC-001], [US-001], [MIL-004], [MIL-011], [ADR-0006], [ADR-0011] | ## Version History | Date | Status | Author | Reviewer | Change | Commit | | --- | --- | --- | --- | --- | --- | -| 2026-10-19 | Accepted | Jens Tirsvad Nielsen | S01 | Initial version; reviewed (RC-080), accepted by the author in chat as stand-in for S01 with target date 2026-11-06 | [c5d5941] | +| 2026-10-19 | Deprecated | Jens Tirsvad Nielsen | S01 | Initial version; reviewed (RC-080), accepted by the author in chat as stand-in for S01 with target date 2026-11-06 | [c5d5941] | +| 2026-10-19 | Accepted | Jens Tirsvad Nielsen | S01 | Added the results of tasks 1, 6 and 9 and the fixes from the review of the first pull request; reviewed (RC-082), accepted by the author in chat as stand-in for S01 | [3325bea] | --- @@ -86,6 +87,28 @@ Release `2.2.0` of this repository with two composite actions, `actions/connect- | 11 | Pin the framework templates | In `TirSystem/sqa-qc-framework`, change `@wireguard-vpn-action` to `@v2` in the two templates and the branch guide, remove the "experimental" notes that the release resolves, and open the pull request after release 2.2.0. This is a change in another repository, in its own pull request. | No | | | 12 | Review | Record `RC-*` for this milestone, the actions with their tests, and the guides, each with a verdict. | No | | + +## Verification results + +Task 1 was run on 2026-10-19 with the throwaway branch `verify-subfolder-action`, which calls both actions in the short form (`TirSystem/github-action/actions/@`) and with the full address (`https://git.tirsystem.com/TirSystem/github-action/actions/@`), with every input empty, so that a resolved action fails with its own `[configuration]` message. + +| Call | Result | Evidence | +| --- | --- | --- | +| Either form at `@wireguard-vpn-action` | Fails before the step: `Unable to resolve wireguard-vpn-action: reference not found`. The branch had been deleted when its pull request merged, so the ref did not exist; this is not a defect of the form. | run 417 (`2424`) | +| Short form at `@main`, both actions | Resolves and runs: the log shows `Run Pre ...@main`, the action's `mask` step and its main step, which fails with `[configuration] set config, or all of private-key, ... (WG_PRIVATE_KEY is empty)` and `[configuration] give authkey or oauth-secret` | run 418 (`2425`), jobs `wg-short`, `ts-short` | +| Full address at `@main`, both actions | The same result as the short form | run 418, jobs `wg-full`, `ts-full` | + +Both forms resolve. The short form is the one the guides show; the full address stays in the guides as the fallback. `gitea.action_path` is set (the script path in the log is under `/var/run/act/actions//actions//`). A `uses:` of a branch that no longer exists fails with "reference not found", so a caller of an experimental ref breaks when its branch is deleted. + +Tasks 6 and 9 were done on 2026-10-19 on the branch `mil-012-review-fixes-and-docs`: + +- **Lint (task 6):** `tools/lint-action-scripts.sh` runs shellcheck and shfmt on `actions/**/*.sh`; `validate-workflows.yml` runs it with pinned tools (`shellcheck-py==0.10.0.1`, `shfmt-py==3.11.0.2`, from `tests/requirements.txt`), and `.editorconfig` sets the shell format (two spaces, indented case labels, as the shell conventions say). `tests/test_lint_action_scripts.py` seeds a shellcheck problem and a badly formatted script and shows both fail, and shows the scripts of the actions pass. +- **Decision (task 9):** [ADR-0011] (Proposed). Objective O6 and the Project Plan entries were added at the acceptance of this milestone ([RC-080], [RC-081]). + +The review of pull request 194 (ten findings) is answered in the same branch: `tailscaled` is waited for before `tailscale up`; the `wireguard-go` choice survives `sudo`; `sudo` is used only when the job is not root; a `tailscaled` that already runs on the runner is not taken over and `down` leaves a daemon it did not start; the Tailscale install needs an exact `version`, with an optional `sha256`; a configuration field with a line break is refused; only the key values are masked; the key file is removed when `wg-quick` fails; and the guides call `@main` until the release. These are offline-tested with stubs; tasks 3 and 4 still have to show them on a runner. + +Tasks 2, 3, 4, 5, 7, 8, 10, 11 and 12 are open: they need the trial runner (task 2), which is the operator's, or follow the verified guides (tasks 7 and 8) or the release (tasks 10 and 11). + --- [BC-001]: ../business-case.md @@ -98,6 +121,9 @@ Release `2.2.0` of this repository with two composite actions, `actions/connect- [MIL-012]: ./mil-012-vpn-actions-for-deployment.md [RC-080]: ../sqa/reviews/rc-080-mil-012-plan.md [RC-081]: ../sqa/reviews/rc-081-bc-001-o6.md +[RC-082]: ../sqa/reviews/rc-082-mil-012-results.md [ADR-0006]: ../adr/adr-0006-release-process-and-version-pinning.md +[ADR-0011]: ../adr/adr-0011-vpn-composite-actions.md [PP-001]: ../project-plan.md [c5d5941]: https://git.tirsystem.com/TirSystem/github-action/commit/c5d59418698316866807388bf61dcb6166d9fa8b +[3325bea]: https://git.tirsystem.com/TirSystem/github-action/commit/3325bea772304b54e8dfffad592b4afadc3ec2b8 diff --git a/docs/sqa/reviews/rc-082-mil-012-results.md b/docs/sqa/reviews/rc-082-mil-012-results.md new file mode 100644 index 0000000..b0ae4f9 --- /dev/null +++ b/docs/sqa/reviews/rc-082-mil-012-results.md @@ -0,0 +1,65 @@ +# Review Record: MIL-012 results of tasks 1, 6 and the review fixes (MIL-012) + +## Metadata +| Key | Value | +| --- | --- | +| ID | RC-082 | +| CrossReference | [MIL-012], [QC-MIL-001] | + +## Version History +| Date | Status | Author | Reviewer | Change | Commit | +| --- | --- | --- | --- | --- | --- | +| 2026-10-19 | Accepted | Jens Tirsvad Nielsen | S01 | Initial version; accepted by the author in chat as stand-in for S01 | [3325bea] | + +--- + +## Artifact Under Review + +- Instance reviewed: [MIL-012] +- Checklist used: [QC-MIL-001] +- Scope: delta re-review of the milestone after [RC-080]: the new `## Verification results` section (task 1, task 6, the fixes made after the review of pull request 194), the added `[ADR-0011]` in the CrossReference, and the claims about open tasks. Criteria 1 to 6 are re-read; none of the milestone's own text for them changed. [ADR-0011] is **not** reviewed here: it stays `Proposed` and needs a review against its own checklist +- Language and domain: n/a (technical type) +- Mode: supervised, not independent (the author, Jens Tirsvad Nielsen, asked in chat on 2026-10-19 for the acceptance and stands in for S01; the AI prepared the evidence) + +## Checklist Results + +| # | Criterion | Status | Evidence/Notes | +| --- | --- | --- | --- | +| 1 | A concrete deliverable is defined for every gate | Pass | Unchanged: release 2.2.0 with the two actions, guides, wiki pages in three languages, ADR, framework templates pinned. The results section does not claim any of it is delivered; it lists tasks 2 to 5, 7, 8 and 10 to 12 as open. | +| 2 | Explicit Go/No-Go criteria are stated for each gate | Pass | Unchanged, ten criteria. The results bear on two of them. Criterion 1 (a `uses:` of a subfolder action resolves, the form recorded): runs 417 and 418 show both forms resolve at `@main`, and the failure at the deleted branch is explained by the log text "reference not found". Criterion 6 (scripts linted in CI, a seeded error fails): met by `tools/lint-action-scripts.sh` and its tests on the working branch; the pull-request run of the new step is the final evidence. The other eight criteria are not claimed. | +| 3 | Dependencies on other milestones are explicitly mapped | Pass | Unchanged; [ADR-0011] is added to the CrossReference and exists. | +| 4 | Each milestone is traceable to a Business Case objective or KPI | Pass | Unchanged: objective O6 and success criterion 6 of [BC-001], reviewed in [RC-081]. | +| 5 | Milestone owner and approving reviewer are identified | Pass | Unchanged: owner S05, approving reviewer S01. | +| 6 | Milestone has a defined target date consistent with project constraints | Pass | Unchanged: 2026-11-06. The open tasks 2 to 5 need the operator's runner (S03); the milestone states the dependency, and the date is at risk until the runner exists. | + +## Results check + +| # | Claim in the results | Status | Evidence/Notes | +| --- | --- | --- | --- | +| 1 | Task 1: both forms resolve at `@main` | Pass | Run 418 (`.../actions/runs/2425`): jobs `wg-short`, `wg-full`, `ts-short`, `ts-full` each executed the action's `mask` step and main step and failed with the action's own `[configuration]` error. Read from the job logs through the API. | +| 2 | Run 417 failed because the branch was deleted | Pass | `git ls-remote` showed no `wireguard-vpn-action` head after pull request 194 merged; the log says `Unable to resolve wireguard-vpn-action: reference not found`. | +| 3 | Task 6: lint script, workflow step, pinned tools, seeded-error test | Pass | `tools/lint-action-scripts.sh`; the test file seeds a shellcheck problem and a badly formatted script, shows both fail, and shows the two action scripts pass; the full suite (590 tests) passes locally. Not yet run by the instance's CI. | +| 4 | The ten findings of the review of pull request 194 are answered | Pass | Each has a code change and an offline test (the daemon wait with a delayed stub, `sudo` that resets the environment, a host daemon not taken over, a line break refused, the key file removed on failure, `latest` refused). The wording of the results says they are offline-tested with stubs and that tasks 3 and 4 must show them on a runner. | +| 5 | Open tasks are listed honestly | Pass | Tasks 2, 3, 4, 5, 7, 8, 10, 11 and 12 are named as open, with the reason. | + +## Overall Verdict + +Go - the results recorded are accurate and claim no more than the evidence shows. The author confirmed the acceptance in chat on 2026-10-19 as stand-in for S01; this is not an independent review. + +## Action Items + +| Action | Owner | Due | +| --- | --- | --- | +| Review [ADR-0011] against its checklist and accept or change it; it is `Proposed` | S01 | Before task 10 (release 2.2.0) | +| Provide the `vpn` runner, the WireGuard peer and the Tailscale tag with an OAuth client for tasks 3 and 4; the target date 2026-11-06 depends on it | S03 | 2026-10-30 | +| Confirm this record by S01 when an independent reading is possible | S01 | 2026-11-02 | + +--- + +[MIL-012]: ../../milestones/mil-012-vpn-actions-for-deployment.md +[BC-001]: ../../business-case.md +[ADR-0011]: ../../adr/adr-0011-vpn-composite-actions.md +[RC-080]: ./rc-080-mil-012-plan.md +[RC-081]: ./rc-081-bc-001-o6.md +[QC-MIL-001]: ../../../framework/qc/artifacts/qc-milestones-gateways.md +[3325bea]: https://git.tirsystem.com/TirSystem/github-action/commit/3325bea772304b54e8dfffad592b4afadc3ec2b8 diff --git a/docs/sqa/traceability-matrix.md b/docs/sqa/traceability-matrix.md index f2b15ee..5159619 100644 --- a/docs/sqa/traceability-matrix.md +++ b/docs/sqa/traceability-matrix.md @@ -4,13 +4,13 @@ | Key | Value | | --- | --- | | ID | TM-001 | -| CrossReference | [BC-001], [SA-001], [PP-001], [MIL-001], [MIL-002], [MIL-003], [MIL-004], [MIL-005], [MIL-006], [MIL-007], [MIL-008], [MIL-009], [MIL-010], [MIL-011], [MIL-012], [UC-008], [RC-001], [RC-002], [RC-003], [RC-004], [RC-005], [RC-006], [RC-007], [RC-008], [RC-009], [RC-010], [RC-011], [RC-012], [RC-013], [RC-014], [RC-015], [RC-016], [RC-017], [RC-018], [RC-019], [RC-020], [RC-021], [RC-022], [RC-023], [RC-024], [RC-025], [RC-026], [RC-027], [RC-028], [RC-029], [RC-030], [RC-031], [RC-032], [RC-033], [RC-034], [RC-035], [RC-036], [RC-037], [RC-038], [RC-039], [RC-040], [RC-041], [RC-042], [RC-043], [RC-044], [RC-045], [RC-046], [RC-047], [RC-048], [RC-049], [RC-050], [RC-051], [RC-052], [RC-053], [RC-054], [RC-055], [RC-056], [RC-057], [RC-058], [RC-059], [RC-060], [RC-061], [RC-062], [RC-063], [RC-064], [RC-065], [RC-066], [RC-067], [RC-068], [RC-069], [RC-070], [RC-071], [RC-072], [RC-073], [RC-074], [RC-075], [RC-076], [RC-077], [RC-078], [RC-079], [RC-080], [RC-081], [ADR-0007], [ADR-0008], [ADR-0009], [ADR-0010], [UC-007], [UC-006], [ADR-0006], [UC-005], [ADR-0005], [UCD-001], [US-001], [UC-001], [UC-002], [UC-003], [UC-004], [ADR-0001], [ADR-0002], [ADR-0003], [ADR-0004] | +| CrossReference | [BC-001], [SA-001], [PP-001], [MIL-001], [MIL-002], [MIL-003], [MIL-004], [MIL-005], [MIL-006], [MIL-007], [MIL-008], [MIL-009], [MIL-010], [MIL-011], [MIL-012], [ADR-0011], [UC-008], [RC-001], [RC-002], [RC-003], [RC-004], [RC-005], [RC-006], [RC-007], [RC-008], [RC-009], [RC-010], [RC-011], [RC-012], [RC-013], [RC-014], [RC-015], [RC-016], [RC-017], [RC-018], [RC-019], [RC-020], [RC-021], [RC-022], [RC-023], [RC-024], [RC-025], [RC-026], [RC-027], [RC-028], [RC-029], [RC-030], [RC-031], [RC-032], [RC-033], [RC-034], [RC-035], [RC-036], [RC-037], [RC-038], [RC-039], [RC-040], [RC-041], [RC-042], [RC-043], [RC-044], [RC-045], [RC-046], [RC-047], [RC-048], [RC-049], [RC-050], [RC-051], [RC-052], [RC-053], [RC-054], [RC-055], [RC-056], [RC-057], [RC-058], [RC-059], [RC-060], [RC-061], [RC-062], [RC-063], [RC-064], [RC-065], [RC-066], [RC-067], [RC-068], [RC-069], [RC-070], [RC-071], [RC-072], [RC-073], [RC-074], [RC-075], [RC-076], [RC-077], [RC-078], [RC-079], [RC-080], [RC-081], [RC-082], [ADR-0007], [ADR-0008], [ADR-0009], [ADR-0010], [UC-007], [UC-006], [ADR-0006], [UC-005], [ADR-0005], [UCD-001], [US-001], [UC-001], [UC-002], [UC-003], [UC-004], [ADR-0001], [ADR-0002], [ADR-0003], [ADR-0004] | ## Version History | Date | Status | Author | Reviewer | Change | Commit | | --- | --- | --- | --- | --- | --- | -| 2026-10-14 | Proposed | Jens Tirsvad Nielsen | S01 | Added RC-079 (MIL-011 outcome, release 2.1.1) | [4aa3fd3] | | 2026-10-19 | Proposed | Jens Tirsvad Nielsen | S01 | Added MIL-012 (VPN actions for deployment), RC-080 and RC-081 (its plan and objective O6); CrossReference lists RC-076 to RC-079 | [c5d5941] | +| 2026-10-19 | Proposed | Jens Tirsvad Nielsen | S01 | Added ADR-0011 and RC-082 (MIL-012 results of tasks 1, 6 and the review fixes) | [3325bea] | --- @@ -32,6 +32,7 @@ updated whenever an artifact instance is created or reviewed. | [MIL-003] | MIL | [BC-001], [PP-001], [MIL-002] | [MIL-004] | [RC-006], [RC-028], [RC-033], [RC-039] | | [MIL-004] | MIL | [BC-001], [PP-001], [MIL-003] | - | [RC-007], [RC-029], [RC-034], [RC-048] | | [ADR-0010] | ADR | [MIL-010], [UC-008], [US-001], [ADR-0005], [ADR-0006] | - | [RC-072], [RC-073] | +| [ADR-0011] | ADR | [MIL-012], [ADR-0005], [ADR-0006] | - | - | | [RC-001] | RC | [BC-001] | - | - | | [RC-002] | RC | [PP-001] | - | - | | [RC-003] | RC | [MIL-001] | - | - | @@ -45,7 +46,7 @@ updated whenever an artifact instance is created or reviewed. | [MIL-009] | MIL | [BC-001], [PP-001], [MIL-008], [ADR-0004], [ADR-0006], [ADR-0009] | - | [RC-064], [RC-066] | | [MIL-010] | MIL | [BC-001], [PP-001], [MIL-003], [MIL-004] | [MIL-011] | [RC-068], [RC-074], [RC-075] | | [MIL-011] | MIL | [BC-001], [PP-001], [MIL-010], [ADR-0006], [ADR-0010] | [MIL-012] | [RC-076], [RC-077], [RC-078], [RC-079] | -| [MIL-012] | MIL | [BC-001], [PP-001], [MIL-004], [MIL-011], [ADR-0006] | - | [RC-080] | +| [MIL-012] | MIL | [BC-001], [PP-001], [MIL-004], [MIL-011], [ADR-0006], [ADR-0011] | - | [RC-080], [RC-082] | | [RC-007] | RC | [MIL-004] | - | - | | [RC-008] | RC | [BC-001] | - | - | | [RC-009] | RC | [PP-001] | - | - | @@ -140,6 +141,7 @@ updated whenever an artifact instance is created or reviewed. | [RC-079] | RC | [MIL-011] | - | - | | [RC-080] | RC | [MIL-012], [PP-001] | - | - | | [RC-081] | RC | [BC-001] | - | - | +| [RC-082] | RC | [MIL-012] | - | - | ## Coverage Notes @@ -166,6 +168,7 @@ updated whenever an artifact instance is created or reviewed. [MIL-012]: ../milestones/mil-012-vpn-actions-for-deployment.md [MIL-009]: ../milestones/mil-009-migrate-wiki-callers-and-remove-scoped-workflow.md [ADR-0010]: ../adr/adr-0010-code-metrics-badges-contract.md +[ADR-0011]: ../adr/adr-0011-vpn-composite-actions.md [RC-001]: ./reviews/rc-001-business-case.md [RC-002]: ./reviews/rc-002-project-plan.md [RC-003]: ./reviews/rc-003-mil-001.md @@ -257,6 +260,7 @@ updated whenever an artifact instance is created or reviewed. [RC-079]: ./reviews/rc-079-mil-011-outcome.md [RC-080]: ./reviews/rc-080-mil-012-plan.md [RC-081]: ./reviews/rc-081-bc-001-o6.md +[RC-082]: ./reviews/rc-082-mil-012-results.md [ADR-0008]: ../adr/adr-0008-cpp-csharp-quality-check-contract.md [ADR-0009]: ../adr/adr-0009-wiki-publishing-reusable-workflow.md [ADR-0007]: ../adr/adr-0007-release-publish-destination.md @@ -267,5 +271,5 @@ updated whenever an artifact instance is created or reviewed. [UC-005]: ../uc-005/uc.md [ADR-0005]: ../adr/adr-0005-quality-check-contract.md [3d7a9df]: https://git.tirsystem.com/TirSystem/github-action/commit/3d7a9df3a3a4712dcc5214a71b0569d90122bf4e -[4aa3fd3]: https://git.tirsystem.com/TirSystem/github-action/commit/4aa3fd38f97c26a5d52abc566e958759583ccb85 [c5d5941]: https://git.tirsystem.com/TirSystem/github-action/commit/c5d59418698316866807388bf61dcb6166d9fa8b +[3325bea]: https://git.tirsystem.com/TirSystem/github-action/commit/3325bea772304b54e8dfffad592b4afadc3ec2b8 diff --git a/guides/vpn-tailscale.md b/guides/vpn-tailscale.md index dc796af..082c82a 100644 --- a/guides/vpn-tailscale.md +++ b/guides/vpn-tailscale.md @@ -5,8 +5,8 @@ a Gitea Actions runner, so the next steps of the same job (a deployment over SSH, a migration) can reach hosts on the tailnet. It does what the GitHub marketplace action `tailscale/github-action` does, written for Gitea Actions: that action is built for GitHub and has not been verified on Gitea. It is -**experimental**: it lives on the branch `wireguard-vpn-action` until a trial -shows it is wanted. For a plain tunnel without a third-party service, see +**experimental**: it is on `main` but not released, and it has not yet run on a +runner with a real tailnet (MIL-012). For a plain tunnel without a third-party service, see [WireGuard](vpn-wireguard.md). A node belongs to one job: every job that deploys joins on its own and logs out @@ -23,7 +23,7 @@ jobs: - uses: actions/checkout@v4.2.2 - name: Connect Tailscale - uses: TirSystem/github-action/actions/connect-tailscale@v2 + uses: TirSystem/github-action/actions/connect-tailscale@main with: oauth-secret: ${{ secrets.TS_OAUTH_SECRET }} tags: tag:ci @@ -35,16 +35,17 @@ jobs: - name: Disconnect Tailscale if: always() - uses: TirSystem/github-action/actions/connect-tailscale@v2 + uses: TirSystem/github-action/actions/connect-tailscale@main with: action: down ``` If the short form does not resolve on your instance (Gitea looks for the action on the host set in `DEFAULT_ACTIONS_URL`), use the full address: -`https://git.tirsystem.com/TirSystem/github-action/actions/connect-tailscale@v2`. -To try the branch before it is released, replace `@v2` with -`@wireguard-vpn-action`. +`https://git.tirsystem.com/TirSystem/github-action/actions/connect-tailscale@main`. + +Until release 2.2.0 the actions exist only on `main`, so the examples call +`@main`; `@v2` does not contain them yet. After the release, use `@v2`. ## Inputs @@ -55,7 +56,8 @@ To try the branch before it is released, replace `@v2` with | `oauth-secret` | | The secret of an OAuth client with the scope `auth_keys`, as a secret. Needs `tags` | | `tags` | | ACL tags of the node, `tag:ci` or `tag:ci,tag:deploy`. Required with `oauth-secret` | | `hostname` | runner name | Name of the node in the tailnet | -| `version` | `latest` | The version to install when the runner has no `tailscale`. Pin it | +| `version` | | The exact version to install when the runner has no `tailscale`, for example `1.80.2`. Empty uses the installed one and fails when there is none | +| `sha256` | | The SHA-256 of the downloaded archive `tailscale__.tgz`; the install fails when it differs. Recommended | | `userspace-networking` | `false` | `true` runs without a TUN device | | `args` | | Extra flags for `tailscale up`, for example `--accept-routes`. Never a secret | | `timeout` | `60` | Seconds to wait for the node | @@ -74,15 +76,23 @@ To try the branch before it is released, replace `@v2` with start with `GITEA_` or `GITHUB_`. 4. Rotate the secret when a person with access leaves. -The node is ephemeral and its state is kept in memory, so it disappears from the -tailnet when the job ends, even if the last step does not run. +The state of the node is kept in memory. With an OAuth client the key is +ephemeral, so the node disappears from the tailnet when the job ends, even if +the last step does not run. With an auth key that is not ephemeral, a cancelled +job can leave the node in the tailnet until `down` runs or it is removed in the +admin console: create the key as ephemeral. ## The runner -- The job runs as root or can use `sudo`. The action downloads the Tailscale - binaries from `pkgs.tailscale.com` when `tailscale` is not on the runner. - Pin `version`; a runner image that already contains Tailscale skips the - download. +- The job runs as root, or `sudo` is available (the action uses `sudo` only + when the job is not root). The action downloads the Tailscale binaries from + `pkgs.tailscale.com` when `tailscale` is not on the runner; `version` is then + required, and `sha256` makes the install check the archive. A runner image + that already contains Tailscale skips the download. +- The action starts its own `tailscaled`. It refuses to run when a `tailscaled` + already answers on the runner, because that daemon belongs to the host and + `down` would log it out: use the action on job containers, not on a runner + that runs jobs directly on a host with its own Tailscale. - **TUN mode** (the default) needs the capability `NET_ADMIN` and the device `/dev/net/tun` in the runner container: in the `act_runner` configuration set under `container:` `options: "--cap-add=NET_ADMIN --device=/dev/net/tun"`. @@ -107,7 +117,9 @@ tailnet when the job ends, even if the last step does not run. never on a command line, and deletes the file at once; - prints errors as `::error::[category] message`, with the categories `configuration`, `environment` and `connection`; -- on `down` logs the node out, stops `tailscaled` and deletes its work folder. +- waits until `tailscaled` answers before `tailscale up` (up to 30 seconds); +- on `down` logs the node out, stops `tailscaled` and deletes its work folder, + and only when this action started that daemon. The `file:` form of `--auth-key` needs a recent Tailscale; with an old pinned version `tailscale up` reports an invalid key. diff --git a/guides/vpn-wireguard.md b/guides/vpn-wireguard.md index 1578f61..ddb02e5 100644 --- a/guides/vpn-wireguard.md +++ b/guides/vpn-wireguard.md @@ -4,7 +4,8 @@ The composite action `actions/connect-wireguard` brings a WireGuard tunnel up on a Gitea Actions runner, so the next steps of the same job (a deployment over SSH, a database migration) can reach hosts that are not on the internet. It is the WireGuard counterpart of the Tailscale action. It is **experimental**: it -lives on a branch until a trial shows it is wanted. +is on `main` but not released, and it has not yet run on a runner with a real +tunnel (MIL-012). A tunnel belongs to one job: every job that deploys brings its own tunnel up and takes it down. It cannot be a reusable workflow, because a called workflow @@ -20,7 +21,7 @@ jobs: - uses: actions/checkout@v4.2.2 - name: Connect the VPN - uses: TirSystem/github-action/actions/connect-wireguard@v2 + uses: TirSystem/github-action/actions/connect-wireguard@main with: config: ${{ secrets.WG_CONFIG }} health-check-host: 10.8.0.1 @@ -30,17 +31,17 @@ jobs: - name: Disconnect the VPN if: always() - uses: TirSystem/github-action/actions/connect-wireguard@v2 + uses: TirSystem/github-action/actions/connect-wireguard@main with: action: down ``` If the short form does not resolve on your instance (Gitea looks for the action on the host set in `DEFAULT_ACTIONS_URL`), use the full address: -`https://git.tirsystem.com/TirSystem/github-action/actions/connect-wireguard@v2`. +`https://git.tirsystem.com/TirSystem/github-action/actions/connect-wireguard@main`. -To try the branch before it is released, replace `@v2` with -`@wireguard-vpn-action`. +Until release 2.2.0 the actions exist only on `main`, so the examples call +`@main`; `@v2` does not contain them yet. After the release, use `@v2`. ## Inputs @@ -71,8 +72,9 @@ To try the branch before it is released, replace `@v2` with A tunnel needs a network interface, so the runner needs more than a plain job container: -- the job runs as root or can use `sudo` (the action installs - `wireguard-tools` with `apt` when it is missing, so a Debian or Ubuntu image); +- the job runs as root, or `sudo` is available (the action uses `sudo` only + when the job is not root, and installs `wireguard-tools` with `apt` when it is + missing, so a Debian or Ubuntu image); - the container has the capability `NET_ADMIN` and the device `/dev/net/tun`. In the `act_runner` configuration set under `container:` `options: "--cap-add=NET_ADMIN --device=/dev/net/tun"`; @@ -85,9 +87,13 @@ privilege. ## What the action does -- masks the keys in the log before anything else; +- masks the keys in the log before anything else (the values of `PrivateKey` + and `PresharedKey` of a full configuration, and the key inputs); +- refuses an input that holds a line break, because an extra line in the file + could add a `PostUp` command that runs as root; - writes `/etc/wireguard/.conf` with mode 600 and brings it up with - `wg-quick`; + `wg-quick` (through `env`, so the `wireguard-go` choice survives `sudo`); the + file is removed again when the interface does not come up; - prints errors as `::error::[category] message`, with the categories `configuration`, `environment` and `connection`; - on `down` takes the interface down and deletes the file. diff --git a/tests/requirements.txt b/tests/requirements.txt index e18e2b4..cfb4328 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -1,3 +1,5 @@ pytest>=8,<10 PyYAML>=6,<7 coverage>=7,<8 +shellcheck-py==0.10.0.1 +shfmt-py==3.11.0.2 diff --git a/tests/test_connect_tailscale.py b/tests/test_connect_tailscale.py index 7c9e077..aa64c30 100644 --- a/tests/test_connect_tailscale.py +++ b/tests/test_connect_tailscale.py @@ -36,6 +36,11 @@ case "$1" in done exit "${UP_EXIT:-0}" ;; ping) exit "${PING_EXIT:-0}" ;; + version) + # the daemon answers only after tailscaled was started, and after DAEMON_DELAY calls + [ -f "$STUB_DAEMON" ] || exit 1 + n=$(( $(cat "$STUB_DAEMON") + 1 )); echo "$n" > "$STUB_DAEMON" + [ "$n" -gt "${DAEMON_DELAY:-0}" ] ;; esac """ @@ -52,13 +57,19 @@ def make_stub(path: Path, body: str) -> None: def run(tmp_path: Path, command: str, **env: str) -> subprocess.CompletedProcess: make_stub(tmp_path / "tailscale", TAILSCALE_STUB) - make_stub(tmp_path / "tailscaled", "#!/usr/bin/env bash\necho \"$*\" >> \"$STUB_LOG\"\n") + make_stub( + tmp_path / "tailscaled", + '#!/usr/bin/env bash\necho "$*" >> "$STUB_LOG"\necho 0 > "$STUB_DAEMON"\n', + ) full = { "PATH": f"{tmp_path}:{os.environ['PATH']}", "TS_SUDO": "", "TS_STATE_DIR": str(tmp_path / "state"), "STUB_LOG": str(tmp_path / "log"), "STUB_KEY": str(tmp_path / "key"), + "STUB_DAEMON": str(tmp_path / "daemon"), + "TS_WAIT_INTERVAL": "0.05", + "TS_START_WAIT": "100", **env, } return subprocess.run( @@ -77,7 +88,7 @@ class TestDefinition: def test_inputs_are_the_contract(self, document): assert set(document["inputs"]) == { "action", "authkey", "oauth-secret", "tags", "hostname", "version", - "userspace-networking", "args", "timeout", "health-check-host", + "sha256", "userspace-networking", "args", "timeout", "health-check-host", } def test_uses_gitea_contexts_only(self): @@ -124,6 +135,8 @@ class TestUp: ({"TS_AUTHKEY": AUTHKEY, "TS_TAGS": "ci"}, "tags must look like"), ({"TS_AUTHKEY": AUTHKEY, "TS_TIMEOUT": "soon"}, "timeout"), ({"TS_AUTHKEY": AUTHKEY, "TS_VERSION": "1.x"}, "version"), + ({"TS_AUTHKEY": AUTHKEY, "TS_VERSION": "latest"}, "version"), + ({"TS_AUTHKEY": AUTHKEY, "TS_SHA256": "abc"}, "sha256"), ({"TS_AUTHKEY": AUTHKEY, "TS_USERSPACE": "yes"}, "userspace-networking"), ({"TS_AUTHKEY": AUTHKEY, "TS_HOSTNAME": "bad name"}, "hostname"), ], @@ -148,6 +161,40 @@ class TestUp: assert "health-check-host" in result.stdout +class TestDaemon: + def test_waits_until_the_daemon_answers(self, tmp_path): + result = run(tmp_path, "up", TS_AUTHKEY=AUTHKEY, DAEMON_DELAY="5") + assert result.returncode == 0, result.stdout + assert "up --auth-key" in log(tmp_path) + + def test_a_daemon_that_never_answers_fails_before_up(self, tmp_path): + result = run(tmp_path, "up", TS_AUTHKEY=AUTHKEY, DAEMON_DELAY="1000", TS_START_WAIT="3") + assert result.returncode == 1 + assert "::error::[connection] tailscaled did not answer" in result.stdout + assert "up --auth-key" not in log(tmp_path) + + def test_a_daemon_of_the_host_is_not_taken_over(self, tmp_path): + (tmp_path / "daemon").write_text("0", encoding="utf-8") + result = run(tmp_path, "up", TS_AUTHKEY=AUTHKEY) + assert result.returncode == 1 + assert "::error::[environment] a tailscaled already runs" in result.stdout + calls = log(tmp_path) # only the probe ran: no second daemon, no up + assert "--state=mem:" not in calls + assert "up --auth-key" not in calls + + def test_down_leaves_a_daemon_it_did_not_start(self, tmp_path): + (tmp_path / "daemon").write_text("0", encoding="utf-8") + result = run(tmp_path, "down") + assert result.returncode == 0 + assert "nothing to do" in result.stdout + assert not (tmp_path / "log").exists() + + def test_without_tailscale_a_version_is_required(self, tmp_path): + result = run(tmp_path, "up", TS_AUTHKEY=AUTHKEY, TS_TAILSCALE="no-such-tailscale") + assert result.returncode == 1 + assert "::error::[configuration] tailscale is not on the runner; set version" in result.stdout + + class TestDownAndMask: def test_down_logs_out_and_removes_the_state(self, tmp_path): run(tmp_path, "up", TS_AUTHKEY=AUTHKEY) diff --git a/tests/test_connect_wireguard.py b/tests/test_connect_wireguard.py index fc60031..27ea763 100644 --- a/tests/test_connect_wireguard.py +++ b/tests/test_connect_wireguard.py @@ -131,6 +131,40 @@ class TestUp: assert "::error::[connection]" in result.stdout +def make_exec(path: Path, body: str) -> Path: + path.write_text(body, encoding="utf-8") + path.chmod(path.stat().st_mode | stat.S_IXUSR) + return path + + +class TestEnvironment: + def test_the_userspace_variable_reaches_wg_quick_through_sudo(self, tmp_path): + # a sudo that resets the environment, as the real one does + sudo = make_exec( + tmp_path / "sudo-reset", + '#!/usr/bin/env bash\nexec env -i "PATH=$PATH" "STUB_LOG=$STUB_LOG" "$@"\n', + ) + quick = make_exec( + tmp_path / "quick-env", + '#!/usr/bin/env bash\necho "$WG_QUICK_USERSPACE_IMPLEMENTATION $*" >> "$STUB_LOG"\n', + ) + result = run(tmp_path, "up", **FIELDS, WG_SUDO=str(sudo), WG_QUICK=str(quick)) + assert result.returncode == 0, result.stdout + result.stderr + assert "wireguard-go up wg0" in (tmp_path / "log").read_text(encoding="utf-8") + + def test_a_field_with_a_line_break_is_refused(self, tmp_path): + result = run(tmp_path, "up", **{**FIELDS, "WG_ALLOWED_IPS": "10.8.0.0/24\nPostUp = id"}) + assert result.returncode == 1 + assert "WG_ALLOWED_IPS must be one line" in result.stdout + assert not (tmp_path / "etc" / "wg0.conf").exists() + + def test_the_key_file_is_removed_when_wg_quick_fails(self, tmp_path): + failing = make_exec(tmp_path / "quick-fail", "#!/usr/bin/env bash\nexit 1\n") + result = run(tmp_path, "up", **FIELDS, WG_QUICK=str(failing)) + assert result.returncode == 1 + assert not (tmp_path / "etc" / "wg0.conf").exists() + + class TestDownAndMask: def test_down_removes_the_config(self, tmp_path): run(tmp_path, "up", **FIELDS) @@ -141,7 +175,8 @@ class TestDownAndMask: def test_mask_prints_every_secret(self, tmp_path): result = run(tmp_path, "mask", WG_PRIVATE_KEY=KEY, WG_CONFIG="[Interface]\nPrivateKey = abc") assert f"::add-mask::{KEY}" in result.stdout - assert "::add-mask::PrivateKey = abc" in result.stdout + assert "::add-mask::abc" in result.stdout + assert "[Interface]" not in result.stdout # only the key values are masked def test_unknown_command_fails(self, tmp_path): assert run(tmp_path, "sideways").returncode == 1 diff --git a/tests/test_lint_action_scripts.py b/tests/test_lint_action_scripts.py new file mode 100644 index 0000000..a13f63c --- /dev/null +++ b/tests/test_lint_action_scripts.py @@ -0,0 +1,86 @@ +"""Offline tests for tools/lint-action-scripts.sh (MIL-012 task 6). + +The script runs shellcheck and shfmt on a folder; the tests give it a clean +script, a script with a shellcheck problem and a badly formatted one, so a +seeded error is shown to fail the run. +""" + +import os +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest +import yaml + +from workflow_source import REPO_ROOT + +SCRIPT = REPO_ROOT / "tools" / "lint-action-scripts.sh" +WORKFLOW = REPO_ROOT / ".gitea" / "workflows" / "validate-workflows.yml" +BASH = shutil.which("bash") +# the tools are in the virtual environment of the interpreter that runs the tests +SEARCH_PATH = os.pathsep.join([str(Path(sys.executable).parent), os.environ.get("PATH", "")]) +SHELLCHECK = shutil.which("shellcheck", path=SEARCH_PATH) +SHFMT = shutil.which("shfmt", path=SEARCH_PATH) +HAVE_TOOLS = BASH and SHELLCHECK and SHFMT + +CLEAN = '#!/usr/bin/env bash\nmain() {\n echo "$1"\n}\n\nmain "$@"\n' +SHELLCHECK_PROBLEM = '#!/usr/bin/env bash\nmain() {\n echo $1\n}\n\nmain "$@"\n' +BADLY_FORMATTED = '#!/usr/bin/env bash\nmain() {\n echo "$1"\n}\n\nmain "$@"\n' + + +def lint(folder: Path) -> subprocess.CompletedProcess: + # shfmt reads .editorconfig from the folder upwards; a temporary folder is + # outside this repository, so it gets a copy of the repository's file + if folder != REPO_ROOT / "actions": + shutil.copy(REPO_ROOT / ".editorconfig", folder / ".editorconfig") + env = {**os.environ, "SHELLCHECK": str(SHELLCHECK), "SHFMT": str(SHFMT)} + return subprocess.run( + [BASH, str(SCRIPT), str(folder)], + capture_output=True, + text=True, + check=False, + cwd=REPO_ROOT, + env=env, + ) + + +@pytest.mark.skipif(not HAVE_TOOLS, reason="bash, shellcheck and shfmt are needed") +class TestLint: + def test_a_clean_script_passes(self, tmp_path): + (tmp_path / "ok.sh").write_bytes(CLEAN.encode()) # LF on every platform + result = lint(tmp_path) + assert result.returncode == 0, result.stdout + result.stderr + assert "clean" in result.stdout + + def test_a_shellcheck_problem_fails(self, tmp_path): + (tmp_path / "bad.sh").write_bytes(SHELLCHECK_PROBLEM.encode()) # LF on every platform + result = lint(tmp_path) + assert result.returncode == 1 + assert "::error::[lint] shellcheck" in result.stdout + + def test_a_badly_formatted_script_fails(self, tmp_path): + (tmp_path / "ugly.sh").write_bytes(BADLY_FORMATTED.encode()) # LF on every platform + result = lint(tmp_path) + assert result.returncode == 1 + assert "::error::[lint] shfmt" in result.stdout + + def test_no_scripts_is_not_a_failure(self, tmp_path): + result = lint(tmp_path) + assert result.returncode == 0 + assert "nothing was checked" in result.stdout + + def test_the_scripts_of_the_actions_are_clean(self): + result = lint(REPO_ROOT / "actions") + assert result.returncode == 0, result.stdout + result.stderr + + +class TestWorkflow: + def test_validate_workflows_runs_the_lint_with_pinned_tools(self): + document = yaml.safe_load(WORKFLOW.read_text(encoding="utf-8")) + commands = [step.get("run", "") for step in document["jobs"]["validate"]["steps"]] + assert any("tools/lint-action-scripts.sh" in command for command in commands) + requirements = (REPO_ROOT / "tests" / "requirements.txt").read_text(encoding="utf-8") + assert "shellcheck-py==" in requirements + assert "shfmt-py==" in requirements diff --git a/tools/lint-action-scripts.sh b/tools/lint-action-scripts.sh new file mode 100644 index 0000000..1e680d5 --- /dev/null +++ b/tools/lint-action-scripts.sh @@ -0,0 +1,44 @@ +#!/usr/bin/env bash +# Lint the shell scripts of the composite actions (MIL-012 task 6): shellcheck, +# and shfmt in the format .editorconfig sets (two spaces, indented case labels). +# validate-workflows.yml runs it on every pull request and push to main. +# +# Usage: lint-action-scripts.sh [] (default: actions) +# Env: SHELLCHECK, SHFMT the commands to run (default: from the PATH) +# Exit: 0 every script is clean (or there is none); 1 at least one finding or +# a missing tool +# Needs: bash, find, shellcheck, shfmt + +SHELLCHECK="${SHELLCHECK:-shellcheck}" +SHFMT="${SHFMT:-shfmt}" + +main() { + set -u + local dir="${1:-actions}" failed=0 tool + for tool in "$SHELLCHECK" "$SHFMT"; do + command -v "$tool" >/dev/null 2>&1 || { + echo "::error::[environment] $tool is not installed." + exit 1 + } + done + local files=() + while IFS= read -r file; do + files+=("$file") + done < <(find "$dir" -name '*.sh' -type f | sort) + if [ "${#files[@]}" -eq 0 ]; then + echo "::notice::No shell scripts under '$dir'; nothing was checked." + exit 0 + fi + "$SHELLCHECK" "${files[@]}" || { + echo "::error::[lint] shellcheck found problems." + failed=1 + } + "$SHFMT" -d "${files[@]}" || { + echo "::error::[lint] shfmt would change a script; run shfmt -w." + failed=1 + } + [ "$failed" -eq 0 ] && echo "Checked ${#files[@]} script(s): clean." + exit "$failed" +} + +main "$@"