From 1b08ffea1cd3da6c5c7ffeaf9b0dffc1a919e04c Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 18 Aug 2023 13:44:04 +0200 Subject: [PATCH 1/2] fix(cli): do not ask for immutable parameters on start and restart --- cli/parameterresolver.go | 8 ++-- cli/restart_test.go | 75 +++++++++++++++++++++++++++++++++++ cli/start_test.go | 84 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 164 insertions(+), 3 deletions(-) diff --git a/cli/parameterresolver.go b/cli/parameterresolver.go index d2f551a17ff4f..e47e5c908bd53 100644 --- a/cli/parameterresolver.go +++ b/cli/parameterresolver.go @@ -177,14 +177,16 @@ func (pr *ParameterResolver) resolveWithInput(resolved []codersdk.WorkspaceBuild if p != nil { continue } + // Parameter has not been resolved yet, so CLI should determine if user should input it. firstTimeUse := pr.isFirstTimeUse(tvp.Name) if (tvp.Ephemeral && pr.promptBuildOptions) || - tvp.Required || + (action == WorkspaceCreate && tvp.Required) || + (action == WorkspaceCreate && !tvp.Ephemeral) || + (action == WorkspaceUpdate && tvp.Required) || (action == WorkspaceUpdate && !tvp.Mutable && firstTimeUse) || - (action == WorkspaceUpdate && tvp.Mutable && !tvp.Ephemeral && pr.promptRichParameters) || - (action == WorkspaceCreate && !tvp.Ephemeral) { + (action == WorkspaceUpdate && tvp.Mutable && !tvp.Ephemeral && pr.promptRichParameters) { parameterValue, err := cliui.RichParameter(inv, tvp) if err != nil { return nil, err diff --git a/cli/restart_test.go b/cli/restart_test.go index be2c6ea423416..db3dbf04fd703 100644 --- a/cli/restart_test.go +++ b/cli/restart_test.go @@ -181,3 +181,78 @@ func TestRestart(t *testing.T) { }) }) } + +func TestRestartWithParameters(t *testing.T) { + t.Parallel() + + echoResponses := &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionPlan: []*proto.Provision_Response{ + { + Type: &proto.Provision_Response_Complete{ + Complete: &proto.Provision_Complete{ + Parameters: []*proto.RichParameter{ + { + Name: immutableParameterName, + Description: immutableParameterDescription, + Required: true, + }, + }, + }, + }, + }, + }, + ProvisionApply: []*proto.Provision_Response{{ + Type: &proto.Provision_Response_Complete{ + Complete: &proto.Provision_Complete{}, + }, + }}, + } + + t.Run("DoNotAskForImmutables", func(t *testing.T) { + t.Parallel() + + // Create the workspace + 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 = []codersdk.WorkspaceBuildParameter{ + { + Name: immutableParameterName, + Value: immutableParameterValue, + }, + } + }) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + + // Restart the workspace again + inv, root := clitest.New(t, "restart", workspace.Name, "-y") + clitest.SetupConfig(t, client, root) + doneChan := make(chan struct{}) + pty := ptytest.New(t).Attach(inv) + go func() { + defer close(doneChan) + err := inv.Run() + assert.NoError(t, err) + }() + + pty.ExpectMatch("workspace has been restarted") + <-doneChan + + // Verify if immutable parameter is set + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + workspace, err := client.WorkspaceByOwnerAndName(ctx, workspace.OwnerName, workspace.Name, codersdk.WorkspaceOptions{}) + require.NoError(t, err) + actualParameters, err := client.WorkspaceBuildParameters(ctx, workspace.LatestBuild.ID) + require.NoError(t, err) + require.Contains(t, actualParameters, codersdk.WorkspaceBuildParameter{ + Name: immutableParameterName, + Value: immutableParameterValue, + }) + }) +} diff --git a/cli/start_test.go b/cli/start_test.go index b532500fb0777..570a684a70152 100644 --- a/cli/start_test.go +++ b/cli/start_test.go @@ -10,6 +10,7 @@ import ( "github.com/coder/coder/cli/clitest" "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/coderd/database" "github.com/coder/coder/codersdk" "github.com/coder/coder/provisioner/echo" "github.com/coder/coder/provisionersdk/proto" @@ -21,6 +22,10 @@ const ( ephemeralParameterName = "ephemeral_parameter" ephemeralParameterDescription = "This is ephemeral parameter" ephemeralParameterValue = "3" + + immutableParameterName = "immutable_parameter" + immutableParameterDescription = "This is immutable parameter" + immutableParameterValue = "abc" ) func TestStart(t *testing.T) { @@ -140,3 +145,82 @@ func TestStart(t *testing.T) { }) }) } + +func TestStartWithParameters(t *testing.T) { + t.Parallel() + + echoResponses := &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionPlan: []*proto.Provision_Response{ + { + Type: &proto.Provision_Response_Complete{ + Complete: &proto.Provision_Complete{ + Parameters: []*proto.RichParameter{ + { + Name: immutableParameterName, + Description: immutableParameterDescription, + Required: true, + }, + }, + }, + }, + }, + }, + ProvisionApply: []*proto.Provision_Response{{ + Type: &proto.Provision_Response_Complete{ + Complete: &proto.Provision_Complete{}, + }, + }}, + } + + t.Run("DoNotAskForImmutables", func(t *testing.T) { + t.Parallel() + + // Create the workspace + 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 = []codersdk.WorkspaceBuildParameter{ + { + Name: immutableParameterName, + Value: immutableParameterValue, + }, + } + }) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + + // Stop the workspace + workspaceBuild := coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStop) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspaceBuild.ID) + + // Start the workspace again + inv, root := clitest.New(t, "start", workspace.Name) + clitest.SetupConfig(t, client, root) + doneChan := make(chan struct{}) + pty := ptytest.New(t).Attach(inv) + go func() { + defer close(doneChan) + err := inv.Run() + assert.NoError(t, err) + }() + + pty.ExpectMatch("workspace has been started") + <-doneChan + + // Verify if immutable parameter is set + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + workspace, err := client.WorkspaceByOwnerAndName(ctx, workspace.OwnerName, workspace.Name, codersdk.WorkspaceOptions{}) + require.NoError(t, err) + actualParameters, err := client.WorkspaceBuildParameters(ctx, workspace.LatestBuild.ID) + require.NoError(t, err) + require.Contains(t, actualParameters, codersdk.WorkspaceBuildParameter{ + Name: immutableParameterName, + Value: immutableParameterValue, + }) + }) +} From 61a445bfce212ce57c8ca5b354cccc62d726f942 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 18 Aug 2023 13:45:27 +0200 Subject: [PATCH 2/2] rephrase --- cli/parameterresolver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/parameterresolver.go b/cli/parameterresolver.go index e47e5c908bd53..733f7e3da9e37 100644 --- a/cli/parameterresolver.go +++ b/cli/parameterresolver.go @@ -177,7 +177,7 @@ func (pr *ParameterResolver) resolveWithInput(resolved []codersdk.WorkspaceBuild if p != nil { continue } - // Parameter has not been resolved yet, so CLI should determine if user should input it. + // Parameter has not been resolved yet, so CLI needs to determine if user should input it. firstTimeUse := pr.isFirstTimeUse(tvp.Name)