feat: add terraform compliance workflow - #1045
Conversation
Correct the reusable workflow filename and let the test branch own the checker checkout ref instead of requiring callers to pass it.
Move the checker to a top-level domain directory, sparse-check it out from the reusable workflow, add category and summary logging, support optional verbose discovery output, emit GitHub Actions error annotations, and add the checker test suite.
Pin the centrally owned checker to Python 3.12 and make verbose compliance output the default without exposing runtime controls to callers.
Reject empty Terraform directory configuration with exit code 2 and an Actions annotation, and report nonexistent configured paths explicitly so workflow misconfiguration cannot pass silently.
Broaden pinned module-source refs to accept bare and product-prefixed semver tags for dependencies outside CC008's own tag convention; replace substring version matching with real semver lower-bound parsing so any constraint form (>, >=, ~>, bare pin) allowing >= 1.0.0 passes and upper-bound-only constraints correctly fail; add product module mandatory variables (juju_controller, proxy, logging-config, risk) per spec. Document the providers.tf and provides/requires deviations in the README.
…ames Refs have no required shape in Terraform/git, so a semver-based regex cannot reliably distinguish a pinned tag from a floating branch. Replace shape validation with a small denylist of conventional default/floating branch names (main, master, trunk, develop, development, head), matched case-insensitively; any other ref is treated as pinned regardless of naming. Also correct the README's explanation of terraform.tf vs providers.tf: reusable CC008 modules are never root modules, so they have no provider configuration to put in providers.tf.
Add --check <slug> and --list-checks to cc008_check.py to run a single check category. Split the reusable workflow's single "Run CC008 compliance check" step into six named steps, one per category (required-files, terraform-configuration, variable-ordering, output-ordering, module-interface, module-sources), each continue-on-error so every category runs and reports independently, with a final step failing the job if any category reported violations.
Add strategy.matrix over inputs.terraform-directories (mirroring terraform_modules_test.yaml) so each configured module directory runs in its own job with its own set of check steps, instead of one job checking every module together. Each matrix job's step summary now correctly reports a single module.
The matrix multiplied fixed per-job overhead (checkout x2, setup-python, pip install, ~30-50s) by the number of configured module directories, for check work that costs under a second total across every category and module. Revert to a single job that runs each check category once against every configured directory (measured: all six categories across two modules complete in under 1s of actual Python execution time). Per-check steps are kept since they add negligible time and give clear per-category pass/fail in the Actions UI.
The final "Fail if any check reported violations" step only said "one or more checks failed" without saying which. List each failed check category by name using each step's outcome, so the failure message is actionable without scrolling up through every step.
…ng failures continue-on-error: true marked every check step's conclusion as success even when cc008_check.py exited non-zero, hiding real failures behind a neutral icon and requiring a separate summary step to detect anything went wrong. Replace it with if: always() on each step after the first: a step without continue-on-error genuinely fails (red X, job conclusion becomes failure) when the checker reports violations, while if: always() ensures later check steps still run even after an earlier one fails. Each step's own output already reports the exact violations, so the extra summary step is no longer needed and is removed.
Remove the GITHUB_ACTIONS environment guard around the ::error annotation lines. The annotation is purely additive to the normal violation output (which prints regardless); gating it added a branch for no functional benefit, since the workflow command syntax is harmless noise outside Actions. Drop the now-unused os import.
juju_controller is not a required product-module input per CC008; drop it from MANDATORY_PRODUCT_VARIABLES, leaving logging-config, proxy, and risk.
Add classify_module_type() to detect three CC008 module categories instead of two: a module with no module blocks is "charm"; a module that composes other modules is "product" if it also defines at least one resource/data block that ties components together (juju_model, juju_secret, juju_integration, juju_offer), otherwise it is "component" (bundles charm modules without such tying resources). check_interface now dispatches on module_type and enforces the mandatory Component-module interface (model_uuid variable, components output) alongside the existing charm and product rules. juju_model presence alone was tried first and rejected: CC008 lets a Product module take model_uuid purely as an input rather than creating the model itself, which this repo's own terraform/product module does; verified real gateway modules classify unchanged (charm, product) and a synthetic Component module fixture (composes modules, no tying resources, model_uuid + components) both passes when compliant and correctly fails when components is missing.
Restore terraform-compliance/tests/test_terraform_modules_compliance.py from the last commit before it was deleted (a2ce5f3 "cleanup"), then update it for the API changes made since: is_product_module -> is_composed_module/classify_module_type (three-way charm/component/product classification), check_interface(..., product: bool) -> check_interface(..., module_type: str), and MANDATORY_PRODUCT_VARIABLES no longer including juju_controller. Added tests for classify_module_type's three outcomes and for the new Component-module mandatory interface (model_uuid variable, components output). 43 tests pass; verified real gateway modules (charm, product) still behave as before.
Add ModuleType(str, Enum) with CHARM/COMPONENT/PRODUCT members, replacing the plain "charm"/"component"/"product" string literals used by classify_module_type(), check_interface(), and ModuleReport.module_type. Inherits str so existing string comparisons (module_type == "charm") and f-string formatting (report.module_type in print output) keep working unchanged. Verified: ruff clean, 43 existing unit tests pass unmodified, and both real gateway modules (charm, product) print identical output to before.
Replace the six MANDATORY_*_VARIABLES/MANDATORY_*_OUTPUTS module-level constants and check_interface's three near-identical branches with a single MANDATORY_INTERFACES: dict[ModuleType, MandatoryInterface] and one generic loop. Adding a new module type going forward means adding a ModuleType member and a MANDATORY_INTERFACES entry, not another check_interface branch. Verified: ruff clean, all 43 unit tests pass unmodified, both real gateway modules produce byte-identical output to before.
_emit_github_error had only one call site and its logic wasn't reused consistently: the "no directories provided" annotation skipped the %/CR/LF escaping entirely. Replace it with a small _escape_annotation() helper used by both annotation emission sites, removing the unnecessary single-purpose wrapper while fixing the escaping inconsistency. Verified: ruff clean, 43/43 tests pass.
cc008_check.py already runs all 6 check categories in a single invocation and reports a per-category PASS/FAIL breakdown plus a summary. Running it 6 times with --check filters (each wired with id + if: always() just to keep every category visible after a failure) added workflow complexity without adding coverage. Replace the 6 steps with one "Check CC008 compliance" step running the checker with no --check filter; the verbose per-category output and GitHub annotations are unchanged. --check/--list-checks remain available on the CLI for local/manual debugging. Verified against the real gateway-api-integrator-operator fixtures: charm module passes all 6 categories, product module fails module-interface with the expected logging-config/proxy/risk violations (unchanged from before), same GitHub annotations emitted. ruff clean, 43/43 tests pass (test suite exercises cc008_check.py directly, unaffected by the workflow change).
…ules
check_interface previously only checked that mandatory variable/output names existed, saying nothing about whether e.g. model_uuid is actually required (no default) or units' default matches the spec. Add per-variable rules to MANDATORY_INTERFACES:
- TypeFamily: a coarse string/number/bool/collection classification of a variable's `type` expression, parsed from python-hcl2's `type` string (handles bare keywords and `${map(...)}`/`${object({...})}` wrapped forms). Deliberately a broad bucket, not an exact type match, to avoid the false-positive risk already seen with shape-based validation elsewhere in this checker (the ref-pinning check had to be walked back for the same reason).
- VariableRule.required: flags variables that must have no `default` (model_uuid across all module types that mandate it).
- VariableRule.default: flags variables whose default, if declared, must equal a specific value (units=1, config={}).
Terraform `output` blocks have no `type` field (inferred from `value`), so outputs remain presence-only checks — validating output type would require guessing from arbitrary value expressions, which is out of scope.
Also closes a known gap: extracted _block_label()/_block_body() helpers, replacing the 3x duplicated `next(key for key in block if key != "__is_block__")` pattern across block_names/_resource_type_labels/check_pinned_module_sources/_module_sources. Added variable_bodies() to expose parsed variable body dicts (name -> body) so check_interface can validate type/default without a second parse.
Fixed stale README content left over from earlier refactors in this session: removed the "product modules require juju_controller" checks-performed bullet (already removed from code, README never updated) and the "workflow runs each category as its own step" description (workflow was collapsed to one step in a previous commit).
Verified: ruff clean, 47/47 tests pass (4 new: model_uuid-with-default rejected, units wrong default rejected, units wrong type family rejected, config map type passes as collection). Real gateway-api-integrator-operator fixtures produce identical PASS/FAIL output to every prior verification in this session (charm passes, product still fails only on the pre-existing missing logging-config/proxy/risk variables). Synthetic component-module fixture verified compliant passes, and a model_uuid-with-default variant is correctly flagged.
…8_spec module Split cc008_check.py into two files: - cc008_spec.py: the single source of truth for *what* CC008 requires - required_files, the terraform.tf provider source/version floor, the Product-module tying-resource types, the floating-ref denylist, and the mandatory variable/output interface per module type (with type family, required, and default constraints). Plain frozen dataclasses instantiated once as CC008_SPEC, no checking logic, no HCL parsing, no comparisons - the full CC008 contract this checker enforces can be read in this file alone. - cc008_check.py: unchanged behavior, now imports CC008_SPEC and reads from it (CC008_SPEC.required_files, .terraform_block, .tying_resource_types, .floating_ref_names, .module_interfaces) instead of holding those as module-level constants alongside the checking logic. This was a pure mechanical move: VariableRule, ModuleInterface (renamed from MandatoryInterface), TypeFamily, ModuleType, and NO_DEFAULT_CHECK (renamed from _NO_DEFAULT_CHECK, now a dedicated sentinel class with a readable repr instead of a bare object()) already existed as exactly this shape in cc008_check.py; they simply moved to their own module. check_interface, check_required_files, check_terraform_block, _defines_tying_resources, and check_pinned_module_sources now read from CC008_SPEC but their logic is byte-for-byte identical. Verified: ruff clean on both files, all 47 existing tests pass with zero test changes (confirming the public API - check_interface, variable_bodies, etc. - is unchanged). Real gateway-api-integrator-operator fixtures produce byte-identical output to every prior verification in this session (charm passes, product fails only on the pre-existing missing logging-config/proxy/ risk variables). The reusable workflow needs no change: its sparse-checkout already pulls the whole terraform-compliance/ directory, so cc008_spec.py is automatically included alongside cc008_check.py.
…ing_
cc008_check.py had a separate _TYPE_FAMILIES dict mapping Terraform type keywords ("map", "object", "list", ...) to TypeFamily.COLLECTION, duplicating knowledge that belongs on the enum itself. TypeFamily now overrides Enum._missing_ so TypeFamily("map"), TypeFamily("object"), etc. resolve directly to TypeFamily.COLLECTION without a lookup table living outside cc008_spec.py - keeping the full definition of what each type keyword means colocated with the enum in the spec module.
The keyword list moved to a module-level _COLLECTION_TYPE_KEYWORDS constant in cc008_spec.py rather than inside the Enum body, since Enum's metaclass would otherwise turn a leading-underscore class attribute into a spurious member. _type_family() in cc008_check.py now just calls TypeFamily(keyword) and catches ValueError for unknown keywords, no dict import needed.
Verified: ruff clean, 47/47 tests pass. Confirmed via direct instantiation that TypeFamily has exactly 4 real members (STRING, NUMBER, BOOL, COLLECTION) and TypeFamily("bogus") still raises ValueError cleanly. Real gateway-api-integrator-operator fixtures produce byte-identical output to every prior verification in this session.
…TypeFamily Tried embedding _COLLECTION_TYPE_KEYWORDS inside the TypeFamily Enum body using enum.nonmember (Python 3.11+) so the keyword set would live directly on the enum instead of as a sibling module-level constant. Reverted: this repo has no Python version pin, and `uv run --with ... python ...` (the exact command documented in README.md) resolved to a 3.10 interpreter in this environment, where `from enum import nonmember` raises ImportError - which would have broken the checker for anyone running the documented local command without explicitly pinning -p 3.12. Kept the module-level frozenset (unchanged behavior from the previous commit), with a comment recording why it isn't embedded, so this doesn't get re-attempted without re-discovering the same constraint. Verified: ruff clean, 47/47 tests pass via the exact unpinned `uv run --with python-hcl2==8.1.3 --with pytest pytest ...` command from README.md. Real gateway-api-integrator-operator module-interface check still passes.
Documentation-only pass: shrink the multi-paragraph module/class/function docstrings and inline comments to one-liners across cc008_spec.py, terraform_hcl.py, and cc008_check.py, and cut README.md from ~185 lines to ~55 (it documents a CI script - usage, a short checks summary, and the key deviations, without the long rationale essays). No code or behavior changes. Verified on Python 3.12: ruff clean, 60/60 tests pass, real gateway fixtures unchanged (charm PASS, product still fails only on the missing logging-config/proxy/risk).
… reasons The trimmed "known deviations" bullet wrongly implied provides/requires are optional for the same subordinate reason as units. They aren't: CC008 makes provides/requires mandatory only when the charm defines that relation (true for any charm, not just subordinates), whereas units is optional specifically because subordinate charms must omit it. Split into two separate bullets.
ad66007 to
6b0b2e3
Compare
There was a problem hiding this comment.
Pull request overview
Adds a CC008 Terraform module compliance checker (Python) and a reusable GitHub Actions workflow to run it in CI for configured Terraform module directories.
Changes:
- Introduces CC008 spec definitions, HCL parsing helpers, and a CLI compliance checker.
- Adds a reusable workflow (
workflow_call) to execute the checker with a provided list of module directories. - Adds a comprehensive pytest suite covering required files, terraform block validation, interface checks, and module source pinning.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
terraform-compliance/cc008_check.py |
Implements the CC008 compliance checks and CLI/reporting logic. |
terraform-compliance/cc008_spec.py |
Defines CC008 requirements (module types, required files, interfaces, pinning rules) as data. |
terraform-compliance/terraform_hcl.py |
Adds Terraform HCL loading and normalization helpers used by the checker. |
terraform-compliance/tests/test_terraform_modules_compliance.py |
Adds unit tests for the checker behavior and CLI outputs. |
terraform-compliance/requirements.txt |
Pins the python-hcl2 dependency for the checker. |
terraform-compliance/README.md |
Documents local usage, test invocation, and the set of enforced checks. |
.github/workflows/terraform_modules_compliance.yaml |
Adds a reusable workflow to run CC008 compliance checks in CI. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@f-atwi could you link an real run in the PR description? (= a repo using the workflow in the branch) |
@seb4stien done |
Test results for commit 05639f6Test coverage for 05639f6 Static code analysis report |
Test results for commit 05639f6Test coverage for 05639f6 Static code analysis report |
Test results for commit 05639f6Test coverage for 05639f6 Static code analysis report |
Test results for commit 05639f6Test coverage for 05639f6 Static code analysis report |
Test results for commit 05639f6Test coverage for 05639f6 Static code analysis report |
Test results for commit 05639f6Test coverage for 05639f6 Static code analysis report |
| @@ -0,0 +1,441 @@ | |||
| # Copyright 2026 Canonical Ltd. | |||
There was a problem hiding this comment.
To be renamed "terrafor_check.py" and the following one "terraform_spec.py" so that we can evolve them over time.
Applicable spec: CC008
Overview
Adds a terraform compliance workflow to ensure that the terraform modules align with the CC008 spec
An example of this workflow running on a PR:
canonical/gateway-api-integrator-operator#343
Rationale
Guards changes to be pushed from drifting away from the spec
Workflow Changes
Checklist