mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: fix template edit overriding with flag defaults (#11564)
This commit is contained in:
+64
-26
@@ -87,30 +87,6 @@ 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
|
||||
}
|
||||
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 unsetAutostopRequirementDaysOfWeek {
|
||||
autostopRequirementDaysOfWeek = []string{}
|
||||
}
|
||||
if failureTTL == 0 {
|
||||
failureTTL = time.Duration(template.FailureTTLMillis) * time.Millisecond
|
||||
}
|
||||
if dormancyThreshold == 0 {
|
||||
dormancyThreshold = time.Duration(template.TimeTilDormantMillis) * time.Millisecond
|
||||
}
|
||||
if dormancyAutoDeletion == 0 {
|
||||
dormancyAutoDeletion = time.Duration(template.TimeTilDormantAutoDeleteMillis) * time.Millisecond
|
||||
}
|
||||
|
||||
// Default values
|
||||
if !userSetOption(inv, "description") {
|
||||
description = template.Description
|
||||
@@ -124,11 +100,73 @@ func (r *RootCmd) templateEdit() *clibase.Cmd {
|
||||
displayName = template.DisplayName
|
||||
}
|
||||
|
||||
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 !userSetOption(inv, "dormancy-threshold") {
|
||||
dormancyThreshold = time.Duration(template.TimeTilDormantMillis) * time.Millisecond
|
||||
}
|
||||
|
||||
if !userSetOption(inv, "dormancy-auto-deletion") {
|
||||
dormancyAutoDeletion = time.Duration(template.TimeTilDormantAutoDeleteMillis) * time.Millisecond
|
||||
}
|
||||
|
||||
if !userSetOption(inv, "require-active-version") {
|
||||
requireActiveVersion = template.RequireActiveVersion
|
||||
}
|
||||
|
||||
if !userSetOption(inv, "autostop-requirement-weekdays") {
|
||||
autostopRequirementDaysOfWeek = template.AutostopRequirement.DaysOfWeek
|
||||
}
|
||||
|
||||
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)
|
||||
|
||||
@@ -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()),
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user