Jul 11, 2023 · Jul 6, 2023 · Jul 6, 2023 · Jul 6, 2023 · Jul 6, 2023 · Jul 6, 2023
diff --git a/cli/templatecreate.go b/cli/templatecreate.go
 return xerrors.Errorf("check for lockfile: %w", err)
 }

 message := uploadFlags.templateMessage(inv)

 // Confirm upload of the directory.
 resp, err := uploadFlags.upload(inv, client)
 if err != nil {
 }

 job, err := createValidTemplateVersion(inv, createValidTemplateVersionArgs{
 Message:         message,
 Client:          client,
 Organization:    organization,
 Provisioner:     database.ProvisionerType(provisioner),

 type createValidTemplateVersionArgs struct {
 Name         string
 Message      string
 Client       *codersdk.Client
 Organization codersdk.Organization
 Provisioner  database.ProvisionerType

 req := codersdk.CreateTemplateVersionRequest{
 Name:               args.Name,
 Message:            args.Message,
 StorageMethod:      codersdk.ProvisionerStorageMethodFile,
 FileID:             args.FileID,
 Provisioner:        codersdk.ProvisionerType(args.Provisioner),
diff --git a/cli/templatepush.go b/cli/templatepush.go
 "fmt"
 "io"
 "path/filepath"
 "strings"
 "time"

 "github.com/briandowns/spinner"
 type templateUploadFlags struct {
 directory      string
 ignoreLockfile bool
 message        string
 }

 func (pf *templateUploadFlags) options() []clibase.Option {
 Description: "Ignore warnings about not having a .terraform.lock.hcl file present in the template.",
 Default:     "false",
 Value:       clibase.BoolOf(&pf.ignoreLockfile),
 }, {
 Flag:          "message",
 FlagShorthand: "m",
 Description:   "Specify a message describing the changes in this version of the template. Messages longer than 72 characters will be displayed as truncated.",
 Value:         clibase.StringOf(&pf.message),
 }}
 }

 return nil
 }

 func (pf *templateUploadFlags) templateMessage(inv *clibase.Invocation) string {
 title := strings.SplitN(pf.message, "\n", 2)[0]
 if len(title) > 72 {
 cliui.Warn(inv.Stdout, "Template message is longer than 72 characters, it will be displayed as truncated.")
 }
 if title != pf.message {
 cliui.Warn(inv.Stdout, "Template message contains newlines, only the first line will be displayed.")
 }
 if pf.message != "" {
 return pf.message
 }
 return "Uploaded from the CLI"
 }

 func (pf *templateUploadFlags) templateName(args []string) (string, error) {
 if pf.stdin() {
 // Can't infer name from directory if none provided.
 return xerrors.Errorf("check for lockfile: %w", err)
 }

 message := uploadFlags.templateMessage(inv)

 resp, err := uploadFlags.upload(inv, client)
 if err != nil {
 return err

 job, err := createValidTemplateVersion(inv, createValidTemplateVersionArgs{
 Name:            versionName,
 Message:         message,
 Client:          client,
 Organization:    organization,
 Provisioner:     database.ProvisionerType(provisioner),
diff --git a/cli/templatepush_test.go b/cli/templatepush_test.go
 "os"
 "path/filepath"
 "runtime"
 "strings"
 "testing"

 "github.com/stretchr/testify/assert"
 require.Equal(t, "example", templateVersions[1].Name)
 })

 t.Run("Message less than or equal to 72 chars", 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)
 source := clitest.CreateTemplateVersionSource(t, &echo.Responses{
 Parse:          echo.ParseComplete,
 ProvisionApply: echo.ProvisionComplete,
 })

 wantMessage := strings.Repeat("a", 72)

 inv, root := clitest.New(t, "templates", "push", template.Name, "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho), "--name", "example", "--message", wantMessage, "--yes")
 clitest.SetupConfig(t, client, root)
 pty := ptytest.New(t).Attach(inv)

 ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium)
 defer cancel()

 inv = inv.WithContext(ctx)
 w := clitest.StartWithWaiter(t, inv)

 pty.ExpectNoMatchBefore(ctx, "Template message is longer than 72 characters", "Updated version at")

 w.RequireSuccess()

 // Assert that the template version changed.
 templateVersions, err := client.TemplateVersionsByTemplate(ctx, codersdk.TemplateVersionsByTemplateRequest{
 TemplateID: template.ID,
 })
 require.NoError(t, err)
 assert.Len(t, templateVersions, 2)
 assert.NotEqual(t, template.ActiveVersionID, templateVersions[1].ID)
 require.Equal(t, wantMessage, templateVersions[1].Message)
 })

 t.Run("Message too long, warn but continue", 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)
 source := clitest.CreateTemplateVersionSource(t, &echo.Responses{
 Parse:          echo.ParseComplete,
 ProvisionApply: echo.ProvisionComplete,
 })

 ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
 defer cancel()

 for i, tt := range []struct {
 wantMessage string
 wantMatch   string
 }{
 {wantMessage: strings.Repeat("a", 73), wantMatch: "Template message is longer than 72 characters"},
 {wantMessage: "This is my title\n\nAnd this is my body.", wantMatch: "Template message contains newlines"},
 } {
 inv, root := clitest.New(t, "templates", "push", template.Name, "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho), "--message", tt.wantMessage, "--yes")
 clitest.SetupConfig(t, client, root)
 pty := ptytest.New(t).Attach(inv)

 inv = inv.WithContext(ctx)
 w := clitest.StartWithWaiter(t, inv)

 pty.ExpectMatchContext(ctx, tt.wantMatch)

 w.RequireSuccess()

 // Assert that the template version changed.
 templateVersions, err := client.TemplateVersionsByTemplate(ctx, codersdk.TemplateVersionsByTemplateRequest{
 TemplateID: template.ID,
 })
 require.NoError(t, err)
 assert.Len(t, templateVersions, 2+i)
 assert.NotEqual(t, template.ActiveVersionID, templateVersions[1+i].ID)
 require.Equal(t, tt.wantMessage, templateVersions[1+i].Message)
 }
 })

 t.Run("NoLockfile", func(t *testing.T) {
 t.Parallel()
 client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
diff --git a/cli/testdata/coder_templates_create_--help.golden b/cli/testdata/coder_templates_create_--help.golden
          Specify an inactivity TTL for workspaces created from this template.
          This licensed feature's default is 0h (off).

  -m, --message string
          Specify a message describing the changes in this version of the
          template. Messages longer than 72 characters will be displayed as
          truncated.

      --private bool
          Disable the default behavior of granting template access to the
          'everyone' group. The template permissions must be updated to allow
diff --git a/cli/testdata/coder_templates_push_--help.golden b/cli/testdata/coder_templates_push_--help.golden
          Ignore warnings about not having a .terraform.lock.hcl file present in
          the template.

  -m, --message string
          Specify a message describing the changes in this version of the
          template. Messages longer than 72 characters will be displayed as
          truncated.

      --name string
          Specify a name for the new template version. It will be automatically
          generated if not provided.
diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go
diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json
diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go
 return database.TemplateVersion{}, err
 }

 if len(arg.Message) > 1048576 {
 return database.TemplateVersion{}, xerrors.New("message too long")
 }

 q.mutex.Lock()
 defer q.mutex.Unlock()

 CreatedAt:      arg.CreatedAt,
 UpdatedAt:      arg.UpdatedAt,
 Name:           arg.Name,
 Message:        arg.Message,
 Readme:         arg.Readme,
 JobID:          arg.JobID,
 CreatedBy:      arg.CreatedBy,
diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go
 CreatedAt:      takeFirst(orig.CreatedAt, database.Now()),
 UpdatedAt:      takeFirst(orig.UpdatedAt, database.Now()),
 Name:           takeFirst(orig.Name, namesgenerator.GetRandomName(1)),
 Message:        orig.Message,
 Readme:         takeFirst(orig.Readme, namesgenerator.GetRandomName(1)),
 JobID:          takeFirst(orig.JobID, uuid.New()),
 CreatedBy:      takeFirst(orig.CreatedBy, uuid.New()),
diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql
diff --git a/coderd/database/migrations/000137_add_template_version_message.down.sql b/coderd/database/migrations/000137_add_template_version_message.down.sql
 ALTER TABLE template_versions DROP COLUMN message;
diff --git a/coderd/database/migrations/000137_add_template_version_message.up.sql b/coderd/database/migrations/000137_add_template_version_message.up.sql
 ALTER TABLE template_versions ADD COLUMN message varchar(1048576) NOT NULL DEFAULT '';

 COMMENT ON COLUMN template_versions.message IS 'Message describing the changes in this version of the template, similar to a Git commit message. Like a commit message, this should be a short, high-level description of the changes in this version of the template. This message is immutable and should not be updated after the fact.';
diff --git a/coderd/database/models.go b/coderd/database/models.go
Original file line number	Diff line number	Diff line change
Expand Up		@@ -92,6 +92,8 @@ func (r RootCmd) templateCreate() clibase.Cmd {
		return xerrors.Errorf("check for lockfile: %w", err)
		}

		message := uploadFlags.templateMessage(inv)

		// Confirm upload of the directory.
		resp, err := uploadFlags.upload(inv, client)
		if err != nil {
Expand All		@@ -104,6 +106,7 @@ func (r RootCmd) templateCreate() clibase.Cmd {
		}

		job, err := createValidTemplateVersion(inv, createValidTemplateVersionArgs{
		Message: message,
		Client: client,
		Organization: organization,
		Provisioner: database.ProvisionerType(provisioner),
Expand DownExpand Up		@@ -205,6 +208,7 @@ func (r RootCmd) templateCreate() clibase.Cmd {

		type createValidTemplateVersionArgs struct {
		Name string
		Message string
		Client *codersdk.Client
		Organization codersdk.Organization
		Provisioner database.ProvisionerType
Expand DownExpand Up		@@ -238,6 +242,7 @@ func createValidTemplateVersion(inv *clibase.Invocation, args createValidTemplat

		req := codersdk.CreateTemplateVersionRequest{
		Name: args.Name,
		Message: args.Message,
		StorageMethod: codersdk.ProvisionerStorageMethodFile,
		FileID: args.FileID,
		Provisioner: codersdk.ProvisionerType(args.Provisioner),
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -5,6 +5,7 @@ import (
		"fmt"
		"io"
		"path/filepath"
		"strings"
		"time"

		"github.com/briandowns/spinner"
Expand All		@@ -21,6 +22,7 @@ import (
		type templateUploadFlags struct {
		directory string
		ignoreLockfile bool
		message string
		}

		func (pf *templateUploadFlags) options() []clibase.Option {
Expand All		@@ -35,6 +37,11 @@ func (pf *templateUploadFlags) options() []clibase.Option {
		Description: "Ignore warnings about not having a .terraform.lock.hcl file present in the template.",
		Default: "false",
		Value: clibase.BoolOf(&pf.ignoreLockfile),
		}, {
		Flag: "message",
		FlagShorthand: "m",
		Description: "Specify a message describing the changes in this version of the template. Messages longer than 72 characters will be displayed as truncated.",
		Value: clibase.StringOf(&pf.message),
		}}
		}

Expand DownExpand Up		@@ -110,6 +117,20 @@ func (pf templateUploadFlags) checkForLockfile(inv clibase.Invocation) error {
		return nil
		}

		func (pf templateUploadFlags) templateMessage(inv clibase.Invocation) string {
		title := strings.SplitN(pf.message, "\n", 2)[0]
		if len(title) > 72 {
		cliui.Warn(inv.Stdout, "Template message is longer than 72 characters, it will be displayed as truncated.")
		}
		if title != pf.message {
		cliui.Warn(inv.Stdout, "Template message contains newlines, only the first line will be displayed.")
		}
		if pf.message != "" {
		return pf.message
		}
		return "Uploaded from the CLI"
mafredri marked this conversation as resolved. Show resolvedHide resolved
		}

		func (pf *templateUploadFlags) templateName(args []string) (string, error) {
		if pf.stdin() {
		// Can't infer name from directory if none provided.
Expand DownExpand Up		@@ -174,6 +195,8 @@ func (r RootCmd) templatePush() clibase.Cmd {
		return xerrors.Errorf("check for lockfile: %w", err)
		}

		message := uploadFlags.templateMessage(inv)

		resp, err := uploadFlags.upload(inv, client)
		if err != nil {
		return err
Expand All		@@ -186,6 +209,7 @@ func (r RootCmd) templatePush() clibase.Cmd {

		job, err := createValidTemplateVersion(inv, createValidTemplateVersionArgs{
		Name: versionName,
		Message: message,
		Client: client,
		Organization: organization,
		Provisioner: database.ProvisionerType(provisioner),
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -6,6 +6,7 @@ import (
		"os"
		"path/filepath"
		"runtime"
		"strings"
		"testing"

		"github.com/stretchr/testify/assert"
Expand DownExpand Up		@@ -70,6 +71,90 @@ func TestTemplatePush(t *testing.T) {
		require.Equal(t, "example", templateVersions[1].Name)
		})

		t.Run("Message less than or equal to 72 chars", 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)
		source := clitest.CreateTemplateVersionSource(t, &echo.Responses{
		Parse: echo.ParseComplete,
		ProvisionApply: echo.ProvisionComplete,
		})

		wantMessage := strings.Repeat("a", 72)

		inv, root := clitest.New(t, "templates", "push", template.Name, "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho), "--name", "example", "--message", wantMessage, "--yes")
		clitest.SetupConfig(t, client, root)
		pty := ptytest.New(t).Attach(inv)

		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium)
		defer cancel()

		inv = inv.WithContext(ctx)
		w := clitest.StartWithWaiter(t, inv)

		pty.ExpectNoMatchBefore(ctx, "Template message is longer than 72 characters", "Updated version at")

		w.RequireSuccess()

		// Assert that the template version changed.
		templateVersions, err := client.TemplateVersionsByTemplate(ctx, codersdk.TemplateVersionsByTemplateRequest{
		TemplateID: template.ID,
		})
		require.NoError(t, err)
		assert.Len(t, templateVersions, 2)
		assert.NotEqual(t, template.ActiveVersionID, templateVersions[1].ID)
		require.Equal(t, wantMessage, templateVersions[1].Message)
		})

		t.Run("Message too long, warn but continue", 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)
		source := clitest.CreateTemplateVersionSource(t, &echo.Responses{
		Parse: echo.ParseComplete,
		ProvisionApply: echo.ProvisionComplete,
		})

		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
		defer cancel()

		for i, tt := range []struct {
		wantMessage string
		wantMatch string
		}{
		{wantMessage: strings.Repeat("a", 73), wantMatch: "Template message is longer than 72 characters"},
		{wantMessage: "This is my title\n\nAnd this is my body.", wantMatch: "Template message contains newlines"},
		} {
		inv, root := clitest.New(t, "templates", "push", template.Name, "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho), "--message", tt.wantMessage, "--yes")
		clitest.SetupConfig(t, client, root)
		pty := ptytest.New(t).Attach(inv)

		inv = inv.WithContext(ctx)
		w := clitest.StartWithWaiter(t, inv)

		pty.ExpectMatchContext(ctx, tt.wantMatch)

		w.RequireSuccess()

		// Assert that the template version changed.
		templateVersions, err := client.TemplateVersionsByTemplate(ctx, codersdk.TemplateVersionsByTemplateRequest{
		TemplateID: template.ID,
		})
		require.NoError(t, err)
		assert.Len(t, templateVersions, 2+i)
		assert.NotEqual(t, template.ActiveVersionID, templateVersions[1+i].ID)
		require.Equal(t, tt.wantMessage, templateVersions[1+i].Message)
		}
		})

		t.Run("NoLockfile", func(t *testing.T) {
		t.Parallel()
		client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -21,6 +21,11 @@ Create a template from the current directory or as specified by flag
		Specify an inactivity TTL for workspaces created from this template.
		This licensed feature's default is 0h (off).

		-m, --message string
		Specify a message describing the changes in this version of the
		template. Messages longer than 72 characters will be displayed as
		truncated.

		--private bool
		Disable the default behavior of granting template access to the
		'everyone' group. The template permissions must be updated to allow
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -17,6 +17,11 @@ Push a new template version from the current directory or as specified by flag
		Ignore warnings about not having a .terraform.lock.hcl file present in
		the template.

		-m, --message string
		Specify a message describing the changes in this version of the
		template. Messages longer than 72 characters will be displayed as
		truncated.

		--name string
		Specify a name for the new template version. It will be automatically
		generated if not provided.
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -3921,6 +3921,10 @@ func (q *fakeQuerier) InsertTemplateVersion(_ context.Context, arg database.Inse
		return database.TemplateVersion{}, err
		}

		if len(arg.Message) > 1048576 {
		return database.TemplateVersion{}, xerrors.New("message too long")
		}

		q.mutex.Lock()
		defer q.mutex.Unlock()

Expand All		@@ -3932,6 +3936,7 @@ func (q *fakeQuerier) InsertTemplateVersion(_ context.Context, arg database.Inse
		CreatedAt: arg.CreatedAt,
		UpdatedAt: arg.UpdatedAt,
		Name: arg.Name,
		Message: arg.Message,
		Readme: arg.Readme,
		JobID: arg.JobID,
		CreatedBy: arg.CreatedBy,
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -477,6 +477,7 @@ func TemplateVersion(t testing.TB, db database.Store, orig database.TemplateVers
		CreatedAt: takeFirst(orig.CreatedAt, database.Now()),
		UpdatedAt: takeFirst(orig.UpdatedAt, database.Now()),
		Name: takeFirst(orig.Name, namesgenerator.GetRandomName(1)),
		Message: orig.Message,
Copy link Member mtojekJul 6, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. nit: what about testing? do we need a random message generated for unit tests or maybe benchmarks in the future? Copy link MemberAuthor mafredriJul 6, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. It's a valid concern, although I didn't do it because I'm sure how we'd set an empty message if we use`takeFirst` here on a randomly generated one. Copy link Member EmyrkJul 7, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. @mafredri the empty`takeFirst` is such an annoying problem 😢
		Readme: takeFirst(orig.Readme, namesgenerator.GetRandomName(1)),
		JobID: takeFirst(orig.JobID, uuid.New()),
		CreatedBy: takeFirst(orig.CreatedBy, uuid.New()),
Expand Down
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		ALTER TABLE template_versions DROP COLUMN message;
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		ALTER TABLE template_versions ADD COLUMN message varchar(1048576) NOT NULL DEFAULT '';

		COMMENT ON COLUMN template_versions.message IS 'Message describing the changes in this version of the template, similar to a Git commit message. Like a commit message, this should be a short, high-level description of the changes in this version of the template. This message is immutable and should not be updated after the fact.';