diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 9df66c5..bff90e2 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -3,8 +3,13 @@ name: CI on: [push, pull_request] jobs: - check: - runs-on: ubuntu-latest + unit-tests: + name: unit (${{ matrix.os }}) + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, macos-latest, windows-latest] + runs-on: ${{ matrix.os }} steps: - name: Checkout uses: actions/checkout@v6 @@ -14,10 +19,24 @@ jobs: - name: Set up Go uses: actions/setup-go@v6 with: - go-version: 1.21 + go-version: '1.24' - name: Unit tests - run: go test ./... + run: go test -race ./... + + integration: + name: integration + goreleaser + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v6 + with: + fetch-depth: 0 + + - name: Set up Go + uses: actions/setup-go@v6 + with: + go-version: '1.24' - name: Integration tests run: ./test.sh diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 77fbcda..975b219 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -20,7 +20,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v6 with: - go-version: 1.21 + go-version: '1.24' - name: Run GoReleaser uses: goreleaser/goreleaser-action@v7 diff --git a/.goreleaser.yaml b/.goreleaser.yaml index 9bf610f..7d81f9d 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -4,6 +4,10 @@ builds: - main: ./cmd/terraform-demux env: - CGO_ENABLED=0 + # Inject the release tag into main.version so `terraform-demux -version` + # reports something other than the package default v0.0.1+dev. + ldflags: + - -s -w -X main.version=v{{.Version}} goos: - linux - windows diff --git a/README.md b/README.md index 7c7415d..05e341f 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ A seamless launcher for Terraform. Simply navigate to any folder that contains Terraform configuration and run `terraform` as you usually would. `terraform-demux` will attempt to locate the appropriate [version constraint](https://www.terraform.io/docs/language/expressions/version-constraints.html) by searching in the current working directory and recursively through parent directories. If `terraform-demux` cannot determine a constraint, it will default to the latest possible version. -### Architecture Compatability +### Architecture Compatibility `terraform-demux` supports a native `arm64` build that can also run `amd64` versions of `terraform` by specifying the `TF_DEMUX_ARCH` environment variable. This might be necessary for `terraform` workspaces that need older `terraform` versions that do not have `arm64` builds, or use older providers that do not have `arm64` builds. @@ -39,11 +39,11 @@ We highly encourage leveraging native Terraform refactoring blocks whenever feas Usage Details -* For Terraform 1.1.0 and above: We recomment utilizing Terraform [moved](https://developer.hashicorp.com/terraform/language/modules/develop/refactoring) block instead `terraform state mv` command. +* For Terraform 1.1.0 and above: we recommend using Terraform's [moved](https://developer.hashicorp.com/terraform/language/modules/develop/refactoring) block instead of the `terraform state mv` command. -* For Terraform 1.5.0 and above: We recomment utilizing Terraform [import](https://developer.hashicorp.com/terraform/language/import) block instead `terraform import` command. +* For Terraform 1.5.0 and above: we recommend using Terraform's [import](https://developer.hashicorp.com/terraform/language/import) block instead of the `terraform import` command. -* For Terraform 1.7.0 and above: We recomment utilizing Terraform [removed](https://developer.hashicorp.com/terraform/language/resources/syntax) block instead `terraform state rm` command. +* For Terraform 1.7.0 and above: we recommend using Terraform's [removed](https://developer.hashicorp.com/terraform/language/resources/syntax) block instead of the `terraform state rm` command. However, if necessary, you can still utilize the Terraform CLI to manipulate states. Before proceeding, ensure to set the environment variable `TF_DEMUX_ALLOW_STATE_COMMANDS=true` to confirm your intent. @@ -53,4 +53,6 @@ Setting the `TF_DEMUX_LOG` environment variable to any non-empty value will caus ## Cache Directory -`terraform-demux` keeps a cache of Hashicorp's releases index and downloaded Terraform binaries in the directory returned by [os.UserCacheDir](https://golang.org/pkg/os/#UserCacheDir), under `terraform-demux/` (e.g. `~/Library/Caches/terraform-demux/` on MacOS). +`terraform-demux` keeps a cache of HashiCorp's releases index and downloaded Terraform binaries in the directory returned by [os.UserCacheDir](https://golang.org/pkg/os/#UserCacheDir), under `terraform-demux/` (e.g. `~/Library/Caches/terraform-demux/` on macOS). + +Setting `TF_DEMUX_CACHE_HOME` overrides this location. This is useful for tests, sandboxes, and CI runners where you don't want the wrapper to touch the user's real cache (note that `XDG_CACHE_HOME` is ignored by `os.UserCacheDir` on macOS). diff --git a/cmd/terraform-demux/main.go b/cmd/terraform-demux/main.go index 60a6959..a1d508a 100644 --- a/cmd/terraform-demux/main.go +++ b/cmd/terraform-demux/main.go @@ -1,7 +1,6 @@ package main import ( - "bytes" "log" "os" "runtime" @@ -9,21 +8,11 @@ import ( "github.com/etsy/terraform-demux/internal/wrapper" ) -var ( - version = "v0.0.1+dev" -) +var version = "v0.0.1+dev" func main() { - // When TF_DEMUX_LOG is unset, hold log output in a buffer so we can - // replay it on stderr if something goes wrong. Otherwise the user only - // sees the final error message and not the trace that led to it. - var logBuf bytes.Buffer - verboseLogging := os.Getenv("TF_DEMUX_LOG") != "" - if verboseLogging { - log.SetOutput(os.Stderr) - } else { - log.SetOutput(&logBuf) - } + verbose := os.Getenv("TF_DEMUX_LOG") != "" + flushLog := wrapper.SetupLogging(verbose, os.Stderr) arch := os.Getenv("TF_DEMUX_ARCH") if arch == "" { @@ -34,9 +23,7 @@ func main() { exitCode, err := wrapper.RunTerraform(os.Args[1:], arch) if err != nil { - if !verboseLogging { - os.Stderr.Write(logBuf.Bytes()) - } + flushLog() log.SetOutput(os.Stderr) log.Fatal("error: ", err) } diff --git a/go.mod b/go.mod index 519aa8e..3a98c6e 100644 --- a/go.mod +++ b/go.mod @@ -1,9 +1,10 @@ module github.com/etsy/terraform-demux -go 1.21 +go 1.24.0 require ( github.com/Masterminds/semver/v3 v3.2.1 + github.com/gofrs/flock v0.13.0 github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 github.com/hashicorp/terraform-config-inspect v0.0.0-20231204233900-a34142ec2a72 github.com/natefinch/atomic v1.0.1 @@ -18,6 +19,8 @@ require ( github.com/hashicorp/hcl/v2 v2.0.0 // indirect github.com/mitchellh/go-wordwrap v1.0.0 // indirect github.com/peterbourgon/diskv v2.0.1+incompatible // indirect + github.com/rogpeppe/go-internal v1.14.1 // indirect github.com/zclconf/go-cty v1.1.0 // indirect + golang.org/x/sys v0.37.0 // indirect golang.org/x/text v0.3.8 // indirect ) diff --git a/go.sum b/go.sum index bb41a52..8f4b9fe 100644 --- a/go.sum +++ b/go.sum @@ -10,6 +10,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/go-test/deep v1.0.3 h1:ZrJSEWsXzPOxaZnFteGEfooLba+ju3FYIbOrS+rQd68= github.com/go-test/deep v1.0.3/go.mod h1:wGDj63lr65AM2AQyKZd/NYHGb0R+1RLqB8NKt3aSFNA= +github.com/gofrs/flock v0.13.0 h1:95JolYOvGMqeH31+FC7D2+uULf6mG61mEZ/A8dRYMzw= +github.com/gofrs/flock v0.13.0/go.mod h1:jxeyy9R1auM5S6JYDBhDt+E2TCo7DkratH4Pgi8P+Z0= github.com/golang/protobuf v1.1.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/google/btree v1.0.1 h1:gK4Kx5IaGY9CD5sPJ36FHiBJ6ZXl0kilRiiCj+jdYp4= github.com/google/btree v1.0.1/go.mod h1:xXMiIv4Fb/0kKde4SpL7qlzvu5cMJDRkFDxJfI9uaxA= @@ -24,8 +26,9 @@ github.com/hashicorp/hcl/v2 v2.0.0 h1:efQznTz+ydmQXq3BOnRa3AXzvCeTq1P4dKj/z5GLlY github.com/hashicorp/hcl/v2 v2.0.0/go.mod h1:oVVDG71tEinNGYCxinCYadcmKU9bglqW9pV3txagJ90= github.com/hashicorp/terraform-config-inspect v0.0.0-20231204233900-a34142ec2a72 h1:nZ5gGjbe5o7XUu1d7j+Y5Ztcxlp+yaumTKH9i0D3wlg= github.com/hashicorp/terraform-config-inspect v0.0.0-20231204233900-a34142ec2a72/go.mod h1:l8HcFPm9cQh6Q0KSWoYPiePqMvRFenybP1CH2MjKdlg= -github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= +github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= +github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= @@ -39,11 +42,16 @@ github.com/natefinch/atomic v1.0.1 h1:ZPYKxkqQOx3KZ+RsbnP/YsgvxWQPGxjC0oBt2AhwV0 github.com/natefinch/atomic v1.0.1/go.mod h1:N/D/ELrljoqDyT3rZrsUmtsuzvHkeB/wWjHV22AZRbM= github.com/peterbourgon/diskv v2.0.1+incompatible h1:UBdAOUP5p4RWqPBg048CAvpKN+vxiaj6gdUUzhl4XmI= github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= +github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= github.com/sergi/go-diff v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ= github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo= github.com/spf13/pflag v1.0.2/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/vmihailenco/msgpack v3.3.3+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk= github.com/zclconf/go-cty v1.1.0 h1:uJwc9HiBOCpoKIObTQaLR+tsEXx1HBHnOsOOpcdhZgw= github.com/zclconf/go-cty v1.1.0/go.mod h1:xnAOWiHeOqg2nWS62VtQ7pbOu17FtxJNW8RLEih+O3s= @@ -55,6 +63,8 @@ golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190502175342-a43fa875dd82/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.37.0 h1:fdNQudmxPjkdUTPnLn5mdQv7Zwvbvpaxqs831goi9kQ= +golang.org/x/sys v0.37.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.8 h1:nAL+RVCQ9uMn3vJZbV+MRnydTJFPf8qqY42YiA6MrqY= @@ -62,3 +72,5 @@ golang.org/x/text v0.3.8/go.mod h1:E6s5w1FMmriuDzIBO73fBruAKo1PCIq6d2Q6DHfQ8WQ= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/releaseapi/client.go b/internal/releaseapi/client.go index 72cc439..1f51383 100644 --- a/internal/releaseapi/client.go +++ b/internal/releaseapi/client.go @@ -1,7 +1,11 @@ +// Package releaseapi talks to releases.hashicorp.com to discover and +// download Terraform releases. It also maintains a local cache for both +// the parsed release index (with a short TTL) and extracted binaries. package releaseapi import ( "archive/zip" + "bytes" "crypto/sha256" "encoding/hex" "encoding/json" @@ -13,8 +17,10 @@ import ( "os" "path/filepath" "strings" + "time" "github.com/Masterminds/semver/v3" + "github.com/gofrs/flock" "github.com/gregjones/httpcache" "github.com/gregjones/httpcache/diskcache" "github.com/natefinch/atomic" @@ -30,18 +36,40 @@ var ( const ( httpCacheSubdir = "http" binCacheSubdir = "bin" + + // indexCacheFile holds a parsed copy of the upstream release index so + // repeated invocations within a short window skip the network fetch + // entirely. This is layered on top of httpcache: httpcache still + // revalidates with HashiCorp on every call (which is a 304 round-trip), + // while this local TTL avoids even that round-trip when the cache is + // fresh. + indexCacheFile = "index.json" + + // indexCacheTTL is short enough that a freshly-released Terraform + // version becomes visible within a few minutes, but long enough that + // rapid-fire invocations (shell completion, scripts) don't hit the + // network repeatedly. + indexCacheTTL = 5 * time.Minute ) +// ReleaseIndex is the parsed shape of releases.hashicorp.com/terraform/index.json. type ReleaseIndex struct { Versions map[string]Release `json:"versions"` } +// Release is one Terraform version, with the per-platform builds and a +// pointer to the SHA256SUMS file that signs them. type Release struct { Version *semver.Version `json:"version"` - Shasums string `json:"shasums"` - Builds []Build `json:"builds"` + // Shasums is the *filename* (not URL) of the SHA256SUMS file for this + // release, e.g. "terraform_1.5.0_SHA256SUMS". Combine with + // ShaSumsURL to get a full URL. + Shasums string `json:"shasums"` + Builds []Build `json:"builds"` } +// Build is one (os, arch) variant of a Release. URL points at the .zip +// archive containing the terraform binary. type Build struct { Version *semver.Version `json:"version"` OS string `json:"os"` @@ -49,7 +77,9 @@ type Build struct { URL string `json:"url"` } +// Client downloads Terraform releases and caches them on disk. type Client struct { + cacheDir string binDir string httpClient *http.Client } @@ -69,10 +99,68 @@ func NewClient(cacheDir string) (*Client, error) { } httpClient := httpcache.NewTransport(diskcache.New(httpDir)).Client() - return &Client{binDir: binDir, httpClient: httpClient}, nil + return &Client{cacheDir: cacheDir, binDir: binDir, httpClient: httpClient}, nil } +// ListReleases returns the parsed Terraform release index, preferring a +// fresh local cache copy over a network round-trip. func (c *Client) ListReleases() (ReleaseIndex, error) { + if idx, ok := c.readIndexCache(); ok { + return idx, nil + } + + idx, err := c.fetchIndex() + if err != nil { + return idx, err + } + + c.writeIndexCache(idx) + return idx, nil +} + +func (c *Client) readIndexCache() (ReleaseIndex, bool) { + var idx ReleaseIndex + path := filepath.Join(c.cacheDir, indexCacheFile) + + info, err := os.Stat(path) + if err != nil { + return idx, false + } + if time.Since(info.ModTime()) > indexCacheTTL { + return idx, false + } + + data, err := os.ReadFile(path) + if err != nil { + return idx, false + } + if err := json.Unmarshal(data, &idx); err != nil { + // Corrupt cache file: log so the user can debug "why is my cache + // not working", and fall through to a network fetch which will + // rewrite a clean index. + log.Printf("ignoring corrupt index cache at %s: %v", path, err) + return idx, false + } + + log.Printf("using local index cache (age %s)", time.Since(info.ModTime()).Truncate(time.Second)) + return idx, true +} + +func (c *Client) writeIndexCache(idx ReleaseIndex) { + path := filepath.Join(c.cacheDir, indexCacheFile) + data, err := json.Marshal(idx) + if err != nil { + log.Printf("could not serialize index for local cache: %v", err) + return + } + // atomic.WriteFile ensures a process killed mid-write doesn't leave a + // half-written file that subsequent reads will treat as the cache. + if err := atomic.WriteFile(path, bytes.NewReader(data)); err != nil { + log.Printf("could not write index cache to %s: %v", path, err) + } +} + +func (c *Client) fetchIndex() (ReleaseIndex, error) { var releaseIndex ReleaseIndex log.Printf("downloading Terraform release index") @@ -103,33 +191,39 @@ func (c *Client) ListReleases() (ReleaseIndex, error) { return releaseIndex, nil } -func (c *Client) DownloadRelease(r Release, os, arch string) (string, error) { - var matchingBuild Build - for _, build := range r.Builds { - if build.OS == os && build.Arch == arch { - matchingBuild = build - break - } - } - if matchingBuild.URL == "" { - return "", fmt.Errorf("could not find matching build for OS %q and arch %q", os, arch) - } - // Mirrors the nil-version skipping done in filterReleases: the cache - // path uses Build.Version.String(), so a missing version field would - // otherwise panic later in executableName(). - if matchingBuild.Version == nil { - matchingBuild.Version = r.Version - } - if matchingBuild.Version == nil { - return "", fmt.Errorf("release for OS %q arch %q has no version", os, arch) +// DownloadRelease resolves and downloads the (goos, arch) variant of r, +// returning the on-disk path of the cached, verified Terraform binary. +func (c *Client) DownloadRelease(r Release, goos, arch string) (string, error) { + build, err := findBuild(r, goos, arch) + if err != nil { + return "", err } - expectedSum, err := c.expectedChecksumFor(r, &matchingBuild) + expectedSum, err := c.expectedChecksumFor(r, &build) if err != nil { return "", err } - return c.downloadBuild(matchingBuild, expectedSum) + return c.downloadBuild(build, expectedSum) +} + +// findBuild picks the matching (goos, arch) variant of r and fills in any +// nil Version field from the parent Release. The cache filename for the +// installed binary uses Build.Version.String(), so a nil pointer here would +// panic later in executableName(). +func findBuild(r Release, goos, arch string) (Build, error) { + for _, build := range r.Builds { + if build.OS == goos && build.Arch == arch { + if build.Version == nil { + build.Version = r.Version + } + if build.Version == nil { + return Build{}, fmt.Errorf("release for OS %q arch %q has no version", goos, arch) + } + return build, nil + } + } + return Build{}, fmt.Errorf("could not find matching build for OS %q and arch %q", goos, arch) } // expectedChecksumFor downloads the SHA256SUMS file for the release and looks @@ -184,6 +278,7 @@ func (c *Client) getReleaseCheckSums(release Release) (string, error) { return string(body), nil } +// ShaSumsURL returns the absolute URL of the SHA256SUMS file for r. func (r *Release) ShaSumsURL() string { return fmt.Sprintf("%s/%s/%s", releaseRootURL, r.Version, r.Shasums) } @@ -191,93 +286,171 @@ func (r *Release) ShaSumsURL() string { func (c *Client) downloadBuild(build Build, expectedSum string) (string, error) { path := cachedExecutablePath(c.binDir, build) - if _, err := os.Stat(path); err == nil { + if cached, err := executableCached(path); err != nil { + return "", err + } else if cached { log.Printf("found cached Terraform executable at %s", path) return path, nil - } else if !os.IsNotExist(err) { - return "", fmt.Errorf("could not stat Terraform executable: %w", err) + } + + // Serialize parallel processes downloading the same binary. Without + // this, two `terraform-demux` invocations both miss the cache, both + // download the archive, and both write to it (last-writer wins via + // atomic.WriteFile). Worse, the loser deletes its tempfile on exit, + // which is fine, but the network bandwidth is wasted. + lockPath := path + ".lock" + fileLock := flock.New(lockPath) + if err := fileLock.Lock(); err != nil { + return "", fmt.Errorf("could not acquire download lock %q: %w", lockPath, err) + } + defer func() { + if err := fileLock.Unlock(); err != nil { + log.Printf("could not release download lock %q: %v", lockPath, err) + } + }() + + // Re-check the cache after acquiring the lock — another process may + // have downloaded while we were waiting. + if cached, err := executableCached(path); err != nil { + return "", err + } else if cached { + log.Printf("found cached Terraform executable at %s (after lock)", path) + return path, nil } log.Printf("downloading release archive from %s", build.URL) - zipFile, zipLength, err := c.downloadReleaseArchive(build) + zipFile, err := c.downloadReleaseArchive(build) if err != nil { return "", err } defer os.Remove(zipFile.Name()) defer zipFile.Close() + if err := verifyChecksum(zipFile, expectedSum, build.URL); err != nil { + return "", err + } + + zipReader, err := openZipReader(zipFile) + if err != nil { + return "", err + } + + if err := extractTerraformBinary(zipReader, build.archiveBinaryName(), path); err != nil { + return "", err + } + return path, nil +} + +// verifyChecksum reads zipFile fully (re-opening it so the read position +// doesn't disturb later zip parsing) and compares its SHA256 to expected. +func verifyChecksum(zipFile *os.File, expected, url string) error { f, err := os.Open(zipFile.Name()) if err != nil { - return "", fmt.Errorf("could not open zip archive: %w", err) + return fmt.Errorf("could not open zip archive: %w", err) } defer f.Close() h := sha256.New() if _, err := io.Copy(h, f); err != nil { - return "", fmt.Errorf("could not compute sha256 for zip archive: %w", err) + return fmt.Errorf("could not compute sha256 for zip archive: %w", err) } - actualSum := hex.EncodeToString(h.Sum(nil)) + actual := hex.EncodeToString(h.Sum(nil)) - if expectedSum != actualSum { - return "", fmt.Errorf("checksum for %s should be %s, got %s", build.URL, expectedSum, actualSum) + if expected != actual { + return fmt.Errorf("checksum for %s should be %s, got %s", url, expected, actual) } log.Printf("checksum match") + return nil +} - zipReader, err := zip.NewReader(zipFile, zipLength) +func openZipReader(zipFile *os.File) (*zip.Reader, error) { + // Use the on-disk size rather than response.ContentLength: chunked + // responses report -1, which would make zip.NewReader fail. + info, err := zipFile.Stat() + if err != nil { + return nil, fmt.Errorf("could not stat zip archive: %w", err) + } + r, err := zip.NewReader(zipFile, info.Size()) if err != nil { - return "", fmt.Errorf("could not unzip release archive: %w", err) + return nil, fmt.Errorf("could not unzip release archive: %w", err) } + return r, nil +} - binaryName := build.archiveBinaryName() +// extractTerraformBinary writes the binary named binaryName from the zip +// archive to dest with executable permissions. If the destination cannot +// be made executable, the partial file is removed so the next invocation +// will re-download cleanly instead of forever cache-hitting a non-exec +// file. +func extractTerraformBinary(zipReader *zip.Reader, binaryName, dest string) error { for _, f := range zipReader.File { if filepath.Base(f.Name) != binaryName { continue } + // Delegate the per-entry work so the source close is scoped + // to its own function — `defer` inside the for loop would + // stack closes until extractTerraformBinary returns, and + // holding the zip entry open across atomic.WriteFile + Chmod + // is unnecessary. + return writeZipEntryAsExecutable(f, dest) + } + return errors.New("could not find executable named 'terraform' in release archive") +} - source, err := f.Open() - if err != nil { - return "", fmt.Errorf("could not read binary in release archive: %w", err) - } - defer source.Close() +func writeZipEntryAsExecutable(f *zip.File, dest string) error { + source, err := f.Open() + if err != nil { + return fmt.Errorf("could not read binary in release archive: %w", err) + } + defer source.Close() - if err := atomic.WriteFile(path, source); err != nil { - return "", fmt.Errorf("could not write binary to the cache directory: %w", err) - } - if err := os.Chmod(path, 0700); err != nil { - return "", fmt.Errorf("could not make binary executable: %w", err) - } - return path, nil + if err := atomic.WriteFile(dest, source); err != nil { + return fmt.Errorf("could not write binary to the cache directory: %w", err) } + if err := os.Chmod(dest, 0700); err != nil { + _ = os.Remove(dest) + return fmt.Errorf("could not make binary executable: %w", err) + } + return nil +} - return "", errors.New("could not find executable named 'terraform' in release archive") +func executableCached(path string) (bool, error) { + _, err := os.Stat(path) + if err == nil { + return true, nil + } + if os.IsNotExist(err) { + return false, nil + } + return false, fmt.Errorf("could not stat Terraform executable: %w", err) } -func (c *Client) downloadReleaseArchive(build Build) (*os.File, int64, error) { +func (c *Client) downloadReleaseArchive(build Build) (*os.File, error) { request, err := http.NewRequest("GET", build.URL, nil) if err != nil { - return nil, 0, fmt.Errorf("could not create request for release archive: %w", err) + return nil, fmt.Errorf("could not create request for release archive: %w", err) } request.Header.Set("Cache-Control", "no-store") response, err := c.httpClient.Do(request) if err != nil { - return nil, 0, fmt.Errorf("could not download release archive: %w", err) + return nil, fmt.Errorf("could not download release archive: %w", err) } defer response.Body.Close() if response.StatusCode != http.StatusOK { - return nil, 0, fmt.Errorf("unexpected status code %d when downloading release archive", response.StatusCode) + return nil, fmt.Errorf("unexpected status code %d when downloading release archive", response.StatusCode) } tmp, err := os.CreateTemp("", filepath.Base(build.URL)) if err != nil { - return nil, 0, fmt.Errorf("could not create temporary file for release archive: %w", err) + return nil, fmt.Errorf("could not create temporary file for release archive: %w", err) } if _, err := io.Copy(tmp, response.Body); err != nil { - return nil, 0, fmt.Errorf("could not copy release archive to temporary file: %w", err) + return nil, fmt.Errorf("could not copy release archive to temporary file: %w", err) } - return tmp, response.ContentLength, nil + return tmp, nil } func cachedExecutablePath(binDir string, b Build) string { diff --git a/internal/releaseapi/client_test.go b/internal/releaseapi/client_test.go index fa62f2c..7e39d67 100644 --- a/internal/releaseapi/client_test.go +++ b/internal/releaseapi/client_test.go @@ -15,71 +15,79 @@ import ( "testing" ) -// fakeReleaseServer serves a single Terraform "release" so we can exercise -// the full ListReleases + DownloadRelease pipeline without hitting -// releases.hashicorp.com. It returns the server, the zip's sha256, and the -// raw zip bytes (useful for tampering tests). -func fakeReleaseServer(t *testing.T, version string) (*httptest.Server, string, []byte) { +// serveRelease stands up an httptest server that serves a single Terraform +// release. Tests can pass their own zipBytes and shasumsBody to simulate +// happy paths, corrupted archives, missing checksum entries, or messy +// SHA256SUMS formats. The returned URL paths follow HashiCorp's layout: +// +// GET /index.json +// GET //terraform__SHA256SUMS +// GET //terraform___.zip +func serveRelease(t *testing.T, version string, zipBytes []byte, shasumsBody string) *httptest.Server { t.Helper() zipName := fmt.Sprintf("terraform_%s_%s_%s.zip", version, runtime.GOOS, runtime.GOARCH) + shasumsName := fmt.Sprintf("terraform_%s_SHA256SUMS", version) + + mux := http.NewServeMux() + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + + mux.HandleFunc("/index.json", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + fmt.Fprintf(w, `{"versions":{%q:{"version":%q,"shasums":%q,"builds":[{"version":%q,"os":%q,"arch":%q,"url":%q}]}}}`, + version, version, shasumsName, version, runtime.GOOS, runtime.GOARCH, + srv.URL+"/"+version+"/"+zipName) + }) + mux.HandleFunc("/"+version+"/"+shasumsName, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/plain") + _, _ = w.Write([]byte(shasumsBody)) + }) + mux.HandleFunc("/"+version+"/"+zipName, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/zip") + _, _ = w.Write(zipBytes) + }) + + return srv +} + +// terraformZip builds a minimal zip whose only entry is the platform's +// terraform binary, named appropriately for runtime.GOOS. +func terraformZip(t *testing.T, payload string) []byte { + t.Helper() binName := "terraform" if runtime.GOOS == "windows" { binName = "terraform.exe" } - - // Build a minimal zip containing a "terraform" binary. - var zipBuf bytes.Buffer - zw := zip.NewWriter(&zipBuf) + var buf bytes.Buffer + zw := zip.NewWriter(&buf) w, err := zw.Create(binName) if err != nil { t.Fatalf("zip create: %v", err) } - if _, err := w.Write([]byte("#!/bin/sh\necho fake terraform " + version + "\n")); err != nil { + if _, err := w.Write([]byte(payload)); err != nil { t.Fatalf("zip write: %v", err) } if err := zw.Close(); err != nil { t.Fatalf("zip close: %v", err) } - zipBytes := zipBuf.Bytes() + return buf.Bytes() +} +// shasumsLine returns the canonical " \n" entry for a +// single archive — what HashiCorp ships in SHA256SUMS files. +func shasumsLine(zipBytes []byte, version string) string { + zipName := fmt.Sprintf("terraform_%s_%s_%s.zip", version, runtime.GOOS, runtime.GOARCH) sum := sha256.Sum256(zipBytes) - hexSum := hex.EncodeToString(sum[:]) - shasumsName := fmt.Sprintf("terraform_%s_SHA256SUMS", version) - shasumsBody := fmt.Sprintf("%s %s\n", hexSum, zipName) - - mux := http.NewServeMux() - srv := httptest.NewServer(mux) - - indexPath := "/index.json" - zipPath := fmt.Sprintf("/%s/%s", version, zipName) - shasumsPath := fmt.Sprintf("/%s/%s", version, shasumsName) - - mux.HandleFunc(indexPath, func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - fmt.Fprintf(w, `{ - "versions": { - %q: { - "version": %q, - "shasums": %q, - "builds": [ - {"version": %q, "os": %q, "arch": %q, "url": %q} - ] - } - } -}`, version, version, shasumsName, version, runtime.GOOS, runtime.GOARCH, srv.URL+zipPath) - }) - mux.HandleFunc(shasumsPath, func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "text/plain") - _, _ = w.Write([]byte(shasumsBody)) - }) - mux.HandleFunc(zipPath, func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/zip") - _, _ = w.Write(zipBytes) - }) + return fmt.Sprintf("%s %s\n", hex.EncodeToString(sum[:]), zipName) +} - t.Cleanup(srv.Close) - return srv, hexSum, zipBytes +// fakeReleaseServer is the happy-path wrapper around serveRelease: it +// produces a release where the SHA256SUMS file correctly signs the zip. +func fakeReleaseServer(t *testing.T, version string) *httptest.Server { + t.Helper() + zipBytes := terraformZip(t, "#!/bin/sh\necho fake terraform "+version+"\n") + return serveRelease(t, version, zipBytes, shasumsLine(zipBytes, version)) } func withTestURLs(t *testing.T, srvURL string) { @@ -93,14 +101,22 @@ func withTestURLs(t *testing.T, srvURL string) { }) } -func TestListReleases_ParsesIndex(t *testing.T) { - srv, _, _ := fakeReleaseServer(t, "1.5.0") +// newTestClient is a small fixture: stand up the fake server, point the +// package URLs at it, return a fresh Client backed by a temp cache. +func newTestClient(t *testing.T, version string) (*Client, *httptest.Server) { + t.Helper() + srv := fakeReleaseServer(t, version) withTestURLs(t, srv.URL) - c, err := NewClient(t.TempDir()) if err != nil { t.Fatalf("NewClient: %v", err) } + return c, srv +} + +func TestListReleases_ParsesIndex(t *testing.T) { + c, _ := newTestClient(t, "1.5.0") + idx, err := c.ListReleases() if err != nil { t.Fatalf("ListReleases: %v", err) @@ -118,7 +134,7 @@ func TestListReleases_ParsesIndex(t *testing.T) { } func TestDownloadRelease_WritesAndCachesBinary(t *testing.T) { - srv, _, _ := fakeReleaseServer(t, "1.5.0") + srv := fakeReleaseServer(t, "1.5.0") withTestURLs(t, srv.URL) cacheDir := t.TempDir() @@ -161,48 +177,11 @@ func TestDownloadRelease_WritesAndCachesBinary(t *testing.T) { } func TestDownloadRelease_RejectsCorruptedArchive(t *testing.T) { - // Stand up a server where the zip body doesn't match the published - // SHA256SUMS — DownloadRelease must refuse it. - version := "1.5.0" - zipName := fmt.Sprintf("terraform_%s_%s_%s.zip", version, runtime.GOOS, runtime.GOARCH) - shasumsName := fmt.Sprintf("terraform_%s_SHA256SUMS", version) - - // Pretend the zip has this checksum, but actually serve different bytes. - bogusSum := strings.Repeat("a", 64) - shasumsBody := fmt.Sprintf("%s %s\n", bogusSum, zipName) - - // Build a real-looking zip with arbitrary content. - var zipBuf bytes.Buffer - zw := zip.NewWriter(&zipBuf) - w, _ := zw.Create("terraform") - _, _ = w.Write([]byte("not the right bytes")) - _ = zw.Close() - - mux := http.NewServeMux() - srv := httptest.NewServer(mux) - t.Cleanup(srv.Close) - - mux.HandleFunc("/index.json", func(w http.ResponseWriter, r *http.Request) { - fmt.Fprintf(w, `{ - "versions": { - %q: { - "version": %q, - "shasums": %q, - "builds": [ - {"version": %q, "os": %q, "arch": %q, "url": %q} - ] - } - } -}`, version, version, shasumsName, version, runtime.GOOS, runtime.GOARCH, - srv.URL+"/"+version+"/"+zipName) - }) - mux.HandleFunc("/"+version+"/"+shasumsName, func(w http.ResponseWriter, r *http.Request) { - _, _ = w.Write([]byte(shasumsBody)) - }) - mux.HandleFunc("/"+version+"/"+zipName, func(w http.ResponseWriter, r *http.Request) { - _, _ = w.Write(zipBuf.Bytes()) - }) - + // Server signs the zip with a checksum that doesn't match its bytes. + zipBytes := terraformZip(t, "not the right bytes") + zipName := fmt.Sprintf("terraform_1.5.0_%s_%s.zip", runtime.GOOS, runtime.GOARCH) + bogusBody := fmt.Sprintf("%s %s\n", strings.Repeat("a", 64), zipName) + srv := serveRelease(t, "1.5.0", zipBytes, bogusBody) withTestURLs(t, srv.URL) c, err := NewClient(t.TempDir()) @@ -213,9 +192,8 @@ func TestDownloadRelease_RejectsCorruptedArchive(t *testing.T) { if err != nil { t.Fatalf("ListReleases: %v", err) } - rel := idx.Versions[version] - _, err = c.DownloadRelease(rel, runtime.GOOS, runtime.GOARCH) + _, err = c.DownloadRelease(idx.Versions["1.5.0"], runtime.GOOS, runtime.GOARCH) if err == nil { t.Fatal("expected checksum mismatch error, got nil") } @@ -225,60 +203,26 @@ func TestDownloadRelease_RejectsCorruptedArchive(t *testing.T) { } func TestDownloadRelease_NoBuildForOSArch(t *testing.T) { - srv, _, _ := fakeReleaseServer(t, "1.5.0") - withTestURLs(t, srv.URL) - - c, err := NewClient(t.TempDir()) - if err != nil { - t.Fatalf("NewClient: %v", err) - } + c, _ := newTestClient(t, "1.5.0") idx, err := c.ListReleases() if err != nil { t.Fatalf("ListReleases: %v", err) } - rel := idx.Versions["1.5.0"] - _, err = c.DownloadRelease(rel, "plan9", "mips") + _, err = c.DownloadRelease(idx.Versions["1.5.0"], "plan9", "mips") if err == nil { t.Fatal("expected error for unknown os/arch, got nil") } } -// TestDownloadRelease_FailsWhenSumNotInChecksumFile is a regression test for -// C1: previously a missing entry in SHA256SUMS caused checkSha256Sum to stay -// "" and the verification was silently skipped. Now the entire flow must -// fail. +// TestDownloadRelease_FailsWhenSumNotInChecksumFile is a regression test +// for C1: previously a missing entry in SHA256SUMS caused the verification +// to be silently skipped. Now the entire flow must fail. func TestDownloadRelease_FailsWhenSumNotInChecksumFile(t *testing.T) { - version := "1.5.0" - zipName := fmt.Sprintf("terraform_%s_%s_%s.zip", version, runtime.GOOS, runtime.GOARCH) - shasumsName := fmt.Sprintf("terraform_%s_SHA256SUMS", version) - - // SHA256SUMS deliberately lists a different filename, so no entry - // matches our build's zip. - shasumsBody := fmt.Sprintf("%s some_other_file.zip\n", strings.Repeat("a", 64)) - - var zipBuf bytes.Buffer - zw := zip.NewWriter(&zipBuf) - w, _ := zw.Create("terraform") - _, _ = w.Write([]byte("payload")) - _ = zw.Close() - - mux := http.NewServeMux() - srv := httptest.NewServer(mux) - t.Cleanup(srv.Close) - - mux.HandleFunc("/index.json", func(w http.ResponseWriter, r *http.Request) { - fmt.Fprintf(w, `{"versions":{%q:{"version":%q,"shasums":%q,"builds":[{"version":%q,"os":%q,"arch":%q,"url":%q}]}}}`, - version, version, shasumsName, version, runtime.GOOS, runtime.GOARCH, - srv.URL+"/"+version+"/"+zipName) - }) - mux.HandleFunc("/"+version+"/"+shasumsName, func(w http.ResponseWriter, r *http.Request) { - _, _ = w.Write([]byte(shasumsBody)) - }) - mux.HandleFunc("/"+version+"/"+zipName, func(w http.ResponseWriter, r *http.Request) { - _, _ = w.Write(zipBuf.Bytes()) - }) - + zipBytes := terraformZip(t, "payload") + // SHASUMS lists a different filename, so no entry matches our zip. + body := fmt.Sprintf("%s some_other_file.zip\n", strings.Repeat("a", 64)) + srv := serveRelease(t, "1.5.0", zipBytes, body) withTestURLs(t, srv.URL) c, err := NewClient(t.TempDir()) @@ -289,9 +233,8 @@ func TestDownloadRelease_FailsWhenSumNotInChecksumFile(t *testing.T) { if err != nil { t.Fatalf("ListReleases: %v", err) } - rel := idx.Versions[version] - _, err = c.DownloadRelease(rel, runtime.GOOS, runtime.GOARCH) + _, err = c.DownloadRelease(idx.Versions["1.5.0"], runtime.GOOS, runtime.GOARCH) if err == nil { t.Fatal("expected error when SHA256SUMS has no matching entry, got nil") } @@ -305,50 +248,19 @@ func TestDownloadRelease_FailsWhenSumNotInChecksumFile(t *testing.T) { // panic the parser (the previous strings.Split(line, " ") + checksum[1] // path would index past the end of the slice). func TestDownloadRelease_TolerantSHASUMSParse(t *testing.T) { - version := "1.5.0" - zipName := fmt.Sprintf("terraform_%s_%s_%s.zip", version, runtime.GOOS, runtime.GOARCH) - shasumsName := fmt.Sprintf("terraform_%s_SHA256SUMS", version) - - binName := "terraform" - if runtime.GOOS == "windows" { - binName = "terraform.exe" - } - var zipBuf bytes.Buffer - zw := zip.NewWriter(&zipBuf) - w, _ := zw.Create(binName) - _, _ = w.Write([]byte("payload")) - _ = zw.Close() - zipBytes := zipBuf.Bytes() + zipBytes := terraformZip(t, "payload") + zipName := fmt.Sprintf("terraform_1.5.0_%s_%s.zip", runtime.GOOS, runtime.GOARCH) sum := sha256.Sum256(zipBytes) - hexSum := hex.EncodeToString(sum[:]) - - // Mix in: blank lines, a malformed single-token line, extra whitespace, - // a legitimate line for our zip. - shasumsBody := strings.Join([]string{ + body := strings.Join([]string{ "", " ", "malformedline", fmt.Sprintf("%s some_other.zip", strings.Repeat("b", 64)), - fmt.Sprintf("%s %s", hexSum, zipName), + fmt.Sprintf("%s %s", hex.EncodeToString(sum[:]), zipName), "", }, "\n") - mux := http.NewServeMux() - srv := httptest.NewServer(mux) - t.Cleanup(srv.Close) - - mux.HandleFunc("/index.json", func(w http.ResponseWriter, r *http.Request) { - fmt.Fprintf(w, `{"versions":{%q:{"version":%q,"shasums":%q,"builds":[{"version":%q,"os":%q,"arch":%q,"url":%q}]}}}`, - version, version, shasumsName, version, runtime.GOOS, runtime.GOARCH, - srv.URL+"/"+version+"/"+zipName) - }) - mux.HandleFunc("/"+version+"/"+shasumsName, func(w http.ResponseWriter, r *http.Request) { - _, _ = w.Write([]byte(shasumsBody)) - }) - mux.HandleFunc("/"+version+"/"+zipName, func(w http.ResponseWriter, r *http.Request) { - _, _ = w.Write(zipBytes) - }) - + srv := serveRelease(t, "1.5.0", zipBytes, body) withTestURLs(t, srv.URL) c, err := NewClient(t.TempDir()) @@ -359,10 +271,8 @@ func TestDownloadRelease_TolerantSHASUMSParse(t *testing.T) { if err != nil { t.Fatalf("ListReleases: %v", err) } - rel := idx.Versions[version] - _, err = c.DownloadRelease(rel, runtime.GOOS, runtime.GOARCH) - if err != nil { + if _, err = c.DownloadRelease(idx.Versions["1.5.0"], runtime.GOOS, runtime.GOARCH); err != nil { t.Fatalf("expected success despite messy SHA256SUMS, got: %v", err) } } @@ -371,13 +281,8 @@ func TestDownloadRelease_TolerantSHASUMSParse(t *testing.T) { // no "version" field on a Build (or it fails to decode), DownloadRelease // must not panic in executableName(). It falls back to Release.Version. func TestDownloadRelease_FallsBackToReleaseVersion(t *testing.T) { - srv, _, _ := fakeReleaseServer(t, "1.5.0") - withTestURLs(t, srv.URL) + c, _ := newTestClient(t, "1.5.0") - c, err := NewClient(t.TempDir()) - if err != nil { - t.Fatalf("NewClient: %v", err) - } idx, err := c.ListReleases() if err != nil { t.Fatalf("ListReleases: %v", err) @@ -396,6 +301,27 @@ func TestDownloadRelease_FallsBackToReleaseVersion(t *testing.T) { } } +// TestListReleases_UsesLocalIndexTTL: a second ListReleases call within +// the TTL window must not hit the network. We assert by closing the +// upstream server and verifying the second call still succeeds. +func TestListReleases_UsesLocalIndexTTL(t *testing.T) { + c, srv := newTestClient(t, "1.5.0") + + if _, err := c.ListReleases(); err != nil { + t.Fatalf("first ListReleases: %v", err) + } + // Take server down — second call must succeed from local cache. + srv.Close() + + idx, err := c.ListReleases() + if err != nil { + t.Fatalf("second ListReleases (server down): %v", err) + } + if _, ok := idx.Versions["1.5.0"]; !ok { + t.Errorf("expected cached index to contain 1.5.0") + } +} + func TestNewClient_CreatesSubdirs(t *testing.T) { cacheDir := t.TempDir() if _, err := NewClient(cacheDir); err != nil { diff --git a/internal/wrapper/checkargs.go b/internal/wrapper/checkargs.go index 6f320d3..edf61eb 100644 --- a/internal/wrapper/checkargs.go +++ b/internal/wrapper/checkargs.go @@ -10,33 +10,48 @@ import ( const stateCommandVar = "TF_DEMUX_ALLOW_STATE_COMMANDS" +// State-command guard thresholds — the lowest Terraform version at which +// each command has a recommended config-block alternative. Initialized at +// package load so a typo in any constraint string is caught at startup +// rather than ignored via discarded errors at every call. +var ( + importStateCmdMin = mustParseConstraint(">= 1.5.0") + movedStateCmdMin = mustParseConstraint(">= 1.1.0") + removedStateCmdMin = mustParseConstraint(">= 1.7.0") +) + +func mustParseConstraint(s string) *semver.Constraints { + c, err := semver.NewConstraint(s) + if err != nil { + panic(fmt.Sprintf("invalid constraint %q: %v", s, err)) + } + return c +} + func checkStateCommand(args []string, version *semver.Version) error { if allowStateCommand(stateCommandVar) { return nil } - versionImport, _ := semver.NewConstraint(">= 1.5.0") - versionMoved, _ := semver.NewConstraint(">= 1.1.0") - versionRemoved, _ := semver.NewConstraint(">= 1.7.0") - cmd, sub := terraformSubcommand(args) switch { - case cmd == "import" && versionImport.Check(version): + case cmd == "import" && importStateCmdMin.Check(version): return refuseStateCommand("import", "import") - case cmd == "state" && sub == "mv" && versionMoved.Check(version): + case cmd == "state" && sub == "mv" && movedStateCmdMin.Check(version): return refuseStateCommand("state mv", "moved") - case cmd == "state" && sub == "rm" && versionRemoved.Check(version): + case cmd == "state" && sub == "rm" && removedStateCmdMin.Check(version): return refuseStateCommand("state rm", "removed") } return nil } -// terraformSubcommand returns the first and second positional arguments, -// ignoring anything that looks like a flag. This avoids false positives where -// a flag value contains a literal like "import" or "mv" (e.g. -var=action=mv). -// It does not handle the rare "-flag value" form, but Terraform's CLI almost -// universally uses "-flag=value". +// terraformSubcommand returns the first and second positional arguments +// from args, treating any token that starts with "-" as a flag and +// skipping it. This catches the common "-flag=value" form. It does not +// reliably handle the "-flag value" (space-separated) form: a flag value +// would be misread as a positional. Terraform's CLI almost universally +// uses "-flag=value", so this is acceptable in practice. func terraformSubcommand(args []string) (cmd, sub string) { var positional []string for _, a := range args { diff --git a/internal/wrapper/checkargs_test.go b/internal/wrapper/checkargs_test.go index 868acbf..f2063e6 100644 --- a/internal/wrapper/checkargs_test.go +++ b/internal/wrapper/checkargs_test.go @@ -1,15 +1,11 @@ package wrapper -import ( - "testing" - - "github.com/Masterminds/semver/v3" -) +import "testing" func TestCheckStateCommand(t *testing.T) { t.Run("import allowed when env var set", func(t *testing.T) { t.Setenv(stateCommandVar, "true") - err := checkStateCommand([]string{"import", "module.foo", "id"}, mustVer(t, "1.5.0")) + err := checkStateCommand([]string{"import", "module.foo", "id"}, mustVersion(t, "1.5.0")) if err != nil { t.Errorf("expected no error, got: %v", err) } @@ -17,7 +13,7 @@ func TestCheckStateCommand(t *testing.T) { t.Run("import allowed on pre-1.5.0 even without env var", func(t *testing.T) { t.Setenv(stateCommandVar, "") - err := checkStateCommand([]string{"import"}, mustVer(t, "1.4.7")) + err := checkStateCommand([]string{"import"}, mustVersion(t, "1.4.7")) if err != nil { t.Errorf("expected no error, got: %v", err) } @@ -25,7 +21,7 @@ func TestCheckStateCommand(t *testing.T) { t.Run("import refused on 1.6.0 without env var", func(t *testing.T) { t.Setenv(stateCommandVar, "") - err := checkStateCommand([]string{"import", "module.foo", "id"}, mustVer(t, "1.6.0")) + err := checkStateCommand([]string{"import", "module.foo", "id"}, mustVersion(t, "1.6.0")) if err == nil { t.Error("expected error, got nil") } @@ -33,7 +29,7 @@ func TestCheckStateCommand(t *testing.T) { t.Run("state mv allowed when env var set", func(t *testing.T) { t.Setenv(stateCommandVar, "true") - err := checkStateCommand([]string{"state", "mv", "--force"}, mustVer(t, "1.6.0")) + err := checkStateCommand([]string{"state", "mv", "--force"}, mustVersion(t, "1.6.0")) if err != nil { t.Errorf("expected no error, got: %v", err) } @@ -41,7 +37,7 @@ func TestCheckStateCommand(t *testing.T) { t.Run("state mv refused on 1.1.0+ without env var", func(t *testing.T) { t.Setenv(stateCommandVar, "") - err := checkStateCommand([]string{"state", "mv", "a", "b"}, mustVer(t, "1.6.0")) + err := checkStateCommand([]string{"state", "mv", "a", "b"}, mustVersion(t, "1.6.0")) if err == nil { t.Error("expected error, got nil") } @@ -49,7 +45,7 @@ func TestCheckStateCommand(t *testing.T) { t.Run("state rm refused on 1.7.0+ without env var", func(t *testing.T) { t.Setenv(stateCommandVar, "") - err := checkStateCommand([]string{"state", "rm", "module.foo"}, mustVer(t, "1.7.0")) + err := checkStateCommand([]string{"state", "rm", "module.foo"}, mustVersion(t, "1.7.0")) if err == nil { t.Error("expected error, got nil") } @@ -60,7 +56,7 @@ func TestCheckStateCommand(t *testing.T) { // only the actual subcommand. t.Run("flag value containing 'import' does not trip guard", func(t *testing.T) { t.Setenv(stateCommandVar, "") - err := checkStateCommand([]string{"apply", "-var=action=import"}, mustVer(t, "1.6.0")) + err := checkStateCommand([]string{"apply", "-var=action=import"}, mustVersion(t, "1.6.0")) if err != nil { t.Errorf("expected no error for apply with -var=action=import, got: %v", err) } @@ -70,7 +66,7 @@ func TestCheckStateCommand(t *testing.T) { t.Setenv(stateCommandVar, "") err := checkStateCommand( []string{"plan", "-target=module.state.mv"}, - mustVer(t, "1.6.0"), + mustVersion(t, "1.6.0"), ) if err != nil { t.Errorf("expected no error for plan with -target=module.state.mv, got: %v", err) @@ -80,7 +76,7 @@ func TestCheckStateCommand(t *testing.T) { t.Run("'mv' before 'state' positionally does not trip guard", func(t *testing.T) { t.Setenv(stateCommandVar, "") // 'mv' is the subcommand here (made up), not 'state mv'. - err := checkStateCommand([]string{"mv", "state"}, mustVer(t, "1.6.0")) + err := checkStateCommand([]string{"mv", "state"}, mustVersion(t, "1.6.0")) if err != nil { t.Errorf("expected no error, got: %v", err) } @@ -108,12 +104,3 @@ func TestTerraformSubcommand(t *testing.T) { } } } - -func mustVer(t *testing.T, s string) *semver.Version { - t.Helper() - v, err := semver.NewVersion(s) - if err != nil { - t.Fatalf("invalid version %q: %v", s, err) - } - return v -} diff --git a/internal/wrapper/exitcode_unix.go b/internal/wrapper/exitcode_unix.go new file mode 100644 index 0000000..d3d931e --- /dev/null +++ b/internal/wrapper/exitcode_unix.go @@ -0,0 +1,19 @@ +//go:build !windows + +package wrapper + +import ( + "os/exec" + "syscall" +) + +// exitCodeFromError returns the exit code matching what a shell would see +// from Terraform. If the child was killed by a signal, return 128+signum +// (the convention real terraform follows when its child is signaled), +// instead of -1 from exec.ExitError.ExitCode(). +func exitCodeFromError(exitError *exec.ExitError) int { + if status, ok := exitError.Sys().(syscall.WaitStatus); ok && status.Signaled() { + return 128 + int(status.Signal()) + } + return exitError.ExitCode() +} diff --git a/internal/wrapper/exitcode_windows.go b/internal/wrapper/exitcode_windows.go new file mode 100644 index 0000000..2a0786b --- /dev/null +++ b/internal/wrapper/exitcode_windows.go @@ -0,0 +1,12 @@ +//go:build windows + +package wrapper + +import "os/exec" + +// exitCodeFromError returns the child's exit code on Windows. There's no +// equivalent of the Unix "killed by signal" path here — a child that's +// killed shows up as a regular non-zero exit code. +func exitCodeFromError(exitError *exec.ExitError) int { + return exitError.ExitCode() +} diff --git a/internal/wrapper/logging.go b/internal/wrapper/logging.go new file mode 100644 index 0000000..e417ce6 --- /dev/null +++ b/internal/wrapper/logging.go @@ -0,0 +1,30 @@ +package wrapper + +import ( + "bytes" + "io" + "log" +) + +// SetupLogging configures the destination of the standard library log +// package for the lifetime of the process. When verbose is true, log +// output streams to errSink immediately. When verbose is false, output +// is held in an in-memory buffer and only flushed to errSink when the +// returned func is called — typically on the error path, so a successful +// run stays silent but a failure shows the full trace that led to it. +// +// SetupLogging mutates log's package-level state, so callers (and tests +// that use it) must serialize their use. +func SetupLogging(verbose bool, errSink io.Writer) (flush func()) { + if verbose { + log.SetOutput(errSink) + return func() {} + } + + buf := &bytes.Buffer{} + log.SetOutput(buf) + return func() { + _, _ = io.Copy(errSink, buf) + buf.Reset() + } +} diff --git a/internal/wrapper/logging_test.go b/internal/wrapper/logging_test.go new file mode 100644 index 0000000..19b8bc3 --- /dev/null +++ b/internal/wrapper/logging_test.go @@ -0,0 +1,46 @@ +package wrapper + +import ( + "bytes" + "log" + "os" + "strings" + "testing" +) + +// SetupLogging mutates the package-level log destination, so these tests +// must not run in parallel with anything that uses log. + +func TestSetupLogging_BuffersUntilFlush(t *testing.T) { + t.Cleanup(func() { log.SetOutput(os.Stderr) }) + + var sink bytes.Buffer + flush := SetupLogging(false, &sink) + + log.Print("first message") + log.Print("second message") + + if sink.Len() != 0 { + t.Errorf("expected nothing on sink before flush, got: %q", sink.String()) + } + + flush() + + out := sink.String() + if !strings.Contains(out, "first message") || !strings.Contains(out, "second message") { + t.Errorf("expected flushed output to contain both messages, got: %q", out) + } +} + +func TestSetupLogging_VerboseStreamsImmediately(t *testing.T) { + t.Cleanup(func() { log.SetOutput(os.Stderr) }) + + var sink bytes.Buffer + _ = SetupLogging(true, &sink) + + log.Print("streamed") + + if !strings.Contains(sink.String(), "streamed") { + t.Errorf("expected verbose mode to stream immediately, got: %q", sink.String()) + } +} diff --git a/internal/wrapper/signals_unix.go b/internal/wrapper/signals_unix.go new file mode 100644 index 0000000..834fe25 --- /dev/null +++ b/internal/wrapper/signals_unix.go @@ -0,0 +1,19 @@ +//go:build !windows + +package wrapper + +import ( + "os" + "syscall" +) + +// forwardedSignals lists the signals we propagate from the wrapper to the +// child Terraform process. Terraform itself decides what to do with them +// (e.g., SIGINT/SIGTERM trigger graceful shutdown). Without SIGHUP the +// child is orphaned when the parent terminal closes. +var forwardedSignals = []os.Signal{ + os.Interrupt, + syscall.SIGTERM, + syscall.SIGHUP, + syscall.SIGQUIT, +} diff --git a/internal/wrapper/signals_windows.go b/internal/wrapper/signals_windows.go new file mode 100644 index 0000000..3bce5db --- /dev/null +++ b/internal/wrapper/signals_windows.go @@ -0,0 +1,12 @@ +//go:build windows + +package wrapper + +import "os" + +// On Windows, os.Interrupt is the only meaningful signal to forward — the +// rest of the Unix signal set isn't delivered to console processes the +// same way. The runTerraform goroutine kills the child on any forwarded +// signal anyway because Windows doesn't honor cmd.Process.Signal for +// arbitrary signals. +var forwardedSignals = []os.Signal{os.Interrupt} diff --git a/internal/wrapper/wrapper.go b/internal/wrapper/wrapper.go index 956271b..fde1595 100644 --- a/internal/wrapper/wrapper.go +++ b/internal/wrapper/wrapper.go @@ -9,7 +9,7 @@ import ( "path/filepath" "runtime" "sort" - "syscall" + "strings" "github.com/etsy/terraform-demux/internal/releaseapi" @@ -17,6 +17,11 @@ import ( "github.com/hashicorp/terraform-config-inspect/tfconfig" ) +// cacheOverrideEnv lets users (and the integration test on macOS, where +// XDG_CACHE_HOME is ignored by os.UserCacheDir) point the cache at a +// specific directory instead of $HOME/Library/Caches/terraform-demux. +const cacheOverrideEnv = "TF_DEMUX_CACHE_HOME" + func RunTerraform(args []string, arch string) (int, error) { cacheDirectory, err := ensureCacheDirectory() if err != nil { @@ -63,6 +68,13 @@ func RunTerraform(args []string, arch string) (int, error) { } func ensureCacheDirectory() (string, error) { + if override := os.Getenv(cacheOverrideEnv); override != "" { + if err := os.MkdirAll(override, 0755); err != nil { + return "", fmt.Errorf("could not create cache directory %q: %w", override, err) + } + return override, nil + } + userCacheDir, err := os.UserCacheDir() if err != nil { return "", fmt.Errorf("could not determine user's cache directory: %w", err) @@ -76,54 +88,72 @@ func ensureCacheDirectory() (string, error) { return wrapperCacheDir, nil } +// getTerraformVersionConstraints walks from directory up to the filesystem +// root, returning the first set of required_version constraints it finds. +// Parse errors are fatal only at the user's cwd; broken .tf files in +// unrelated parents are logged and skipped so the wrapper still works. func getTerraformVersionConstraints(directory string) ([]*semver.Constraints, error) { currentDirectory := directory + isCwd := true for { - log.Printf("inspecting terraform module in %s", currentDirectory) - - module, diags := tfconfig.LoadModule(currentDirectory) - - // LoadModule returns success with an empty module when the directory - // has no .tf files, so HasErrors() means a real parse problem in - // something the user wrote — surface it instead of silently - // falling through to the latest stable release. - if diags.HasErrors() { - return nil, fmt.Errorf("invalid terraform configuration in %s: %w", currentDirectory, diags.Err()) + constraints, found, err := loadConstraintsAt(currentDirectory, isCwd) + if err != nil { + return nil, err } - - if len(module.RequiredCore) > 0 { - var allConstraints []*semver.Constraints - - for _, constraintString := range module.RequiredCore { - constraints, err := semver.NewConstraint(constraintString) - if err != nil { - return nil, fmt.Errorf("could not parse required_version %q: %w", constraintString, err) - } - allConstraints = append(allConstraints, constraints) - } - - log.Printf("found constraints: %v", allConstraints) - return allConstraints, nil + if found { + return constraints, nil } parentDirectory := filepath.Dir(currentDirectory) - if parentDirectory == currentDirectory { log.Printf("no constraints found") return nil, nil } - currentDirectory = parentDirectory + isCwd = false } } +// loadConstraintsAt parses any Terraform module in dir. It returns +// (constraints, true, nil) if the module declares required_version, +// (nil, false, nil) if there's nothing here to consider, and +// (nil, false, err) only for fatal parse errors at the user's cwd. +func loadConstraintsAt(dir string, isCwd bool) ([]*semver.Constraints, bool, error) { + log.Printf("inspecting terraform module in %s", dir) + + module, diags := tfconfig.LoadModule(dir) + if diags.HasErrors() { + // Only the user's actual working directory is a hard failure. + // A broken .tf in some unrelated parent (e.g. ~/scratch/foo) + // must not brick every wrapper invocation underneath it. + if isCwd { + return nil, false, fmt.Errorf("invalid terraform configuration in %s: %w", dir, diags.Err()) + } + log.Printf("ignoring parse errors in parent %s: %v", dir, diags.Err()) + return nil, false, nil + } + if len(module.RequiredCore) == 0 { + return nil, false, nil + } + + var allConstraints []*semver.Constraints + for _, constraintString := range module.RequiredCore { + c, err := semver.NewConstraint(constraintString) + if err != nil { + return nil, false, fmt.Errorf("could not parse required_version %q: %w", constraintString, err) + } + allConstraints = append(allConstraints, c) + } + + log.Printf("found constraints: %v", allConstraints) + return allConstraints, true, nil +} + func filterReleases(index releaseapi.ReleaseIndex, constraints []*semver.Constraints) (releaseapi.Release, error) { var versions semver.Collection for _, release := range index.Versions { - // Defensive: a release may decode without a Version (missing or - // unparseable field). Skip rather than panic on later access. if release.Version == nil { continue } @@ -161,7 +191,7 @@ func runTerraform(executable string, args []string) (int, error) { // Buffered so signal.Notify never drops a delivery. c := make(chan os.Signal, 1) - signal.Notify(c, os.Interrupt, syscall.SIGTERM) + signal.Notify(c, forwardedSignals...) defer signal.Stop(c) done := make(chan struct{}) @@ -172,9 +202,13 @@ func runTerraform(executable string, args []string) (int, error) { select { case s := <-c: if runtime.GOOS == "windows" { - _ = cmd.Process.Kill() + if err := cmd.Process.Kill(); err != nil { + log.Printf("could not kill Terraform: %v", err) + } } else { - _ = cmd.Process.Signal(s) + if err := cmd.Process.Signal(s); err != nil { + log.Printf("could not forward %s to Terraform: %v", s, err) + } } case <-done: return @@ -184,7 +218,7 @@ func runTerraform(executable string, args []string) (int, error) { if err := cmd.Wait(); err != nil { if exitError, ok := err.(*exec.ExitError); ok { - return exitError.ExitCode(), nil + return exitCodeFromError(exitError), nil } return 1, fmt.Errorf("error running Terraform: %w", err) } @@ -197,10 +231,25 @@ func runTerraform(executable string, args []string) (int, error) { func makeTerraformCmd(executable string, args []string) *exec.Cmd { cmd := exec.Command(executable, args...) - cmd.Env = os.Environ() + cmd.Env = filterDemuxEnv(os.Environ()) cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr return cmd } + +// filterDemuxEnv strips TF_DEMUX_* variables before exec'ing the child +// Terraform. They are wrapper-internal config (logging, arch override, +// state-command bypass, cache override) and must not leak into Terraform's +// own subprocesses, which can include another terraform-demux invocation. +func filterDemuxEnv(env []string) []string { + out := make([]string, 0, len(env)) + for _, e := range env { + if strings.HasPrefix(e, "TF_DEMUX_") { + continue + } + out = append(out, e) + } + return out +} diff --git a/internal/wrapper/wrapper_test.go b/internal/wrapper/wrapper_test.go index 21cddb2..d758cff 100644 --- a/internal/wrapper/wrapper_test.go +++ b/internal/wrapper/wrapper_test.go @@ -177,6 +177,71 @@ func TestGetTerraformVersionConstraints_SurfacesParseError(t *testing.T) { } } +// H-A regression: an unrelated parent directory with a broken .tf must +// not block the wrapper from running in the (clean) child directory. We +// only care about the user's actual cwd, not arbitrary ancestors. +func TestGetTerraformVersionConstraints_TolerantOfParentParseErrors(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "terraform.tf"), []byte("totally broken {{{"), 0644); err != nil { + t.Fatalf("write parent tf: %v", err) + } + child := filepath.Join(root, "child") + if err := os.MkdirAll(child, 0755); err != nil { + t.Fatalf("mkdir: %v", err) + } + + constraints, err := getTerraformVersionConstraints(child) + if err != nil { + t.Fatalf("expected parent parse error to be tolerated, got: %v", err) + } + if constraints != nil { + t.Errorf("expected nil constraints, got %v", constraints) + } +} + +func TestFilterDemuxEnv_StripsTFDemuxKeys(t *testing.T) { + in := []string{ + "PATH=/usr/bin", + "TF_DEMUX_LOG=1", + "HOME=/tmp", + "TF_DEMUX_ALLOW_STATE_COMMANDS=true", + "TF_DEMUX_ARCH=amd64", + "TF_DEMUX_CACHE_HOME=/tmp/cache", + "USER=alice", + } + out := filterDemuxEnv(in) + + want := map[string]bool{"PATH=/usr/bin": true, "HOME=/tmp": true, "USER=alice": true} + if len(out) != len(want) { + t.Fatalf("expected %d entries, got %d: %v", len(want), len(out), out) + } + for _, e := range out { + if !want[e] { + t.Errorf("unexpected entry %q in filtered env", e) + } + } +} + +func TestEnsureCacheDirectory_HonorsOverrideEnv(t *testing.T) { + override := filepath.Join(t.TempDir(), "td-cache") + t.Setenv(cacheOverrideEnv, override) + + got, err := ensureCacheDirectory() + if err != nil { + t.Fatalf("ensureCacheDirectory: %v", err) + } + if got != override { + t.Errorf("expected cache dir %q, got %q", override, got) + } + info, err := os.Stat(got) + if err != nil { + t.Fatalf("stat: %v", err) + } + if !info.IsDir() { + t.Errorf("override path %q is not a directory", got) + } +} + // H2 regression: a release with a nil Version (e.g., the upstream JSON // omits or fails to decode the "version" field) must be skipped, not // dereferenced. diff --git a/test.sh b/test.sh index 5062e9b..90fd8f2 100755 --- a/test.sh +++ b/test.sh @@ -11,7 +11,10 @@ else # shellcheck disable=SC2064 trap "rm -r $TMP_DIR" EXIT - export XDG_CACHE_HOME="$TMP_DIR" + # XDG_CACHE_HOME is ignored by os.UserCacheDir on macOS, so use the + # wrapper's explicit override env var to keep the integration suite + # from polluting the user's real cache. + export TF_DEMUX_CACHE_HOME="$TMP_DIR" fi function terraform_demux_version() {