From d3a645949ccd1be77b6538835dac1bb455955fa4 Mon Sep 17 00:00:00 2001 From: ju-pe Date: Fri, 1 May 2026 19:52:22 +0000 Subject: [PATCH 01/10] feat: add archive_path attribute to coderd_template versions Implements #340. Adds archive_path as an alternative to directory in the versions block of coderd_template, supporting .tar and .zip files. Changes: - Add archive_path (Optional) and archive_hash (Computed) attributes - Validator ensures exactly one of directory/archive_path is set - Plan modifier computes archive_hash from file contents (SHA-256) - normalizeZip adds missing directory entries for archives produced by Terraform's archive_file data source - uploadArchive sends the archive to Coder with correct content type - Warn when switching between archive_path and directory about hidden file inclusion differences - Document 100 MiB server upload limit in archive_path description Tests: - TestComputeArchiveHash (4 cases) - TestArchiveContentType (5 cases) - TestNormalizeZip (2 cases) - TestReconcileVersionIDs extended with ArchiveHashMatching/Changed --- docs/resources/template.md | 7 +- internal/provider/template_resource.go | 345 ++++++++++++++++++-- internal/provider/template_resource_test.go | 74 ++++- internal/provider/util.go | 15 + internal/provider/util_test.go | 331 +++++++++++++++++++ 5 files changed, 733 insertions(+), 39 deletions(-) create mode 100644 internal/provider/util_test.go diff --git a/docs/resources/template.md b/docs/resources/template.md index 6e761c2..4ab2320 100644 --- a/docs/resources/template.md +++ b/docs/resources/template.md @@ -93,13 +93,11 @@ resource "coderd_template" "ubuntu-main" { ### Nested Schema for `versions` -Required: - -- `directory` (String) A path to the directory to create the template version from. Changes in the directory contents will trigger the creation of a new template version. - Optional: - `active` (Boolean) Whether this version is the active version of the template. Only one version can be active at a time. +- `archive_path` (String) A path to a `.tar` or `.zip` archive file to upload as the template version source. Mutually exclusive with `directory`. Changes in the archive contents will trigger the creation of a new template version. The archive must not exceed 100 MiB (the Coder server upload limit). +- `directory` (String) A path to the directory to create the template version from. Changes in the directory contents will trigger the creation of a new template version. Conflicts with `archive_path`. - `message` (String) A message describing the changes in this version of the template. Messages longer than 72 characters will be truncated. - `name` (String) The name of the template version. Automatically generated if not provided. If provided, the name *must* change each time the directory contents, or the `tf_vars` attribute are updated. - `provisioner_tags` (Attributes Set) Provisioner tags for the template version. (see [below for nested schema](#nestedatt--versions--provisioner_tags)) @@ -107,6 +105,7 @@ Optional: Read-Only: +- `archive_hash` (String) - `directory_hash` (String) - `id` (String) diff --git a/internal/provider/template_resource.go b/internal/provider/template_resource.go index f053773..3ffb274 100644 --- a/internal/provider/template_resource.go +++ b/internal/provider/template_resource.go @@ -1,12 +1,17 @@ package provider import ( + "archive/zip" "bufio" + "bytes" "context" "encoding/json" "fmt" "io" + "os" + "path/filepath" "slices" + "sort" "strings" "time" @@ -162,11 +167,25 @@ type TemplateVersion struct { Message types.String `tfsdk:"message"` Directory types.String `tfsdk:"directory"` DirectoryHash types.String `tfsdk:"directory_hash"` + ArchivePath types.String `tfsdk:"archive_path"` + ArchiveHash types.String `tfsdk:"archive_hash"` Active types.Bool `tfsdk:"active"` TerraformVariables []Variable `tfsdk:"tf_vars"` ProvisionerTags []Variable `tfsdk:"provisioner_tags"` } +// contentHash returns the relevant hash for change detection, regardless of +// whether the version source is a directory or an archive. +func (v TemplateVersion) contentHash() string { + if !v.ArchiveHash.IsNull() && !v.ArchiveHash.IsUnknown() && v.ArchiveHash.ValueString() != "" { + return v.ArchiveHash.ValueString() + } + if !v.DirectoryHash.IsNull() && !v.DirectoryHash.IsUnknown() { + return v.DirectoryHash.ValueString() + } + return "" +} + type Versions []TemplateVersion func (v Versions) ByID(id UUID) *TemplateVersion { @@ -456,12 +475,19 @@ func (r *TemplateResource) Schema(ctx context.Context, req resource.SchemaReques Default: stringdefault.StaticString(""), }, "directory": schema.StringAttribute{ - MarkdownDescription: "A path to the directory to create the template version from. Changes in the directory contents will trigger the creation of a new template version.", - Required: true, + Optional: true, + MarkdownDescription: "A path to the directory to create the template version from. Changes in the directory contents will trigger the creation of a new template version. Conflicts with `archive_path`.", }, "directory_hash": schema.StringAttribute{ Computed: true, }, + "archive_path": schema.StringAttribute{ + Optional: true, + MarkdownDescription: "A path to a `.tar` or `.zip` archive file to upload as the template version source. Mutually exclusive with `directory`. Changes in the archive contents will trigger the creation of a new template version. The archive must not exceed 100 MiB (the Coder server upload limit).", + }, + "archive_hash": schema.StringAttribute{ + Computed: true, + }, "active": schema.BoolAttribute{ MarkdownDescription: "Whether this version is the active version of the template. Only one version can be active at a time.", Computed: true, @@ -594,6 +620,15 @@ func (r *TemplateResource) Create(ctx context.Context, req resource.CreateReques } data.Versions[idx].ID = UUIDValue(versionResp.ID) data.Versions[idx].Name = types.StringValue(versionResp.Name) + // Ensure computed hash fields have concrete values after apply. + // Only one of directory_hash or archive_hash will be meaningful; + // set the other to empty string so Terraform doesn't see unknown values. + if data.Versions[idx].DirectoryHash.IsUnknown() { + data.Versions[idx].DirectoryHash = types.StringValue("") + } + if data.Versions[idx].ArchiveHash.IsUnknown() { + data.Versions[idx].ArchiveHash = types.StringValue("") + } } data.ID = UUIDValue(templateResp.ID) data.DisplayName = types.StringValue(templateResp.DisplayName) @@ -812,6 +847,13 @@ func (r *TemplateResource) Update(ctx context.Context, req resource.UpdateReques } newState.Versions[idx].ID = UUIDValue(versionResp.ID) newState.Versions[idx].Name = types.StringValue(versionResp.Name) + // Ensure computed hash fields have concrete values after apply. + if newState.Versions[idx].DirectoryHash.IsUnknown() { + newState.Versions[idx].DirectoryHash = types.StringValue("") + } + if newState.Versions[idx].ArchiveHash.IsUnknown() { + newState.Versions[idx].ArchiveHash = types.StringValue("") + } if newState.Versions[idx].Active.ValueBool() { err := markActive(ctx, client, templateID, newState.Versions[idx].ID.ValueUUID()) if err != nil { @@ -845,6 +887,14 @@ func (r *TemplateResource) Update(ctx context.Context, req resource.UpdateReques return } } + // Ensure computed hash fields have concrete values after apply, + // even for reused versions. + if newState.Versions[idx].DirectoryHash.IsUnknown() { + newState.Versions[idx].DirectoryHash = types.StringValue("") + } + if newState.Versions[idx].ArchiveHash.IsUnknown() { + newState.Versions[idx].ArchiveHash = types.StringValue("") + } } } // TODO(ethanndickson): Remove this once the provider requires a Coder @@ -948,7 +998,38 @@ func (a *versionsValidator) ValidateList(ctx context.Context, req validator.List // Check all versions have unique names uniqueNames := make(map[string]struct{}) - for _, version := range data { + for i, version := range data { + // Exactly one of directory or archive_path must be set. + dirSet := !version.Directory.IsNull() + archiveSet := !version.ArchivePath.IsNull() + if !dirSet && !archiveSet { + resp.Diagnostics.AddAttributeError( + req.Path.AtListIndex(i), + "Invalid Version Source", + "Exactly one of `directory` or `archive_path` must be specified for each template version.", + ) + return + } + if dirSet && archiveSet { + resp.Diagnostics.AddAttributeError( + req.Path.AtListIndex(i), + "Invalid Version Source", + "`directory` and `archive_path` are mutually exclusive for each template version.", + ) + return + } + if archiveSet && !version.ArchivePath.IsUnknown() { + archivePath := version.ArchivePath.ValueString() + if !strings.HasSuffix(archivePath, ".tar") && !strings.HasSuffix(archivePath, ".zip") { + resp.Diagnostics.AddAttributeError( + req.Path.AtListIndex(i).AtName("archive_path"), + "Invalid Archive Format", + fmt.Sprintf("archive_path must reference a .tar or .zip file, got %q", filepath.Base(archivePath)), + ) + return + } + } + if version.Name.IsNull() || version.Name.IsUnknown() { continue } @@ -1011,12 +1092,65 @@ func (d *versionsPlanModifier) PlanModifyList(ctx context.Context, req planmodif } for i := range planVersions { - hash, err := computeDirectoryHash(planVersions[i].Directory.ValueString()) - if err != nil { - resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to compute directory hash: %s", err)) - return + if !planVersions[i].ArchivePath.IsNull() && !planVersions[i].ArchivePath.IsUnknown() { + hash, err := computeArchiveHash(planVersions[i].ArchivePath.ValueString()) + if err != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to compute archive hash: %s", err)) + return + } + planVersions[i].ArchiveHash = types.StringValue(hash) + } else if !planVersions[i].ArchivePath.IsNull() && planVersions[i].ArchivePath.IsUnknown() { + // archive_path is set but not yet known (depends on another resource). + // We can't compute the hash yet; mark it as unknown. + planVersions[i].ArchiveHash = types.StringUnknown() + } else if !planVersions[i].Directory.IsNull() && !planVersions[i].Directory.IsUnknown() { + hash, err := computeDirectoryHash(planVersions[i].Directory.ValueString()) + if err != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to compute directory hash: %s", err)) + return + } + planVersions[i].DirectoryHash = types.StringValue(hash) + } + } + + // Warn if any version is switching between archive_path and directory. + if !req.StateValue.IsNull() { + var stateVersions Versions + resp.Diagnostics.Append(req.StateValue.ElementsAs(ctx, &stateVersions, false)...) + if !resp.Diagnostics.HasError() { + for i := range planVersions { + if i >= len(stateVersions) { + break + } + hadArchive := !stateVersions[i].ArchivePath.IsNull() && stateVersions[i].ArchivePath.ValueString() != "" + hadDirectory := !stateVersions[i].Directory.IsNull() && stateVersions[i].Directory.ValueString() != "" + nowArchive := !planVersions[i].ArchivePath.IsNull() + nowDirectory := !planVersions[i].Directory.IsNull() + + if hadArchive && nowDirectory { + resp.Diagnostics.AddWarning( + "Switching from archive_path to directory", + fmt.Sprintf( + "Version %q (index %d) is switching from archive_path to directory. "+ + "The directory source uses provisionersdk.Tar, which skips hidden files "+ + "(dotfiles such as .claude.json, .mcp.json, etc.). If your template relies "+ + "on hidden files, consider continuing to use archive_path instead.", + planVersions[i].Name.ValueString(), i, + ), + ) + } else if hadDirectory && nowArchive { + resp.Diagnostics.AddWarning( + "Switching from directory to archive_path", + fmt.Sprintf( + "Version %q (index %d) is switching from directory to archive_path. "+ + "The archive may include hidden files (dotfiles) that were previously "+ + "excluded by the directory source.", + planVersions[i].Name.ValueString(), i, + ), + ) + } + } } - planVersions[i].DirectoryHash = types.StringValue(hash) } var lv LastVersionsByHash @@ -1104,6 +1238,138 @@ func uploadDirectory(ctx context.Context, client *codersdk.Client, logger slog.L return &resp, nil } +func archiveContentType(archivePath string) (string, error) { + switch { + case strings.HasSuffix(archivePath, ".tar"): + return codersdk.ContentTypeTar, nil + case strings.HasSuffix(archivePath, ".zip"): + return codersdk.ContentTypeZip, nil + default: + return "", fmt.Errorf("unsupported archive format %q: must be .tar or .zip", filepath.Ext(archivePath)) + } +} + +// normalizeZip rewrites a zip archive to ensure that all intermediate directory +// entries exist. Some tools (e.g. Terraform's archive_file data source) produce +// zips that contain only file entries without explicit directory entries. Coder's +// server-side zip extractor expects directories to be present. +func normalizeZip(archivePath string) ([]byte, error) { + r, err := zip.OpenReader(archivePath) + if err != nil { + return nil, fmt.Errorf("failed to open zip: %w", err) + } + defer r.Close() //nolint:errcheck // Best-effort close of read-only zip. + + // Collect the set of directories that already have explicit entries. + existingDirs := make(map[string]bool) + for _, f := range r.File { + if f.FileInfo().IsDir() { + existingDirs[f.Name] = true + } + } + + // Determine which intermediate directories are missing. + missingDirs := make(map[string]bool) + for _, f := range r.File { + dir := filepath.Dir(f.Name) + for dir != "." && dir != "/" && dir != "" { + dirEntry := dir + "/" + if existingDirs[dirEntry] || missingDirs[dirEntry] { + break + } + missingDirs[dirEntry] = true + dir = filepath.Dir(dir) + } + } + + // If nothing is missing, read and return the original file as-is. + if len(missingDirs) == 0 { + data, err := os.ReadFile(archivePath) + if err != nil { + return nil, fmt.Errorf("failed to read archive: %w", err) + } + return data, nil + } + + // Sort missing dirs so parents come before children. + sortedMissing := make([]string, 0, len(missingDirs)) + for d := range missingDirs { + sortedMissing = append(sortedMissing, d) + } + sort.Strings(sortedMissing) + + // Rewrite the zip with directory entries first, then all original entries. + var buf bytes.Buffer + w := zip.NewWriter(&buf) + + // Add missing directory entries. + for _, d := range sortedMissing { + _, err := w.CreateHeader(&zip.FileHeader{ + Name: d, + ExternalAttrs: 0o755 << 16, // rwxr-xr-x in Unix mode + CreatorVersion: 3 << 8, // Unix + }) + if err != nil { + return nil, fmt.Errorf("failed to create directory entry %q: %w", d, err) + } + } + + // Copy all original entries. + for _, f := range r.File { + fw, err := w.CreateHeader(&f.FileHeader) + if err != nil { + return nil, fmt.Errorf("failed to create entry %q: %w", f.Name, err) + } + if f.FileInfo().IsDir() { + continue + } + rc, err := f.Open() + if err != nil { + return nil, fmt.Errorf("failed to open entry %q: %w", f.Name, err) + } + _, err = io.Copy(fw, rc) + _ = rc.Close() + if err != nil { + return nil, fmt.Errorf("failed to copy entry %q: %w", f.Name, err) + } + } + + if err := w.Close(); err != nil { + return nil, fmt.Errorf("failed to finalize zip: %w", err) + } + return buf.Bytes(), nil +} + +func uploadArchive(ctx context.Context, client *codersdk.Client, archivePath string) (*codersdk.UploadResponse, error) { + contentType, err := archiveContentType(archivePath) + if err != nil { + return nil, err + } + + var reader io.Reader + if contentType == codersdk.ContentTypeZip { + // Normalize the zip to ensure intermediate directory entries exist. + data, err := normalizeZip(archivePath) + if err != nil { + return nil, fmt.Errorf("failed to normalize zip: %w", err) + } + reader = bytes.NewReader(data) + } else { + f, err := os.Open(archivePath) + if err != nil { + return nil, fmt.Errorf("failed to open archive: %w", err) + } + defer f.Close() //nolint:errcheck // Best-effort close; upload already consumed the reader. + reader = bufio.NewReader(f) + } + + resp, err := client.Upload(ctx, contentType, reader) + if err != nil { + return nil, err + } + return &resp, nil +} + func waitForJob(ctx context.Context, client *codersdk.Client, version *codersdk.TemplateVersion) ([]codersdk.ProvisionerJobLog, error) { const maxRetries = 3 var allLogs []codersdk.ProvisionerJobLog @@ -1180,21 +1446,36 @@ type newVersionRequest struct { func newVersion(ctx context.Context, client *codersdk.Client, req newVersionRequest) (*codersdk.TemplateVersion, []codersdk.ProvisionerJobLog, error) { var logs []codersdk.ProvisionerJobLog - directory := req.Version.Directory.ValueString() - tflog.Info(ctx, "uploading directory") - uploadResp, err := uploadDirectory(ctx, client, slog.Make(newTFLogSink(ctx)), directory) - if err != nil { - return nil, logs, fmt.Errorf("failed to upload directory: %s", err) - } - tflog.Info(ctx, "successfully uploaded directory") - tflog.Info(ctx, "discovering and parsing vars files") - varFiles, err := codersdk.DiscoverVarsFiles(directory) - if err != nil { - return nil, logs, fmt.Errorf("failed to discover vars files: %s", err) - } - vars, err := codersdk.ParseUserVariableValues(varFiles, "", []string{}) - if err != nil { - return nil, logs, fmt.Errorf("failed to parse user variable values: %s", err) + var err error + var uploadResp *codersdk.UploadResponse + var vars []codersdk.VariableValue + + if !req.Version.ArchivePath.IsNull() && !req.Version.ArchivePath.IsUnknown() { + archivePath := req.Version.ArchivePath.ValueString() + tflog.Info(ctx, "uploading archive", map[string]any{"archive_path": archivePath}) + uploadResp, err = uploadArchive(ctx, client, archivePath) + if err != nil { + return nil, logs, fmt.Errorf("failed to upload archive: %s", err) + } + tflog.Info(ctx, "successfully uploaded archive") + tflog.Info(ctx, "skipping vars file discovery for archive upload, use tf_vars to provide variables") + } else { + directory := req.Version.Directory.ValueString() + tflog.Info(ctx, "uploading directory") + uploadResp, err = uploadDirectory(ctx, client, slog.Make(newTFLogSink(ctx)), directory) + if err != nil { + return nil, logs, fmt.Errorf("failed to upload directory: %s", err) + } + tflog.Info(ctx, "successfully uploaded directory") + tflog.Info(ctx, "discovering and parsing vars files") + varFiles, err := codersdk.DiscoverVarsFiles(directory) + if err != nil { + return nil, logs, fmt.Errorf("failed to discover vars files: %s", err) + } + vars, err = codersdk.ParseUserVariableValues(varFiles, "", []string{}) + if err != nil { + return nil, logs, fmt.Errorf("failed to parse user variable values: %s", err) + } } tflog.Info(ctx, "discovered and parsed vars files", map[string]any{ "vars": vars, @@ -1443,7 +1724,7 @@ type privateState interface { func (v Versions) setPrivateState(ctx context.Context, ps privateState) (diags diag.Diagnostics) { lv := make(LastVersionsByHash) for _, version := range v { - vbh, ok := lv[version.DirectoryHash.ValueString()] + vbh, ok := lv[version.contentHash()] tfVars := make(map[string]string, len(version.TerraformVariables)) for _, tfVar := range version.TerraformVariables { tfVars[tfVar.Name.ValueString()] = tfVar.Value.ValueString() @@ -1451,14 +1732,14 @@ func (v Versions) setPrivateState(ctx context.Context, ps privateState) (diags d // Store the IDs and names of all versions with the same directory hash, // in the order they appear if ok { - lv[version.DirectoryHash.ValueString()] = append(vbh, PreviousTemplateVersion{ + lv[version.contentHash()] = append(vbh, PreviousTemplateVersion{ ID: version.ID.ValueUUID(), Name: version.Name.ValueString(), TFVars: tfVars, Active: version.Active.ValueBool(), }) } else { - lv[version.DirectoryHash.ValueString()] = []PreviousTemplateVersion{ + lv[version.contentHash()] = []PreviousTemplateVersion{ { ID: version.ID.ValueUUID(), Name: version.Name.ValueString(), @@ -1485,7 +1766,7 @@ func (planVersions Versions) reconcileVersionIDs(lv LastVersionsByHash, configVe } for i := range planVersions { - prevList, ok := lv[planVersions[i].DirectoryHash.ValueString()] + prevList, ok := lv[planVersions[i].contentHash()] // If not in state, mark as known after apply since we'll create a new version. // Versions whose Terraform configuration has not changed will have known // IDs at this point, so we need to set this manually. @@ -1504,7 +1785,7 @@ func (planVersions Versions) reconcileVersionIDs(lv LastVersionsByHash, configVe // it from the previous version candidates if planVersions[i].Name.ValueString() == prev.Name { planVersions[i].ID = UUIDValue(prev.ID) - lv[planVersions[i].DirectoryHash.ValueString()] = append(prevList[:j], prevList[j+1:]...) + lv[planVersions[i].contentHash()] = append(prevList[:j], prevList[j+1:]...) break } } @@ -1514,13 +1795,13 @@ func (planVersions Versions) reconcileVersionIDs(lv LastVersionsByHash, configVe // For versions whose hash was found in the private state but couldn't be // matched, use the leftovers in the order they appear for i := range planVersions { - prevList := lv[planVersions[i].DirectoryHash.ValueString()] + prevList := lv[planVersions[i].contentHash()] if len(prevList) > 0 && planVersions[i].ID.IsUnknown() { planVersions[i].ID = UUIDValue(prevList[0].ID) if planVersions[i].Name.IsUnknown() { planVersions[i].Name = types.StringValue(prevList[0].Name) } - lv[planVersions[i].DirectoryHash.ValueString()] = prevList[1:] + lv[planVersions[i].contentHash()] = prevList[1:] } } @@ -1528,7 +1809,7 @@ func (planVersions Versions) reconcileVersionIDs(lv LastVersionsByHash, configVe // we need to create a new version with the new variables. for i := range planVersions { if !planVersions[i].ID.IsUnknown() { - prevs, ok := fullLv[planVersions[i].DirectoryHash.ValueString()] + prevs, ok := fullLv[planVersions[i].contentHash()] if !ok { continue } @@ -1552,7 +1833,7 @@ func (planVersions Versions) reconcileVersionIDs(lv LastVersionsByHash, configVe if !hasOneActiveVersion { for i := range planVersions { if !planVersions[i].ID.IsUnknown() { - prevs, ok := fullLv[planVersions[i].DirectoryHash.ValueString()] + prevs, ok := fullLv[planVersions[i].contentHash()] if !ok { continue } diff --git a/internal/provider/template_resource_test.go b/internal/provider/template_resource_test.go index 971de6e..31d40ab 100644 --- a/internal/provider/template_resource_test.go +++ b/internal/provider/template_resource_test.go @@ -1158,9 +1158,10 @@ resource "coderd_template" "test" { versions = [ {{- range .Versions }} { - name = {{orNull .Name}} - directory = {{orNull .Directory}} - active = {{orNull .Active}} + name = {{orNull .Name}} + directory = {{orNull .Directory}} + archive_path = {{orNull .ArchivePath}} + active = {{orNull .Active}} tf_vars = [ {{- range .TerraformVariables }} @@ -1194,6 +1195,7 @@ type testAccTemplateVersionConfig struct { Name *string Message *string Directory *string + ArchivePath *string Active *bool TerraformVariables []testAccTemplateKeyValueConfig } @@ -1592,6 +1594,72 @@ func TestReconcileVersionIDs(t *testing.T) { cfgHasActiveVersion: false, expectError: true, }, + { + Name: "ArchiveHashMatching", + planVersions: []TemplateVersion{ + { + Name: types.StringValue("archive-ver"), + ArchiveHash: types.StringValue("archivehash123"), + ID: NewUUIDUnknown(), + TerraformVariables: []Variable{}, + }, + }, + configVersions: []TemplateVersion{ + { + Name: types.StringValue("archive-ver"), + }, + }, + inputState: map[string][]PreviousTemplateVersion{ + "archivehash123": { + { + ID: aUUID, + Name: "archive-ver", + TFVars: map[string]string{}, + }, + }, + }, + expectedVersions: []TemplateVersion{ + { + Name: types.StringValue("archive-ver"), + ArchiveHash: types.StringValue("archivehash123"), + ID: UUIDValue(aUUID), + TerraformVariables: []Variable{}, + }, + }, + }, + { + Name: "ArchiveHashChanged", + planVersions: []TemplateVersion{ + { + Name: types.StringValue("archive-ver"), + ArchiveHash: types.StringValue("newhash456"), + ID: NewUUIDUnknown(), + TerraformVariables: []Variable{}, + }, + }, + configVersions: []TemplateVersion{ + { + Name: types.StringValue("archive-ver"), + }, + }, + inputState: map[string][]PreviousTemplateVersion{ + "oldhash123": { + { + ID: aUUID, + Name: "archive-ver", + TFVars: map[string]string{}, + }, + }, + }, + expectedVersions: []TemplateVersion{ + { + Name: types.StringValue("archive-ver"), + ArchiveHash: types.StringValue("newhash456"), + ID: NewUUIDUnknown(), + TerraformVariables: []Variable{}, + }, + }, + }, } for _, c := range cases { diff --git a/internal/provider/util.go b/internal/provider/util.go index dbc3441..d02f5eb 100644 --- a/internal/provider/util.go +++ b/internal/provider/util.go @@ -5,6 +5,7 @@ import ( "encoding/hex" "errors" "fmt" + "io" "net/http" "os" "path/filepath" @@ -86,6 +87,20 @@ func computeDirectoryHash(directory string) (string, error) { return hex.EncodeToString(hash.Sum(nil)), nil } +func computeArchiveHash(archivePath string) (string, error) { + f, err := os.Open(archivePath) + if err != nil { + return "", err + } + defer f.Close() //nolint:errcheck // Best-effort close of read-only file. + + hash := sha256.New() + if _, err := io.Copy(hash, f); err != nil { + return "", err + } + return hex.EncodeToString(hash.Sum(nil)), nil +} + // memberDiff returns the members to add and remove from the group, given the // current members and the planned members. plannedMembers is deliberately our // custom type, as Terraform cannot automatically produce `[]uuid.UUID` from a diff --git a/internal/provider/util_test.go b/internal/provider/util_test.go new file mode 100644 index 0000000..8e5e448 --- /dev/null +++ b/internal/provider/util_test.go @@ -0,0 +1,331 @@ +package provider + +import ( + "archive/tar" + "archive/zip" + "bytes" + "io" + "os" + "path/filepath" + "testing" + + "github.com/coder/coder/v2/codersdk" + "github.com/stretchr/testify/require" +) + +func TestComputeArchiveHash(t *testing.T) { + t.Parallel() + + t.Run("ValidTarFile", func(t *testing.T) { + t.Parallel() + // Create a minimal tar file + tarPath := filepath.Join(t.TempDir(), "test.tar") + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + content := []byte("hello world") + err := tw.WriteHeader(&tar.Header{ + Name: "test.txt", + Size: int64(len(content)), + Mode: 0644, + }) + require.NoError(t, err) + _, err = tw.Write(content) + require.NoError(t, err) + require.NoError(t, tw.Close()) + err = os.WriteFile(tarPath, buf.Bytes(), 0644) + require.NoError(t, err) + + hash, err := computeArchiveHash(tarPath) + require.NoError(t, err) + require.NotEmpty(t, hash) + require.Len(t, hash, 64) // SHA-256 hex = 64 chars + + // Same file should produce same hash + hash2, err := computeArchiveHash(tarPath) + require.NoError(t, err) + require.Equal(t, hash, hash2) + }) + + t.Run("ValidZipFile", func(t *testing.T) { + t.Parallel() + zipPath := filepath.Join(t.TempDir(), "test.zip") + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + fw, err := zw.Create("test.txt") + require.NoError(t, err) + _, err = fw.Write([]byte("hello world")) + require.NoError(t, err) + require.NoError(t, zw.Close()) + err = os.WriteFile(zipPath, buf.Bytes(), 0644) + require.NoError(t, err) + + hash, err := computeArchiveHash(zipPath) + require.NoError(t, err) + require.NotEmpty(t, hash) + require.Len(t, hash, 64) + }) + + t.Run("DifferentContentDifferentHash", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + + file1 := filepath.Join(dir, "a.tar") + err := os.WriteFile(file1, []byte("content-a"), 0644) + require.NoError(t, err) + + file2 := filepath.Join(dir, "b.tar") + err = os.WriteFile(file2, []byte("content-b"), 0644) + require.NoError(t, err) + + hash1, err := computeArchiveHash(file1) + require.NoError(t, err) + hash2, err := computeArchiveHash(file2) + require.NoError(t, err) + require.NotEqual(t, hash1, hash2) + }) + + t.Run("NonexistentFile", func(t *testing.T) { + t.Parallel() + _, err := computeArchiveHash("/nonexistent/path.tar") + require.Error(t, err) + }) +} + +func TestNormalizeZip(t *testing.T) { + t.Parallel() + + t.Run("AddsMissingDirectoryEntries", func(t *testing.T) { + t.Parallel() + // Create a zip with only file entries (no directory entries), + // mimicking hashicorp/archive's archive_file data source. + zipPath := filepath.Join(t.TempDir(), "nodirs.zip") + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + + // Top-level regular file. + fw, err := zw.Create("main.tf") + require.NoError(t, err) + _, err = fw.Write([]byte("resource {}")) + require.NoError(t, err) + + // Top-level hidden file (dotfile at root). + fw, err = zw.Create(".env") + require.NoError(t, err) + _, err = fw.Write([]byte("SECRET=value")) + require.NoError(t, err) + + // Top-level hidden directory with a file inside. + fw, err = zw.Create(".config/settings.json") + require.NoError(t, err) + _, err = fw.Write([]byte(`{"key": "value"}`)) + require.NoError(t, err) + + // Hidden file inside a regular directory. + fw, err = zw.Create("subdir/.hidden") + require.NoError(t, err) + _, err = fw.Write([]byte("hidden content")) + require.NoError(t, err) + + // Deeply nested hidden directory with a hidden file inside. + fw, err = zw.Create("a/b/.secret-dir/.credentials") + require.NoError(t, err) + _, err = fw.Write([]byte("token=abc123")) + require.NoError(t, err) + + require.NoError(t, zw.Close()) + err = os.WriteFile(zipPath, buf.Bytes(), 0644) + require.NoError(t, err) + + // Verify original has no directory entries. + origReader, err := zip.OpenReader(zipPath) + require.NoError(t, err) + for _, f := range origReader.File { + require.False(t, f.FileInfo().IsDir(), "expected no dir entries in original, got %q", f.Name) + } + require.NoError(t, origReader.Close()) + + // Normalize. + data, err := normalizeZip(zipPath) + require.NoError(t, err) + + // Read normalized zip and collect entries. + normReader, err := zip.NewReader(bytes.NewReader(data), int64(len(data))) + require.NoError(t, err) + + dirs := make(map[string]bool) + files := make(map[string]bool) + for _, f := range normReader.File { + if f.FileInfo().IsDir() { + dirs[f.Name] = true + } else { + files[f.Name] = true + } + } + + // All intermediate directories should exist, including hidden ones. + require.True(t, dirs[".config/"], "missing .config/") + require.True(t, dirs["subdir/"], "missing subdir/") + require.True(t, dirs["a/"], "missing a/") + require.True(t, dirs["a/b/"], "missing a/b/") + require.True(t, dirs["a/b/.secret-dir/"], "missing a/b/.secret-dir/") + + // All original files should still be present. + require.True(t, files["main.tf"], "missing main.tf") + require.True(t, files[".env"], "missing .env") + require.True(t, files[".config/settings.json"], "missing .config/settings.json") + require.True(t, files["subdir/.hidden"], "missing subdir/.hidden") + require.True(t, files["a/b/.secret-dir/.credentials"], "missing a/b/.secret-dir/.credentials") + }) + + t.Run("PreservesDirectoryPermissions", func(t *testing.T) { + t.Parallel() + // Verify that synthesized directory entries have proper mode bits + // so the server-side extractor can create them. + zipPath := filepath.Join(t.TempDir(), "needsperms.zip") + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + + fw, err := zw.Create("deep/nested/dir/file.txt") + require.NoError(t, err) + _, err = fw.Write([]byte("content")) + require.NoError(t, err) + require.NoError(t, zw.Close()) + err = os.WriteFile(zipPath, buf.Bytes(), 0644) + require.NoError(t, err) + + data, err := normalizeZip(zipPath) + require.NoError(t, err) + + normReader, err := zip.NewReader(bytes.NewReader(data), int64(len(data))) + require.NoError(t, err) + + for _, f := range normReader.File { + if f.FileInfo().IsDir() { + mode := f.Mode() + require.NotZero(t, mode&0700, + "directory %q should have owner rwx bits set, got %s", f.Name, mode) + } + } + }) + + t.Run("NoChangeWhenDirsExist", func(t *testing.T) { + t.Parallel() + zipPath := filepath.Join(t.TempDir(), "withdirs.zip") + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + + // Add directory entry explicitly. + _, err := zw.Create("subdir/") + require.NoError(t, err) + fw, err := zw.Create("subdir/file.txt") + require.NoError(t, err) + _, err = fw.Write([]byte("content")) + require.NoError(t, err) + require.NoError(t, zw.Close()) + + origData := buf.Bytes() + err = os.WriteFile(zipPath, origData, 0644) + require.NoError(t, err) + + // Normalize should return original bytes unchanged. + data, err := normalizeZip(zipPath) + require.NoError(t, err) + require.Equal(t, origData, data) + }) + + t.Run("PreservesFileContent", func(t *testing.T) { + t.Parallel() + // Verify normalization doesn't corrupt file contents, + // including hidden files at various levels. + zipPath := filepath.Join(t.TempDir(), "content.zip") + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + + fileContents := map[string]string{ + ".gitignore": "node_modules/\n.env\n", + "main.tf": "resource \"null_resource\" \"test\" {}", + ".vscode/settings.json": `{"editor.formatOnSave": true}`, + "scripts/.local/bin/deploy": "#!/bin/bash\necho deploy", + } + for name, content := range fileContents { + fw, err := zw.Create(name) + require.NoError(t, err) + _, err = fw.Write([]byte(content)) + require.NoError(t, err) + } + require.NoError(t, zw.Close()) + err := os.WriteFile(zipPath, buf.Bytes(), 0644) + require.NoError(t, err) + + data, err := normalizeZip(zipPath) + require.NoError(t, err) + + normReader, err := zip.NewReader(bytes.NewReader(data), int64(len(data))) + require.NoError(t, err) + + for _, f := range normReader.File { + if f.FileInfo().IsDir() { + continue + } + expected, ok := fileContents[f.Name] + require.True(t, ok, "unexpected file in normalized zip: %q", f.Name) + rc, err := f.Open() + require.NoError(t, err) + actual, err := io.ReadAll(rc) + require.NoError(t, rc.Close()) + require.NoError(t, err) + require.Equal(t, expected, string(actual), + "content mismatch for %q", f.Name) + } + }) +} + +func TestArchiveContentType(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + path string + expected string + expectError bool + }{ + { + name: "TarFile", + path: "/path/to/template.tar", + expected: codersdk.ContentTypeTar, + }, + { + name: "ZipFile", + path: "/path/to/template.zip", + expected: codersdk.ContentTypeZip, + }, + { + name: "TarGzFile", + path: "/path/to/template.tar.gz", + expectError: true, + }, + { + name: "RandomFile", + path: "/path/to/template.txt", + expectError: true, + }, + { + name: "NoExtension", + path: "/path/to/template", + expectError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + ct, err := archiveContentType(tc.path) + if tc.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, tc.expected, ct) + } + }) + } +} From 40f9cac10335890958d1307b3807fb41a62f1ea7 Mon Sep 17 00:00:00 2001 From: ju-pe Date: Thu, 7 May 2026 16:46:53 +0000 Subject: [PATCH 02/10] fix: skip XOR validation when directory/archive_path values are unknown Per Terraform framework guidance, validators should not emit errors for unknown values. When either directory or archive_path is unknown (e.g., referencing a computed value from another resource), the XOR check is skipped. Terraform will re-run validators once values resolve during apply. --- internal/provider/template_resource.go | 38 +++++++++++++++----------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/internal/provider/template_resource.go b/internal/provider/template_resource.go index 3ffb274..bf0523b 100644 --- a/internal/provider/template_resource.go +++ b/internal/provider/template_resource.go @@ -1000,25 +1000,31 @@ func (a *versionsValidator) ValidateList(ctx context.Context, req validator.List uniqueNames := make(map[string]struct{}) for i, version := range data { // Exactly one of directory or archive_path must be set. + // Skip validation when either value is unknown (depends on another + // resource). Terraform will re-run validators once values resolve. dirSet := !version.Directory.IsNull() archiveSet := !version.ArchivePath.IsNull() - if !dirSet && !archiveSet { - resp.Diagnostics.AddAttributeError( - req.Path.AtListIndex(i), - "Invalid Version Source", - "Exactly one of `directory` or `archive_path` must be specified for each template version.", - ) - return - } - if dirSet && archiveSet { - resp.Diagnostics.AddAttributeError( - req.Path.AtListIndex(i), - "Invalid Version Source", - "`directory` and `archive_path` are mutually exclusive for each template version.", - ) - return + dirUnknown := version.Directory.IsUnknown() + archiveUnknown := version.ArchivePath.IsUnknown() + if !dirUnknown && !archiveUnknown { + if !dirSet && !archiveSet { + resp.Diagnostics.AddAttributeError( + req.Path.AtListIndex(i), + "Invalid Version Source", + "Exactly one of `directory` or `archive_path` must be specified for each template version.", + ) + return + } + if dirSet && archiveSet { + resp.Diagnostics.AddAttributeError( + req.Path.AtListIndex(i), + "Invalid Version Source", + "`directory` and `archive_path` are mutually exclusive for each template version.", + ) + return + } } - if archiveSet && !version.ArchivePath.IsUnknown() { + if archiveSet && !archiveUnknown { archivePath := version.ArchivePath.ValueString() if !strings.HasSuffix(archivePath, ".tar") && !strings.HasSuffix(archivePath, ".zip") { resp.Diagnostics.AddAttributeError( From 56807e6c0b94b72859067a99b78f87daee5bb51b Mon Sep 17 00:00:00 2001 From: ju-pe Date: Thu, 7 May 2026 17:58:30 +0000 Subject: [PATCH 03/10] fix: use path.Dir for zip entries and enhance switch warning - Use stdpath.Dir (stdlib path package) instead of filepath.Dir when traversing ZIP entry names. ZIP entries always use forward slashes per spec, and filepath.Dir is OS-specific (would produce backslashes on Windows). - Enhance the directory-to-archive_path switch warning to also mention that automatic tfvars file discovery is not performed for archive uploads, directing users to use tf_vars explicitly. --- internal/provider/template_resource.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/internal/provider/template_resource.go b/internal/provider/template_resource.go index bf0523b..613cccd 100644 --- a/internal/provider/template_resource.go +++ b/internal/provider/template_resource.go @@ -9,6 +9,7 @@ import ( "fmt" "io" "os" + stdpath "path" "path/filepath" "slices" "sort" @@ -1150,7 +1151,9 @@ func (d *versionsPlanModifier) PlanModifyList(ctx context.Context, req planmodif fmt.Sprintf( "Version %q (index %d) is switching from directory to archive_path. "+ "The archive may include hidden files (dotfiles) that were previously "+ - "excluded by the directory source.", + "excluded by the directory source. Additionally, automatic tfvars file "+ + "discovery (terraform.tfvars, *.auto.tfvars) is not performed for archive "+ + "uploads — use the `tf_vars` attribute to provide variable values explicitly.", planVersions[i].Name.ValueString(), i, ), ) @@ -1277,14 +1280,14 @@ func normalizeZip(archivePath string) ([]byte, error) { // Determine which intermediate directories are missing. missingDirs := make(map[string]bool) for _, f := range r.File { - dir := filepath.Dir(f.Name) + dir := stdpath.Dir(f.Name) for dir != "." && dir != "/" && dir != "" { dirEntry := dir + "/" if existingDirs[dirEntry] || missingDirs[dirEntry] { break } missingDirs[dirEntry] = true - dir = filepath.Dir(dir) + dir = stdpath.Dir(dir) } } From f2c6dc6506b556c31043dd4d03105f11dd271f00 Mon Sep 17 00:00:00 2001 From: ju-pe Date: Wed, 13 May 2026 18:01:28 +0000 Subject: [PATCH 04/10] refactor: consolidate hash tracking to directory_hash for both sources - Remove archive_hash attribute entirely - Store archive file hash in directory_hash (same as directory hashing) - Simplify contentHash() to only check directory_hash - Both directory and archive uploads now use directory_hash for change detection, which stores the hash of the actual content (not archive metadata) This unifies hash tracking regardless of source type, making the change detection logic simpler and ensuring the hash represents actual template content for both sources. --- internal/provider/template_resource.go | 23 +++------------------ internal/provider/template_resource_test.go | 8 +++---- 2 files changed, 7 insertions(+), 24 deletions(-) diff --git a/internal/provider/template_resource.go b/internal/provider/template_resource.go index 613cccd..414aa9c 100644 --- a/internal/provider/template_resource.go +++ b/internal/provider/template_resource.go @@ -169,18 +169,13 @@ type TemplateVersion struct { Directory types.String `tfsdk:"directory"` DirectoryHash types.String `tfsdk:"directory_hash"` ArchivePath types.String `tfsdk:"archive_path"` - ArchiveHash types.String `tfsdk:"archive_hash"` Active types.Bool `tfsdk:"active"` TerraformVariables []Variable `tfsdk:"tf_vars"` ProvisionerTags []Variable `tfsdk:"provisioner_tags"` } -// contentHash returns the relevant hash for change detection, regardless of -// whether the version source is a directory or an archive. +// contentHash returns the relevant hash for change detection. func (v TemplateVersion) contentHash() string { - if !v.ArchiveHash.IsNull() && !v.ArchiveHash.IsUnknown() && v.ArchiveHash.ValueString() != "" { - return v.ArchiveHash.ValueString() - } if !v.DirectoryHash.IsNull() && !v.DirectoryHash.IsUnknown() { return v.DirectoryHash.ValueString() } @@ -486,9 +481,6 @@ func (r *TemplateResource) Schema(ctx context.Context, req resource.SchemaReques Optional: true, MarkdownDescription: "A path to a `.tar` or `.zip` archive file to upload as the template version source. Mutually exclusive with `directory`. Changes in the archive contents will trigger the creation of a new template version. The archive must not exceed 100 MiB (the Coder server upload limit).", }, - "archive_hash": schema.StringAttribute{ - Computed: true, - }, "active": schema.BoolAttribute{ MarkdownDescription: "Whether this version is the active version of the template. Only one version can be active at a time.", Computed: true, @@ -627,9 +619,6 @@ func (r *TemplateResource) Create(ctx context.Context, req resource.CreateReques if data.Versions[idx].DirectoryHash.IsUnknown() { data.Versions[idx].DirectoryHash = types.StringValue("") } - if data.Versions[idx].ArchiveHash.IsUnknown() { - data.Versions[idx].ArchiveHash = types.StringValue("") - } } data.ID = UUIDValue(templateResp.ID) data.DisplayName = types.StringValue(templateResp.DisplayName) @@ -852,9 +841,6 @@ func (r *TemplateResource) Update(ctx context.Context, req resource.UpdateReques if newState.Versions[idx].DirectoryHash.IsUnknown() { newState.Versions[idx].DirectoryHash = types.StringValue("") } - if newState.Versions[idx].ArchiveHash.IsUnknown() { - newState.Versions[idx].ArchiveHash = types.StringValue("") - } if newState.Versions[idx].Active.ValueBool() { err := markActive(ctx, client, templateID, newState.Versions[idx].ID.ValueUUID()) if err != nil { @@ -893,9 +879,6 @@ func (r *TemplateResource) Update(ctx context.Context, req resource.UpdateReques if newState.Versions[idx].DirectoryHash.IsUnknown() { newState.Versions[idx].DirectoryHash = types.StringValue("") } - if newState.Versions[idx].ArchiveHash.IsUnknown() { - newState.Versions[idx].ArchiveHash = types.StringValue("") - } } } // TODO(ethanndickson): Remove this once the provider requires a Coder @@ -1105,11 +1088,11 @@ func (d *versionsPlanModifier) PlanModifyList(ctx context.Context, req planmodif resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to compute archive hash: %s", err)) return } - planVersions[i].ArchiveHash = types.StringValue(hash) + planVersions[i].DirectoryHash = types.StringValue(hash) } else if !planVersions[i].ArchivePath.IsNull() && planVersions[i].ArchivePath.IsUnknown() { // archive_path is set but not yet known (depends on another resource). // We can't compute the hash yet; mark it as unknown. - planVersions[i].ArchiveHash = types.StringUnknown() + planVersions[i].DirectoryHash = types.StringUnknown() } else if !planVersions[i].Directory.IsNull() && !planVersions[i].Directory.IsUnknown() { hash, err := computeDirectoryHash(planVersions[i].Directory.ValueString()) if err != nil { diff --git a/internal/provider/template_resource_test.go b/internal/provider/template_resource_test.go index 31d40ab..1ba259a 100644 --- a/internal/provider/template_resource_test.go +++ b/internal/provider/template_resource_test.go @@ -1599,7 +1599,7 @@ func TestReconcileVersionIDs(t *testing.T) { planVersions: []TemplateVersion{ { Name: types.StringValue("archive-ver"), - ArchiveHash: types.StringValue("archivehash123"), + DirectoryHash: types.StringValue("archivehash123"), ID: NewUUIDUnknown(), TerraformVariables: []Variable{}, }, @@ -1621,7 +1621,7 @@ func TestReconcileVersionIDs(t *testing.T) { expectedVersions: []TemplateVersion{ { Name: types.StringValue("archive-ver"), - ArchiveHash: types.StringValue("archivehash123"), + DirectoryHash: types.StringValue("archivehash123"), ID: UUIDValue(aUUID), TerraformVariables: []Variable{}, }, @@ -1632,7 +1632,7 @@ func TestReconcileVersionIDs(t *testing.T) { planVersions: []TemplateVersion{ { Name: types.StringValue("archive-ver"), - ArchiveHash: types.StringValue("newhash456"), + DirectoryHash: types.StringValue("newhash456"), ID: NewUUIDUnknown(), TerraformVariables: []Variable{}, }, @@ -1654,7 +1654,7 @@ func TestReconcileVersionIDs(t *testing.T) { expectedVersions: []TemplateVersion{ { Name: types.StringValue("archive-ver"), - ArchiveHash: types.StringValue("newhash456"), + DirectoryHash: types.StringValue("newhash456"), ID: NewUUIDUnknown(), TerraformVariables: []Variable{}, }, From 50bfa768fe2db7592152a694f4d6940efa0a7342 Mon Sep 17 00:00:00 2001 From: ju-pe Date: Wed, 13 May 2026 18:08:30 +0000 Subject: [PATCH 05/10] feat: add 100 MiB archive size validation Validate that uploaded archives do not exceed the 100 MiB server limit before attempting to upload. This provides faster feedback to users with oversized archives and prevents server-side upload failures. --- internal/provider/template_resource.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/internal/provider/template_resource.go b/internal/provider/template_resource.go index 414aa9c..1ff37eb 100644 --- a/internal/provider/template_resource.go +++ b/internal/provider/template_resource.go @@ -1444,6 +1444,15 @@ func newVersion(ctx context.Context, client *codersdk.Client, req newVersionRequ if !req.Version.ArchivePath.IsNull() && !req.Version.ArchivePath.IsUnknown() { archivePath := req.Version.ArchivePath.ValueString() + // Check archive file size (100 MiB limit) + const maxArchiveSize = 10 * (10 << 20) // 100 MiB + fileInfo, err := os.Stat(archivePath) + if err != nil { + return nil, logs, fmt.Errorf("failed to stat archive: %s", err) + } + if fileInfo.Size() > maxArchiveSize { + return nil, logs, fmt.Errorf("archive file exceeds 100 MiB limit: %d bytes", fileInfo.Size()) + } tflog.Info(ctx, "uploading archive", map[string]any{"archive_path": archivePath}) uploadResp, err = uploadArchive(ctx, client, archivePath) if err != nil { From f04ce646c75f03c15029e31ac6946b3400c1eed7 Mon Sep 17 00:00:00 2001 From: ju-pe Date: Wed, 13 May 2026 18:49:50 +0000 Subject: [PATCH 06/10] test: add archive path acceptance and edge case tests --- internal/provider/template_resource.go | 10 +- internal/provider/template_resource_test.go | 140 ++++++++++++++++++++ internal/provider/util.go | 13 ++ internal/provider/util_test.go | 54 ++++++++ 4 files changed, 209 insertions(+), 8 deletions(-) diff --git a/internal/provider/template_resource.go b/internal/provider/template_resource.go index 1ff37eb..cf1ba27 100644 --- a/internal/provider/template_resource.go +++ b/internal/provider/template_resource.go @@ -1444,14 +1444,8 @@ func newVersion(ctx context.Context, client *codersdk.Client, req newVersionRequ if !req.Version.ArchivePath.IsNull() && !req.Version.ArchivePath.IsUnknown() { archivePath := req.Version.ArchivePath.ValueString() - // Check archive file size (100 MiB limit) - const maxArchiveSize = 10 * (10 << 20) // 100 MiB - fileInfo, err := os.Stat(archivePath) - if err != nil { - return nil, logs, fmt.Errorf("failed to stat archive: %s", err) - } - if fileInfo.Size() > maxArchiveSize { - return nil, logs, fmt.Errorf("archive file exceeds 100 MiB limit: %d bytes", fileInfo.Size()) + if err := validateArchiveSize(archivePath); err != nil { + return nil, logs, err } tflog.Info(ctx, "uploading archive", map[string]any{"archive_path": archivePath}) uploadResp, err = uploadArchive(ctx, client, archivePath) diff --git a/internal/provider/template_resource_test.go b/internal/provider/template_resource_test.go index 1ba259a..e7ec369 100644 --- a/internal/provider/template_resource_test.go +++ b/internal/provider/template_resource_test.go @@ -1,9 +1,13 @@ package provider import ( + "archive/tar" + "archive/zip" + "bytes" "context" "fmt" "os" + "path/filepath" "regexp" "slices" "strings" @@ -1677,3 +1681,139 @@ func TestReconcileVersionIDs(t *testing.T) { } } + +func TestAccArchiveUploadFlow(t *testing.T) { + t.Parallel() + if os.Getenv("TF_ACC") == "" { + t.Skip("Acceptance tests are disabled.") + } + + t.Run("TarArchive", func(t *testing.T) { + t.Parallel() + tmpDir := t.TempDir() + + // Create a test file + testFile := filepath.Join(tmpDir, "template.tf") + err := os.WriteFile(testFile, []byte("resource \"null_resource\" \"test\" {}"), 0644) + require.NoError(t, err) + + // Create a tar archive + tarPath := filepath.Join(tmpDir, "template.tar") + tarFile, err := os.Create(tarPath) + require.NoError(t, err) + defer tarFile.Close() //nolint:errcheck + + tw := tar.NewWriter(tarFile) + defer tw.Close() //nolint:errcheck + + fileInfo, err := os.Stat(testFile) + require.NoError(t, err) + + header := &tar.Header{ + Name: "template.tf", + Size: fileInfo.Size(), + Mode: 0644, + } + err = tw.WriteHeader(header) + require.NoError(t, err) + + content, err := os.ReadFile(testFile) + require.NoError(t, err) + _, err = tw.Write(content) + require.NoError(t, err) + + // Verify archive content type + ct, err := archiveContentType(tarPath) + require.NoError(t, err) + require.Equal(t, "application/x-tar", ct) + + // Verify archive passes size validation + err = validateArchiveSize(tarPath) + require.NoError(t, err) + + // Verify archive hash is consistent + hash1, err := computeArchiveHash(tarPath) + require.NoError(t, err) + hash2, err := computeArchiveHash(tarPath) + require.NoError(t, err) + require.Equal(t, hash1, hash2) + }) + + t.Run("ZipArchive", func(t *testing.T) { + t.Parallel() + tmpDir := t.TempDir() + + // Create a test file + testFile := filepath.Join(tmpDir, "template.tf") + err := os.WriteFile(testFile, []byte("resource \"null_resource\" \"test\" {}"), 0644) + require.NoError(t, err) + + // Create a zip archive + zipPath := filepath.Join(tmpDir, "template.zip") + zipFile, err := os.Create(zipPath) + require.NoError(t, err) + defer zipFile.Close() //nolint:errcheck + + zw := zip.NewWriter(zipFile) + defer zw.Close() //nolint:errcheck + + content, err := os.ReadFile(testFile) + require.NoError(t, err) + + w, err := zw.Create("template.tf") + require.NoError(t, err) + _, err = w.Write(content) + require.NoError(t, err) + + // Verify archive content type + ct, err := archiveContentType(zipPath) + require.NoError(t, err) + require.Equal(t, "application/zip", ct) + + // Verify archive passes size validation + err = validateArchiveSize(zipPath) + require.NoError(t, err) + + // Verify archive hash is consistent + hash1, err := computeArchiveHash(zipPath) + require.NoError(t, err) + hash2, err := computeArchiveHash(zipPath) + require.NoError(t, err) + require.Equal(t, hash1, hash2) + }) + + t.Run("ZipArchiveNormalization", func(t *testing.T) { + t.Parallel() + tmpDir := t.TempDir() + + // Create a zip with a file in a nested directory but no directory entries + zipPath := filepath.Join(tmpDir, "template.zip") + zipFile, err := os.Create(zipPath) + require.NoError(t, err) + defer zipFile.Close() //nolint:errcheck + + zw := zip.NewWriter(zipFile) + w, err := zw.Create("subdir/template.tf") + require.NoError(t, err) + _, err = w.Write([]byte("resource \"null_resource\" \"test\" {}")) + require.NoError(t, err) + _ = zw.Close() //nolint:errcheck + + // Normalize the zip + normalized, err := normalizeZip(zipPath) + require.NoError(t, err) + + // Verify normalized zip has directory entry for "subdir/" + zr, err := zip.NewReader(bytes.NewReader(normalized), int64(len(normalized))) + require.NoError(t, err) + + hasSubdirEntry := false + for _, f := range zr.File { + if f.Name == "subdir/" { + hasSubdirEntry = true + break + } + } + require.True(t, hasSubdirEntry, "normalized zip should contain directory entry for subdir/") + }) +} diff --git a/internal/provider/util.go b/internal/provider/util.go index d02f5eb..a3cedee 100644 --- a/internal/provider/util.go +++ b/internal/provider/util.go @@ -159,3 +159,16 @@ func corsPtr(v types.String) *codersdk.CORSBehavior { b := codersdk.CORSBehavior(v.ValueString()) return &b } + +// validateArchiveSize checks that an archive file does not exceed the server upload limit. +func validateArchiveSize(archivePath string) error { + const maxArchiveSize = 10 * (10 << 20) // 100 MiB + fileInfo, err := os.Stat(archivePath) + if err != nil { + return fmt.Errorf("failed to stat archive: %s", err) + } + if fileInfo.Size() > maxArchiveSize { + return fmt.Errorf("archive file exceeds 100 MiB limit: %d bytes", fileInfo.Size()) + } + return nil +} diff --git a/internal/provider/util_test.go b/internal/provider/util_test.go index 8e5e448..28e7379 100644 --- a/internal/provider/util_test.go +++ b/internal/provider/util_test.go @@ -329,3 +329,57 @@ func TestArchiveContentType(t *testing.T) { }) } } + +func TestValidateArchiveSize(t *testing.T) { + t.Parallel() + + t.Run("ValidArchiveUnder100MiB", func(t *testing.T) { + t.Parallel() + tmpFile, err := os.CreateTemp(t.TempDir(), "archive_*.tar") + require.NoError(t, err) + defer tmpFile.Close() //nolint:errcheck + + // Create a 10 MiB file + _, err = tmpFile.Write(make([]byte, 10*1024*1024)) + require.NoError(t, err) + + err = validateArchiveSize(tmpFile.Name()) + require.NoError(t, err) + }) + + t.Run("InvalidArchiveExceeds100MiB", func(t *testing.T) { + t.Parallel() + tmpFile, err := os.CreateTemp(t.TempDir(), "archive_*.tar") + require.NoError(t, err) + defer tmpFile.Close() //nolint:errcheck + + // Create a 101 MiB file (exceeds limit) + _, err = tmpFile.Write(make([]byte, 101*1024*1024)) + require.NoError(t, err) + + err = validateArchiveSize(tmpFile.Name()) + require.Error(t, err) + require.Contains(t, err.Error(), "exceeds 100 MiB limit") + }) + + t.Run("NonexistentFile", func(t *testing.T) { + t.Parallel() + err := validateArchiveSize("/nonexistent/path/archive.tar") + require.Error(t, err) + require.Contains(t, err.Error(), "failed to stat archive") + }) + + t.Run("ExactlyAtLimit", func(t *testing.T) { + t.Parallel() + tmpFile, err := os.CreateTemp(t.TempDir(), "archive_*.tar") + require.NoError(t, err) + defer tmpFile.Close() //nolint:errcheck + + // Create exactly 100 MiB file + _, err = tmpFile.Write(make([]byte, 100*1024*1024)) + require.NoError(t, err) + + err = validateArchiveSize(tmpFile.Name()) + require.NoError(t, err) + }) +} From 9ebda2492d228aef2ab9bca9db7787f99958ebb5 Mon Sep 17 00:00:00 2001 From: ju-pe Date: Wed, 13 May 2026 18:59:05 +0000 Subject: [PATCH 07/10] docs: regenerate template documentation --- docs/resources/template.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/resources/template.md b/docs/resources/template.md index 4ab2320..7115b9a 100644 --- a/docs/resources/template.md +++ b/docs/resources/template.md @@ -105,7 +105,6 @@ Optional: Read-Only: -- `archive_hash` (String) - `directory_hash` (String) - `id` (String) From d014249205db936049ef31e85aea3f1afd6bfa0e Mon Sep 17 00:00:00 2001 From: ju-pe Date: Wed, 13 May 2026 20:00:45 +0000 Subject: [PATCH 08/10] refactor: remove normalizeZip - server handles zips without directory entries The Coder server's upload handler (coderd/files.go) converts uploaded ZIPs to tar via archive.CreateTarFromZip(), and the tar extractor (provisionersdk.Untar) calls os.MkdirAll(filepath.Dir(target)) for each file entry, creating parent directories automatically. There is no requirement for explicit directory entries in the uploaded ZIP. --- internal/provider/template_resource.go | 116 +----------- internal/provider/template_resource_test.go | 36 ---- internal/provider/util_test.go | 190 -------------------- 3 files changed, 5 insertions(+), 337 deletions(-) diff --git a/internal/provider/template_resource.go b/internal/provider/template_resource.go index cf1ba27..d64c5f3 100644 --- a/internal/provider/template_resource.go +++ b/internal/provider/template_resource.go @@ -1,18 +1,14 @@ package provider import ( - "archive/zip" "bufio" - "bytes" "context" "encoding/json" "fmt" "io" "os" - stdpath "path" "path/filepath" "slices" - "sort" "strings" "time" @@ -1241,121 +1237,19 @@ func archiveContentType(archivePath string) (string, error) { } } -// normalizeZip rewrites a zip archive to ensure that all intermediate directory -// entries exist. Some tools (e.g. Terraform's archive_file data source) produce -// zips that contain only file entries without explicit directory entries. Coder's -// server-side zip extractor expects directories to be present. -func normalizeZip(archivePath string) ([]byte, error) { - r, err := zip.OpenReader(archivePath) - if err != nil { - return nil, fmt.Errorf("failed to open zip: %w", err) - } - defer r.Close() //nolint:errcheck // Best-effort close of read-only zip. - - // Collect the set of directories that already have explicit entries. - existingDirs := make(map[string]bool) - for _, f := range r.File { - if f.FileInfo().IsDir() { - existingDirs[f.Name] = true - } - } - - // Determine which intermediate directories are missing. - missingDirs := make(map[string]bool) - for _, f := range r.File { - dir := stdpath.Dir(f.Name) - for dir != "." && dir != "/" && dir != "" { - dirEntry := dir + "/" - if existingDirs[dirEntry] || missingDirs[dirEntry] { - break - } - missingDirs[dirEntry] = true - dir = stdpath.Dir(dir) - } - } - - // If nothing is missing, read and return the original file as-is. - if len(missingDirs) == 0 { - data, err := os.ReadFile(archivePath) - if err != nil { - return nil, fmt.Errorf("failed to read archive: %w", err) - } - return data, nil - } - - // Sort missing dirs so parents come before children. - sortedMissing := make([]string, 0, len(missingDirs)) - for d := range missingDirs { - sortedMissing = append(sortedMissing, d) - } - sort.Strings(sortedMissing) - - // Rewrite the zip with directory entries first, then all original entries. - var buf bytes.Buffer - w := zip.NewWriter(&buf) - - // Add missing directory entries. - for _, d := range sortedMissing { - _, err := w.CreateHeader(&zip.FileHeader{ - Name: d, - ExternalAttrs: 0o755 << 16, // rwxr-xr-x in Unix mode - CreatorVersion: 3 << 8, // Unix - }) - if err != nil { - return nil, fmt.Errorf("failed to create directory entry %q: %w", d, err) - } - } - - // Copy all original entries. - for _, f := range r.File { - fw, err := w.CreateHeader(&f.FileHeader) - if err != nil { - return nil, fmt.Errorf("failed to create entry %q: %w", f.Name, err) - } - if f.FileInfo().IsDir() { - continue - } - rc, err := f.Open() - if err != nil { - return nil, fmt.Errorf("failed to open entry %q: %w", f.Name, err) - } - _, err = io.Copy(fw, rc) - _ = rc.Close() - if err != nil { - return nil, fmt.Errorf("failed to copy entry %q: %w", f.Name, err) - } - } - - if err := w.Close(); err != nil { - return nil, fmt.Errorf("failed to finalize zip: %w", err) - } - return buf.Bytes(), nil -} - func uploadArchive(ctx context.Context, client *codersdk.Client, archivePath string) (*codersdk.UploadResponse, error) { contentType, err := archiveContentType(archivePath) if err != nil { return nil, err } - var reader io.Reader - if contentType == codersdk.ContentTypeZip { - // Normalize the zip to ensure intermediate directory entries exist. - data, err := normalizeZip(archivePath) - if err != nil { - return nil, fmt.Errorf("failed to normalize zip: %w", err) - } - reader = bytes.NewReader(data) - } else { - f, err := os.Open(archivePath) - if err != nil { - return nil, fmt.Errorf("failed to open archive: %w", err) - } - defer f.Close() //nolint:errcheck // Best-effort close; upload already consumed the reader. - reader = bufio.NewReader(f) + f, err := os.Open(archivePath) + if err != nil { + return nil, fmt.Errorf("failed to open archive: %w", err) } + defer f.Close() //nolint:errcheck // Best-effort close; upload already consumed the reader. - resp, err := client.Upload(ctx, contentType, reader) + resp, err := client.Upload(ctx, contentType, bufio.NewReader(f)) if err != nil { return nil, err } diff --git a/internal/provider/template_resource_test.go b/internal/provider/template_resource_test.go index e7ec369..1ed7192 100644 --- a/internal/provider/template_resource_test.go +++ b/internal/provider/template_resource_test.go @@ -3,7 +3,6 @@ package provider import ( "archive/tar" "archive/zip" - "bytes" "context" "fmt" "os" @@ -1781,39 +1780,4 @@ func TestAccArchiveUploadFlow(t *testing.T) { require.NoError(t, err) require.Equal(t, hash1, hash2) }) - - t.Run("ZipArchiveNormalization", func(t *testing.T) { - t.Parallel() - tmpDir := t.TempDir() - - // Create a zip with a file in a nested directory but no directory entries - zipPath := filepath.Join(tmpDir, "template.zip") - zipFile, err := os.Create(zipPath) - require.NoError(t, err) - defer zipFile.Close() //nolint:errcheck - - zw := zip.NewWriter(zipFile) - w, err := zw.Create("subdir/template.tf") - require.NoError(t, err) - _, err = w.Write([]byte("resource \"null_resource\" \"test\" {}")) - require.NoError(t, err) - _ = zw.Close() //nolint:errcheck - - // Normalize the zip - normalized, err := normalizeZip(zipPath) - require.NoError(t, err) - - // Verify normalized zip has directory entry for "subdir/" - zr, err := zip.NewReader(bytes.NewReader(normalized), int64(len(normalized))) - require.NoError(t, err) - - hasSubdirEntry := false - for _, f := range zr.File { - if f.Name == "subdir/" { - hasSubdirEntry = true - break - } - } - require.True(t, hasSubdirEntry, "normalized zip should contain directory entry for subdir/") - }) } diff --git a/internal/provider/util_test.go b/internal/provider/util_test.go index 28e7379..65de828 100644 --- a/internal/provider/util_test.go +++ b/internal/provider/util_test.go @@ -4,7 +4,6 @@ import ( "archive/tar" "archive/zip" "bytes" - "io" "os" "path/filepath" "testing" @@ -91,195 +90,6 @@ func TestComputeArchiveHash(t *testing.T) { }) } -func TestNormalizeZip(t *testing.T) { - t.Parallel() - - t.Run("AddsMissingDirectoryEntries", func(t *testing.T) { - t.Parallel() - // Create a zip with only file entries (no directory entries), - // mimicking hashicorp/archive's archive_file data source. - zipPath := filepath.Join(t.TempDir(), "nodirs.zip") - var buf bytes.Buffer - zw := zip.NewWriter(&buf) - - // Top-level regular file. - fw, err := zw.Create("main.tf") - require.NoError(t, err) - _, err = fw.Write([]byte("resource {}")) - require.NoError(t, err) - - // Top-level hidden file (dotfile at root). - fw, err = zw.Create(".env") - require.NoError(t, err) - _, err = fw.Write([]byte("SECRET=value")) - require.NoError(t, err) - - // Top-level hidden directory with a file inside. - fw, err = zw.Create(".config/settings.json") - require.NoError(t, err) - _, err = fw.Write([]byte(`{"key": "value"}`)) - require.NoError(t, err) - - // Hidden file inside a regular directory. - fw, err = zw.Create("subdir/.hidden") - require.NoError(t, err) - _, err = fw.Write([]byte("hidden content")) - require.NoError(t, err) - - // Deeply nested hidden directory with a hidden file inside. - fw, err = zw.Create("a/b/.secret-dir/.credentials") - require.NoError(t, err) - _, err = fw.Write([]byte("token=abc123")) - require.NoError(t, err) - - require.NoError(t, zw.Close()) - err = os.WriteFile(zipPath, buf.Bytes(), 0644) - require.NoError(t, err) - - // Verify original has no directory entries. - origReader, err := zip.OpenReader(zipPath) - require.NoError(t, err) - for _, f := range origReader.File { - require.False(t, f.FileInfo().IsDir(), "expected no dir entries in original, got %q", f.Name) - } - require.NoError(t, origReader.Close()) - - // Normalize. - data, err := normalizeZip(zipPath) - require.NoError(t, err) - - // Read normalized zip and collect entries. - normReader, err := zip.NewReader(bytes.NewReader(data), int64(len(data))) - require.NoError(t, err) - - dirs := make(map[string]bool) - files := make(map[string]bool) - for _, f := range normReader.File { - if f.FileInfo().IsDir() { - dirs[f.Name] = true - } else { - files[f.Name] = true - } - } - - // All intermediate directories should exist, including hidden ones. - require.True(t, dirs[".config/"], "missing .config/") - require.True(t, dirs["subdir/"], "missing subdir/") - require.True(t, dirs["a/"], "missing a/") - require.True(t, dirs["a/b/"], "missing a/b/") - require.True(t, dirs["a/b/.secret-dir/"], "missing a/b/.secret-dir/") - - // All original files should still be present. - require.True(t, files["main.tf"], "missing main.tf") - require.True(t, files[".env"], "missing .env") - require.True(t, files[".config/settings.json"], "missing .config/settings.json") - require.True(t, files["subdir/.hidden"], "missing subdir/.hidden") - require.True(t, files["a/b/.secret-dir/.credentials"], "missing a/b/.secret-dir/.credentials") - }) - - t.Run("PreservesDirectoryPermissions", func(t *testing.T) { - t.Parallel() - // Verify that synthesized directory entries have proper mode bits - // so the server-side extractor can create them. - zipPath := filepath.Join(t.TempDir(), "needsperms.zip") - var buf bytes.Buffer - zw := zip.NewWriter(&buf) - - fw, err := zw.Create("deep/nested/dir/file.txt") - require.NoError(t, err) - _, err = fw.Write([]byte("content")) - require.NoError(t, err) - require.NoError(t, zw.Close()) - err = os.WriteFile(zipPath, buf.Bytes(), 0644) - require.NoError(t, err) - - data, err := normalizeZip(zipPath) - require.NoError(t, err) - - normReader, err := zip.NewReader(bytes.NewReader(data), int64(len(data))) - require.NoError(t, err) - - for _, f := range normReader.File { - if f.FileInfo().IsDir() { - mode := f.Mode() - require.NotZero(t, mode&0700, - "directory %q should have owner rwx bits set, got %s", f.Name, mode) - } - } - }) - - t.Run("NoChangeWhenDirsExist", func(t *testing.T) { - t.Parallel() - zipPath := filepath.Join(t.TempDir(), "withdirs.zip") - var buf bytes.Buffer - zw := zip.NewWriter(&buf) - - // Add directory entry explicitly. - _, err := zw.Create("subdir/") - require.NoError(t, err) - fw, err := zw.Create("subdir/file.txt") - require.NoError(t, err) - _, err = fw.Write([]byte("content")) - require.NoError(t, err) - require.NoError(t, zw.Close()) - - origData := buf.Bytes() - err = os.WriteFile(zipPath, origData, 0644) - require.NoError(t, err) - - // Normalize should return original bytes unchanged. - data, err := normalizeZip(zipPath) - require.NoError(t, err) - require.Equal(t, origData, data) - }) - - t.Run("PreservesFileContent", func(t *testing.T) { - t.Parallel() - // Verify normalization doesn't corrupt file contents, - // including hidden files at various levels. - zipPath := filepath.Join(t.TempDir(), "content.zip") - var buf bytes.Buffer - zw := zip.NewWriter(&buf) - - fileContents := map[string]string{ - ".gitignore": "node_modules/\n.env\n", - "main.tf": "resource \"null_resource\" \"test\" {}", - ".vscode/settings.json": `{"editor.formatOnSave": true}`, - "scripts/.local/bin/deploy": "#!/bin/bash\necho deploy", - } - for name, content := range fileContents { - fw, err := zw.Create(name) - require.NoError(t, err) - _, err = fw.Write([]byte(content)) - require.NoError(t, err) - } - require.NoError(t, zw.Close()) - err := os.WriteFile(zipPath, buf.Bytes(), 0644) - require.NoError(t, err) - - data, err := normalizeZip(zipPath) - require.NoError(t, err) - - normReader, err := zip.NewReader(bytes.NewReader(data), int64(len(data))) - require.NoError(t, err) - - for _, f := range normReader.File { - if f.FileInfo().IsDir() { - continue - } - expected, ok := fileContents[f.Name] - require.True(t, ok, "unexpected file in normalized zip: %q", f.Name) - rc, err := f.Open() - require.NoError(t, err) - actual, err := io.ReadAll(rc) - require.NoError(t, rc.Close()) - require.NoError(t, err) - require.Equal(t, expected, string(actual), - "content mismatch for %q", f.Name) - } - }) -} - func TestArchiveContentType(t *testing.T) { t.Parallel() From 78bfe82981e7bbe7d3e41128c6fa8391ebc0800b Mon Sep 17 00:00:00 2001 From: ju-pe Date: Wed, 13 May 2026 22:22:08 +0000 Subject: [PATCH 09/10] refactor: inline contentHash - DirectoryHash is always set --- internal/provider/template_resource.go | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/internal/provider/template_resource.go b/internal/provider/template_resource.go index d64c5f3..e5302eb 100644 --- a/internal/provider/template_resource.go +++ b/internal/provider/template_resource.go @@ -170,14 +170,6 @@ type TemplateVersion struct { ProvisionerTags []Variable `tfsdk:"provisioner_tags"` } -// contentHash returns the relevant hash for change detection. -func (v TemplateVersion) contentHash() string { - if !v.DirectoryHash.IsNull() && !v.DirectoryHash.IsUnknown() { - return v.DirectoryHash.ValueString() - } - return "" -} - type Versions []TemplateVersion func (v Versions) ByID(id UUID) *TemplateVersion { @@ -1613,7 +1605,7 @@ type privateState interface { func (v Versions) setPrivateState(ctx context.Context, ps privateState) (diags diag.Diagnostics) { lv := make(LastVersionsByHash) for _, version := range v { - vbh, ok := lv[version.contentHash()] + vbh, ok := lv[version.DirectoryHash.ValueString()] tfVars := make(map[string]string, len(version.TerraformVariables)) for _, tfVar := range version.TerraformVariables { tfVars[tfVar.Name.ValueString()] = tfVar.Value.ValueString() @@ -1621,14 +1613,14 @@ func (v Versions) setPrivateState(ctx context.Context, ps privateState) (diags d // Store the IDs and names of all versions with the same directory hash, // in the order they appear if ok { - lv[version.contentHash()] = append(vbh, PreviousTemplateVersion{ + lv[version.DirectoryHash.ValueString()] = append(vbh, PreviousTemplateVersion{ ID: version.ID.ValueUUID(), Name: version.Name.ValueString(), TFVars: tfVars, Active: version.Active.ValueBool(), }) } else { - lv[version.contentHash()] = []PreviousTemplateVersion{ + lv[version.DirectoryHash.ValueString()] = []PreviousTemplateVersion{ { ID: version.ID.ValueUUID(), Name: version.Name.ValueString(), @@ -1655,7 +1647,7 @@ func (planVersions Versions) reconcileVersionIDs(lv LastVersionsByHash, configVe } for i := range planVersions { - prevList, ok := lv[planVersions[i].contentHash()] + prevList, ok := lv[planVersions[i].DirectoryHash.ValueString()] // If not in state, mark as known after apply since we'll create a new version. // Versions whose Terraform configuration has not changed will have known // IDs at this point, so we need to set this manually. @@ -1674,7 +1666,7 @@ func (planVersions Versions) reconcileVersionIDs(lv LastVersionsByHash, configVe // it from the previous version candidates if planVersions[i].Name.ValueString() == prev.Name { planVersions[i].ID = UUIDValue(prev.ID) - lv[planVersions[i].contentHash()] = append(prevList[:j], prevList[j+1:]...) + lv[planVersions[i].DirectoryHash.ValueString()] = append(prevList[:j], prevList[j+1:]...) break } } @@ -1684,13 +1676,13 @@ func (planVersions Versions) reconcileVersionIDs(lv LastVersionsByHash, configVe // For versions whose hash was found in the private state but couldn't be // matched, use the leftovers in the order they appear for i := range planVersions { - prevList := lv[planVersions[i].contentHash()] + prevList := lv[planVersions[i].DirectoryHash.ValueString()] if len(prevList) > 0 && planVersions[i].ID.IsUnknown() { planVersions[i].ID = UUIDValue(prevList[0].ID) if planVersions[i].Name.IsUnknown() { planVersions[i].Name = types.StringValue(prevList[0].Name) } - lv[planVersions[i].contentHash()] = prevList[1:] + lv[planVersions[i].DirectoryHash.ValueString()] = prevList[1:] } } @@ -1698,7 +1690,7 @@ func (planVersions Versions) reconcileVersionIDs(lv LastVersionsByHash, configVe // we need to create a new version with the new variables. for i := range planVersions { if !planVersions[i].ID.IsUnknown() { - prevs, ok := fullLv[planVersions[i].contentHash()] + prevs, ok := fullLv[planVersions[i].DirectoryHash.ValueString()] if !ok { continue } @@ -1722,7 +1714,7 @@ func (planVersions Versions) reconcileVersionIDs(lv LastVersionsByHash, configVe if !hasOneActiveVersion { for i := range planVersions { if !planVersions[i].ID.IsUnknown() { - prevs, ok := fullLv[planVersions[i].contentHash()] + prevs, ok := fullLv[planVersions[i].DirectoryHash.ValueString()] if !ok { continue } From 2412e5bd142fdb92b3defed5ec2e960fe8931cf2 Mon Sep 17 00:00:00 2001 From: ju-pe Date: Wed, 13 May 2026 22:29:02 +0000 Subject: [PATCH 10/10] refactor: remove contentHash indirection, compute hash at apply time when unknown - Removed contentHash() helper; inline DirectoryHash.ValueString() at call sites - Replace empty-string fallback with actual hash computation in Create/Update when DirectoryHash is unknown (source path depended on another resource at plan time) - Remove stale archive_hash comments --- internal/provider/template_resource.go | 51 +++++++++++++++++++++----- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/internal/provider/template_resource.go b/internal/provider/template_resource.go index e5302eb..2c5018a 100644 --- a/internal/provider/template_resource.go +++ b/internal/provider/template_resource.go @@ -601,11 +601,21 @@ func (r *TemplateResource) Create(ctx context.Context, req resource.CreateReques } data.Versions[idx].ID = UUIDValue(versionResp.ID) data.Versions[idx].Name = types.StringValue(versionResp.Name) - // Ensure computed hash fields have concrete values after apply. - // Only one of directory_hash or archive_hash will be meaningful; - // set the other to empty string so Terraform doesn't see unknown values. + // If the plan modifier couldn't compute the hash (source path was unknown + // at plan time), compute it now that all values are resolved. if data.Versions[idx].DirectoryHash.IsUnknown() { - data.Versions[idx].DirectoryHash = types.StringValue("") + var hash string + var hashErr error + if !data.Versions[idx].ArchivePath.IsNull() { + hash, hashErr = computeArchiveHash(data.Versions[idx].ArchivePath.ValueString()) + } else { + hash, hashErr = computeDirectoryHash(data.Versions[idx].Directory.ValueString()) + } + if hashErr != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to compute content hash: %s", hashErr)) + return + } + data.Versions[idx].DirectoryHash = types.StringValue(hash) } } data.ID = UUIDValue(templateResp.ID) @@ -825,9 +835,21 @@ func (r *TemplateResource) Update(ctx context.Context, req resource.UpdateReques } newState.Versions[idx].ID = UUIDValue(versionResp.ID) newState.Versions[idx].Name = types.StringValue(versionResp.Name) - // Ensure computed hash fields have concrete values after apply. + // If the plan modifier couldn't compute the hash (source path was unknown + // at plan time), compute it now that all values are resolved. if newState.Versions[idx].DirectoryHash.IsUnknown() { - newState.Versions[idx].DirectoryHash = types.StringValue("") + var hash string + var hashErr error + if !newState.Versions[idx].ArchivePath.IsNull() { + hash, hashErr = computeArchiveHash(newState.Versions[idx].ArchivePath.ValueString()) + } else { + hash, hashErr = computeDirectoryHash(newState.Versions[idx].Directory.ValueString()) + } + if hashErr != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to compute content hash: %s", hashErr)) + return + } + newState.Versions[idx].DirectoryHash = types.StringValue(hash) } if newState.Versions[idx].Active.ValueBool() { err := markActive(ctx, client, templateID, newState.Versions[idx].ID.ValueUUID()) @@ -862,10 +884,21 @@ func (r *TemplateResource) Update(ctx context.Context, req resource.UpdateReques return } } - // Ensure computed hash fields have concrete values after apply, - // even for reused versions. + // If the plan modifier couldn't compute the hash (source path was unknown + // at plan time), compute it now that all values are resolved. if newState.Versions[idx].DirectoryHash.IsUnknown() { - newState.Versions[idx].DirectoryHash = types.StringValue("") + var hash string + var hashErr error + if !newState.Versions[idx].ArchivePath.IsNull() { + hash, hashErr = computeArchiveHash(newState.Versions[idx].ArchivePath.ValueString()) + } else { + hash, hashErr = computeDirectoryHash(newState.Versions[idx].Directory.ValueString()) + } + if hashErr != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to compute content hash: %s", hashErr)) + return + } + newState.Versions[idx].DirectoryHash = types.StringValue(hash) } } }