From 380f6a949c714ae1920a3413c9647f5622e2ebd1 Mon Sep 17 00:00:00 2001 From: Lucas Koontz Date: Sat, 3 Oct 2026 21:25:41 -0700 Subject: [PATCH] feat(actions): add a hosted build mode, PR-env inputs and commit pins build-push-ecr gains a `builder` input. The default, `remote`, runs the same create-repository, set-repository-policy, buildx create and build calls as before. `local` builds on the docker-container builder that setup-buildx-action starts on the runner. It caches layers in the GitHub Actions cache under module-name, and it never creates an ECR repository or changes a repository policy. So a caller job can push with a role it assumes through GitHub OIDC, and that role needs push rights only. In local mode, crazy-max/ghaction-github-runtime first exports the cache service's ACTIONS_* variables, which buildx needs and run steps do not get. The BuildKit container runs moby/buildkit v0.33.1, pinned to its multi-arch index digest, because it holds the ECR registry credentials while it pushes. argocd-pr-env-deploy gains `repository`, `pr-number` and `head-sha` inputs. They default to the pull_request event's values, so today's callers pass nothing new. A scheduled or dispatched workflow passes them to deploy a named PR. Before any API, ECR or ArgoCD call, the action refuses a value that is not owner/name, a number or a 40-character SHA, and it never prints the refused value. Outside a pull_request run it deploys the PR head's development-head- tag, because that run built no image of its own. The action's ECR existence check now uses one repository per repo. When mindsdb--dev exists, it checks PR image tags there and only there. Otherwise it checks mindsdb-. It asks describe-images whether the -dev repository exists, so ecr:DescribeImages stays the only ECR permission the action needs. Every third-party action in build-push-ecr, snyk-docker-scan and setup-env is pinned to the commit its tag points at today, with the version in a comment. snyk-docker-scan also pins the Snyk CLI to v1.1307.4, the current release. stale-deploy-label's prune job runs on ubuntu-latest instead of mdb-dev. The README documents the local builder, the new inputs, the one-repository check and how to bump the BuildKit pin. Its secret-source table describes what an environment's deployment branch policy can do. New pytest suites run the build and deploy step scripts under bash with aws, docker, curl and argocd stubbed, and assert on the argv of each aws, docker and argocd call. Another suite fails if a pin loses its commit, digest or version. Part of Lucas Koontz's ticket "Take the public repos off the credentialed runner group and require devops to release a privileged run". Refs: ENG-2000 --- .github/workflows/stale-deploy-label.yml | 10 +- README.md | 59 +++- argocd-pr-env-deploy/action.yml | 161 ++++++--- build-push-ecr/action.yml | 76 ++++- setup-env/action.yml | 2 +- snyk-docker-scan/action.yml | 13 +- tests/conftest.py | 72 ++++ tests/test_argocd_pr_env_deploy.py | 405 +++++++++++++++++++++++ tests/test_build_push_ecr.py | 162 +++++++++ tests/test_third_party_pins.py | 76 +++++ 10 files changed, 974 insertions(+), 62 deletions(-) create mode 100644 tests/conftest.py create mode 100644 tests/test_argocd_pr_env_deploy.py create mode 100644 tests/test_build_push_ecr.py create mode 100644 tests/test_third_party_pins.py diff --git a/.github/workflows/stale-deploy-label.yml b/.github/workflows/stale-deploy-label.yml index 425a711..12b3806 100644 --- a/.github/workflows/stale-deploy-label.yml +++ b/.github/workflows/stale-deploy-label.yml @@ -24,7 +24,9 @@ on: jobs: prune: - runs-on: mdb-dev + # GitHub-hosted: the job needs only `gh` and the caller's token, and one + # caller is a public repository, which must not reach the self-hosted runners. + runs-on: ubuntu-latest env: GH_TOKEN: ${{ github.token }} TARGET_REPO: ${{ inputs.target-repo }} @@ -34,9 +36,9 @@ jobs: run: | set -euo pipefail - # Cross-platform `date` cutoff. Self-hosted mdb-dev runners are Linux — - # `date -u -d` is GNU coreutils. If this workflow ever moves to macOS - # runners, swap to `date -u -v-${STALE_DAYS}d -Iseconds`. + # Cross-platform `date` cutoff. ubuntu-latest is Linux, so `date -u -d` + # is GNU coreutils. If this workflow ever moves to macOS runners, swap + # to `date -u -v-${STALE_DAYS}d -Iseconds`. THRESHOLD_ISO="$(date -u -d "${STALE_DAYS} days ago" +%Y-%m-%dT%H:%M:%SZ)" echo "Threshold: ${THRESHOLD_ISO} (PRs older than this lose the label)" diff --git a/README.md b/README.md index cea57bb..a065f62 100644 --- a/README.md +++ b/README.md @@ -407,15 +407,56 @@ There is no blanket answer, and "prefer Kubernetes because it is the source of t | The value is | Source | Why | | --- | --- | --- | | Ephemeral and namespace-local (a per-PR environment's own credentials) | **Kubernetes**, via `k8s-secret` | No GitHub Environment can exist for `pr--204`, so a copy is impossible; the namespace is the only source there is | -| A permanent environment's real credential (prod/staging DB, Stripe live, prod vendor tokens) | **GitHub Environment** | A job can only read it by declaring `environment: prod`, and that environment requires a reviewer. A k8s Secret has no such gate — *any* job on a runner with cluster read can fetch it, unreviewed | +| A permanent environment's real credential (prod/staging DB, Stripe live, prod vendor tokens) | **GitHub Environment** | A job can only read it by declaring `environment: prod`, and the environment's deployment branch policy can limit which branches may declare it. A k8s Secret has no such gate: *any* job on a runner with cluster read can fetch it | | Needed to reach the cluster, or used on a GitHub-hosted runner | **GitHub secret** | Package-install tokens, the ArgoCD token that creates the namespace, Snyk, Slack. Reading these from the cluster is circular | -The middle row is the one worth being explicit about, because the intuition points the wrong way. Moving prod credentials out of a GitHub Environment and into a cluster read would *remove* the required-reviewer gate standing in front of them today, and it would force the jobs that use them onto `mdb-prod` (nothing else can reach newprod), which means granting *more* jobs prod cluster access. Both are the wrong direction. +The middle row is the one worth being explicit about, because the intuition points the wrong way. Moving prod credentials out of a GitHub Environment and into a cluster read would *remove* the gate an environment can put in front of them. A job reads an environment secret only by naming the environment. The environment's deployment branch policy can limit which branches may name it, and without one any branch can. A cluster read has no such gate. The move would also force the jobs that use them onto `mdb-prod` (nothing else can reach newprod), which means granting *more* jobs prod cluster access. Both are the wrong direction. What actually goes wrong with a GitHub secret is different and has a cheaper fix: a reference to a secret that does not exist resolves to the **empty string** rather than failing, so a rename reaches the vendor as no credential and comes back as an unexplained 401. The fix for that is to assert the value is non-empty and name it in the error, not to migrate its storage. `k8s-secret` is also not a containment boundary. The ability to read Secrets belongs to the self-hosted runner, which holds cluster credentials because it deploys. What bounds *that* is who can trigger a run on such a runner — hence a `pull_request`-triggered job on a public repo needs `if: github.event.pull_request.head.repo.full_name == github.repository` — and the runner ServiceAccount's RBAC. +## Building an image into ECR + +`build-push-ecr` builds a Docker image and pushes it to ECR as `-`, `` and `latest`. On a `pull_request` event it also pushes `-head-`. The `image` output names the `-` tag. + +The `builder` input decides where the build runs. Tags, build arguments and the output are the same in both modes. + +| `builder` | Runs on | AWS credentials | ECR repository | +| --- | --- | --- | --- | +| `remote` (default) | `mdb-dev` only, because it builds on the in-cluster BuildKit service | The runner pod's own role | The action creates it, with `shared-ecr-policy.json`, the first time it pushes there | +| `local` | GitHub-hosted runners, with layers cached in the GitHub Actions cache under `module-name` | A role the caller job assumes through GitHub OIDC | Must exist already. The action never creates a repository or changes its policy | + +Use `local` in a public repository. GitHub [recommends GitHub-hosted runners for public repositories](https://docs.github.com/en/actions/reference/security/secure-use#hardening-for-self-hosted-runners), because anyone can open a pull request there. Give the role it assumes push access to that repository's ECR repositories and nothing else. The caller job grants `id-token: write` and configures the credentials before the build: + +```yaml + build: + runs-on: ubuntu-latest + permissions: + contents: read + id-token: write # lets configure-aws-credentials request an OIDC token + steps: + - uses: actions/checkout@ # v5 + - uses: aws-actions/configure-aws-credentials@ # v6 + with: + role-to-assume: arn:aws:iam:::role/ + aws-region: us-east-1 + - id: build + uses: mindsdb/github-actions/build-push-ecr@main + with: + builder: local + module-name: + build-for-environment: development +``` + +`docker buildx` finds the GitHub Actions cache through `ACTIONS_*` variables that the runner gives to actions but not to `run:` steps. In `local` mode the action runs `crazy-max/ghaction-github-runtime`, which exports them to the job's environment, so later steps in the caller job can read them too. + +`snyk-docker-scan` logs in to ECR with whatever credentials the job has. A scan job on a GitHub-hosted runner therefore runs `configure-aws-credentials` first, with a role that can pull the image. + +Every third-party action inside `build-push-ecr`, `snyk-docker-scan` and `setup-env` is pinned to a full commit SHA, with its version in a comment. `setup-env` is on that list because it runs in the same build job, and any step in a job that grants `id-token: write` can request the job's OIDC token. `snyk-docker-scan` also pins the Snyk CLI release it installs, because the CLI runs with the job's AWS credentials in its environment. In `local` mode, the BuildKit container that builds and pushes the image runs one `moby/buildkit` release, pinned to its multi-arch index digest, because that container holds the ECR registry credentials while it pushes. So a moved tag or a new upstream release cannot change that code until someone bumps a pin here. + +The BuildKit pin needs manual bumps, because nothing in this repository updates it. To bump it, take the newest plain `vX.Y.Z` tag from [moby/buildkit releases](https://github.com/moby/buildkit/releases). Release candidates and `dockerfile/` releases do not count. Read that version's index digest from the `Digest:` line of `docker buildx imagetools inspect moby/buildkit:`. Then change the version and the digest together in the `driver-opts` line of `build-push-ecr/action.yml`. `tests/test_third_party_pins.py` fails if the pin loses its version or its digest. + ## PR environment rollout `argocd-pr-env-deploy` applies a PR's image tags to its ArgoCD environment and @@ -450,6 +491,20 @@ instead of both being set by hand. The `ci-pr-envs` token is a JWT the server validates itself, so it works unchanged over either endpoint. +In a `pull_request` run the action takes the repository, PR number and head SHA from the event. A workflow that deploys PRs from any other event, such as a schedule that syncs every open PR labelled `deploy`, passes them as inputs. The action accepts only an owner/name, a number and a 40-character SHA. That run built no image, so it deploys `development-head-`, the tag `build-push-ecr` pushes for every PR head. + +```yaml + - uses: mindsdb/github-actions/argocd-pr-env-deploy@main + with: + argocd-token: ${{ secrets.ARGOCD_AUTH_TOKEN }} + gh-token: ${{ secrets.GH_PULL_REQUEST_READ_TOKEN }} + repository: ${{ matrix.repository }} # owner/name + pr-number: ${{ matrix.pr-number }} + head-sha: ${{ matrix.head-sha }} +``` + +Before it pins a tag, the action checks that the tag exists in the ECR repository that holds that repo's PR images. A repo with a dev tier pushes its PR images to `mindsdb--dev` and nowhere else. So when that repository exists, the action checks there and only there. Otherwise it checks `mindsdb-`. A tag missing from one repository is never looked up in the other. Underscores in the repo name become hyphens. The action also learns whether `mindsdb--dev` exists from `ecr:DescribeImages`, so that is the only ECR permission it needs. + ## PR environment comments `pr-env-comment.yml` posts and keeps updating one comment saying where a PR's environment is and how to sign in. The account, Secret name, namespace, and hosts arrive as **inputs** — this repo is public, and while none of those is a credential, together they are a free recon package. The mechanism is shared; the facts stay in the private caller. A reusable that cannot be described without naming our infrastructure has not earned promotion here. diff --git a/argocd-pr-env-deploy/action.yml b/argocd-pr-env-deploy/action.yml index f978ed5..bec3a70 100644 --- a/argocd-pr-env-deploy/action.yml +++ b/argocd-pr-env-deploy/action.yml @@ -2,7 +2,8 @@ # # Resolves: # - This PR (anchor) — own merge SHA (image built in this same run), repo -# short name from $GITHUB_REPOSITORY. +# short name from $GITHUB_REPOSITORY. A caller outside a pull_request run +# names the PR with inputs instead (see PR IDENTITY below). # - Any `Deploys: repo#N` or `Deploys: org/repo#N` line in the anchor PR # body (owner defaults to this repo's org) — looks up each linked PR's # head + merge SHAs via the GitHub REST API. @@ -35,11 +36,16 @@ # linked repo's pipeline), instead of pinning a phantom tag and leaving # the env in ImagePullBackOff silently. # Every pinned tag — the anchor's own included — is existence-checked -# against ECR (`aws ecr describe-images`; GitHub short name maps to the -# `mindsdb-` ECR repo). Only Image/RepositoryNotFound -# count as absent; if the aws CLI is missing or errors for infra reasons -# (auth, throttle, network) the checks are skipped with one warning rather -# than blocking a deploy that would have worked. +# against ECR (`aws ecr describe-images`), in the one ECR repository that holds +# the repo's PR images. A repo with a dev tier pushes its PR images to +# `mindsdb--dev` and nowhere else, so when that repository +# exists its tags are checked there alone. Otherwise they are checked in +# `mindsdb-`. Whether the dev-tier repository exists +# decides it, never whether it holds a given tag, so a tag missing from one +# repository is never looked up in the other. Only Image/RepositoryNotFound +# count as absent; if the aws CLI is missing or errors for infra reasons (auth, +# throttle, network) the checks are skipped with one warning rather than +# blocking a deploy that would have worked. # # `Deploys:`-looking body lines that don't match the accepted forms (e.g. a # pasted PR URL) emit a ::warning:: instead of being silently ignored. @@ -56,8 +62,13 @@ # block (native GH Deployment). The PR sidebar pill is `in_progress` while # the job runs and flips to `success`/`failure` on the job's exit code. # -# Must run from a `pull_request` event; PR number and head SHA are read from -# $GITHUB_EVENT_PATH so callers don't have to pass them. +# PR IDENTITY. A `pull_request` run takes the repository, PR number and head +# SHA from the event, so its callers pass nothing. Any other caller, such as a +# scheduled sync of open PRs, passes `repository`, `pr-number` and `head-sha`. +# The action refuses values that are not owner/name, a number and a +# 40-character SHA. Such a run built no image, so the anchor's tag is +# `development-head-`, which build-push-ecr pushes for every PR head, +# instead of `development-`. # # ENDPOINT. The action talks to the in-cluster Service # (`argo-cd-argocd-server.argocd.svc.cluster.local`) by default, not to @@ -129,6 +140,24 @@ inputs: know this repo's envs come up faster. required: false default: "1800" + repository: + description: >- + owner/name of the PR's repository. Defaults to the repository the workflow + runs in. A caller outside a `pull_request` event passes this, `pr-number` + and `head-sha`. + required: false + default: ${{ github.repository }} + pr-number: + description: The PR to deploy. Defaults to the `pull_request` event's PR. + required: false + default: ${{ github.event.pull_request.number }} + head-sha: + description: >- + The PR's head commit, used as the chart revision. Defaults to the + `pull_request` event's head SHA. Outside that event it also picks the + PR's image, `development-head-`. + required: false + default: ${{ github.event.pull_request.head.sha }} runs: using: composite @@ -142,6 +171,9 @@ runs: GH_TOKEN: ${{ inputs.gh-token }} STACKS_DISPATCH_TOKEN: ${{ inputs.stacks-dispatch-token || inputs.gh-token }} WAIT_TIMEOUT: ${{ inputs.wait-timeout }} + PR_REPOSITORY: ${{ inputs.repository }} + PR_NUMBER: ${{ inputs.pr-number }} + PR_HEAD_SHA: ${{ inputs.head-sha }} run: | set -euo pipefail @@ -150,7 +182,27 @@ runs: : "${ARGOCD_AUTH_TOKEN:?ARGOCD_AUTH_TOKEN is empty — check the org/repo secret is set and visible to this repo}" : "${GH_TOKEN:?GH_TOKEN is empty — pass GH_PRIVATE_ACCESS_TOKEN via the gh-token input}" - OWN_REPO="$GITHUB_REPOSITORY" + # The repository, PR number and head SHA end up in GitHub API paths, + # argocd arguments and Application names. Outside a pull_request run + # the caller supplies them, so accept only the shapes GitHub produces. + # The values are not echoed: a caller may have copied them from a PR. + # An owner is letters, digits and hyphens. A repository name may also + # hold `_` and `.`, but `.` and `..` are path segments, not names. + if ! [[ "$PR_REPOSITORY" =~ ^[A-Za-z0-9-]+/[A-Za-z0-9_.-]+$ ]] \ + || [ "${PR_REPOSITORY#*/}" = . ] || [ "${PR_REPOSITORY#*/}" = .. ]; then + echo "::error::repository must be owner/name." + exit 1 + fi + if ! [[ "$PR_NUMBER" =~ ^[0-9]+$ ]]; then + echo "::error::pr-number must be a PR number. Outside a pull_request event, pass repository, pr-number and head-sha." + exit 1 + fi + if ! [[ "$PR_HEAD_SHA" =~ ^[0-9a-f]{40}$ ]]; then + echo "::error::head-sha must be a full 40-character commit SHA." + exit 1 + fi + + OWN_REPO="$PR_REPOSITORY" OWN_SHORT="${OWN_REPO##*/}" # Slugified form for K8s resource names + labels. RFC 1123 forbids # underscores in Application names / Namespaces / hostnames, but @@ -158,21 +210,21 @@ runs: # underscored everywhere it identifies the repo (helm parameter # keys, the `Deploys:` regex, ECR image tags). OWN_SLUG="${OWN_SHORT//_/-}" - PR_NUMBER="$(jq -r .pull_request.number "$GITHUB_EVENT_PATH")" # Two SHAs, used for two different things: # $GITHUB_SHA = merge SHA (PR head merged into base, a # synthetic commit only reachable via # refs/pull/N/merge). build-push-ecr tags # images as `development-${GITHUB_SHA}`, - # so the image-tag override uses this. - # .pull_request.head.sha = PR branch tip — the commit ArgoCD's + # so a pull_request run's image-tag + # override uses this. + # head-sha = PR branch tip, the commit ArgoCD's # repo-server can actually fetch as a # `targetRevision`. Used for the chart # revision so chart changes on the PR # branch deploy with the PR. OWN_MERGE_SHA="$GITHUB_SHA" - OWN_HEAD_SHA="$(jq -r .pull_request.head.sha "$GITHUB_EVENT_PATH")" + OWN_HEAD_SHA="$PR_HEAD_SHA" PARENT_APP="pr-${OWN_SLUG}-${PR_NUMBER}" # GitHub REST helper — auth + sensible headers in one place. No `gh` @@ -183,26 +235,17 @@ runs: "https://api.github.com$1" } - # ECR image-existence guard. Maps a GitHub repo short name to its - # ECR repository (`mindsdb-` + short name, underscores to hyphens) - # and asks ECR whether exists. Returns 0 = present, 1 = absent. - # Only ImageNotFoundException / RepositoryNotFoundException count as - # absent; any other failure (no aws CLI, auth, throttle, network) - # must never brick a deploy that would have worked — warn ONCE, - # remember ECR is unusable, and report every tag as present from - # then on. + # ECR image-existence guard, in two parts. + # + # ecr_describe runs `aws ecr describe-images` with its arguments and + # returns 0 = found, 1 = not found. Only ImageNotFoundException / + # RepositoryNotFoundException count as not found. Any other failure + # (auth, throttle, network) must never brick a deploy that would have + # worked, so it warns ONCE, remembers ECR is unusable, and returns 0. ECR_CHECKS=on - ecr_tag_exists() { - local ecr_repo="mindsdb-${1//_/-}" tag="$2" err - [ "$ECR_CHECKS" = on ] || return 0 - if ! command -v aws >/dev/null 2>&1; then - ECR_CHECKS=off - echo "::warning::aws CLI not found on this runner — ECR image-existence checks skipped, tags assumed present." - return 0 - fi - if err="$(aws ecr describe-images \ - --repository-name "$ecr_repo" \ - --image-ids "imageTag=${tag}" \ + ecr_describe() { + local err + if err="$(aws ecr describe-images "$@" \ --region "${AWS_REGION:-us-east-1}" 2>&1 >/dev/null)"; then return 0 fi @@ -210,12 +253,36 @@ runs: *ImageNotFoundException*|*RepositoryNotFoundException*) return 1 ;; - *) - ECR_CHECKS=off - echo "::warning::ECR describe-images failed (${err//$'\n'/ }) — image-existence checks skipped, tags assumed present." - return 0 - ;; esac + ECR_CHECKS=off + echo "::warning::ECR describe-images failed (${err//$'\n'/ }) — image-existence checks skipped, tags assumed present." + return 0 + } + + # ecr_tag_exists returns 0 = present, + # 1 = absent, and every tag counts as present once ECR is unusable. + # It first picks the repository that holds the short name's PR images + # (`mindsdb-` + short name, underscores to hyphens, plus `-dev` when + # that repository exists; see the header), then checks the tag there + # only. Asking describe-images about the repository, with no tag, + # keeps the action on the one ECR permission it already needs. The + # repository it checked stays in PR_IMAGE_REPO for error messages. + PR_IMAGE_REPO="" + ecr_tag_exists() { + local image="mindsdb-${1//_/-}" tag="$2" + [ "$ECR_CHECKS" = on ] || return 0 + if ! command -v aws >/dev/null 2>&1; then + ECR_CHECKS=off + echo "::warning::aws CLI not found on this runner — ECR image-existence checks skipped, tags assumed present." + return 0 + fi + if ecr_describe --repository-name "${image}-dev" --max-items 1; then + PR_IMAGE_REPO="${image}-dev" + else + PR_IMAGE_REPO="$image" + fi + [ "$ECR_CHECKS" = on ] || return 0 + ecr_describe --repository-name "$PR_IMAGE_REPO" --image-ids "imageTag=${tag}" } # Fetch the PR body ONCE — the strict `Deploys:` extraction and the @@ -250,10 +317,6 @@ runs: # (missing/closed/unmergeable PR) fails the job and comments on the # anchor PR rather than silently deploying from the default branch. # - # The anchor's own image is tagged from the merge SHA by the build - # job in this same run — no cross-PR race — but it still gets the - # existence check so a failed/skipped build fails HERE, not as an - # ImagePullBackOff in the env. # Repos that publish no image. Their deploy is the git revision alone: # mindshub_services' pr-env-stacks workflow builds a SAM stack from # revisions.mindshub_services. Applies to the anchor itself and to @@ -268,9 +331,19 @@ runs: if is_revision_only_repo "$OWN_SHORT"; then set_args=(--helm-set "revisions.${OWN_SHORT}=${OWN_HEAD_SHA}") else - OWN_TAG="development-${OWN_MERGE_SHA}" + # A pull_request run built this image earlier in the same run and + # tagged it with the merge SHA, so no other PR's build can race it. + # Any other run has no build of its own, so it deploys the tag + # build-push-ecr pushed for the PR head. Either tag still gets the + # existence check, so a failed or skipped build fails here, not as + # an ImagePullBackOff in the env. + if [ "$GITHUB_EVENT_NAME" = pull_request ]; then + OWN_TAG="development-${OWN_MERGE_SHA}" + else + OWN_TAG="development-head-${OWN_HEAD_SHA}" + fi if ! ecr_tag_exists "$OWN_SHORT" "$OWN_TAG"; then - echo "::error::ECR image mindsdb-${OWN_SLUG}:${OWN_TAG} not found — did the build job in this run succeed?" + echo "::error::ECR image ${PR_IMAGE_REPO}:${OWN_TAG} not found. Check that the PR's build job succeeded and pushed it." exit 1 fi set_args=( @@ -356,7 +429,7 @@ runs: if [ "$ECR_CHECKS" = on ] && ecr_tag_exists "$repo" "$head_tag" && [ "$ECR_CHECKS" = on ]; then tag="$head_tag" elif [ "$ECR_CHECKS" = on ] && ! ecr_tag_exists "$repo" "$merge_tag"; then - echo "::error::No ECR image for linked PR ${link}: neither ${head_tag} nor ${merge_tag} exists in mindsdb-${repo//_/-}. The linked PR's base moved after its last build regenerated the merge SHA; re-run the linked repo's pipeline to build a fresh image, then re-run this job." + echo "::error::No ECR image for linked PR ${link}: neither ${head_tag} nor ${merge_tag} exists in ${PR_IMAGE_REPO}. The linked PR's base moved after its last build regenerated the merge SHA; re-run the linked repo's pipeline to build a fresh image, then re-run this job." exit 1 fi set_args+=( diff --git a/build-push-ecr/action.yml b/build-push-ecr/action.yml index e02232f..0416f0b 100644 --- a/build-push-ecr/action.yml +++ b/build-push-ecr/action.yml @@ -6,6 +6,21 @@ # new hash, identical content — whenever the PR's base or head moves, so # it's not a stable handle. The head SHA is immutable, making the extra # tag the one deploy-time consumers can pin race-free. +# +# The `builder` input picks where the image is built. Tags, build args and the +# `image` output are the same either way. +# remote (default) The in-cluster BuildKit service. Only an `mdb-dev` runner +# pod can reach it, and that pod's own AWS identity pushes. +# The action creates the ECR repository and sets +# shared-ecr-policy.json on it the first time. +# local The docker-container builder that setup-buildx-action +# starts on the runner, on a pinned BuildKit image, with the +# GitHub Actions cache scoped to `module-name`. For +# GitHub-hosted runners. It never creates a repository or +# touches its policy, so the repository must already exist. +# The caller job grants `id-token: write` and runs +# aws-actions/configure-aws-credentials with `role-to-assume` +# before this action. outputs: image: @@ -28,20 +43,46 @@ inputs: image-ref: description: "The version number or sha used in creating image tag" required: false + builder: + description: >- + Where to build: `remote` (default) uses the in-cluster BuildKit service + and only works on an `mdb-dev` runner. `local` builds on the runner itself + with the GitHub Actions cache, for GitHub-hosted runners, and needs AWS + credentials configured earlier in the job. + required: false + default: remote runs: using: 'composite' steps: - - uses: FranzDiebold/github-env-vars-action@v2 + - uses: FranzDiebold/github-env-vars-action@e8118971bd6b60524ae66a2d870bc83088e3a3c9 # v2.9.0 # https://github.com/aws-actions/amazon-ecr-login - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v4 + uses: docker/setup-buildx-action@f87e5991a6d7451dcb8d9637bfbc97413f497069 # v4.4.1 + with: + # In `local` mode the BuildKit container this step starts builds the + # image, and it holds the ECR registry credentials while it pushes. So + # it runs one moby/buildkit release, pinned to its multi-arch index + # digest, instead of buildx's moving `buildx-stable-1` default. + # Nothing bumps this pin: change the version and the digest together + # by hand (see the README). `remote` builds on the in-cluster service, + # so it keeps the default. + driver-opts: ${{ inputs.builder == 'local' && 'image=moby/buildkit:v0.33.1@sha256:cec9f139f45e93c5c69c60f8b07cfad9f43f4ef6b6a6cd917527fea5ff2e3dea' || '' }} - name: Login to Amazon ECR id: login-ecr - uses: aws-actions/amazon-ecr-login@v2 + uses: aws-actions/amazon-ecr-login@03f1aad4c6c7ffd436567f42f9384779290529bd # v2.1.7 + # `docker buildx` reads the cache service's URL and token from ACTIONS_* + # variables, which the runner gives to actions but not to `run:` steps. + # This step exports them for the rest of the job, so the build step below + # can use `type=gha`. + - name: Expose the GitHub Actions cache to docker buildx + if: inputs.builder == 'local' + uses: crazy-max/ghaction-github-runtime@04d248b84655b509d8c44dc1d6f990c879747487 # v4.0.0 - shell: bash id: build + env: + BUILDER: ${{ inputs.builder }} run: | # Env var parsing @@ -65,17 +106,34 @@ runs: HEAD_TAG="-t $REPO_IMAGE:$ENVIRONMENT-head-$PR_HEAD_SHA" fi - # Create repo if needed - aws ecr create-repository --repository-name $IMAGE_NAME && \ - aws ecr set-repository-policy --repository-name $IMAGE_NAME --policy-text "$(cat ${{ github.action_path }}/shared-ecr-policy.json)" || \ - true # Just let this fail if the repo already exists + # CACHE_ARGS is expanded unquoted on the build line, like HEAD_TAG, so + # the remote builder's empty value contributes zero args. + CACHE_ARGS="" + case "$BUILDER" in + remote) + # Create repo if needed + aws ecr create-repository --repository-name $IMAGE_NAME && \ + aws ecr set-repository-policy --repository-name $IMAGE_NAME --policy-text "$(cat ${{ github.action_path }}/shared-ecr-policy.json)" || \ + true # Just let this fail if the repo already exists - docker buildx create --name=remote-buildkit-agent --driver=remote --use tcp://remote-buildkit-agent.infrastructure.svc.cluster.local:80 || true # Create the builder (might already exist) + docker buildx create --name=remote-buildkit-agent --driver=remote --use tcp://remote-buildkit-agent.infrastructure.svc.cluster.local:80 || true # Create the builder (might already exist) + ;; + local) + # setup-buildx-action's docker-container builder is already the + # current one. The repository and its policy must exist already, + # because a role scoped to pushing can create or change neither. + CACHE_ARGS="--cache-from type=gha,scope=$IMAGE_NAME --cache-to type=gha,scope=$IMAGE_NAME,mode=max" + ;; + *) + echo "::error::builder must be 'remote' or 'local', got '$BUILDER'." + exit 1 + ;; + esac cd $SRC_PATH BUILD_ARGS="--build-arg BUILD_FOR_ENVIRONMENT=$ENVIRONMENT --build-arg IMAGE_TAG=$IMAGE_TAG" # Finally, build our runner container - docker buildx build ${{ inputs.extra-build-args }} $BUILD_ARGS $HEAD_TAG -t $REPO_IMAGE:$IMAGE_TAG -t $REPO_IMAGE:$ENVIRONMENT -t $REPO_IMAGE:latest --push . + docker buildx build ${{ inputs.extra-build-args }} $BUILD_ARGS $CACHE_ARGS $HEAD_TAG -t $REPO_IMAGE:$IMAGE_TAG -t $REPO_IMAGE:$ENVIRONMENT -t $REPO_IMAGE:latest --push . echo "image=$REPO_IMAGE:$IMAGE_TAG" >> $GITHUB_OUTPUT diff --git a/setup-env/action.yml b/setup-env/action.yml index a7fbe4f..c23ecce 100644 --- a/setup-env/action.yml +++ b/setup-env/action.yml @@ -4,7 +4,7 @@ runs: using: "composite" steps: # Get clean environment variables via https://github.com/marketplace/actions/github-environment-variables-action - - uses: FranzDiebold/github-env-vars-action@v2 + - uses: FranzDiebold/github-env-vars-action@e8118971bd6b60524ae66a2d870bc83088e3a3c9 # v2.9.0 - shell: bash run: | echo "REF_SLUG=${CI_REF_NAME_SLUG:-$CI_HEAD_REF_SLUG}" >> $GITHUB_ENV # Use whichever env ref is supplied (push or merge). diff --git a/snyk-docker-scan/action.yml b/snyk-docker-scan/action.yml index af2f07a..b47ecb3 100644 --- a/snyk-docker-scan/action.yml +++ b/snyk-docker-scan/action.yml @@ -7,6 +7,10 @@ # # If input allow-vulns is set (any non-empty value), the scan may exit 0 even when Snyk reports # issues (`|| true`). Callers typically pass `allow-vulns: ${{ vars.ALLOW_SNYK_VULNS }}`. +# +# AWS: the ECR login uses whatever credentials the job already has. On an `mdb-dev` runner that is +# the pod's own identity. On a GitHub-hosted runner, run aws-actions/configure-aws-credentials with a +# role that can pull the image before this action. inputs: image: @@ -36,9 +40,14 @@ runs: using: "composite" steps: - name: Install Snyk CLI - uses: snyk/actions/setup@master + uses: snyk/actions/setup@9adf32b1121593767fc3c057af55b55db032dc04 # v1.0.0 + with: + # Without this, setup installs whatever release Snyk last published. + # The CLI runs with the job's AWS credentials in its environment, so it + # changes only when this line does. + snyk-version: v1.1307.4 - name: Login to Amazon ECR - uses: aws-actions/amazon-ecr-login@v2 + uses: aws-actions/amazon-ecr-login@03f1aad4c6c7ffd436567f42f9384779290529bd # v2.1.7 - name: Run Snyk container test shell: bash env: diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..27875bf --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,72 @@ +"""Run a composite action's shell step for real, with its external commands faked. + +YAML assertions can say what a step's script contains, not what it executes. The +tests that need the second run the script under bash with `aws`, `docker`, +`curl` and `argocd` replaced by stubs that record every call, then assert on the +recorded argv. The stubs are written per test, so nothing reaches a real cloud, +cluster or API. +""" + +from __future__ import annotations + +import os +import shutil +import subprocess +from pathlib import Path + +import pytest + +# Field and record separators for the call log. Arguments can hold spaces and +# newlines (a JSON policy document does), so neither can delimit them. +FIELD = "\x1f" +RECORD = "\x1e" + + +class Stubs: + """Fake commands on PATH. Each one appends its argv to a log, then runs its body.""" + + def __init__(self, root: Path) -> None: + self.bin = root / "bin" + self.bin.mkdir() + self.log = root / "calls.log" + self.log.touch() + + def add(self, name: str, body: str = "exit 0") -> None: + script = self.bin / name + script.write_text( + "#!/usr/bin/env bash\n" + f"{{ printf '%s\\037' {name} \"$@\"; printf '\\036'; }} >> \"$STUB_LOG\"\n" + f"{body}\n", + encoding="utf-8", + ) + script.chmod(0o755) + + def calls(self, name: str | None = None) -> list[list[str]]: + records = self.log.read_text(encoding="utf-8").split(RECORD) + argvs = [record.split(FIELD)[:-1] for record in records if record] + return [argv for argv in argvs if name is None or argv[0] == name] + + def run(self, script: str, env: dict[str, str], cwd: Path) -> subprocess.CompletedProcess[str]: + """Run `script` the way a composite `shell: bash` step runs it.""" + path = cwd / "step.sh" + path.write_text(script, encoding="utf-8") + bash = shutil.which("bash") + assert bash, "bash is required to run composite action steps" + return subprocess.run( + [bash, "--noprofile", "--norc", "-eo", "pipefail", str(path)], + env={ + "PATH": f"{self.bin}{os.pathsep}{os.environ['PATH']}", + "HOME": str(cwd), + "STUB_LOG": str(self.log), + **env, + }, + cwd=cwd, + capture_output=True, + text=True, + check=False, + ) + + +@pytest.fixture +def stubs(tmp_path: Path) -> Stubs: + return Stubs(tmp_path) diff --git a/tests/test_argocd_pr_env_deploy.py b/tests/test_argocd_pr_env_deploy.py new file mode 100644 index 0000000..8e3262f --- /dev/null +++ b/tests/test_argocd_pr_env_deploy.py @@ -0,0 +1,405 @@ +"""Behaviour tests for `argocd-pr-env-deploy/action.yml`. + +The action used to work only inside a `pull_request` run, because it read the PR +from the event. A scheduled sync of open PRs now passes `repository`, +`pr-number` and `head-sha` instead. These tests run the real script with `curl`, +`aws` and `argocd` faked and pin four things: a `pull_request` run deploys +exactly what it did before, a run with inputs deploys the PR it was told to, +each image tag is checked in the one ECR repository that holds that repo's PR +images, and input that is not a repo name, a number or a SHA stops the script +before any API, ECR or ArgoCD call. +""" + +from __future__ import annotations + +import json +import subprocess +from pathlib import Path + +import pytest +import yaml + +ACTION = yaml.safe_load( + (Path(__file__).resolve().parents[1] / "argocd-pr-env-deploy" / "action.yml").read_text(encoding="utf-8") +) +STEP = ACTION["runs"]["steps"][0] + +_BASH_MAJOR = int( + subprocess.run(["bash", "-c", "echo ${BASH_VERSINFO[0]}"], capture_output=True, text=True, check=False).stdout + or 0 +) +runs_the_script = pytest.mark.skipif( + _BASH_MAJOR < 4, + reason="bash 3.2 (macOS) cannot parse the script's `case` inside a process substitution; GitHub runners run bash 5", +) + +MERGE_SHA = "a" * 40 +HEAD_SHA = "b" * 40 +LINKED_HEAD_SHA = "c" * 40 +LINKED_MERGE_SHA = "d" * 40 +AWS_REGION = "us-east-1" + +CURL = """ +url=""; post=0 +for arg in "$@"; do + case "$arg" in + https://*) url="$arg" ;; + POST) post=1 ;; + esac +done +if [ "$post" = 1 ]; then cat >/dev/null; exit 0; fi +fixture="$STUB_FIXTURES/github/${url#https://api.github.com/}.json" +[ -f "$fixture" ] || exit 22 +cat "$fixture" +""" + +# `aws ecr describe-images` over three fixture files: the repositories that +# exist, their `:` images, and repositories that answer +# AccessDenied. Without --image-ids the call lists a repository's images, so it +# succeeds exactly when the repository exists. +AWS = """ +repo=""; tag="" +while [ "$#" -gt 0 ]; do + case "$1" in + --repository-name) repo="$2"; shift ;; + --image-ids) tag="${2#imageTag=}"; shift ;; + esac + shift +done +if grep -qxF "$repo" "$STUB_FIXTURES/ecr-denied"; then + echo "An error occurred (AccessDeniedException) when calling the DescribeImages operation" >&2 + exit 254 +fi +if ! grep -qxF "$repo" "$STUB_FIXTURES/ecr-repos"; then + echo "An error occurred (RepositoryNotFoundException) when calling the DescribeImages operation" >&2 + exit 254 +fi +[ -z "$tag" ] && exit 0 +grep -qxF "$repo:$tag" "$STUB_FIXTURES/ecr-images" && exit 0 +echo "An error occurred (ImageNotFoundException) when calling the DescribeImages operation" >&2 +exit 254 +""" + +ARGOCD = """ +if [ "$1 $2" = "app terminate-op" ]; then + echo "Unable to terminate operation. No operation is in progress" >&2 + exit 1 +fi +""" + + +def run_deploy( + stubs, + tmp_path: Path, + *, + event: str, + repository: str, + pr_number: str, + head_sha: str, + prs: dict[str, dict] | None = None, + ecr_repos: tuple[str, ...] = (), + ecr_images: tuple[str, ...] = (), + ecr_denied: tuple[str, ...] = (), +): + """Run the step's script as GitHub would after resolving the inputs. + + `repository`, `pr_number` and `head_sha` are the resolved input values: + the event's own in a pull_request run, whatever the caller passed otherwise. + `prs` maps an API path such as `repos/mindsdb/auth/pulls/7` to its response. + `ecr_denied` lists repositories whose every describe-images call fails with + AccessDenied. + """ + fixtures = tmp_path / "fixtures" + for api_path, body in (prs or {}).items(): + target = fixtures / "github" / f"{api_path}.json" + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(json.dumps(body), encoding="utf-8") + fixtures.mkdir(exist_ok=True) + (fixtures / "ecr-repos").write_text("".join(f"{r}\n" for r in ecr_repos), encoding="utf-8") + (fixtures / "ecr-images").write_text("".join(f"{i}\n" for i in ecr_images), encoding="utf-8") + (fixtures / "ecr-denied").write_text("".join(f"{r}\n" for r in ecr_denied), encoding="utf-8") + + stubs.add("curl", CURL) + stubs.add("aws", AWS) + stubs.add("argocd", ARGOCD) + return stubs.run( + STEP["run"], + { + "STUB_FIXTURES": str(fixtures), + "ARGOCD_SERVER": ACTION["inputs"]["argocd-server"]["default"], + "ARGOCD_PLAINTEXT": "auto", + "ARGOCD_AUTH_TOKEN": "argocd-token", + "ARGOCD_VERSION": "v0.0.0", + "GH_TOKEN": "gh-token", + "STACKS_DISPATCH_TOKEN": "gh-token", + "WAIT_TIMEOUT": "60", + "PR_REPOSITORY": repository, + "PR_NUMBER": pr_number, + "PR_HEAD_SHA": head_sha, + "GITHUB_EVENT_NAME": event, + "GITHUB_SHA": MERGE_SHA, + "GITHUB_REPOSITORY": "mindsdb/deployer" if event != "pull_request" else repository, + "RUNNER_TEMP": str(tmp_path), + "AWS_REGION": AWS_REGION, + }, + tmp_path, + ) + + +def app_set(stubs) -> list[str]: + calls = [argv for argv in stubs.calls("argocd") if argv[1:3] == ["app", "set"]] + assert len(calls) == 1, stubs.calls("argocd") + return calls[0] + + +def repository_lookup(repository: str) -> list[str]: + """The call that asks ECR whether `repository` exists. It asks for one + image, because without `--max-items 1` the CLI pages through every image in + the repository, one request per 100 images.""" + return [ + "aws", "ecr", "describe-images", "--repository-name", repository, + "--max-items", "1", "--region", AWS_REGION, + ] + + +def tag_lookup(repository: str, tag: str) -> list[str]: + """The call that asks ECR whether `repository` holds `tag`.""" + return [ + "aws", "ecr", "describe-images", "--repository-name", repository, + "--image-ids", f"imageTag={tag}", "--region", AWS_REGION, + ] + + +class TestInputsDefaultToThePullRequestEvent: + """A caller in a pull_request run passes nothing and gets today's values.""" + + @pytest.mark.parametrize( + ("name", "default", "env"), + [ + ("repository", "${{ github.repository }}", "PR_REPOSITORY"), + ("pr-number", "${{ github.event.pull_request.number }}", "PR_NUMBER"), + ("head-sha", "${{ github.event.pull_request.head.sha }}", "PR_HEAD_SHA"), + ], + ) + def test_input_defaults_to_the_event_and_reaches_the_script_through_env(self, name, default, env): + assert ACTION["inputs"][name]["required"] is False + assert ACTION["inputs"][name]["default"] == default + assert STEP["env"][env] == f"${{{{ inputs.{name} }}}}" + + def test_the_script_expands_no_template_itself(self): + """Every input reaches bash as an environment variable, never as text + spliced into the script, which is what makes refusing bad input enough.""" + assert "${{" not in STEP["run"] + + +@runs_the_script +class TestAPullRequestRunIsUnchanged: + def test_it_deploys_the_merge_sha_image_this_run_built(self, stubs, tmp_path): + result = run_deploy( + stubs, tmp_path, + event="pull_request", repository="mindsdb/auth", pr_number="7", head_sha=HEAD_SHA, + prs={"repos/mindsdb/auth/pulls/7": {"body": "No links."}}, + ecr_repos=("mindsdb-auth",), + ecr_images=(f"mindsdb-auth:development-{MERGE_SHA}",), + ) + + assert result.returncode == 0, result.stdout + result.stderr + assert app_set(stubs) == [ + "argocd", "app", "set", "pr-auth-7", "--grpc-web", "--plaintext", + "--helm-set", f"tags.auth=development-{MERGE_SHA}", + "--helm-set", f"revisions.auth={HEAD_SHA}", + "--revision", "main", + ] + wait = [argv for argv in stubs.calls("argocd") if argv[1:3] == ["app", "wait"]] + assert wait[0][3:5] == ["-l", "pr-env.mindsdb.com/anchor-repo=auth,pr-env.mindsdb.com/pr-number=7"] + + +@runs_the_script +class TestARunWithInputsDeploysTheNamedPr: + """What a scheduled or manually dispatched sync of open PRs does: no PR event, + no build in this run.""" + + @pytest.mark.parametrize("event", ["schedule", "workflow_dispatch"]) + def test_it_deploys_the_head_image_of_the_pr_it_names(self, stubs, tmp_path, event): + result = run_deploy( + stubs, tmp_path, + event=event, repository="mindsdb/cowork", pr_number="12", head_sha=HEAD_SHA, + prs={"repos/mindsdb/cowork/pulls/12": {"body": ""}}, + ecr_repos=("mindsdb-cowork-dev", "mindsdb-cowork"), + ecr_images=(f"mindsdb-cowork-dev:development-head-{HEAD_SHA}",), + ) + + assert result.returncode == 0, result.stdout + result.stderr + assert app_set(stubs) == [ + "argocd", "app", "set", "pr-cowork-12", "--grpc-web", "--plaintext", + "--helm-set", f"tags.cowork=development-head-{HEAD_SHA}", + "--helm-set", f"revisions.cowork={HEAD_SHA}", + "--revision", "main", + ] + gets = [argv[-1] for argv in stubs.calls("curl") if "POST" not in argv] + assert gets == ["https://api.github.com/repos/mindsdb/cowork/pulls/12"] + + +LINKS_COWORK_5 = { + "repos/mindsdb/auth/pulls/7": {"body": "Deploys: cowork#5"}, + "repos/mindsdb/cowork/pulls/5": {"merge_commit_sha": LINKED_MERGE_SHA, "head": {"sha": LINKED_HEAD_SHA}}, +} + + +@runs_the_script +class TestEachTagIsCheckedInOneRepository: + """A repo with a dev tier pushes its PR images to `mindsdb--dev` and + nowhere else, so when that repository exists a tag is checked there alone. + Otherwise it is checked in `mindsdb-`. Whether `-dev` exists decides + it, never whether `-dev` holds the tag.""" + + def test_a_repo_with_a_dev_tier_is_checked_there_only(self, stubs, tmp_path): + """The image sits in `mindsdb-cowork` alone. A check that went on to + that repository after missing in `-dev` would find it and deploy.""" + result = run_deploy( + stubs, tmp_path, + event="schedule", repository="mindsdb/cowork", pr_number="12", head_sha=HEAD_SHA, + prs={"repos/mindsdb/cowork/pulls/12": {"body": ""}}, + ecr_repos=("mindsdb-cowork-dev", "mindsdb-cowork"), + ecr_images=(f"mindsdb-cowork:development-head-{HEAD_SHA}",), + ) + + assert result.returncode == 1 + assert f"::error::ECR image mindsdb-cowork-dev:development-head-{HEAD_SHA} not found." in result.stdout + assert stubs.calls("aws") == [ + repository_lookup("mindsdb-cowork-dev"), + tag_lookup("mindsdb-cowork-dev", f"development-head-{HEAD_SHA}"), + ] + assert stubs.calls("argocd") == [] + + def test_a_repo_without_a_dev_tier_is_checked_in_its_own_repository(self, stubs, tmp_path): + result = run_deploy( + stubs, tmp_path, + event="schedule", repository="mindsdb/mindshub_inference", pr_number="3", head_sha=HEAD_SHA, + prs={"repos/mindsdb/mindshub_inference/pulls/3": {"body": ""}}, + ecr_repos=("mindsdb-mindshub-inference",), + ecr_images=(f"mindsdb-mindshub-inference:development-head-{HEAD_SHA}",), + ) + + assert result.returncode == 0, result.stdout + result.stderr + assert stubs.calls("aws") == [ + repository_lookup("mindsdb-mindshub-inference-dev"), + tag_lookup("mindsdb-mindshub-inference", f"development-head-{HEAD_SHA}"), + ] + assert f"tags.mindshub_inference=development-head-{HEAD_SHA}" in app_set(stubs) + + def test_a_linked_pr_image_is_found_in_its_dev_tier(self, stubs, tmp_path): + result = run_deploy( + stubs, tmp_path, + event="pull_request", repository="mindsdb/auth", pr_number="7", head_sha=HEAD_SHA, + prs=LINKS_COWORK_5, + ecr_repos=("mindsdb-auth", "mindsdb-cowork-dev", "mindsdb-cowork"), + ecr_images=( + f"mindsdb-auth:development-{MERGE_SHA}", + f"mindsdb-cowork-dev:development-head-{LINKED_HEAD_SHA}", + ), + ) + + assert result.returncode == 0, result.stdout + result.stderr + assert f"tags.cowork=development-head-{LINKED_HEAD_SHA}" in app_set(stubs) + assert stubs.calls("aws")[-2:] == [ + repository_lookup("mindsdb-cowork-dev"), + tag_lookup("mindsdb-cowork-dev", f"development-head-{LINKED_HEAD_SHA}"), + ] + + def test_a_linked_pr_image_outside_its_dev_tier_does_not_count(self, stubs, tmp_path): + """Both of the linked PR's tags sit in `mindsdb-cowork` alone, so neither + counts, and the script never asks that repository.""" + result = run_deploy( + stubs, tmp_path, + event="pull_request", repository="mindsdb/auth", pr_number="7", head_sha=HEAD_SHA, + prs=LINKS_COWORK_5, + ecr_repos=("mindsdb-auth", "mindsdb-cowork-dev", "mindsdb-cowork"), + ecr_images=( + f"mindsdb-auth:development-{MERGE_SHA}", + f"mindsdb-cowork:development-head-{LINKED_HEAD_SHA}", + f"mindsdb-cowork:development-{LINKED_MERGE_SHA}", + ), + ) + + assert result.returncode == 1 + assert ( + f"neither development-head-{LINKED_HEAD_SHA} nor development-{LINKED_MERGE_SHA}" + " exists in mindsdb-cowork-dev." + ) in result.stdout + assert stubs.calls("aws") == [ + repository_lookup("mindsdb-auth-dev"), + tag_lookup("mindsdb-auth", f"development-{MERGE_SHA}"), + repository_lookup("mindsdb-cowork-dev"), + tag_lookup("mindsdb-cowork-dev", f"development-head-{LINKED_HEAD_SHA}"), + repository_lookup("mindsdb-cowork-dev"), + tag_lookup("mindsdb-cowork-dev", f"development-{LINKED_MERGE_SHA}"), + ] + assert stubs.calls("argocd") == [] + + def test_an_ecr_error_while_picking_the_repository_turns_the_checks_off(self, stubs, tmp_path): + """AccessDenied says nothing about whether `-dev` exists. It turns the + checks off like any other ECR error, rather than sending the check to + `mindsdb-cowork` as if cowork had no dev tier.""" + result = run_deploy( + stubs, tmp_path, + event="schedule", repository="mindsdb/cowork", pr_number="12", head_sha=HEAD_SHA, + prs={"repos/mindsdb/cowork/pulls/12": {"body": ""}}, + ecr_repos=("mindsdb-cowork-dev", "mindsdb-cowork"), + ecr_denied=("mindsdb-cowork-dev",), + ) + + assert result.returncode == 0, result.stdout + result.stderr + assert stubs.calls("aws") == [repository_lookup("mindsdb-cowork-dev")] + assert result.stdout.count("::warning::ECR describe-images failed") == 1 + assert f"tags.cowork=development-head-{HEAD_SHA}" in app_set(stubs) + + +BAD_REPOSITORY = "::error::repository must be owner/name.\n" +BAD_PR_NUMBER = ( + "::error::pr-number must be a PR number. Outside a pull_request event, " + "pass repository, pr-number and head-sha.\n" +) +BAD_HEAD_SHA = "::error::head-sha must be a full 40-character commit SHA.\n" + + +@runs_the_script +class TestInputThatIsNotAPrIsRefused: + @pytest.mark.parametrize( + ("repository", "pr_number", "head_sha", "message"), + [ + ("mindsdb/cowork", "12; curl attacker.example", HEAD_SHA, BAD_PR_NUMBER), + ("mindsdb/cowork", "12\n::add-mask::x", HEAD_SHA, BAD_PR_NUMBER), + ("mindsdb/cowork", "12", HEAD_SHA[:7], BAD_HEAD_SHA), + ("mindsdb/cowork", "12", HEAD_SHA.upper(), BAD_HEAD_SHA), + ("mindsdb/cowork/../auth", "12", HEAD_SHA, BAD_REPOSITORY), + ("mindsdb", "12", HEAD_SHA, BAD_REPOSITORY), + ("mindsdb/..", "12", HEAD_SHA, BAD_REPOSITORY), + ("mindsdb/.", "12", HEAD_SHA, BAD_REPOSITORY), + ("../cowork", "12", HEAD_SHA, BAD_REPOSITORY), + ], + ) + def test_it_stops_before_any_call_and_prints_only_the_rule( + self, stubs, tmp_path, repository, pr_number, head_sha, message + ): + """The one output line is the rule, never the value: a value copied from + a PR could otherwise carry a workflow command into the log.""" + result = run_deploy( + stubs, tmp_path, + event="schedule", repository=repository, pr_number=pr_number, head_sha=head_sha, + ) + + assert result.returncode == 1 + assert (result.stdout, result.stderr) == (message, "") + assert stubs.calls() == [] + + def test_a_run_outside_a_pull_request_without_inputs_says_what_to_pass(self, stubs, tmp_path): + """Outside a pull_request event the defaults resolve to empty strings.""" + result = run_deploy( + stubs, tmp_path, + event="workflow_dispatch", repository="mindsdb/deployer", pr_number="", head_sha="", + ) + + assert result.returncode == 1 + assert result.stdout == BAD_PR_NUMBER + assert stubs.calls() == [] diff --git a/tests/test_build_push_ecr.py b/tests/test_build_push_ecr.py new file mode 100644 index 0000000..70635d2 --- /dev/null +++ b/tests/test_build_push_ecr.py @@ -0,0 +1,162 @@ +"""Behaviour tests for `build-push-ecr/action.yml`. + +Private repositories call this action on `mdb-dev` with the default +`builder: remote`, and their builds have to keep running exactly the commands +they ran before `local` existed. So the remote case pins the full argv of every +`aws` and `docker` call, and the local case is asserted as a difference from it: +no repository setup, no remote builder, and the GitHub Actions cache flags added. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest +import yaml + +ACTION_DIR = Path(__file__).resolve().parents[1] / "build-push-ecr" +ACTION = yaml.safe_load((ACTION_DIR / "action.yml").read_text(encoding="utf-8")) +STEPS = ACTION["runs"]["steps"] +POLICY = (ACTION_DIR / "shared-ecr-policy.json").read_text(encoding="utf-8").rstrip("\n") + +REGISTRY = "123456789012.dkr.ecr.us-east-1.amazonaws.com" +MERGE_SHA = "a" * 40 +HEAD_SHA = "b" * 40 +EXTRA_BUILD_ARGS = "-f docker/Dockerfile --secret id=github_token,env=GITHUB_TOKEN" + + +def step(step_id: str) -> dict: + matches = [candidate for candidate in STEPS if candidate.get("id") == step_id] + assert len(matches) == 1, f"expected exactly one step with id '{step_id}'" + return matches[0] + + +def render(script: str, values: dict[str, str]) -> str: + """Substitute `${{ ... }}` the way the runner does before bash sees the script. + + An expression missing from `values` fails the test, so a new one added to the + action has to be given a value here rather than silently rendering empty. + """ + + def substitute(match: re.Match[str]) -> str: + expression = match.group(1).strip() + assert expression in values, f"no test value for ${{{{ {expression} }}}}" + return values[expression] + + return re.sub(r"\$\{\{(.*?)\}\}", substitute, script) + + +def run_build(stubs, tmp_path: Path, *, builder: str, pr_head_sha: str = HEAD_SHA): + stubs.add("aws") + stubs.add("docker") + script = render( + step("build")["run"], + { + "inputs.src-path": "", + "inputs.image-ref": "", + "inputs.module-name": "mindsdb-example", + "steps.login-ecr.outputs.registry": REGISTRY, + "inputs.build-for-environment": "development", + "env.ENV_NAME": "example-feature", + "github.event.pull_request.head.sha": pr_head_sha, + "github.action_path": str(ACTION_DIR), + "inputs.extra-build-args": EXTRA_BUILD_ARGS, + }, + ) + output = tmp_path / "github_output" + output.touch() + result = stubs.run( + script, + {"BUILDER": builder, "CI_SHA": MERGE_SHA, "GITHUB_OUTPUT": str(output)}, + tmp_path, + ) + return result, output.read_text(encoding="utf-8") + + +IMAGE = f"{REGISTRY}/mindsdb-example" +BUILD_ARGV = [ + "docker", "buildx", "build", + *EXTRA_BUILD_ARGS.split(), + "--build-arg", "BUILD_FOR_ENVIRONMENT=development", + "--build-arg", f"IMAGE_TAG=development-{MERGE_SHA}", + "-t", f"{IMAGE}:development-head-{HEAD_SHA}", + "-t", f"{IMAGE}:development-{MERGE_SHA}", + "-t", f"{IMAGE}:development", + "-t", f"{IMAGE}:latest", + "--push", ".", +] +CACHE_ARGV = [ + "--cache-from", "type=gha,scope=mindsdb-example", + "--cache-to", "type=gha,scope=mindsdb-example,mode=max", +] + + +class TestRemoteKeepsTodaysCommands: + """The default every private caller gets, argv for argv.""" + + def test_builder_defaults_to_remote(self): + assert ACTION["inputs"]["builder"]["default"] == "remote" + + def test_remote_runs_the_same_aws_and_docker_calls(self, stubs, tmp_path): + result, output = run_build(stubs, tmp_path, builder="remote") + + assert result.returncode == 0, result.stderr + assert stubs.calls() == [ + ["aws", "ecr", "create-repository", "--repository-name", "mindsdb-example"], + ["aws", "ecr", "set-repository-policy", "--repository-name", "mindsdb-example", "--policy-text", POLICY], + [ + "docker", "buildx", "create", "--name=remote-buildkit-agent", "--driver=remote", "--use", + "tcp://remote-buildkit-agent.infrastructure.svc.cluster.local:80", + ], + BUILD_ARGV, + ] + assert output == f"image={IMAGE}:development-{MERGE_SHA}\n" + + def test_a_push_build_gets_no_head_tag(self, stubs, tmp_path): + result, _ = run_build(stubs, tmp_path, builder="remote", pr_head_sha="") + + assert result.returncode == 0, result.stderr + build = stubs.calls("docker")[-1] + assert not any("-head-" in arg for arg in build) + + def test_the_cache_step_is_skipped(self): + cache_steps = [s for s in STEPS if "crazy-max/ghaction-github-runtime@" in s.get("uses", "")] + assert len(cache_steps) == 1 + assert cache_steps[0]["if"] == "inputs.builder == 'local'" + + +class TestLocalBuildsOnTheRunner: + """For GitHub-hosted runners: no in-cluster builder, no repository setup.""" + + def test_local_makes_no_aws_call_and_no_remote_builder(self, stubs, tmp_path): + result, _ = run_build(stubs, tmp_path, builder="local") + + assert result.returncode == 0, result.stderr + assert stubs.calls("aws") == [] + assert [argv[:3] for argv in stubs.calls("docker")] == [["docker", "buildx", "build"]] + + def test_local_adds_only_the_cache_flags(self, stubs, tmp_path): + result, output = run_build(stubs, tmp_path, builder="local") + + assert result.returncode == 0, result.stderr + build = stubs.calls("docker")[-1] + assert [arg for arg in build if arg not in CACHE_ARGV] == BUILD_ARGV + cache_at = build.index("--cache-from") + assert build[cache_at:cache_at + len(CACHE_ARGV)] == CACHE_ARGV + assert output == f"image={IMAGE}:development-{MERGE_SHA}\n" + + def test_the_runtime_is_exposed_before_the_build(self): + names = [s.get("uses", s.get("id", "")) for s in STEPS] + runtime = next(i for i, n in enumerate(names) if n.startswith("crazy-max/ghaction-github-runtime@")) + assert runtime < names.index("build") + + +@pytest.mark.parametrize("builder", ["", "Local", "cluster"]) +def test_an_unknown_builder_fails_before_any_aws_or_docker_call(stubs, tmp_path, builder): + result, output = run_build(stubs, tmp_path, builder=builder) + + assert result.returncode != 0 + assert "builder must be 'remote' or 'local'" in result.stdout + assert stubs.calls() == [] + assert output == "" diff --git a/tests/test_third_party_pins.py b/tests/test_third_party_pins.py new file mode 100644 index 0000000..8e53cfe --- /dev/null +++ b/tests/test_third_party_pins.py @@ -0,0 +1,76 @@ +"""Third-party actions inside the build and deploy composites are pinned to commits. + +These composites run next to the caller's AWS, cluster or ArgoCD credentials, +and some of their callers are public repositories. `setup-env` is here because +it runs in the same build job as `build-push-ecr`, and any step in a job that +grants `id-token: write` can request that job's OIDC token. A tag is mutable, +so a compromised upstream account could change what runs there without any +change here. A full commit SHA cannot move, and the `# vX.Y.Z` comment says +which release it is. In-org actions are exempt: they float on `main` by +decision, behind this repo's code-owner review (see `zizmor.yml`). + +The Snyk CLI that `snyk-docker-scan` installs is pinned to a release for the +same reason: it runs with the job's AWS credentials in its environment. So is +the BuildKit image that `build-push-ecr` builds on in `local` mode, which holds +the ECR registry credentials while it pushes. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest +import yaml + +ROOT = Path(__file__).resolve().parents[1] +USES = re.compile(r"^\s*(?:-\s+)?uses:\s*(?P\S+)(?P.*)$") +PINNED = re.compile(r"[\w.-]+/[\w./-]+@[0-9a-f]{40}") +VERSION_COMMENT = re.compile(r"\s+#\s+v\d+\.\d+\.\d+\s*$") + + +@pytest.mark.parametrize( + "action", ["build-push-ecr", "snyk-docker-scan", "helm-deploy", "argocd-pr-env-deploy", "setup-env"] +) +def test_every_third_party_action_is_pinned_to_a_commit_with_its_version(action): + lines = (ROOT / action / "action.yml").read_text(encoding="utf-8").splitlines() + for number, line in enumerate(lines, start=1): + match = USES.match(line) + if not match or match["ref"].startswith(("./", "mindsdb/")): + continue + assert PINNED.fullmatch(match["ref"]), f"{action}/action.yml:{number} is not pinned to a commit: {line.strip()}" + assert VERSION_COMMENT.fullmatch(match["rest"]), f"{action}/action.yml:{number} lacks a '# vX.Y.Z' comment" + + +def test_snyk_docker_scan_installs_one_snyk_cli_release(): + """`snyk/actions/setup` installs the newest release unless `snyk-version` names one.""" + action = yaml.safe_load((ROOT / "snyk-docker-scan" / "action.yml").read_text(encoding="utf-8")) + setups = [step for step in action["runs"]["steps"] if step.get("uses", "").startswith("snyk/actions/setup@")] + assert len(setups) == 1, "expected exactly one snyk/actions/setup step" + assert re.fullmatch(r"v\d+\.\d+\.\d+", setups[0].get("with", {}).get("snyk-version", "")), ( + "snyk/actions/setup must pass snyk-version: vX.Y.Z" + ) + + +BUILDKIT_DRIVER_OPTS = re.compile( + r"\$\{\{ inputs\.builder == 'local'" + r" && 'image=moby/buildkit:v\d+\.\d+\.\d+@sha256:[0-9a-f]{64}'" + r" \|\| '' \}\}" +) + + +def test_build_push_ecr_local_mode_runs_one_buildkit_release_by_digest(): + """Without an `image=` driver option, setup-buildx starts buildx's default, + the moving `moby/buildkit:buildx-stable-1` tag. `local` mode must name a + release and its multi-arch index digest, so neither a moved tag nor a new + release changes the container that pushes. `remote` builds elsewhere and + keeps the default, so its setup stays as it was.""" + action = yaml.safe_load((ROOT / "build-push-ecr" / "action.yml").read_text(encoding="utf-8")) + setups = [ + step for step in action["runs"]["steps"] if step.get("uses", "").startswith("docker/setup-buildx-action@") + ] + assert len(setups) == 1, "expected exactly one docker/setup-buildx-action step" + assert BUILDKIT_DRIVER_OPTS.fullmatch((setups[0].get("with") or {}).get("driver-opts", "")), ( + "docker/setup-buildx-action must pass driver-opts: image=moby/buildkit:vX.Y.Z@sha256:<64 hex digits>" + " when builder is local, and nothing otherwise" + )