Skip to content

Commit 1c23184

Browse files
authored
fix(env): ensure BackendRuntimeConfig.Envs overrides base Envs (#341)
Signed-off-by: googs1025 <[email protected]>
1 parent 94c9d76 commit 1c23184

File tree

3 files changed

+117
-1
lines changed

3 files changed

+117
-1
lines changed

pkg/controller/inference/playground_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ func buildTemplate(models []*coreapi.OpenModel, playground *inferenceapi.Playgro
264264
// envs
265265
envs := parser.Envs()
266266
if playground.Spec.BackendRuntimeConfig != nil {
267-
envs = append(envs, playground.Spec.BackendRuntimeConfig.Envs...)
267+
envs = util.MergeEnvs(envs, playground.Spec.BackendRuntimeConfig.Envs)
268268
}
269269

270270
// args

pkg/util/util.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,27 @@ func MergeResources(toMerge corev1.ResourceList, toBeMerged corev1.ResourceList)
3838
return toMerge
3939
}
4040

41+
// MergeEnvs merges two env var list and ensures that entries in BackendRuntimeConfig.Env take precedence.
42+
// This function takes two slices of corev1.EnvVar: 'base' and 'overrides'. It returns a new slice of corev1.EnvVar
43+
// where the 'overrides' values overwrite any duplicate names in 'base'.
44+
func MergeEnvs(base []corev1.EnvVar, overrides []corev1.EnvVar) []corev1.EnvVar {
45+
envMap := make(map[string]corev1.EnvVar)
46+
47+
for _, env := range base {
48+
envMap[env.Name] = env
49+
}
50+
51+
for _, env := range overrides {
52+
envMap[env.Name] = env
53+
}
54+
55+
result := make([]corev1.EnvVar, 0, len(envMap))
56+
for _, env := range envMap {
57+
result = append(result, env)
58+
}
59+
return result
60+
}
61+
4162
// MergeKVs will merge kvs in toBeMerged to toMerge.
4263
// If kvs exist in toMerge, they will be replaced.
4364
func MergeKVs(toMerge map[string]string, toBeMerged map[string]string) map[string]string {

pkg/util/util_test.go

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,101 @@ func TestMergeResources(t *testing.T) {
7171
}
7272
}
7373

74+
func TestMergeEnvs(t *testing.T) {
75+
testCases := []struct {
76+
name string
77+
base []corev1.EnvVar
78+
overrides []corev1.EnvVar
79+
want []corev1.EnvVar
80+
}{
81+
{
82+
name: "overrides should overwrite base",
83+
base: []corev1.EnvVar{
84+
{Name: "VAR1", Value: "value1"},
85+
{Name: "VAR2", Value: "value2"},
86+
},
87+
overrides: []corev1.EnvVar{
88+
{Name: "VAR2", Value: "new_value2"},
89+
{Name: "VAR3", Value: "value3"},
90+
},
91+
want: []corev1.EnvVar{
92+
{Name: "VAR1", Value: "value1"},
93+
{Name: "VAR2", Value: "new_value2"},
94+
{Name: "VAR3", Value: "value3"},
95+
},
96+
},
97+
{
98+
name: "base has exclusive keys",
99+
base: []corev1.EnvVar{
100+
{Name: "VAR1", Value: "value1"},
101+
},
102+
overrides: []corev1.EnvVar{
103+
{Name: "VAR2", Value: "value2"},
104+
},
105+
want: []corev1.EnvVar{
106+
{Name: "VAR1", Value: "value1"},
107+
{Name: "VAR2", Value: "value2"},
108+
},
109+
},
110+
{
111+
name: "base is empty",
112+
base: []corev1.EnvVar{},
113+
overrides: []corev1.EnvVar{
114+
{Name: "VAR1", Value: "value1"},
115+
{Name: "VAR2", Value: "value2"},
116+
},
117+
want: []corev1.EnvVar{
118+
{Name: "VAR1", Value: "value1"},
119+
{Name: "VAR2", Value: "value2"},
120+
},
121+
},
122+
{
123+
name: "overrides is empty",
124+
base: []corev1.EnvVar{
125+
{Name: "VAR1", Value: "value1"},
126+
{Name: "VAR2", Value: "value2"},
127+
},
128+
overrides: []corev1.EnvVar{},
129+
want: []corev1.EnvVar{
130+
{Name: "VAR1", Value: "value1"},
131+
{Name: "VAR2", Value: "value2"},
132+
},
133+
},
134+
{
135+
name: "both base and overrides are empty",
136+
base: []corev1.EnvVar{},
137+
overrides: []corev1.EnvVar{},
138+
want: []corev1.EnvVar{},
139+
},
140+
{
141+
name: "both base and overrides are nil",
142+
base: nil,
143+
overrides: nil,
144+
want: []corev1.EnvVar{},
145+
},
146+
}
147+
148+
for _, tc := range testCases {
149+
t.Run(tc.name, func(t *testing.T) {
150+
got := MergeEnvs(tc.base, tc.overrides)
151+
152+
gotMap := make(map[string]string)
153+
for _, env := range got {
154+
gotMap[env.Name] = env.Value
155+
}
156+
157+
wantMap := make(map[string]string)
158+
for _, env := range tc.want {
159+
wantMap[env.Name] = env.Value
160+
}
161+
162+
if diff := cmp.Diff(gotMap, wantMap); diff != "" {
163+
t.Fatalf("unexpected envs: %s", diff)
164+
}
165+
})
166+
}
167+
}
168+
74169
func TestMergeKVs(t *testing.T) {
75170
testCases := []struct {
76171
name string

0 commit comments

Comments
 (0)