pkg/terraform: improvements

pkg/terraform/executor.go

@@ -516,28 +516,27 @@ func (ex *Executor) logPath(id int) string {
 func (ex *Executor) checkVersion() error {
 	vOutput, err := ex.executeSync("--version")
 	if err != nil {
-		return fmt.Errorf("checking Terraform version: %w", err)
+		return fmt.Errorf("executing 'terraform --version': %w", err)
 	}
 
+	format := "Terraform v%s\n"
 	var vStr string
-	n, err := fmt.Sscanf(string(vOutput), "Terraform v%s\n", &vStr)
+	n, err := fmt.Sscanf(string(vOutput), format, &vStr)
 	if err != nil {
-		return fmt.Errorf("checking Terraform version: %w", err)
+		return fmt.Errorf("output %q does not match format %q: %w", string(vOutput), format, err)
 	}
 
 	if n != 1 {
-		return fmt.Errorf("error parsing Terraform version")
+		return fmt.Errorf("version not found in 'terraform --version' output")
 	}
 
 	v, err := version.NewVersion(vStr)
 	if err != nil {
-		return fmt.Errorf("checking Terraform version: %w", err)
+		return fmt.Errorf("parsing Terraform version %q: %w", vStr, err)
 	}
 
-	constraints, err := version.NewConstraint(requiredVersion)
-	if err != nil {
-		return fmt.Errorf("checking Terraform version: %w", err)
-	}
+	// requiredVersion is const, so we test it in unit tests.
+	constraints, _ := version.NewConstraint(requiredVersion)
 
 	if !constraints.Check(v) {
 		return fmt.Errorf("version '%s' of Terraform not supported. Needed %s", v, constraints)

pkg/terraform/executor_e2e_test.go

@@ -0,0 +1,75 @@
+// Copyright 2020 The Lokomotive Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//+build e2e
+
+package terraform_test
+
+import (
+	"io/ioutil"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/kinvolk/lokomotive/pkg/terraform"
+)
+
+func executor(t *testing.T) *terraform.Executor {
+	tmpDir, err := ioutil.TempDir("", "lokoctl-tests-")
+	if err != nil {
+		t.Fatalf("Creating tmp dir should succeed, got: %v", err)
+	}
+
+	t.Cleanup(func() {
+		if err := os.RemoveAll(tmpDir); err != nil {
+			t.Logf("Removing directory %q: %v", tmpDir, err)
+		}
+	})
+
+	conf := terraform.Config{
+		Verbose:    false,
+		WorkingDir: tmpDir,
+	}
+
+	ex, err := terraform.NewExecutor(conf)
+	if err != nil {
+		t.Fatalf("Creating new executor should succeed, got: %v", err)
+	}
+
+	return ex
+}
+
+func TestExecuteCheckErrors(t *testing.T) {
+	ex := executor(t)
+
+	if err := ex.Apply(); err == nil {
+		t.Fatalf("Applying on empty directory should fail")
+	}
+}
+
+func TestOutputIncludeKeyInError(t *testing.T) {
+	ex := executor(t)
+
+	k := "foo"
+	o := ""
+
+	err := ex.Output(k, &o)
+	if err == nil {
+		t.Fatalf("Output should fail on non existing installation")
+	}
+
+	if !strings.Contains(err.Error(), k) {
+		t.Fatalf("Error message should contain key, got: %v", err)
+	}
+}

pkg/terraform/executor_internal_test.go

@@ -0,0 +1,27 @@
+// Copyright 2020 The Lokomotive Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package terraform
+
+import (
+	"testing"
+
+	"github.com/hashicorp/go-version"
+)
+
+func TestRequiredVersion(t *testing.T) {
+	if _, err := version.NewConstraint(requiredVersion); err != nil {
+		t.Fatalf("requiredVersion const must be valid version constraint, got: %v", err)
+	}
+}

pkg/terraform/executor_test.go

@@ -1,55 +1,100 @@
-//+build e2e
+// Copyright 2020 The Lokomotive Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
 
-package terraform
+package terraform_test
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
-	"strings"
+	"path/filepath"
 	"testing"
+
+	"github.com/kinvolk/lokomotive/pkg/terraform"
 )
 
-func executor(t *testing.T) *Executor {
-	tmpDir, err := ioutil.TempDir("", "lokoctl-tests-")
-	if err != nil {
-		t.Fatalf("Creating tmp dir should succeed, got: %v", err)
+//nolint:funlen
+func TestVersionConstraint(t *testing.T) {
+	cases := map[string]struct {
+		output      string
+		expectError bool
+	}{
+		"valid": {
+			output: "Terraform v0.12.10",
+		},
+		"outdated": {
+			output:      "Terraform v0.11.0",
+			expectError: true,
+		},
+		"unsupported": {
+			output:      "Terraform v0.13.5",
+			expectError: true,
+		},
+		"with extra test": {
+			output: `Terraform v0.12.11
+
+Your version of Terraform is out of date! The latest version
+is 0.13.3. You can update by downloading from https://www.terraform.io/downloads.html`,
+		},
 	}
 
-	defer os.RemoveAll(tmpDir)
+	for n, c := range cases {
+		c := c
 
-	conf := Config{
-		Verbose:    false,
-		WorkingDir: tmpDir,
-	}
+		t.Run(n, func(t *testing.T) {
+			tmpDir, err := ioutil.TempDir("", "lokoctl-tests-")
+			if err != nil {
+				t.Fatalf("Creating tmp dir should succeed, got: %v", err)
+			}
 
-	ex, err := NewExecutor(conf)
-	if err != nil {
-		t.Fatalf("Creating new executor should succeed, got: %v", err)
-	}
+			t.Cleanup(func() {
+				if err := os.RemoveAll(tmpDir); err != nil {
+					t.Logf("Removing directory %q: %v", tmpDir, err)
+				}
+			})
 
-	return ex
-}
+			v := []byte(fmt.Sprintf(`#!/bin/sh
+		cat <<EOF
+%s
+EOF
+		`, c.output))
 
-func TestExecuteCheckErrors(t *testing.T) {
-	ex := executor(t)
+			path := filepath.Join(tmpDir, "terraform")
 
-	if err := ex.Apply(); err == nil {
-		t.Fatalf("Applying on empty directory should fail")
-	}
-}
+			// #nosec G306 // File must be executable to pretend it's a Terraform binary.
+			if err := ioutil.WriteFile(path, v, 0o700); err != nil {
+				t.Fatalf("Writing file %q: %v", path, err)
+			}
 
-func TestOutputIncludeKeyInError(t *testing.T) {
-	ex := executor(t)
+			if err := os.Setenv("PATH", fmt.Sprintf("%s:%s", tmpDir, os.Getenv("PATH"))); err != nil {
+				t.Fatalf("Overriding PATH variable for testing: %v", err)
+			}
 
-	k := "foo"
-	o := ""
+			conf := terraform.Config{
+				Verbose:    false,
+				WorkingDir: tmpDir,
+			}
 
-	err := ex.Output(k, &o)
-	if err == nil {
-		t.Fatalf("Output should fail on non existing installation")
-	}
+			_, err = terraform.NewExecutor(conf)
+
+			if !c.expectError && err != nil {
+				t.Fatalf("Creating new executor should succeed, got: %v", err)
+			}
 
-	if !strings.Contains(err.Error(), k) {
-		t.Fatalf("Error message should contain key, got: %v", err)
+			if c.expectError && err == nil {
+				t.Fatalf("Creating new executor should fail")
+			}
+		})
 	}
 }