From b09fe9ac1f0790806c567aaad7cb4ac20d51ca3b Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 15 Nov 2024 17:34:30 +0000 Subject: [PATCH 1/5] feat(testutil): extract testutil.Create(Zip|Tar) helpers --- provisioner/terraform/tfparse/tfparse_test.go | 41 ++-------------- provisionerd/provisionerd_test.go | 48 +++++-------------- testutil/archive.go | 46 ++++++++++++++++++ 3 files changed, 63 insertions(+), 72 deletions(-) create mode 100644 testutil/archive.go diff --git a/provisioner/terraform/tfparse/tfparse_test.go b/provisioner/terraform/tfparse/tfparse_test.go index a08f9ff76887e..b47be7e99530a 100644 --- a/provisioner/terraform/tfparse/tfparse_test.go +++ b/provisioner/terraform/tfparse/tfparse_test.go @@ -1,8 +1,6 @@ package tfparse_test import ( - "archive/tar" - "bytes" "context" "io" "log" @@ -11,7 +9,6 @@ import ( "cdr.dev/slog" "cdr.dev/slog/sloggers/sloghuman" "cdr.dev/slog/sloggers/slogtest" - "github.com/coder/coder/v2/archive" "github.com/coder/coder/v2/provisioner/terraform/tfparse" "github.com/coder/coder/v2/testutil" @@ -363,7 +360,7 @@ func Test_WorkspaceTagDefaultsFromFile(t *testing.T) { t.Run(tc.name+"/tar", func(t *testing.T) { t.Parallel() ctx := testutil.Context(t, testutil.WaitShort) - tar := createTar(t, tc.files) + tar := testutil.CreateTar(t, tc.files) logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug) tmpDir := t.TempDir() tfparse.WriteArchive(tar, "application/x-tar", tmpDir) @@ -381,7 +378,7 @@ func Test_WorkspaceTagDefaultsFromFile(t *testing.T) { t.Run(tc.name+"/zip", func(t *testing.T) { t.Parallel() ctx := testutil.Context(t, testutil.WaitShort) - zip := createZip(t, tc.files) + zip := testutil.CreateZip(t, tc.files) logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug) tmpDir := t.TempDir() tfparse.WriteArchive(zip, "application/zip", tmpDir) @@ -399,36 +396,6 @@ func Test_WorkspaceTagDefaultsFromFile(t *testing.T) { } } -func createTar(t testing.TB, files map[string]string) []byte { - var buffer bytes.Buffer - writer := tar.NewWriter(&buffer) - for path, content := range files { - err := writer.WriteHeader(&tar.Header{ - Name: path, - Size: int64(len(content)), - Uid: 65534, // nobody - Gid: 65534, // nogroup - Mode: 0o666, // -rw-rw-rw- - }) - require.NoError(t, err) - - _, err = writer.Write([]byte(content)) - require.NoError(t, err) - } - - err := writer.Flush() - require.NoError(t, err) - return buffer.Bytes() -} - -func createZip(t testing.TB, files map[string]string) []byte { - ta := createTar(t, files) - tr := tar.NewReader(bytes.NewReader(ta)) - za, err := archive.CreateZipFromTar(tr, int64(len(ta))) - require.NoError(t, err) - return za -} - // Last run results: // goos: linux // goarch: amd64 @@ -460,8 +427,8 @@ func BenchmarkWorkspaceTagDefaultsFromFile(b *testing.B) { } }`, } - tarFile := createTar(b, files) - zipFile := createZip(b, files) + tarFile := testutil.CreateTar(b, files) + zipFile := testutil.CreateZip(b, files) logger := discardLogger(b) b.ResetTimer() b.Run("Tar", func(b *testing.B) { diff --git a/provisionerd/provisionerd_test.go b/provisionerd/provisionerd_test.go index b3cf08fc1ca5a..a0e15435587e8 100644 --- a/provisionerd/provisionerd_test.go +++ b/provisionerd/provisionerd_test.go @@ -1,8 +1,6 @@ package provisionerd_test import ( - "archive/tar" - "bytes" "context" "fmt" "io" @@ -97,7 +95,7 @@ func TestProvisionerd(t *testing.T) { err := stream.Send(&proto.AcquiredJob{ JobId: "test", Provisioner: "someprovisioner", - TemplateSourceArchive: createTar(t, map[string]string{ + TemplateSourceArchive: testutil.CreateTar(t, map[string]string{ "test.txt": "content", }), Type: &proto.AcquiredJob_TemplateImport_{ @@ -150,7 +148,7 @@ func TestProvisionerd(t *testing.T) { acq = newAcquireOne(t, &proto.AcquiredJob{ JobId: "test", Provisioner: "someprovisioner", - TemplateSourceArchive: createTar(t, map[string]string{ + TemplateSourceArchive: testutil.CreateTar(t, map[string]string{ "../../../etc/passwd": "content", }), Type: &proto.AcquiredJob_TemplateImport_{ @@ -194,7 +192,7 @@ func TestProvisionerd(t *testing.T) { err := stream.Send(&proto.AcquiredJob{ JobId: "test", Provisioner: "someprovisioner", - TemplateSourceArchive: createTar(t, map[string]string{ + TemplateSourceArchive: testutil.CreateTar(t, map[string]string{ "test.txt": "content", }), Type: &proto.AcquiredJob_TemplateImport_{ @@ -243,7 +241,7 @@ func TestProvisionerd(t *testing.T) { acq = newAcquireOne(t, &proto.AcquiredJob{ JobId: "test", Provisioner: "someprovisioner", - TemplateSourceArchive: createTar(t, map[string]string{ + TemplateSourceArchive: testutil.CreateTar(t, map[string]string{ "test.txt": "content", provisionersdk.ReadmeFile: "# A cool template 😎\n", }), @@ -325,7 +323,7 @@ func TestProvisionerd(t *testing.T) { acq = newAcquireOne(t, &proto.AcquiredJob{ JobId: "test", Provisioner: "someprovisioner", - TemplateSourceArchive: createTar(t, map[string]string{ + TemplateSourceArchive: testutil.CreateTar(t, map[string]string{ "test.txt": "content", }), Type: &proto.AcquiredJob_TemplateDryRun_{ @@ -396,7 +394,7 @@ func TestProvisionerd(t *testing.T) { acq = newAcquireOne(t, &proto.AcquiredJob{ JobId: "test", Provisioner: "someprovisioner", - TemplateSourceArchive: createTar(t, map[string]string{ + TemplateSourceArchive: testutil.CreateTar(t, map[string]string{ "test.txt": "content", }), Type: &proto.AcquiredJob_WorkspaceBuild_{ @@ -459,7 +457,7 @@ func TestProvisionerd(t *testing.T) { acq = newAcquireOne(t, &proto.AcquiredJob{ JobId: "test", Provisioner: "someprovisioner", - TemplateSourceArchive: createTar(t, map[string]string{ + TemplateSourceArchive: testutil.CreateTar(t, map[string]string{ "test.txt": "content", }), Type: &proto.AcquiredJob_WorkspaceBuild_{ @@ -549,7 +547,7 @@ func TestProvisionerd(t *testing.T) { acq = newAcquireOne(t, &proto.AcquiredJob{ JobId: "test", Provisioner: "someprovisioner", - TemplateSourceArchive: createTar(t, map[string]string{ + TemplateSourceArchive: testutil.CreateTar(t, map[string]string{ "test.txt": "content", }), Type: &proto.AcquiredJob_WorkspaceBuild_{ @@ -645,7 +643,7 @@ func TestProvisionerd(t *testing.T) { err := stream.Send(&proto.AcquiredJob{ JobId: "test", Provisioner: "someprovisioner", - TemplateSourceArchive: createTar(t, map[string]string{ + TemplateSourceArchive: testutil.CreateTar(t, map[string]string{ "test.txt": "content", }), Type: &proto.AcquiredJob_WorkspaceBuild_{ @@ -725,7 +723,7 @@ func TestProvisionerd(t *testing.T) { err := stream.Send(&proto.AcquiredJob{ JobId: "test", Provisioner: "someprovisioner", - TemplateSourceArchive: createTar(t, map[string]string{ + TemplateSourceArchive: testutil.CreateTar(t, map[string]string{ "test.txt": "content", }), Type: &proto.AcquiredJob_WorkspaceBuild_{ @@ -819,7 +817,7 @@ func TestProvisionerd(t *testing.T) { job := &proto.AcquiredJob{ JobId: "test", Provisioner: "someprovisioner", - TemplateSourceArchive: createTar(t, map[string]string{ + TemplateSourceArchive: testutil.CreateTar(t, map[string]string{ "test.txt": "content", }), Type: &proto.AcquiredJob_WorkspaceBuild_{ @@ -916,7 +914,7 @@ func TestProvisionerd(t *testing.T) { job := &proto.AcquiredJob{ JobId: "test", Provisioner: "someprovisioner", - TemplateSourceArchive: createTar(t, map[string]string{ + TemplateSourceArchive: testutil.CreateTar(t, map[string]string{ "test.txt": "content", }), Type: &proto.AcquiredJob_WorkspaceBuild_{ @@ -1010,7 +1008,7 @@ func TestProvisionerd(t *testing.T) { err := stream.Send(&proto.AcquiredJob{ JobId: "test", Provisioner: "someprovisioner", - TemplateSourceArchive: createTar(t, map[string]string{ + TemplateSourceArchive: testutil.CreateTar(t, map[string]string{ "test.txt": "content", }), Type: &proto.AcquiredJob_WorkspaceBuild_{ @@ -1078,26 +1076,6 @@ func TestProvisionerd(t *testing.T) { }) } -// Creates an in-memory tar of the files provided. -func createTar(t *testing.T, files map[string]string) []byte { - var buffer bytes.Buffer - writer := tar.NewWriter(&buffer) - for path, content := range files { - err := writer.WriteHeader(&tar.Header{ - Name: path, - Size: int64(len(content)), - }) - require.NoError(t, err) - - _, err = writer.Write([]byte(content)) - require.NoError(t, err) - } - - err := writer.Flush() - require.NoError(t, err) - return buffer.Bytes() -} - // Creates a provisionerd implementation with the provided dialer and provisioners. func createProvisionerd(t *testing.T, dialer provisionerd.Dialer, connector provisionerd.LocalProvisioners) *provisionerd.Server { server := provisionerd.New(dialer, &provisionerd.Options{ diff --git a/testutil/archive.go b/testutil/archive.go new file mode 100644 index 0000000000000..630a66f4b3624 --- /dev/null +++ b/testutil/archive.go @@ -0,0 +1,46 @@ +package testutil + +import ( + "archive/tar" + "bytes" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/archive" +) + +// Creates an in-memory tar of the files provided. +// Files in the archive are written with nobody +// owner/group, and -rw-rw-rw- permissions. +func CreateTar(t testing.TB, files map[string]string) []byte { + var buffer bytes.Buffer + writer := tar.NewWriter(&buffer) + for path, content := range files { + err := writer.WriteHeader(&tar.Header{ + Name: path, + Size: int64(len(content)), + Uid: 65534, // nobody + Gid: 65534, // nogroup + Mode: 0o666, // -rw-rw-rw- + }) + require.NoError(t, err) + + _, err = writer.Write([]byte(content)) + require.NoError(t, err) + } + + err := writer.Flush() + require.NoError(t, err) + return buffer.Bytes() +} + +// Creates an in-memory zip of the files provided. +// Uses archive.CreateZipFromTar under the hood. +func CreateZip(t testing.TB, files map[string]string) []byte { + ta := CreateTar(t, files) + tr := tar.NewReader(bytes.NewReader(ta)) + za, err := archive.CreateZipFromTar(tr, int64(len(ta))) + require.NoError(t, err) + return za +} From a81b5617e1fa022d0458d6db5cb0e41e6495f338 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 15 Nov 2024 18:11:12 +0000 Subject: [PATCH 2/5] found another one --- provisioner/terraform/parse_test.go | 3 +- provisioner/terraform/provision_test.go | 51 ++++++++++++------------- provisioner/terraform/timings_test.go | 2 +- 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/provisioner/terraform/parse_test.go b/provisioner/terraform/parse_test.go index 3ff6181dc8624..7d176cb886469 100644 --- a/provisioner/terraform/parse_test.go +++ b/provisioner/terraform/parse_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/require" "github.com/coder/coder/v2/provisionersdk/proto" + "github.com/coder/coder/v2/testutil" ) func TestParse(t *testing.T) { @@ -380,7 +381,7 @@ func TestParse(t *testing.T) { t.Parallel() session := configure(ctx, t, api, &proto.Config{ - TemplateSourceArchive: makeTar(t, testCase.Files), + TemplateSourceArchive: testutil.CreateTar(t, testCase.Files), }) err := session.Send(&proto.Request{Type: &proto.Request_Parse{Parse: &proto.ParseRequest{}}}) diff --git a/provisioner/terraform/provision_test.go b/provisioner/terraform/provision_test.go index 0ffbe7903ec08..6a4976e1fd951 100644 --- a/provisioner/terraform/provision_test.go +++ b/provisioner/terraform/provision_test.go @@ -3,8 +3,6 @@ package terraform_test import ( - "archive/tar" - "bytes" "context" "encoding/json" "errors" @@ -28,6 +26,7 @@ import ( "github.com/coder/coder/v2/provisioner/terraform" "github.com/coder/coder/v2/provisionersdk" "github.com/coder/coder/v2/provisionersdk/proto" + "github.com/coder/coder/v2/testutil" ) type provisionerServeOptions struct { @@ -78,24 +77,24 @@ func setupProvisioner(t *testing.T, opts *provisionerServeOptions) (context.Cont return ctx, api } -func makeTar(t *testing.T, files map[string]string) []byte { - t.Helper() - var buffer bytes.Buffer - writer := tar.NewWriter(&buffer) - for name, content := range files { - err := writer.WriteHeader(&tar.Header{ - Name: name, - Size: int64(len(content)), - Mode: 0o644, - }) - require.NoError(t, err) - _, err = writer.Write([]byte(content)) - require.NoError(t, err) - } - err := writer.Flush() - require.NoError(t, err) - return buffer.Bytes() -} +// func testutil.CreateTar(t *testing.T, files map[string]string) []byte { +// t.Helper() +// var buffer bytes.Buffer +// writer := tar.NewWriter(&buffer) +// for name, content := range files { +// err := writer.WriteHeader(&tar.Header{ +// Name: name, +// Size: int64(len(content)), +// Mode: 0o644, +// }) +// require.NoError(t, err) +// _, err = writer.Write([]byte(content)) +// require.NoError(t, err) +// } +// err := writer.Flush() +// require.NoError(t, err) +// return buffer.Bytes() +// } func configure(ctx context.Context, t *testing.T, client proto.DRPCProvisionerClient, config *proto.Config) proto.DRPCProvisioner_SessionClient { t.Helper() @@ -186,7 +185,7 @@ func TestProvision_Cancel(t *testing.T) { binaryPath: binPath, }) sess := configure(ctx, t, api, &proto.Config{ - TemplateSourceArchive: makeTar(t, nil), + TemplateSourceArchive: testutil.CreateTar(t, nil), }) err = sendPlan(sess, proto.WorkspaceTransition_START) @@ -257,7 +256,7 @@ func TestProvision_CancelTimeout(t *testing.T) { }) sess := configure(ctx, t, api, &proto.Config{ - TemplateSourceArchive: makeTar(t, nil), + TemplateSourceArchive: testutil.CreateTar(t, nil), }) // provisioner requires plan before apply, so test cancel with plan. @@ -346,7 +345,7 @@ func TestProvision_TextFileBusy(t *testing.T) { }) sess := configure(ctx, t, api, &proto.Config{ - TemplateSourceArchive: makeTar(t, nil), + TemplateSourceArchive: testutil.CreateTar(t, nil), }) err = sendPlan(sess, proto.WorkspaceTransition_START) @@ -758,7 +757,7 @@ func TestProvision(t *testing.T) { ctx, api := setupProvisioner(t, nil) sess := configure(ctx, t, api, &proto.Config{ - TemplateSourceArchive: makeTar(t, testCase.Files), + TemplateSourceArchive: testutil.CreateTar(t, testCase.Files), }) planRequest := &proto.Request{Type: &proto.Request_Plan{Plan: &proto.PlanRequest{ @@ -863,7 +862,7 @@ func TestProvision_ExtraEnv(t *testing.T) { ctx, api := setupProvisioner(t, nil) sess := configure(ctx, t, api, &proto.Config{ - TemplateSourceArchive: makeTar(t, map[string]string{"main.tf": `resource "null_resource" "A" {}`}), + TemplateSourceArchive: testutil.CreateTar(t, map[string]string{"main.tf": `resource "null_resource" "A" {}`}), }) err := sendPlan(sess, proto.WorkspaceTransition_START) @@ -913,7 +912,7 @@ func TestProvision_SafeEnv(t *testing.T) { ctx, api := setupProvisioner(t, nil) sess := configure(ctx, t, api, &proto.Config{ - TemplateSourceArchive: makeTar(t, map[string]string{"main.tf": echoResource}), + TemplateSourceArchive: testutil.CreateTar(t, map[string]string{"main.tf": echoResource}), }) err := sendPlan(sess, proto.WorkspaceTransition_START) diff --git a/provisioner/terraform/timings_test.go b/provisioner/terraform/timings_test.go index 0d11040db9bfc..e128b4d654d56 100644 --- a/provisioner/terraform/timings_test.go +++ b/provisioner/terraform/timings_test.go @@ -34,7 +34,7 @@ func TestTimingsFromProvision(t *testing.T) { binaryPath: fakeBin, }) sess := configure(ctx, t, api, &proto.Config{ - TemplateSourceArchive: makeTar(t, nil), + TemplateSourceArchive: testutil.CreateTar(t, nil), }) ctx, cancel := context.WithTimeout(ctx, testutil.WaitLong) From 482b9b987e239c179e121601f98edf31e59d3c5e Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 18 Nov 2024 08:22:02 +0000 Subject: [PATCH 3/5] rm commented code --- provisioner/terraform/provision_test.go | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/provisioner/terraform/provision_test.go b/provisioner/terraform/provision_test.go index 6a4976e1fd951..8fbe79e3ea242 100644 --- a/provisioner/terraform/provision_test.go +++ b/provisioner/terraform/provision_test.go @@ -77,25 +77,6 @@ func setupProvisioner(t *testing.T, opts *provisionerServeOptions) (context.Cont return ctx, api } -// func testutil.CreateTar(t *testing.T, files map[string]string) []byte { -// t.Helper() -// var buffer bytes.Buffer -// writer := tar.NewWriter(&buffer) -// for name, content := range files { -// err := writer.WriteHeader(&tar.Header{ -// Name: name, -// Size: int64(len(content)), -// Mode: 0o644, -// }) -// require.NoError(t, err) -// _, err = writer.Write([]byte(content)) -// require.NoError(t, err) -// } -// err := writer.Flush() -// require.NoError(t, err) -// return buffer.Bytes() -// } - func configure(ctx context.Context, t *testing.T, client proto.DRPCProvisionerClient, config *proto.Config) proto.DRPCProvisioner_SessionClient { t.Helper() sess, err := client.Session(ctx) From a2969b873e7ed1c19c4261c756e8eca8b889435c Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 18 Nov 2024 08:34:32 +0000 Subject: [PATCH 4/5] add directories to archive --- testutil/archive.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/testutil/archive.go b/testutil/archive.go index 630a66f4b3624..88b7c428cdcf9 100644 --- a/testutil/archive.go +++ b/testutil/archive.go @@ -3,6 +3,7 @@ package testutil import ( "archive/tar" "bytes" + "path/filepath" "testing" "github.com/stretchr/testify/require" @@ -16,7 +17,21 @@ import ( func CreateTar(t testing.TB, files map[string]string) []byte { var buffer bytes.Buffer writer := tar.NewWriter(&buffer) + // Keep track of directories previously added. + addedDirs := make(map[string]bool) for path, content := range files { + // Add parent directories if they don't exist + dir := filepath.Dir(path) + if dir != "." && !addedDirs[dir] { + err := writer.WriteHeader(&tar.Header{ + Name: dir + "/", // Directory names must end with / + Mode: 0o755, + Typeflag: tar.TypeDir, + }) + require.NoError(t, err) + addedDirs[dir] = true + } + err := writer.WriteHeader(&tar.Header{ Name: path, Size: int64(len(content)), From abecc98f10205b5203f0186b8c217861b1f8379f Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 18 Nov 2024 08:41:37 +0000 Subject: [PATCH 5/5] post-merge fixup --- provisioner/terraform/provision_test.go | 37 +------------------------ 1 file changed, 1 insertion(+), 36 deletions(-) diff --git a/provisioner/terraform/provision_test.go b/provisioner/terraform/provision_test.go index 70a3b36f0178a..50369468151c9 100644 --- a/provisioner/terraform/provision_test.go +++ b/provisioner/terraform/provision_test.go @@ -3,8 +3,6 @@ package terraform_test import ( - "archive/tar" - "bytes" "context" "encoding/json" "errors" @@ -79,39 +77,6 @@ func setupProvisioner(t *testing.T, opts *provisionerServeOptions) (context.Cont return ctx, api } -func makeTar(t *testing.T, files map[string]string) []byte { - t.Helper() - var buffer bytes.Buffer - writer := tar.NewWriter(&buffer) - - addedDirs := make(map[string]bool) - for name, content := range files { - // Add parent directories if they don't exist - dir := filepath.Dir(name) - if dir != "." && !addedDirs[dir] { - err := writer.WriteHeader(&tar.Header{ - Name: dir + "/", // Directory names must end with / - Mode: 0o755, - Typeflag: tar.TypeDir, - }) - require.NoError(t, err) - addedDirs[dir] = true - } - - err := writer.WriteHeader(&tar.Header{ - Name: name, - Size: int64(len(content)), - Mode: 0o644, - }) - require.NoError(t, err) - _, err = writer.Write([]byte(content)) - require.NoError(t, err) - } - err := writer.Flush() - require.NoError(t, err) - return buffer.Bytes() -} - func configure(ctx context.Context, t *testing.T, client proto.DRPCProvisionerClient, config *proto.Config) proto.DRPCProvisioner_SessionClient { t.Helper() sess, err := client.Session(ctx) @@ -998,7 +963,7 @@ func TestProvision_MalformedModules(t *testing.T) { ctx, api := setupProvisioner(t, nil) sess := configure(ctx, t, api, &proto.Config{ - TemplateSourceArchive: makeTar(t, map[string]string{ + TemplateSourceArchive: testutil.CreateTar(t, map[string]string{ "main.tf": `module "hello" { source = "./module" }`, "module/module.tf": `resource "null_`, }),