chore(cli): warn on template push or create when no lockfile present (#8059)

cli/clitest/clitest.go

@@ -6,7 +6,6 @@ import (
 	"context"
 	"errors"
 	"io"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
@@ -86,7 +85,10 @@ func SetupConfig(t *testing.T, client *codersdk.Client, root config.Root) {
 // new temporary testing directory.
 func CreateTemplateVersionSource(t *testing.T, responses *echo.Responses) string {
 	directory := t.TempDir()
-	f, err := ioutil.TempFile(directory, "*.tf")
+	f, err := os.CreateTemp(directory, "*.tf")
+	require.NoError(t, err)
+	_ = f.Close()
+	f, err = os.Create(filepath.Join(directory, ".terraform.lock.hcl"))
 	require.NoError(t, err)
 	_ = f.Close()
 	data, err := echo.Tar(responses)

cli/templatecreate.go

@@ -87,6 +87,11 @@ func (r *RootCmd) templateCreate() *clibase.Cmd {
 				return xerrors.Errorf("A template already exists named %q!", templateName)
 			}
 
+			err = uploadFlags.checkForLockfile(inv)
+			if err != nil {
+				return xerrors.Errorf("check for lockfile: %w", err)
+			}
+
 			// Confirm upload of the directory.
 			resp, err := uploadFlags.upload(inv, client)
 			if err != nil {
@@ -185,7 +190,6 @@ func (r *RootCmd) templateCreate() *clibase.Cmd {
 			Default:     "0h",
 			Value:       clibase.DurationOf(&inactivityTTL),
 		},
-		uploadFlags.option(),
 		{
 			Flag:        "test.provisioner",
 			Description: "Customize the provisioner backend.",
@@ -195,6 +199,7 @@ func (r *RootCmd) templateCreate() *clibase.Cmd {
 		},
 		cliui.SkipPromptOption(),
 	}
+	cmd.Options = append(cmd.Options, uploadFlags.options()...)
 	return cmd
 }
 

cli/templatecreate_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -80,6 +81,87 @@ func TestTemplateCreate(t *testing.T) {
 			}
 		}
 	})
+	t.Run("CreateNoLockfile", func(t *testing.T) {
+		t.Parallel()
+		client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
+		coderdtest.CreateFirstUser(t, client)
+		source := clitest.CreateTemplateVersionSource(t, &echo.Responses{
+			Parse:          echo.ParseComplete,
+			ProvisionApply: provisionCompleteWithAgent,
+		})
+		require.NoError(t, os.Remove(filepath.Join(source, ".terraform.lock.hcl")))
+		args := []string{
+			"templates",
+			"create",
+			"my-template",
+			"--directory", source,
+			"--test.provisioner", string(database.ProvisionerTypeEcho),
+			"--default-ttl", "24h",
+		}
+		inv, root := clitest.New(t, args...)
+		clitest.SetupConfig(t, client, root)
+		pty := ptytest.New(t).Attach(inv)
+
+		execDone := make(chan error)
+		go func() {
+			execDone <- inv.Run()
+		}()
+
+		matches := []struct {
+			match string
+			write string
+		}{
+			{match: "No .terraform.lock.hcl file found"},
+			{match: "Upload", write: "no"},
+		}
+		for _, m := range matches {
+			pty.ExpectMatch(m.match)
+			if len(m.write) > 0 {
+				pty.WriteLine(m.write)
+			}
+		}
+
+		// cmd should error once we say no.
+		require.Error(t, <-execDone)
+	})
+	t.Run("CreateNoLockfileIgnored", func(t *testing.T) {
+		t.Parallel()
+		client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
+		coderdtest.CreateFirstUser(t, client)
+		source := clitest.CreateTemplateVersionSource(t, &echo.Responses{
+			Parse:          echo.ParseComplete,
+			ProvisionApply: provisionCompleteWithAgent,
+		})
+		require.NoError(t, os.Remove(filepath.Join(source, ".terraform.lock.hcl")))
+		args := []string{
+			"templates",
+			"create",
+			"my-template",
+			"--directory", source,
+			"--test.provisioner", string(database.ProvisionerTypeEcho),
+			"--default-ttl", "24h",
+			"--ignore-lockfile",
+		}
+		inv, root := clitest.New(t, args...)
+		clitest.SetupConfig(t, client, root)
+		pty := ptytest.New(t).Attach(inv)
+
+		execDone := make(chan error)
+		go func() {
+			execDone <- inv.Run()
+		}()
+
+		{
+			ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium)
+			defer cancel()
+
+			pty.ExpectNoMatchBefore(ctx, "No .terraform.lock.hcl file found", "Upload")
+			pty.WriteLine("no")
+		}
+
+		// cmd should error once we say no.
+		require.Error(t, <-execDone)
+	})
 
 	t.Run("CreateStdin", func(t *testing.T) {
 		t.Parallel()

cli/templatepush.go

@@ -19,17 +19,23 @@ import (
 
 // templateUploadFlags is shared by `templates create` and `templates push`.
 type templateUploadFlags struct {
-	directory string
+	directory      string
+	ignoreLockfile bool
 }
 
-func (pf *templateUploadFlags) option() clibase.Option {
+func (pf *templateUploadFlags) options() []clibase.Option {
-	return clibase.Option{
+	return []clibase.Option{{
 		Flag:          "directory",
 		FlagShorthand: "d",
 		Description:   "Specify the directory to create from, use '-' to read tar from stdin.",
 		Default:       ".",
 		Value:         clibase.StringOf(&pf.directory),
-	}
+	}, {
+		Flag:        "ignore-lockfile",
+		Description: "Ignore warnings about not having a .terraform.lock.hcl file present in the template.",
+		Default:     "false",
+		Value:       clibase.BoolOf(&pf.ignoreLockfile),
+	}}
 }
 
 func (pf *templateUploadFlags) setWorkdir(wd string) {
@@ -84,6 +90,26 @@ func (pf *templateUploadFlags) upload(inv *clibase.Invocation, client *codersdk.
 	return &resp, nil
 }
 
+func (pf *templateUploadFlags) checkForLockfile(inv *clibase.Invocation) error {
+	if pf.stdin() || pf.ignoreLockfile {
+		// Just assume there's a lockfile if reading from stdin.
+		return nil
+	}
+
+	hasLockfile, err := provisionersdk.DirHasLockfile(pf.directory)
+	if err != nil {
+		return xerrors.Errorf("dir has lockfile: %w", err)
+	}
+
+	if !hasLockfile {
+		cliui.Warn(inv.Stdout, "No .terraform.lock.hcl file found",
+			"When provisioning, Coder will be unable to cache providers without a lockfile and must download them from the internet each time.",
+			"Create one by running "+cliui.DefaultStyles.Code.Render("terraform init")+" in your template directory.",
+		)
+	}
+	return nil
+}
+
 func (pf *templateUploadFlags) templateName(args []string) (string, error) {
 	if pf.stdin() {
 		// Can't infer name from directory if none provided.
@@ -143,6 +169,11 @@ func (r *RootCmd) templatePush() *clibase.Cmd {
 				return err
 			}
 
+			err = uploadFlags.checkForLockfile(inv)
+			if err != nil {
+				return xerrors.Errorf("check for lockfile: %w", err)
+			}
+
 			resp, err := uploadFlags.upload(inv, client)
 			if err != nil {
 				return err
@@ -236,7 +267,7 @@ func (r *RootCmd) templatePush() *clibase.Cmd {
 			Value:       clibase.BoolOf(&activate),
 		},
 		cliui.SkipPromptOption(),
-		uploadFlags.option(),
 	}
+	cmd.Options = append(cmd.Options, uploadFlags.options()...)
 	return cmd
 }

cli/templatepush_test.go

@@ -18,6 +18,7 @@ import (
 	"github.com/coder/coder/provisioner/echo"
 	"github.com/coder/coder/provisionersdk/proto"
 	"github.com/coder/coder/pty/ptytest"
+	"github.com/coder/coder/testutil"
 )
 
 func TestTemplatePush(t *testing.T) {
@@ -69,6 +70,86 @@ func TestTemplatePush(t *testing.T) {
 		require.Equal(t, "example", templateVersions[1].Name)
 	})
 
+	t.Run("NoLockfile", func(t *testing.T) {
+		t.Parallel()
+		client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
+		user := coderdtest.CreateFirstUser(t, client)
+		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
+		_ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
+
+		template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
+
+		// Test the cli command.
+		source := clitest.CreateTemplateVersionSource(t, &echo.Responses{
+			Parse:          echo.ParseComplete,
+			ProvisionApply: echo.ProvisionComplete,
+		})
+		require.NoError(t, os.Remove(filepath.Join(source, ".terraform.lock.hcl")))
+
+		inv, root := clitest.New(t, "templates", "push", template.Name, "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho), "--name", "example")
+		clitest.SetupConfig(t, client, root)
+		pty := ptytest.New(t).Attach(inv)
+
+		execDone := make(chan error)
+		go func() {
+			execDone <- inv.Run()
+		}()
+
+		matches := []struct {
+			match string
+			write string
+		}{
+			{match: "No .terraform.lock.hcl file found"},
+			{match: "Upload", write: "no"},
+		}
+		for _, m := range matches {
+			pty.ExpectMatch(m.match)
+			if m.write != "" {
+				pty.WriteLine(m.write)
+			}
+		}
+
+		// cmd should error once we say no.
+		require.Error(t, <-execDone)
+	})
+
+	t.Run("NoLockfileIgnored", func(t *testing.T) {
+		t.Parallel()
+		client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
+		user := coderdtest.CreateFirstUser(t, client)
+		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
+		_ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
+
+		template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
+
+		// Test the cli command.
+		source := clitest.CreateTemplateVersionSource(t, &echo.Responses{
+			Parse:          echo.ParseComplete,
+			ProvisionApply: echo.ProvisionComplete,
+		})
+		require.NoError(t, os.Remove(filepath.Join(source, ".terraform.lock.hcl")))
+
+		inv, root := clitest.New(t, "templates", "push", template.Name, "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho), "--name", "example", "--ignore-lockfile")
+		clitest.SetupConfig(t, client, root)
+		pty := ptytest.New(t).Attach(inv)
+
+		execDone := make(chan error)
+		go func() {
+			execDone <- inv.Run()
+		}()
+
+		{
+			ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium)
+			defer cancel()
+
+			pty.ExpectNoMatchBefore(ctx, "No .terraform.lock.hcl file found", "Upload")
+			pty.WriteLine("no")
+		}
+
+		// cmd should error once we say no.
+		require.Error(t, <-execDone)
+	})
+
 	t.Run("PushInactiveTemplateVersion", func(t *testing.T) {
 		t.Parallel()
 		client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})

cli/testdata/coder_templates_create_--help.golden

@@ -13,6 +13,10 @@ Create a template from the current directory or as specified by flag
           Specify a failure TTL for workspaces created from this template. This
           licensed feature's default is 0h (off).
 
+      --ignore-lockfile bool (default: false)
+          Ignore warnings about not having a .terraform.lock.hcl file present in
+          the template.
+
       --inactivity-ttl duration (default: 0h)
           Specify an inactivity TTL for workspaces created from this template.
           This licensed feature's default is 0h (off).

cli/testdata/coder_templates_push_--help.golden

@@ -13,6 +13,10 @@ Push a new template version from the current directory or as specified by flag
   -d, --directory string (default: .)
           Specify the directory to create from, use '-' to read tar from stdin.
 
+      --ignore-lockfile bool (default: false)
+          Ignore warnings about not having a .terraform.lock.hcl file present in
+          the template.
+
       --name string
           Specify a name for the new template version. It will be automatically
           generated if not provided.

docs/cli/templates_create.md

@@ -39,6 +39,15 @@ Specify the directory to create from, use '-' to read tar from stdin.
 
 Specify a failure TTL for workspaces created from this template. This licensed feature's default is 0h (off).
 
+### --ignore-lockfile
+
+|         |                    |
+| ------- | ------------------ |
+| Type    | <code>bool</code>  |
+| Default | <code>false</code> |
+
+Ignore warnings about not having a .terraform.lock.hcl file present in the template.
+
 ### --inactivity-ttl
 
 |         |                       |

docs/cli/templates_push.md

@@ -38,6 +38,15 @@ Always prompt all parameters. Does not pull parameter values from active templat
 
 Specify the directory to create from, use '-' to read tar from stdin.
 
+### --ignore-lockfile
+
+|         |                    |
+| ------- | ------------------ |
+| Type    | <code>bool</code>  |
+| Default | <code>false</code> |
+
+Ignore warnings about not having a .terraform.lock.hcl file present in the template.
+
 ### --name
 
 |      |                     |

provisionersdk/archive.go

@@ -34,6 +34,10 @@ func dirHasExt(dir string, exts ...string) (bool, error) {
 	return false, nil
 }
 
+func DirHasLockfile(dir string) (bool, error) {
+	return dirHasExt(dir, ".terraform.lock.hcl")
+}
+
 // Tar archives a Terraform directory.
 func Tar(w io.Writer, directory string, limit int64) error {
 	// The total bytes written must be under the limit, so use -1

pty/ptytest/ptytest.go

@@ -181,6 +181,39 @@ func (e *outExpecter) ExpectMatchContext(ctx context.Context, str string) string
 	return buffer.String()
 }
 
+// ExpectNoMatchBefore validates that `match` does not occur before `before`.
+func (e *outExpecter) ExpectNoMatchBefore(ctx context.Context, match, before string) string {
+	e.t.Helper()
+
+	var buffer bytes.Buffer
+	err := e.doMatchWithDeadline(ctx, "ExpectNoMatchBefore", func() error {
+		for {
+			r, _, err := e.runeReader.ReadRune()
+			if err != nil {
+				return err
+			}
+			_, err = buffer.WriteRune(r)
+			if err != nil {
+				return err
+			}
+
+			if strings.Contains(buffer.String(), match) {
+				return xerrors.Errorf("found %q before %q", match, before)
+			}
+
+			if strings.Contains(buffer.String(), before) {
+				return nil
+			}
+		}
+	})
+	if err != nil {
+		e.fatalf("read error", "%v (wanted no %q before %q; got %q)", err, match, before, buffer.String())
+		return ""
+	}
+	e.logf("matched %q = %q", before, stripansi.Strip(buffer.String()))
+	return buffer.String()
+}
+
 func (e *outExpecter) Peek(ctx context.Context, n int) []byte {
 	e.t.Helper()