diff --git a/cli/templateedit.go b/cli/templateedit.go index 8c5ace25c5855..6df67f10101d8 100644 --- a/cli/templateedit.go +++ b/cli/templateedit.go @@ -87,48 +87,86 @@ func (r *RootCmd) templateEdit() *clibase.Cmd { return xerrors.Errorf("get workspace template: %w", err) } - // Copy the default value if the list is empty, or if the user - // specified the "none" value clear the list. - if len(autostopRequirementDaysOfWeek) == 0 { - autostopRequirementDaysOfWeek = template.AutostopRequirement.DaysOfWeek + // Default values + if !userSetOption(inv, "description") { + description = template.Description } - if len(autostartRequirementDaysOfWeek) == 1 && autostartRequirementDaysOfWeek[0] == "all" { - // Set it to every day of the week - autostartRequirementDaysOfWeek = []string{"monday", "tuesday", "wednesday", "thursday", "friday", "saturday", "sunday"} - } else if len(autostartRequirementDaysOfWeek) == 0 { - autostartRequirementDaysOfWeek = template.AutostartRequirement.DaysOfWeek + + if !userSetOption(inv, "icon") { + icon = template.Icon } - if unsetAutostopRequirementDaysOfWeek { - autostopRequirementDaysOfWeek = []string{} + + if !userSetOption(inv, "display-name") { + displayName = template.DisplayName } - if failureTTL == 0 { + + if !userSetOption(inv, "max-ttl") { + maxTTL = time.Duration(template.MaxTTLMillis) * time.Millisecond + } + + if !userSetOption(inv, "default-ttl") { + defaultTTL = time.Duration(template.DefaultTTLMillis) * time.Millisecond + } + + if !userSetOption(inv, "allow-user-autostop") { + allowUserAutostop = template.AllowUserAutostop + } + + if !userSetOption(inv, "allow-user-autostart") { + allowUserAutostart = template.AllowUserAutostart + } + + if !userSetOption(inv, "allow-user-cancel-workspace-jobs") { + allowUserCancelWorkspaceJobs = template.AllowUserCancelWorkspaceJobs + } + + if !userSetOption(inv, "failure-ttl") { failureTTL = time.Duration(template.FailureTTLMillis) * time.Millisecond } - if dormancyThreshold == 0 { + + if !userSetOption(inv, "dormancy-threshold") { dormancyThreshold = time.Duration(template.TimeTilDormantMillis) * time.Millisecond } - if dormancyAutoDeletion == 0 { + + if !userSetOption(inv, "dormancy-auto-deletion") { dormancyAutoDeletion = time.Duration(template.TimeTilDormantAutoDeleteMillis) * time.Millisecond } - // Default values - if !userSetOption(inv, "description") { - description = template.Description + if !userSetOption(inv, "require-active-version") { + requireActiveVersion = template.RequireActiveVersion } - if !userSetOption(inv, "icon") { - icon = template.Icon + if !userSetOption(inv, "autostop-requirement-weekdays") { + autostopRequirementDaysOfWeek = template.AutostopRequirement.DaysOfWeek } - if !userSetOption(inv, "display-name") { - displayName = template.DisplayName + if unsetAutostopRequirementDaysOfWeek { + autostopRequirementDaysOfWeek = []string{} + } + + if !userSetOption(inv, "autostop-requirement-weeks") { + autostopRequirementWeeks = template.AutostopRequirement.Weeks + } + + if len(autostartRequirementDaysOfWeek) == 1 && autostartRequirementDaysOfWeek[0] == "all" { + // Set it to every day of the week + autostartRequirementDaysOfWeek = []string{"monday", "tuesday", "wednesday", "thursday", "friday", "saturday", "sunday"} + } else if !userSetOption(inv, "autostart-requirement-weekdays") { + autostartRequirementDaysOfWeek = template.AutostartRequirement.DaysOfWeek + } else if len(autostartRequirementDaysOfWeek) == 0 { + autostartRequirementDaysOfWeek = []string{} } var deprecated *string - if !userSetOption(inv, "deprecated") { + if userSetOption(inv, "deprecated") { deprecated = &deprecationMessage } + var disableEveryoneGroup bool + if userSetOption(inv, "private") { + disableEveryoneGroup = disableEveryone + } + req := codersdk.UpdateTemplateMeta{ Name: name, DisplayName: displayName, @@ -151,7 +189,7 @@ func (r *RootCmd) templateEdit() *clibase.Cmd { AllowUserAutostop: allowUserAutostop, RequireActiveVersion: requireActiveVersion, DeprecationMessage: deprecated, - DisableEveryoneGroupAccess: disableEveryone, + DisableEveryoneGroupAccess: disableEveryoneGroup, } _, err = client.UpdateTemplateMeta(inv.Context(), template.ID, req) diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index 6df7befb0e37a..a4101151d2858 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -72,6 +72,9 @@ func Template(t testing.TB, db database.Store, seed database.Template) database. seed.OrganizationID.String(): []rbac.Action{rbac.ActionRead}, } } + if seed.UserACL == nil { + seed.UserACL = database.TemplateACL{} + } err := db.InsertTemplate(genCtx, database.InsertTemplateParams{ ID: id, CreatedAt: takeFirst(seed.CreatedAt, dbtime.Now()), diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 1ed0d1f73692c..3aaa0455a5be9 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -6374,6 +6374,7 @@ func (q *FakeQuerier) UpdateTemplateMetaByID(_ context.Context, arg database.Upd tpl.Description = arg.Description tpl.Icon = arg.Icon tpl.GroupACL = arg.GroupACL + tpl.AllowUserCancelWorkspaceJobs = arg.AllowUserCancelWorkspaceJobs q.templates[idx] = tpl return nil } diff --git a/coderd/externalauth/externalauth.go b/coderd/externalauth/externalauth.go index 64e8c9c5c21e1..6f78ade19fdca 100644 --- a/coderd/externalauth/externalauth.go +++ b/coderd/externalauth/externalauth.go @@ -325,7 +325,7 @@ func (c *DeviceAuth) AuthorizeDevice(ctx context.Context) (*codersdk.ExternalAut // return a better error. switch resp.StatusCode { case http.StatusTooManyRequests: - return nil, fmt.Errorf("rate limit hit, unable to authorize device. please try again later") + return nil, xerrors.New("rate limit hit, unable to authorize device. please try again later") default: return nil, err } diff --git a/coderd/promoauth/github.go b/coderd/promoauth/github.go index 7acbdb725592c..3f2a97d241b7f 100644 --- a/coderd/promoauth/github.go +++ b/coderd/promoauth/github.go @@ -1,10 +1,11 @@ package promoauth import ( - "fmt" "net/http" "strconv" "time" + + "golang.org/x/xerrors" ) type rateLimits struct { @@ -81,7 +82,7 @@ func (p *headerParser) string(key string) string { v := p.header.Get(key) if v == "" { - p.errors[key] = fmt.Errorf("missing header %q", key) + p.errors[key] = xerrors.Errorf("missing header %q", key) } return v } diff --git a/coderd/templates.go b/coderd/templates.go index d4c33a454ce16..78f918fe18180 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -669,7 +669,7 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { groupACL := template.GroupACL if req.DisableEveryoneGroupAccess { - groupACL = database.TemplateACL{} + delete(groupACL, template.OrganizationID.String()) } var err error diff --git a/enterprise/cli/templateedit_test.go b/enterprise/cli/templateedit_test.go index 75417196a6b8f..29575e5ab5046 100644 --- a/enterprise/cli/templateedit_test.go +++ b/enterprise/cli/templateedit_test.go @@ -4,11 +4,15 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/coder/coder/v2/cli/clitest" "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbfake" "github.com/coder/coder/v2/coderd/rbac" + "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" "github.com/coder/coder/v2/enterprise/coderd/license" @@ -105,7 +109,6 @@ func TestTemplateEdit(t *testing.T) { _ = coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdmin, version.ID) template := coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, version.ID) require.False(t, template.RequireActiveVersion) - const ( expectedFailureTTL = time.Hour * 3 expectedDormancyThreshold = time.Hour * 4 @@ -150,4 +153,168 @@ func TestTemplateEdit(t *testing.T) { require.Equal(t, expectedDormancyThreshold.Milliseconds(), template.TimeTilDormantMillis) require.Equal(t, expectedDormancyAutoDeletion.Milliseconds(), template.TimeTilDormantAutoDeleteMillis) }) + + // Test that omitting a flag does not update a template with the + // default for a flag. + t.Run("DefaultsDontOverride", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + ownerClient, db, owner := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureAdvancedTemplateScheduling: 1, + codersdk.FeatureAccessControl: 1, + codersdk.FeatureTemplateRBAC: 1, + }, + }, + }) + + dbtemplate := dbfake.TemplateVersion(t, db).Seed(database.TemplateVersion{ + CreatedBy: owner.UserID, + OrganizationID: owner.OrganizationID, + }).Do().Template + + var ( + expectedName = "template" + expectedDisplayName = "template_display" + expectedDescription = "My description" + expectedIcon = "icon.pjg" + expectedDefaultTTLMillis = time.Hour.Milliseconds() + expectedMaxTTLMillis = (time.Hour * 24).Milliseconds() + expectedAllowAutostart = false + expectedAllowAutostop = false + expectedFailureTTLMillis = time.Minute.Milliseconds() + expectedDormancyMillis = 2 * time.Minute.Milliseconds() + expectedAutoDeleteMillis = 3 * time.Minute.Milliseconds() + expectedRequireActiveVersion = true + expectedAllowCancelJobs = false + deprecationMessage = "Deprecate me" + expectedDisableEveryone = true + expectedAutostartDaysOfWeek = []string{} + expectedAutoStopDaysOfWeek = []string{} + expectedAutoStopWeeks = 1 + ) + + assertFieldsFn := func(t *testing.T, tpl codersdk.Template, acl codersdk.TemplateACL) { + t.Helper() + + assert.Equal(t, expectedName, tpl.Name) + assert.Equal(t, expectedDisplayName, tpl.DisplayName) + assert.Equal(t, expectedDescription, tpl.Description) + assert.Equal(t, expectedIcon, tpl.Icon) + assert.Equal(t, expectedDefaultTTLMillis, tpl.DefaultTTLMillis) + assert.Equal(t, expectedMaxTTLMillis, tpl.MaxTTLMillis) + assert.Equal(t, expectedAllowAutostart, tpl.AllowUserAutostart) + assert.Equal(t, expectedAllowAutostop, tpl.AllowUserAutostop) + assert.Equal(t, expectedFailureTTLMillis, tpl.FailureTTLMillis) + assert.Equal(t, expectedDormancyMillis, tpl.TimeTilDormantMillis) + assert.Equal(t, expectedAutoDeleteMillis, tpl.TimeTilDormantAutoDeleteMillis) + assert.Equal(t, expectedRequireActiveVersion, tpl.RequireActiveVersion) + assert.Equal(t, deprecationMessage, tpl.DeprecationMessage) + assert.Equal(t, expectedAllowCancelJobs, tpl.AllowUserCancelWorkspaceJobs) + assert.Equal(t, len(acl.Groups) == 0, expectedDisableEveryone) + assert.Equal(t, expectedAutostartDaysOfWeek, tpl.AutostartRequirement.DaysOfWeek) + assert.Equal(t, expectedAutoStopDaysOfWeek, tpl.AutostopRequirement.DaysOfWeek) + assert.Equal(t, int64(expectedAutoStopWeeks), tpl.AutostopRequirement.Weeks) + } + + template, err := ownerClient.UpdateTemplateMeta(ctx, dbtemplate.ID, codersdk.UpdateTemplateMeta{ + Name: expectedName, + DisplayName: expectedDisplayName, + Description: expectedDescription, + Icon: expectedIcon, + DefaultTTLMillis: expectedDefaultTTLMillis, + MaxTTLMillis: expectedMaxTTLMillis, + AllowUserAutostop: expectedAllowAutostop, + AllowUserAutostart: expectedAllowAutostart, + FailureTTLMillis: expectedFailureTTLMillis, + TimeTilDormantMillis: expectedDormancyMillis, + TimeTilDormantAutoDeleteMillis: expectedAutoDeleteMillis, + RequireActiveVersion: expectedRequireActiveVersion, + DeprecationMessage: ptr.Ref(deprecationMessage), + DisableEveryoneGroupAccess: expectedDisableEveryone, + AllowUserCancelWorkspaceJobs: expectedAllowCancelJobs, + AutostartRequirement: &codersdk.TemplateAutostartRequirement{ + DaysOfWeek: expectedAutostartDaysOfWeek, + }, + }) + require.NoError(t, err) + + templateACL, err := ownerClient.TemplateACL(ctx, template.ID) + require.NoError(t, err) + + assertFieldsFn(t, template, templateACL) + + expectedName = "newName" + inv, conf := newCLI(t, "templates", + "edit", template.Name, + "--name=newName", + "-y", + ) + + clitest.SetupConfig(t, ownerClient, conf) + + err = inv.Run() + require.NoError(t, err) + + template, err = ownerClient.Template(ctx, template.ID) + require.NoError(t, err) + templateACL, err = ownerClient.TemplateACL(ctx, template.ID) + require.NoError(t, err) + assertFieldsFn(t, template, templateACL) + + expectedAutostartDaysOfWeek = []string{"monday", "wednesday", "friday"} + expectedAutoStopDaysOfWeek = []string{"tuesday", "thursday"} + expectedAutoStopWeeks = 2 + expectedMaxTTLMillis = 0 + + template, err = ownerClient.UpdateTemplateMeta(ctx, dbtemplate.ID, codersdk.UpdateTemplateMeta{ + Name: expectedName, + DisplayName: expectedDisplayName, + Description: expectedDescription, + Icon: expectedIcon, + DefaultTTLMillis: expectedDefaultTTLMillis, + AllowUserAutostop: expectedAllowAutostop, + AllowUserAutostart: expectedAllowAutostart, + FailureTTLMillis: expectedFailureTTLMillis, + TimeTilDormantMillis: expectedDormancyMillis, + TimeTilDormantAutoDeleteMillis: expectedAutoDeleteMillis, + RequireActiveVersion: expectedRequireActiveVersion, + DeprecationMessage: ptr.Ref(deprecationMessage), + DisableEveryoneGroupAccess: expectedDisableEveryone, + AllowUserCancelWorkspaceJobs: expectedAllowCancelJobs, + AutostartRequirement: &codersdk.TemplateAutostartRequirement{ + DaysOfWeek: expectedAutostartDaysOfWeek, + }, + + AutostopRequirement: &codersdk.TemplateAutostopRequirement{ + DaysOfWeek: expectedAutoStopDaysOfWeek, + Weeks: int64(expectedAutoStopWeeks), + }, + }) + require.NoError(t, err) + assertFieldsFn(t, template, templateACL) + + // Rerun the update so we can assert that autostop days aren't + // mucked with. + expectedName = "newName2" + inv, conf = newCLI(t, "templates", + "edit", template.Name, + "--name=newName2", + "-y", + ) + + clitest.SetupConfig(t, ownerClient, conf) + + err = inv.Run() + require.NoError(t, err) + + template, err = ownerClient.Template(ctx, template.ID) + require.NoError(t, err) + + templateACL, err = ownerClient.TemplateACL(ctx, template.ID) + require.NoError(t, err) + assertFieldsFn(t, template, templateACL) + }) }