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" + )