From 95f8186f01b412a5bebda82c5cc440a3d1fdce98 Mon Sep 17 00:00:00 2001 From: Cory Johns Date: Tue, 6 Oct 2026 16:12:54 +0000 Subject: [PATCH 01/10] feat: add the validate-min-terraform action (ENG-8760) [ENG-8760](https://stacklet.atlassian.net/browse/ENG-8760) What ---- - Adds `validate-min-terraform`, a composite action that reads a module's Terraform floor out of `required_version` and runs `terraform validate` at that release. - Sets the repository up for the tools that follow: one directory per tool, each with an `action.yml` and a `README.md`, plus a lint workflow, a prek config, a justfile and a README that records the layout. Why ---- Five of the AWS and Azure onboarding module repositories need this job, and the version of it that ENG-8760 wrote carries enough shell to be worth maintaining in one place. Each repository validates at the release pinned in `.tool-versions`, which sits near the top of the supported range, so nothing covered the bottom of it. That is how `required_version = ">= 1"` survived in a module that no release below 1.8 can even parse. The action derives the floor rather than taking it as an input, so editing the constraint moves what CI tests in the same commit. It runs terraform-docs to parse the configuration instead of matching a regex against one file, which is what the published actions do and what makes them wrong on a second `.tf`, a `.tf.json`, or a constraint split across lines. The terraform-docs image is pinned by digest because the project publishes no immutable releases, so both a tag and a release checksum can change underneath us. Callers pin by commit SHA. This repository publishes no release tags, and a floating ref would put the module repositories' CI under whatever lands here next. Testing ---- - [x] Ran the floor derivation against a module carrying three constraints at once, one of them in a `.tf.json` and one split across lines: it reports the highest lower bound, where the regex-based action reports an empty string that `semver` then reads as "any version". - [x] Ran it against all five onboarding modules, each of which resolves to the floor its `required_version` states. - [x] Checked the operator handling across a matrix: `>= 1.14.0`, `>= 1.14.0, < 2.0.0` and `~> 1.14` resolve; `!=`, a bare version and `>` are rejected; an upper bound on its own reports that the constraint states no lower bound. - [x] Checked that a container that fails to run prints docker's own error, rather than surfacing as an empty parse further down. - [x] `just lint` clean, with zizmor re-run under a token to confirm the five online audits schedule rather than skip. zizmor audits `action.yml` as well as the workflow. - [ ] Confirm the action runs end to end once this merges and the module repositories point at it. Docs ---- `README.md` lists the tools and says how to add one. The action carries its own `README.md` with usage, inputs, outputs and a note on the one case it cannot catch: `terraform validate` never evaluates `validation` blocks, so a floor that is only wrong inside one still passes. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 29 ++++++++ .pre-commit-config.yaml | 15 +++++ .tool-versions | 2 + README.md | 23 ++++++- justfile | 3 + validate-min-terraform/README.md | 49 ++++++++++++++ validate-min-terraform/action.yml | 107 ++++++++++++++++++++++++++++++ 7 files changed, 227 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/ci.yml create mode 100644 .pre-commit-config.yaml create mode 100644 .tool-versions create mode 100644 justfile create mode 100644 validate-min-terraform/README.md create mode 100644 validate-min-terraform/action.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..16c9cb3 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,29 @@ +name: CI + +on: + push: + branches: + - main + pull_request: + branches: + - main + +permissions: {} + +jobs: + lint: + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - uses: j178/prek-action@4e14d07f9231acabce116ccfca13b13dd9755ece # v3.0.0 + env: + # zizmor's five online audits skip silently without a token, and its + # summary line reads the same either way, so CI would look green with + # the supply-chain audits never having run. + GH_TOKEN: ${{ github.token }} diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..d253669 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,15 @@ +repos: + - repo: https://github.com/rhysd/actionlint + rev: 393031adb9afb225ee52ae2ccd7a5af5525e03e8 # frozen: v1.7.11 + hooks: + - id: actionlint + + - repo: https://github.com/zizmorcore/zizmor-pre-commit + rev: ea2eb407b4cbce87cf0d502f36578950494f5ac9 # frozen: v1.23.1 + hooks: + - id: zizmor + + - repo: https://github.com/codespell-project/codespell + rev: 2ccb47ff45ad361a21071a7eedda4c37e6ae8c5a # frozen: v2.4.2 + hooks: + - id: codespell diff --git a/.tool-versions b/.tool-versions new file mode 100644 index 0000000..30935bd --- /dev/null +++ b/.tool-versions @@ -0,0 +1,2 @@ +just 1.40.0 +uv 0.8.14 diff --git a/README.md b/README.md index 89d40ee..00bcf93 100644 --- a/README.md +++ b/README.md @@ -1,2 +1,23 @@ # github-tooling -Reusable GitHub Actions and workflows shared across Stacklet repositories + +Reusable GitHub Actions and workflows shared across Stacklet repositories. + +Each tool lives in its own top-level directory with an `action.yml` and a `README.md`. Reference one by path: + +```yaml +- uses: stacklet/github-tooling/@ +``` + +Pin by commit SHA. This repository publishes no release tags, so a SHA is the only reference that cannot move. + +## Tools + +| Tool | Purpose | +| --- | --- | +| [validate-min-terraform](validate-min-terraform/) | Runs `terraform validate` at the oldest release a module's `required_version` allows. | + +## Adding a tool + +1. Create a directory named for the tool. +2. Add an `action.yml` and a `README.md` covering usage, inputs, and outputs. +3. Add a row to the table above. diff --git a/justfile b/justfile new file mode 100644 index 0000000..fc11899 --- /dev/null +++ b/justfile @@ -0,0 +1,3 @@ +# lint files +lint: + uvx prek run --all-files diff --git a/validate-min-terraform/README.md b/validate-min-terraform/README.md new file mode 100644 index 0000000..92465aa --- /dev/null +++ b/validate-min-terraform/README.md @@ -0,0 +1,49 @@ +# validate-min-terraform + +Runs `terraform validate` against a module at the oldest Terraform release the module says it supports. + +A repository's lint job validates at the release pinned in `.tool-versions`, which is near the top of the supported range. Nothing covers the bottom of the range, so a module can declare `required_version = ">= 1.0"` and use syntax that no release below 1.8 parses. This action closes that gap. + +## Usage + +```yaml +jobs: + validate-min-terraform: + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - uses: stacklet/github-tooling/validate-min-terraform@ +``` + +## Inputs + +| Name | Default | Description | +| --- | --- | --- | +| `working-directory` | `.` | Directory holding the module to validate. | +| `terraform-docs-image` | a digest-pinned terraform-docs 0.20.0 | Image used to parse the configuration. | + +## Outputs + +| Name | Description | +| --- | --- | +| `version` | The Terraform release the module declares as its floor. | + +## How the floor is derived + +`terraform-docs` parses the configuration and reports every `required_version` in the module. Terraform applies all of them at once, so the effective floor is the highest lower bound among them. Only `>=` and `~>` state that bound literally, and `<` and `<=` cannot move it. + +The action fails rather than guessing on anything else. A constraint with no lower bound has no floor to test, and an operator such as `!=` or `=` can rule out the bound another term states, which would otherwise install a release the module excludes. + +Running a real parser matters. A regex over `*.tf` misses a constraint in a `.tf.json`, misses a second `.tf` file, and reads a constraint split across lines as empty, which several published actions then treat as "any version". + +The terraform-docs image is pinned by digest because the project publishes no immutable releases, so both a tag and a release checksum can change underneath us. Bumping the version means bumping the digest in `action.yml`. + +## Known limitation + +`terraform validate` run against a module directory treats every root variable as unknown, so it never evaluates `validation` blocks. A module whose floor is wrong only inside a variable validation still passes here. Closing that needs `terraform test` with `mock_provider`. diff --git a/validate-min-terraform/action.yml b/validate-min-terraform/action.yml new file mode 100644 index 0000000..34559c2 --- /dev/null +++ b/validate-min-terraform/action.yml @@ -0,0 +1,107 @@ +name: Validate the declared Terraform floor +description: >- + Reads the Terraform floor out of a module's required_version and runs + terraform validate at that release. + +inputs: + working-directory: + description: Directory holding the Terraform module to validate. + required: false + default: . + terraform-docs-image: + description: >- + Image used to parse the configuration. Pin by digest, not by tag. + required: false + # terraform-docs publishes no immutable releases, which leaves both a tag + # and a release checksum able to change underneath us. A digest is content + # addressed, so it cannot. This one is terraform-docs 0.20.0. + default: quay.io/terraform-docs/terraform-docs@sha256:37329e2dc2518e7f719a986a3954b10771c3fe000f50f83fd4d98d489df2eae2 + +outputs: + version: + description: The Terraform release the module declares as its floor. + value: ${{ steps.floor.outputs.version }} + +runs: + using: composite + steps: + # The floor is read out of required_version rather than passed in, so that + # editing the constraint moves what CI tests in the same commit. + # terraform-docs parses the configuration instead of matching a regex + # against one file, so it reports every required_version in the module, + # including one in a .tf.json or split across lines. + - name: Read the declared Terraform floor + id: floor + shell: bash + working-directory: ${{ inputs.working-directory }} + env: + TERRAFORM_DOCS_IMAGE: ${{ inputs.terraform-docs-image }} + run: | + # terraform-docs writes its own diagnostics to stderr, and merging those + # into the pipe below breaks the JSON parse. They are kept aside rather + # than discarded so that a failed pull or a parse error still says what + # went wrong instead of surfacing as empty output. + stderr=$(mktemp) + if ! parsed=$(docker run --rm -u "$(id -u):$(id -g)" -v "$PWD:/wd" -w /wd \ + "$TERRAFORM_DOCS_IMAGE" json . --output-file '' 2>"$stderr"); then + echo "terraform-docs could not read this module:" >&2 + cat "$stderr" >&2 + exit 1 + fi + constraints=$(printf '%s' "$parsed" \ + | jq -r '.requirements[] | select(.name == "terraform") | .version') + if [ -z "$constraints" ]; then + echo "this module declares no required_version" >&2 + exit 1 + fi + + # Terraform applies every required_version in the module at once, so the + # effective floor is the highest lower bound among them. Only >= and ~> + # state that bound literally, and an operator such as != or = can rule + # out the bound another term states. Resolving either needs the release + # list, so reject what this cannot read rather than guess at it. + bounds="" + while IFS= read -r term; do + term=$(printf '%s' "$term" | tr -d '[:space:]') + [ -n "$term" ] || continue + case "$term" in + '>='*|'~>'*) bound=${term#??} ;; + # An upper bound cannot move the floor, so it needs no reading. + '<'*) continue ;; + *) + echo "cannot resolve a Terraform floor from: $term" >&2 + echo "required_version may use >=, ~>, < and <= only" >&2 + exit 1 + ;; + esac + if ! printf '%s' "$bound" | grep -qE '^[0-9]+\.[0-9]+(\.[0-9]+)?$'; then + echo "not a version this can resolve: $term" >&2 + exit 1 + fi + case "$bound" in *.*.*) ;; *) bound="$bound.0" ;; esac + bounds=$(printf '%s\n%s' "$bounds" "$bound") + done <&2 + printf ' %s\n' "$constraints" >&2 + exit 1 + fi + echo "version=$floor" >> "$GITHUB_OUTPUT" + + - name: Setup Terraform + uses: hashicorp/setup-terraform@dfe3c3f87815947d99a8997f908cb6525fc44e9e # v4.0.1 + with: + terraform_version: ${{ steps.floor.outputs.version }} + + - name: Validate at the declared floor + shell: bash + working-directory: ${{ inputs.working-directory }} + run: | + terraform init -backend=false + terraform validate From 13a7e2f5d01a90a8092e93e89d6f5a288d499696 Mon Sep 17 00:00:00 2001 From: Cory Johns Date: Tue, 6 Oct 2026 18:11:25 +0000 Subject: [PATCH 02/10] Accept a single-component bound, and read requirements explicitly `>= 1` is the constraint this ticket started from, and the parser rejected it outright. Terraform reads a short bound as the lowest release matching it, so it now pads to three components rather than failing. `--show requirements` overrides a `.terraform-docs.yml` that hides the requirements section, which would otherwise report a module with a constraint as declaring none. Co-Authored-By: Claude Opus 5 --- validate-min-terraform/README.md | 14 ++++++++++++-- validate-min-terraform/action.yml | 18 +++++++++++++----- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/validate-min-terraform/README.md b/validate-min-terraform/README.md index 92465aa..7919121 100644 --- a/validate-min-terraform/README.md +++ b/validate-min-terraform/README.md @@ -36,14 +36,24 @@ jobs: ## How the floor is derived -`terraform-docs` parses the configuration and reports every `required_version` in the module. Terraform applies all of them at once, so the effective floor is the highest lower bound among them. Only `>=` and `~>` state that bound literally, and `<` and `<=` cannot move it. +`terraform-docs` parses the configuration and reports every `required_version` in the directory. Terraform applies all of them at once, so the effective floor is the highest lower bound among them. Only `>=` and `~>` state that bound literally, and `<` and `<=` cannot move it. -The action fails rather than guessing on anything else. A constraint with no lower bound has no floor to test, and an operator such as `!=` or `=` can rule out the bound another term states, which would otherwise install a release the module excludes. +The action fails rather than guessing on anything else: + +- A constraint with no lower bound, such as `< 2.0.0` alone, names no floor to test. +- `!=` and `=` can rule out the bound another term states, which would otherwise install a release the module excludes. +- `>` is exclusive, so it states no release to install. Write `>= 1.14.1` rather than `> 1.14.0`. + +The action reads one directory. A repository with modules in subdirectories needs one call per directory, which a matrix over `working-directory` covers. Running a real parser matters. A regex over `*.tf` misses a constraint in a `.tf.json`, misses a second `.tf` file, and reads a constraint split across lines as empty, which several published actions then treat as "any version". The terraform-docs image is pinned by digest because the project publishes no immutable releases, so both a tag and a release checksum can change underneath us. Bumping the version means bumping the digest in `action.yml`. +## Requirements + +The runner needs Docker, since the parse runs in a container. `ubuntu-latest` has it; the macOS runners do not. + ## Known limitation `terraform validate` run against a module directory treats every root variable as unknown, so it never evaluates `validation` blocks. A module whose floor is wrong only inside a variable validation still passes here. Closing that needs `terraform test` with `mock_provider`. diff --git a/validate-min-terraform/action.yml b/validate-min-terraform/action.yml index 34559c2..2187cc5 100644 --- a/validate-min-terraform/action.yml +++ b/validate-min-terraform/action.yml @@ -28,7 +28,7 @@ runs: # The floor is read out of required_version rather than passed in, so that # editing the constraint moves what CI tests in the same commit. # terraform-docs parses the configuration instead of matching a regex - # against one file, so it reports every required_version in the module, + # against one file, so it reports every required_version in the directory, # including one in a .tf.json or split across lines. - name: Read the declared Terraform floor id: floor @@ -42,8 +42,10 @@ runs: # than discarded so that a failed pull or a parse error still says what # went wrong instead of surfacing as empty output. stderr=$(mktemp) + trap 'rm -f "$stderr"' EXIT if ! parsed=$(docker run --rm -u "$(id -u):$(id -g)" -v "$PWD:/wd" -w /wd \ - "$TERRAFORM_DOCS_IMAGE" json . --output-file '' 2>"$stderr"); then + "$TERRAFORM_DOCS_IMAGE" json . --output-file '' --show requirements \ + 2>"$stderr"); then echo "terraform-docs could not read this module:" >&2 cat "$stderr" >&2 exit 1 @@ -74,11 +76,17 @@ runs: exit 1 ;; esac - if ! printf '%s' "$bound" | grep -qE '^[0-9]+\.[0-9]+(\.[0-9]+)?$'; then + if ! printf '%s' "$bound" | grep -qE '^[0-9]+(\.[0-9]+){0,2}$'; then echo "not a version this can resolve: $term" >&2 exit 1 fi - case "$bound" in *.*.*) ;; *) bound="$bound.0" ;; esac + # Terraform reads a short bound as the lowest release matching it, so + # >= 1 means 1.0.0 and setup-terraform needs all three components. + case "$bound" in + *.*.*) ;; + *.*) bound="$bound.0" ;; + *) bound="$bound.0.0" ;; + esac bounds=$(printf '%s\n%s' "$bounds" "$bound") done <&2 - printf ' %s\n' "$constraints" >&2 + printf '%s\n' "$constraints" | sed 's/^/ /' >&2 exit 1 fi echo "version=$floor" >> "$GITHUB_OUTPUT" From a9388038cb3f5b61218914a9e3e616bd495feee3 Mon Sep 17 00:00:00 2001 From: Cory Johns Date: Tue, 6 Oct 2026 20:00:26 +0000 Subject: [PATCH 03/10] Move the parse into a script, test it, and clamp the floor Review raised three things the inline shell could not answer. The parse now lives in `read-floor.sh`, so shellcheck reads it and a test can run it without a runner. `test-read-floor.sh` covers every constraint form the parser accepts or rejects, and CI runs it. The floor now has a floor. The release this installs comes out of the configuration under test, so without a clamp a branch picks which Terraform runs in CI, and Terraform below 1.2.2 bundles a go-getter affected by HCSEC-2022-13. `minimum-floor` defaults there and callers can raise it. The parse container also drops to a read-only mount with no network, neither of which terraform-docs needs to read a configuration. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 13 +++ .pre-commit-config.yaml | 5 ++ README.md | 3 +- justfile | 4 + validate-min-terraform/README.md | 13 +++ validate-min-terraform/action.yml | 78 +++------------- validate-min-terraform/read-floor.sh | 88 +++++++++++++++++++ validate-min-terraform/test-read-floor.sh | 81 +++++++++++++++++ .../hidden-requirements/.terraform-docs.yml | 4 + .../testdata/hidden-requirements/main.tf | 3 + .../testdata/several-files/extra.tf.json | 1 + .../testdata/several-files/main.tf | 4 + .../testdata/several-files/provider.tf | 3 + 13 files changed, 235 insertions(+), 65 deletions(-) create mode 100755 validate-min-terraform/read-floor.sh create mode 100755 validate-min-terraform/test-read-floor.sh create mode 100644 validate-min-terraform/testdata/hidden-requirements/.terraform-docs.yml create mode 100644 validate-min-terraform/testdata/hidden-requirements/main.tf create mode 100644 validate-min-terraform/testdata/several-files/extra.tf.json create mode 100644 validate-min-terraform/testdata/several-files/main.tf create mode 100644 validate-min-terraform/testdata/several-files/provider.tf diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 16c9cb3..44c3d43 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,3 +27,16 @@ jobs: # summary line reads the same either way, so CI would look green with # the supply-chain audits never having run. GH_TOKEN: ${{ github.token }} + + test: + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Run the tests + run: ./validate-min-terraform/test-read-floor.sh diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d253669..29fe32c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -9,6 +9,11 @@ repos: hooks: - id: zizmor + - repo: https://github.com/shellcheck-py/shellcheck-py + rev: 745eface02aef23e168a8afb6b5737818efbea95 # frozen: v0.11.0.1 + hooks: + - id: shellcheck + - repo: https://github.com/codespell-project/codespell rev: 2ccb47ff45ad361a21071a7eedda4c37e6ae8c5a # frozen: v2.4.2 hooks: diff --git a/README.md b/README.md index 00bcf93..a05a978 100644 --- a/README.md +++ b/README.md @@ -20,4 +20,5 @@ Pin by commit SHA. This repository publishes no release tags, so a SHA is the on 1. Create a directory named for the tool. 2. Add an `action.yml` and a `README.md` covering usage, inputs, and outputs. -3. Add a row to the table above. +3. Keep any non-trivial shell in its own script beside the `action.yml`, so that shellcheck reads it and a test can run it without a runner. +4. Add a row to the table above. diff --git a/justfile b/justfile index fc11899..7e0713b 100644 --- a/justfile +++ b/justfile @@ -1,3 +1,7 @@ # lint files lint: uvx prek run --all-files + +# run the tests +test: + ./validate-min-terraform/test-read-floor.sh diff --git a/validate-min-terraform/README.md b/validate-min-terraform/README.md index 7919121..bb45b93 100644 --- a/validate-min-terraform/README.md +++ b/validate-min-terraform/README.md @@ -26,6 +26,7 @@ jobs: | Name | Default | Description | | --- | --- | --- | | `working-directory` | `.` | Directory holding the module to validate. | +| `minimum-floor` | `1.2.2` | Refuse to run below this release. | | `terraform-docs-image` | a digest-pinned terraform-docs 0.20.0 | Image used to parse the configuration. | ## Outputs @@ -50,10 +51,22 @@ Running a real parser matters. A regex over `*.tf` misses a constraint in a `.tf The terraform-docs image is pinned by digest because the project publishes no immutable releases, so both a tag and a release checksum can change underneath us. Bumping the version means bumping the digest in `action.yml`. +## Why there is a floor on the floor + +The release this installs comes out of the configuration under test, so on a pull request the branch decides which Terraform runs in CI. That is the point of the action, and it is also a way to reach an old release for the sake of its bugs rather than for support. `terraform init` installs providers and fetches module sources whatever `-backend=false` says, and Terraform 1.0.0 through 1.2.1 bundle a go-getter affected by HCSEC-2022-13, which HashiCorp never backported to those lines. + +`minimum-floor` defaults to 1.2.2, the first release past that. Raise it to your own support floor if you would rather not run old releases at all. + +Two things this does not solve. A pull request still chooses which providers `init` installs, and a provider plugin is a binary that `validate` executes to read its schema, so the job must hold no secrets and no write permission. The usage example above is the shape to keep. Committing `.terraform.lock.hcl` would pin provider checksums, but a pull request can edit the lock file in the same commit, so it narrows the window rather than closing it. + ## Requirements The runner needs Docker, since the parse runs in a container. `ubuntu-latest` has it; the macOS runners do not. +## Tests + +`just test` runs `validate-min-terraform/test-read-floor.sh`, which exercises the floor derivation against generated modules and the fixtures under `testdata/`. CI runs the same script. Add a case there for any constraint form the parser learns to read or to reject. + ## Known limitation `terraform validate` run against a module directory treats every root variable as unknown, so it never evaluates `validation` blocks. A module whose floor is wrong only inside a variable validation still passes here. Closing that needs `terraform test` with `mock_provider`. diff --git a/validate-min-terraform/action.yml b/validate-min-terraform/action.yml index 2187cc5..d9b3275 100644 --- a/validate-min-terraform/action.yml +++ b/validate-min-terraform/action.yml @@ -8,6 +8,17 @@ inputs: description: Directory holding the Terraform module to validate. required: false default: . + minimum-floor: + description: >- + Refuse to run below this release. The floor comes out of the + configuration under test, so on a pull request the branch decides which + Terraform CI installs. + required: false + # Terraform 1.2.2 is the first release past HCSEC-2022-13, which covers + # four go-getter issues reachable while init fetches modules. HashiCorp + # never backported the fix to the 1.0 or 1.1 lines. Raise this to your own + # support floor if you would rather not run old releases at all. + default: 1.2.2 terraform-docs-image: description: >- Image used to parse the configuration. Pin by digest, not by tag. @@ -36,70 +47,9 @@ runs: working-directory: ${{ inputs.working-directory }} env: TERRAFORM_DOCS_IMAGE: ${{ inputs.terraform-docs-image }} + MINIMUM_FLOOR: ${{ inputs.minimum-floor }} run: | - # terraform-docs writes its own diagnostics to stderr, and merging those - # into the pipe below breaks the JSON parse. They are kept aside rather - # than discarded so that a failed pull or a parse error still says what - # went wrong instead of surfacing as empty output. - stderr=$(mktemp) - trap 'rm -f "$stderr"' EXIT - if ! parsed=$(docker run --rm -u "$(id -u):$(id -g)" -v "$PWD:/wd" -w /wd \ - "$TERRAFORM_DOCS_IMAGE" json . --output-file '' --show requirements \ - 2>"$stderr"); then - echo "terraform-docs could not read this module:" >&2 - cat "$stderr" >&2 - exit 1 - fi - constraints=$(printf '%s' "$parsed" \ - | jq -r '.requirements[] | select(.name == "terraform") | .version') - if [ -z "$constraints" ]; then - echo "this module declares no required_version" >&2 - exit 1 - fi - - # Terraform applies every required_version in the module at once, so the - # effective floor is the highest lower bound among them. Only >= and ~> - # state that bound literally, and an operator such as != or = can rule - # out the bound another term states. Resolving either needs the release - # list, so reject what this cannot read rather than guess at it. - bounds="" - while IFS= read -r term; do - term=$(printf '%s' "$term" | tr -d '[:space:]') - [ -n "$term" ] || continue - case "$term" in - '>='*|'~>'*) bound=${term#??} ;; - # An upper bound cannot move the floor, so it needs no reading. - '<'*) continue ;; - *) - echo "cannot resolve a Terraform floor from: $term" >&2 - echo "required_version may use >=, ~>, < and <= only" >&2 - exit 1 - ;; - esac - if ! printf '%s' "$bound" | grep -qE '^[0-9]+(\.[0-9]+){0,2}$'; then - echo "not a version this can resolve: $term" >&2 - exit 1 - fi - # Terraform reads a short bound as the lowest release matching it, so - # >= 1 means 1.0.0 and setup-terraform needs all three components. - case "$bound" in - *.*.*) ;; - *.*) bound="$bound.0" ;; - *) bound="$bound.0.0" ;; - esac - bounds=$(printf '%s\n%s' "$bounds" "$bound") - done <&2 - printf '%s\n' "$constraints" | sed 's/^/ /' >&2 - exit 1 - fi + floor=$("$GITHUB_ACTION_PATH/read-floor.sh") echo "version=$floor" >> "$GITHUB_OUTPUT" - name: Setup Terraform @@ -111,5 +61,5 @@ runs: shell: bash working-directory: ${{ inputs.working-directory }} run: | - terraform init -backend=false + terraform init -backend=false -input=false terraform validate diff --git a/validate-min-terraform/read-floor.sh b/validate-min-terraform/read-floor.sh new file mode 100755 index 0000000..25b315f --- /dev/null +++ b/validate-min-terraform/read-floor.sh @@ -0,0 +1,88 @@ +#!/usr/bin/env bash +# Print the oldest Terraform release the module in the working directory +# supports, read out of its required_version constraints. +set -euo pipefail + +: "${TERRAFORM_DOCS_IMAGE:?}" +: "${MINIMUM_FLOOR:?}" + +# terraform-docs writes its own diagnostics to stderr, and merging those into +# the pipe below breaks the JSON parse. They are kept aside rather than +# discarded so that a failed pull or a parse error still says what went wrong +# instead of surfacing as empty output. +stderr=$(mktemp) +trap 'rm -f "$stderr"' EXIT + +# --show overrides a .terraform-docs.yml that hides the requirements section, +# which would otherwise report a module that declares a constraint as one that +# declares none. --output-file keeps terraform-docs from writing its render +# into the mounted directory under a config-supplied output.file. +if ! parsed=$(docker run --rm --network none -u "$(id -u):$(id -g)" \ + -v "$PWD:/wd:ro" -w /wd "$TERRAFORM_DOCS_IMAGE" \ + json . --output-file '' --show requirements 2>"$stderr"); then + echo "terraform-docs could not read this module:" >&2 + cat "$stderr" >&2 + exit 1 +fi + +constraints=$(printf '%s' "$parsed" \ + | jq -r '.requirements[] | select(.name == "terraform") | .version') +if [ -z "$constraints" ]; then + echo "this module declares no required_version" >&2 + exit 1 +fi + +# Terraform applies every required_version in the directory at once, so the +# effective floor is the highest lower bound among them. Only >= and ~> state +# that bound literally, and an operator such as != or = can rule out the bound +# another term states. Resolving either needs the release list, so reject what +# this cannot read rather than guess at it. +bounds="" +while IFS= read -r term; do + term=$(printf '%s' "$term" | tr -d '[:space:]') + [ -n "$term" ] || continue + case "$term" in + '>='*|'~>'*) bound=${term#??} ;; + # An upper bound cannot move the floor, so it needs no reading. + '<'*) continue ;; + *) + echo "cannot resolve a Terraform floor from: $term" >&2 + echo "required_version may use >=, ~>, < and <= only" >&2 + exit 1 + ;; + esac + if ! printf '%s' "$bound" | grep -qE '^[0-9]+(\.[0-9]+){0,2}$'; then + echo "not a version this can resolve: $term" >&2 + exit 1 + fi + # Terraform reads a short bound as the lowest release matching it, so >= 1 + # means 1.0.0 and setup-terraform needs all three components. + case "$bound" in + *.*.*) ;; + *.*) bound="$bound.0" ;; + *) bound="$bound.0.0" ;; + esac + bounds=$(printf '%s\n%s' "$bounds" "$bound") +done <&2 + printf '%s\n' "$constraints" | sed 's/^/ /' >&2 + exit 1 +fi + +# The floor comes out of the configuration under test, and on a pull request +# that configuration is whatever the pull request says. Without a clamp the +# branch picks which Terraform release CI runs, which is a way to reach an old +# release for the sake of its bugs rather than for support. +if [ "$(printf '%s\n%s' "$MINIMUM_FLOOR" "$floor" | sort -V | head -1)" != "$MINIMUM_FLOOR" ]; then + echo "declared floor $floor is below the minimum this action will run: $MINIMUM_FLOOR" >&2 + exit 1 +fi + +printf '%s\n' "$floor" diff --git a/validate-min-terraform/test-read-floor.sh b/validate-min-terraform/test-read-floor.sh new file mode 100755 index 0000000..87efefa --- /dev/null +++ b/validate-min-terraform/test-read-floor.sh @@ -0,0 +1,81 @@ +#!/usr/bin/env bash +# Exercise read-floor.sh. Most cases are one constraint in one generated +# module; the cases that need more than that live under testdata/. +set -uo pipefail + +here=$(cd "$(dirname "$0")" && pwd) +export TERRAFORM_DOCS_IMAGE=${TERRAFORM_DOCS_IMAGE:-$( + sed -n 's|.*default: \(quay.io/terraform-docs.*\)|\1|p' "$here/action.yml" +)} +export MINIMUM_FLOOR=${MINIMUM_FLOOR:-1.2.2} + +work=$(mktemp -d) +trap 'rm -rf "$work"' EXIT +failures=0 + +check() { + local label=$1 expected=$2 actual=$3 rc=$4 + if [ "$expected" = FAIL ]; then + if [ "$rc" -eq 0 ]; then + echo "FAIL $label: expected a non-zero exit, got $actual" + failures=$((failures + 1)) + return + fi + elif [ "$actual" != "$expected" ]; then + echo "FAIL $label: expected $expected, got ${actual:-nothing} (exit $rc)" + failures=$((failures + 1)) + return + fi + echo "ok $label" +} + +# One constraint per generated module. FAIL means the script must reject it. +while IFS='|' read -r constraint expected; do + [ -n "$constraint" ] || continue + dir="$work/$(printf '%s' "$constraint" | tr -c 'a-zA-Z0-9.' '_')" + mkdir -p "$dir" + printf 'terraform {\n required_version = "%s"\n}\n' "$constraint" > "$dir/main.tf" + actual=$(cd "$dir" && "$here/read-floor.sh" 2>/dev/null) + check "$constraint" "$expected" "$actual" "$?" +done <<'CASES' +>= 1.14.0|1.14.0 +>= 1.14.0, < 2.0.0|1.14.0 +~> 1.14|1.14.0 +>= 1.9, >= 1.14.0, < 2.0.0|1.14.0 +>= 1.2.2|1.2.2 +>= 1|FAIL +~> 1|FAIL +< 2.0.0|FAIL +1.14.0|FAIL +> 1.14.0|FAIL +>= 1.14.0, != 1.14.0|FAIL +>= 1.14.0-beta1|FAIL +CASES + +# A short bound resolves rather than failing; it only trips the clamp. +dir="$work/short-bound" && mkdir -p "$dir" +printf 'terraform {\n required_version = ">= 1"\n}\n' > "$dir/main.tf" +actual=$(cd "$dir" && MINIMUM_FLOOR=1.0.0 "$here/read-floor.sh" 2>/dev/null) +check "the floor pads a short bound" 1.0.0 "$actual" "$?" + +# Terraform applies every constraint in the directory, including one in a +# .tf.json and one split across lines, so the highest lower bound wins. +actual=$(cd "$here/testdata/several-files" && "$here/read-floor.sh" 2>/dev/null) +check "several files in one directory" 1.99.0 "$actual" "$?" + +# A config that hides the requirements section must not make a module that +# declares a constraint look like one that declares none. +actual=$(cd "$here/testdata/hidden-requirements" && "$here/read-floor.sh" 2>/dev/null) +check "a config hiding requirements" 1.14.0 "$actual" "$?" + +# A module with no constraint at all is the one case that reports nothing. +dir="$work/no-constraint" && mkdir -p "$dir" +printf 'output "a" {\n value = 1\n}\n' > "$dir/main.tf" +actual=$(cd "$dir" && "$here/read-floor.sh" 2>/dev/null) +check "no required_version" FAIL "$actual" "$?" + +if [ "$failures" -gt 0 ]; then + echo "$failures case(s) failed" + exit 1 +fi +echo "all cases passed" diff --git a/validate-min-terraform/testdata/hidden-requirements/.terraform-docs.yml b/validate-min-terraform/testdata/hidden-requirements/.terraform-docs.yml new file mode 100644 index 0000000..20858f1 --- /dev/null +++ b/validate-min-terraform/testdata/hidden-requirements/.terraform-docs.yml @@ -0,0 +1,4 @@ +formatter: markdown + +sections: + hide: [requirements] diff --git a/validate-min-terraform/testdata/hidden-requirements/main.tf b/validate-min-terraform/testdata/hidden-requirements/main.tf new file mode 100644 index 0000000..48f553e --- /dev/null +++ b/validate-min-terraform/testdata/hidden-requirements/main.tf @@ -0,0 +1,3 @@ +terraform { + required_version = ">= 1.14.0" +} diff --git a/validate-min-terraform/testdata/several-files/extra.tf.json b/validate-min-terraform/testdata/several-files/extra.tf.json new file mode 100644 index 0000000..c37c280 --- /dev/null +++ b/validate-min-terraform/testdata/several-files/extra.tf.json @@ -0,0 +1 @@ +{"terraform": [{"required_version": ">= 1.99.0"}]} diff --git a/validate-min-terraform/testdata/several-files/main.tf b/validate-min-terraform/testdata/several-files/main.tf new file mode 100644 index 0000000..465d1d0 --- /dev/null +++ b/validate-min-terraform/testdata/several-files/main.tf @@ -0,0 +1,4 @@ +terraform { + required_version = + ">= 1.2.0" +} diff --git a/validate-min-terraform/testdata/several-files/provider.tf b/validate-min-terraform/testdata/several-files/provider.tf new file mode 100644 index 0000000..48f553e --- /dev/null +++ b/validate-min-terraform/testdata/several-files/provider.tf @@ -0,0 +1,3 @@ +terraform { + required_version = ">= 1.14.0" +} From 904e90a978529d3f81edc397d81eea4f5b54dd30 Mon Sep 17 00:00:00 2001 From: Cory Johns Date: Tue, 6 Oct 2026 20:44:27 +0000 Subject: [PATCH 04/10] Port the floor parser from shell to Python The parse was three mechanisms doing one job: a case statement for the operator, tr to strip whitespace, and two greps to validate what was left. Extending it meant touching all three. It is now one regex per term, and the version comparison is a tuple rather than a sort -V idiom. Splitting the derivation from the terraform-docs call makes the constraint cases unit tests. Fourteen of the sixteen cases no longer start a container, so the suite runs in a second and a new case costs nothing. It also retires a trap. The old loop read from a heredoc so that it ran in the current shell and its accumulator survived; rewriting it as a pipe would have silently emptied the result and failed every valid module. ruff replaces shellcheck, since no shell is left to check. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 2 +- .gitignore | 1 + .pre-commit-config.yaml | 7 +- README.md | 2 +- justfile | 2 +- validate-min-terraform/README.md | 4 +- validate-min-terraform/action.yml | 2 +- validate-min-terraform/read-floor.sh | 88 ----------- validate-min-terraform/read_floor.py | 141 ++++++++++++++++++ validate-min-terraform/test-read-floor.sh | 81 ---------- validate-min-terraform/test_read_floor.py | 133 +++++++++++++++++ .../testdata/no-constraint/main.tf | 3 + 12 files changed, 288 insertions(+), 178 deletions(-) create mode 100644 .gitignore delete mode 100755 validate-min-terraform/read-floor.sh create mode 100755 validate-min-terraform/read_floor.py delete mode 100755 validate-min-terraform/test-read-floor.sh create mode 100755 validate-min-terraform/test_read_floor.py create mode 100644 validate-min-terraform/testdata/no-constraint/main.tf diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 44c3d43..1c42e5f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,4 +39,4 @@ jobs: persist-credentials: false - name: Run the tests - run: ./validate-min-terraform/test-read-floor.sh + run: python3 -m unittest discover -s validate-min-terraform -p 'test_*.py' diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..c18dd8d --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +__pycache__/ diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 29fe32c..44024de 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -9,10 +9,11 @@ repos: hooks: - id: zizmor - - repo: https://github.com/shellcheck-py/shellcheck-py - rev: 745eface02aef23e168a8afb6b5737818efbea95 # frozen: v0.11.0.1 + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: f12be1ebaa5351c1fc76472de98db2c3446c8253 # frozen: v0.16.10 hooks: - - id: shellcheck + - id: ruff-check + - id: ruff-format - repo: https://github.com/codespell-project/codespell rev: 2ccb47ff45ad361a21071a7eedda4c37e6ae8c5a # frozen: v2.4.2 diff --git a/README.md b/README.md index a05a978..edb29e7 100644 --- a/README.md +++ b/README.md @@ -20,5 +20,5 @@ Pin by commit SHA. This repository publishes no release tags, so a SHA is the on 1. Create a directory named for the tool. 2. Add an `action.yml` and a `README.md` covering usage, inputs, and outputs. -3. Keep any non-trivial shell in its own script beside the `action.yml`, so that shellcheck reads it and a test can run it without a runner. +3. Keep any non-trivial logic in its own script beside the `action.yml`, so that a linter reads it and a test can run it without a runner. Prefer Python over shell for anything that parses or compares. 4. Add a row to the table above. diff --git a/justfile b/justfile index 7e0713b..bfcd85c 100644 --- a/justfile +++ b/justfile @@ -4,4 +4,4 @@ lint: # run the tests test: - ./validate-min-terraform/test-read-floor.sh + python3 -m unittest discover -s validate-min-terraform -p 'test_*.py' diff --git a/validate-min-terraform/README.md b/validate-min-terraform/README.md index bb45b93..da08c87 100644 --- a/validate-min-terraform/README.md +++ b/validate-min-terraform/README.md @@ -61,11 +61,11 @@ Two things this does not solve. A pull request still chooses which providers `in ## Requirements -The runner needs Docker, since the parse runs in a container. `ubuntu-latest` has it; the macOS runners do not. +The runner needs Docker, since the parse runs in a container, and `python3`, which reads the result. `ubuntu-latest` has both. The macOS runners have no Docker. ## Tests -`just test` runs `validate-min-terraform/test-read-floor.sh`, which exercises the floor derivation against generated modules and the fixtures under `testdata/`. CI runs the same script. Add a case there for any constraint form the parser learns to read or to reject. +`just test` runs `test_read_floor.py`, and CI runs the same command. The constraint cases call the parser directly and need no container, so adding one for a constraint form the parser learns to read or to reject costs nothing. Only the cases that turn on what terraform-docs reports run the script end to end, against the fixtures under `testdata/`. ## Known limitation diff --git a/validate-min-terraform/action.yml b/validate-min-terraform/action.yml index d9b3275..1cf6867 100644 --- a/validate-min-terraform/action.yml +++ b/validate-min-terraform/action.yml @@ -49,7 +49,7 @@ runs: TERRAFORM_DOCS_IMAGE: ${{ inputs.terraform-docs-image }} MINIMUM_FLOOR: ${{ inputs.minimum-floor }} run: | - floor=$("$GITHUB_ACTION_PATH/read-floor.sh") + floor=$("$GITHUB_ACTION_PATH/read_floor.py") echo "version=$floor" >> "$GITHUB_OUTPUT" - name: Setup Terraform diff --git a/validate-min-terraform/read-floor.sh b/validate-min-terraform/read-floor.sh deleted file mode 100755 index 25b315f..0000000 --- a/validate-min-terraform/read-floor.sh +++ /dev/null @@ -1,88 +0,0 @@ -#!/usr/bin/env bash -# Print the oldest Terraform release the module in the working directory -# supports, read out of its required_version constraints. -set -euo pipefail - -: "${TERRAFORM_DOCS_IMAGE:?}" -: "${MINIMUM_FLOOR:?}" - -# terraform-docs writes its own diagnostics to stderr, and merging those into -# the pipe below breaks the JSON parse. They are kept aside rather than -# discarded so that a failed pull or a parse error still says what went wrong -# instead of surfacing as empty output. -stderr=$(mktemp) -trap 'rm -f "$stderr"' EXIT - -# --show overrides a .terraform-docs.yml that hides the requirements section, -# which would otherwise report a module that declares a constraint as one that -# declares none. --output-file keeps terraform-docs from writing its render -# into the mounted directory under a config-supplied output.file. -if ! parsed=$(docker run --rm --network none -u "$(id -u):$(id -g)" \ - -v "$PWD:/wd:ro" -w /wd "$TERRAFORM_DOCS_IMAGE" \ - json . --output-file '' --show requirements 2>"$stderr"); then - echo "terraform-docs could not read this module:" >&2 - cat "$stderr" >&2 - exit 1 -fi - -constraints=$(printf '%s' "$parsed" \ - | jq -r '.requirements[] | select(.name == "terraform") | .version') -if [ -z "$constraints" ]; then - echo "this module declares no required_version" >&2 - exit 1 -fi - -# Terraform applies every required_version in the directory at once, so the -# effective floor is the highest lower bound among them. Only >= and ~> state -# that bound literally, and an operator such as != or = can rule out the bound -# another term states. Resolving either needs the release list, so reject what -# this cannot read rather than guess at it. -bounds="" -while IFS= read -r term; do - term=$(printf '%s' "$term" | tr -d '[:space:]') - [ -n "$term" ] || continue - case "$term" in - '>='*|'~>'*) bound=${term#??} ;; - # An upper bound cannot move the floor, so it needs no reading. - '<'*) continue ;; - *) - echo "cannot resolve a Terraform floor from: $term" >&2 - echo "required_version may use >=, ~>, < and <= only" >&2 - exit 1 - ;; - esac - if ! printf '%s' "$bound" | grep -qE '^[0-9]+(\.[0-9]+){0,2}$'; then - echo "not a version this can resolve: $term" >&2 - exit 1 - fi - # Terraform reads a short bound as the lowest release matching it, so >= 1 - # means 1.0.0 and setup-terraform needs all three components. - case "$bound" in - *.*.*) ;; - *.*) bound="$bound.0" ;; - *) bound="$bound.0.0" ;; - esac - bounds=$(printf '%s\n%s' "$bounds" "$bound") -done <&2 - printf '%s\n' "$constraints" | sed 's/^/ /' >&2 - exit 1 -fi - -# The floor comes out of the configuration under test, and on a pull request -# that configuration is whatever the pull request says. Without a clamp the -# branch picks which Terraform release CI runs, which is a way to reach an old -# release for the sake of its bugs rather than for support. -if [ "$(printf '%s\n%s' "$MINIMUM_FLOOR" "$floor" | sort -V | head -1)" != "$MINIMUM_FLOOR" ]; then - echo "declared floor $floor is below the minimum this action will run: $MINIMUM_FLOOR" >&2 - exit 1 -fi - -printf '%s\n' "$floor" diff --git a/validate-min-terraform/read_floor.py b/validate-min-terraform/read_floor.py new file mode 100755 index 0000000..df371c7 --- /dev/null +++ b/validate-min-terraform/read_floor.py @@ -0,0 +1,141 @@ +#!/usr/bin/env python3 +"""Print the oldest Terraform release the module in the working directory supports. + +The floor is read out of the module's required_version constraints rather than +passed in, so that editing a constraint moves what CI tests in the same commit. +""" + +import json +import os +import re +import subprocess +import sys + +# >= and ~> are the only operators that state a lower bound literally. < and <= +# cannot move the floor. Anything else either states no bound at all or can rule +# out the bound another term states, and resolving that needs the release list. +LOWER_BOUND = re.compile(r"^(?:>=|~>)\s*(\d+(?:\.\d+){0,2})$") +UPPER_BOUND = re.compile(r"^<=?\s*\S+$") + + +class FloorError(Exception): + """A constraint this cannot resolve a floor from.""" + + +def parse_version(text): + """Read a version as a comparable tuple. + + Terraform reads a short bound as the lowest release matching it, so >= 1 + means 1.0.0. + """ + parts = [int(part) for part in text.split(".")] + return tuple(parts + [0] * (3 - len(parts))) + + +def format_version(version): + return ".".join(str(part) for part in version) + + +def lower_bounds(constraints): + """Yield the lower bound stated by each term of each constraint.""" + for constraint in constraints: + for term in constraint.split(","): + term = term.strip() + if not term: + continue + match = LOWER_BOUND.match(term) + if match: + yield parse_version(match.group(1)) + elif not UPPER_BOUND.match(term): + raise FloorError( + f"cannot resolve a Terraform floor from: {term}\n" + "required_version may use >=, ~>, < and <= only" + ) + + +def derive_floor(constraints, minimum): + """Return the release to install, given every constraint in the directory. + + Terraform applies all of them at once, so the effective floor is the highest + lower bound among them. + """ + if not constraints: + raise FloorError("this module declares no required_version") + + bounds = list(lower_bounds(constraints)) + if not bounds: + found = "\n".join(f" {constraint}" for constraint in constraints) + raise FloorError(f"required_version states no lower bound; found:\n{found}") + + floor = max(bounds) + # The floor comes out of the configuration under test, and on a pull request + # that configuration is whatever the pull request says. Without a clamp the + # branch picks which Terraform release CI runs, which is a way to reach an + # old release for the sake of its bugs rather than for support. + if floor < minimum: + raise FloorError( + f"declared floor {format_version(floor)} is below the minimum " + f"this action will run: {format_version(minimum)}" + ) + return floor + + +def read_constraints(image, directory): + """Return every Terraform required_version terraform-docs finds in a directory. + + --show overrides a .terraform-docs.yml that hides the requirements section, + which would otherwise report a module that declares a constraint as one that + declares none. --output-file keeps terraform-docs from writing its render + into the mounted directory under a config-supplied output.file. + """ + result = subprocess.run( + [ + "docker", + "run", + "--rm", + "--network", + "none", + "-u", + f"{os.getuid()}:{os.getgid()}", + "-v", + f"{directory}:/wd:ro", + "-w", + "/wd", + image, + "json", + ".", + "--output-file", + "", + "--show", + "requirements", + ], + capture_output=True, + text=True, + check=False, + ) + if result.returncode != 0: + raise FloorError( + f"terraform-docs could not read this module:\n{result.stderr.rstrip()}" + ) + requirements = json.loads(result.stdout).get("requirements") or [] + return [ + requirement["version"] + for requirement in requirements + if requirement.get("name") == "terraform" + ] + + +def main(): + image = os.environ["TERRAFORM_DOCS_IMAGE"] + minimum = parse_version(os.environ["MINIMUM_FLOOR"]) + try: + constraints = read_constraints(image, os.getcwd()) + print(format_version(derive_floor(constraints, minimum))) + except FloorError as error: + print(error, file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/validate-min-terraform/test-read-floor.sh b/validate-min-terraform/test-read-floor.sh deleted file mode 100755 index 87efefa..0000000 --- a/validate-min-terraform/test-read-floor.sh +++ /dev/null @@ -1,81 +0,0 @@ -#!/usr/bin/env bash -# Exercise read-floor.sh. Most cases are one constraint in one generated -# module; the cases that need more than that live under testdata/. -set -uo pipefail - -here=$(cd "$(dirname "$0")" && pwd) -export TERRAFORM_DOCS_IMAGE=${TERRAFORM_DOCS_IMAGE:-$( - sed -n 's|.*default: \(quay.io/terraform-docs.*\)|\1|p' "$here/action.yml" -)} -export MINIMUM_FLOOR=${MINIMUM_FLOOR:-1.2.2} - -work=$(mktemp -d) -trap 'rm -rf "$work"' EXIT -failures=0 - -check() { - local label=$1 expected=$2 actual=$3 rc=$4 - if [ "$expected" = FAIL ]; then - if [ "$rc" -eq 0 ]; then - echo "FAIL $label: expected a non-zero exit, got $actual" - failures=$((failures + 1)) - return - fi - elif [ "$actual" != "$expected" ]; then - echo "FAIL $label: expected $expected, got ${actual:-nothing} (exit $rc)" - failures=$((failures + 1)) - return - fi - echo "ok $label" -} - -# One constraint per generated module. FAIL means the script must reject it. -while IFS='|' read -r constraint expected; do - [ -n "$constraint" ] || continue - dir="$work/$(printf '%s' "$constraint" | tr -c 'a-zA-Z0-9.' '_')" - mkdir -p "$dir" - printf 'terraform {\n required_version = "%s"\n}\n' "$constraint" > "$dir/main.tf" - actual=$(cd "$dir" && "$here/read-floor.sh" 2>/dev/null) - check "$constraint" "$expected" "$actual" "$?" -done <<'CASES' ->= 1.14.0|1.14.0 ->= 1.14.0, < 2.0.0|1.14.0 -~> 1.14|1.14.0 ->= 1.9, >= 1.14.0, < 2.0.0|1.14.0 ->= 1.2.2|1.2.2 ->= 1|FAIL -~> 1|FAIL -< 2.0.0|FAIL -1.14.0|FAIL -> 1.14.0|FAIL ->= 1.14.0, != 1.14.0|FAIL ->= 1.14.0-beta1|FAIL -CASES - -# A short bound resolves rather than failing; it only trips the clamp. -dir="$work/short-bound" && mkdir -p "$dir" -printf 'terraform {\n required_version = ">= 1"\n}\n' > "$dir/main.tf" -actual=$(cd "$dir" && MINIMUM_FLOOR=1.0.0 "$here/read-floor.sh" 2>/dev/null) -check "the floor pads a short bound" 1.0.0 "$actual" "$?" - -# Terraform applies every constraint in the directory, including one in a -# .tf.json and one split across lines, so the highest lower bound wins. -actual=$(cd "$here/testdata/several-files" && "$here/read-floor.sh" 2>/dev/null) -check "several files in one directory" 1.99.0 "$actual" "$?" - -# A config that hides the requirements section must not make a module that -# declares a constraint look like one that declares none. -actual=$(cd "$here/testdata/hidden-requirements" && "$here/read-floor.sh" 2>/dev/null) -check "a config hiding requirements" 1.14.0 "$actual" "$?" - -# A module with no constraint at all is the one case that reports nothing. -dir="$work/no-constraint" && mkdir -p "$dir" -printf 'output "a" {\n value = 1\n}\n' > "$dir/main.tf" -actual=$(cd "$dir" && "$here/read-floor.sh" 2>/dev/null) -check "no required_version" FAIL "$actual" "$?" - -if [ "$failures" -gt 0 ]; then - echo "$failures case(s) failed" - exit 1 -fi -echo "all cases passed" diff --git a/validate-min-terraform/test_read_floor.py b/validate-min-terraform/test_read_floor.py new file mode 100755 index 0000000..d17f83a --- /dev/null +++ b/validate-min-terraform/test_read_floor.py @@ -0,0 +1,133 @@ +#!/usr/bin/env python3 +"""Exercise read_floor.py. + +The constraint cases run against the parser directly, so they need no container. +The cases that turn on what terraform-docs reports run the script against the +fixtures under testdata/. +""" + +import os +import pathlib +import re +import subprocess +import unittest + +import read_floor + +HERE = pathlib.Path(__file__).parent +DEFAULT_MINIMUM = (1, 2, 2) + +# The image the action defaults to, so the tests and the action cannot drift. +IMAGE = os.environ.get("TERRAFORM_DOCS_IMAGE") or re.search( + r"default: (quay\.io/terraform-docs/\S+)", (HERE / "action.yml").read_text() +).group(1) + + +RESOLVES = [ + ([">= 1.14.0"], "1.14.0"), + ([">= 1.14.0, < 2.0.0"], "1.14.0"), + (["~> 1.14"], "1.14.0"), + ([">= 1.9, >= 1.14.0, < 2.0.0"], "1.14.0"), + ([">= 1.2.2"], "1.2.2"), + # Terraform applies every constraint in the directory at once. + ([">= 1.99.0", ">= 1.2.0", ">= 1.14.0"], "1.99.0"), + # A short bound is the lowest release matching it. + ([">= 1"], "1.0.0"), + (["~> 1"], "1.0.0"), +] + +REJECTS = [ + # No lower bound to test. + (["< 2.0.0"], "states no lower bound"), + (["<= 2.0.0"], "states no lower bound"), + # An operator that can rule out the bound another term states. + (["1.14.0"], "cannot resolve"), + ([">= 1.14.0, != 1.14.0"], "cannot resolve"), + ([">= 1.14.0, = 1.15.0"], "cannot resolve"), + # Exclusive, so it names no release to install. + (["> 1.14.0"], "cannot resolve"), + ([">= 1.14.0-beta1"], "cannot resolve"), + ([], "declares no required_version"), +] + + +class DeriveFloorTest(unittest.TestCase): + def test_resolves(self): + for constraints, expected in RESOLVES: + with self.subTest(constraints=constraints): + minimum = (1, 0, 0) if expected == "1.0.0" else DEFAULT_MINIMUM + floor = read_floor.derive_floor(constraints, minimum) + self.assertEqual(read_floor.format_version(floor), expected) + + def test_rejects(self): + for constraints, message in REJECTS: + with self.subTest(constraints=constraints): + with self.assertRaises(read_floor.FloorError) as caught: + read_floor.derive_floor(constraints, DEFAULT_MINIMUM) + self.assertIn(message, str(caught.exception)) + + def test_a_floor_below_the_minimum_is_refused(self): + with self.assertRaises(read_floor.FloorError) as caught: + read_floor.derive_floor([">= 1.1.0"], DEFAULT_MINIMUM) + self.assertIn("below the minimum", str(caught.exception)) + + def test_the_minimum_itself_is_allowed(self): + floor = read_floor.derive_floor([">= 1.2.2"], DEFAULT_MINIMUM) + self.assertEqual(floor, DEFAULT_MINIMUM) + + +class ReadFloorScriptTest(unittest.TestCase): + """End to end, including the terraform-docs run. These need Docker.""" + + def run_script(self, directory, minimum="1.2.2"): + return subprocess.run( + [str(HERE / "read_floor.py")], + cwd=directory, + capture_output=True, + text=True, + check=False, + env={ + **os.environ, + "TERRAFORM_DOCS_IMAGE": IMAGE, + "MINIMUM_FLOOR": minimum, + }, + ) + + def test_several_files_in_one_directory(self): + """A constraint in a .tf.json, and one split across lines, both count.""" + result = self.run_script(HERE / "testdata" / "several-files") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout.strip(), "1.99.0") + + def test_a_config_hiding_requirements(self): + """A module that declares a constraint must not look like one that does not.""" + result = self.run_script(HERE / "testdata" / "hidden-requirements") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout.strip(), "1.14.0") + + def test_no_required_version(self): + result = self.run_script(HERE / "testdata" / "no-constraint") + self.assertEqual(result.returncode, 1) + self.assertIn("declares no required_version", result.stderr) + + def test_a_container_that_cannot_run_says_so(self): + result = self.run_script(HERE / "testdata" / "several-files") + self.assertEqual(result.returncode, 0, result.stderr) + broken = subprocess.run( + [str(HERE / "read_floor.py")], + cwd=HERE / "testdata" / "several-files", + capture_output=True, + text=True, + check=False, + env={ + **os.environ, + "TERRAFORM_DOCS_IMAGE": "quay.io/terraform-docs/terraform-docs:no-such-tag", + "MINIMUM_FLOOR": "1.2.2", + }, + ) + self.assertEqual(broken.returncode, 1) + self.assertIn("could not read this module", broken.stderr) + + +if __name__ == "__main__": + unittest.main() diff --git a/validate-min-terraform/testdata/no-constraint/main.tf b/validate-min-terraform/testdata/no-constraint/main.tf new file mode 100644 index 0000000..6b89823 --- /dev/null +++ b/validate-min-terraform/testdata/no-constraint/main.tf @@ -0,0 +1,3 @@ +output "a" { + value = 1 +} From 34049abd851890b52142058162e39fef38219687 Mon Sep 17 00:00:00 2001 From: Cory Johns Date: Tue, 6 Oct 2026 21:26:45 +0000 Subject: [PATCH 05/10] Pin the prek version the lint job installs Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 10 ++++++++++ .tool-versions | 1 + justfile | 2 +- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1c42e5f..060b5a4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,7 +21,17 @@ jobs: with: persist-credentials: false + - name: Set tool versions + uses: wistia/parse-tool-versions@32f568a4ffd4bfa7720ebf93f171597d1ebc979a # v2.1.1 + with: + postfix: _TOOL_VERSION + - uses: j178/prek-action@4e14d07f9231acabce116ccfca13b13dd9755ece # v3.0.0 + with: + # Without a pin this installs whatever prek released most recently, + # so a hostile release reaches CI with no change here. The version + # comes from .tool-versions, so CI and a local run agree on it. + prek-version: ${{ env.PREK_TOOL_VERSION }} env: # zizmor's five online audits skip silently without a token, and its # summary line reads the same either way, so CI would look green with diff --git a/.tool-versions b/.tool-versions index 30935bd..aad993c 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,2 +1,3 @@ just 1.40.0 +prek 0.5.5 uv 0.8.14 diff --git a/justfile b/justfile index bfcd85c..35984f2 100644 --- a/justfile +++ b/justfile @@ -1,6 +1,6 @@ # lint files lint: - uvx prek run --all-files + prek run --all-files # run the tests test: From 4f0a80bbfef6e051cb01536e092a2bbe0d4ae206 Mon Sep 17 00:00:00 2001 From: Cory Johns Date: Wed, 7 Oct 2026 23:53:24 +0000 Subject: [PATCH 06/10] Cover the multiline constraint, and say where the floor check bites The three-constraint fixture could not fail on the constraint split across lines: the one in the .tf.json is higher, so dropping the multiline file leaves the expected floor unchanged. A second fixture makes the multiline form the deciding bound, where ignoring it yields 1.3.0 rather than 1.14.0. The tool table said the action validates at the declared floor without saying it refuses one below the configured minimum, and the note on `>` did not say why an exclusive bound names nothing to install. Co-Authored-By: Claude Opus 5 --- README.md | 2 +- validate-min-terraform/README.md | 2 +- validate-min-terraform/test_read_floor.py | 8 +++++++- .../testdata/multiline-constraint/main.tf | 4 ++++ .../testdata/multiline-constraint/other.tf | 3 +++ 5 files changed, 16 insertions(+), 3 deletions(-) create mode 100644 validate-min-terraform/testdata/multiline-constraint/main.tf create mode 100644 validate-min-terraform/testdata/multiline-constraint/other.tf diff --git a/README.md b/README.md index edb29e7..967c90c 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ Pin by commit SHA. This repository publishes no release tags, so a SHA is the on | Tool | Purpose | | --- | --- | -| [validate-min-terraform](validate-min-terraform/) | Runs `terraform validate` at the oldest release a module's `required_version` allows. | +| [validate-min-terraform](validate-min-terraform/) | Runs `terraform validate` at the floor a module's `required_version` declares, and fails when that floor is below a configured minimum. | ## Adding a tool diff --git a/validate-min-terraform/README.md b/validate-min-terraform/README.md index da08c87..ee4bc5b 100644 --- a/validate-min-terraform/README.md +++ b/validate-min-terraform/README.md @@ -43,7 +43,7 @@ The action fails rather than guessing on anything else: - A constraint with no lower bound, such as `< 2.0.0` alone, names no floor to test. - `!=` and `=` can rule out the bound another term states, which would otherwise install a release the module excludes. -- `>` is exclusive, so it states no release to install. Write `>= 1.14.1` rather than `> 1.14.0`. +- `>` is exclusive, so it names no release to install, even though it permits everything above the one it names. Use the inclusive form with a real release: `>= 1.14.1` rather than `> 1.14.0`. The action reads one directory. A repository with modules in subdirectories needs one call per directory, which a matrix over `working-directory` covers. diff --git a/validate-min-terraform/test_read_floor.py b/validate-min-terraform/test_read_floor.py index d17f83a..23e411d 100755 --- a/validate-min-terraform/test_read_floor.py +++ b/validate-min-terraform/test_read_floor.py @@ -94,11 +94,17 @@ def run_script(self, directory, minimum="1.2.2"): ) def test_several_files_in_one_directory(self): - """A constraint in a .tf.json, and one split across lines, both count.""" + """The highest bound wins, and here it is the one in the .tf.json.""" result = self.run_script(HERE / "testdata" / "several-files") self.assertEqual(result.returncode, 0, result.stderr) self.assertEqual(result.stdout.strip(), "1.99.0") + def test_a_constraint_split_across_lines_decides(self): + """The multiline form is the one a regex over the file reads as empty.""" + result = self.run_script(HERE / "testdata" / "multiline-constraint") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout.strip(), "1.14.0") + def test_a_config_hiding_requirements(self): """A module that declares a constraint must not look like one that does not.""" result = self.run_script(HERE / "testdata" / "hidden-requirements") diff --git a/validate-min-terraform/testdata/multiline-constraint/main.tf b/validate-min-terraform/testdata/multiline-constraint/main.tf new file mode 100644 index 0000000..b758363 --- /dev/null +++ b/validate-min-terraform/testdata/multiline-constraint/main.tf @@ -0,0 +1,4 @@ +terraform { + required_version = + ">= 1.14.0" +} diff --git a/validate-min-terraform/testdata/multiline-constraint/other.tf b/validate-min-terraform/testdata/multiline-constraint/other.tf new file mode 100644 index 0000000..12ad22a --- /dev/null +++ b/validate-min-terraform/testdata/multiline-constraint/other.tf @@ -0,0 +1,3 @@ +terraform { + required_version = ">= 1.3.0" +} From 8522610efa2d0661e2b3de41f2743860838e6789 Mon Sep 17 00:00:00 2001 From: Cory Johns Date: Thu, 8 Oct 2026 00:07:29 +0000 Subject: [PATCH 07/10] Raise the minimum floor to a supported release, and simplify the parse The clamp defaulted to 1.2.2, which closed one advisory and otherwise had nothing to recommend it: that line stopped receiving fixes years ago. It now defaults to 1.14.0, the oldest release still inside HashiCorp's support window, and the README records the rule and the review it needs when a new minor lands. The docker invocation is one shlex.split of an interpolated string rather than twenty lines of one argument each. read_floor.py writes the step output itself, so the action step is a bare invocation with no shell logic. just and uv in .tool-versions were both months behind. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 4 +- .tool-versions | 4 +- validate-min-terraform/README.md | 58 +++++++++++------------ validate-min-terraform/action.yml | 20 ++++---- validate-min-terraform/read_floor.py | 38 +++++++-------- validate-min-terraform/test_read_floor.py | 32 ++++++++++--- 6 files changed, 85 insertions(+), 71 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 060b5a4..f1ec714 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,8 +34,8 @@ jobs: prek-version: ${{ env.PREK_TOOL_VERSION }} env: # zizmor's five online audits skip silently without a token, and its - # summary line reads the same either way, so CI would look green with - # the supply-chain audits never having run. + # summary line reads the same either way, so CI would pass while the + # supply-chain audits never ran. GH_TOKEN: ${{ github.token }} test: diff --git a/.tool-versions b/.tool-versions index aad993c..9373257 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,3 +1,3 @@ -just 1.40.0 +just 1.58.0 prek 0.5.5 -uv 0.8.14 +uv 0.12.23 diff --git a/validate-min-terraform/README.md b/validate-min-terraform/README.md index ee4bc5b..176e317 100644 --- a/validate-min-terraform/README.md +++ b/validate-min-terraform/README.md @@ -1,8 +1,8 @@ # validate-min-terraform -Runs `terraform validate` against a module at the oldest Terraform release the module says it supports. - -A repository's lint job validates at the release pinned in `.tool-versions`, which is near the top of the supported range. Nothing covers the bottom of the range, so a module can declare `required_version = ">= 1.0"` and use syntax that no release below 1.8 parses. This action closes that gap. +Runs `terraform validate` against a Terraform module at the oldest release its +`required_version` allows, so the bottom of the declared range gets tested +rather than assumed. ## Usage @@ -21,12 +21,17 @@ jobs: - uses: stacklet/github-tooling/validate-min-terraform@ ``` +> [!WARNING] +> `terraform init` installs the providers the module names, and `validate` runs +> each provider binary to read its schema. On a pull request that configuration +> is untrusted, so give the job `contents: read`, and no secrets. + ## Inputs | Name | Default | Description | | --- | --- | --- | -| `working-directory` | `.` | Directory holding the module to validate. | -| `minimum-floor` | `1.2.2` | Refuse to run below this release. | +| `working-directory` | `.` | Directory holding the module. One directory per call, with no descent into subdirectories. | +| `minimum-floor` | `1.14.0` | Refuse to run below this release. The default is the oldest release HashiCorp still ships fixes for. | | `terraform-docs-image` | a digest-pinned terraform-docs 0.20.0 | Image used to parse the configuration. | ## Outputs @@ -35,38 +40,29 @@ jobs: | --- | --- | | `version` | The Terraform release the module declares as its floor. | -## How the floor is derived - -`terraform-docs` parses the configuration and reports every `required_version` in the directory. Terraform applies all of them at once, so the effective floor is the highest lower bound among them. Only `>=` and `~>` state that bound literally, and `<` and `<=` cannot move it. - -The action fails rather than guessing on anything else: - -- A constraint with no lower bound, such as `< 2.0.0` alone, names no floor to test. -- `!=` and `=` can rule out the bound another term states, which would otherwise install a release the module excludes. -- `>` is exclusive, so it names no release to install, even though it permits everything above the one it names. Use the inclusive form with a real release: `>= 1.14.1` rather than `> 1.14.0`. - -The action reads one directory. A repository with modules in subdirectories needs one call per directory, which a matrix over `working-directory` covers. - -Running a real parser matters. A regex over `*.tf` misses a constraint in a `.tf.json`, misses a second `.tf` file, and reads a constraint split across lines as empty, which several published actions then treat as "any version". - -The terraform-docs image is pinned by digest because the project publishes no immutable releases, so both a tag and a release checksum can change underneath us. Bumping the version means bumping the digest in `action.yml`. +## Requirements -## Why there is a floor on the floor +The runner needs Docker and `python3`. `ubuntu-latest` has both, and the macOS +runners have no Docker. -The release this installs comes out of the configuration under test, so on a pull request the branch decides which Terraform runs in CI. That is the point of the action, and it is also a way to reach an old release for the sake of its bugs rather than for support. `terraform init` installs providers and fetches module sources whatever `-backend=false` says, and Terraform 1.0.0 through 1.2.1 bundle a go-getter affected by HCSEC-2022-13, which HashiCorp never backported to those lines. +## Version constraints -`minimum-floor` defaults to 1.2.2, the first release past that. Raise it to your own support floor if you would rather not run old releases at all. +`required_version` must state a lower bound with `>=` or `~>`. Upper bounds are +allowed and ignored. The action rejects any constraint it cannot resolve to a +definite release, and fails when the floor it resolves falls below +`minimum-floor`. -Two things this does not solve. A pull request still chooses which providers `init` installs, and a provider plugin is a binary that `validate` executes to read its schema, so the job must hold no secrets and no write permission. The usage example above is the shape to keep. Committing `.terraform.lock.hcl` would pin provider checksums, but a pull request can edit the lock file in the same commit, so it narrows the window rather than closing it. +Every `required_version` in the directory counts, including one in a `.tf.json` +or split across lines. The floor is the highest lower bound among them, which +is what Terraform itself applies. -## Requirements +## Known limitation -The runner needs Docker, since the parse runs in a container, and `python3`, which reads the result. `ubuntu-latest` has both. The macOS runners have no Docker. +> [!NOTE] +> `terraform validate` treats every root variable as unknown, so it never +> evaluates `validation` blocks. A floor that is wrong only inside a variable +> validation still passes here. ## Tests -`just test` runs `test_read_floor.py`, and CI runs the same command. The constraint cases call the parser directly and need no container, so adding one for a constraint form the parser learns to read or to reject costs nothing. Only the cases that turn on what terraform-docs reports run the script end to end, against the fixtures under `testdata/`. - -## Known limitation - -`terraform validate` run against a module directory treats every root variable as unknown, so it never evaluates `validation` blocks. A module whose floor is wrong only inside a variable validation still passes here. Closing that needs `terraform test` with `mock_provider`. +`just test` runs `test_read_floor.py`, and CI runs the same command. diff --git a/validate-min-terraform/action.yml b/validate-min-terraform/action.yml index 1cf6867..49c4b4b 100644 --- a/validate-min-terraform/action.yml +++ b/validate-min-terraform/action.yml @@ -14,18 +14,20 @@ inputs: configuration under test, so on a pull request the branch decides which Terraform CI installs. required: false - # Terraform 1.2.2 is the first release past HCSEC-2022-13, which covers - # four go-getter issues reachable while init fetches modules. HashiCorp - # never backported the fix to the 1.0 or 1.1 lines. Raise this to your own - # support floor if you would rather not run old releases at all. - default: 1.2.2 + # HashiCorp routes a version to a support regime by release date, and 1.14 + # falls in the band still receiving fixes and security patches, so this is + # the oldest release with a published claim to them. It wants revisiting + # when a new minor lands. See the README. + default: 1.14.0 terraform-docs-image: description: >- Image used to parse the configuration. Pin by digest, not by tag. required: false # terraform-docs publishes no immutable releases, which leaves both a tag # and a release checksum able to change underneath us. A digest is content - # addressed, so it cannot. This one is terraform-docs 0.20.0. + # addressed, so it cannot. This one is terraform-docs 0.20.0, the last + # release published as a multi-arch index. Later tags are per-architecture, + # so a digest pin would have to choose one. default: quay.io/terraform-docs/terraform-docs@sha256:37329e2dc2518e7f719a986a3954b10771c3fe000f50f83fd4d98d489df2eae2 outputs: @@ -48,9 +50,9 @@ runs: env: TERRAFORM_DOCS_IMAGE: ${{ inputs.terraform-docs-image }} MINIMUM_FLOOR: ${{ inputs.minimum-floor }} - run: | - floor=$("$GITHUB_ACTION_PATH/read_floor.py") - echo "version=$floor" >> "$GITHUB_OUTPUT" + # The script writes the step output itself, so this step carries no shell + # logic. A composite step has to name a shell even so. + run: '"$GITHUB_ACTION_PATH/read_floor.py"' - name: Setup Terraform uses: hashicorp/setup-terraform@dfe3c3f87815947d99a8997f908cb6525fc44e9e # v4.0.1 diff --git a/validate-min-terraform/read_floor.py b/validate-min-terraform/read_floor.py index df371c7..5cf09c6 100755 --- a/validate-min-terraform/read_floor.py +++ b/validate-min-terraform/read_floor.py @@ -8,6 +8,7 @@ import json import os import re +import shlex import subprocess import sys @@ -89,26 +90,12 @@ def read_constraints(image, directory): into the mounted directory under a config-supplied output.file. """ result = subprocess.run( - [ - "docker", - "run", - "--rm", - "--network", - "none", - "-u", - f"{os.getuid()}:{os.getgid()}", - "-v", - f"{directory}:/wd:ro", - "-w", - "/wd", - image, - "json", - ".", - "--output-file", - "", - "--show", - "requirements", - ], + shlex.split( + "docker run --rm --network none" + f" -u {os.getuid()}:{os.getgid()}" + f" -v {shlex.quote(f'{directory}:/wd:ro')} -w /wd {shlex.quote(image)}" + " json . --output-file '' --show requirements" + ), capture_output=True, text=True, check=False, @@ -130,10 +117,19 @@ def main(): minimum = parse_version(os.environ["MINIMUM_FLOOR"]) try: constraints = read_constraints(image, os.getcwd()) - print(format_version(derive_floor(constraints, minimum))) + floor = format_version(derive_floor(constraints, minimum)) except FloorError as error: print(error, file=sys.stderr) return 1 + + print(floor) + # This keeps the shell logic out of the action. Outside a workflow the + # variable is unset and stdout carries the whole result, which is what the + # tests read. + output = os.environ.get("GITHUB_OUTPUT") + if output: + with open(output, "a") as handle: + handle.write(f"version={floor}\n") return 0 diff --git a/validate-min-terraform/test_read_floor.py b/validate-min-terraform/test_read_floor.py index 23e411d..2dad1cf 100755 --- a/validate-min-terraform/test_read_floor.py +++ b/validate-min-terraform/test_read_floor.py @@ -10,12 +10,13 @@ import pathlib import re import subprocess +import tempfile import unittest import read_floor HERE = pathlib.Path(__file__).parent -DEFAULT_MINIMUM = (1, 2, 2) +DEFAULT_MINIMUM = (1, 14, 0) # The image the action defaults to, so the tests and the action cannot drift. IMAGE = os.environ.get("TERRAFORM_DOCS_IMAGE") or re.search( @@ -28,7 +29,7 @@ ([">= 1.14.0, < 2.0.0"], "1.14.0"), (["~> 1.14"], "1.14.0"), ([">= 1.9, >= 1.14.0, < 2.0.0"], "1.14.0"), - ([">= 1.2.2"], "1.2.2"), + ([">= 1.15.0"], "1.15.0"), # Terraform applies every constraint in the directory at once. ([">= 1.99.0", ">= 1.2.0", ">= 1.14.0"], "1.99.0"), # A short bound is the lowest release matching it. @@ -68,18 +69,18 @@ def test_rejects(self): def test_a_floor_below_the_minimum_is_refused(self): with self.assertRaises(read_floor.FloorError) as caught: - read_floor.derive_floor([">= 1.1.0"], DEFAULT_MINIMUM) + read_floor.derive_floor([">= 1.13.0"], DEFAULT_MINIMUM) self.assertIn("below the minimum", str(caught.exception)) def test_the_minimum_itself_is_allowed(self): - floor = read_floor.derive_floor([">= 1.2.2"], DEFAULT_MINIMUM) + floor = read_floor.derive_floor([">= 1.14.0"], DEFAULT_MINIMUM) self.assertEqual(floor, DEFAULT_MINIMUM) class ReadFloorScriptTest(unittest.TestCase): """End to end, including the terraform-docs run. These need Docker.""" - def run_script(self, directory, minimum="1.2.2"): + def run_script(self, directory, minimum="1.14.0", output=None): return subprocess.run( [str(HERE / "read_floor.py")], cwd=directory, @@ -90,9 +91,21 @@ def run_script(self, directory, minimum="1.2.2"): **os.environ, "TERRAFORM_DOCS_IMAGE": IMAGE, "MINIMUM_FLOOR": minimum, + **({"GITHUB_OUTPUT": str(output)} if output else {}), }, ) + def test_it_writes_the_step_output(self): + """The action reads the floor from GITHUB_OUTPUT, not from stdout.""" + with tempfile.TemporaryDirectory() as work: + output = pathlib.Path(work) / "output" + output.touch() + result = self.run_script( + HERE / "testdata" / "multiline-constraint", output=output + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(output.read_text(), "version=1.14.0\n") + def test_several_files_in_one_directory(self): """The highest bound wins, and here it is the one in the .tf.json.""" result = self.run_script(HERE / "testdata" / "several-files") @@ -111,6 +124,13 @@ def test_a_config_hiding_requirements(self): self.assertEqual(result.returncode, 0, result.stderr) self.assertEqual(result.stdout.strip(), "1.14.0") + def test_a_floor_below_the_minimum_is_refused(self): + result = self.run_script( + HERE / "testdata" / "multiline-constraint", minimum="1.15.0" + ) + self.assertEqual(result.returncode, 1) + self.assertIn("below the minimum", result.stderr) + def test_no_required_version(self): result = self.run_script(HERE / "testdata" / "no-constraint") self.assertEqual(result.returncode, 1) @@ -128,7 +148,7 @@ def test_a_container_that_cannot_run_says_so(self): env={ **os.environ, "TERRAFORM_DOCS_IMAGE": "quay.io/terraform-docs/terraform-docs:no-such-tag", - "MINIMUM_FLOOR": "1.2.2", + "MINIMUM_FLOOR": "1.14.0", }, ) self.assertEqual(broken.returncode, 1) From 8a5dff073c0b5a1d45ff0d0d853c41ec16797fb9 Mon Sep 17 00:00:00 2001 From: Cory Johns Date: Thu, 8 Oct 2026 16:05:15 +0000 Subject: [PATCH 08/10] Accept an exact version as a floor `= 1.14.0` and a bare `1.14.0` both name a definite minimum, and Terraform reads them as the same constraint, so rejecting either was a gap rather than a guard. `!=` stays rejected, since it can rule out the bound another term states. Co-Authored-By: Claude Opus 5 --- validate-min-terraform/README.md | 8 ++++---- validate-min-terraform/read_floor.py | 11 ++++++----- validate-min-terraform/test_read_floor.py | 8 ++++++-- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/validate-min-terraform/README.md b/validate-min-terraform/README.md index 176e317..0b5fa64 100644 --- a/validate-min-terraform/README.md +++ b/validate-min-terraform/README.md @@ -47,10 +47,10 @@ runners have no Docker. ## Version constraints -`required_version` must state a lower bound with `>=` or `~>`. Upper bounds are -allowed and ignored. The action rejects any constraint it cannot resolve to a -definite release, and fails when the floor it resolves falls below -`minimum-floor`. +`required_version` must state a lower bound with `>=`, `~>` or `=`, or name an +exact version such as `1.14.0`. Upper bounds are allowed and ignored. The action +rejects any constraint it cannot resolve to a definite release, and fails when +the floor it resolves falls below `minimum-floor`. Every `required_version` in the directory counts, including one in a `.tf.json` or split across lines. The floor is the highest lower bound among them, which diff --git a/validate-min-terraform/read_floor.py b/validate-min-terraform/read_floor.py index 5cf09c6..a0c8eb5 100755 --- a/validate-min-terraform/read_floor.py +++ b/validate-min-terraform/read_floor.py @@ -12,10 +12,11 @@ import subprocess import sys -# >= and ~> are the only operators that state a lower bound literally. < and <= -# cannot move the floor. Anything else either states no bound at all or can rule -# out the bound another term states, and resolving that needs the release list. -LOWER_BOUND = re.compile(r"^(?:>=|~>)\s*(\d+(?:\.\d+){0,2})$") +# >=, ~> and = state a lower bound literally, as does a bare version, which +# Terraform reads as an exact pin. < and <= cannot move the floor. Anything else +# either states no bound at all or can rule out the bound another term states, +# and resolving that needs the release list. +LOWER_BOUND = re.compile(r"^(?:>=|~>|=)?\s*(\d+(?:\.\d+){0,2})$") UPPER_BOUND = re.compile(r"^<=?\s*\S+$") @@ -50,7 +51,7 @@ def lower_bounds(constraints): elif not UPPER_BOUND.match(term): raise FloorError( f"cannot resolve a Terraform floor from: {term}\n" - "required_version may use >=, ~>, < and <= only" + "required_version may use >=, ~>, =, < and <= only" ) diff --git a/validate-min-terraform/test_read_floor.py b/validate-min-terraform/test_read_floor.py index 2dad1cf..70335e5 100755 --- a/validate-min-terraform/test_read_floor.py +++ b/validate-min-terraform/test_read_floor.py @@ -32,6 +32,12 @@ ([">= 1.15.0"], "1.15.0"), # Terraform applies every constraint in the directory at once. ([">= 1.99.0", ">= 1.2.0", ">= 1.14.0"], "1.99.0"), + # An exact pin states its own floor, alone or beside a lower bound. + (["= 1.14.0"], "1.14.0"), + (["=1.15.0"], "1.15.0"), + ([">= 1.14.0, = 1.15.0"], "1.15.0"), + # Terraform reads a bare version as an exact pin. + (["1.14.0"], "1.14.0"), # A short bound is the lowest release matching it. ([">= 1"], "1.0.0"), (["~> 1"], "1.0.0"), @@ -42,9 +48,7 @@ (["< 2.0.0"], "states no lower bound"), (["<= 2.0.0"], "states no lower bound"), # An operator that can rule out the bound another term states. - (["1.14.0"], "cannot resolve"), ([">= 1.14.0, != 1.14.0"], "cannot resolve"), - ([">= 1.14.0, = 1.15.0"], "cannot resolve"), # Exclusive, so it names no release to install. (["> 1.14.0"], "cannot resolve"), ([">= 1.14.0-beta1"], "cannot resolve"), From e47cbf9e81e1ecb0656495a5fed66476364875b2 Mon Sep 17 00:00:00 2001 From: Cory Johns Date: Thu, 8 Oct 2026 16:10:47 +0000 Subject: [PATCH 09/10] Reject an exclusion only when it removes the floor `>= 1.14.0, != 1.14.2` resolves to 1.14.0, since the exclusion does not touch the bound. `>= 1.14.0, != 1.14.0` still fails, because finding the next release above an excluded floor needs the release list. Co-Authored-By: Claude Opus 5 --- validate-min-terraform/README.md | 7 ++--- validate-min-terraform/read_floor.py | 33 ++++++++++++++++------- validate-min-terraform/test_read_floor.py | 6 +++-- 3 files changed, 31 insertions(+), 15 deletions(-) diff --git a/validate-min-terraform/README.md b/validate-min-terraform/README.md index 0b5fa64..2ea3eec 100644 --- a/validate-min-terraform/README.md +++ b/validate-min-terraform/README.md @@ -48,9 +48,10 @@ runners have no Docker. ## Version constraints `required_version` must state a lower bound with `>=`, `~>` or `=`, or name an -exact version such as `1.14.0`. Upper bounds are allowed and ignored. The action -rejects any constraint it cannot resolve to a definite release, and fails when -the floor it resolves falls below `minimum-floor`. +exact version such as `1.14.0`. Upper bounds are allowed and ignored, as is +`!=` unless it excludes the floor itself. The action rejects any constraint it +cannot resolve to a definite release, and fails when the floor it resolves +falls below `minimum-floor`. Every `required_version` in the directory counts, including one in a `.tf.json` or split across lines. The floor is the highest lower bound among them, which diff --git a/validate-min-terraform/read_floor.py b/validate-min-terraform/read_floor.py index a0c8eb5..e4d60fd 100755 --- a/validate-min-terraform/read_floor.py +++ b/validate-min-terraform/read_floor.py @@ -13,11 +13,12 @@ import sys # >=, ~> and = state a lower bound literally, as does a bare version, which -# Terraform reads as an exact pin. < and <= cannot move the floor. Anything else -# either states no bound at all or can rule out the bound another term states, -# and resolving that needs the release list. +# Terraform reads as an exact pin. < and <= cannot move the floor, and != only +# matters when it removes the floor another term states. Anything else needs the +# release list to resolve, which this does not read. LOWER_BOUND = re.compile(r"^(?:>=|~>|=)?\s*(\d+(?:\.\d+){0,2})$") UPPER_BOUND = re.compile(r"^<=?\s*\S+$") +EXCLUSION = re.compile(r"^!=\s*(\d+(?:\.\d+){0,2})$") class FloorError(Exception): @@ -38,21 +39,27 @@ def format_version(version): return ".".join(str(part) for part in version) -def lower_bounds(constraints): - """Yield the lower bound stated by each term of each constraint.""" +def read_terms(constraints): + """Return the lower bounds stated across every constraint, and the exclusions.""" + bounds = [] + excluded = set() for constraint in constraints: for term in constraint.split(","): term = term.strip() if not term: continue - match = LOWER_BOUND.match(term) - if match: - yield parse_version(match.group(1)) + bound = LOWER_BOUND.match(term) + exclusion = EXCLUSION.match(term) + if bound: + bounds.append(parse_version(bound.group(1))) + elif exclusion: + excluded.add(parse_version(exclusion.group(1))) elif not UPPER_BOUND.match(term): raise FloorError( f"cannot resolve a Terraform floor from: {term}\n" - "required_version may use >=, ~>, =, < and <= only" + "required_version may use >=, ~>, =, !=, < and <= only" ) + return bounds, excluded def derive_floor(constraints, minimum): @@ -64,12 +71,18 @@ def derive_floor(constraints, minimum): if not constraints: raise FloorError("this module declares no required_version") - bounds = list(lower_bounds(constraints)) + bounds, excluded = read_terms(constraints) if not bounds: found = "\n".join(f" {constraint}" for constraint in constraints) raise FloorError(f"required_version states no lower bound; found:\n{found}") floor = max(bounds) + if floor in excluded: + raise FloorError( + f"required_version excludes {format_version(floor)}, the floor it " + "states. Resolving the next release above it needs the release list." + ) + # The floor comes out of the configuration under test, and on a pull request # that configuration is whatever the pull request says. Without a clamp the # branch picks which Terraform release CI runs, which is a way to reach an diff --git a/validate-min-terraform/test_read_floor.py b/validate-min-terraform/test_read_floor.py index 70335e5..abc1eda 100755 --- a/validate-min-terraform/test_read_floor.py +++ b/validate-min-terraform/test_read_floor.py @@ -38,6 +38,8 @@ ([">= 1.14.0, = 1.15.0"], "1.15.0"), # Terraform reads a bare version as an exact pin. (["1.14.0"], "1.14.0"), + # An exclusion that leaves the floor alone does not stop the resolution. + ([">= 1.14.0, != 1.14.2"], "1.14.0"), # A short bound is the lowest release matching it. ([">= 1"], "1.0.0"), (["~> 1"], "1.0.0"), @@ -47,8 +49,8 @@ # No lower bound to test. (["< 2.0.0"], "states no lower bound"), (["<= 2.0.0"], "states no lower bound"), - # An operator that can rule out the bound another term states. - ([">= 1.14.0, != 1.14.0"], "cannot resolve"), + # An exclusion that removes the floor the other term states. + ([">= 1.14.0, != 1.14.0"], "excludes 1.14.0"), # Exclusive, so it names no release to install. (["> 1.14.0"], "cannot resolve"), ([">= 1.14.0-beta1"], "cannot resolve"), From c9cef9bf2460b6faf752da72f763656ea491a368 Mon Sep 17 00:00:00 2001 From: Cory Johns Date: Fri, 9 Oct 2026 21:06:33 +0000 Subject: [PATCH 10/10] Add CODEOWNERS Branch protection sets require_code_owner_reviews on a public repo, and with no CODEOWNERS file that setting matches nobody. The other five repos in this change carry the same one-line file. Co-Authored-By: Claude Opus 5 --- .github/CODEOWNERS | 1 + 1 file changed, 1 insertion(+) create mode 100644 .github/CODEOWNERS diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 0000000..6931f70 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1 @@ +* @stacklet/engineering