From 6b3d4377c3a50a2742eef13ceefc17565c8d3e00 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 2 Feb 2026 09:03:18 -0600 Subject: [PATCH] feat: archive modules in size order until limit is hit (#21773) Archiving modules attempts to save as many modules as it can before it hits the limit. Enabling the template as much as it can, rather than a hard failure. --- coderd/parameters_test.go | 12 +- coderd/util/xio/limitwriter.go | 4 + .../extending-templates/dynamic-parameters.md | 14 ++ provisioner/terraform/modules.go | 99 ++++++++- .../terraform/modules_internal_test.go | 209 +++++++++++++++++- provisioner/terraform/provision.go | 18 +- 6 files changed, 336 insertions(+), 20 deletions(-) diff --git a/coderd/parameters_test.go b/coderd/parameters_test.go index 07c00d2ef2..1229a61dc9 100644 --- a/coderd/parameters_test.go +++ b/coderd/parameters_test.go @@ -83,8 +83,9 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) { dynamicParametersTerraformSource, err := os.ReadFile("testdata/parameters/modules/main.tf") require.NoError(t, err) - modulesArchive, err := terraform.GetModulesArchive(os.DirFS("testdata/parameters/modules")) + modulesArchive, skipped, err := terraform.GetModulesArchive(os.DirFS("testdata/parameters/modules")) require.NoError(t, err) + require.Len(t, skipped, 0) setup := setupDynamicParamsTest(t, setupDynamicParamsTestParams{ provisionerDaemonVersion: provProto.CurrentVersion.String(), @@ -198,8 +199,9 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) { dynamicParametersTerraformSource, err := os.ReadFile("testdata/parameters/modules/main.tf") require.NoError(t, err) - modulesArchive, err := terraform.GetModulesArchive(os.DirFS("testdata/parameters/modules")) + modulesArchive, skipped, err := terraform.GetModulesArchive(os.DirFS("testdata/parameters/modules")) require.NoError(t, err) + require.Len(t, skipped, 0) c := atomic.NewInt32(0) reject := &dbRejectGitSSHKey{Store: db, hook: func(d *dbRejectGitSSHKey) { @@ -232,8 +234,9 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) { dynamicParametersTerraformSource, err := os.ReadFile("testdata/parameters/modules/main.tf") require.NoError(t, err) - modulesArchive, err := terraform.GetModulesArchive(os.DirFS("testdata/parameters/modules")) + modulesArchive, skipped, err := terraform.GetModulesArchive(os.DirFS("testdata/parameters/modules")) require.NoError(t, err) + require.Len(t, skipped, 0) setup := setupDynamicParamsTest(t, setupDynamicParamsTestParams{ provisionerDaemonVersion: provProto.CurrentVersion.String(), @@ -318,8 +321,9 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) { dynamicParametersTerraformSource, err := os.ReadFile("testdata/parameters/modules/main.tf") require.NoError(t, err) - modulesArchive, err := terraform.GetModulesArchive(os.DirFS("testdata/parameters/modules")) + modulesArchive, skipped, err := terraform.GetModulesArchive(os.DirFS("testdata/parameters/modules")) require.NoError(t, err) + require.Len(t, skipped, 0) setup := setupDynamicParamsTest(t, setupDynamicParamsTestParams{ provisionerDaemonVersion: provProto.CurrentVersion.String(), diff --git a/coderd/util/xio/limitwriter.go b/coderd/util/xio/limitwriter.go index 8357d5d97a..c5a806d8b8 100644 --- a/coderd/util/xio/limitwriter.go +++ b/coderd/util/xio/limitwriter.go @@ -41,3 +41,7 @@ func (l *LimitWriter) Write(p []byte) (int, error) { l.N += int64(n) return n, err } + +func (l *LimitWriter) Remaining() int64 { + return l.Limit - l.N +} diff --git a/docs/admin/templates/extending-templates/dynamic-parameters.md b/docs/admin/templates/extending-templates/dynamic-parameters.md index 96a4dcc694..d45323fdcb 100644 --- a/docs/admin/templates/extending-templates/dynamic-parameters.md +++ b/docs/admin/templates/extending-templates/dynamic-parameters.md @@ -830,3 +830,17 @@ Unless explicitly mentioned, no registry modules require Dynamic Parameters. Later in 2025, more registry modules will be converted to Dynamic Parameters to improve their UX. In the meantime, you can safely convert existing templates and build new parameters on top of the functionality provided in the registry. + +### "Module not loaded" errors when using Dynamic Parameters + +Dynamic Parameters require Terraform modules to be archived and stored in the database. Coder limits module archives to **20MB total** to prevent database bloat. If your template uses modules that exceed this limit, some modules will be unavailable for parameter declarations. + +**Symptoms:** + +You may see warnings in the provisioner logs: + +```text +[API] 2026-01-29 22:00:22.691 [warn] provisionerd-nixos-0.executor: some (or all) terraform modules were not archived, template will have reduced function skipped_modules=large:git::https://github.com/coder/large-module.git +``` + +If encountered, reduce the size of the module by removing unnecessary files. diff --git a/provisioner/terraform/modules.go b/provisioner/terraform/modules.go index 38bfd65e84..158fa2b70a 100644 --- a/provisioner/terraform/modules.go +++ b/provisioner/terraform/modules.go @@ -4,9 +4,11 @@ import ( "archive/tar" "bytes" "encoding/json" + "fmt" "io" "io/fs" "os" + "slices" "strings" "time" @@ -35,6 +37,11 @@ type module struct { Dir string `json:"Dir"` } +type moduleWithEstimatedSize struct { + *module + EstimatedSize int64 +} + type modulesFile struct { Modules []*module `json:"Modules"` } @@ -78,26 +85,49 @@ func getModules(files tfpath.Layout) ([]*proto.Module, error) { return filteredModules, nil } -func GetModulesArchive(root fs.FS) ([]byte, error) { +func GetModulesArchive(root fs.FS) ([]byte, []string, error) { + return GetModulesArchiveWithLimit(root, MaximumModuleArchiveSize) +} + +// GetModulesArchiveWithLimit returns the tar archive, the skipped modules, and an error if any. +func GetModulesArchiveWithLimit(root fs.FS, maxArchiveSize int64) ([]byte, []string, error) { modulesFileContent, err := fs.ReadFile(root, ".terraform/modules/modules.json") if err != nil { if xerrors.Is(err, fs.ErrNotExist) { - return []byte{}, nil + return []byte{}, []string{}, nil } - return nil, xerrors.Errorf("failed to read modules.json: %w", err) + return nil, []string{}, xerrors.Errorf("failed to read modules.json: %w", err) } var m modulesFile if err := json.Unmarshal(modulesFileContent, &m); err != nil { - return nil, xerrors.Errorf("failed to parse modules.json: %w", err) + return nil, []string{}, xerrors.Errorf("failed to parse modules.json: %w", err) } empty := true var b bytes.Buffer - lw := xio.NewLimitWriter(&b, MaximumModuleArchiveSize) + lw := xio.NewLimitWriter(&b, maxArchiveSize) w := tar.NewWriter(lw) + sized := make([]*moduleWithEstimatedSize, 0, len(m.Modules)) for _, it := range m.Modules { + sz, err := estimateModuleSize(root, it.Dir) + if err != nil { + return nil, []string{}, xerrors.Errorf("failed to estimate module size for %q: %w", it.Dir, err) + } + sized = append(sized, &moduleWithEstimatedSize{ + module: it, + EstimatedSize: sz, + }) + } + + // Sort modules by estimated size descending so that we skip the largest + slices.SortFunc(sized, func(a, b *moduleWithEstimatedSize) int { + return int(a.EstimatedSize - b.EstimatedSize) + }) + skippedModules := []string{} + + for _, it := range sized { // Check to make sure that the module is a remote module fetched by // Terraform. Any module that doesn't start with this path is already local, // and should be part of the template files already. @@ -105,6 +135,12 @@ func GetModulesArchive(root fs.FS) ([]byte, error) { continue } + // Leave 1024 bytes for the footer + if it.EstimatedSize > lw.Remaining()-1024 { + skippedModules = append(skippedModules, fmt.Sprintf("%s:%s", it.Key, it.Source)) + continue + } + err := fs.WalkDir(root, it.Dir, func(filePath string, d fs.DirEntry, err error) error { if err != nil { return xerrors.Errorf("failed to create modules archive: %w", err) @@ -149,26 +185,67 @@ func GetModulesArchive(root fs.FS) ([]byte, error) { return nil }) if err != nil { - return nil, err + return nil, skippedModules, err } } err = w.WriteHeader(defaultFileHeader(".terraform/modules/modules.json", len(modulesFileContent))) if err != nil { - return nil, xerrors.Errorf("failed to write modules.json to archive: %w", err) + return nil, skippedModules, xerrors.Errorf("failed to write modules.json to archive: %w", err) } if _, err := w.Write(modulesFileContent); err != nil { - return nil, xerrors.Errorf("failed to write modules.json to archive: %w", err) + return nil, skippedModules, xerrors.Errorf("failed to write modules.json to archive: %w", err) } if err := w.Close(); err != nil { - return nil, xerrors.Errorf("failed to close module files archive: %w", err) + return nil, skippedModules, xerrors.Errorf("failed to close module files archive: %w", err) } // Don't persist empty tar files in the database if empty { - return []byte{}, nil + return []byte{}, skippedModules, nil } - return b.Bytes(), nil + return b.Bytes(), skippedModules, nil +} + +// estimateModuleSize estimates the size impact of adding the specified module +// directory to a tar archive. +func estimateModuleSize(root fs.FS, moduleDir string) (int64, error) { + size := int64(0) + err := fs.WalkDir(root, moduleDir, func(_ string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + fileMode := d.Type() + if !fileMode.IsRegular() && !fileMode.IsDir() { + return nil + } + + // .git directories are not needed in the archive and only cause + // hash differences for identical modules. + if fileMode.IsDir() && d.Name() == ".git" { + return fs.SkipDir + } + + fileInfo, err := d.Info() + if err != nil { + return xerrors.Errorf("file info: %w", err) + } + + size += 512 // tar header size + if !fileMode.IsRegular() { + return nil // Dirs have no content size + } + + fileSize := fileInfo.Size() + size += fileSize + // Pad to 512 bytes + size += 512 - (fileSize % 512) + return nil + }) + if err != nil { + return -1, err + } + return size, err } func fileHeader(filePath string, fileMode fs.FileMode, fileInfo fs.FileInfo) (*tar.Header, error) { diff --git a/provisioner/terraform/modules_internal_test.go b/provisioner/terraform/modules_internal_test.go index 9deff602fe..34fe5be216 100644 --- a/provisioner/terraform/modules_internal_test.go +++ b/provisioner/terraform/modules_internal_test.go @@ -4,6 +4,7 @@ import ( "bytes" "crypto/sha256" "encoding/hex" + "encoding/json" "io/fs" "os" "path/filepath" @@ -26,8 +27,9 @@ func TestGetModulesArchive(t *testing.T) { t.Run("Success", func(t *testing.T) { t.Parallel() - archive, err := GetModulesArchive(os.DirFS(filepath.Join("testdata", "modules-source-caching"))) + archive, skipped, err := GetModulesArchive(os.DirFS(filepath.Join("testdata", "modules-source-caching"))) require.NoError(t, err) + require.Len(t, skipped, 0) // Check that all of the files it should contain are correct b := bytes.NewBuffer(archive) @@ -70,8 +72,211 @@ func TestGetModulesArchive(t *testing.T) { root := afero.NewMemMapFs() afero.WriteFile(root, ".terraform/modules/modules.json", []byte(`{"Modules":[{"Key":"","Source":"","Dir":"."}]}`), 0o644) - archive, err := GetModulesArchive(afero.NewIOFS(root)) + archive, skipped, err := GetModulesArchive(afero.NewIOFS(root)) require.NoError(t, err) + require.Len(t, skipped, 0) require.Equal(t, []byte{}, archive) }) + + t.Run("ModulesTooLarge", func(t *testing.T) { + t.Parallel() + + memFS := moduleArchiveFS(t, map[string]moduleDef{ + "small": { + payload: []byte("small module content"), + }, + "large": { + payload: bytes.Repeat([]byte("A"), 10000), + }, + }) + archive, skipped, err := GetModulesArchiveWithLimit(memFS, 5000) + require.NoError(t, err) + require.Len(t, skipped, 1) + require.Equal(t, "large:large", skipped[0]) + + // Verify small module is in the archive + tarfs := archivefs.FromTarReader(bytes.NewBuffer(archive)) + _, err = fs.ReadFile(tarfs, ".terraform/modules/small/payload") + require.NoError(t, err, "small module should be included") + }) + + // TestModulePackingPrioritizesSmallest verifies that when space is limited, + // smaller modules are included first to maximize the number of modules archived. + t.Run("PackingPrioritizesSmallest", func(t *testing.T) { + t.Parallel() + + // Create modules of varying sizes. With a limit that can fit + // small + medium but not large, we should see small and medium included. + memFS := moduleArchiveFS(t, map[string]moduleDef{ + "small": { + payload: bytes.Repeat([]byte("S"), 500), + }, + "medium": { + payload: bytes.Repeat([]byte("M"), 1500), + }, + "large": { + payload: bytes.Repeat([]byte("L"), 5000), + }, + }) + + // Estimate: each module needs ~512 (dir) + 512 (file header) + content + padding + // small: ~1536 bytes, medium: ~2560 bytes, large: ~6144 bytes + // Plus modules.json overhead (~1024) and tar end blocks (1024). + // Set limit to fit small + medium + overhead but not large. + archive, skipped, err := GetModulesArchiveWithLimit(memFS, 8000) + require.NoError(t, err) + + require.Len(t, skipped, 1, "only the large module should be skipped") + require.Equal(t, "large:large", skipped[0]) + + // Verify correct modules are in archive + tarfs := archivefs.FromTarReader(bytes.NewBuffer(archive)) + _, err = fs.ReadFile(tarfs, ".terraform/modules/small/payload") + require.NoError(t, err, "small module should be included") + _, err = fs.ReadFile(tarfs, ".terraform/modules/medium/payload") + require.NoError(t, err, "medium module should be included") + _, err = fs.ReadFile(tarfs, ".terraform/modules/large/payload") + require.Error(t, err, "large module should NOT be included") + }) + + // TestModulePackingAllFit verifies all modules are included when under budget. + t.Run("PackingAllFit", func(t *testing.T) { + t.Parallel() + + memFS := moduleArchiveFS(t, map[string]moduleDef{ + "mod1": {payload: []byte("module one")}, + "mod2": {payload: []byte("module two")}, + "mod3": {payload: []byte("module three")}, + }) + + // Large limit - everything should fit + archive, skipped, err := GetModulesArchiveWithLimit(memFS, 100000) + require.NoError(t, err) + require.Empty(t, skipped, "no modules should be skipped") + + tarfs := archivefs.FromTarReader(bytes.NewBuffer(archive)) + _, err = fs.ReadFile(tarfs, ".terraform/modules/mod1/payload") + require.NoError(t, err) + _, err = fs.ReadFile(tarfs, ".terraform/modules/mod2/payload") + require.NoError(t, err) + _, err = fs.ReadFile(tarfs, ".terraform/modules/mod3/payload") + require.NoError(t, err) + }) + + // TestModulePackingNoneFit verifies behavior when no modules fit. + t.Run("PackingNoneFit", func(t *testing.T) { + t.Parallel() + + memFS := moduleArchiveFS(t, map[string]moduleDef{ + "mod1": {payload: bytes.Repeat([]byte("X"), 2000)}, + "mod2": {payload: bytes.Repeat([]byte("Y"), 3000)}, + }) + + // Set limit that's enough for modules.json but not for the modules themselves + // modules.json needs ~512 header + content + padding + 1024 end blocks + archive, skipped, err := GetModulesArchiveWithLimit(memFS, 2500) + require.NoError(t, err) + require.Len(t, skipped, 2, "both modules should be skipped") + + // Archive should just contain modules.json (empty means no module content) + require.True(t, len(archive) == 0 || len(archive) < 2500, + "archive should be empty or minimal when no modules fit") + }) + + // TestModulePackingEdgeCaseExactFit tests when a module exactly fits the remaining space. + // The second module should be skipped, because the first module is perfect. + t.Run("PackingEdgeCaseExactFit", func(t *testing.T) { + t.Parallel() + + originalDef := map[string]moduleDef{ + "exact": {payload: bytes.Repeat([]byte("E"), 1000)}, + } + // Create a single module and measure its actual archive size + memFS := moduleArchiveFS(t, originalDef) + + // First, get the actual size with no limit + archive, skipped, err := GetModulesArchiveWithLimit(memFS, 100000) + require.NoError(t, err) + require.Empty(t, skipped) + actualSize := int64(len(archive)) + + originalDef["extra"] = moduleDef{payload: bytes.Repeat([]byte("X"), 1001)} + memFS = moduleArchiveFS(t, originalDef) + + // Now test with exact size - should just fit + archive, skipped, err = GetModulesArchiveWithLimit(memFS, actualSize) + require.NoError(t, err) + require.Len(t, skipped, 1) + require.Equal(t, skipped[0], "extra:extra", "extra module should be skipped") + require.Equal(t, actualSize, int64(len(archive))) + }) + + // TestModulePackingMultipleSkipped verifies correct behavior when multiple + // large modules must be skipped. + t.Run("PackingMultipleSkipped", func(t *testing.T) { + t.Parallel() + + memFS := moduleArchiveFS(t, map[string]moduleDef{ + "tiny": {payload: []byte("t")}, + "small": {payload: bytes.Repeat([]byte("S"), 200)}, + "large1": {payload: bytes.Repeat([]byte("L"), 5000)}, + "large2": {payload: bytes.Repeat([]byte("L"), 6000)}, + "large3": {payload: bytes.Repeat([]byte("L"), 7000)}, + }) + + // Set limit to fit tiny + small + overhead but not the large ones + // tiny: ~1536, small: ~1536, overhead (modules.json + tar end): ~3072 + archive, skipped, err := GetModulesArchiveWithLimit(memFS, 7000) + require.NoError(t, err) + + require.Len(t, skipped, 3, "all three large modules should be skipped") + + tarfs := archivefs.FromTarReader(bytes.NewBuffer(archive)) + _, err = fs.ReadFile(tarfs, ".terraform/modules/tiny/payload") + require.NoError(t, err, "tiny module should be included") + _, err = fs.ReadFile(tarfs, ".terraform/modules/small/payload") + require.NoError(t, err, "small module should be included") + }) +} + +type moduleDef struct { + payload []byte +} + +func moduleArchiveFS(t *testing.T, defs map[string]moduleDef) fs.FS { + memFS := afero.NewMemMapFs() + modRoot := ".terraform/modules" + err := memFS.MkdirAll(modRoot, 0o755) + require.NoError(t, err) + + mods := []*module{} + for name, def := range defs { + modDir := filepath.Join(modRoot, name) + err = memFS.Mkdir(modDir, 0o755) + require.NoError(t, err) + + f, err := memFS.Create(filepath.Join(modDir, "payload")) + require.NoError(t, err) + _, err = f.Write(def.payload) + require.NoError(t, err) + f.Close() + + mods = append(mods, &module{ + Source: name, + Version: "v0.1.0", + Key: name, + Dir: modDir, + }) + } + + data, _ := json.Marshal(modulesFile{ + Modules: mods, + }) + jm, err := memFS.Create(filepath.Join(modRoot, "modules.json")) + require.NoError(t, err) + _, err = jm.Write(data) + require.NoError(t, err) + jm.Close() + + return afero.NewIOFS(memFS) } diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index 4b95d6d2f2..3dd0adbb83 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -136,10 +136,22 @@ func (s *server) Init( // a workspace build. This removes some added costs of sending the modules // payload back to coderd if coderd is just going to ignore it. if !request.OmitModuleFiles { - moduleFiles, err = GetModulesArchive(os.DirFS(e.files.WorkDirectory())) + var skipped []string + moduleFiles, skipped, err = GetModulesArchive(os.DirFS(e.files.WorkDirectory())) if err != nil { - // TODO: we probably want to persist this error or make it louder eventually - e.logger.Warn(ctx, "failed to archive terraform modules", slog.Error(err)) + // Making this a fatal error would block the template from functioning. This + // error means the template has some reduced functionality, which will be raised + // on the workspace create page. This is not ideal, but it is better to have + // limited functionality, then none. + e.logger.Error(ctx, "failed to archive modules: %v", slog.Error(err)) + } + + if len(skipped) > 0 { + // TODO: This information needs to be raised on the template page somehow. + // Essentially some of the modules were not archived because they were too large. + e.logger.Warn(ctx, "some (or all) terraform modules were not archived, template will have reduced function", + slog.F("skipped_modules", strings.Join(skipped, ", ")), + ) } }