address PR comments, actually enforce template level limits

coder · johnstcn · Jun 14, 2022 · Jun 14, 2022 · Jun 14, 2022 · Jun 14, 2022
commit 1708c212179e32d224555b66ccf1506d0138ab3e
diff --git a/cli/bump.go b/cli/bump.go
@@ -5,24 +5,29 @@ import (
 	"strings"
 	"time"
 
-	"github.com/coder/coder/coderd/util/tz"
-
 	"github.com/spf13/cobra"
 	"golang.org/x/xerrors"
 
+	"github.com/coder/coder/coderd/util/tz"
 	"github.com/coder/coder/codersdk"
 )
 
 const (
-	bumpDescriptionLong = `Specify a duration from now when you would like your workspace to shut down.`
+	bumpDescriptionShort = `Shut your workspace down after a given duration has passed.`
+	bumpDescriptionLong  = `Modify the time at which your workspace will shut down automatically.
+* Provide a duration from now (for example, 1h30m).
+* The minimum duration is 30 minutes.
+* If the workspace template restricts the maximum runtime of a workspace, this will be enforced here.
+* If the workspace does not already have a shutdown scheduled, this does nothing.
+`
 )
 
 func bump() *cobra.Command {
 	bumpCmd := &cobra.Command{
 		Args:        cobra.RangeArgs(1, 2),
 		Annotations: workspaceCommand,
 		Use:         "bump <workspace-name> <duration from now>",
-		Short:       "Specify a duration from now when you would like your workspace to shut down.",
+		Short:       bumpDescriptionShort,
 		Long:        bumpDescriptionLong,
 		Example:     "coder bump my-workspace 90m",
 		RunE: func(cmd *cobra.Command, args []string) error {
@@ -43,7 +48,7 @@ func bump() *cobra.Command {
 
 			loc, err := tz.TimezoneIANA()
 			if err != nil {
-				loc = time.UTC // best guess
+				loc = time.UTC // best effort
 			}
 
 			if bumpDuration < 29*time.Minute {

diff --git a/coderd/workspaces.go b/coderd/workspaces.go
@@ -575,21 +575,47 @@ func (api *API) putExtendWorkspace(rw http.ResponseWriter, r *http.Request) {
 	resp := httpapi.Response{}
 
 	err := api.Database.InTx(func(s database.Store) error {
+		template, err := s.GetTemplateByID(r.Context(), workspace.TemplateID)
+		if err != nil {
+			code = http.StatusInternalServerError
+			resp.Message = "Error fetching workspace template!"
+			return xerrors.Errorf("get workspace template: %w", err)
+		}
+
 		build, err := s.GetLatestWorkspaceBuildByWorkspaceID(r.Context(), workspace.ID)
 		if err != nil {
 			code = http.StatusInternalServerError
-			resp.Message = "Workspace not found."
+			resp.Message = "Error fetching workspace build."
 			return xerrors.Errorf("get latest workspace build: %w", err)
 		}
 
+		job, err := s.GetProvisionerJobByID(r.Context(), build.JobID)
+		if err != nil {
+			code = http.StatusInternalServerError
+			resp.Message = "Error fetching workspace provisioner job."
+			return xerrors.Errorf("get provisioner job: %w", err)
+		}
+
 		if build.Transition != database.WorkspaceTransitionStart {
 			code = http.StatusConflict
 			resp.Message = "Workspace must be started, current status: " + string(build.Transition)
 			return xerrors.Errorf("workspace must be started, current status: %s", build.Transition)
 		}
 
+		if !job.CompletedAt.Valid {
+			code = http.StatusConflict
+			resp.Message = "Workspace is still building!"
+			return xerrors.Errorf("workspace is still building")
+		}
+
+		if build.Deadline.IsZero() {
+			code = http.StatusConflict
+			resp.Message = "Workspace shutdown is manual."
+			return xerrors.Errorf("workspace shutdown is manual")
+		}
+
 		newDeadline := req.Deadline.UTC()
-		if err := validWorkspaceDeadline(build.Deadline, newDeadline); err != nil {
+		if err := validWorkspaceDeadline(job.CompletedAt.Time, newDeadline, time.Duration(template.MaxTtl)); err != nil {
 			code = http.StatusBadRequest
 			resp.Message = "Bad extend workspace request."
 			resp.Validations = append(resp.Validations, httpapi.Error{Field: "deadline", Detail: err.Error()})
@@ -878,16 +904,22 @@ func validWorkspaceTTLMillis(millis *int64, max time.Duration) (sql.NullInt64, e
 	}, nil
 }
 
-func validWorkspaceDeadline(old, new time.Time) error {
-	if old.IsZero() {
-		return xerrors.New("nothing to do: no existing deadline set")
-	}
-
+func validWorkspaceDeadline(startedAt, newDeadline time.Time, max time.Duration) error {
 	soon := time.Now().Add(29 * time.Minute)
-	if new.Before(soon) {
+	if newDeadline.Before(soon) {
 		return xerrors.New("new deadline must be at least 30 minutes in the future")
 	}
 
+	// No idea how this could happen.
+	if newDeadline.Before(startedAt) {
+		return xerrors.Errorf("new deadline must be before workspace start time")
+	}
+
+	delta := newDeadline.Sub(startedAt)
+	if delta > max {
+		return xerrors.New("new deadline is greater than template allows")
+	}
+
 	return nil
 }
 

diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go
@@ -981,8 +981,8 @@ func TestWorkspaceExtend(t *testing.T) {
 		user        = coderdtest.CreateFirstUser(t, client)
 		version     = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
 		_           = coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
-		project     = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
-		workspace   = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
+		template    = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
+		workspace   = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
 			cwr.TTLMillis = ptr.Ref(ttl.Milliseconds())
 		})
 		_ = coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
@@ -1017,6 +1017,13 @@ func TestWorkspaceExtend(t *testing.T) {
 	})
 	require.ErrorContains(t, err, "new deadline must be at least 30 minutes in the future", "setting a deadline less than 30 minutes in the future should fail")
 
+	// And with a deadline greater than the template max_ttl should also fail
+	deadlineExceedsMaxTTL := time.Now().Add(time.Duration(template.MaxTTLMillis) * time.Millisecond).Add(time.Minute)
+	err = client.PutExtendWorkspace(ctx, workspace.ID, codersdk.PutExtendWorkspaceRequest{
+		Deadline: deadlineExceedsMaxTTL,
+	})
+	require.ErrorContains(t, err, "new deadline is greater than template allows", "setting a deadline greater than that allowed by the template should fail")
+
 	// Updating with a deadline 30 minutes in the future should succeed
 	deadlineJustSoonEnough := time.Now().Add(30 * time.Minute)
 	err = client.PutExtendWorkspace(ctx, workspace.ID, codersdk.PutExtendWorkspaceRequest{