Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
johnstcn merged 8 commits intomainfromcj/gh-3640/template-max-ttl-bounds
Aug 24, 2022
Merged
Show file tree
Hide file tree
Changes fromall commits
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
4 changes: 2 additions & 2 deletionscli/templateedit.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand Down
1 change: 1 addition & 0 deletionscoderd/authorize.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand Down
4 changes: 0 additions & 4 deletionscoderd/database/databasefake/databasefake.go
View file
Open in desktop
Original file line numberDiff line numberDiff 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
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
-- this is a no-op
View file
Open in desktop
Original file line numberDiff line numberDiff 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;
33 changes: 29 additions & 4 deletionscoderd/templates.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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) {
Expand DownExpand Up@@ -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{
Expand DownExpand Up@@ -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)
}
Expand Down
139 changes: 139 additions & 0 deletionscoderd/templates_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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)
defercancel()

_,err:=client.CreateTemplate(ctx,user.OrganizationID, codersdk.CreateTemplateRequest{
Name:"testing",
VersionID:version.ID,
MaxTTLMillis:ptr.Ref(int64(-1)),
})
varapiErr*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)
defercancel()

_,err:=client.CreateTemplate(ctx,user.OrganizationID, codersdk.CreateTemplateRequest{
Name:"testing",
VersionID:version.ID,
MaxTTLMillis:ptr.Ref(365*24*time.Hour.Milliseconds()),
})
varapiErr*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)
defercancel()

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 DownExpand Up@@ -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)
defercancel()

_,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)
defercancel()

_,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)
defercancel()

_,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()

Expand Down
17 changes: 2 additions & 15 deletionscoderd/workspaces.go
View file
Open in desktop
Original file line numberDiff line numberDiff 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 DownExpand Up@@ -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,
Expand DownExpand Up@@ -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())
}

Expand DownExpand Up@@ -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 changes: 31 additions & 0 deletionscoderd/workspaces_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff 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 DownExpand 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
[8]ページ先頭
©2009-2025 Movatter.jp