Skip to content

feat!: update terraform to version 1.6.x, relax max version constraint #12027

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 6, 2024
Merged

feat!: update terraform to version 1.6.x, relax max version constraint #12027

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
170f0da
feat(provisioner): relax max terraform version constraint
johnstcn Feb 6, 2024
c35c9ff
feat!(scripts/Dockerfile.base): update bundled terraform to 1.6.x
johnstcn Feb 6, 2024
a2f3b77
bump terraform version in Dogfood image
johnstcn Feb 6, 2024
6bb145f
fix over-zealous rename
johnstcn Feb 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions dogfood/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,9 @@ RUN apt-get update --quiet && apt-get install --yes \
# Configure FIPS-compliant policies
update-crypto-policies --set FIPS

# NOTE: In scripts/Dockerfile.base we specifically install Terraform version 1.5.7
# as it is the last version licensed under the MPL. Installing the same version
# here for consistency.
RUN wget -O /tmp/terraform.zip "https://releases.hashicorp.com/terraform/1.5.7/terraform_1.5.7_linux_amd64.zip" && \
# NOTE: In scripts/Dockerfile.base we specifically install Terraform version 1.6.6.
# Installing the same version here to match.
RUN wget -O /tmp/terraform.zip "https://releases.hashicorp.com/terraform/1.6.6/terraform_1.6.6_linux_amd64.zip" && \
unzip /tmp/terraform.zip -d /usr/local/bin && \
rm -f /tmp/terraform.zip && \
chmod +x /usr/local/bin/terraform && \
Expand Down
4 changes: 2 additions & 2 deletions provisioner/terraform/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ var (
// TerraformVersion is the version of Terraform used internally
// when Terraform is not available on the system.
// NOTE: Keep this in sync with the version in scripts/Dockerfile.base.
TerraformVersion = version.Must(version.NewVersion("1.4.6"))
TerraformVersion = version.Must(version.NewVersion("1.6.6"))

minTerraformVersion = version.Must(version.NewVersion("1.1.0"))
maxTerraformVersion = version.Must(version.NewVersion("1.5.9")) // use .9 to automatically allow patch releases
maxTerraformVersion = version.Must(version.NewVersion("1.6.9")) // use .9 to automatically allow patch releases

terraformMinorVersionMismatch = xerrors.New("Terraform binary minor version mismatch.")
)
Expand Down
26 changes: 22 additions & 4 deletions provisioner/terraform/serve.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ type ServeOptions struct {
ExitTimeout time.Duration
}

