From cc6bdab4f24a04a235df00253f277ee3a18599b1 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Mon, 26 Oct 2026 05:23:47 +0000 Subject: [PATCH 1/2] fix(iac/federation): ship the role module inside the Azure Terraform 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" (#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 --- internal/api/federation_bundle_modules.go | 85 +++++++++++++ .../api/federation_bundle_modules_test.go | 117 ++++++++++++++++++ internal/api/handler_federation.go | 3 +- terraform/embed.go | 16 +++ 4 files changed, 220 insertions(+), 1 deletion(-) create mode 100644 internal/api/federation_bundle_modules.go create mode 100644 internal/api/federation_bundle_modules_test.go create mode 100644 terraform/embed.go diff --git a/internal/api/federation_bundle_modules.go b/internal/api/federation_bundle_modules.go new file mode 100644 index 000000000..0901592bc --- /dev/null +++ b/internal/api/federation_bundle_modules.go @@ -0,0 +1,85 @@ +package api + +import ( + "archive/zip" + "bytes" + "fmt" + "io/fs" + "strings" + + cudlyiac "github.com/LeanerCloud/cloud-commitments-platform/iac" + cudlytf "github.com/LeanerCloud/cloud-commitments-platform/terraform" +) + +// bundledModule is a Terraform module that a federation bundle calls from +// outside its own embedded directory. The bundle ships a copy of it under +// terraform/ and points the call at that copy, so `terraform init` +// works from the unzipped bundle (#1431). +type bundledModule struct { + repoSource string // the module source as written in the repo's .tf files + fsys fs.ReadFileFS // where the module's files are embedded + dir string // the module's directory in fsys + zipDir string // its directory in the zip, relative to terraform/ +} + +// bundleExtraModules returns the out-of-tree modules a bundle's Terraform +// calls. Only the Azure bundle has one: the custom reservation-purchaser role, +// shared with the host side and checked by scripts/check-azure-role-parity.sh. +func bundleExtraModules(target string) []bundledModule { + if target != "azure" { + return nil + } + return []bundledModule{{ + repoSource: "../../../../terraform/modules/iam/azure/cudly-reservation-role", + fsys: cudlytf.ReservationRoleModule, + dir: cudlytf.ReservationRoleModuleDir, + zipDir: "modules/cudly-reservation-role", + }} +} + +// addBundleTerraformDir writes the embedded Terraform directory srcDir to the +// zip under terraform/, followed by every module in mods, with each module +// call rewritten to the shipped copy. A module whose repoSource no .tf file +// calls is an error: the rewrite would otherwise silently stop applying after +// the call is moved. +func addBundleTerraformDir(zw *zip.Writer, srcDir string, mods []bundledModule) error { + used := make([]bool, len(mods)) + err := fs.WalkDir(cudlyiac.Modules, srcDir, func(p string, d fs.DirEntry, err error) error { + if err != nil || d.IsDir() { + return err + } + b, err := cudlyiac.Modules.ReadFile(p) + if err != nil { + return err + } + if strings.HasSuffix(p, ".tf") { + b = rewriteModuleSources(b, mods, used) + } + return addBytesToZip(zw, "terraform/"+strings.TrimPrefix(p, srcDir+"/"), b) + }) + if err != nil { + return err + } + for i, m := range mods { + if !used[i] { + return fmt.Errorf("no module under %s calls %q", srcDir, m.repoSource) + } + if err := addDirToZip(zw, m.fsys, m.dir, "terraform/"+m.zipDir); err != nil { + return fmt.Errorf("module %s: %w", m.zipDir, err) + } + } + return nil +} + +// rewriteModuleSources points each quoted repoSource in src at the module's +// copy in the zip, and marks the modules it rewrote in used. +func rewriteModuleSources(src []byte, mods []bundledModule, used []bool) []byte { + for i, m := range mods { + quoted := []byte(`"` + m.repoSource + `"`) + if bytes.Contains(src, quoted) { + src = bytes.ReplaceAll(src, quoted, []byte(`"./`+m.zipDir+`"`)) + used[i] = true + } + } + return src +} diff --git a/internal/api/federation_bundle_modules_test.go b/internal/api/federation_bundle_modules_test.go new file mode 100644 index 000000000..0fb741f72 --- /dev/null +++ b/internal/api/federation_bundle_modules_test.go @@ -0,0 +1,117 @@ +package api + +import ( + "os" + "path" + "path/filepath" + "regexp" + "sort" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// localModuleSourceRe matches a module source that is a local path. Registry +// and provider sources never start with "./" or "../". +var localModuleSourceRe = regexp.MustCompile(`(?m)^\s*source\s*=\s*"(\.\.?/[^"]+)"`) + +// unresolvedLocalModules returns "file: source" for every local module source +// in a bundle's .tf files that does not point at a directory of .tf files +// inside the bundle. terraform init fails on each of them with "Unreadable +// module directory" (#1431). +func unresolvedLocalModules(files map[string][]byte) []string { + tfDirs := map[string]bool{} + for name := range files { + if strings.HasSuffix(name, ".tf") { + tfDirs[path.Dir(name)] = true + } + } + var bad []string + for name, content := range files { + if !strings.HasSuffix(name, ".tf") { + continue + } + for _, m := range localModuleSourceRe.FindAllSubmatch(content, -1) { + if !tfDirs[path.Join(path.Dir(name), string(m[1]))] { + bad = append(bad, name+": "+string(m[1])) + } + } + } + sort.Strings(bad) + return bad +} + +func TestFederationBundles_LocalModuleSourcesResolveInsideZip(t *testing.T) { + for _, c := range federationBundleCombos { + t.Run(c.module, func(t *testing.T) { + files := renderBundleFiles(t, c.target, c.source, c.sourceCloud) + assert.Empty(t, unresolvedLocalModules(files), + "every local module source in the %s bundle must resolve to a directory inside the zip", c.module) + }) + } +} + +// TestUnresolvedLocalModules_DetectsEscape proves the guard above can fail: a +// source that leaves the bundle, or names a directory the zip lacks, is +// reported, and one that resolves inside the zip is not. +func TestUnresolvedLocalModules_DetectsEscape(t *testing.T) { + files := map[string][]byte{ + "terraform/main.tf": []byte(`module "a" { + source = "../../../../terraform/modules/iam/azure/cudly-reservation-role" +} +module "b" { + source = "./modules/missing" +} +module "c" { + source = "./modules/present" +} +terraform { + required_providers { + azurerm = { + source = "hashicorp/azurerm" + } + } +} +`), + "terraform/modules/present/main.tf": []byte(`resource "null_resource" "x" {}`), + } + assert.Equal(t, []string{ + "terraform/main.tf: ../../../../terraform/modules/iam/azure/cudly-reservation-role", + "terraform/main.tf: ./modules/missing", + }, unresolvedLocalModules(files)) +} + +// TestAzureBundle_RoleModuleMatchesSharedModule keeps the role module shipped +// in the Azure bundle identical to terraform/modules/iam/azure/ +// cudly-reservation-role, which the host side and scripts/check-azure-role- +// parity.sh use, so the customer role cannot drift from the audited one. +func TestAzureBundle_RoleModuleMatchesSharedModule(t *testing.T) { + const shared = "../../terraform/modules/iam/azure/cudly-reservation-role" + files := renderBundleFiles(t, "azure", "gcp", "gcp") + + want, err := filepath.Glob(filepath.Join(shared, "*.tf")) + require.NoError(t, err) + require.NotEmpty(t, want, "no .tf files in %s", shared) + + var got []string + for name := range files { + if strings.HasPrefix(name, "terraform/modules/cudly-reservation-role/") { + got = append(got, path.Base(name)) + } + } + sort.Strings(got) + wantNames := make([]string, 0, len(want)) + for _, f := range want { + wantNames = append(wantNames, filepath.Base(f)) + } + require.Equal(t, wantNames, got, "bundle must ship exactly the shared module's .tf files") + + for _, f := range want { + disk, err := os.ReadFile(f) + require.NoError(t, err) + assert.Equal(t, string(disk), string(files["terraform/modules/cudly-reservation-role/"+filepath.Base(f)]), + "bundled %s differs from %s", filepath.Base(f), f) + } +} diff --git a/internal/api/handler_federation.go b/internal/api/handler_federation.go index 0bed66911..28613f409 100644 --- a/internal/api/handler_federation.go +++ b/internal/api/handler_federation.go @@ -665,7 +665,7 @@ func buildAzureTemplateReadme(data federationIaCData, format string) string { // addBundleTerraform adds the Terraform module files and generated .tfvars to the zip. func addBundleTerraform(zw *zip.Writer, data federationIaCData, target, source, slug string) error { tfDir := bundleModuleDir(target, source) + "/terraform" - if err := addDirToZip(zw, cudlyiac.Modules, tfDir, "terraform"); err != nil { + if err := addBundleTerraformDir(zw, tfDir, bundleExtraModules(target)); err != nil { return fmt.Errorf("bundle: terraform dir: %w", err) } tfvarsTmpl, tfvarsName := bundleTfvarsSpec(target, source, slug) @@ -808,6 +808,7 @@ func buildBundleReadme(data federationIaCData, target, source string) string { sb.WriteString("After apply, set aws_auth_mode=workload_identity_federation and aws_role_arn in CUDly.\n") case target == "azure": sb.WriteString("Contents:\n terraform/ - Azure App Registration + federated-identity-credential Terraform module\n") + sb.WriteString(" terraform/modules/ - Custom reservation-purchaser role module called by terraform/main.tf\n") sb.WriteString(" terraform/*.auto.tfvars - Pre-filled variable values (auto-loaded by Terraform)\n\n") sb.WriteString("This uses true Workload Identity Federation — no certificate, no private key,\nno client secret is created or stored. CUDly's OIDC issuer signs a short-lived\nJWT on each Azure API call and Azure AD verifies it against the federated\ncredential provisioned here.\n\n") sb.WriteString("Deploy (Terraform):\n") diff --git a/terraform/embed.go b/terraform/embed.go new file mode 100644 index 000000000..1f0dbc54a --- /dev/null +++ b/terraform/embed.go @@ -0,0 +1,16 @@ +// Package terraform embeds the shared Terraform modules that ship inside the +// customer federation bundles built by internal/api. +package terraform + +import "embed" + +// ReservationRoleModule holds terraform/modules/iam/azure/cudly-reservation-role, +// which the Azure federation bundle calls. The bundle zips only +// iac/federation/azure-target/terraform, so the module is shipped alongside it +// from here (#1431). Dot files such as the lock file are not embedded. +// +//go:embed modules/iam/azure/cudly-reservation-role/*.tf +var ReservationRoleModule embed.FS + +// ReservationRoleModuleDir is the module's directory inside ReservationRoleModule. +const ReservationRoleModuleDir = "modules/iam/azure/cudly-reservation-role" From b9b735cf30deb12573c6789d932fed78529a4961 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Mon, 26 Oct 2026 05:23:48 +0000 Subject: [PATCH 2/2] test(iac/federation): run terraform on every unzipped federation bundle 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 (#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 --- ...ation_bundle_terraform_integration_test.go | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 internal/api/federation_bundle_terraform_integration_test.go diff --git a/internal/api/federation_bundle_terraform_integration_test.go b/internal/api/federation_bundle_terraform_integration_test.go new file mode 100644 index 000000000..113846300 --- /dev/null +++ b/internal/api/federation_bundle_terraform_integration_test.go @@ -0,0 +1,90 @@ +//go:build integration + +package api + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// registryOptInEnv opts in to the part of the test below that downloads +// providers from the public Terraform registry. +const registryOptInEnv = "CUDLY_TEST_TERRAFORM_REGISTRY" + +// TestFederationBundles_TerraformInitFromUnzippedBundle builds every federation +// bundle through the real API path, writes it to an empty directory, and runs +// terraform there as a customer would (#1431). Nothing else from the repo is +// reachable from that directory. Skips when terraform is not on PATH. +// +// `terraform get` installs the modules, which is where a module call that +// leaves the bundle fails ("Unreadable module directory"). It needs no +// network, so it runs under this package's no-network guard (#855). +// +// With CUDLY_TEST_TERRAFORM_REGISTRY=1 the test also runs `terraform init +// -backend=false` and `terraform validate`, which download the providers from +// the public registry. Those runs get the environment without the guard's +// proxy variables. Nothing is planned or applied. +func TestFederationBundles_TerraformInitFromUnzippedBundle(t *testing.T) { + tf, err := exec.LookPath("terraform") + if err != nil { + t.Skip("terraform not on PATH") + } + registry := os.Getenv(registryOptInEnv) == "1" + pluginCache := os.Getenv("TF_PLUGIN_CACHE_DIR") + if pluginCache == "" { + pluginCache = t.TempDir() + } + for _, c := range federationBundleCombos { + t.Run(c.module, func(t *testing.T) { + dir := t.TempDir() + for name, content := range renderBundleFiles(t, c.target, c.source, c.sourceCloud) { + dst := filepath.Join(dir, filepath.FromSlash(name)) + require.NoError(t, os.MkdirAll(filepath.Dir(dst), 0o755)) + require.NoError(t, os.WriteFile(dst, content, 0o600)) + } + tfDir := filepath.Join(dir, "terraform") + runTerraform(t, tf, tfDir, os.Environ(), "get", "-no-color") + if !registry { + return + } + env := append(withoutProxy(os.Environ()), "TF_PLUGIN_CACHE_DIR="+pluginCache) + runTerraform(t, tf, tfDir, env, "init", "-backend=false", "-input=false", "-no-color") + runTerraform(t, tf, tfDir, env, "validate", "-no-color") + }) + } + if !registry { + t.Logf("set %s=1 to also run terraform init and validate against the public registry", registryOptInEnv) + } +} + +func runTerraform(t *testing.T, tf, dir string, env []string, args ...string) { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) + defer cancel() + cmd := exec.CommandContext(ctx, tf, args...) + cmd.Dir = dir + cmd.Env = append(env, "TF_IN_AUTOMATION=1", "CHECKPOINT_DISABLE=1") + out, err := cmd.CombinedOutput() + require.NoError(t, err, "terraform %v in %s:\n%s", args, dir, out) +} + +// withoutProxy drops the proxy variables the no-network guard sets. +func withoutProxy(env []string) []string { + out := make([]string, 0, len(env)) + for _, kv := range env { + k, _, _ := strings.Cut(kv, "=") + switch strings.ToUpper(k) { + case "HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY": + continue + } + out = append(out, kv) + } + return out +}