allow setting template max_ttl to 0

johnstcn · johnstcn · commit fc95dbede4e1 · 2022-08-24T12:37:59.000Z
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/templates.go b/coderd/templates.go
@@ -173,7 +173,7 @@ 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 {
diff --git a/coderd/templates_test.go b/coderd/templates_test.go
@@ -148,6 +148,24 @@ func TestPostTemplateByOrganization(t *testing.T) {
 		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)
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,10 +310,10 @@ 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))}
-	}
+	// 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,
@@ -923,7 +921,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 +1049,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
`@@ -173,7 +173,7 @@ func (api API) postTemplateByOrganization(rw http.ResponseWriter, r http.Reque`
`173`	`173`	`}`
`174`	`174`
`175`	`175`	`maxTTL := maxTTLDefault`
`176`		`- if !ptr.NilOrZero(createTemplate.MaxTTLMillis) {`
	`176`	`+ if createTemplate.MaxTTLMillis != nil {`
`177`	`177`	`maxTTL = time.Duration(createTemplate.MaxTTLMillis) time.Millisecond`
`178`	`178`	`}`
`179`	`179`	`if maxTTL < 0 {`