diff --git a/.github/workflows/tflint-unused.yml b/.github/workflows/tflint-unused.yml index b2be4f8f5..11c7e1c4e 100644 --- a/.github/workflows/tflint-unused.yml +++ b/.github/workflows/tflint-unused.yml @@ -1,4 +1,4 @@ -name: tflint-unused-declarations +name: terraform-lint on: pull_request: @@ -7,7 +7,7 @@ on: jobs: check: - name: Check for unused declarations + name: Lint changed Terraform runs-on: ubuntu-24.04 steps: - name: Checkout repository @@ -52,3 +52,15 @@ jobs: echo "::error::tflint found unused declarations in one or more changed modules (see groups above)." exit 1 fi + + # CI backstop for the local pre-commit hook. + - name: Check version pinning + if: steps.changed.outputs.dirs != '' + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + files="$(git diff --name-only "$BASE_SHA" "$HEAD_SHA" -- '*.tf' || true)" + [ -z "$files" ] && exit 0 + # shellcheck disable=SC2086 + ./scripts/check-version-pinning.sh $files diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 3a7551db7..246c08c4d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -23,6 +23,13 @@ repos: require_serial: true exclude: '\.terraform/|\.terragrunt-cache/' + - id: check-version-pinning + name: check version pinning + entry: scripts/check-version-pinning.sh + language: script + files: '\.tf$' + exclude: '\.terraform/|\.terragrunt-cache/' + - id: block-superpowers-files name: block superpowers files entry: >- diff --git a/README.md b/README.md index eea697ad0..6ed387a9b 100644 --- a/README.md +++ b/README.md @@ -30,7 +30,7 @@ Reference any module via its Git source, pinned to a release tag: ```hcl module "vpc" { - source = "git::https://github.com/nullplatform/tofu-modules.git//infrastructure/aws/vpc?ref=v6.11.0" + source = "git::https://github.com/nullplatform/tofu-modules.git//infrastructure/aws/vpc?ref=v6.19.1" # module inputs ... } @@ -46,6 +46,11 @@ tofu apply See the [latest releases](https://github.com/nullplatform/tofu-modules/releases) for available versions. +Several modules require you to pin a chart version, an image tag or a git ref, with no +default. **[VERSIONS.md](VERSIONS.md) lists every one of them with its current value and a +ready-to-paste block** — read it before your first apply rather than hunting the numbers down +one variable at a time. + ## Versioning Releases follow [Semantic Versioning](https://semver.org/) and are automated via [release-please](https://github.com/googleapis/release-please). See [CHANGELOG.md](CHANGELOG.md) for the full release history. diff --git a/VERSIONS.md b/VERSIONS.md new file mode 100644 index 000000000..b2352a36a --- /dev/null +++ b/VERSIONS.md @@ -0,0 +1,134 @@ +# Pinned versions + +Every version these modules deploy — Helm charts, container images, and the git refs the agent +clones — is listed here with the value to pin. + +For the version of *these modules*, see the +[releases](https://github.com/nullplatform/tofu-modules/releases). + +## Why it matters + +`latest` and branch names resolve at deploy time, not at apply time. A pod restart can pull a +different build with no change on your side and no diff to review. Every default below names +a specific release, so an upgrade is something someone decides. + +## What to pin + +Verified 2026-08-27. + +| Component | Current | Variable | Module | +| --- | --- | --- | --- | +| `nullplatform-base` chart | `2.44.0` | `nullplatform_base_helm_version` | `nullplatform/base` | +| `nullplatform-agent` chart | `2.37.0` | `nullplatform_agent_helm_version` | `nullplatform/agent` | +| `cert-manager` chart | `v1.21.1` | `cert_manager_version` | `infrastructure/commons/cert_manager` | +| `prometheus` chart | `29.27.0` | `prometheus_version` | `infrastructure/commons/prometheus` | +| `k8s-logs-controller` | `1.6.0` | `logging_controller_image_tag` | `nullplatform/base` | +| `controlplane-agent` | `0.9.2` | `control_plane_agent_image_tag` | `nullplatform/base` | +| `k8s-traffic-manager` | `1.8.0` | `agent_traffic_manager_tag` | `nullplatform/agent` | +| traffic manager (provider config) | `1.8.0` | `traffic_manager_version` | `container_orchestration/eks` | +| `scopes` repository | `v1.15.1` | `agent_repos_scope` | `nullplatform/agent` | + +**Read your cluster before copying these.** The rule is to pin what you are already running, +so the change stays functionally inert. Four of these were previously unpinnable and resolved +at deploy time, so what you run may not match the table: `cert_manager_version`, +`prometheus_version`, `logging_controller_image_tag`, and `traffic_manager_version` on eks. + +## Ready to paste + +```hcl +module "base" { + nullplatform_base_helm_version = "2.44.0" + logging_controller_image_tag = "1.6.0" + control_plane_agent_image_tag = "0.9.2" +} + +module "agent" { + nullplatform_agent_helm_version = "2.37.0" + image_tag = "0.9.2" + agent_repos_scope_tag = "v1.15.1" + agent_traffic_manager_tag = "1.8.0" + + agent_repos_extra = [ + "https://github.com/nullplatform/scopes-lambda.git#v0.3.1", + "https://github.com/nullplatform/scopes-static-files.git#v0.4.0", + ] +} + +# eks, aks and gke all take this +module "container_orchestration" { + traffic_manager_version = "1.8.0" +} + +module "cert_manager" { + cert_manager_version = "v1.21.1" +} + +module "prometheus" { + prometheus_version = "29.27.0" +} + +module "service_definition" { + # No value listed: repository_org and repository_name are configurable, so which spec + # repository you read is your choice and so is its ref. + repository_branch = "..." +} +``` + +To find what an install is actually running before changing anything: + +```bash +helm -n nullplatform-tools get values nullplatform-base +helm -n list -o json | jq -r '.[] | "\(.name)\t\(.chart)"' +kubectl -n nullplatform-tools get deploy \ + -o jsonpath='{range .items[*]}{.metadata.name}{"\t"}{.spec.template.spec.containers[*].image}{"\n"}{end}' +``` + +The traffic manager image is assembled from `agent_traffic_manager_tag` and published to the +agent as `TRAFFIC_CONTAINER_IMAGE`. `extra_envs` still takes precedence over it, so a digest +or a mirrored registry path can be passed the way it was before the tag was exposed. + +## Caveats + +**The scopes ref steps back.** `agent_repos_scope` used to point at `scopes.git#main`, and +`main` has moved past `v1.15.1`. Pinning the tag is deliberate — it is the ref named in the +migration request — but it is not the same tree the branch tip pointed at. + +**cert-manager and prometheus were not pinnable at all.** `cert_manager_version` existed but +was never wired to its `helm_release`, and `prometheus` had no version argument, so both +tracked whatever their chart repository served. + +**Not everything is covered yet.** The scopes and service-spec repositories are read through +eleven other paths, in `scope_definition`, `scope_definition_agent_association`, +`parameter_storage_definition` and `service_definition`, and those still default to a moving +branch. Pinning `agent_repos_scope` does not cover them: the agent clones the ref while the +definition modules read the branch. They are listed in +`scripts/version-pinning-baseline.txt` with the reason. + +**A name cannot prove immutability.** The checks below reject `latest`, `main`, `master` and +`HEAD`. A tag called `beta` or a branch called `develop` passes. Nothing distinguishes a +mutable ref from a fixed one by name alone. + +## Keeping this current + +There is no automation that bumps these numbers, on purpose. Bumping a documented version to +whatever is newest would put the drift back in documentation form, and it contradicts the rule +above about pinning what you already run. When a new version ships, someone decides and edits +this table. + +What is automated is the opposite direction: `scripts/check-version-pinning.sh` rejects a *new* +moving default, a repository URL pinned to a branch, or a `helm_release` with no `version`. It +runs in pre-commit and again as a step in the `terraform-lint` workflow, so skipping the local +hook does not skip the check. Deliberately deferred violations live in +`scripts/version-pinning-baseline.txt` with the reason; that file should only ever shrink. + +One trap worth knowing before bumping an image by hand: **`k8s-traffic-manager` publishes a +`v2.0.2` built 2026-02-09 while `1.8.0` was built 2026-07-29**, and `k8s-logs-controller` a +`v2.0.1` from that same February against a `1.6.0` from August. The higher version number is +the older build, from a line that was not continued. Compare build dates, not version numbers: + +```bash +tag=1.8.0; repo=nullplatform/k8s-traffic-manager +t=$(curl -s "https://public.ecr.aws/token/?scope=repository:$repo:pull" | jq -r .token) +curl -s -H "Authorization: Bearer $t" "https://public.ecr.aws/v2/$repo/tags/list" | jq -r '.tags[]' +``` + diff --git a/scripts/check-version-pinning.sh b/scripts/check-version-pinning.sh new file mode 100755 index 000000000..4c77edce9 --- /dev/null +++ b/scripts/check-version-pinning.sh @@ -0,0 +1,81 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Rejects a new moving version reference. See VERSIONS.md. +# +# Usage: check-version-pinning.sh ... + +BASELINE="$(dirname "$0")/version-pinning-baseline.txt" +FAILED=0 + +baselined() { + [ -f "$BASELINE" ] || return 1 + grep -qxF "$1" <(grep -v '^[[:space:]]*#' "$BASELINE" | grep -v '^[[:space:]]*$') 2>/dev/null +} + +for file in "$@"; do + [ -f "$file" ] || continue + case "$file" in + */.terraform/*|*/.terragrunt-cache/*) continue ;; + *.tf) ;; + *) continue ;; + esac + + while IFS='|' read -r key message; do + [ -n "$key" ] || continue + if baselined "$key"; then + continue + fi + printf ' %s\n %s\n' "$key" "$message" >&2 + FAILED=1 + done < <( + awk -v F="$file" ' + function emit(key, msg) { print F ":" key "|" msg } + + /^variable[[:space:]]+"/ { + match($0, /"[^"]+"/) + vname = substr($0, RSTART + 1, RLENGTH - 2) + invar = 1; next + } + invar && /^}/ { invar = 0; vname = ""; next } + + invar && $0 ~ /^[[:space:]]*default[[:space:]]*=[[:space:]]*"(latest|main|master|HEAD)"[[:space:]]*$/ { + emit(vname, "defaults to a moving reference. Pin it, or drop the default so the caller has to pin it.") + next + } + invar && $0 ~ /^[[:space:]]*default[[:space:]]*=[[:space:]]*".*#(latest|main|master|HEAD)"[[:space:]]*$/ { + emit(vname, "default pins a git ref to a moving branch. Expose the ref as its own variable without a default.") + next + } + invar && $0 ~ /^[[:space:]]*default[[:space:]]*=[[:space:]]*".*\/refs\/heads"?[[:space:]]*$/ { + emit(vname, "default hardcodes refs/heads, so a caller cannot pin to a tag without also rewriting the URL. Expose the ref namespace as a variable.") + next + } + + /^resource[[:space:]]+"helm_release"[[:space:]]+"/ { + n = $0 + sub(/^resource[[:space:]]+"helm_release"[[:space:]]+"/, "", n) + sub(/".*$/, "", n) + rname = n; inres = 1; hasver = 0; next + } + inres && $0 ~ /^[[:space:]]+version[[:space:]]*=/ { hasver = 1; next } + inres && /^}/ { + if (!hasver) { + emit("helm_release." rname, "has no version argument, so Helm resolves whatever the chart repository serves at apply time with no diff to review. Add version = var..") + } + inres = 0; next + } + ' "$file" + ) +done + +if [ "$FAILED" -ne 0 ]; then + { + echo "" + echo "Version pinning check failed." + echo "Fix the finding, or -- if the fix is deliberately deferred -- add the key to" + echo "scripts/version-pinning-baseline.txt with a comment saying why and where it is tracked." + } >&2 +fi + +exit $FAILED diff --git a/scripts/version-pinning-baseline.txt b/scripts/version-pinning-baseline.txt new file mode 100644 index 000000000..92cb25f64 --- /dev/null +++ b/scripts/version-pinning-baseline.txt @@ -0,0 +1,26 @@ +# Deliberately deferred version-pinning violations. Each group says why. +# This file should only ever shrink. +# +# Format: : + +# The scopes and service-spec repositories, read through ten paths across three +# modules, all defaulting to a moving branch. Fixable without a breaking change; +# deferred for its own review. +nullplatform/parameter_storage_definition/variables.tf:repository_parameter_storage_spec +nullplatform/parameter_storage_definition/variables.tf:repository_parameter_storage_spec_branch +nullplatform/scope_definition/variables.tf:repository_action_templates +nullplatform/scope_definition/variables.tf:repository_action_templates_branch +nullplatform/scope_definition/variables.tf:repository_scope_template +nullplatform/scope_definition/variables.tf:repository_scope_template_branch +nullplatform/scope_definition/variables.tf:repository_service_spec +nullplatform/scope_definition/variables.tf:repository_service_spec_branch +nullplatform/scope_definition_agent_association/variables.tf:repository_notification_channel +nullplatform/scope_definition_agent_association/variables.tf:repository_notification_channel_branch + +# Fixed by the "require an explicit version" PR, whose base does not contain this +# file. Remove these when it merges. +infrastructure/commons/cert_manager/main.tf:helm_release.cert_manager +infrastructure/commons/prometheus/main.tf:helm_release.prometheus +nullplatform/agent/variables.tf:agent_repos_scope +nullplatform/container_orchestration/eks/variables.tf:traffic_manager_version +nullplatform/service_definition/variables.tf:repository_branch