func absoluteBinaryPath(ctx context.Context) (string, error) {
func absoluteBinaryPath(ctx context.Context, logger slog.Logger) (string, error) {
binaryPath, err := safeexec.LookPath("terraform")
if err != nil {
return "", xerrors.Errorf("Terraform binary not found: %w", err)
Expand All @@ -56,22 +56,37 @@ func absoluteBinaryPath(ctx context.Context) (string, error) {
}

// Checking the installed version of Terraform.
version, err := versionFromBinaryPath(ctx, absoluteBinary)
installedVersion, err := versionFromBinaryPath(ctx, absoluteBinary)
if err != nil {
return "", xerrors.Errorf("Terraform binary get version failed: %w", err)
}

if version.LessThan(minTerraformVersion) || version.GreaterThan(maxTerraformVersion) {
logger.Info(ctx, "detected terraform version",
slog.F("installed_version", installedVersion.String()),
slog.F("min_version", maxTerraformVersion.String()),
slog.F("max_version", maxTerraformVersion.String()))

if installedVersion.LessThan(minTerraformVersion) {
logger.Warn(ctx, "installed terraform version too old, will download known good version to cache")
return "", terraformMinorVersionMismatch
}

// Warn if the installed version is newer than what we've decided is the max.
// We used to ignore it and download our own version but this makes it easier
// to test out newer versions of Terraform.
if installedVersion.GreaterThanOrEqual(maxTerraformVersion) {
logger.Warn(ctx, "installed terraform version newer than expected, you may experience bugs",
slog.F("installed_version", installedVersion.String()),
slog.F("max_version", maxTerraformVersion.String()))
}

return absoluteBinary, nil
}

// Serve starts a dRPC server on the provided transport speaking Terraform provisioner.
func Serve(ctx context.Context, options *ServeOptions) error {
if options.BinaryPath == "" {
absoluteBinary, err := absoluteBinaryPath(ctx)
absoluteBinary, err := absoluteBinaryPath(ctx, options.Logger)
if err != nil {
// This is an early exit to prevent extra execution in case the context is canceled.
// It generally happens in unit tests since this method is asynchronous and
Expand All @@ -80,6 +95,9 @@ func Serve(ctx context.Context, options *ServeOptions) error {
return xerrors.Errorf("absolute binary context canceled: %w", err)
}

options.Logger.Warn(ctx, "no usable terraform binary found, downloading to cache dir",
slog.F("terraform_version", TerraformVersion.String()),
slog.F("cache_dir", options.CachePath))
binPath, err := Install(ctx, options.Logger, options.CachePath, TerraformVersion)
if err != nil {
return xerrors.Errorf("install terraform: %w", err)
Expand Down
22 changes: 12 additions & 10 deletions provisioner/terraform/serve_internal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package terraform

import (
"context"
"fmt"
"os"
"path/filepath"
Expand All @@ -11,40 +10,41 @@ import (

"github.com/stretchr/testify/require"
"golang.org/x/xerrors"

"github.com/coder/coder/v2/testutil"

"cdr.dev/slog/sloggers/slogtest"
)

// nolint:paralleltest
func Test_absoluteBinaryPath(t *testing.T) {
type args struct {
ctx context.Context
}
tests := []struct {
name string
args args
terraformVersion string
expectedErr error
}{
{
name: "TestCorrectVersion",
args: args{ctx: context.Background()},
terraformVersion: "1.3.0",
expectedErr: nil,
},
{
name: "TestOldVersion",
args: args{ctx: context.Background()},
terraformVersion: "1.0.9",
expectedErr: terraformMinorVersionMismatch,
},
{
name: "TestNewVersion",
args: args{ctx: context.Background()},
terraformVersion: "1.3.0",
expectedErr: nil,
},
{
name: "TestNewestNewVersion",
terraformVersion: "9.9.9",
expectedErr: nil,
},
{
name: "TestMalformedVersion",
args: args{ctx: context.Background()},
terraformVersion: "version",
expectedErr: xerrors.Errorf("Terraform binary get version failed: Malformed version: version"),
},
Expand All @@ -56,6 +56,7 @@ func Test_absoluteBinaryPath(t *testing.T) {
t.Skip("Dummy terraform executable on Windows requires sh which isn't very practical.")
}

log := slogtest.Make(t, nil)
// Create a temp dir with the binary
tempDir := t.TempDir()
terraformBinaryOutput := fmt.Sprintf(`#!/bin/sh
Expand Down Expand Up @@ -85,7 +86,8 @@ func Test_absoluteBinaryPath(t *testing.T) {
expectedAbsoluteBinary = filepath.Join(tempDir, "terraform")
}

actualAbsoluteBinary, actualErr := absoluteBinaryPath(tt.args.ctx)
ctx := testutil.Context(t, testutil.WaitShort)
actualAbsoluteBinary, actualErr := absoluteBinaryPath(ctx, log)

require.Equal(t, expectedAbsoluteBinary, actualAbsoluteBinary)
if tt.expectedErr == nil {
Expand Down
4 changes: 1 addition & 3 deletions scripts/Dockerfile.base
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ RUN apk add --no-cache \
# Terraform was disabled in the edge repo due to a build issue.
# https://gitlab.alpinelinux.org/alpine/aports/-/commit/f3e263d94cfac02d594bef83790c280e045eba35
# Using wget for now. Note that busybox unzip doesn't support streaming.
#
# WARNING: Do not update to 1.6.x, as it is the first release licensed under BSL instead of MPL.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're OK with this from a legal standpoint?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're OK with this from a legal standpoint?

I'm assuming so based on @bpmct 's wording here but good to super confirm.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we're good

RUN ARCH="$(arch)"; if [ "${ARCH}" == "x86_64" ]; then ARCH="amd64"; elif [ "${ARCH}" == "aarch64" ]; then ARCH="arm64"; fi; wget -O /tmp/terraform.zip "https://releases.hashicorp.com/terraform/1.5.7/terraform_1.5.7_linux_${ARCH}.zip" && \
RUN ARCH="$(arch)"; if [ "${ARCH}" == "x86_64" ]; then ARCH="amd64"; elif [ "${ARCH}" == "aarch64" ]; then ARCH="arm64"; fi; wget -O /tmp/terraform.zip "https://releases.hashicorp.com/terraform/1.6.6/terraform_1.6.6_linux_${ARCH}.zip" && \
busybox unzip /tmp/terraform.zip -d /usr/local/bin && \
rm -f /tmp/terraform.zip && \
chmod +x /usr/local/bin/terraform && \
Expand Down