coder · johnstcn · Aug 24, 2022 · Aug 24, 2022 · Aug 24, 2022 · Aug 24, 2022
diff --git a/cli/templateedit.go b/cli/templateedit.go
@@ -59,8 +59,8 @@ func templateEdit() *cobra.Command {
 	cmd.Flags().StringVarP(&name, "name", "", "", "Edit the template name")
 	cmd.Flags().StringVarP(&description, "description", "", "", "Edit the template description")
 	cmd.Flags().StringVarP(&icon, "icon", "", "", "Edit the template icon path")
-	cmd.Flags().DurationVarP(&maxTTL, "max-ttl", "", 0, "Edit the template maximum time before shutdown")
-	cmd.Flags().DurationVarP(&minAutostartInterval, "min-autostart-interval", "", 0, "Edit the template minimum autostart interval")
+	cmd.Flags().DurationVarP(&maxTTL, "max-ttl", "", 0, "Edit the template maximum time before shutdown - workspaces created from this template cannot stay running longer than this.")
+	cmd.Flags().DurationVarP(&minAutostartInterval, "min-autostart-interval", "", 0, "Edit the template minimum autostart interval - workspaces created from this template must wait at least this long between autostarts.")
 	cliui.AllowSkipPrompt(cmd)
 
 	return cmd

diff --git a/coderd/authorize.go b/coderd/authorize.go
@@ -49,6 +49,7 @@ func (api *API) Authorize(r *http.Request, action rbac.Action, object rbac.Objec
 // This function will log appropriately, but the caller must return an
 // error to the api client.
 // Eg:
+//
 //	if !h.Authorize(...) {
 //		httpapi.Forbidden(rw)
 //		return

diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go
@@ -1564,10 +1564,6 @@ func (q *fakeQuerier) InsertTemplate(_ context.Context, arg database.InsertTempl
 	q.mutex.Lock()
 	defer q.mutex.Unlock()
 
-	// default values
-	if arg.MaxTtl == 0 {
-		arg.MaxTtl = int64(168 * time.Hour)
-	}
 	if arg.MinAutostartInterval == 0 {
 		arg.MinAutostartInterval = int64(time.Hour)
 	}

diff --git a/coderd/database/migrations/000038_template_max_ttl_cap_7_days.down.sql b/coderd/database/migrations/000038_template_max_ttl_cap_7_days.down.sql
@@ -0,0 +1 @@
+-- this is a no-op
diff --git a/coderd/database/migrations/000038_template_max_ttl_cap_7_days.up.sql b/coderd/database/migrations/000038_template_max_ttl_cap_7_days.up.sql
@@ -0,0 +1,2 @@
+-- Set a cap of 7 days on template max_ttl
+UPDATE templates SET max_ttl = 604800000000000 WHERE max_ttl > 604800000000000;
diff --git a/coderd/templates.go b/coderd/templates.go
@@ -173,9 +173,28 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque
 	}
 
 	maxTTL := maxTTLDefault
-	if !ptr.NilOrZero(createTemplate.MaxTTLMillis) {
+	if createTemplate.MaxTTLMillis != nil {
 		maxTTL = time.Duration(*createTemplate.MaxTTLMillis) * time.Millisecond
 	}
+	if maxTTL < 0 {
+		httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{
+			Message: "Invalid create template request.",
+			Validations: []codersdk.ValidationError{
+				{Field: "max_ttl_ms", Detail: "Must be a positive integer."},
+			},
+		})
+		return
+	}
+
+	if maxTTL > maxTTLDefault {
+		httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{
+			Message: "Invalid create template request.",
+			Validations: []codersdk.ValidationError{
+				{Field: "max_ttl_ms", Detail: "Cannot be greater than " + maxTTLDefault.String()},
+			},
+		})
+		return
+	}
 
 	minAutostartInterval := minAutostartIntervalDefault
 	if !ptr.NilOrZero(createTemplate.MinAutostartIntervalMillis) {
@@ -384,6 +403,15 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
 	if req.MinAutostartIntervalMillis < 0 {
 		validErrs = append(validErrs, codersdk.ValidationError{Field: "min_autostart_interval_ms", Detail: "Must be a positive integer."})
 	}
+	if req.MaxTTLMillis > maxTTLDefault.Milliseconds() {
+		httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{
+			Message: "Invalid create template request.",
+			Validations: []codersdk.ValidationError{
+				{Field: "max_ttl_ms", Detail: "Cannot be greater than " + maxTTLDefault.String()},
+			},
+		})
+		return
+	}
 
 	if len(validErrs) > 0 {
 		httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{
@@ -433,9 +461,6 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
 		if icon == "" {
 			icon = template.Icon
 		}
-		if maxTTL == 0 {
-			maxTTL = time.Duration(template.MaxTtl)
-		}
 		if minAutostartInterval == 0 {
 			minAutostartInterval = time.Duration(template.MinAutostartInterval)
 		}

diff --git a/coderd/templates_test.go b/coderd/templates_test.go
@@ -108,6 +108,64 @@ func TestPostTemplateByOrganization(t *testing.T) {
 		require.Equal(t, http.StatusConflict, apiErr.StatusCode())
 	})
 
+	t.Run("MaxTTLTooLow", func(t *testing.T) {
+		t.Parallel()
+		client := coderdtest.New(t, nil)
+		user := coderdtest.CreateFirstUser(t, client)
+		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
+
+		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+		defer cancel()
+
+		_, err := client.CreateTemplate(ctx, user.OrganizationID, codersdk.CreateTemplateRequest{
+			Name:         "testing",
+			VersionID:    version.ID,
+			MaxTTLMillis: ptr.Ref(int64(-1)),
+		})
+		var apiErr *codersdk.Error
+		require.ErrorAs(t, err, &apiErr)
+		require.Equal(t, http.StatusBadRequest, apiErr.StatusCode())
+		require.Contains(t, err.Error(), "max_ttl_ms: Must be a positive integer")
+	})
+
+	t.Run("MaxTTLTooHigh", func(t *testing.T) {
+		t.Parallel()
+		client := coderdtest.New(t, nil)
+		user := coderdtest.CreateFirstUser(t, client)
+		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
+
+		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+		defer cancel()
+
+		_, err := client.CreateTemplate(ctx, user.OrganizationID, codersdk.CreateTemplateRequest{
+			Name:         "testing",
+			VersionID:    version.ID,
+			MaxTTLMillis: ptr.Ref(365 * 24 * time.Hour.Milliseconds()),
+		})
+		var apiErr *codersdk.Error
+		require.ErrorAs(t, err, &apiErr)
+		require.Equal(t, http.StatusBadRequest, apiErr.StatusCode())
+		require.Contains(t, err.Error(), "max_ttl_ms: Cannot be greater than")
+	})
+
+	t.Run("NoMaxTTL", func(t *testing.T) {
+		t.Parallel()
+		client := coderdtest.New(t, nil)
+		user := coderdtest.CreateFirstUser(t, client)
+		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
+
+		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+		defer cancel()
+
+		got, err := client.CreateTemplate(ctx, user.OrganizationID, codersdk.CreateTemplateRequest{
+			Name:         "testing",
+			VersionID:    version.ID,
+			MaxTTLMillis: ptr.Ref(int64(0)),
+		})
+		require.NoError(t, err)
+		require.Zero(t, got.MaxTTLMillis)
+	})
+
 	t.Run("Unauthorized", func(t *testing.T) {
 		t.Parallel()
 		client := coderdtest.New(t, nil)
@@ -271,6 +329,87 @@ func TestPatchTemplateMeta(t *testing.T) {
 		assert.Equal(t, req.MinAutostartIntervalMillis, updated.MinAutostartIntervalMillis)
 	})
 
+	t.Run("NoMaxTTL", func(t *testing.T) {
+		t.Parallel()
+
+		client := coderdtest.New(t, nil)
+		user := coderdtest.CreateFirstUser(t, client)
+		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
+		template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) {
+			ctr.MaxTTLMillis = ptr.Ref(24 * time.Hour.Milliseconds())
+		})
+		req := codersdk.UpdateTemplateMeta{
+			MaxTTLMillis: 0,
+		}
+
+		// We're too fast! Sleep so we can be sure that updatedAt is greater
+		time.Sleep(time.Millisecond * 5)
+
+		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+		defer cancel()
+
+		_, err := client.UpdateTemplateMeta(ctx, template.ID, req)
+		require.NoError(t, err)
+
+		// Extra paranoid: did it _really_ happen?
+		updated, err := client.Template(ctx, template.ID)
+		require.NoError(t, err)
+		assert.Greater(t, updated.UpdatedAt, template.UpdatedAt)
+		assert.Equal(t, req.MaxTTLMillis, updated.MaxTTLMillis)
+	})
+
+	t.Run("MaxTTLTooLow", func(t *testing.T) {
+		t.Parallel()
+
+		client := coderdtest.New(t, nil)
+		user := coderdtest.CreateFirstUser(t, client)
+		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
+		template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) {
+			ctr.MaxTTLMillis = ptr.Ref(24 * time.Hour.Milliseconds())
+		})
+		req := codersdk.UpdateTemplateMeta{
+			MaxTTLMillis: -1,
+		}
+
+		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+		defer cancel()
+
+		_, err := client.UpdateTemplateMeta(ctx, template.ID, req)
+		require.ErrorContains(t, err, "max_ttl_ms: Must be a positive integer")
+
+		// Ensure no update occurred
+		updated, err := client.Template(ctx, template.ID)
+		require.NoError(t, err)
+		assert.Equal(t, updated.UpdatedAt, template.UpdatedAt)
+		assert.Equal(t, updated.MaxTTLMillis, template.MaxTTLMillis)
+	})
+
+	t.Run("MaxTTLTooHigh", func(t *testing.T) {
+		t.Parallel()
+
+		client := coderdtest.New(t, nil)
+		user := coderdtest.CreateFirstUser(t, client)
+		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
+		template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) {
+			ctr.MaxTTLMillis = ptr.Ref(24 * time.Hour.Milliseconds())
+		})
+		req := codersdk.UpdateTemplateMeta{
+			MaxTTLMillis: 365 * 24 * time.Hour.Milliseconds(),
+		}
+
+		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+		defer cancel()
+
+		_, err := client.UpdateTemplateMeta(ctx, template.ID, req)
+		require.ErrorContains(t, err, "max_ttl_ms: Cannot be greater than")
+
+		// Ensure no update occurred
+		updated, err := client.Template(ctx, template.ID)
+		require.NoError(t, err)
+		assert.Equal(t, updated.UpdatedAt, template.UpdatedAt)
+		assert.Equal(t, updated.MaxTTLMillis, template.MaxTTLMillis)
+	})
+
 	t.Run("NotModified", func(t *testing.T) {
 		t.Parallel()
 

diff --git a/coderd/workspaces.go b/coderd/workspaces.go
@@ -32,8 +32,6 @@ import (
 	"github.com/coder/coder/codersdk"
 )
 
-const workspaceDefaultTTL = 12 * time.Hour
-
 var (
 	ttlMin = time.Minute //nolint:revive // min here means 'minimum' not 'minutes'
 	ttlMax = 7 * 24 * time.Hour
@@ -312,11 +310,6 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
 		return
 	}
 
-	if !dbTTL.Valid {
-		// Default to min(12 hours, template maximum). Just defaulting to template maximum can be surprising.
-		dbTTL = sql.NullInt64{Valid: true, Int64: min(template.MaxTtl, int64(workspaceDefaultTTL))}
-	}
-
 	workspace, err := api.Database.GetWorkspaceByOwnerIDAndName(r.Context(), database.GetWorkspaceByOwnerIDAndNameParams{
 		OwnerID: apiKey.UserID,
 		Name:    createWorkspace.Name,
@@ -923,7 +916,8 @@ func validWorkspaceTTLMillis(millis *int64, max time.Duration) (sql.NullInt64, e
 		return sql.NullInt64{}, errTTLMax
 	}
 
-	if truncated > max {
+	// template level
+	if max > 0 && truncated > max {
 		return sql.NullInt64{}, xerrors.Errorf("time until shutdown must be below template maximum %s", max.String())
 	}
 
@@ -1050,10 +1044,3 @@ func splitQueryParameterByDelimiter(query string, delimiter rune, maintainQuotes
 
 	return parts
 }
-
-func min(x, y int64) int64 {
-	if x < y {
-		return x
-	}
-	return y
-}
diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go
@@ -191,6 +191,26 @@ func TestPostWorkspacesByOrganization(t *testing.T) {
 		_ = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
 	})
 
+	t.Run("TemplateNoTTL", func(t *testing.T) {
+		t.Parallel()
+		client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
+		user := coderdtest.CreateFirstUser(t, client)
+		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
+		template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) {
+			ctr.MaxTTLMillis = ptr.Ref(int64(0))
+		})
+		// Given: the template has no max TTL set
+		require.Zero(t, template.MaxTTLMillis)
+		coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
+
+		// When: we create a workspace with autostop not enabled
+		workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
+			cwr.TTLMillis = ptr.Ref(int64(0))
+		})
+		// Then: No TTL should be set by the template
+		require.Nil(t, workspace.TTLMillis)
+	})
+
 	t.Run("TemplateCustomTTL", func(t *testing.T) {
 		t.Parallel()
 		client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
@@ -1051,6 +1071,17 @@ func TestWorkspaceUpdateTTL(t *testing.T) {
 			expectedError:  "ttl_ms: time until shutdown must be below template maximum 8h0m0s",
 			modifyTemplate: func(ctr *codersdk.CreateTemplateRequest) { ctr.MaxTTLMillis = ptr.Ref((8 * time.Hour).Milliseconds()) },
 		},
+		{
+			name:           "no template maximum ttl",
+			ttlMillis:      ptr.Ref((7 * 24 * time.Hour).Milliseconds()),
+			modifyTemplate: func(ctr *codersdk.CreateTemplateRequest) { ctr.MaxTTLMillis = ptr.Ref(int64(0)) },
+		},
+		{
+			name:           "above maximum ttl even with no template max",
+			ttlMillis:      ptr.Ref((365 * 24 * time.Hour).Milliseconds()),
+			expectedError:  "ttl_ms: time until shutdown must be less than 7 days",
+			modifyTemplate: func(ctr *codersdk.CreateTemplateRequest) { ctr.MaxTTLMillis = ptr.Ref(int64(0)) },
+		},
 	}
 
 	for _, testCase := range testCases {
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		-- Set a cap of 7 days on template max_ttl
		UPDATE templates SET max_ttl = 604800000000000 WHERE max_ttl > 604800000000000;