Skip to content

feat: add the validate-min-terraform action (ENG-8760) - #1

Open
johnsca wants to merge 10 commits into
mainfrom
johnsca/fix/ENG-8760/min-terraform-version
Open

johnsca wants to merge 10 commits into
mainfrom
johnsca/fix/ENG-8760/min-terraform-version

Conversation

@johnsca

@johnsca johnsca commented Oct 6, 2026 •

Copy link
Copy Markdown

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, with the parse in its own script and a test suite over it. Lays out the repository for the tools that follow it: one directory per tool, each carrying an action.yml and a README.md, alongside a lint workflow, a prek config, a justfile and a README recording the layout.

why

Five of the AWS and Azure onboarding module repositories need this check, and the version of it that ENG-8760 wrote carries enough shell to be worth one copy rather than five. Those repositories validate 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 parse.

The action derives the floor instead of 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 rather than matching a regex against one file. The regex approach is what the published actions do, and it is why they misread a second .tf, a .tf.json, or a constraint split across lines, the last of which yields an empty string that semver then reads as "any version".

The terraform-docs image is pinned by digest, not by tag. The project publishes no immutable releases, so a tag and the checksum published beside a release can both change underneath us, while a digest is content addressed and cannot.

The release the action installs comes out of the configuration under test, so on a pull request the branch decides which Terraform runs in CI. minimum-floor puts a bound under that, defaulting to 1.14.0. HashiCorp publishes no support window for the community CLI, and its support policy routes a version to a regime by release date, which puts 1.14 in the band still receiving fixes and security patches. That reading is a judgement call, recorded in the README alongside the number, and the default wants revisiting whenever a new minor lands. Callers can raise it to their own support floor.

Two things the clamp does not solve, and the action cannot. A pull request still chooses which providers init installs, and a provider plugin is a binary that validate runs to read its schema. So the job must hold no secrets and no write permission, which is the shape the usage example and all five callers have. The same is already true of the terraform_validate pre-commit hook these repos run today, so this is not new exposure, but it is worth stating where someone will read it.

The parse lives in read_floor.py rather than inline in the action. Inline, nothing linted it: the pre-commit actionlint hook only reads .github/workflows/, and a composite action.yml is not a workflow. In a script a linter reads it and a test can run it without a runner.

It is Python rather than shell because the work is parsing and comparing. One regex per term replaces a case statement, a tr, and two greps that all had to agree, and the version comparison is a tuple rather than a sort -V idiom. Splitting the derivation from the terraform-docs call is what makes the constraint cases unit tests: fourteen of the sixteen no longer start a container.

Callers pin by commit SHA, because this repository publishes no release tags and a floating ref would put five repositories' CI under whatever lands here next. If the repository grows enough tools to want versioned releases, that is the point to add tags and switch the callers over.

testing

  • Ran the floor derivation against a module carrying three constraints at once, one in a .tf.json and one split across lines. It reports the highest lower bound, which is the one Terraform's own init enforces.
  • Ran it against all five onboarding modules. Each resolves to the floor its required_version states.
  • Checked the operator handling across a matrix of constraints. >= 1.14.0, >= 1.14.0, < 2.0.0 and ~> 1.14 resolve, while !=, a bare version and > are rejected with the term that could not be read, and an upper bound on its own reports that the constraint states no lower bound. Review on the module PRs raised the first of those.
  • Checked that a container which fails to run prints docker's own error, rather than surfacing further down as an empty parse. Review raised this one too.
  • just test covers every constraint form the parser accepts or rejects, a constraint split across lines where that is the deciding bound, the three-constraint fixture, a .terraform-docs.yml that hides the requirements section, a module with no constraint at all, a floor under the clamp, the step output written to GITHUB_OUTPUT, and a container that cannot run. CI runs the same command. The suite takes under two seconds, since most cases call the parser directly and start no container.
  • Review caught four defects, all now covered by that suite. A single-component bound such as >= 1 was rejected outright, which is the constraint this ticket started from. A config hiding the requirements section made a module that declares a constraint look like one that declares none. The multiline fixture could not fail, because a higher bound in the sibling .tf.json decided the result either way. The clamp defaulted to a release nobody has patched in years.
  • just lint clean, and zizmor re-run under a token to confirm its five online audits schedule rather than skip. The prek release that job installs now comes from .tool-versions rather than being resolved at run time, so CI and a local run agree on it. zizmor audits action.yml as well as the workflow.
  • Confirm the action runs end to end once this merges and the five module PRs point at it. Their uses: pin names the commit on this branch, so it resolves as soon as the branch exists on GitHub, but it wants repointing at the commit on main after this merges.

