Repository navigation
fix(iac/federation): ship the role module inside the Azure Terraform bundle so terraform init works - #1440
Conversation
…bundle The Azure bundle's main.tf calls the custom reservation-purchaser role module at ../../../../terraform/modules/iam/azure/cudly-reservation-role, but the bundle zips only iac/federation/azure-target/terraform, so `terraform init` on the unzipped bundle failed with "Unreadable module directory" (LeanerCloud#1431). The module is now embedded from its one source of truth (terraform/ embed.go) and written to terraform/modules/cudly-reservation-role in the zip, and the bundle's copy of main.tf points at ./modules/ cudly-reservation-role. The repo's main.tf, the host-side callers and scripts/check-azure-role-parity.sh keep using the shared module, so the customer role cannot drift from the audited one. If the call in main.tf stops matching, building the bundle fails rather than shipping the broken path. Tests: every bundle's local module sources must resolve inside the zip, and the bundled role module must be byte-identical to the shared one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Builds each bundle through the API handler, writes it to an empty directory and runs `terraform get` there, which is where a module call that leaves the bundle fails (LeanerCloud#1431). It needs no network, so it runs under the package's no-network guard. With CUDLY_TEST_TERRAFORM_REGISTRY=1 it also runs `terraform init -backend=false` and `terraform validate`, which download providers from the public registry. Skips when terraform is not on PATH. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reached
This review includes 5 billable files and costs up to $1.25. Or wait 58 minutes for your next included review. View limit details
Comment |
|
Opened at head |
|
Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE Blocking:
Non-blocking:
Attacked:
Body-stale:
Reviewer: rC2. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths. |
|
Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE Blocking:
Non-blocking:
Attacked:
Body-stale:
Reviewer: rC3. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths. |
|
Local verification at |
|
Merged by IAmJSD's agent at IAmJSD's request (IAmJSD is acting owner while the maintainers are away), under the bar IAmJSD set: 2 approving independent review(s) on |
…undles (#1467) * refactor(iacbundle): share the bundle Terraform writer between API and script The API's federation bundle ships the out-of-tree Azure role module (#1431), but the helper that does it is unexported in internal/api, so scripts/generate-federation-iac.go cannot use it and still builds an Azure bundle that calls a module outside the zip (#1442). Move the writer, the module list and the "local module sources resolve inside the zip" check into a small package both builders can import. It reads any fs.FS so the script can pass its on-disk modules directory, and skips hidden files and directories so a developer's .terraform/ or lock file never lands in a zip. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(api): build the bundle Terraform through internal/iacbundle Use the shared writer for the API's federation bundles so the API and the developer script cannot drift apart again. Behaviour is unchanged: the embedded FS has no hidden files, and the module set and rewrite are the same. The bundle test keeps running every combo through the shared unresolved-module check; the guard's own self-test moved with it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(scripts): ship the Azure role module in generate-federation-iac bundles --format bundle --target azure zipped only iac/federation/azure-target/ terraform, whose main.tf calls ../../../../terraform/modules/iam/azure/ cudly-reservation-role, so `terraform init` on the unzipped bundle failed with "Unreadable module directory". Build the Terraform part through internal/iacbundle, as the API does since #1440: the module ships at terraform/modules/cudly-reservation-role and the call points at it. Like the API's bundle it now also carries the target's terraform/tests/ directory. The new test runs every bundle the script builds through the same resolves-inside-the-zip check as the API's bundles; the Azure case fails on main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Closes #1431
Root cause
iac/federation/azure-target/terraform/main.tfcalls the custom reservation-purchaser role module at../../../../terraform/modules/iam/azure/cudly-reservation-role. The server embeds onlyiac/federation(iac/embed.go), andaddBundleTerraformzips onlyfederation/azure-target/terraform. The module directory was never in the zip, soterraform initon the unzipped Azure "Terraform bundle" failed withUnreadable module directory. This has been broken since #744.Fix
terraform/embed.go(new packageterraform) embedsmodules/iam/azure/cudly-reservation-role/*.tf.go:embedcan't reach outsideiac/, so the module is embedded from the directory that owns it, and the repo still has a single copy.internal/api/federation_bundle_modules.go:addBundleTerraformDirreplaces the oldaddDirToZipcall for the bundle's Terraform dir. For the Azure bundle it writes the module toterraform/modules/cudly-reservation-role/and rewrites the call in the bundledmain.tfto./modules/cudly-reservation-role. If no.tffile contains the expected source string any more, building the bundle returns an error instead of shipping a broken path. The other bundles are unchanged.terraform/modules/.main.tf, the host-side callers (terraform/environments/azure/ci-cd-permissions, the container-apps module) orscripts/check-azure-role-parity.sh. They all still use the shared module. That's also why I chose embed-and-rewrite over keeping a second copy underiac/federation.Tests
TestFederationBundles_LocalModuleSourcesResolveInsideZip(unit): renders every federation bundle throughgetFederationIaCand checks that every localmodulesourcein every.tffile points at a directory of.tffiles inside the zip.TestUnresolvedLocalModules_DetectsEscapeproves this check can fail.TestAzureBundle_RoleModuleMatchesSharedModule(unit): the bundle ships exactly the shared module's.tffiles, byte for byte.TestFederationBundles_TerraformInitFromUnzippedBundle(-tags integration): writes each rendered bundle to an empty temp dir and runsterraform getinterraform/, which is where the bug fails. It needs no network, so it runs under the package's no-network guard (internal/api tests call real AWS Cost Explorer and probe IMDS #855). WithCUDLY_TEST_TERRAFORM_REGISTRY=1it also runsterraform init -backend=falseandterraform validate, which download providers from the public registry, so those runs get the environment without the guard's proxy variables. It skips whenterraformisn't on PATH. Nothing is planned or applied.Verification
All local, linux/amd64, Terraform 1.10.5.
origin/main6b586cc:go test ./internal/api/ -run 'LocalModuleSources|RoleModuleMatches': FAIL, withterraform/main.tf: ../../../../terraform/modules/iam/azure/cudly-reservation-roleunresolved, and the bundle has no role module files.go test -tags integration ./internal/api/ -run TestFederationBundles_TerraformInitFromUnzippedBundle: azure-target FAILs withError: Unreadable module directory. The other four bundles pass.CUDLY_TEST_TERRAFORM_REGISTRY=1 go test -tags integration ... -run TestFederationBundles_TerraformInitFromUnzippedBundle:terraform init -backend=falseandterraform validatepass for all five unzipped bundles (aws-cross-account, aws-target, azure-target, gcp-sa-impersonation, gcp-target). Total 310 s, mostly provider downloads.go build ./... && go vet ./... && go vet -tags integration ./... && (cd tests/e2e && go vet -tags e2e ./...): OK.golangci-lint run ./internal/api/... ./terraform/ ./iac/..., with and without--build-tags integration: 0 issues.gocyclo -over 10on the new non-test files: nothing.go test -race -short -count=1 ./internal/api/... ./terraform/ ./iac/...: ok.go test -race -tags integration -count=1 ./internal/api/ ./terraform/ ./iac/: ok (internal/api 384 s).scripts/check-azure-role-parity.sh: OK.scripts/test-azure-role-parity.sh: 78 passed. Secrets scan on the changed files: clean.scripts/test-git-secrets-allowlist.sh: 183 PASS.Not verified:
🤖 Generated with Claude Code