fix(terraform): align provider version floors across modules#1242
fix(terraform): align provider version floors across modules#1242cristim wants to merge 2 commits into
Conversation
cristim
added
triaged
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (25)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
Adversarial reviewVerified the version-floor logic across every The 10 regenerated module lock files all pin to the versions the consuming roots apply (aws 5.100.0, google 5.45.2, azurerm 3.117.1) with multi-platform hashes. The UNSTABLE not from this PRThe Lint Code, Integration Tests, Security Scanning, and CI Success failures all exist on In-scope fixes pushed to fix/inf-08-fix (f5f5524)
Out-of-scope follow-up filed: #1291Same INF-08 unbounded- |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
|
Rebased onto origin/main (27fdb06). Clean 2-commit rebase, no conflicts. Gates: go build/vet exit 0. @coderabbitai review |
|
✅ Action performedReview finished.
|
|
Rebased onto origin/main (89658a3 -> a945344) which includes #1437 (tflint plugin cache + retry) and #1438 (trivy |
The azure root and all consumed modules pin azurerm ~> 3.0, but three
orphaned modules (registry/azure, monitoring/azure,
compute/azure/cleanup-function) pinned ~> 4.0, a mutually exclusive
range that would break terraform init the moment they are wired into
the environment root. AWS and GCP module floors were also mixed:
unbounded >= 5.0 in seven modules vs ~> 5.0 in the environment roots,
which let module-local lock files drift to provider majors (aws 6.33.0,
google 7.20.0, azurerm 4.70.0) that no consuming root ever applies.
Changes:
- Pin the three azure 4.x modules to ~> 3.0, matching the azure
environment root and every other azurerm consumer.
- Convert >= 5.0 to ~> 5.0 in registry/{aws,gcp}, monitoring/{aws,gcp},
compute/aws/{fargate,cleanup-lambda} and compute/gcp/cleanup-function.
- Pin the aws ci-cd-permissions bootstrap root to ~> 6.0 (its committed
lock file is already applied at 6.36.0; pinning to 5.x would force a
provider downgrade of live bootstrap state).
- Keep iam/azure/cudly-reservation-role at >= 3.0 with a comment: it is
consumed by both an azurerm 3.x root (environments/azure/
ci-cd-permissions) and an azurerm 4.x root (iac/federation/
azure-target), so its constraint must intersect both lines.
- Regenerate the ten module-local lock files under the aligned
constraints; they now select the same provider versions the consuming
environment roots actually apply (aws 5.100.0, google 5.45.2,
azurerm 3.117.1) with multi-platform hashes.
- Convert registry/azure's two azurerm 4.x-only scalar attributes
(trust_policy_enabled, retention_policy_in_days) to the 3.x nested
trust_policy/retention_policy blocks, emitted only for the Premium
SKU; same effective configuration, now valid under azurerm 3.x.
The orphaned modules themselves (registry/azure, monitoring/{aws,gcp,
azure}, the three cleanup modules) are kept: they implement the
staging-TTL cleanup and monitoring stacks with live Go code in
cmd/cleanup-lambda, and wiring them into the environment roots is a
deployment decision tracked separately.
Verified with terraform init -backend=false && terraform validate in
all ten modules and the five roots (aws/gcp/azure environments plus
aws and azure ci-cd-permissions); all pass with no root lock churn.
Closes #1188
…le validate exclusion Two follow-ups from the adversarial review on the INF-08 version-floor PR: - The bootstrap root constraint in `terraform/environments/aws/ci-cd-permissions/main.tf` was changed from `>= 5.0` to `~> 6.0` to match the live 6.36.0 lock, but the lock file's own `constraints` metadata still recorded `>= 5.0.0` (it self-heals on the next `terraform init`, but committing it stale leaves the file inconsistent with its source). Update the constraints line in `.terraform.lock.hcl` to `~> 6.0` so the metadata matches the source pin without waiting for a deploy to rewrite it. - The `.pre-commit-config.yaml` `terraform_validate` exclusion for `terraform/modules/compute/azure/cleanup-function/` and `terraform/modules/registry/azure/` was justified by a comment about `dynamic` blocks around scalar attributes being invalid HCL. The original PR specifically converted those scalar attributes back to azurerm 3.x nested `trust_policy`/`retention_policy` blocks emitted via `dynamic` (for registry/azure) and dropped the dynamic-around-scalar wrapper in cleanup-function. Both directories now validate clean under the pinned azurerm 3.117.1: confirmed by running `terraform init -backend=false && terraform validate` locally, and by `pre-commit run terraform_validate --all-files` passing without exclusions. Drop the exclusion (and the now-stale comment) so future regressions are caught by pre-commit instead of slipping through. No source-code or lockfile-version changes; provider versions stay at aws 6.36.0 and azurerm 3.117.1.
1 participant
Problem
Code-review finding INF-08 (#1188): the azure environment root and all consumed modules pin
azurerm ~> 3.0, while three orphaned modules (registry/azure,monitoring/azure,compute/azure/cleanup-function) pinned~> 4.0. The ranges are mutually exclusive, soterraform initwould fail the moment those modules are wired into the root. AWS and GCP floors were also mixed: seven modules used unbounded>= 5.0against the roots'~> 5.0, which let their module-local lock files drift to provider majors no consuming root ever applies (aws 6.33.0, google 7.20.0, azurerm 4.70.0).Fix
registry/azure,monitoring/azure, andcompute/azure/cleanup-functiontoazurerm ~> 3.0, matching the azure root and every other azurerm consumer (also drops the stale comment in monitoring/azure referencing a previously-committed 4.61.0 lock that no longer exists).>= 5.0to~> 5.0inregistry/{aws,gcp},monitoring/{aws,gcp},compute/aws/{fargate,cleanup-lambda}, andcompute/gcp/cleanup-function.environments/aws/ci-cd-permissionsbootstrap root to~> 6.0instead of>= 5.0: its committed lock is already applied at aws 6.36.0, so aligning it down to~> 5.0would force a provider downgrade of live bootstrap state. A comment documents the deliberate divergence from the workload roots.iam/azure/cudly-reservation-roleat>= 3.0with a comment: it is consumed by both an azurerm 3.x root (environments/azure/ci-cd-permissions) and an azurerm 4.x root (iac/federation/azure-target), so its constraint must intersect both provider lines..terraform.lock.hclfiles under the aligned constraints. They now select the same versions the consuming roots actually apply (aws 5.100.0, google 5.45.2, azurerm 3.117.1), with multi-platform hashes (linux/darwin, amd64/arm64).registry/azure's two azurerm 4.x-only scalar attributes (trust_policy_enabled,retention_policy_in_days) to the 3.x nestedtrust_policy/retention_policyblocks, emitted viadynamiconly for the Premium SKU. Same effective configuration; the module now validates under azurerm 3.117.1.Orphaned modules: kept, not deleted
Grep confirms nothing instantiates
registry/azure,monitoring/{aws,gcp,azure},compute/azure/cleanup-function,compute/aws/cleanup-lambda, orcompute/gcp/cleanup-function(no module block, workflow, script, or Makefile reference). They are deliberately kept rather than removed:cmd/cleanup-lambda; the GCP cleanup-function was hardened in 3c6c9f3).terraform_validateexclusion forregistry/azureandcompute/azure/cleanup-functionis tracked separately per its own comment and is untouched here; both directories now validate clean locally, so that exclusion can likely be dropped in its own cleanup.Test evidence
terraform init -backend=false && terraform validaterun in all 15 affected directories (10 modules + environments/{aws,gcp,azure} + aws and azure ci-cd-permissions roots): all validate clean. The azure 4.x-to-3.x downgrades were schema-checked against the real azurerm 3.117.1 provider via validate. No churn in any environment root lock file.Closes #1188