Skip to content

Commit e845dea

Browse files
mtojekmtojek
authored
fix: prompt when parameter options are incompatible (#9247)
1 parent 8bfa312 commit e845dea

File tree

15 files changed

+219
-62
lines changed

15 files changed

+219
-62
lines changed

‎cli/parameterresolver.go

+28
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,10 @@ next:
141141
continue // immutables should not be passed to consecutive builds
142142
}
143143

144+
if len(tvp.Options) > 0 && !isValidTemplateParameterOption(buildParameter, tvp.Options) {
145+
continue // do not propagate invalid options
146+
}
147+
144148
for i, r := range resolved {
145149
if r.Name == buildParameter.Name {
146150
resolved[i].Value = buildParameter.Value
@@ -180,10 +184,12 @@ func (pr *ParameterResolver) resolveWithInput(resolved []codersdk.WorkspaceBuild
180184
// Parameter has not been resolved yet, so CLI needs to determine if user should input it.
181185

182186
firstTimeUse := pr.isFirstTimeUse(tvp.Name)
187+
promptParameterOption := pr.isLastBuildParameterInvalidOption(tvp)
183188

184189
if (tvp.Ephemeral && pr.promptBuildOptions) ||
185190
(action == WorkspaceCreate && tvp.Required) ||
186191
(action == WorkspaceCreate && !tvp.Ephemeral) ||
192+
(action == WorkspaceUpdate && promptParameterOption) ||
187193
(action == WorkspaceUpdate && tvp.Mutable && tvp.Required) ||
188194
(action == WorkspaceUpdate && !tvp.Mutable && firstTimeUse) ||
189195
(action == WorkspaceUpdate && tvp.Mutable && !tvp.Ephemeral && pr.promptRichParameters) {
@@ -207,6 +213,19 @@ func (pr *ParameterResolver) isFirstTimeUse(parameterName string) bool {
207213
return findWorkspaceBuildParameter(parameterName, pr.lastBuildParameters) == nil
208214
}
209215

216+
func (pr *ParameterResolver) isLastBuildParameterInvalidOption(templateVersionParameter codersdk.TemplateVersionParameter) bool {
217+
if len(templateVersionParameter.Options) == 0 {
218+
return false
219+
}
220+
221+
for _, buildParameter := range pr.lastBuildParameters {
222+
if buildParameter.Name == templateVersionParameter.Name {
223+
return !isValidTemplateParameterOption(buildParameter, templateVersionParameter.Options)
224+
}
225+
}
226+
return false
227+
}
228+
210229
func findTemplateVersionParameter(workspaceBuildParameter codersdk.WorkspaceBuildParameter, templateVersionParameters []codersdk.TemplateVersionParameter) *codersdk.TemplateVersionParameter {
211230
for _, tvp := range templateVersionParameters {
212231
if tvp.Name == workspaceBuildParameter.Name {
@@ -224,3 +243,12 @@ func findWorkspaceBuildParameter(parameterName string, params []codersdk.Workspa
224243
}
225244
return nil
226245
}
246+
247+
func isValidTemplateParameterOption(buildParameter codersdk.WorkspaceBuildParameter, options []codersdk.TemplateVersionParameterOption) bool {
248+
for _, opt := range options {
249+
if opt.Value == buildParameter.Value {
250+
return true
251+
}
252+
}
253+
return false
254+
}

‎cli/root.go

+2
Original file line numberDiff line numberDiff line change
@@ -981,6 +981,8 @@ func (p *prettyErrorFormatter) format(err error) {
981981
msg = sdkError.Message
982982
if sdkError.Helper != "" {
983983
msg = msg + "\n" + sdkError.Helper
984+
} else if sdkError.Detail != "" {
985+
msg = msg + "\n" + sdkError.Detail
984986
}
985987
// The SDK error is usually good enough, and we don't want to overwhelm
986988
// the user with output.

‎cli/update_test.go

+126
Original file line numberDiff line numberDiff line change
@@ -593,12 +593,138 @@ func TestUpdateValidateRichParameters(t *testing.T) {
593593
assert.NoError(t, err)
594594
}()
595595

596+
pty.ExpectMatch("Planning workspace...")
597+
<-doneChan
598+
})
599+
600+
t.Run("ParameterOptionChanged", func(t *testing.T) {
601+
t.Parallel()
602+
603+
// Create template and workspace
604+
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
605+
user := coderdtest.CreateFirstUser(t, client)
606+
607+
templateParameters := []*proto.RichParameter{
608+
{Name: stringParameterName, Type: "string", Mutable: true, Required: true, Options: []*proto.RichParameterOption{
609+
{Name: "First option", Description: "This is first option", Value: "1st"},
610+
{Name: "Second option", Description: "This is second option", Value: "2nd"},
611+
{Name: "Third option", Description: "This is third option", Value: "3rd"},
612+
}},
613+
}
614+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, prepareEchoResponses(templateParameters))
615+
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
616+
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
617+
618+
inv, root := clitest.New(t, "create", "my-workspace", "--yes", "--template", template.Name, "--parameter", fmt.Sprintf("%s=%s", stringParameterName, "2nd"))
619+
clitest.SetupConfig(t, client, root)
620+
err := inv.Run()
621+
require.NoError(t, err)
622+
623+
// Update template
624+
updatedTemplateParameters := []*proto.RichParameter{
625+
{Name: stringParameterName, Type: "string", Mutable: true, Required: true, Options: []*proto.RichParameterOption{
626+
{Name: "first_option", Description: "This is first option", Value: "1"},
627+
{Name: "second_option", Description: "This is second option", Value: "2"},
628+
{Name: "third_option", Description: "This is third option", Value: "3"},
629+
}},
630+
}
631+
632+
updatedVersion := coderdtest.UpdateTemplateVersion(t, client, user.OrganizationID, prepareEchoResponses(updatedTemplateParameters), template.ID)
633+
coderdtest.AwaitTemplateVersionJob(t, client, updatedVersion.ID)
634+
err = client.UpdateActiveTemplateVersion(context.Background(), template.ID, codersdk.UpdateActiveTemplateVersion{
635+
ID: updatedVersion.ID,
636+
})
637+
require.NoError(t, err)
638+
639+
// Update the workspace
640+
inv, root = clitest.New(t, "update", "my-workspace")
641+
clitest.SetupConfig(t, client, root)
642+
doneChan := make(chan struct{})
643+
pty := ptytest.New(t).Attach(inv)
644+
go func() {
645+
defer close(doneChan)
646+
err := inv.Run()
647+
assert.NoError(t, err)
648+
}()
649+
596650
matches := []string{
651+
stringParameterName, "second_option",
597652
"Planning workspace...", "",
598653
}
599654
for i := 0; i < len(matches); i += 2 {
600655
match := matches[i]
656+
value := matches[i+1]
601657
pty.ExpectMatch(match)
658+
659+
if value != "" {
660+
pty.WriteLine(value)
661+
}
662+
}
663+
<-doneChan
664+
})
665+
666+
t.Run("ParameterOptionDisappeared", func(t *testing.T) {
667+
t.Parallel()
668+
669+
// Create template and workspace
670+
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
671+
user := coderdtest.CreateFirstUser(t, client)
672+
673+
templateParameters := []*proto.RichParameter{
674+
{Name: stringParameterName, Type: "string", Mutable: true, Required: true, Options: []*proto.RichParameterOption{
675+
{Name: "First option", Description: "This is first option", Value: "1st"},
676+
{Name: "Second option", Description: "This is second option", Value: "2nd"},
677+
{Name: "Third option", Description: "This is third option", Value: "3rd"},
678+
}},
679+
}
680+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, prepareEchoResponses(templateParameters))
681+
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
682+
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
683+
684+
inv, root := clitest.New(t, "create", "my-workspace", "--yes", "--template", template.Name, "--parameter", fmt.Sprintf("%s=%s", stringParameterName, "2nd"))
685+
clitest.SetupConfig(t, client, root)
686+
err := inv.Run()
687+
require.NoError(t, err)
688+
689+
// Update template - 2nd option disappeared, 4th option added
690+
updatedTemplateParameters := []*proto.RichParameter{
691+
{Name: stringParameterName, Type: "string", Mutable: true, Required: true, Options: []*proto.RichParameterOption{
692+
{Name: "First option", Description: "This is first option", Value: "1st"},
693+
{Name: "Third option", Description: "This is third option", Value: "3rd"},
694+
{Name: "Fourth option", Description: "This is fourth option", Value: "4th"},
695+
}},
696+
}
697+
698+
updatedVersion := coderdtest.UpdateTemplateVersion(t, client, user.OrganizationID, prepareEchoResponses(updatedTemplateParameters), template.ID)
699+
coderdtest.AwaitTemplateVersionJob(t, client, updatedVersion.ID)
700+
err = client.UpdateActiveTemplateVersion(context.Background(), template.ID, codersdk.UpdateActiveTemplateVersion{
701+
ID: updatedVersion.ID,
702+
})
703+
require.NoError(t, err)
704+
705+
// Update the workspace
706+
inv, root = clitest.New(t, "update", "my-workspace")
707+
clitest.SetupConfig(t, client, root)
708+
doneChan := make(chan struct{})
709+
pty := ptytest.New(t).Attach(inv)
710+
go func() {
711+
defer close(doneChan)
712+
err := inv.Run()
713+
assert.NoError(t, err)
714+
}()
715+
716+
matches := []string{
717+
stringParameterName, "Third option",
718+
"Planning workspace...", "",
719+
}
720+
for i := 0; i < len(matches); i += 2 {
721+
match := matches[i]
722+
value := matches[i+1]
723+
pty.ExpectMatch(match)
724+
725+
if value != "" {
726+
pty.WriteLine(value)
727+
}
602728
}
603729
<-doneChan
604730
})

‎coderd/apidoc/docs.go

-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

+3-6
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/wsbuilder/wsbuilder.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,7 @@ func (b *Builder) getParameters() (names, values []string, err error) {
525525
// At this point, we've queried all the data we need from the database,
526526
// so the only errors are problems with the request (missing data, failed
527527
// validation, immutable parameters, etc.)
528-
return nil, nil, BuildError{http.StatusBadRequest, err.Error(), err}
528+
return nil, nil, BuildError{http.StatusBadRequest, fmt.Sprintf("Unable to validate parameter %q", templateVersionParameter.Name), err}
529529
}
530530
names = append(names, templateVersionParameter.Name)
531531
values = append(values, value)

‎codersdk/provisionerdaemons.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ const (
6969
type JobErrorCode string
7070

7171
const (
72-
MissingTemplateParameter JobErrorCode = "MISSING_TEMPLATE_PARAMETER"
7372
RequiredTemplateVariables JobErrorCode = "REQUIRED_TEMPLATE_VARIABLES"
7473
)
7574

@@ -81,7 +80,7 @@ type ProvisionerJob struct {
8180
CompletedAt *time.Time `json:"completed_at,omitempty" format:"date-time"`
8281
CanceledAt *time.Time `json:"canceled_at,omitempty" format:"date-time"`
8382
Error string `json:"error,omitempty"`
84-
ErrorCode JobErrorCode `json:"error_code,omitempty" enums:"MISSING_TEMPLATE_PARAMETER,REQUIRED_TEMPLATE_VARIABLES"`
83+
ErrorCode JobErrorCode `json:"error_code,omitempty" enums:"REQUIRED_TEMPLATE_VARIABLES"`
8584
Status ProvisionerJobStatus `json:"status" enums:"pending,running,succeeded,canceling,canceled,failed"`
8685
WorkerID *uuid.UUID `json:"worker_id,omitempty" format:"uuid"`
8786
FileID uuid.UUID `json:"file_id" format:"uuid"`

‎docs/api/builds.md

+5-6
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎docs/api/schemas.md

+6-8
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)