Skip to content

fix: template: enforce bounds of template max_ttl #3662

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Aug 24, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
allow setting template max_ttl to 0
  • Loading branch information
johnstcn committed Aug 24, 2022
commit fc95dbede4e1bb540411a7c9ec48b6c7de55b5ba
4 changes: 0 additions & 4 deletions coderd/database/databasefake/databasefake.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion coderd/templates.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
18 changes: 18 additions & 0 deletions coderd/templates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
20 changes: 6 additions & 14 deletions coderd/workspaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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())
}

Expand Down Expand Up @@ -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
}
31 changes: 31 additions & 0 deletions coderd/workspaces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down Expand Up @@ -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 {
Expand Down