From 825b827809f679247c0be45b75e821fdd96e2a5d Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Wed, 18 Oct 2023 23:03:54 +0000 Subject: [PATCH 01/10] feat: add cli support for --require-active-version --- cli/templatecreate.go | 48 ++++++++++---- cli/templatecreate_test.go | 32 +++++++++ codersdk/deployment.go | 1 + enterprise/cli/templatecreate_test.go | 95 +++++++++++++++++++++++++++ 4 files changed, 165 insertions(+), 11 deletions(-) create mode 100644 enterprise/cli/templatecreate_test.go diff --git a/cli/templatecreate.go b/cli/templatecreate.go index b2e9a45cc8be8..3584220de6184 100644 --- a/cli/templatecreate.go +++ b/cli/templatecreate.go @@ -24,11 +24,12 @@ import ( func (r *RootCmd) templateCreate() *clibase.Cmd { var ( - provisioner string - provisionerTags []string - variablesFile string - variables []string - disableEveryone bool + provisioner string + provisionerTags []string + variablesFile string + variables []string + disableEveryone bool + requireActiveVersion bool defaultTTL time.Duration failureTTL time.Duration @@ -46,17 +47,35 @@ func (r *RootCmd) templateCreate() *clibase.Cmd { r.InitClient(client), ), Handler: func(inv *clibase.Invocation) error { - if failureTTL != 0 || inactivityTTL != 0 || maxTTL != 0 { + isTemplateSchedulingOptionsSet := failureTTL != 0 || inactivityTTL != 0 || maxTTL != 0 + + if isTemplateSchedulingOptionsSet || requireActiveVersion { entitlements, err := client.Entitlements(inv.Context()) - var sdkErr *codersdk.Error - if xerrors.As(err, &sdkErr) && sdkErr.StatusCode() == http.StatusNotFound { - return xerrors.Errorf("your deployment appears to be an AGPL deployment, so you cannot set --failure-ttl or --inactivityTTL") + if cerr, ok := codersdk.AsError(err); ok && cerr.StatusCode() == http.StatusNotFound { + return xerrors.Errorf("your deployment appears to be an AGPL deployment, so you cannot set enterprise-only flags") } else if err != nil { return xerrors.Errorf("get entitlements: %w", err) } - if !entitlements.Features[codersdk.FeatureAdvancedTemplateScheduling].Enabled { - return xerrors.Errorf("your license is not entitled to use advanced template scheduling, so you cannot set --failure-ttl or --inactivityTTL") + if isTemplateSchedulingOptionsSet { + if !entitlements.Features[codersdk.FeatureAdvancedTemplateScheduling].Enabled { + return xerrors.Errorf("your license is not entitled to use advanced template scheduling, so you cannot set --failure-ttl or --inactivityTTL") + } + } + + if requireActiveVersion { + experiments, exErr := client.Experiments(inv.Context()) + if exErr != nil { + return xerrors.Errorf("get experiments: %w", exErr) + } + + if !experiments.Enabled(codersdk.ExperimentTemplateUpdatePolicies) { + return xerrors.Errorf("--require-active-version is an experimental feature, pass 'template_update_policies' to the CODER_EXPERIMENTS env var to use this option") + } + + if !entitlements.Features[codersdk.FeatureAccessControl].Enabled { + return xerrors.Errorf("your license is not entitled to use template access control, so you cannot set --require-active-version") + } } } @@ -129,6 +148,7 @@ func (r *RootCmd) templateCreate() *clibase.Cmd { MaxTTLMillis: ptr.Ref(maxTTL.Milliseconds()), TimeTilDormantMillis: ptr.Ref(inactivityTTL.Milliseconds()), DisableEveryoneGroupAccess: disableEveryone, + RequireActiveVersion: requireActiveVersion, } _, err = client.CreateTemplate(inv.Context(), organization.ID, createReq) @@ -205,6 +225,12 @@ func (r *RootCmd) templateCreate() *clibase.Cmd { Value: clibase.StringOf(&provisioner), Hidden: true, }, + { + Flag: "require-active-version", + Description: "Requires workspace builds to use the active template version. This setting does not apply to template admins. This is an enterprise-only feature.", + Value: clibase.BoolOf(&requireActiveVersion), + }, + cliui.SkipPromptOption(), } cmd.Options = append(cmd.Options, uploadFlags.options()...) diff --git a/cli/templatecreate_test.go b/cli/templatecreate_test.go index ba5dad7b4ac6a..7ce576b405d36 100644 --- a/cli/templatecreate_test.go +++ b/cli/templatecreate_test.go @@ -13,6 +13,7 @@ import ( "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/codersdk" "github.com/coder/coder/v2/provisioner/echo" "github.com/coder/coder/v2/provisionersdk/proto" "github.com/coder/coder/v2/pty/ptytest" @@ -393,6 +394,37 @@ func TestTemplateCreate(t *testing.T) { } } }) + + t.Run("RequireActiveVersionInvalid", func(t *testing.T) { + t.Parallel() + + dv := coderdtest.DeploymentValues(t) + dv.Experiments = []string{ + string(codersdk.ExperimentTemplateUpdatePolicies), + } + + client := coderdtest.New(t, &coderdtest.Options{ + IncludeProvisionerDaemon: true, + DeploymentValues: dv, + }) + coderdtest.CreateFirstUser(t, client) + source := clitest.CreateTemplateVersionSource(t, completeWithAgent()) + args := []string{ + "templates", + "create", + "my-template", + "--directory", source, + "--test.provisioner", string(database.ProvisionerTypeEcho), + "--require-active-version", + } + inv, root := clitest.New(t, args...) + clitest.SetupConfig(t, client, root) + + err := inv.Run() + require.Error(t, err) + require.Contains(t, err.Error(), "your deployment appears to be an AGPL deployment, so you cannot set enterprise-only flags") + }) + } // Need this for Windows because of a known issue with Go: diff --git a/codersdk/deployment.go b/codersdk/deployment.go index bfcead815cf7c..7ad2cf527ca48 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -2002,6 +2002,7 @@ const ( // ExperimentDashboardTheme mutates the dashboard to use a new, dark color scheme. ExperimentDashboardTheme Experiment = "dashboard_theme" + ExperimentTemplateUpdatePolicies Experiment = "template_update_policies" // Add new experiments here! // ExperimentExample Experiment = "example" ) diff --git a/enterprise/cli/templatecreate_test.go b/enterprise/cli/templatecreate_test.go new file mode 100644 index 0000000000000..2c106398b3a81 --- /dev/null +++ b/enterprise/cli/templatecreate_test.go @@ -0,0 +1,95 @@ +package cli_test + +import ( + "testing" + + "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/codersdk" + "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" + "github.com/coder/coder/v2/enterprise/coderd/license" + "github.com/coder/coder/v2/provisioner/echo" + "github.com/coder/coder/v2/testutil" +) + +func TestTemplateCreate(t *testing.T) { + t.Parallel() + + t.Run("OK", func(t *testing.T) { + t.Parallel() + + dv := coderdtest.DeploymentValues(t) + dv.Experiments = []string{ + string(codersdk.ExperimentTemplateUpdatePolicies), + } + + client, user := coderdenttest.New(t, &coderdenttest.Options{ + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureAccessControl: 1, + }, + }, + Options: &coderdtest.Options{ + DeploymentValues: dv, + IncludeProvisionerDaemon: true, + }, + }) + + source := clitest.CreateTemplateVersionSource(t, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionApply: echo.ApplyComplete, + }) + + inv, conf := newCLI(t, "templates", + "create", "new", + "--directory", source, + "--test.provisioner", string(database.ProvisionerTypeEcho), + "--require-active-version", + "-y", + ) + + clitest.SetupConfig(t, client, conf) + + err := inv.Run() + require.NoError(t, err) + + ctx := testutil.Context(t, testutil.WaitMedium) + template, err := client.TemplateByName(ctx, user.OrganizationID, "new") + require.NoError(t, err) + require.True(t, template.RequireActiveVersion) + }) + + t.Run("NoEntitlement", func(t *testing.T) { + t.Parallel() + + dv := coderdtest.DeploymentValues(t) + dv.Experiments = []string{ + string(codersdk.ExperimentTemplateUpdatePolicies), + } + + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{}, + }, + Options: &coderdtest.Options{ + DeploymentValues: dv, + IncludeProvisionerDaemon: true, + }, + }) + + inv, conf := newCLI(t, "templates", + "create", "new", + "--require-active-version", + "-y", + ) + + clitest.SetupConfig(t, client, conf) + + err := inv.Run() + require.Error(t, err) + require.Contains(t, err.Error(), "your license is not entitled to use template access control, so you cannot set --require-active-version") + }) +} From 86442011c8e6beaee6c43904abebf5051e850131 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 19 Oct 2023 00:14:59 +0000 Subject: [PATCH 02/10] add support for template edit --- cli/templatecreate.go | 9 +-- cli/templateedit.go | 37 ++++++++-- cli/templateedit_test.go | 26 +++++++ enterprise/cli/templatecreate_test.go | 2 +- enterprise/cli/templateedit_test.go | 97 +++++++++++++++++++++++++++ 5 files changed, 161 insertions(+), 10 deletions(-) create mode 100644 enterprise/cli/templateedit_test.go diff --git a/cli/templatecreate.go b/cli/templatecreate.go index 3584220de6184..6eba71462acf9 100644 --- a/cli/templatecreate.go +++ b/cli/templatecreate.go @@ -64,6 +64,10 @@ func (r *RootCmd) templateCreate() *clibase.Cmd { } if requireActiveVersion { + if !entitlements.Features[codersdk.FeatureAccessControl].Enabled { + return xerrors.Errorf("your license is not entitled to use template access control, so you cannot set --require-active-version") + } + experiments, exErr := client.Experiments(inv.Context()) if exErr != nil { return xerrors.Errorf("get experiments: %w", exErr) @@ -72,10 +76,6 @@ func (r *RootCmd) templateCreate() *clibase.Cmd { if !experiments.Enabled(codersdk.ExperimentTemplateUpdatePolicies) { return xerrors.Errorf("--require-active-version is an experimental feature, pass 'template_update_policies' to the CODER_EXPERIMENTS env var to use this option") } - - if !entitlements.Features[codersdk.FeatureAccessControl].Enabled { - return xerrors.Errorf("your license is not entitled to use template access control, so you cannot set --require-active-version") - } } } @@ -229,6 +229,7 @@ func (r *RootCmd) templateCreate() *clibase.Cmd { Flag: "require-active-version", Description: "Requires workspace builds to use the active template version. This setting does not apply to template admins. This is an enterprise-only feature.", Value: clibase.BoolOf(&requireActiveVersion), + Default: "false", }, cliui.SkipPromptOption(), diff --git a/cli/templateedit.go b/cli/templateedit.go index ba079f99f7dd0..69fafd1cf07fe 100644 --- a/cli/templateedit.go +++ b/cli/templateedit.go @@ -31,6 +31,7 @@ func (r *RootCmd) templateEdit() *clibase.Cmd { allowUserCancelWorkspaceJobs bool allowUserAutostart bool allowUserAutostop bool + requireActiveVersion bool ) client := new(codersdk.Client) @@ -43,7 +44,7 @@ func (r *RootCmd) templateEdit() *clibase.Cmd { Short: "Edit the metadata of a template by name.", Handler: func(inv *clibase.Invocation) error { unsetAutostopRequirementDaysOfWeek := len(autostopRequirementDaysOfWeek) == 1 && autostopRequirementDaysOfWeek[0] == "none" - requiresEntitlement := (len(autostopRequirementDaysOfWeek) > 0 && !unsetAutostopRequirementDaysOfWeek) || + requiresScheduling := (len(autostopRequirementDaysOfWeek) > 0 && !unsetAutostopRequirementDaysOfWeek) || autostopRequirementWeeks > 0 || !allowUserAutostart || !allowUserAutostop || @@ -52,18 +53,37 @@ func (r *RootCmd) templateEdit() *clibase.Cmd { inactivityTTL != 0 || len(autostartRequirementDaysOfWeek) > 0 + requiresEntitlement := requiresScheduling || requireActiveVersion if requiresEntitlement { entitlements, err := client.Entitlements(inv.Context()) - var sdkErr *codersdk.Error - if xerrors.As(err, &sdkErr) && sdkErr.StatusCode() == http.StatusNotFound { - return xerrors.Errorf("your deployment appears to be an AGPL deployment, so you cannot set --max-ttl, --failure-ttl, --inactivityTTL, --allow-user-autostart=false or --allow-user-autostop=false") + if cerr, ok := codersdk.AsError(err); ok && cerr.StatusCode() == http.StatusNotFound { + return xerrors.Errorf("your deployment appears to be an AGPL deployment, so you cannot set enterprise-only flags") } else if err != nil { return xerrors.Errorf("get entitlements: %w", err) } - if !entitlements.Features[codersdk.FeatureAdvancedTemplateScheduling].Enabled { + if requiresScheduling && !entitlements.Features[codersdk.FeatureAdvancedTemplateScheduling].Enabled { return xerrors.Errorf("your license is not entitled to use advanced template scheduling, so you cannot set --max-ttl, --failure-ttl, --inactivityTTL, --allow-user-autostart=false or --allow-user-autostop=false") } + + if requireActiveVersion { + if !entitlements.Features[codersdk.FeatureAccessControl].Enabled { + return xerrors.Errorf("your license is not entitled to use template access control, so you cannot set --require-active-version") + } + + experiments, exErr := client.Experiments(inv.Context()) + if exErr != nil { + return xerrors.Errorf("get experiments: %w", exErr) + } + + if !experiments.Enabled(codersdk.ExperimentTemplateUpdatePolicies) { + return xerrors.Errorf("--require-active-version is an experimental feature, pass 'template_update_policies' to the CODER_EXPERIMENTS env var to use this option") + } + if !entitlements.Features[codersdk.FeatureAccessControl].Enabled { + return xerrors.Errorf("your license is not entitled to use template access control, so you cannot set --require-active-version") + + } + } } organization, err := CurrentOrganization(inv, client) @@ -110,6 +130,7 @@ func (r *RootCmd) templateEdit() *clibase.Cmd { AllowUserCancelWorkspaceJobs: allowUserCancelWorkspaceJobs, AllowUserAutostart: allowUserAutostart, AllowUserAutostop: allowUserAutostop, + RequireActiveVersion: requireActiveVersion, } _, err = client.UpdateTemplateMeta(inv.Context(), template.ID, req) @@ -222,6 +243,12 @@ func (r *RootCmd) templateEdit() *clibase.Cmd { Default: "true", Value: clibase.BoolOf(&allowUserAutostop), }, + { + Flag: "require-active-version", + Description: "Requires workspace builds to use the active template version. This setting does not apply to template admins. This is an enterprise-only feature.", + Value: clibase.BoolOf(&requireActiveVersion), + Default: "false", + }, cliui.SkipPromptOption(), } diff --git a/cli/templateedit_test.go b/cli/templateedit_test.go index cf286adacf427..de2e52894a444 100644 --- a/cli/templateedit_test.go +++ b/cli/templateedit_test.go @@ -1021,4 +1021,30 @@ func TestTemplateEdit(t *testing.T) { assert.Equal(t, template.TimeTilDormantMillis, updated.TimeTilDormantMillis) }) }) + + t.Run("RequireActiveVersion", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + owner := coderdtest.CreateFirstUser(t, client) + + version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, nil) + _ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) {}) + + // Test the cli command with --allow-user-autostart. + cmdArgs := []string{ + "templates", + "edit", + template.Name, + "--require-active-version", + } + inv, root := clitest.New(t, cmdArgs...) + //nolint + clitest.SetupConfig(t, client, root) + + ctx := testutil.Context(t, testutil.WaitLong) + err := inv.WithContext(ctx).Run() + require.Error(t, err) + require.ErrorContains(t, err, "appears to be an AGPL deployment") + }) } diff --git a/enterprise/cli/templatecreate_test.go b/enterprise/cli/templatecreate_test.go index 2c106398b3a81..368493e78a68c 100644 --- a/enterprise/cli/templatecreate_test.go +++ b/enterprise/cli/templatecreate_test.go @@ -62,7 +62,7 @@ func TestTemplateCreate(t *testing.T) { require.True(t, template.RequireActiveVersion) }) - t.Run("NoEntitlement", func(t *testing.T) { + t.Run("NotEntitled", func(t *testing.T) { t.Parallel() dv := coderdtest.DeploymentValues(t) diff --git a/enterprise/cli/templateedit_test.go b/enterprise/cli/templateedit_test.go new file mode 100644 index 0000000000000..45cdf69d350b5 --- /dev/null +++ b/enterprise/cli/templateedit_test.go @@ -0,0 +1,97 @@ +package cli_test + +import ( + "testing" + + "github.com/coder/coder/v2/cli/clitest" + "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/rbac" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" + "github.com/coder/coder/v2/enterprise/coderd/license" + "github.com/coder/coder/v2/testutil" + "github.com/stretchr/testify/require" +) + +func TestTemplateEdit(t *testing.T) { + t.Parallel() + + t.Run("OK", func(t *testing.T) { + t.Parallel() + + dv := coderdtest.DeploymentValues(t) + dv.Experiments = []string{ + string(codersdk.ExperimentTemplateUpdatePolicies), + } + + ownerClient, owner := coderdenttest.New(t, &coderdenttest.Options{ + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureAccessControl: 1, + }, + }, + Options: &coderdtest.Options{ + DeploymentValues: dv, + IncludeProvisionerDaemon: true, + }, + }) + + templateAdmin, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin()) + version := coderdtest.CreateTemplateVersion(t, templateAdmin, owner.OrganizationID, nil) + _ = coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdmin, version.ID) + template := coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, version.ID) + require.False(t, template.RequireActiveVersion) + + inv, conf := newCLI(t, "templates", + "edit", template.Name, + "--require-active-version", + "-y", + ) + + clitest.SetupConfig(t, templateAdmin, conf) + + err := inv.Run() + require.NoError(t, err) + + ctx := testutil.Context(t, testutil.WaitMedium) + template, err = templateAdmin.Template(ctx, template.ID) + require.NoError(t, err) + require.True(t, template.RequireActiveVersion) + }) + + t.Run("NotEntitled", func(t *testing.T) { + t.Parallel() + + dv := coderdtest.DeploymentValues(t) + dv.Experiments = []string{ + string(codersdk.ExperimentTemplateUpdatePolicies), + } + + client, owner := coderdenttest.New(t, &coderdenttest.Options{ + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{}, + }, + Options: &coderdtest.Options{ + DeploymentValues: dv, + IncludeProvisionerDaemon: true, + }, + }) + + version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, nil) + _ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID) + require.False(t, template.RequireActiveVersion) + + inv, conf := newCLI(t, "templates", + "edit", template.Name, + "--require-active-version", + "-y", + ) + + clitest.SetupConfig(t, client, conf) + + err := inv.Run() + require.Error(t, err) + require.Contains(t, err.Error(), "your license is not entitled to use template access control, so you cannot set --require-active-version") + }) +} From e9c7b2f1951c74660ef61a0b51b3b5df7ca2e81b Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 19 Oct 2023 00:16:08 +0000 Subject: [PATCH 03/10] make gen --- cli/templatecreate_test.go | 1 - cli/templateedit.go | 1 - coderd/apidoc/docs.go | 6 ++++-- coderd/apidoc/swagger.json | 6 ++++-- docs/api/schemas.md | 1 + docs/cli/templates_create.md | 9 +++++++++ docs/cli/templates_edit.md | 9 +++++++++ enterprise/cli/templateedit_test.go | 3 ++- site/src/api/typesGenerated.ts | 4 +++- 9 files changed, 32 insertions(+), 8 deletions(-) diff --git a/cli/templatecreate_test.go b/cli/templatecreate_test.go index 7ce576b405d36..ec1720ba2a6a4 100644 --- a/cli/templatecreate_test.go +++ b/cli/templatecreate_test.go @@ -424,7 +424,6 @@ func TestTemplateCreate(t *testing.T) { require.Error(t, err) require.Contains(t, err.Error(), "your deployment appears to be an AGPL deployment, so you cannot set enterprise-only flags") }) - } // Need this for Windows because of a known issue with Go: diff --git a/cli/templateedit.go b/cli/templateedit.go index 69fafd1cf07fe..97b02428fa2b5 100644 --- a/cli/templateedit.go +++ b/cli/templateedit.go @@ -81,7 +81,6 @@ func (r *RootCmd) templateEdit() *clibase.Cmd { } if !entitlements.Features[codersdk.FeatureAccessControl].Enabled { return xerrors.Errorf("your license is not entitled to use template access control, so you cannot set --require-active-version") - } } } diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 4151c62b40202..9d75c5385bb56 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -8541,7 +8541,8 @@ const docTemplate = `{ "single_tailnet", "template_autostop_requirement", "deployment_health_page", - "dashboard_theme" + "dashboard_theme", + "template_update_policies" ], "x-enum-varnames": [ "ExperimentMoons", @@ -8549,7 +8550,8 @@ const docTemplate = `{ "ExperimentSingleTailnet", "ExperimentTemplateAutostopRequirement", "ExperimentDeploymentHealthPage", - "ExperimentDashboardTheme" + "ExperimentDashboardTheme", + "ExperimentTemplateUpdatePolicies" ] }, "codersdk.ExternalAuth": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index df54a31a85a54..fc42019342e28 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -7653,7 +7653,8 @@ "single_tailnet", "template_autostop_requirement", "deployment_health_page", - "dashboard_theme" + "dashboard_theme", + "template_update_policies" ], "x-enum-varnames": [ "ExperimentMoons", @@ -7661,7 +7662,8 @@ "ExperimentSingleTailnet", "ExperimentTemplateAutostopRequirement", "ExperimentDeploymentHealthPage", - "ExperimentDashboardTheme" + "ExperimentDashboardTheme", + "ExperimentTemplateUpdatePolicies" ] }, "codersdk.ExternalAuth": { diff --git a/docs/api/schemas.md b/docs/api/schemas.md index da6715839647a..ff9aac1436700 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -2850,6 +2850,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in | `template_autostop_requirement` | | `deployment_health_page` | | `dashboard_theme` | +| `template_update_policies` | ## codersdk.ExternalAuth diff --git a/docs/cli/templates_create.md b/docs/cli/templates_create.md index 2811e4a1ce021..83c3b3c5b9aff 100644 --- a/docs/cli/templates_create.md +++ b/docs/cli/templates_create.md @@ -89,6 +89,15 @@ Disable the default behavior of granting template access to the 'everyone' group Specify a set of tags to target provisioner daemons. +### --require-active-version + +| | | +| ------- | ------------------ | +| Type | bool | +| Default | false | + +Requires workspace builds to use the active template version. This setting does not apply to template admins. This is an enterprise-only feature. + ### --var | | | diff --git a/docs/cli/templates_edit.md b/docs/cli/templates_edit.md index b58d1f61fc806..cd65ac99ef9d0 100644 --- a/docs/cli/templates_edit.md +++ b/docs/cli/templates_edit.md @@ -113,6 +113,15 @@ Edit the template maximum time before shutdown - workspaces created from this te Edit the template name. +### --require-active-version + +| | | +| ------- | ------------------ | +| Type | bool | +| Default | false | + +Requires workspace builds to use the active template version. This setting does not apply to template admins. This is an enterprise-only feature. + ### -y, --yes | | | diff --git a/enterprise/cli/templateedit_test.go b/enterprise/cli/templateedit_test.go index 45cdf69d350b5..67c1c1cde2c4c 100644 --- a/enterprise/cli/templateedit_test.go +++ b/enterprise/cli/templateedit_test.go @@ -3,6 +3,8 @@ package cli_test import ( "testing" + "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/rbac" @@ -10,7 +12,6 @@ import ( "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" "github.com/coder/coder/v2/enterprise/coderd/license" "github.com/coder/coder/v2/testutil" - "github.com/stretchr/testify/require" ) func TestTemplateEdit(t *testing.T) { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 805ab0d2bacf9..9209feea44176 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1706,7 +1706,8 @@ export type Experiment = | "moons" | "single_tailnet" | "tailnet_pg_coordinator" - | "template_autostop_requirement"; + | "template_autostop_requirement" + | "template_update_policies"; export const Experiments: Experiment[] = [ "dashboard_theme", "deployment_health_page", @@ -1714,6 +1715,7 @@ export const Experiments: Experiment[] = [ "single_tailnet", "tailnet_pg_coordinator", "template_autostop_requirement", + "template_update_policies", ]; // From codersdk/deployment.go From e0e28a264c902a016ab54929c980d7c817a60e2e Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 19 Oct 2023 00:45:53 +0000 Subject: [PATCH 04/10] golden files --- cli/testdata/coder_templates_create_--help.golden | 5 +++++ cli/testdata/coder_templates_edit_--help.golden | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/cli/testdata/coder_templates_create_--help.golden b/cli/testdata/coder_templates_create_--help.golden index 446c43f7e11ae..f458d3954dd62 100644 --- a/cli/testdata/coder_templates_create_--help.golden +++ b/cli/testdata/coder_templates_create_--help.golden @@ -49,6 +49,11 @@ OPTIONS: --provisioner-tag string-array Specify a set of tags to target provisioner daemons. + --require-active-version bool (default: false) + Requires workspace builds to use the active template version. This + setting does not apply to template admins. This is an enterprise-only + feature. + --var string-array Alias of --variable. diff --git a/cli/testdata/coder_templates_edit_--help.golden b/cli/testdata/coder_templates_edit_--help.golden index d86be791db616..fd5841125e708 100644 --- a/cli/testdata/coder_templates_edit_--help.golden +++ b/cli/testdata/coder_templates_edit_--help.golden @@ -59,6 +59,11 @@ OPTIONS: --name string Edit the template name. + --require-active-version bool (default: false) + Requires workspace builds to use the active template version. This + setting does not apply to template admins. This is an enterprise-only + feature. + -y, --yes bool Bypass prompts. From de17520c05ebe4e4514b635ccd68b41ed405e7d8 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 19 Oct 2023 05:10:34 +0000 Subject: [PATCH 05/10] add test for start --- cli/restart.go | 37 ++++++-- cli/start.go | 40 +++++++-- enterprise/cli/start_test.go | 165 +++++++++++++++++++++++++++++++++++ 3 files changed, 230 insertions(+), 12 deletions(-) create mode 100644 enterprise/cli/start_test.go diff --git a/cli/restart.go b/cli/restart.go index a936c30594878..668d1b51deee1 100644 --- a/cli/restart.go +++ b/cli/restart.go @@ -40,19 +40,44 @@ func (r *RootCmd) restart() *clibase.Cmd { return err } - template, err := client.Template(inv.Context(), workspace.TemplateID) + buildOptions, err := asWorkspaceBuildParameters(parameterFlags.buildOptions) if err != nil { - return err + return xerrors.Errorf("can't parse build options: %w", err) } - buildOptions, err := asWorkspaceBuildParameters(parameterFlags.buildOptions) + template, err := client.Template(inv.Context(), workspace.TemplateID) if err != nil { - return xerrors.Errorf("can't parse build options: %w", err) + return xerrors.Errorf("get template: %w", err) + } + + versionID := workspace.LatestBuild.TemplateVersionID + if template.RequireActiveVersion { + key := "template" + resp, err := client.AuthCheck(inv.Context(), codersdk.AuthorizationRequest{ + Checks: map[string]codersdk.AuthorizationCheck{ + key: { + Object: codersdk.AuthorizationObject{ + ResourceType: codersdk.ResourceTemplate, + OwnerID: workspace.OwnerID.String(), + OrganizationID: workspace.OrganizationID.String(), + ResourceID: template.ID.String(), + }, + Action: "update", + }, + }, + }) + if err != nil { + return xerrors.Errorf("auth check: %w", err) + } + // We don't have template admin privileges. + if !resp[key] { + versionID = template.ActiveVersionID + } } buildParameters, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ - Action: WorkspaceRestart, - Template: template, + Action: WorkspaceRestart, + TemplateVersionID: versionID, LastBuildParameters: lastBuildParameters, diff --git a/cli/start.go b/cli/start.go index 32f14985c7991..4104a1bef7712 100644 --- a/cli/start.go +++ b/cli/start.go @@ -6,6 +6,8 @@ import ( "golang.org/x/xerrors" + "github.com/google/uuid" + "github.com/coder/coder/v2/cli/clibase" "github.com/coder/coder/v2/cli/cliui" "github.com/coder/coder/v2/codersdk" @@ -37,7 +39,32 @@ func (r *RootCmd) start() *clibase.Cmd { template, err := client.Template(inv.Context(), workspace.TemplateID) if err != nil { - return err + return xerrors.Errorf("get template: %w", err) + } + + versionID := workspace.LatestBuild.TemplateVersionID + if template.RequireActiveVersion { + key := "template" + resp, err := client.AuthCheck(inv.Context(), codersdk.AuthorizationRequest{ + Checks: map[string]codersdk.AuthorizationCheck{ + key: { + Object: codersdk.AuthorizationObject{ + ResourceType: codersdk.ResourceTemplate, + OwnerID: workspace.OwnerID.String(), + OrganizationID: workspace.OrganizationID.String(), + ResourceID: template.ID.String(), + }, + Action: "update", + }, + }, + }) + if err != nil { + return xerrors.Errorf("auth check: %w", err) + } + // We don't have template admin privileges. + if !resp[key] { + versionID = template.ActiveVersionID + } } buildOptions, err := asWorkspaceBuildParameters(parameterFlags.buildOptions) @@ -46,8 +73,8 @@ func (r *RootCmd) start() *clibase.Cmd { } buildParameters, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ - Action: WorkspaceStart, - Template: template, + Action: WorkspaceStart, + TemplateVersionID: versionID, LastBuildParameters: lastBuildParameters, @@ -61,6 +88,7 @@ func (r *RootCmd) start() *clibase.Cmd { build, err := client.CreateWorkspaceBuild(inv.Context(), workspace.ID, codersdk.CreateWorkspaceBuildRequest{ Transition: codersdk.WorkspaceTransitionStart, RichParameterValues: buildParameters, + TemplateVersionID: versionID, }) if err != nil { return err @@ -82,8 +110,8 @@ func (r *RootCmd) start() *clibase.Cmd { } type prepStartWorkspaceArgs struct { - Action WorkspaceCLIAction - Template codersdk.Template + Action WorkspaceCLIAction + TemplateVersionID uuid.UUID LastBuildParameters []codersdk.WorkspaceBuildParameter @@ -94,7 +122,7 @@ type prepStartWorkspaceArgs struct { func prepStartWorkspace(inv *clibase.Invocation, client *codersdk.Client, args prepStartWorkspaceArgs) ([]codersdk.WorkspaceBuildParameter, error) { ctx := inv.Context() - templateVersion, err := client.TemplateVersion(ctx, args.Template.ActiveVersionID) + templateVersion, err := client.TemplateVersion(ctx, args.TemplateVersionID) if err != nil { return nil, xerrors.Errorf("get template version: %w", err) } diff --git a/enterprise/cli/start_test.go b/enterprise/cli/start_test.go new file mode 100644 index 0000000000000..b002a1390bb1e --- /dev/null +++ b/enterprise/cli/start_test.go @@ -0,0 +1,165 @@ +package cli_test + +import ( + "testing" + + "github.com/google/uuid" + "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/rbac" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" + "github.com/coder/coder/v2/enterprise/coderd/license" + "github.com/coder/coder/v2/testutil" +) + +func TestStart(t *testing.T) { + t.Parallel() + + t.Run("RequireActiveVersion", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + ownerClient, owner := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + IncludeProvisionerDaemon: true, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureAccessControl: 1, + codersdk.FeatureTemplateRBAC: 1, + codersdk.FeatureAdvancedTemplateScheduling: 1, + }, + }, + }) + + // Create an initial version. + oldVersion := coderdtest.CreateTemplateVersion(t, ownerClient, owner.OrganizationID, nil) + // Create a template that mandates the promoted version. + // This should be enforced for everyone except template admins. + template := coderdtest.CreateTemplate(t, ownerClient, owner.OrganizationID, oldVersion.ID) + coderdtest.AwaitTemplateVersionJobCompleted(t, ownerClient, oldVersion.ID) + require.Equal(t, oldVersion.ID, template.ActiveVersionID) + template, err := ownerClient.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ + RequireActiveVersion: true, + }) + require.NoError(t, err) + require.True(t, template.RequireActiveVersion) + + // Create a new version that we will promote. + activeVersion := coderdtest.CreateTemplateVersion(t, ownerClient, owner.OrganizationID, nil, func(ctvr *codersdk.CreateTemplateVersionRequest) { + ctvr.TemplateID = template.ID + }) + coderdtest.AwaitTemplateVersionJobCompleted(t, ownerClient, activeVersion.ID) + err = ownerClient.UpdateActiveTemplateVersion(ctx, template.ID, codersdk.UpdateActiveTemplateVersion{ + ID: activeVersion.ID, + }) + require.NoError(t, err) + err = ownerClient.UpdateActiveTemplateVersion(ctx, template.ID, codersdk.UpdateActiveTemplateVersion{ + ID: activeVersion.ID, + }) + require.NoError(t, err) + + templateAdminClient, templateAdmin := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin()) + templateACLAdminClient, templateACLAdmin := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID) + templateGroupACLAdminClient, templateGroupACLAdmin := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID) + memberClient, member := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID) + + // Create a group so we can also test group template admin ownership. + group, err := ownerClient.CreateGroup(ctx, owner.OrganizationID, codersdk.CreateGroupRequest{ + Name: "test", + }) + require.NoError(t, err) + + // Add the user who gains template admin via group membership. + group, err = ownerClient.PatchGroup(ctx, group.ID, codersdk.PatchGroupRequest{ + AddUsers: []string{templateGroupACLAdmin.ID.String()}, + }) + require.NoError(t, err) + + // Update the template for both users and groups. + err = ownerClient.UpdateTemplateACL(ctx, template.ID, codersdk.UpdateTemplateACL{ + UserPerms: map[string]codersdk.TemplateRole{ + templateACLAdmin.ID.String(): codersdk.TemplateRoleAdmin, + }, + GroupPerms: map[string]codersdk.TemplateRole{ + group.ID.String(): codersdk.TemplateRoleAdmin, + }, + }) + require.NoError(t, err) + + type testcase struct { + Name string + Client *codersdk.Client + WorkspaceOwner uuid.UUID + ExpectedVersion uuid.UUID + } + + cases := []testcase{ + { + Name: "OwnerUnchanged", + Client: ownerClient, + WorkspaceOwner: owner.UserID, + ExpectedVersion: oldVersion.ID, + }, + { + Name: "TemplateAdminUnchanged", + Client: templateAdminClient, + WorkspaceOwner: templateAdmin.ID, + ExpectedVersion: oldVersion.ID, + }, + { + Name: "TemplateACLAdminUnchanged", + Client: templateACLAdminClient, + WorkspaceOwner: templateACLAdmin.ID, + ExpectedVersion: oldVersion.ID, + }, + { + Name: "TemplateGroupACLAdminUnchanged", + Client: templateGroupACLAdminClient, + WorkspaceOwner: templateGroupACLAdmin.ID, + ExpectedVersion: oldVersion.ID, + }, + { + Name: "MemberUpdates", + Client: memberClient, + WorkspaceOwner: member.ID, + ExpectedVersion: activeVersion.ID, + }, + } + + for _, c := range cases { + t.Run(c.Name, func(t *testing.T) { + // Instantiate a new context for each subtest since + // they can potentially be lengthy. + ctx = testutil.Context(t, testutil.WaitMedium) + // Create the workspace using the admin since we want + // to force the old version. + ws, err := ownerClient.CreateWorkspace(ctx, owner.OrganizationID, c.WorkspaceOwner.String(), codersdk.CreateWorkspaceRequest{ + TemplateVersionID: oldVersion.ID, + Name: "abc123", + AutomaticUpdates: codersdk.AutomaticUpdatesNever, + }) + require.NoError(t, err) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, c.Client, ws.LatestBuild.ID) + // Stop the workspace so that we can start it. + coderdtest.MustTransitionWorkspace(t, c.Client, ws.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop, func(req *codersdk.CreateWorkspaceBuildRequest) { + req.TemplateVersionID = oldVersion.ID + }) + + // Start the workspace. Every test permutation should + // pass. + inv, conf := newCLI(t, "start", ws.Name) + clitest.SetupConfig(t, c.Client, conf) + err = inv.Run() + require.NoError(t, err) + + ws = coderdtest.MustWorkspace(t, c.Client, ws.ID) + require.Equal(t, c.ExpectedVersion, ws.LatestBuild.TemplateVersionID) + }) + } + }) +} From 202d29ab046d6a19e739e03c32aac052e2a0bf75 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 19 Oct 2023 05:26:01 +0000 Subject: [PATCH 06/10] add tests for start/restart --- cli/restart.go | 1 + coderd/coderdtest/coderdtest.go | 8 ++-- enterprise/cli/start_test.go | 68 +++++++++++++++++++-------------- 3 files changed, 45 insertions(+), 32 deletions(-) diff --git a/cli/restart.go b/cli/restart.go index 668d1b51deee1..351a60e82edd1 100644 --- a/cli/restart.go +++ b/cli/restart.go @@ -110,6 +110,7 @@ func (r *RootCmd) restart() *clibase.Cmd { build, err = client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ Transition: codersdk.WorkspaceTransitionStart, RichParameterValues: buildParameters, + TemplateVersionID: versionID, }) if err != nil { return err diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 6310a48d04e2e..85ceeba1be349 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -610,7 +610,7 @@ func CreateAnotherUserMutators(t testing.TB, client *codersdk.Client, organizati func createAnotherUserRetry(t testing.TB, client *codersdk.Client, organizationID uuid.UUID, retries int, roles []string, mutators ...func(r *codersdk.CreateUserRequest)) (*codersdk.Client, codersdk.User) { req := codersdk.CreateUserRequest{ Email: namesgenerator.GetRandomName(10) + "@coder.com", - Username: randomUsername(t), + Username: RandomUsername(t), Password: "SomeSecurePassword!", OrganizationID: organizationID, } @@ -744,7 +744,7 @@ func CreateWorkspaceBuild( // compatibility with testing. The name assigned is randomly generated. func CreateTemplate(t testing.TB, client *codersdk.Client, organization uuid.UUID, version uuid.UUID, mutators ...func(*codersdk.CreateTemplateRequest)) codersdk.Template { req := codersdk.CreateTemplateRequest{ - Name: randomUsername(t), + Name: RandomUsername(t), VersionID: version, } for _, mut := range mutators { @@ -906,7 +906,7 @@ func CreateWorkspace(t testing.TB, client *codersdk.Client, organization uuid.UU t.Helper() req := codersdk.CreateWorkspaceRequest{ TemplateID: templateID, - Name: randomUsername(t), + Name: RandomUsername(t), AutostartSchedule: ptr.Ref("CRON_TZ=US/Central 30 9 * * 1-5"), TTLMillis: ptr.Ref((8 * time.Hour).Milliseconds()), AutomaticUpdates: codersdk.AutomaticUpdatesNever, @@ -1170,7 +1170,7 @@ func NewAzureInstanceIdentity(t testing.TB, instanceID string) (x509.VerifyOptio } } -func randomUsername(t testing.TB) string { +func RandomUsername(t testing.TB) string { suffix, err := cryptorand.String(3) require.NoError(t, err) suffix = "-" + suffix diff --git a/enterprise/cli/start_test.go b/enterprise/cli/start_test.go index b002a1390bb1e..9b26e9f14d350 100644 --- a/enterprise/cli/start_test.go +++ b/enterprise/cli/start_test.go @@ -16,6 +16,7 @@ import ( "github.com/coder/coder/v2/testutil" ) +// TestStart also tests restart since the tests are virtually identical. func TestStart(t *testing.T) { t.Parallel() @@ -131,34 +132,45 @@ func TestStart(t *testing.T) { }, } - for _, c := range cases { - t.Run(c.Name, func(t *testing.T) { - // Instantiate a new context for each subtest since - // they can potentially be lengthy. - ctx = testutil.Context(t, testutil.WaitMedium) - // Create the workspace using the admin since we want - // to force the old version. - ws, err := ownerClient.CreateWorkspace(ctx, owner.OrganizationID, c.WorkspaceOwner.String(), codersdk.CreateWorkspaceRequest{ - TemplateVersionID: oldVersion.ID, - Name: "abc123", - AutomaticUpdates: codersdk.AutomaticUpdatesNever, - }) - require.NoError(t, err) - coderdtest.AwaitWorkspaceBuildJobCompleted(t, c.Client, ws.LatestBuild.ID) - // Stop the workspace so that we can start it. - coderdtest.MustTransitionWorkspace(t, c.Client, ws.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop, func(req *codersdk.CreateWorkspaceBuildRequest) { - req.TemplateVersionID = oldVersion.ID - }) - - // Start the workspace. Every test permutation should - // pass. - inv, conf := newCLI(t, "start", ws.Name) - clitest.SetupConfig(t, c.Client, conf) - err = inv.Run() - require.NoError(t, err) - - ws = coderdtest.MustWorkspace(t, c.Client, ws.ID) - require.Equal(t, c.ExpectedVersion, ws.LatestBuild.TemplateVersionID) + for _, cmd := range []string{"start", "restart"} { + cmd := cmd + t.Run(cmd, func(t *testing.T) { + t.Parallel() + for _, c := range cases { + c := c + t.Run(c.Name, func(t *testing.T) { + t.Parallel() + + // Instantiate a new context for each subtest since + // they can potentially be lengthy. + ctx = testutil.Context(t, testutil.WaitMedium) + // Create the workspace using the admin since we want + // to force the old version. + ws, err := ownerClient.CreateWorkspace(ctx, owner.OrganizationID, c.WorkspaceOwner.String(), codersdk.CreateWorkspaceRequest{ + TemplateVersionID: oldVersion.ID, + Name: coderdtest.RandomUsername(t), + AutomaticUpdates: codersdk.AutomaticUpdatesNever, + }) + require.NoError(t, err) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, c.Client, ws.LatestBuild.ID) + + if cmd == "start" { + // Stop the workspace so that we can start it. + coderdtest.MustTransitionWorkspace(t, c.Client, ws.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop, func(req *codersdk.CreateWorkspaceBuildRequest) { + req.TemplateVersionID = oldVersion.ID + }) + } + // Start the workspace. Every test permutation should + // pass. + inv, conf := newCLI(t, cmd, ws.Name, "-y") + clitest.SetupConfig(t, c.Client, conf) + err = inv.Run() + require.NoError(t, err) + + ws = coderdtest.MustWorkspace(t, c.Client, ws.ID) + require.Equal(t, c.ExpectedVersion, ws.LatestBuild.TemplateVersionID) + }) + } }) } }) From a67768f4a522eab2ef9ee0c9aa5a39830c0449d1 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 19 Oct 2023 06:11:40 +0000 Subject: [PATCH 07/10] fix race --- enterprise/cli/start_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enterprise/cli/start_test.go b/enterprise/cli/start_test.go index 9b26e9f14d350..665b67d4f829f 100644 --- a/enterprise/cli/start_test.go +++ b/enterprise/cli/start_test.go @@ -143,7 +143,7 @@ func TestStart(t *testing.T) { // Instantiate a new context for each subtest since // they can potentially be lengthy. - ctx = testutil.Context(t, testutil.WaitMedium) + ctx := testutil.Context(t, testutil.WaitMedium) // Create the workspace using the admin since we want // to force the old version. ws, err := ownerClient.CreateWorkspace(ctx, owner.OrganizationID, c.WorkspaceOwner.String(), codersdk.CreateWorkspaceRequest{ From 8e19b79d7235c303887689a6e470163c62fe1221 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 19 Oct 2023 07:43:10 +0000 Subject: [PATCH 08/10] pr comments --- cli/restart.go | 71 ++++++++++++++++++++++--------------------- cli/start.go | 70 +++++++++++++++++++++--------------------- cli/templatecreate.go | 2 +- cli/templateedit.go | 5 +-- 4 files changed, 73 insertions(+), 75 deletions(-) diff --git a/cli/restart.go b/cli/restart.go index 351a60e82edd1..1539fb122e3f0 100644 --- a/cli/restart.go +++ b/cli/restart.go @@ -2,6 +2,7 @@ package cli import ( "fmt" + "net/http" "time" "golang.org/x/xerrors" @@ -45,39 +46,9 @@ func (r *RootCmd) restart() *clibase.Cmd { return xerrors.Errorf("can't parse build options: %w", err) } - template, err := client.Template(inv.Context(), workspace.TemplateID) - if err != nil { - return xerrors.Errorf("get template: %w", err) - } - - versionID := workspace.LatestBuild.TemplateVersionID - if template.RequireActiveVersion { - key := "template" - resp, err := client.AuthCheck(inv.Context(), codersdk.AuthorizationRequest{ - Checks: map[string]codersdk.AuthorizationCheck{ - key: { - Object: codersdk.AuthorizationObject{ - ResourceType: codersdk.ResourceTemplate, - OwnerID: workspace.OwnerID.String(), - OrganizationID: workspace.OrganizationID.String(), - ResourceID: template.ID.String(), - }, - Action: "update", - }, - }, - }) - if err != nil { - return xerrors.Errorf("auth check: %w", err) - } - // We don't have template admin privileges. - if !resp[key] { - versionID = template.ActiveVersionID - } - } - buildParameters, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ Action: WorkspaceRestart, - TemplateVersionID: versionID, + TemplateVersionID: workspace.LatestBuild.TemplateVersionID, LastBuildParameters: lastBuildParameters, @@ -107,14 +78,44 @@ func (r *RootCmd) restart() *clibase.Cmd { return err } - build, err = client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ + req := codersdk.CreateWorkspaceBuildRequest{ Transition: codersdk.WorkspaceTransitionStart, RichParameterValues: buildParameters, - TemplateVersionID: versionID, - }) - if err != nil { + TemplateVersionID: workspace.LatestBuild.TemplateVersionID, + } + + build, err = client.CreateWorkspaceBuild(ctx, workspace.ID, req) + // It's possible for a workspace build to fail due to the template requiring starting + // workspaces with the active version. + if cerr, ok := codersdk.AsError(err); ok && cerr.StatusCode() == http.StatusUnauthorized { + template, err := client.Template(inv.Context(), workspace.TemplateID) + if err != nil { + return xerrors.Errorf("get template: %w", err) + } + + buildParameters, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ + Action: WorkspaceStart, + TemplateVersionID: template.ActiveVersionID, + + LastBuildParameters: lastBuildParameters, + + PromptBuildOptions: parameterFlags.promptBuildOptions, + BuildOptions: buildOptions, + }) + if err != nil { + return err + } + + req.RichParameterValues = buildParameters + req.TemplateVersionID = template.ActiveVersionID + build, err = client.CreateWorkspaceBuild(inv.Context(), workspace.ID, req) + if err != nil { + return err + } + } else if err != nil { return err } + err = cliui.WorkspaceBuild(ctx, out, client, build.ID) if err != nil { return err diff --git a/cli/start.go b/cli/start.go index 4104a1bef7712..2dba6b7414574 100644 --- a/cli/start.go +++ b/cli/start.go @@ -2,6 +2,7 @@ package cli import ( "fmt" + "net/http" "time" "golang.org/x/xerrors" @@ -37,36 +38,6 @@ func (r *RootCmd) start() *clibase.Cmd { return err } - template, err := client.Template(inv.Context(), workspace.TemplateID) - if err != nil { - return xerrors.Errorf("get template: %w", err) - } - - versionID := workspace.LatestBuild.TemplateVersionID - if template.RequireActiveVersion { - key := "template" - resp, err := client.AuthCheck(inv.Context(), codersdk.AuthorizationRequest{ - Checks: map[string]codersdk.AuthorizationCheck{ - key: { - Object: codersdk.AuthorizationObject{ - ResourceType: codersdk.ResourceTemplate, - OwnerID: workspace.OwnerID.String(), - OrganizationID: workspace.OrganizationID.String(), - ResourceID: template.ID.String(), - }, - Action: "update", - }, - }, - }) - if err != nil { - return xerrors.Errorf("auth check: %w", err) - } - // We don't have template admin privileges. - if !resp[key] { - versionID = template.ActiveVersionID - } - } - buildOptions, err := asWorkspaceBuildParameters(parameterFlags.buildOptions) if err != nil { return xerrors.Errorf("unable to parse build options: %w", err) @@ -74,7 +45,7 @@ func (r *RootCmd) start() *clibase.Cmd { buildParameters, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ Action: WorkspaceStart, - TemplateVersionID: versionID, + TemplateVersionID: workspace.LatestBuild.TemplateVersionID, LastBuildParameters: lastBuildParameters, @@ -85,12 +56,41 @@ func (r *RootCmd) start() *clibase.Cmd { return err } - build, err := client.CreateWorkspaceBuild(inv.Context(), workspace.ID, codersdk.CreateWorkspaceBuildRequest{ + req := codersdk.CreateWorkspaceBuildRequest{ Transition: codersdk.WorkspaceTransitionStart, RichParameterValues: buildParameters, - TemplateVersionID: versionID, - }) - if err != nil { + TemplateVersionID: workspace.LatestBuild.TemplateVersionID, + } + + build, err := client.CreateWorkspaceBuild(inv.Context(), workspace.ID, req) + // It's possible for a workspace build to fail due to the template requiring starting + // workspaces with the active version. + if cerr, ok := codersdk.AsError(err); ok && cerr.StatusCode() == http.StatusUnauthorized { + template, err := client.Template(inv.Context(), workspace.TemplateID) + if err != nil { + return xerrors.Errorf("get template: %w", err) + } + + buildParameters, err = prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ + Action: WorkspaceStart, + TemplateVersionID: template.ActiveVersionID, + + LastBuildParameters: lastBuildParameters, + + PromptBuildOptions: parameterFlags.promptBuildOptions, + BuildOptions: buildOptions, + }) + if err != nil { + return err + } + + req.RichParameterValues = buildParameters + req.TemplateVersionID = template.ActiveVersionID + build, err = client.CreateWorkspaceBuild(inv.Context(), workspace.ID, req) + if err != nil { + return err + } + } else if err != nil { return err } diff --git a/cli/templatecreate.go b/cli/templatecreate.go index 6eba71462acf9..15542de6ec923 100644 --- a/cli/templatecreate.go +++ b/cli/templatecreate.go @@ -74,7 +74,7 @@ func (r *RootCmd) templateCreate() *clibase.Cmd { } if !experiments.Enabled(codersdk.ExperimentTemplateUpdatePolicies) { - return xerrors.Errorf("--require-active-version is an experimental feature, pass 'template_update_policies' to the CODER_EXPERIMENTS env var to use this option") + return xerrors.Errorf("--require-active-version is an experimental feature, contact an administrator to enable the 'template_update_policies' experiment on your Coder server") } } } diff --git a/cli/templateedit.go b/cli/templateedit.go index 97b02428fa2b5..19dd0632524c5 100644 --- a/cli/templateedit.go +++ b/cli/templateedit.go @@ -77,10 +77,7 @@ func (r *RootCmd) templateEdit() *clibase.Cmd { } if !experiments.Enabled(codersdk.ExperimentTemplateUpdatePolicies) { - return xerrors.Errorf("--require-active-version is an experimental feature, pass 'template_update_policies' to the CODER_EXPERIMENTS env var to use this option") - } - if !entitlements.Features[codersdk.FeatureAccessControl].Enabled { - return xerrors.Errorf("your license is not entitled to use template access control, so you cannot set --require-active-version") + return xerrors.Errorf("--require-active-version is an experimental feature, contact an administrator to enable the 'template_update_policies' experiment on your Coder server") } } } From 9b96767af0e69673d2a2856345ff4f04858dfc5d Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 19 Oct 2023 21:15:02 +0000 Subject: [PATCH 09/10] pr comments --- cli/restart.go | 28 ++++-------------- cli/start.go | 68 +++++++++++++++++++++++++++++-------------- cli/templatecreate.go | 2 +- cli/templateedit.go | 2 +- 4 files changed, 54 insertions(+), 46 deletions(-) diff --git a/cli/restart.go b/cli/restart.go index 1539fb122e3f0..e5182ff481d1c 100644 --- a/cli/restart.go +++ b/cli/restart.go @@ -7,11 +7,10 @@ import ( "golang.org/x/xerrors" - "github.com/coder/pretty" - "github.com/coder/coder/v2/cli/clibase" "github.com/coder/coder/v2/cli/cliui" "github.com/coder/coder/v2/codersdk" + "github.com/coder/pretty" ) func (r *RootCmd) restart() *clibase.Cmd { @@ -88,29 +87,14 @@ func (r *RootCmd) restart() *clibase.Cmd { // It's possible for a workspace build to fail due to the template requiring starting // workspaces with the active version. if cerr, ok := codersdk.AsError(err); ok && cerr.StatusCode() == http.StatusUnauthorized { - template, err := client.Template(inv.Context(), workspace.TemplateID) - if err != nil { - return xerrors.Errorf("get template: %w", err) - } - - buildParameters, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ - Action: WorkspaceStart, - TemplateVersionID: template.ActiveVersionID, - + build, err = startWorkspaceActiveVersion(inv, client, startWorkspaceActiveVersionArgs{ + BuildOptions: buildOptions, LastBuildParameters: lastBuildParameters, - - PromptBuildOptions: parameterFlags.promptBuildOptions, - BuildOptions: buildOptions, + PromptBuildOptions: parameterFlags.promptBuildOptions, + Workspace: workspace, }) if err != nil { - return err - } - - req.RichParameterValues = buildParameters - req.TemplateVersionID = template.ActiveVersionID - build, err = client.CreateWorkspaceBuild(inv.Context(), workspace.ID, req) - if err != nil { - return err + return xerrors.Errorf("start workspace with active template version: %w", err) } } else if err != nil { return err diff --git a/cli/start.go b/cli/start.go index 2dba6b7414574..b74426570e75f 100644 --- a/cli/start.go +++ b/cli/start.go @@ -5,9 +5,8 @@ import ( "net/http" "time" - "golang.org/x/xerrors" - "github.com/google/uuid" + "golang.org/x/xerrors" "github.com/coder/coder/v2/cli/clibase" "github.com/coder/coder/v2/cli/cliui" @@ -66,29 +65,14 @@ func (r *RootCmd) start() *clibase.Cmd { // It's possible for a workspace build to fail due to the template requiring starting // workspaces with the active version. if cerr, ok := codersdk.AsError(err); ok && cerr.StatusCode() == http.StatusUnauthorized { - template, err := client.Template(inv.Context(), workspace.TemplateID) - if err != nil { - return xerrors.Errorf("get template: %w", err) - } - - buildParameters, err = prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ - Action: WorkspaceStart, - TemplateVersionID: template.ActiveVersionID, - + build, err = startWorkspaceActiveVersion(inv, client, startWorkspaceActiveVersionArgs{ + BuildOptions: buildOptions, LastBuildParameters: lastBuildParameters, - - PromptBuildOptions: parameterFlags.promptBuildOptions, - BuildOptions: buildOptions, + PromptBuildOptions: parameterFlags.promptBuildOptions, + Workspace: workspace, }) if err != nil { - return err - } - - req.RichParameterValues = buildParameters - req.TemplateVersionID = template.ActiveVersionID - build, err = client.CreateWorkspaceBuild(inv.Context(), workspace.ID, req) - if err != nil { - return err + return xerrors.Errorf("start workspace with active template version: %w", err) } } else if err != nil { return err @@ -138,3 +122,43 @@ func prepStartWorkspace(inv *clibase.Invocation, client *codersdk.Client, args p WithBuildOptions(args.BuildOptions) return resolver.Resolve(inv, args.Action, templateVersionParameters) } + +type startWorkspaceActiveVersionArgs struct { + BuildOptions []codersdk.WorkspaceBuildParameter + LastBuildParameters []codersdk.WorkspaceBuildParameter + PromptBuildOptions bool + Workspace codersdk.Workspace +} + +func startWorkspaceActiveVersion(inv *clibase.Invocation, client *codersdk.Client, args startWorkspaceActiveVersionArgs) (codersdk.WorkspaceBuild, error) { + _, _ = fmt.Fprintln(inv.Stdout, "Failed to restart with the template version from your last build. Policy may require you to restart with the current active template version.") + + template, err := client.Template(inv.Context(), args.Workspace.TemplateID) + if err != nil { + return codersdk.WorkspaceBuild{}, xerrors.Errorf("get template: %w", err) + } + + buildParameters, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ + Action: WorkspaceStart, + TemplateVersionID: template.ActiveVersionID, + + LastBuildParameters: args.LastBuildParameters, + + PromptBuildOptions: args.PromptBuildOptions, + BuildOptions: args.BuildOptions, + }) + if err != nil { + return codersdk.WorkspaceBuild{}, err + } + + build, err := client.CreateWorkspaceBuild(inv.Context(), args.Workspace.ID, codersdk.CreateWorkspaceBuildRequest{ + Transition: codersdk.WorkspaceTransitionStart, + RichParameterValues: buildParameters, + TemplateVersionID: template.ActiveVersionID, + }) + if err != nil { + return codersdk.WorkspaceBuild{}, err + } + + return build, nil +} diff --git a/cli/templatecreate.go b/cli/templatecreate.go index 15542de6ec923..13dd2de64e7f1 100644 --- a/cli/templatecreate.go +++ b/cli/templatecreate.go @@ -65,7 +65,7 @@ func (r *RootCmd) templateCreate() *clibase.Cmd { if requireActiveVersion { if !entitlements.Features[codersdk.FeatureAccessControl].Enabled { - return xerrors.Errorf("your license is not entitled to use template access control, so you cannot set --require-active-version") + return xerrors.Errorf("your license is not entitled to use enterprise access control, so you cannot set --require-active-version") } experiments, exErr := client.Experiments(inv.Context()) diff --git a/cli/templateedit.go b/cli/templateedit.go index 19dd0632524c5..5bd1cd236f7df 100644 --- a/cli/templateedit.go +++ b/cli/templateedit.go @@ -68,7 +68,7 @@ func (r *RootCmd) templateEdit() *clibase.Cmd { if requireActiveVersion { if !entitlements.Features[codersdk.FeatureAccessControl].Enabled { - return xerrors.Errorf("your license is not entitled to use template access control, so you cannot set --require-active-version") + return xerrors.Errorf("your license is not entitled to use enterprise access control, so you cannot set --require-active-version") } experiments, exErr := client.Experiments(inv.Context()) From 0ed711e726db884e5ea08e3637127e9c60bfdea8 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 19 Oct 2023 21:35:45 +0000 Subject: [PATCH 10/10] fix tests --- enterprise/cli/templatecreate_test.go | 2 +- enterprise/cli/templateedit_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/enterprise/cli/templatecreate_test.go b/enterprise/cli/templatecreate_test.go index 368493e78a68c..7fe699d531e20 100644 --- a/enterprise/cli/templatecreate_test.go +++ b/enterprise/cli/templatecreate_test.go @@ -90,6 +90,6 @@ func TestTemplateCreate(t *testing.T) { err := inv.Run() require.Error(t, err) - require.Contains(t, err.Error(), "your license is not entitled to use template access control, so you cannot set --require-active-version") + require.Contains(t, err.Error(), "your license is not entitled to use enterprise access control, so you cannot set --require-active-version") }) } diff --git a/enterprise/cli/templateedit_test.go b/enterprise/cli/templateedit_test.go index 67c1c1cde2c4c..5c26c19d820d7 100644 --- a/enterprise/cli/templateedit_test.go +++ b/enterprise/cli/templateedit_test.go @@ -93,6 +93,6 @@ func TestTemplateEdit(t *testing.T) { err := inv.Run() require.Error(t, err) - require.Contains(t, err.Error(), "your license is not entitled to use template access control, so you cannot set --require-active-version") + require.Contains(t, err.Error(), "your license is not entitled to use enterprise access control, so you cannot set --require-active-version") }) }