Generated archive contents include an extra (empty) file when `output_path` is configured within same directory as `source_dir`.

[theipster]

Terraform CLI and Provider Versions

All Terraform versions covered by the CI test suite (0.12.31, 0.13.7, 0.14.11, 0.15.1).

Terraform Configuration

data "archive_file" "foo" {
  type        = "zip"
  source_dir  = "${path.module}/foo"
  output_path = "${path.module}/foo/bar.zip"
}

Expected Behavior

The output archive bar.zip's contents should exactly replicate the files contained in /foo prior to Terraform execution.

For example, if the /foo directory structure looks like:

/foo
  - baz.txt

then the expected output archive contents should look like:

/foo
  - baz.txt

Specifically, if the /foo source directory does not contain an empty file named bar.zip prior to Terraform execution, then the output archive bar.zip's contents should also not contain an empty file named bar.zip.

Also, ideally, if the /foo source directory does contain a file named bar.zip prior to Terraform execution, then the output archive bar.zip's contents should also contain a file named bar.zip with the same file contents.

Actual Behavior

The output archive bar.zip's contents includes an extra (empty) file named bar.zip.

For example, if the /foo directory structure looks like:

/foo
  - baz.txt

then the actual output archive contents looks like:

/foo
  - bar.zip
  - baz.txt

Steps to Reproduce

This can be replicated with an extra unit test in internal/provider/zip_archiver_test.go:

func TestZipArchiver_Dir_OutputSameDir(t *testing.T) {
	zipfilepath := "./test-fixtures/test-dir/archive-dir.zip"
	archiver := NewZipArchiver(zipfilepath)
	defer os.Remove(zipfilepath)
	if err := archiver.ArchiveDir("./test-fixtures/test-dir", []string{""}); err != nil {
		t.Fatalf("unexpected error: %s", err)
	}

	ensureContents(t, zipfilepath, map[string][]byte{
		"file1.txt": []byte("This is file 1"),
		"file2.txt": []byte("This is file 2"),
		"file3.txt": []byte("This is file 3"),
	})
}

which produces the following test output:

$ go test -v -cover ./internal/provider/

...

=== RUN   TestZipArchiver_FileModified
--- PASS: TestZipArchiver_FileModified (0.00s)
=== RUN   TestZipArchiver_Dir
--- PASS: TestZipArchiver_Dir (0.00s)
=== RUN   TestZipArchiver_Dir_OutputSameDir
    zip_archiver_test.go:121: mismatched file count, got 4, want 3
    zip_archiver_test.go:121: additional file in zip: archive-dir.zip
=== RUN   TestZipArchiver_Dir_Exclude
--- PASS: TestZipArchiver_Dir_Exclude (0.00s)
=== RUN   TestZipArchiver_Dir_Exclude_With_Directory
--- PASS: TestZipArchiver_Dir_Exclude_With_Directory (0.00s)

How much impact is this issue causing?

Medium

Logs

No response

Additional Information

The reason this bug occurs is because in the ArchiveDir() implementation (see internal/provider/zip_archiver.go), a.open() is called before filepath.Walk().

a.open() calls os.Create(a.filepath) and therefore creates the empty file in the source directory. filepath.Walk() then proceeds to iterate through the source directory including this new empty file.

Potential solutions

1. Replace os.Create(a.filepath) with os.CreateTemp("", "*.zip") to initially build the archive file in a temporary location. Once built, call os.Rename() (or io.Copy() plus os.Remove()) to move the archive file to the target path.

2. Dynamically append the output path a.filepath to the excludes list, so that it will exclude itself. (However, this solution won't handle the scenario when there is a legitimate source file at path a.filepath that needs to be included.)

Code of Conduct

• [X] I agree to follow this project's Code of Conduct

[theipster]

On a related note, I noticed that all of the unit tests that involve generating a zip file (e.g. archive-dir.zip) don't clean up after themselves.

This could be fixed with an extra defer os.Remove(zipfilepath) (for example, see my Steps to reproduce above).

[theipster]

Rebased #175 onto c16f42e9.

(I'm aware that #170 cleans up the orphaned zip files for a lot of these tests, but I believe there are still a few gaps that were missed that #175 will cover.)