diff --git a/provisioner/terraform/executor.go b/provisioner/terraform/executor.go index 44b6d4d9e8275..4eb432252a849 100644 --- a/provisioner/terraform/executor.go +++ b/provisioner/terraform/executor.go @@ -23,6 +23,7 @@ import ( ) type executor struct { + initMu sync.Locker binaryPath string cachePath string workdir string @@ -142,6 +143,19 @@ func (e executor) init(ctx context.Context, logr logger) error { "-no-color", "-input=false", } + + // When cache path is set, we must protect against multiple calls + // to `terraform init`. + // + // From the Terraform documentation: + // Note: The plugin cache directory is not guaranteed to be + // concurrency safe. The provider installer's behavior in + // environments with multiple terraform init calls is undefined. + if e.cachePath != "" { + e.initMu.Lock() + defer e.initMu.Unlock() + } + return e.execWriteOutput(ctx, args, e.basicEnv(), outWriter, errWriter) } diff --git a/provisioner/terraform/parse_test.go b/provisioner/terraform/parse_test.go index dab994d335b7d..0a88ab21d5b78 100644 --- a/provisioner/terraform/parse_test.go +++ b/provisioner/terraform/parse_test.go @@ -3,40 +3,20 @@ package terraform_test import ( - "context" "encoding/json" "os" "path/filepath" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/coder/coder/provisioner/terraform" - "github.com/coder/coder/provisionersdk" "github.com/coder/coder/provisionersdk/proto" ) func TestParse(t *testing.T) { t.Parallel() - // Create an in-memory provisioner to communicate with. - client, server := provisionersdk.TransportPipe() - ctx, cancelFunc := context.WithCancel(context.Background()) - t.Cleanup(func() { - _ = client.Close() - _ = server.Close() - cancelFunc() - }) - go func() { - err := terraform.Serve(ctx, &terraform.ServeOptions{ - ServeOptions: &provisionersdk.ServeOptions{ - Listener: server, - }, - }) - assert.NoError(t, err) - }() - api := proto.NewDRPCProvisionerClient(provisionersdk.Conn(client)) + ctx, api := setupProvisioner(t) testCases := []struct { Name string @@ -175,7 +155,8 @@ func TestParse(t *testing.T) { DefaultDestination: &proto.ParameterDestination{ Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE, }, - }}, + }, + }, }, }, }, diff --git a/provisioner/terraform/provision_test.go b/provisioner/terraform/provision_test.go index 10c9e0fe92504..4b9e203ab4f32 100644 --- a/provisioner/terraform/provision_test.go +++ b/provisioner/terraform/provision_test.go @@ -23,21 +23,25 @@ import ( ) func setupProvisioner(t *testing.T) (context.Context, proto.DRPCProvisionerClient) { + cachePath := t.TempDir() client, server := provisionersdk.TransportPipe() ctx, cancelFunc := context.WithCancel(context.Background()) + serverErr := make(chan error, 1) t.Cleanup(func() { _ = client.Close() _ = server.Close() cancelFunc() + err := <-serverErr + assert.NoError(t, err) }) go func() { - err := terraform.Serve(ctx, &terraform.ServeOptions{ + serverErr <- terraform.Serve(ctx, &terraform.ServeOptions{ ServeOptions: &provisionersdk.ServeOptions{ Listener: server, }, - Logger: slogtest.Make(t, nil).Leveled(slog.LevelDebug), + CachePath: cachePath, + Logger: slogtest.Make(t, nil).Leveled(slog.LevelDebug), }) - assert.NoError(t, err) }() api := proto.NewDRPCProvisionerClient(provisionersdk.Conn(client)) return ctx, api diff --git a/provisioner/terraform/serve.go b/provisioner/terraform/serve.go index 66e819fbdb806..1a75483bda35d 100644 --- a/provisioner/terraform/serve.go +++ b/provisioner/terraform/serve.go @@ -3,6 +3,7 @@ package terraform import ( "context" "path/filepath" + "sync" "github.com/cli/safeexec" "github.com/hashicorp/go-version" @@ -109,15 +110,20 @@ func Serve(ctx context.Context, options *ServeOptions) error { } type server struct { + // initMu protects against executors running `terraform init` + // concurrently when cache path is set. + initMu sync.Mutex + binaryPath string cachePath string logger slog.Logger } -func (t server) executor(workdir string) executor { +func (s *server) executor(workdir string) executor { return executor{ - binaryPath: t.binaryPath, - cachePath: t.cachePath, + initMu: &s.initMu, + binaryPath: s.binaryPath, + cachePath: s.cachePath, workdir: workdir, } }