Skip to content

Commit 3b25ce3

Browse files
committed
fix(plugin): consistent name handling in addConfig/removeConfig and add testify tests
Signed-off-by: cannarelladev <cannarella.dev@gmail.com>
1 parent 7c7962e commit 3b25ce3

File tree

2 files changed

+454
-88
lines changed

2 files changed

+454
-88
lines changed

controllers/artifact/plugin/controller.go

Lines changed: 22 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -248,25 +248,20 @@ type PluginConfig struct {
248248
}
249249

250250
func (p *PluginConfig) isSame(other *PluginConfig) bool {
251-
if p.Name != other.Name {
251+
if p.LibraryPath != other.LibraryPath {
252+
return false
253+
}
254+
if p.OpenParams != other.OpenParams {
252255
return false
253256
}
254-
// Check if the maps are equal.
255257
if len(p.InitConfig) != len(other.InitConfig) {
256258
return false
257259
}
258-
// Check if the keys and values are equal.
259260
for key, value := range p.InitConfig {
260261
if otherValue, ok := other.InitConfig[key]; !ok || value != otherValue {
261262
return false
262263
}
263264
}
264-
if p.LibraryPath != other.LibraryPath {
265-
return false
266-
}
267-
if p.OpenParams != other.OpenParams {
268-
return false
269-
}
270265
return true
271266
}
272267

@@ -282,7 +277,6 @@ func (pc *PluginsConfig) addConfig(plugin *artifactv1alpha1.Plugin) {
282277
Name: plugin.Name,
283278
}
284279

285-
// If not nil, set the values that are not empty.
286280
if plugin.Spec.Config != nil {
287281
if plugin.Spec.Config.InitConfig != nil {
288282
config.InitConfig = plugin.Spec.Config.InitConfig
@@ -298,54 +292,44 @@ func (pc *PluginsConfig) addConfig(plugin *artifactv1alpha1.Plugin) {
298292
}
299293
}
300294

301-
// Check if the pluginConfig already exists in the list.
295+
// If an entry with the same name already exists and is identical, skip the update
296+
// to avoid unnecessary writes to the config file mounted in the pod.
302297
for i, c := range pc.Configs {
303-
if c.isSame(&config) {
304-
// Remove the plugin from the list and add the current plugin.
298+
if c.Name == config.Name {
299+
if c.isSame(&config) {
300+
return
301+
}
305302
pc.Configs = append(pc.Configs[:i], pc.Configs[i+1:]...)
306303
break
307304
}
308305
}
306+
pc.Configs = append(pc.Configs, config)
309307

310-
// Add the plugin to the list if it doesn't exist.
311-
if len(pc.Configs) == 0 {
312-
pc.Configs = append(pc.Configs, config)
313-
} else {
314-
found := false
315-
for _, c := range pc.Configs {
316-
if c.Name == plugin.Name {
317-
found = true
318-
break
319-
}
320-
}
321-
if !found {
322-
pc.Configs = append(pc.Configs, config)
323-
}
324-
}
325-
326-
// Check if the plugin is already in the list.
308+
// Add to LoadPlugins if not already present (use config.Name for consistency).
327309
for _, c := range pc.LoadPlugins {
328-
if c == plugin.Name {
310+
if c == config.Name {
329311
return
330312
}
331313
}
332-
pc.LoadPlugins = append(pc.LoadPlugins, plugin.Name)
314+
pc.LoadPlugins = append(pc.LoadPlugins, config.Name)
333315
}
334316

335317
func (pc *PluginsConfig) removeConfig(plugin *artifactv1alpha1.Plugin) {
336-
// Check if the pluginConfig already exists in the list.
318+
// Resolve the effective config name (same logic as addConfig).
319+
name := plugin.Name
320+
if plugin.Spec.Config != nil && plugin.Spec.Config.Name != "" {
321+
name = plugin.Spec.Config.Name
322+
}
323+
337324
for i, c := range pc.Configs {
338-
if c.Name == plugin.Name {
339-
// Remove the plugin from the list.
325+
if c.Name == name {
340326
pc.Configs = append(pc.Configs[:i], pc.Configs[i+1:]...)
341327
break
342328
}
343329
}
344330

345-
// Check if the plugin is already in the list.
346331
for i, c := range pc.LoadPlugins {
347-
if c == plugin.Name {
348-
// Remove the plugin from the list.
332+
if c == name {
349333
pc.LoadPlugins = append(pc.LoadPlugins[:i], pc.LoadPlugins[i+1:]...)
350334
break
351335
}

0 commit comments

Comments
 (0)