From ccbc30d48aacb616678e0991bff17aa311c5e13e Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 4 Apr 2023 12:28:04 +0200 Subject: [PATCH 1/2] Backend fixes --- coderd/workspacebuilds.go | 10 +- coderd/workspacebuilds_test.go | 184 +++++++++++++++++++++++++++++++++ 2 files changed, 190 insertions(+), 4 deletions(-) diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index 20d6bac5bea10..c6f2e591d5bdd 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -530,10 +530,12 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { // Check if parameter value is in request if buildParameter, found := findWorkspaceBuildParameter(createBuild.RichParameterValues, templateVersionParameter.Name); found { if !templateVersionParameter.Mutable { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: fmt.Sprintf("Parameter %q is not mutable, so it can't be updated after creating a workspace.", templateVersionParameter.Name), - }) - return + if _, found := findWorkspaceBuildParameter(apiLastBuildParameters, templateVersionParameter.Name); found { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: fmt.Sprintf("Parameter %q is not mutable, so it can't be updated after creating a workspace.", templateVersionParameter.Name), + }) + return + } } parameters = append(parameters, *buildParameter) continue diff --git a/coderd/workspacebuilds_test.go b/coderd/workspacebuilds_test.go index 79ff8e48b2c27..be91a94902b2a 100644 --- a/coderd/workspacebuilds_test.go +++ b/coderd/workspacebuilds_test.go @@ -781,6 +781,190 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) { }) require.Error(t, err) }) + + t.Run("NewImmutableRequiredParameterAdded", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, echoResponses) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) { + cwr.RichParameterValues = initialBuildParameters + }) + + workspaceBuild := coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + require.Equal(t, codersdk.WorkspaceStatusRunning, workspaceBuild.Status) + + // Push new template revision + const newImmutableParameterName = "new_immutable_parameter" + const newImmutableParameterDescription = "This is also an immutable parameter" + version2 := coderdtest.UpdateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionPlan: []*proto.Provision_Response{ + { + Type: &proto.Provision_Response_Complete{ + Complete: &proto.Provision_Complete{ + Parameters: []*proto.RichParameter{ + {Name: firstParameterName, Description: firstParameterDescription, Mutable: true}, + {Name: secondParameterName, Description: secondParameterDescription, Mutable: true}, + {Name: immutableParameterName, Description: immutableParameterDescription, Mutable: false}, + {Name: newImmutableParameterName, Description: newImmutableParameterDescription, Mutable: false, Required: true}, + }, + }, + }, + }, + }, + ProvisionApply: []*proto.Provision_Response{{ + Type: &proto.Provision_Response_Complete{ + Complete: &proto.Provision_Complete{}, + }, + }}, + }, template.ID) + coderdtest.AwaitTemplateVersionJob(t, client, version2.ID) + err := client.UpdateActiveTemplateVersion(context.Background(), template.ID, codersdk.UpdateActiveTemplateVersion{ + ID: version2.ID, + }) + require.NoError(t, err) + + // Update build parameters + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + nextBuildParameters := []codersdk.WorkspaceBuildParameter{ + {Name: newImmutableParameterName, Value: "good"}, + } + _, err = client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ + TemplateVersionID: version2.ID, + Transition: codersdk.WorkspaceTransitionStart, + RichParameterValues: nextBuildParameters, + }) + require.NoError(t, err) + }) + + t.Run("NewImmutableOptionalParameterAdded", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, echoResponses) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) { + cwr.RichParameterValues = initialBuildParameters + }) + + workspaceBuild := coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + require.Equal(t, codersdk.WorkspaceStatusRunning, workspaceBuild.Status) + + // Push new template revision + const newImmutableParameterName = "new_immutable_parameter" + const newImmutableParameterDescription = "This is also an immutable parameter" + version2 := coderdtest.UpdateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionPlan: []*proto.Provision_Response{ + { + Type: &proto.Provision_Response_Complete{ + Complete: &proto.Provision_Complete{ + Parameters: []*proto.RichParameter{ + {Name: firstParameterName, Description: firstParameterDescription, Mutable: true}, + {Name: secondParameterName, Description: secondParameterDescription, Mutable: true}, + {Name: immutableParameterName, Description: immutableParameterDescription, Mutable: false}, + {Name: newImmutableParameterName, Description: newImmutableParameterDescription, Mutable: false, DefaultValue: "12345"}, + }, + }, + }, + }, + }, + ProvisionApply: []*proto.Provision_Response{{ + Type: &proto.Provision_Response_Complete{ + Complete: &proto.Provision_Complete{}, + }, + }}, + }, template.ID) + coderdtest.AwaitTemplateVersionJob(t, client, version2.ID) + err := client.UpdateActiveTemplateVersion(context.Background(), template.ID, codersdk.UpdateActiveTemplateVersion{ + ID: version2.ID, + }) + require.NoError(t, err) + + // Update build parameters + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + nextBuildParameters := []codersdk.WorkspaceBuildParameter{ + {Name: newImmutableParameterName, Value: "good"}, + } + _, err = client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ + TemplateVersionID: version2.ID, + Transition: codersdk.WorkspaceTransitionStart, + RichParameterValues: nextBuildParameters, + }) + require.NoError(t, err) + }) + + t.Run("NewImmutableOptionalParameterUsesDefault", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, echoResponses) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) { + cwr.RichParameterValues = initialBuildParameters + }) + + workspaceBuild := coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + require.Equal(t, codersdk.WorkspaceStatusRunning, workspaceBuild.Status) + + // Push new template revision + const newImmutableParameterName = "new_immutable_parameter" + const newImmutableParameterDescription = "This is also an immutable parameter" + version2 := coderdtest.UpdateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionPlan: []*proto.Provision_Response{ + { + Type: &proto.Provision_Response_Complete{ + Complete: &proto.Provision_Complete{ + Parameters: []*proto.RichParameter{ + {Name: firstParameterName, Description: firstParameterDescription, Mutable: true}, + {Name: secondParameterName, Description: secondParameterDescription, Mutable: true}, + {Name: immutableParameterName, Description: immutableParameterDescription, Mutable: false}, + {Name: newImmutableParameterName, Description: newImmutableParameterDescription, Mutable: false, DefaultValue: "12345"}, + }, + }, + }, + }, + }, + ProvisionApply: []*proto.Provision_Response{{ + Type: &proto.Provision_Response_Complete{ + Complete: &proto.Provision_Complete{}, + }, + }}, + }, template.ID) + coderdtest.AwaitTemplateVersionJob(t, client, version2.ID) + err := client.UpdateActiveTemplateVersion(context.Background(), template.ID, codersdk.UpdateActiveTemplateVersion{ + ID: version2.ID, + }) + require.NoError(t, err) + + // Update build parameters + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + var nextBuildParameters []codersdk.WorkspaceBuildParameter + _, err = client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ + TemplateVersionID: version2.ID, + Transition: codersdk.WorkspaceTransitionStart, + RichParameterValues: nextBuildParameters, + }) + require.NoError(t, err) + }) } func TestWorkspaceBuildValidateRichParameters(t *testing.T) { From d18bb7f42da8cd6a35b01d02048f4883518a1f15 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 4 Apr 2023 12:41:14 +0200 Subject: [PATCH 2/2] CLI: adjust update flow --- cli/create.go | 29 +++++++++++++++++++++++++++-- cli/update.go | 4 +++- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/cli/create.go b/cli/create.go index 06901cf43d22e..a9238eaebb6c3 100644 --- a/cli/create.go +++ b/cli/create.go @@ -6,6 +6,7 @@ import ( "io" "time" + "github.com/google/uuid" "golang.org/x/exp/slices" "golang.org/x/xerrors" @@ -213,6 +214,7 @@ type prepWorkspaceBuildArgs struct { NewWorkspaceName string UpdateWorkspace bool + WorkspaceID uuid.UUID } type buildParameters struct { @@ -340,8 +342,17 @@ PromptRichParamLoop: } if args.UpdateWorkspace && !templateVersionParameter.Mutable { - _, _ = fmt.Fprintln(inv.Stdout, cliui.Styles.Warn.Render(fmt.Sprintf(`Parameter %q is not mutable, so can't be customized after workspace creation.`, templateVersionParameter.Name))) - continue + // Check if the immutable parameter was used in the previous build. If so, then it isn't a fresh one + // and the user should be warned. + exists, err := workspaceBuildParameterExists(ctx, client, args.WorkspaceID, templateVersionParameter) + if err != nil { + return nil, err + } + + if exists { + _, _ = fmt.Fprintln(inv.Stdout, cliui.Styles.Warn.Render(fmt.Sprintf(`Parameter %q is not mutable, so can't be customized after workspace creation.`, templateVersionParameter.Name))) + continue + } } parameterValue, err := getWorkspaceBuildParameterValueFromMapOrInput(inv, parameterMapFromFile, templateVersionParameter) @@ -414,3 +425,17 @@ PromptRichParamLoop: richParameters: richParameters, }, nil } + +func workspaceBuildParameterExists(ctx context.Context, client *codersdk.Client, workspaceID uuid.UUID, templateVersionParameter codersdk.TemplateVersionParameter) (bool, error) { + lastBuildParameters, err := client.WorkspaceBuildParameters(ctx, workspaceID) + if err != nil { + return false, xerrors.Errorf("can't fetch last workspace build parameters: %w", err) + } + + for _, p := range lastBuildParameters { + if p.Name == templateVersionParameter.Name { + return true, nil + } + } + return false, nil +} diff --git a/cli/update.go b/cli/update.go index 6f9a077fde1d3..4250715c387ae 100644 --- a/cli/update.go +++ b/cli/update.go @@ -59,7 +59,9 @@ func (r *RootCmd) update() *clibase.Cmd { ExistingRichParams: existingRichParams, RichParameterFile: richParameterFile, NewWorkspaceName: workspace.Name, - UpdateWorkspace: true, + + UpdateWorkspace: true, + WorkspaceID: workspace.LatestBuild.ID, }) if err != nil { return nil