synced/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2025-04-10 07:50:06 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: template: enforce bounds of template max_ttl (#3662)

Browse Source 
This PR makes the following changes:

- enforces lower and upper limits on template `max_ttl_ms`
- adds a migration to enforce 7-day cap on `max_ttl`
- allows setting template `max_ttl` to 0
- updates template edit CLI help to be clearer
This commit is contained in:
Cian Johnston
2022-08-24 15:45:14 +01:00
committed by GitHub
parent 343d1184b2
commit eba753ba87
9 changed files with 207 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
cli/templateedit.go
View File

@ -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

1
coderd/authorize.go
View File

@ -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

4
coderd/database/databasefake/databasefake.go
View File

@ -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)
}

1
coderd/database/migrations/000038_template_max_ttl_cap_7_days.down.sql Normal file
View File

@ -0,0 +1 @@
-- this is a no-op

2
coderd/database/migrations/000038_template_max_ttl_cap_7_days.up.sql Normal file
View File

@ -0,0 +1,2 @@
-- Set a cap of 7 days on template max_ttl
UPDATE templates SET max_ttl = 604800000000000 WHERE max_ttl > 604800000000000;

33
coderd/templates.go
View File

@ -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)
}

139
coderd/templates_test.go
View File

@ -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()

17
coderd/workspaces.go
View File

@ -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
}

31
coderd/workspaces_test.go
View File

@ -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 {