docs

README.md lists the tools and says how to add one. The action carries its own README.md with usage, inputs, outputs and the one case it cannot catch: terraform validate against a module directory treats every root variable as unknown and so never evaluates validation blocks, which leaves a floor that is only wrong inside one still passing.

This repository is not described in cloud-infra, so its settings and required checks are unmanaged. Worth adding there separately.

🤖 Generated with Claude Code

[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 <noreply@anthropic.com>
@johnsca

johnsca commented Oct 6, 2026

Copy link
Copy Markdown
Author

@cubic-dev-ai please review

johnsca and others added 2 commits October 6, 2026 18:11
`>= 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
@johnsca
johnsca marked this pull request as ready for review October 6, 2026 20:24
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 <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 16 files

Architecture diagram
sequenceDiagram
    participant Dev as Developer
    participant GH as GitHub Actions
    participant Action as validate-min-terraform Action
    participant TD as terraform-docs Container
    participant Script as read_floor.py
    participant TF as Terraform CLI
    participant Runner as CI Runner

    Note over Dev,Runner: NEW: validate-min-terraform composite action flow

    Dev->>GH: Push/PR to main
    GH->>Runner: Trigger CI workflow
    
    Note over Runner: CI workflow runs lint + test jobs
    Runner->>Runner: lint: prek (actionlint, zizmor, ruff)
    Runner->>Runner: test: unittest discover

    alt Test job
        Runner->>Script: Run read_floor.py tests
        Script->>Script: Parse constraint cases (no container)
        Script->>TD: Docker run for fixture tests
        TD-->>Script: Parse requirements
        Script-->>Runner: Test results
    end

    Note over GH,Runner: Real module validation flow

    Runner->>Action: Composite action starts
    Action->>Action: Set working-directory
    
    Note over Action: Step 1: Read floor
    Action->>Script: Execute read_floor.py
    Script->>Script: Read required_version from module
    Script->>TD: Docker: terraform-docs json --show requirements
    TD->>TD: Parse all .tf/.tf.json files
    TD-->>Script: requirements JSON
    Script->>Script: Derive highest lower bound
    alt Valid floor found
        Script->>Script: Check minimum-floor clamp
        alt Floor >= minimum (1.2.2)
            Script-->>Action: version output
        else Floor < minimum
            Script-->>Action: Error + exit 1
        end
    else No constraint or unparseable
        Script-->>Action: Error + exit 1
    end

    Action->>Action: Write version to GITHUB_OUTPUT

    Note over Action: Step 2: Setup Terraform
    Action->>TF: Install terraform (version from floor)
    TF-->>Action: terraform ready

    Note over Action: Step 3: Validate
    Action->>TF: terraform init -backend=false
    TF->>TF: Install providers, fetch modules
    Action->>TF: terraform validate
    TF-->>Action: Validation results
    Action-->>Runner: Step completed

    Note over Dev,Runner: SECURITY: no secrets, no write permissions
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread README.md Outdated
Comment thread justfile Outdated
Comment thread validate-min-terraform/testdata/several-files/main.tf
Comment thread validate-min-terraform/README.md Outdated
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@johnsca
johnsca force-pushed the johnsca/fix/ENG-8760/min-terraform-version branch from 145094a to 34049ab Compare October 6, 2026 22:13

@jtroup jtroup left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments inline aren't blocking, but please either address or respond to the Cubic feedback. Thanks @johnsca

# including one in a .tf.json or split across lines.
- name: Read the declared Terraform floor
id: floor
shell: bash

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnsca Hmm, why?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hold-over from pre-refactor.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A composite step has to declare a shell, so shell: bash itself cannot go. The hold-over was the logic on that line.

read_floor.py now writes version= to $GITHUB_OUTPUT itself, so the step is a bare invocation of the script. A test covers the file contents.

Filed by Claude on Cory's behalf.

Comment thread validate-min-terraform/README.md Outdated

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnsca to be honest, given https://endoflife.date/terraform and terraform's relatively short support lifecycle, I'm left wondering whether we might not be better off simply enforcing a modern terraform minimum floor?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The official support info from HashiCorp has 1.14.x getting fixes and security patches though August of next year, per the Support Lifecycle Addendum. Which is a bit complicated, so we could instead just follow the 2 year primary LTS window.

Should I change the action to figure out the minimum version based on the 2-year LTS window and validate with that and that the constraint falls within that LTS window instead? That would force us to keep it up-to-date instead of letting it drift like we had.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping it a literal rather than deriving the window at run time.

Deriving it means a network call to HashiCorp on every run, a new failure mode when that endpoint moves or rate-limits, and a check whose result changes with no commit to point at. A module that passes on Monday fails on Tuesday because a window rolled.

The literal is 1.14.0, with the reasoning and its ambiguity in the README beside it. If drift is the worry, a scheduled job that opens a pull request raising the default keeps the change reviewable.

Filed by Claude on Cory's behalf.

Comment thread validate-min-terraform/README.md Outdated
| Name | Default | Description |
| --- | --- | --- |
| `working-directory` | `.` | Directory holding the module to validate. |
| `minimum-floor` | `1.2.2` | Refuse to run below this release. |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnsca Shouldn't the minimum floor be something supported and modern? The version you mention has been end of life for over 3 years and has a ton of security problems.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and changed. The default is now 1.14.0 rather than 1.2.2.

1.2.2 cleared one advisory, the go-getter issues reachable while init fetches modules. Nothing else recommended it.

1.14 rests on a judgement call. HashiCorp publishes no support window for the community CLI. Its support policy routes a version to a regime by release date, and 1.14 lands in the band still receiving fixes and security patches. The Support Lifecycle Addendum that band points at lists only Self-Managed products, so no row names a CLI line. We read the routing table as the applicable statement, which puts 1.14 at the oldest release with a published claim to fixes. If you read it differently, the input exists and the number is one line.

The README records that reasoning, and says the default wants revisiting whenever a new minor lands.

Filed by Claude on Cory's behalf.

Comment thread .tool-versions Outdated
@@ -0,0 +1,3 @@
just 1.40.0

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnsca this is very old - you may as well update it before shipping this? same with uv.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated here and in the five module repositories: just 1.58.0, uv 0.12.23, and in the modules terraform 1.16.5 and tflint 0.64.0. just lint passes at all of them.

terraform-docs stays at 0.20.0. The docs job runs terraform-docs/gh-actions v1.4.1, which builds FROM quay.io/terraform-docs/terraform-docs:0.20.0. Moving it would have CI check the README against a different renderer than just docs uses.

The image this action parses with stays there too. 0.20.0 is the last release published as a multi-arch index, and later tags are per-architecture, so a digest pin would have to pick one.

Filed by Claude on Cory's behalf.

Comment thread validate-min-terraform/read_floor.py Outdated
johnsca and others added 4 commits October 7, 2026 23:53
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
`= 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 <noreply@anthropic.com>
`>= 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 <noreply@anthropic.com>
@johnsca
johnsca force-pushed the johnsca/fix/ENG-8760/min-terraform-version branch from a52f36f to e47cbf9 Compare October 8, 2026 16:13
@johnsca

johnsca commented Oct 9, 2026

Copy link
Copy Markdown
Author

@cubic-dev-ai please re-review

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 9, 2026

Copy link
Copy Markdown

@cubic-dev-ai please re-review

@johnsca I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 18 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.
Architecture diagram
sequenceDiagram
    participant U as User Module Repo
    participant W as GitHub Workflow
    participant A as validate-min-terraform Action
    participant P as read_floor.py
    participant D as terraform-docs Container
    participant T as Terraform CLI
    participant CI as CI Pipeline

    Note over U,CI: Composite Action: Validate Terraform Floor

    U->>W: Trigger CI (push/PR)
    W->>A: Invoke composite action
    A->>P: Execute read_floor.py
    P->>D: docker run (parse .tf files)
    D->>D: Extract required_version constraints
    D-->>P: JSON with requirements
    P->>P: Parse constraints, derive floor
    alt Floor below minimum
        P-->>A: Error: below minimum floor
        A-->>W: Fail action
    else Valid floor
        P->>P: Write version to GITHUB_OUTPUT
        P-->>A: Return version
        A->>T: Setup Terraform at floor version
        A->>T: Run terraform init -backend=false
        T-->>A: Init complete
        A->>T: Run terraform validate
        T-->>A: Validation result
        A-->>W: Complete action
    end
    W-->>U: CI result (pass/fail)

    Note over U,CI: Tests run locally via unittest
    P->>P: Execute unit tests (no container)
    P->>D: End-to-end tests with Docker
Loading

Re-trigger cubic

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 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants