Skip to content

Commit dbf1682

Browse files
committed
feat!: clear the deferred backlog
- github-res-repository: hardcode `auto_init` so a new repository has a branch for `github_branch_default`; ignored, so adopting an existing repo is a no-op - azure-res-policy-set-definition: select the resource type from the scope, since `management_group_id` is removed in azurerm v5 - fortios-utl-network-cidr and both system-interface modules: a /31 reports RFC 3021 semantics — two usable hosts, not zero and an inverted range - vap-nac: add `triggers_replace` so a changed binding is re-applied - fortios-ptn-fortigate-system-settings: drop `ntp.interfaces`; listeners are owned by fortios-ptn-fortigate-system-ntp-interface - managedswitch-ports: move the `port/` submodule under `modules/` Documentation no longer restates what the code declares, so a release or a provider bump does not mean editing every file that mentions a version: - READMEs pin `?ref=vX.Y.Z`, explained once in usage.md - version bullets and the provider table give way to `versions.tf`; non-version requirements such as the RBAC a module needs are kept - README input and output tables give way to `variables.tf` and `outputs.tf`, which carry a description on every entry and are CI-enforced to - the empty "Known deferred work" and the stale "Outstanding repository work" sections are removed BREAKING CHANGE: - azure-res-policy-set-definition at management-group scope recreates the initiative and every assignment referencing it; see docs/modules/azure.md - vap-nac replaces each binding once, rebinding live VAPs - a /31 changes ipv4_usable_count, _first, _last and _usable_range - ntp.interfaces is no longer accepted - consumers sourcing managedswitch-ports/port move to modules/port
1 parent 0613df0 commit dbf1682

43 files changed

Lines changed: 402 additions & 756 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CLAUDE.md

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -69,20 +69,16 @@ These are the ones that cause real damage when ignored. Each links to the reason
6969
addressable by git ref, so external callers reach them directly.
7070
`azure-res-network-privatednszone/modules/vnet-link` reads as dead code and is not.
7171
([why](docs/conventions.md#nested-submodules))
72-
3. **Do not fix the deferred items in passing.** They were reviewed and postponed because the fix is
73-
riskier than the defect — resource-address changes, live-device writes, values callers already
74-
consume. They need a version tag and consumer coordination.
75-
([the list](docs/contributing.md#known-deferred-work))
76-
4. **Keep consumer identifiers out.** Public repo: no customer names, real hostnames, switch or SSID
72+
3. **Keep consumer identifiers out.** Public repo: no customer names, real hostnames, switch or SSID
7773
names, domains, serials, directory object IDs, or downstream filesystem paths. Use `example.com`,
7874
`core-sw-01`, `corp-nac`, `rg-example`.
79-
5. **Never commit `.terraform.lock.hcl`.** Gitignored on purpose — lock files in reusable modules
75+
4. **Never commit `.terraform.lock.hcl`.** Gitignored on purpose — lock files in reusable modules
8076
cause cross-platform checksum mismatches. Consumers pin their own. Same for state and tfvars.
81-
6. **No `provider` blocks inside modules.** `required_providers` in `versions.tf`; the consumer
77+
5. **No `provider` blocks inside modules.** `required_providers` in `versions.tf`; the consumer
8278
configures and authenticates. (The one exception in the tree is under `examples/`.)
83-
7. **Every provider constraint keeps an upper bound below the next major.**
84-
([matrix](docs/usage.md#version-requirements))
85-
8. **`GitVersion.yaml` and `cliff.toml` diverge on purpose — keep the anchoring identical.**
79+
6. **Every provider constraint keeps an upper bound below the next major.**
80+
([why](docs/usage.md#version-requirements))
81+
7. **`GitVersion.yaml` and `cliff.toml` diverge on purpose — keep the anchoring identical.**
8682
`cliff.toml` flags all three breaking markers (`<type>!:`, a `BREAKING CHANGE:` footer,
8783
`+semver: major`) in the notes; GitVersion majors on the last one only. That gap is the design.
8884
What must not drift is the `(?m)` anchoring: a marker on a body line has to behave the same in
@@ -105,7 +101,7 @@ These are the ones that cause real damage when ignored. Each links to the reason
105101
([why](docs/modules/fortios.md#perpetual-diffs-are-the-dominant-failure-mode))
106102
- **Tautological validation never fires.** `alltrue([for v in values(x) : v.scope != null || true])`
107103
is always true. Reference the second variable directly — cross-variable validation is legal and
108-
every module pins `>= 1.15`. ([more](docs/conventions.md#validation))
104+
`required_version` is high enough everywhere. ([more](docs/conventions.md#validation))
109105
- **Don't guess enum values you cannot verify.** The fortios provider does not publish accepted values
110106
for many attributes; a guessed `contains(...)` rejects working configurations.
111107
- **The Aegis tag contract is fixed.** Consumers key off the tag name and value. Read the banner in

README.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ environment composition here — modules are consumed from other repositories by
99

1010
```hcl
1111
module "example" {
12-
source = "git::https://github.com/emberstack/terraform.git//src/modules/<module-name>?ref=v0.1.0"
12+
source = "git::https://github.com/emberstack/terraform.git//src/modules/<module-name>?ref=vX.Y.Z"
1313
1414
# ...
1515
}
@@ -50,9 +50,9 @@ provider family behaves.
5050

5151
## Requirements
5252

53-
Terraform `>= 1.15` across every module. Provider constraints are declared per module in its
54-
`versions.tf` and all carry an upper bound below the next major, so a provider major release cannot
55-
reach you unannounced — see the [version matrix](docs/usage.md#version-requirements).
53+
Terraform and provider constraints are declared per module in its `versions.tf`, which is the
54+
authoritative source. All carry an upper bound below the next major, so a provider major release
55+
cannot reach you unannounced — see [Usage](docs/usage.md#version-requirements).
5656

5757
## Repository layout
5858

docs/contributing.md

Lines changed: 16 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -189,9 +189,9 @@ If the sweep ever resolves zero directories it fails rather than reporting a gre
189189

190190
### Why there is no `actions/cache`
191191

192-
`build` sets `TF_PLUGIN_CACHE_DIR` so all 75 `init` runs share one provider directory. That part is
193-
not optional — without it Terraform downloads a private copy of every provider per module, which runs
194-
to gigabytes and a dozen copies of azurerm.
192+
`build` sets `TF_PLUGIN_CACHE_DIR` so every `init` shares one provider directory. That part is not
193+
optional — without it Terraform downloads a private copy of every provider per module, which runs to
194+
gigabytes and a dozen copies of azurerm.
195195

196196
Persisting that directory between runs with `actions/cache` is a different question, and the answer
197197
is no. `TF_PLUGIN_CACHE_DIR` never evicts: on each provider bump the restore-key pulls the previous
@@ -257,8 +257,8 @@ terraform validate
257257

258258
### Sweep the whole tree
259259

260-
The same 75 directories CI covers — the 74 module directories plus the one `examples/basic`.
261-
**Set a plugin cache first** or you will download the same providers 75 times:
260+
The same directories CI covers — every module directory plus `examples/basic`.
261+
**Set a plugin cache first** or you will download the same providers once per directory:
262262

263263
```bash
264264
export TF_PLUGIN_CACHE_DIR="$HOME/.terraform.d/plugin-cache"
@@ -288,7 +288,7 @@ Re-run with `terraform init -upgrade`. This is a local artefact, never a reposit
288288
sibling in `src/modules/`. FortiGate modules need the `fortigate` platform segment.
289289
2. **Create all four files**`main.tf`, `variables.tf`, `outputs.tf`, `versions.tf` — even if one is
290290
short. Never fold them together.
291-
3. **Pin versions** in `versions.tf`: `required_version = ">= 1.15"` and a provider constraint with
291+
3. **Pin versions** in `versions.tf`: `required_version` matching the rest of the tree, and a provider constraint with
292292
both a floor and an upper bound below the next major.
293293
4. **Describe every variable and output.** The tree is at 100% description coverage; keep it there.
294294
5. **Validate inputs** — length bounds, enums, regex — to the azure/entra standard, except where the
@@ -302,8 +302,8 @@ Re-run with `terraform init -upgrade`. This is a local artefact, never a reposit
302302
8. `terraform fmt`, `init -backend=false`, `validate`, and
303303
[`check-docs.py`](#consistency-checks) — the last one is what verifies steps 2, 4 and 7.
304304

305-
A `README.md` and an `examples/basic/` are welcome but not required — coverage is 16 of 63 and 1 of 63
306-
respectively, so a new module without them is not an outlier.
305+
A `README.md` and an `examples/basic/` are welcome but not required — most modules have neither,
306+
so a new module without them is not an outlier.
307307

308308
Where a parent gains a submodule, add a **Submodules** section to the parent's README if it has one.
309309

@@ -326,24 +326,6 @@ triggers are scoped to `main`, so a branch builds once a pull request is open ag
326326
before. Nothing but `main` publishes, and the version `discovery` reports on a pull request is the
327327
plain `X.Y.Z` that merging would release — there are no prerelease versions in this repository.
328328

329-
## Known deferred work
330-
331-
These were reviewed and consciously postponed because the fix is riskier than the defect. They are
332-
**revisit, not won't-fix** — each needs to land behind a version tag with consumer coordination.
333-
Don't fix them in passing.
334-
335-
| Item | Why deferred | Cost while deferred |
336-
|---|---|---|
337-
| `management_group_id` on `azurerm_policy_set_definition` | Migrating to `azurerm_management_group_policy_set_definition` changes the resource address — destroys and recreates live policy set definitions, and any assignment referencing them, without `moved` blocks | **The `< 5.0` azurerm cap is load-bearing.** azurerm v5 is unreachable until this lands. Deprecation warning on every plan. |
338-
| `auto_init` on `github-res-repository` | Adding it may force repository replacement | The module only works against repositories that already have a commit — creating a brand-new empty repository fails at `github_branch_default` |
339-
| `triggers_replace` on the `vap-nac` NAC binding | Re-binds live VAPs on next apply | Binding changes are not re-applied automatically |
340-
| `ntp.interfaces` inside `ignore_changes` (`fortios-ptn-fortigate-system-settings`) | Ownership question against the sibling NTP module | Two modules can each believe they own NTP interface assignment |
341-
| `/31` semantics in `fortios-utl-network-cidr` | Changes a value callers already consume | A `/31` reports `0` usable hosts and an inverted usable range |
342-
343-
Two open structural items, both cosmetic and both breaking to fix: the submodule at `port/` instead of
344-
`modules/port/`, and `fortios-res-fortigate-wirelesscontroller-settings` being plural where the
345-
underlying resource is `fortios_wirelesscontroller_setting`.
346-
347329
## Releases
348330

349331
The pipeline tags and publishes a GitHub Release on every push to `main` **that changes something
@@ -450,13 +432,6 @@ The manual trigger takes no inputs on purpose. It re-runs the pipeline against t
450432
selected branch, derives the version the same way a push would, and the existing-tag check makes a
451433
dispatch against an already-released commit a no-op rather than a duplicate.
452434

453-
## Outstanding repository work
454-
455-
- **README coverage is 16 of 63 modules**; `examples/` coverage is 1 of 63.
456-
- **Validation coverage is uneven** — heavy in azure/entra, sparse in fortios.
457-
- Workflow actions are pinned to version tags rather than commit SHAs. This is a settled choice, not
458-
a gap — see [how actions are pinned](#how-actions-are-pinned) for the trade being made.
459-
460435
## Dependency updates
461436

462437
Renovate runs from this repository —
@@ -474,16 +449,15 @@ to the pinned action for the length of one step rather than left sitting on the
474449
| Branches | `chore/renovate/…`, commits prefixed `chore(deps):` |
475450

476451
**Provider constraint bumps are not automated.** A floor in `versions.tf` is a compatibility promise
477-
to consumers, not a number to keep current — raising `>= 4.81` forces every consumer to upgrade.
452+
to consumers, not a number to keep current — raising a floor forces every consumer to upgrade.
478453
With `rangeStrategy: in-range-only` Renovate never rewrites a declared range, so no provider pull
479454
request opens and nothing waits on the dashboard either.
480455

481-
That covers `hashicorp/azurerm` majors too. The [`< 5.0` cap](#known-deferred-work) in `versions.tf`
482-
is unchanged and still load-bearing, and moving to v5 is a deliberate manual edit: doing the
483-
`management_group_id` migration on `azure-res-policy-set-definition` first, with `moved` blocks, as
484-
a marked breaking change.
456+
That covers `hashicorp/azurerm` majors too. The azurerm major cap in `versions.tf` is unchanged, and moving
457+
to v5 stays a deliberate manual edit — the `management_group_id` migration that used to block it is
458+
done, but the remaining azurerm modules have not been checked against v5.
485459

486-
**Terraform itself is tracked in two places, differently.** The `required_version = ">= 1.15"` in every
460+
**Terraform itself is tracked in two places, differently.** The `required_version` in every
487461
`versions.tf` is picked up by the built-in `terraform` manager, so the same rule leaves it alone.
488462
`TERRAFORM_VERSION` in `pipeline.yaml` is a plain YAML value that no built-in manager can see, so a
489463
`customManagers` regex matches it and resolves it against `hashicorp/terraform` releases.
@@ -559,11 +533,9 @@ commit all survive; the proof does not.
559533

560534
### How actions are pinned
561535

562-
Every action is pinned to a **version tag, never a commit SHA**. Most sit on a major tag
563-
(`actions/checkout@v7`, `actions/stale@v10`, `dorny/paths-filter@v4`, `hashicorp/setup-terraform@v4`)
564-
and pick up patches silently. The rest pin an exact version — `gittools/actions@v4.7.0`,
565-
`renovatebot/github-action@v46.1.20`, `Mattraks/delete-workflow-runs@v2.1.0` — and get a Renovate pull
566-
request for each release instead.
536+
Every action is pinned to a **version tag, never a commit SHA**. Most sit on a major tag and pick up
537+
patches silently; the rest pin an exact version and get a Renovate pull request for each release
538+
instead. The workflows themselves are the list.
567539

568540
Tags are mutable, so this is a deliberate trade: readable diffs and a legible upgrade history, against
569541
trusting each action's owner not to move a tag underneath us. It also means the `sha_pinning_required`

docs/conventions.md

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ module — see [Usage](usage.md#providers-are-the-callers-responsibility).
5858

5959
Three optional additions sit alongside them:
6060

61-
- **`README.md`** — usage, Inputs, Outputs, Requirements, Notes. Coverage is currently 16 of 63
62-
modules, so don't assume one exists.
61+
- **`README.md`** — usage, Inputs, Outputs, Notes. Most modules do not have one
62+
yet, so don't assume one exists.
6363
- **`examples/basic/`** — sources the parent with `source = "../../"` and *does* declare a `provider`
6464
block. Only `entra-ptn-group-collection` has one today; treat examples as encouraged rather than
6565
established.
@@ -75,9 +75,6 @@ Submodule paths are addressable by git ref exactly like top-level modules. **A s
7575
in-repo caller is not dead code** — external consumers reach it directly. Never delete or re-address
7676
one based on in-repo reference count.
7777

78-
There is one layout exception: `fortios-ptn-fortigate-switchcontroller-managedswitch-ports` puts its
79-
child at `port/` rather than `modules/port/`. Match `modules/<child>/` in new code.
80-
8178
## Variable style
8279

8380
Collection inputs are **maps of objects keyed by a stable identifier**, using
@@ -108,7 +105,7 @@ Two things to watch:
108105

109106
- **Tautologies never fire.** `condition = alltrue([for v in values(x) : v.scope != null || true])` is
110107
always true. If a check needs a second variable, reference it directly — cross-variable validation
111-
has been legal since Terraform 1.9 and every module here pins `>= 1.15`.
108+
is legal at the `required_version` every module declares.
112109
- **Don't validate enums you cannot verify.** The `fortios` provider does not publish its accepted
113110
values in the schema for many attributes. A guessed `contains(...)` list rejects working
114111
configurations. Validate what you can prove.
@@ -150,7 +147,7 @@ around, an ordering dependency invisible in the graph.
150147
# =============================================================================
151148
```
152149

153-
A banner is **not** a mandatory file header — 35 of 74 module `main.tf` files carry one. Add a banner
150+
A banner is **not** a mandatory file header; roughly half the module `main.tf` files carry one. Add a banner
154151
when you have something non-obvious to say. A title-only rule is fine as a section marker in a long
155152
multi-resource file; what makes a banner noise is a body that restates the resource type.
156153

docs/modules/azure.md

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Azure
22

3-
17 modules on `hashicorp/azurerm` (`>= 4.81, < 5.0`), plus one nested submodule.
3+
17 modules on `hashicorp/azurerm`, plus one nested submodule.
44

55
Input shapes mirror [Azure Verified Modules](https://azure.github.io/Azure-Verified-Modules/) where an
66
AVM equivalent exists — `name`, `resource_group_name`, `tags`, `role_assignments` — and add what AVM
@@ -96,11 +96,36 @@ Choose between it and the collection pattern by ownership, not by count:
9696
Nothing in this repository sources `vnet-link`, and that means nothing — submodule paths are
9797
addressable by git ref, so external consumers reach it directly.
9898

99-
## Known issues
99+
## Migrating a management-group-scoped initiative
100100

101-
`azure-res-policy-set-definition` uses `management_group_id`, which azurerm v5.0 removes. This is why
102-
the provider constraint is capped below 5.0, and the migration is
103-
[deliberately deferred](../contributing.md#known-deferred-work) — it changes the resource address.
101+
`azure-res-policy-set-definition` picks its resource type from the scope: `management_group_id` set
102+
gives `azurerm_management_group_policy_set_definition`, null gives `azurerm_policy_set_definition`.
103+
The split exists because `management_group_id` on the latter is deprecated and removed in azurerm
104+
v5.0. Both types carry identical schemas, so no input or output changes shape.
104105

105-
`azure-ptn-policy-aegis-shield-tag-protection` inherits the `management_group_id` deprecation
106-
through the `azure-res-policy-set-definition` module it calls.
106+
Changing scope type changes the Terraform address. The two types address the same ARM resource ID,
107+
but azurerm does not implement `MoveState` for the pair — a `moved` block fails with *"Move Resource
108+
State Not Supported"* — so the module deliberately ships none.
109+
110+
**By default the initiative is destroyed and recreated**, along with every assignment referencing it.
111+
The resource itself comes back identical, but the assignment is **unenforced for the length of the
112+
apply**. For a deny-effect initiative that is a real gap: schedule it, and do not run it as a
113+
side effect of an unrelated change.
114+
115+
If you cannot accept that gap, re-address the state instead. Because the ARM resource ID is
116+
unchanged, this is a state-only operation and the plan afterwards is empty. Take a state backup
117+
first, then:
118+
119+
```bash
120+
terraform state rm 'module.<path>.azurerm_policy_set_definition.this'
121+
```
122+
123+
```bash
124+
terraform import 'module.<path>.azurerm_management_group_policy_set_definition.management_group[0]' '/providers/Microsoft.Management/managementGroups/<mg>/providers/Microsoft.Authorization/policySetDefinitions/<name>'
125+
```
126+
127+
Then plan. A correct migration reports **no changes** — nothing is created or destroyed in Azure.
128+
A plan that still shows a destroy means the import did not land; do not apply it.
129+
130+
The azurerm major cap stays in place for now. This module no longer blocks it, but the other
131+
azurerm modules have not been checked against v5.

docs/modules/entra.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Entra ID
22

3-
4 modules on `hashicorp/azuread` (`>= 3.9, < 4.0`), plus two nested submodules.
3+
4 modules on `hashicorp/azuread`, plus two nested submodules.
44

55
## Modules
66

0 commit comments

Comments
 (0)