Skip to content

Commit 4fbc3bb

Browse files
committed
fix: update command now applies new default values
1 parent dc57271 commit 4fbc3bb

5 files changed

Lines changed: 182 additions & 20 deletions

File tree

internal/handlers/compose/update.go

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,22 @@ func UpdateHandler(params UpdateHandlerParams) error {
9696
"project", project,
9797
)
9898

99-
// Pull config
99+
// Load existing config FIRST, before any template pulls or scripts run
100+
// This is critical to capture the OLD template defaults for smart merging
101+
confFile, err := models.CreateFile(configPath, "values.yaml")
102+
if err != nil {
103+
logger.Error(err)
104+
105+
return err
106+
}
107+
existingConfig, err := models.LoadConfigFrom(confFile, false)
108+
if err != nil {
109+
logger.Error(err)
110+
111+
return err
112+
}
113+
114+
// Pull config AFTER loading existing config to avoid overwriting old template
100115
logger.Debug("pulling latest template")
101116
if registry != "" {
102117
registryInfo := models.RegistryInfo{
@@ -119,7 +134,7 @@ func UpdateHandler(params UpdateHandlerParams) error {
119134
}
120135
}
121136

122-
// Execute update script
137+
// Execute update script AFTER loading existing config and pulling new template
123138
// Validate script paths to prevent arbitrary code execution
124139
allowedScriptDirs := []string{app.TemplatesFolder}
125140
prerunPath := filepath.Join(destPath, "sbin/pre-run")
@@ -149,20 +164,6 @@ func UpdateHandler(params UpdateHandlerParams) error {
149164
return err
150165
}
151166

152-
// Load existing config
153-
confFile, err := models.CreateFile(configPath, "values.yaml")
154-
if err != nil {
155-
logger.Error(err)
156-
157-
return err
158-
}
159-
existingConfig, err := models.LoadConfigFrom(confFile, false)
160-
if err != nil {
161-
logger.Error(err)
162-
163-
return err
164-
}
165-
166167
// Create new config
167168
newConfFile, err := models.CreateFile(templatePath, "config.yaml")
168169
if err != nil {
@@ -187,7 +188,10 @@ func UpdateHandler(params UpdateHandlerParams) error {
187188
// Extract and set values from args and existing config
188189
paramsArgs := utils.ExtractArgs(args)
189190
newConfig.SetProject(project)
190-
newConfig.GetParams().SetValues(existingConfig.GetParams().GetVariablesValues())
191+
// Use SetValuesSmartMerge to intelligently merge old values
192+
// Only values that differ from old defaults are preserved (user customizations)
193+
// Values that match old defaults are replaced with new defaults
194+
newConfig.GetParams().SetValuesSmartMerge(existingConfig.GetParams())
191195
newConfig.GetArbitrary().SetArbitrary(paramsArgs)
192196
err = newConfig.GetParams().SetLooseValues(paramsArgs)
193197
if err != nil {

internal/models/config.utils.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,9 +256,12 @@ func processTemplate(data map[string]interface{}, tpls []string,
256256
// Execute the template
257257
err = tmpl.Execute(destFile, data)
258258
if err != nil {
259-
splited := strings.Split(err.Error(), "error calling fail: ")
260-
fmt.Println("Failed instanciating template.", splited[1])
261-
return err
259+
// Try to extract a more readable error message
260+
errMsg := err.Error()
261+
if parts := strings.Split(errMsg, "error calling fail: "); len(parts) > 1 {
262+
errMsg = parts[1]
263+
}
264+
return fmt.Errorf("failed to instantiate template %s: %s", filepath.Base(path), errMsg)
262265
}
263266

264267
// Set permissions for shell scripts

internal/models/parameters.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,41 @@ func (p *Parameters) SetValues(values map[string]*Variable) {
190190
}
191191
}
192192

193+
// SetValuesSmartMerge intelligently merges values from an old configuration
194+
// It only preserves values that were actually customized by the user
195+
// A value is considered customized if it differs from the old template's default
196+
func (p *Parameters) SetValuesSmartMerge(oldParams *Parameters) {
197+
for key, newParam := range *p {
198+
oldParam, exists := (*oldParams)[key]
199+
if !exists {
200+
// Parameter doesn't exist in old config, use new default
201+
continue
202+
}
203+
204+
if oldParam.Variable.IsNil() {
205+
// Old parameter has no value set, use new default
206+
continue
207+
}
208+
209+
if oldParam.Default.IsNil() {
210+
// No old default to compare against, preserve the old value
211+
newParam.Variable = oldParam.Variable
212+
continue
213+
}
214+
215+
// Compare old value against old default
216+
oldValue := oldParam.Variable.AsString()
217+
oldDefault := oldParam.Default.AsString()
218+
219+
if oldValue != oldDefault {
220+
// Value differs from old default - user customized it
221+
// Preserve the customization
222+
newParam.Variable = oldParam.Variable
223+
}
224+
// else: value equals old default - let new default apply
225+
}
226+
}
227+
193228
func (p *Parameters) SetLooseValues(values map[string]string) error {
194229
for key, value := range values {
195230
if (*p)[key] != nil {

internal/models/parameters.map.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,42 @@ func (p *Parameters) GetVariablesValues(keys ...string) map[string]*Variable {
5656
return values
5757
}
5858

59+
// Returns only the customized (non-default) variable values
60+
// A value is considered customized if it differs from its default
61+
func (p *Parameters) GetCustomizedValues(keys ...string) map[string]*Variable {
62+
values := make(map[string]*Variable)
63+
for key, param := range *p {
64+
// Skip if keys filter is provided and key doesn't match
65+
if len(keys) > 0 {
66+
matched := false
67+
for _, k := range keys {
68+
if strings.HasPrefix(key, k) {
69+
matched = true
70+
break
71+
}
72+
}
73+
if !matched {
74+
continue
75+
}
76+
}
77+
78+
// Only include if the variable differs from the default
79+
// This indicates the user customized this value
80+
if !param.Variable.IsNil() && !param.Default.IsNil() {
81+
// Compare variable against default to detect customization
82+
varStr := param.Variable.AsString()
83+
defStr := param.Default.AsString()
84+
if varStr != defStr {
85+
values[key] = &param.Variable
86+
}
87+
} else if !param.Variable.IsNil() && param.Default.IsNil() {
88+
// If there's a variable but no default, preserve it
89+
values[key] = &param.Variable
90+
}
91+
}
92+
return values
93+
}
94+
5995
// Returns an ordered slices of the parameters keys
6096
func (p *Parameters) GetOrdered() []string {
6197
keys := make([]string, 0, len(*p))

internal/models/parameters_test.go

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,3 +447,87 @@ func TestGetParameters(t *testing.T) {
447447
})
448448
}
449449
}
450+
451+
func TestGetCustomizedValues(t *testing.T) {
452+
// Create parameters with both variable and default values
453+
// param1: value equals default (not customized)
454+
param1 := &Parameter{
455+
Type: "string",
456+
Variable: CreateVariableString("default1"),
457+
Default: CreateVariableString("default1"),
458+
}
459+
// param2: value differs from default (customized)
460+
param2 := &Parameter{
461+
Type: "string",
462+
Variable: CreateVariableString("custom2"),
463+
Default: CreateVariableString("default2"),
464+
}
465+
// param3: value differs from default (customized)
466+
param3 := &Parameter{
467+
Type: "int",
468+
Variable: CreateVariableInt(10),
469+
Default: CreateVariableInt(5),
470+
}
471+
// param4: value equals default (not customized)
472+
param4 := &Parameter{
473+
Type: "bool",
474+
Variable: CreateVariableBool(true),
475+
Default: CreateVariableBool(true),
476+
}
477+
// param5: has variable but no default (should be included)
478+
param5 := &Parameter{
479+
Type: "string",
480+
Variable: CreateVariableString("value5"),
481+
}
482+
483+
params := &Parameters{
484+
"param1": param1,
485+
"param2": param2,
486+
"param3": param3,
487+
"param4": param4,
488+
"param5": param5,
489+
"other.param2": param2,
490+
}
491+
492+
tests := []struct {
493+
name string
494+
keys []string
495+
want map[string]*Variable
496+
}{
497+
{
498+
name: "No keys - returns only customized values",
499+
keys: []string{},
500+
want: map[string]*Variable{
501+
"param2": &param2.Variable,
502+
"param3": &param3.Variable,
503+
"param5": &param5.Variable,
504+
"other.param2": &param2.Variable,
505+
},
506+
},
507+
{
508+
name: "Filter by prefix - returns only customized matching prefix",
509+
keys: []string{"param"},
510+
want: map[string]*Variable{
511+
"param2": &param2.Variable,
512+
"param3": &param3.Variable,
513+
"param5": &param5.Variable,
514+
},
515+
},
516+
{
517+
name: "Filter by other prefix",
518+
keys: []string{"other"},
519+
want: map[string]*Variable{
520+
"other.param2": &param2.Variable,
521+
},
522+
},
523+
}
524+
525+
for _, tt := range tests {
526+
t.Run(tt.name, func(t *testing.T) {
527+
got := params.GetCustomizedValues(tt.keys...)
528+
if !reflect.DeepEqual(got, tt.want) {
529+
t.Errorf("GetCustomizedValues() = %v, want %v", got, tt.want)
530+
}
531+
})
532+
}
533+
}

0 commit comments

Comments
 (0)