fix(coderd): avoid fetching extra parameters for a preset

SasSwart · SasSwart · commit 4e57ac196d3b · 2025-02-20T06:32:43.000Z
diff --git a/coderd/presets.go b/coderd/presets.go
@@ -45,6 +45,10 @@ func (api *API) templateVersionPresets(rw http.ResponseWriter, r *http.Request)
 			Name: preset.Name,
 		}
 		for _, presetParam := range presetParams {
+			if presetParam.TemplateVersionPresetID != preset.ID {
+				continue
+			}
+
 			sdkPreset.Parameters = append(sdkPreset.Parameters, codersdk.PresetParameter{
 				Name:  presetParam.Name,
 				Value: presetParam.Value,
diff --git a/coderd/presets_test.go b/coderd/presets_test.go
@@ -17,58 +17,128 @@ import (
 func TestTemplateVersionPresets(t *testing.T) {
 	t.Parallel()
 
-	givenPreset := codersdk.Preset{
-		Name: "My Preset",
-		Parameters: []codersdk.PresetParameter{
-			{
-				Name:  "preset_param1",
-				Value: "A1B2C3",
+	testCases := []struct {
+		name    string
+		presets []codersdk.Preset
+	}{
+		{
+			name:    "no presets",
+			presets: []codersdk.Preset{},
+		},
+		{
+			name: "single preset with parameters",
+			presets: []codersdk.Preset{
+				{
+					Name: "My Preset",
+					Parameters: []codersdk.PresetParameter{
+						{
+							Name:  "preset_param1",
+							Value: "A1B2C3",
+						},
+						{
+							Name:  "preset_param2",
+							Value: "D4E5F6",
+						},
+					},
+				},
 			},
-			{
-				Name:  "preset_param2",
-				Value: "D4E5F6",
+		},
+		{
+			name: "multiple presets with overlapping parameters",
+			presets: []codersdk.Preset{
+				{
+					Name: "Preset 1",
+					Parameters: []codersdk.PresetParameter{
+						{
+							Name:  "shared_param",
+							Value: "value1",
+						},
+						{
+							Name:  "unique_param1",
+							Value: "unique1",
+						},
+					},
+				},
+				{
+					Name: "Preset 2",
+					Parameters: []codersdk.PresetParameter{
+						{
+							Name:  "shared_param",
+							Value: "value2",
+						},
+						{
+							Name:  "unique_param2",
+							Value: "unique2",
+						},
+					},
+				},
 			},
 		},
 	}
-	ctx := testutil.Context(t, testutil.WaitShort)
 
-	client, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
-	user := coderdtest.CreateFirstUser(t, client)
-	version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			ctx := testutil.Context(t, testutil.WaitShort)
 
-	// nolint:gocritic // This is a test
-	provisionerCtx := dbauthz.AsProvisionerd(ctx)
+			client, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
+			user := coderdtest.CreateFirstUser(t, client)
+			version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
 
-	dbPreset, err := db.InsertPreset(provisionerCtx, database.InsertPresetParams{
-		Name:              givenPreset.Name,
-		TemplateVersionID: version.ID,
-	})
-	require.NoError(t, err)
+			// nolint:gocritic // This is a test
+			provisionerCtx := dbauthz.AsProvisionerd(ctx)
 
-	var presetParameterNames []string
-	var presetParameterValues []string
-	for _, presetParameter := range givenPreset.Parameters {
-		presetParameterNames = append(presetParameterNames, presetParameter.Name)
-		presetParameterValues = append(presetParameterValues, presetParameter.Value)
-	}
-	_, err = db.InsertPresetParameters(provisionerCtx, database.InsertPresetParametersParams{
-		TemplateVersionPresetID: dbPreset.ID,
-		Names:                   presetParameterNames,
-		Values:                  presetParameterValues,
-	})
-	require.NoError(t, err)
+			// Insert all presets for this test case
+			for _, givenPreset := range tc.presets {
+				dbPreset, err := db.InsertPreset(provisionerCtx, database.InsertPresetParams{
+					Name:              givenPreset.Name,
+					TemplateVersionID: version.ID,
+				})
+				require.NoError(t, err)
+
+				if len(givenPreset.Parameters) > 0 {
+					var presetParameterNames []string
+					var presetParameterValues []string
+					for _, presetParameter := range givenPreset.Parameters {
+						presetParameterNames = append(presetParameterNames, presetParameter.Name)
+						presetParameterValues = append(presetParameterValues, presetParameter.Value)
+					}
+					_, err = db.InsertPresetParameters(provisionerCtx, database.InsertPresetParametersParams{
+						TemplateVersionPresetID: dbPreset.ID,
+						Names:                   presetParameterNames,
+						Values:                  presetParameterValues,
+					})
+					require.NoError(t, err)
+				}
+			}
+
+			userSubject, _, err := httpmw.UserRBACSubject(ctx, db, user.UserID, rbac.ScopeAll)
+			require.NoError(t, err)
+			userCtx := dbauthz.As(ctx, userSubject)
 
-	userSubject, _, err := httpmw.UserRBACSubject(ctx, db, user.UserID, rbac.ScopeAll)
-	require.NoError(t, err)
-	userCtx := dbauthz.As(ctx, userSubject)
+			gotPresets, err := client.TemplateVersionPresets(userCtx, version.ID)
+			require.NoError(t, err)
 
-	gotPresets, err := client.TemplateVersionPresets(userCtx, version.ID)
-	require.NoError(t, err)
+			require.Equal(t, len(tc.presets), len(gotPresets))
 
-	require.Equal(t, 1, len(gotPresets))
-	require.Equal(t, givenPreset.Name, gotPresets[0].Name)
+			for _, expectedPreset := range tc.presets {
+				found := false
+				for _, gotPreset := range gotPresets {
+					if gotPreset.Name == expectedPreset.Name {
+						found = true
 
-	for _, presetParameter := range givenPreset.Parameters {
-		require.Contains(t, gotPresets[0].Parameters, presetParameter)
+						// verify not only that we get the right number of parameters, but that we get the right parameters
+						// This ensures that we don't get extra parameters from other presets
+						require.Equal(t, len(expectedPreset.Parameters), len(gotPreset.Parameters))
+						for _, expectedParam := range expectedPreset.Parameters {
+							require.Contains(t, gotPreset.Parameters, expectedParam)
+						}
+						break
+					}
+				}
+				require.True(t, found, "Expected preset %s not found in results", expectedPreset.Name)
+			}
+		})
 	}
 }
