Skip to content

Commit 2fad945

Browse files
SasSwartSasSwart
committed
fix: set preset parameters in the API instead of relying on the frontend
1 parent 19ec747 commit 2fad945

File tree

2 files changed

+256
-54
lines changed

2 files changed

+256
-54
lines changed

‎coderd/workspaces_test.go

+226-42
Original file line numberDiff line numberDiff line change
@@ -428,39 +428,66 @@ func TestWorkspace(t *testing.T) {
428428
t.Parallel()
429429

430430
testCases := []struct {
431-
name string
432-
presets []*proto.Preset
433-
expectedCount int
434-
selectedPresetIndex int // Index of the preset to use, or -1 if no preset should be used
431+
name string
432+
presets []*proto.Preset
433+
templateVersionParameters []*proto.RichParameter
434+
selectedPresetIndex int // -1 if no preset should be used
435435
}{
436436
{
437-
name: "No Presets",
438-
presets: []*proto.Preset{},
439-
expectedCount: 0,
440-
selectedPresetIndex: -1,
437+
name: "No Presets",
438+
presets: []*proto.Preset{},
439+
templateVersionParameters: []*proto.RichParameter{},
440+
selectedPresetIndex: -1,
441441
},
442442
{
443443
name: "Single Preset - No Parameters",
444+
presets: []*proto.Preset{{
445+
Name: "test",
446+
}},
447+
templateVersionParameters: []*proto.RichParameter{},
448+
selectedPresetIndex: 0,
449+
},
450+
{
451+
name: "Single Preset - With Parameters But No Template Parameters",
452+
presets: []*proto.Preset{{
453+
Name: "test",
454+
Parameters: []*proto.PresetParameter{
455+
{Name: "param1", Value: "value1"},
456+
{Name: "param2", Value: "value2"},
457+
},
458+
}},
459+
templateVersionParameters: []*proto.RichParameter{},
460+
selectedPresetIndex: 0,
461+
},
462+
{
463+
name: "Single Preset - With Matching Parameters",
444464
presets: []*proto.Preset{{
445465
Name: "test",
446466
Parameters: []*proto.PresetParameter{
447467
{Name: "param1", Value: "value1"},
448468
{Name: "param2", Value: "value2"},
449469
},
450470
}},
451-
expectedCount: 1,
471+
templateVersionParameters: []*proto.RichParameter{
472+
{Name: "param1", Type: "string", Required: true},
473+
{Name: "param2", Type: "string", Required: true},
474+
},
452475
selectedPresetIndex: 0,
453476
},
454477
{
455-
name: "Single Preset - With Parameters",
478+
name: "Single Preset - With Partial Matching Parameters",
456479
presets: []*proto.Preset{{
457480
Name: "test",
458481
Parameters: []*proto.PresetParameter{
459482
{Name: "param1", Value: "value1"},
460483
{Name: "param2", Value: "value2"},
484+
{Name: "param3", Value: "value3"}, // This parameter doesn't exist in template
461485
},
462486
}},
463-
expectedCount: 1,
487+
templateVersionParameters: []*proto.RichParameter{
488+
{Name: "param1", Type: "string", Required: false},
489+
{Name: "param2", Type: "string", Required: false},
490+
},
464491
selectedPresetIndex: 0,
465492
},
466493
{
@@ -470,8 +497,8 @@ func TestWorkspace(t *testing.T) {
470497
{Name: "test2"},
471498
{Name: "test3"},
472499
},
473-
expectedCount: 3,
474-
selectedPresetIndex: 0,
500+
templateVersionParameters: []*proto.RichParameter{},
501+
selectedPresetIndex: 0,
475502
},
476503
{
477504
name: "Multiple Presets - First Has Parameters",
@@ -486,7 +513,26 @@ func TestWorkspace(t *testing.T) {
486513
{Name: "test2"},
487514
{Name: "test3"},
488515
},
489-
expectedCount: 3,
516+
templateVersionParameters: []*proto.RichParameter{},
517+
selectedPresetIndex: 0,
518+
},
519+
{
520+
name: "Multiple Presets - First Has Matching Parameters",
521+
presets: []*proto.Preset{
522+
{
523+
Name: "test1",
524+
Parameters: []*proto.PresetParameter{
525+
{Name: "param1", Value: "value1"},
526+
{Name: "param2", Value: "value2"},
527+
},
528+
},
529+
{Name: "test2"},
530+
{Name: "test3"},
531+
},
532+
templateVersionParameters: []*proto.RichParameter{
533+
{Name: "param1", Type: "string", Required: true},
534+
{Name: "param2", Type: "string", Required: true},
535+
},
490536
selectedPresetIndex: 0,
491537
},
492538
{
@@ -502,7 +548,26 @@ func TestWorkspace(t *testing.T) {
502548
},
503549
{Name: "test3"},
504550
},
505-
expectedCount: 3,
551+
templateVersionParameters: []*proto.RichParameter{},
552+
selectedPresetIndex: 1,
553+
},
554+
{
555+
name: "Multiple Presets - Middle Has Matching Parameters",
556+
presets: []*proto.Preset{
557+
{Name: "test1"},
558+
{
559+
Name: "test2",
560+
Parameters: []*proto.PresetParameter{
561+
{Name: "param1", Value: "value1"},
562+
{Name: "param2", Value: "value2"},
563+
},
564+
},
565+
{Name: "test3"},
566+
},
567+
templateVersionParameters: []*proto.RichParameter{
568+
{Name: "param1", Type: "string", Required: true},
569+
{Name: "param2", Type: "string", Required: true},
570+
},
506571
selectedPresetIndex: 1,
507572
},
508573
{
@@ -518,7 +583,26 @@ func TestWorkspace(t *testing.T) {
518583
},
519584
},
520585
},
521-
expectedCount: 3,
586+
templateVersionParameters: []*proto.RichParameter{},
587+
selectedPresetIndex: 2,
588+
},
589+
{
590+
name: "Multiple Presets - Last Has Matching Parameters",
591+
presets: []*proto.Preset{
592+
{Name: "test1"},
593+
{Name: "test2"},
594+
{
595+
Name: "test3",
596+
Parameters: []*proto.PresetParameter{
597+
{Name: "param1", Value: "value1"},
598+
{Name: "param2", Value: "value2"},
599+
},
600+
},
601+
},
602+
templateVersionParameters: []*proto.RichParameter{
603+
{Name: "param1", Type: "string", Required: true},
604+
{Name: "param2", Type: "string", Required: true},
605+
},
522606
selectedPresetIndex: 2,
523607
},
524608
{
@@ -543,7 +627,36 @@ func TestWorkspace(t *testing.T) {
543627
},
544628
},
545629
},
546-
expectedCount: 3,
630+
templateVersionParameters: []*proto.RichParameter{},
631+
selectedPresetIndex: 1,
632+
},
633+
{
634+
name: "Multiple Presets - All Have Matching Parameters",
635+
presets: []*proto.Preset{
636+
{
637+
Name: "test1",
638+
Parameters: []*proto.PresetParameter{
639+
{Name: "param1", Value: "value1"},
640+
},
641+
},
642+
{
643+
Name: "test2",
644+
Parameters: []*proto.PresetParameter{
645+
{Name: "param2", Value: "value2"},
646+
},
647+
},
648+
{
649+
Name: "test3",
650+
Parameters: []*proto.PresetParameter{
651+
{Name: "param3", Value: "value3"},
652+
},
653+
},
654+
},
655+
templateVersionParameters: []*proto.RichParameter{
656+
{Name: "param1", Type: "string", Required: false},
657+
{Name: "param2", Type: "string", Required: true},
658+
{Name: "param3", Type: "string", Required: false},
659+
},
547660
selectedPresetIndex: 1,
548661
},
549662
{
@@ -562,7 +675,29 @@ func TestWorkspace(t *testing.T) {
562675
},
563676
},
564677
},
565-
expectedCount: 2,
678+
templateVersionParameters: []*proto.RichParameter{},
679+
selectedPresetIndex: -1,
680+
},
681+
{
682+
name: "Multiple Presets - With Matching Parameters But Not Used",
683+
presets: []*proto.Preset{
684+
{
685+
Name: "test1",
686+
Parameters: []*proto.PresetParameter{
687+
{Name: "param1", Value: "value1"},
688+
},
689+
},
690+
{
691+
Name: "test2",
692+
Parameters: []*proto.PresetParameter{
693+
{Name: "param2", Value: "value2"},
694+
},
695+
},
696+
},
697+
templateVersionParameters: []*proto.RichParameter{
698+
{Name: "param1", Type: "string", Required: false},
699+
{Name: "param2", Type: "string", Required: false},
700+
},
566701
selectedPresetIndex: -1,
567702
},
568703
}
@@ -576,11 +711,12 @@ func TestWorkspace(t *testing.T) {
576711
user := coderdtest.CreateFirstUser(t, client)
577712
authz := coderdtest.AssertRBAC(t, api, client)
578713

579-
// Create a plan response with the specified presets
714+
// Create a plan response with the specified presets and parameters
580715
planResponse := &proto.Response{
581716
Type: &proto.Response_Plan{
582717
Plan: &proto.PlanComplete{
583-
Presets: tc.presets,
718+
Presets: tc.presets,
719+
Parameters: tc.templateVersionParameters,
584720
},
585721
},
586722
}
@@ -598,9 +734,9 @@ func TestWorkspace(t *testing.T) {
598734
// Check presets
599735
presets, err := client.TemplateVersionPresets(ctx, version.ID)
600736
require.NoError(t, err)
601-
require.Equal(t, tc.expectedCount, len(presets))
737+
require.Equal(t, len(tc.presets), len(presets))
602738

603-
if tc.expectedCount > 0 {
739+
if len(tc.presets) > 0 {
604740
// Verify preset names and parameters
605741
for i, preset := range presets {
606742
require.Equal(t, tc.presets[i].Name, preset.Name)
@@ -626,15 +762,10 @@ func TestWorkspace(t *testing.T) {
626762

627763
// Create workspace with or without preset
628764
var workspace codersdk.Workspace
629-
if tc.selectedPresetIndex >= 0 && tc.expectedCount > 0 {
765+
if tc.selectedPresetIndex >= 0 {
630766
// Use the selected preset
631-
selectedIndex := tc.selectedPresetIndex
632-
if selectedIndex >= len(presets) {
633-
selectedIndex = 0 // Fallback to first preset if index is out of bounds
634-
}
635-
636767
workspace = coderdtest.CreateWorkspace(t, client, template.ID, func(request *codersdk.CreateWorkspaceRequest) {
637-
request.TemplateVersionPresetID = presets[selectedIndex].ID
768+
request.TemplateVersionPresetID = presets[tc.selectedPresetIndex].ID
638769
})
639770
} else {
640771
workspace = coderdtest.CreateWorkspace(t, client, template.ID)
@@ -649,23 +780,76 @@ func TestWorkspace(t *testing.T) {
649780
require.Equal(t, codersdk.BuildReasonInitiator, ws.LatestBuild.Reason)
650781

651782
// Check preset ID if expected
652-
if tc.selectedPresetIndex >= 0 && tc.expectedCount > 0 {
783+
if tc.selectedPresetIndex >= 0 {
653784
require.NotNil(t, ws.LatestBuild.TemplateVersionPresetID)
785+
require.Equal(t, presets[tc.selectedPresetIndex].ID, *ws.LatestBuild.TemplateVersionPresetID)
786+
787+
// If the selected preset has parameters and there are template version parameters,
788+
// verify that only matching parameters were applied
789+
if tc.presets[tc.selectedPresetIndex].Parameters != nil && len(tc.templateVersionParameters) > 0 {
790+
builds, err := client.WorkspaceBuilds(ctx, codersdk.WorkspaceBuildsRequest{
791+
WorkspaceID: ws.ID,
792+
})
793+
require.NoError(t, err)
794+
require.Equal(t, 1, len(builds))
795+
parameters, err := client.WorkspaceBuildParameters(ctx, builds[0].ID)
796+
require.NoError(t, err)
797+
parametersSetByPreset := 0
798+
for _, param := range parameters {
799+
for _, presetParam := range tc.presets[tc.selectedPresetIndex].Parameters {
800+
if param.Name == presetParam.Name && param.Value == presetParam.Value {
801+
parametersSetByPreset++
802+
break
803+
}
804+
}
805+
}
654806

655-
// Use the selected preset index
656-
selectedIndex := tc.selectedPresetIndex
657-
if selectedIndex >= len(presets) {
658-
selectedIndex = 0 // Fallback to first preset if index is out of bounds
659-
}
807+
// Track which template parameters were set by the preset
808+
expectedParamCount := 0
809+
for _, presetParam := range tc.presets[tc.selectedPresetIndex].Parameters {
810+
for _, templateParam := range tc.templateVersionParameters {
811+
if presetParam.Name == templateParam.Name {
812+
expectedParamCount++
813+
break
814+
}
815+
}
816+
}
660817

661-
require.Equal(t, presets[selectedIndex].ID, *ws.LatestBuild.TemplateVersionPresetID)
818+
// Verify that only the expected number of parameters were set
819+
require.Equal(t, expectedParamCount, parametersSetByPreset,
820+
"Expected %d parameters to be set, but found %d", expectedParamCount, parametersSetByPreset)
821+
822+
// Verify each parameter that should have been set
823+
for _, presetParam := range tc.presets[tc.selectedPresetIndex].Parameters {
824+
// Check if this parameter exists in the template version
825+
paramExists := false
826+
for _, templateParam := range tc.templateVersionParameters {
827+
if presetParam.Name == templateParam.Name {
828+
paramExists = true
829+
break
830+
}
831+
}
662832

663-
// If the selected preset has parameters, verify they were applied
664-
if tc.presets[selectedIndex].Parameters != nil {
665-
// This would require additional verification based on how parameters
666-
// are stored in the workspace. For now, we'll just log that we checked.
667-
t.Logf("Selected preset %s has parameters that should be applied to the workspace",
668-
tc.presets[selectedIndex].Name)
833+
if paramExists {
834+
// This parameter should have been set
835+
paramFound := false
836+
for _, appliedParam := range parameters {
837+
if appliedParam.Name == presetParam.Name {
838+
require.Equal(t, presetParam.Value, appliedParam.Value,
839+
"Parameter %s should have value %s", presetParam.Name, presetParam.Value)
840+
paramFound = true
841+
break
842+
}
843+
}
844+
require.True(t, paramFound, "Parameter %s should be applied to the workspace", presetParam.Name)
845+
} else {
846+
// This parameter should NOT have been set
847+
for _, appliedParam := range parameters {
848+
require.NotEqual(t, presetParam.Name, appliedParam.Name,
849+
"Parameter %s should not be applied to the workspace", presetParam.Name)
850+
}
851+
}
852+
}
669853
}
670854
} else {
671855
require.Nil(t, ws.LatestBuild.TemplateVersionPresetID)

0 commit comments

Comments
 (0)