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.

Author: PepziC0les

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
 	}

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/")
-	})
 }

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()