Skip to content

Commit

Permalink
fix: prevent terraform init races (#4985)
Browse files Browse the repository at this point in the history
  • Loading branch information
deansheather committed Nov 9, 2022
1 parent d277e28 commit 45f81a7
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 16 deletions.
22 changes: 17 additions & 5 deletions provisioner/terraform/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,28 @@ import (
"strings"
"sync"

"golang.org/x/xerrors"

"github.com/hashicorp/go-version"
tfjson "github.com/hashicorp/terraform-json"
"golang.org/x/xerrors"

"github.com/coder/coder/provisionersdk/proto"
)

// initMut is a global mutex that protects the Terraform cache directory from
// concurrent usage by path. Only `terraform init` commands are guarded by this
// mutex.
//
// 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.
var initMut = &sync.Mutex{}

type executor struct {
initMu sync.Locker
binaryPath string
cachePath string
workdir string
Expand Down Expand Up @@ -181,8 +193,8 @@ func (e executor) init(ctx, killCtx context.Context, logr logger) error {
// 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()
initMut.Lock()
defer initMut.Unlock()
}

return e.execWriteOutput(ctx, killCtx, args, e.basicEnv(), outWriter, errWriter)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// nolint:testpackage
package terraform

import (
Expand Down
13 changes: 3 additions & 10 deletions provisioner/terraform/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package terraform
import (
"context"
"path/filepath"
"sync"
"time"

"github.com/cli/safeexec"
Expand Down Expand Up @@ -100,20 +99,14 @@ 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

binaryPath string
cachePath string
logger slog.Logger
exitTimeout time.Duration
}

func (s *server) executor(workdir string) executor {
return executor{
initMu: &s.initMu,
binaryPath: s.binaryPath,
cachePath: s.cachePath,
workdir: workdir,
Expand Down

0 comments on commit 45f81a7

Please sign in to comment.