From b777d599aa1869871d67eb43cf64908d513976ee Mon Sep 17 00:00:00 2001 From: jamesread Date: Sat, 7 Feb 2026 00:09:46 +0000 Subject: [PATCH 1/2] fix: Massive cleanup of template parsing --- service/internal/api/apiActions.go | 16 +-- service/internal/api/dashboard_entities.go | 10 +- service/internal/api/dashboards.go | 2 +- service/internal/executor/arguments.go | 61 +++------ service/internal/executor/arguments_test.go | 140 +++++++++++++------- service/internal/executor/executor.go | 6 +- service/internal/executor/executor_test.go | 28 ++-- service/internal/tpl/templates.go | 107 ++++++++++----- 8 files changed, 218 insertions(+), 152 deletions(-) diff --git a/service/internal/api/apiActions.go b/service/internal/api/apiActions.go index 3ca12de..23f8f40 100644 --- a/service/internal/api/apiActions.go +++ b/service/internal/api/apiActions.go @@ -67,7 +67,7 @@ func evaluateEnabledExpression(action *config.Action, entity *entities.Entity) b return true } - result := tpl.ParseTemplateWith(action.EnabledExpression, entity) + result := tpl.ParseTemplateOfActionBeforeExec(action.EnabledExpression, entity) result = strings.TrimSpace(result) if result == "" { @@ -106,11 +106,11 @@ func evaluateResultValue(result string) bool { return false } -func getDefaultValue(cfgArg config.ActionArgument, entity *entities.Entity) string { +func getDefaultArgumentValue(cfgArg config.ActionArgument, entity *entities.Entity) string { defaultValue := cfgArg.Default if defaultValue != "" { - defaultValue = tpl.ParseTemplateWith(defaultValue, entity) + defaultValue = tpl.ParseTemplateOfActionBeforeExec(defaultValue, entity) } return defaultValue @@ -131,8 +131,8 @@ func buildAction(actionBinding *executor.ActionBinding, rr *DashboardRenderReque btn := apiv1.Action{ BindingId: actionBinding.ID, - Title: tpl.ParseTemplateWith(action.Title, actionBinding.Entity), - Icon: tpl.ParseTemplateWith(action.Icon, actionBinding.Entity), + Title: tpl.ParseTemplateOfActionBeforeExec(action.Title, actionBinding.Entity), + Icon: tpl.ParseTemplateOfActionBeforeExec(action.Icon, actionBinding.Entity), CanExec: aclCanExec && enabledExprCanExec, PopupOnStart: action.PopupOnStart, Order: int32(actionBinding.ConfigOrder), @@ -146,7 +146,7 @@ func buildAction(actionBinding *executor.ActionBinding, rr *DashboardRenderReque Title: cfgArg.Title, Type: cfgArg.Type, Description: cfgArg.Description, - DefaultValue: getDefaultValue(cfgArg, actionBinding.Entity), + DefaultValue: getDefaultArgumentValue(cfgArg, actionBinding.Entity), Choices: buildChoices(cfgArg), Suggestions: cfgArg.Suggestions, SuggestionsBrowserKey: cfgArg.SuggestionsBrowserKey, @@ -173,8 +173,8 @@ func buildChoicesEntity(firstChoice config.ActionArgumentChoice, entityTitle str for _, ent := range entList { ret = append(ret, &apiv1.ActionArgumentChoice{ - Value: tpl.ParseTemplateWith(firstChoice.Value, ent), - Title: tpl.ParseTemplateWith(firstChoice.Title, ent), + Value: tpl.ParseTemplateOfActionBeforeExec(firstChoice.Value, ent), + Title: tpl.ParseTemplateOfActionBeforeExec(firstChoice.Title, ent), }) } diff --git a/service/internal/api/dashboard_entities.go b/service/internal/api/dashboard_entities.go index 8ae21a0..7bec464 100644 --- a/service/internal/api/dashboard_entities.go +++ b/service/internal/api/dashboard_entities.go @@ -26,10 +26,10 @@ func buildEntityFieldsets(entityTitle string, tpl *config.DashboardComponent, rr func buildEntityFieldset(component *config.DashboardComponent, ent *entities.Entity, rr *DashboardRenderRequest) *apiv1.DashboardComponent { return &apiv1.DashboardComponent{ - Title: tpl.ParseTemplateWith(component.Title, ent), + Title: tpl.ParseTemplateOfActionBeforeExec(component.Title, ent), Type: "fieldset", Contents: removeFieldsetIfHasNoLinks(buildEntityFieldsetContents(component.Contents, ent, component.Entity, rr)), - CssClass: tpl.ParseTemplateWith(component.CssClass, ent), + CssClass: tpl.ParseTemplateOfActionBeforeExec(component.CssClass, ent), Action: rr.findAction(component.Title), EntityType: component.Entity, EntityKey: ent.UniqueKey, @@ -69,7 +69,7 @@ func buildEntityFieldsetContents(contents []*config.DashboardComponent, ent *ent func cloneItem(subitem *config.DashboardComponent, ent *entities.Entity, entityType string, rr *DashboardRenderRequest) *apiv1.DashboardComponent { clone := &apiv1.DashboardComponent{} - clone.CssClass = tpl.ParseTemplateWith(subitem.CssClass, ent) + clone.CssClass = tpl.ParseTemplateOfActionBeforeExec(subitem.CssClass, ent) if isLinkType(subitem.Type) { return cloneLinkItem(subitem, ent, clone, rr) @@ -84,7 +84,7 @@ func isLinkType(itemType string) bool { func cloneLinkItem(subitem *config.DashboardComponent, ent *entities.Entity, clone *apiv1.DashboardComponent, rr *DashboardRenderRequest) *apiv1.DashboardComponent { clone.Type = "link" - clone.Title = tpl.ParseTemplateWith(subitem.Title, ent) + clone.Title = tpl.ParseTemplateOfActionBeforeExec(subitem.Title, ent) // Prefer an entity-specific action when available, but fall back to a // non-entity-scoped action with the same title. This allows inline actions // defined inside entity dashboards to work without requiring an explicit @@ -99,7 +99,7 @@ func cloneLinkItem(subitem *config.DashboardComponent, ent *entities.Entity, clo } func cloneNonLinkItem(subitem *config.DashboardComponent, ent *entities.Entity, entityType string, clone *apiv1.DashboardComponent, rr *DashboardRenderRequest) *apiv1.DashboardComponent { - clone.Title = tpl.ParseTemplateWith(subitem.Title, ent) + clone.Title = tpl.ParseTemplateOfActionBeforeExec(subitem.Title, ent) clone.Type = subitem.Type if isDirectoryWithEntity(clone.Type, ent, entityType) { diff --git a/service/internal/api/dashboards.go b/service/internal/api/dashboards.go index 08ef241..11c6cf3 100644 --- a/service/internal/api/dashboards.go +++ b/service/internal/api/dashboards.go @@ -237,7 +237,7 @@ func buildDashboardComponentSimpleWithEntity(subitem *config.DashboardComponent, title := subitem.Title if entity != nil { - title = tpl.ParseTemplateWith(subitem.Title, entity) + title = tpl.ParseTemplateOfActionBeforeExec(subitem.Title, entity) } newitem := &apiv1.DashboardComponent{ diff --git a/service/internal/executor/arguments.go b/service/internal/executor/arguments.go index 89d984e..a63eb0b 100644 --- a/service/internal/executor/arguments.go +++ b/service/internal/executor/arguments.go @@ -25,29 +25,12 @@ var ( } ) -func parseCommandForReplacements(shellCommand string, values map[string]string, entity any) (string, error) { - r := regexp.MustCompile(`{{ *?([a-zA-Z0-9_]+?) *?}}`) - foundArgumentNames := r.FindAllStringSubmatch(shellCommand, -1) - - for _, match := range foundArgumentNames { - argName := match[1] - argValue, argProvided := values[argName] - - if !argProvided { - return "", fmt.Errorf("required arg not provided: %v", argName) - } - - shellCommand = strings.ReplaceAll(shellCommand, match[0], argValue) - } - - return shellCommand, nil -} - // parseExecArray parses all exec arguments in the action. func parseExecArray(action *config.Action, values map[string]string, entity *entities.Entity) ([]string, error) { parsed := make([]string, len(action.Exec)) - for i, a := range action.Exec { - out, err := parseSingleExec(a, values, entity) + + for i, segment := range action.Exec { + out, err := parseExecSegment(segment, values, entity) if err != nil { return nil, err } @@ -63,20 +46,19 @@ func parseActionExec(values map[string]string, action *config.Action, entity *en if err := validateArguments(values, action); err != nil { return nil, err } + parsed, err := parseExecArray(action, values, entity) + if err != nil { return nil, err } + logParsedExec(action, parsed, values) return parsed, nil } -func parseSingleExec(a string, values map[string]string, entity *entities.Entity) (string, error) { - arg, err := parseCommandForReplacements(a, values, entity) - if err != nil { - return "", err - } - return tpl.ParseTemplateWithArgs(arg, entity, values), nil +func parseExecSegment(arg string, values map[string]string, entity *entities.Entity) (string, error) { + return tpl.ParseTemplateWithActionContext(arg, entity, values) } func validateArguments(values map[string]string, action *config.Action) error { @@ -94,19 +76,17 @@ func logParsedExec(action *config.Action, parsed []string, values map[string]str log.WithFields(log.Fields{"actionTitle": action.Title, "cmd": redacted}).Infof("Action parse args - After (Exec)") } -func parseActionArguments(values map[string]string, action *config.Action, entity *entities.Entity) (string, error) { +func parseActionArguments(req *ExecutionRequest) (string, error) { log.WithFields(log.Fields{ - "actionTitle": action.Title, - "cmd": action.Shell, + "actionTitle": req.Binding.Action.Title, + "cmd": req.Binding.Action.Shell, }).Infof("Action parse args - Before") - rawShellCommand, err := parseCommandForReplacements(action.Shell, values, entity) - - for _, arg := range action.Arguments { + for _, arg := range req.Binding.Action.Arguments { argName := arg.Name - argValue := values[argName] + argValue := req.Arguments[argName] - err := typecheckActionArgument(&arg, argValue, action) + err := typecheckActionArgument(&arg, argValue, req.Binding.Action) if err != nil { return "", err @@ -118,15 +98,16 @@ func parseActionArguments(values map[string]string, action *config.Action, entit }).Debugf("Arg assigned") } - parsedShellCommand := tpl.ParseTemplateWithArgs(rawShellCommand, entity, values) - redactedShellCommand := redactShellCommand(parsedShellCommand, action.Arguments, values) + parsedShellCommand, err := tpl.ParseTemplateWithActionContext(req.Binding.Action.Shell, req.Binding.Entity, req.Arguments) if err != nil { return "", err } + redactedShellCommand := redactShellCommand(parsedShellCommand, req.Binding.Action.Arguments, req.Arguments) + log.WithFields(log.Fields{ - "actionTitle": action.Title, + "actionTitle": req.Binding.Action.Title, "cmd": redactedShellCommand, }).Infof("Action parse args - After") @@ -173,7 +154,7 @@ func typecheckActionArgument(arg *config.ActionArgument, value string, action *c return fmt.Errorf("argument name cannot be empty") } - return typecheckActionArgumentFound(value, action, arg) + return typecheckActionArgumentFound(value, arg) } // ValidateArgument validates a single argument value using the same logic as the executor. @@ -195,7 +176,7 @@ func ValidateArgument(arg *config.ActionArgument, value string, action *config.A return typecheckActionArgument(arg, mangledValue, action) } -func typecheckActionArgumentFound(value string, action *config.Action, arg *config.ActionArgument) error { +func typecheckActionArgumentFound(value string, arg *config.ActionArgument) error { if value == "" { return typecheckNull(arg) } @@ -257,7 +238,7 @@ func typecheckChoiceEntity(value string, arg *config.ActionArgument) error { templateChoice := arg.Choices[0].Value for _, ent := range entities.GetEntityInstances(arg.Entity) { - choice := tpl.ParseTemplateWith(templateChoice, ent) + choice := tpl.ParseTemplateOfActionBeforeExec(templateChoice, ent) if value == choice { return nil diff --git a/service/internal/executor/arguments_test.go b/service/internal/executor/arguments_test.go index 3e8242b..877b10c 100644 --- a/service/internal/executor/arguments_test.go +++ b/service/internal/executor/arguments_test.go @@ -6,6 +6,7 @@ import ( config "github.com/OliveTin/OliveTin/internal/config" "github.com/OliveTin/OliveTin/internal/entities" + "github.com/OliveTin/OliveTin/internal/tpl" log "github.com/sirupsen/logrus" "testing" @@ -114,36 +115,47 @@ func TestValidateArgumentCheckboxWithChoices(t *testing.T) { assert.NotNil(t, err, "Expected unknown checkbox title to be rejected against choices") } +func newExecRequest() *ExecutionRequest { + return &ExecutionRequest{ + Arguments: make(map[string]string), + Binding: &ActionBinding{ + Action: &config.Action{}, + }, + } +} + func TestArgumentValueNullable(t *testing.T) { - a1 := config.Action{ + req := newExecRequest() + req.Binding.Action = &config.Action{ Title: "Release the hounds", Shell: "echo 'Releasing {{ count }} hounds'", Arguments: []config.ActionArgument{ { - Name: "count", - Type: "int", + Name: "count", + Type: "int", + RejectNull: false, }, }, } - - values := map[string]string{ + req.Arguments = map[string]string{ "count": "", } - out, err := parseActionArguments(values, &a1, nil) + out, err := parseActionArguments(req) assert.Equal(t, "echo 'Releasing hounds'", out) assert.Nil(t, err) - a1.Arguments[0].RejectNull = true + req.Binding.Action.Arguments[0].RejectNull = true - _, err = parseActionArguments(values, &a1, nil) + _, err = parseActionArguments(req) assert.NotNil(t, err) } func TestArgumentNameNumbers(t *testing.T) { - a1 := config.Action{ + req := newExecRequest() + req.Binding.Action = &config.Action{ Title: "Do some tickles", Shell: "echo 'Tickling {{ person1name }}'", Arguments: []config.ActionArgument{ @@ -154,18 +166,19 @@ func TestArgumentNameNumbers(t *testing.T) { }, } - values := map[string]string{ + req.Arguments = map[string]string{ "person1name": "Fred", } - out, err := parseActionArguments(values, &a1, nil) + out, err := parseActionArguments(req) assert.Equal(t, "echo 'Tickling Fred'", out) assert.Nil(t, err) } func TestArgumentNotProvided(t *testing.T) { - a1 := config.Action{ + req := newExecRequest() + req.Binding.Action = &config.Action{ Title: "Do some tickles", Shell: "echo 'Tickling {{ personName }}'", Arguments: []config.ActionArgument{ @@ -176,24 +189,25 @@ func TestArgumentNotProvided(t *testing.T) { }, } - values := map[string]string{} + req.Arguments = map[string]string{} - out, err := parseActionArguments(values, &a1, nil) + out, err := parseActionArguments(req) assert.Equal(t, "", out) assert.Equal(t, err.Error(), "required arg not provided: personName") } func TestExecArrayParsing(t *testing.T) { - a1 := config.Action{ + req := newExecRequest() + req.Binding.Action = &config.Action{ Title: "List files", Exec: []string{"ls", "-alh"}, Arguments: []config.ActionArgument{}, } - values := map[string]string{} + req.Arguments = map[string]string{} - out, err := parseActionExec(values, &a1, nil) + out, err := parseActionExec(req.Arguments, req.Binding.Action, req.Binding.Entity) assert.Nil(t, err) assert.Equal(t, []string{"ls", "-alh"}, out) @@ -636,7 +650,7 @@ func TestParseCommandForReplacements(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - output, err := parseCommandForReplacements(tt.shellCommand, tt.values, nil) + output, err := tpl.ParseTemplateWithActionContext(tt.shellCommand, nil, tt.values) if tt.expectError { assert.NotNil(t, err, "Expected error but got none") @@ -654,56 +668,87 @@ func TestParseCommandForReplacements(t *testing.T) { func TestArgumentChoicesValidation(t *testing.T) { tests := []struct { name string - action config.Action - values map[string]string + req *ExecutionRequest expectError bool description string }{ { name: "Valid choice", - action: config.Action{ - Title: "Test choices", - Shell: "echo {{ option }}", - Arguments: []config.ActionArgument{ - { - Name: "option", - Type: "ascii", - Choices: []config.ActionArgumentChoice{ - {Value: "option1", Title: "Option 1"}, - {Value: "option2", Title: "Option 2"}, + req: &ExecutionRequest{ + Binding: &ActionBinding{ + Action: &config.Action{ + Title: "Test choices", + Shell: "echo {{ option }}", + Arguments: []config.ActionArgument{ + { + Name: "option", + Type: "ascii", + Choices: []config.ActionArgumentChoice{ + {Value: "option1", Title: "Option 1"}, + {Value: "option2", Title: "Option 2"}, + }, + }, }, }, }, + Arguments: map[string]string{"option": "option1"}, }, - values: map[string]string{"option": "option1"}, expectError: false, description: "Should accept valid choice", }, { name: "Invalid choice", - action: config.Action{ - Title: "Test choices", - Shell: "echo {{ option }}", - Arguments: []config.ActionArgument{ - { - Name: "option", - Type: "ascii", - Choices: []config.ActionArgumentChoice{ - {Value: "option1", Title: "Option 1"}, - {Value: "option2", Title: "Option 2"}, + req: &ExecutionRequest{ + Binding: &ActionBinding{ + Action: &config.Action{ + Title: "Test choices", + Shell: "echo {{ option }}", + Arguments: []config.ActionArgument{ + { + Name: "option", + Type: "ascii", + Choices: []config.ActionArgumentChoice{ + {Value: "option1", Title: "Option 1"}, + {Value: "option2", Title: "Option 2"}, + }, + }, }, }, }, + Arguments: map[string]string{"option": "invalid_option"}, }, - values: map[string]string{"option": "invalid_option"}, expectError: true, description: "Should reject invalid choice", }, + { + name: "Invalid choice", + req: &ExecutionRequest{ + Binding: &ActionBinding{ + Action: &config.Action{ + Title: "Test choices", + Shell: "echo {{ option }}", + Arguments: []config.ActionArgument{ + { + Name: "option", + Type: "ascii", + Choices: []config.ActionArgumentChoice{ + {Value: "option1", Title: "Option 1"}, + {Value: "option2", Title: "Option 2"}, + }, + }, + }, + }, + }, + Arguments: map[string]string{"option": "option1"}, + }, + expectError: false, + description: "Should accept valid choice", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := parseActionArguments(tt.values, &tt.action, nil) + _, err := parseActionArguments(tt.req) if tt.expectError { assert.NotNil(t, err, tt.description) @@ -737,7 +782,8 @@ func TestTypeSafetyCheckVeryDangerousRawString(t *testing.T) { } func TestParseActionArgumentsWithEntityPrefix(t *testing.T) { - action := config.Action{ + req := newExecRequest() + req.Binding.Action = &config.Action{ Title: "Test entity prefix", Shell: "echo 'Processing {{ name }} for entity'", Arguments: []config.ActionArgument{ @@ -745,16 +791,16 @@ func TestParseActionArgumentsWithEntityPrefix(t *testing.T) { }, } - values := map[string]string{ + req.Arguments = map[string]string{ "name": "testuser", } - ent := &entities.Entity{ + req.Binding.Entity = &entities.Entity{ Title: "entity_123", } // Test with entity prefix - output, err := parseActionArguments(values, &action, ent) + output, err := parseActionArguments(req) assert.Nil(t, err) assert.Contains(t, output, "testuser") } diff --git a/service/internal/executor/executor.go b/service/internal/executor/executor.go index 4214201..274d4b1 100644 --- a/service/internal/executor/executor.go +++ b/service/internal/executor/executor.go @@ -690,7 +690,7 @@ func handleShellBranch(req *ExecutionRequest) bool { return fail(req, err) } - cmd, err := parseActionArguments(req.Arguments, req.Binding.Action, req.Binding.Entity) + cmd, err := parseActionArguments(req) if err != nil { return fail(req, err) @@ -738,7 +738,7 @@ func stepRequestAction(req *ExecutionRequest) bool { req.logEntry.Binding = req.Binding req.logEntry.ActionConfigTitle = req.Binding.Action.Title - req.logEntry.ActionTitle = tpl.ParseTemplateWith(req.Binding.Action.Title, req.Binding.Entity) + req.logEntry.ActionTitle = tpl.ParseTemplateOfActionBeforeExec(req.Binding.Action.Title, req.Binding.Entity) req.logEntry.ActionIcon = req.Binding.Action.Icon req.logEntry.Tags = req.Tags @@ -903,7 +903,7 @@ func stepExecAfter(req *ExecutionRequest) bool { "ot_username": req.AuthenticatedUser.Username, } - finalParsedCommand, err := parseCommandForReplacements(req.Binding.Action.ShellAfterCompleted, args, req.Binding.Entity) + finalParsedCommand, err := tpl.ParseTemplateWithActionContext(req.Binding.Action.ShellAfterCompleted, req.Binding.Entity, args) if err != nil { msg := "Could not prepare shellAfterCompleted command: " + err.Error() + "\n" diff --git a/service/internal/executor/executor_test.go b/service/internal/executor/executor_test.go index 89559bb..f9efefe 100644 --- a/service/internal/executor/executor_test.go +++ b/service/internal/executor/executor_test.go @@ -74,7 +74,8 @@ func TestExecNonExistant(t *testing.T) { } func TestArgumentNameCamelCase(t *testing.T) { - a1 := &config.Action{ + req := newExecRequest() + req.Binding.Action = &config.Action{ Title: "Do some tickles", Shell: "echo 'Tickling {{ personName }}'", Arguments: []config.ActionArgument{ @@ -85,18 +86,19 @@ func TestArgumentNameCamelCase(t *testing.T) { }, } - values := map[string]string{ + req.Arguments = map[string]string{ "personName": "Fred", } - out, err := parseActionArguments(values, a1, nil) + out, err := parseActionArguments(req) assert.Equal(t, "echo 'Tickling Fred'", out) assert.Nil(t, err) } func TestArgumentNameSnakeCase(t *testing.T) { - a1 := &config.Action{ + req := newExecRequest() + req.Binding.Action = &config.Action{ Title: "Do some tickles", Shell: "echo 'Tickling {{ person_name }}'", Arguments: []config.ActionArgument{ @@ -107,11 +109,11 @@ func TestArgumentNameSnakeCase(t *testing.T) { }, } - values := map[string]string{ + req.Arguments = map[string]string{ "person_name": "Fred", } - out, err := parseActionArguments(values, a1, nil) + out, err := parseActionArguments(req) assert.Equal(t, "echo 'Tickling Fred'", out) assert.Nil(t, err) @@ -205,7 +207,8 @@ func TestGetPagingIndexes(t *testing.T) { } func TestUnsetRequiredArgument(t *testing.T) { - a1 := &config.Action{ + req := newExecRequest() + req.Binding.Action = &config.Action{ Title: "Print your name", Shell: "echo 'Your name is: {{ name }}'", Arguments: []config.ActionArgument{ @@ -216,16 +219,17 @@ func TestUnsetRequiredArgument(t *testing.T) { }, } - values := map[string]string{} + req.Arguments = map[string]string{} - out, err := parseActionArguments(values, a1, nil) + out, err := parseActionArguments(req) assert.Equal(t, "", out) assert.NotNil(t, err) } func TestUnusedArgumentStillPassesTypeSafetyCheck(t *testing.T) { - a1 := &config.Action{ + req := newExecRequest() + req.Binding.Action = &config.Action{ Title: "Print your name", Shell: "echo 'Your name is: {{ name }}'", Arguments: []config.ActionArgument{ @@ -240,12 +244,12 @@ func TestUnusedArgumentStillPassesTypeSafetyCheck(t *testing.T) { }, } - values := map[string]string{ + req.Arguments = map[string]string{ "name": "Fred", "age": "Not an integer", } - out, err := parseActionArguments(values, a1, nil) + out, err := parseActionArguments(req) assert.Equal(t, "", out) assert.NotNil(t, err) diff --git a/service/internal/tpl/templates.go b/service/internal/tpl/templates.go index a0aea20..26f9925 100644 --- a/service/internal/tpl/templates.go +++ b/service/internal/tpl/templates.go @@ -19,8 +19,8 @@ type olivetinInfo struct { Runtime *installationinfo.RuntimeInfo } -var legacyArgumentRegex = regexp.MustCompile(`{{ ([a-zA-Z0-9_]+) }}`) -var legacyEntityPropertiesRegex = regexp.MustCompile(`{{ ([a-zA-Z0-9_]+)\.([a-zA-Z0-9_\.]+) }}`) +var legacyArgumentRegex = regexp.MustCompile(`{{\s*([a-zA-Z0-9_]+)\s*}}`) +var legacyEntityPropertiesRegex = regexp.MustCompile(`{{\s*([a-zA-Z0-9_]+)\.([a-zA-Z0-9_\.]+)\s*}}`) type generalTemplateContext struct { OliveTin olivetinInfo @@ -106,40 +106,33 @@ func migrateLegacyEntityProperties(rawShellCommand string) string { } func migrateLegacyArgumentNames(rawShellCommand string) string { - foundArgumentNames := legacyArgumentRegex.FindAllStringSubmatch(rawShellCommand, -1) + matches := legacyArgumentRegex.FindAllStringSubmatchIndex(rawShellCommand, -1) - for _, match := range foundArgumentNames { - argName := match[1] + for i := len(matches) - 1; i >= 0; i-- { + match := matches[i] + fullMatchStart := match[0] + fullMatchEnd := match[1] + argNameStart := match[2] + argNameEnd := match[3] - if !strings.HasPrefix(argName, ".Arguments.") { - log.WithFields(log.Fields{ - "old": argName, - "new": ".Arguments." + argName, - }).Debugf("Legacy variable name found, changing to Argument") + argName := rawShellCommand[argNameStart:argNameEnd] - rawShellCommand = strings.ReplaceAll(rawShellCommand, argName, ".Arguments."+argName) - } + log.WithFields(log.Fields{ + "old": argName, + "new": ".Arguments." + argName, + }).Debugf("Legacy variable name found, changing to Argument") + + replacement := "{{ .Arguments." + argName + " }}" + rawShellCommand = rawShellCommand[:fullMatchStart] + replacement + rawShellCommand[fullMatchEnd:] } return rawShellCommand } -func ParseTemplateWithArgs(source string, ent *entities.Entity, args map[string]string) string { +func ParseTemplateWithActionContext(source string, ent *entities.Entity, args map[string]string) (string, error) { source = migrateLegacyArgumentNames(source) source = migrateLegacyEntityProperties(source) - ret := "" - - t, err := tpl.Parse(source) - - if err != nil { - log.WithFields(log.Fields{ - "source": source, - "err": err, - }).Error("Error parsing template") - return fmt.Sprintf("tpl parse error: %v", err.Error()) - } - var entdata any if ent != nil { @@ -154,31 +147,73 @@ func ParseTemplateWithArgs(source string, ent *entities.Entity, args map[string] CurrentEntity: entdata, } + result, err := parseTemplate(source, templateVariables) + + if isMissingArgumentError, argName := checkMissingArgumentError(err); isMissingArgumentError { + return "", fmt.Errorf("required arg not provided: %s", argName) + } + + if err != nil { + return "", err + } + + return result, nil +} + +func checkMissingArgumentError(err error) (bool, string) { + if err == nil { + return false, "" + } + + if strings.Contains(err.Error(), "map has no entry for key") { + re := regexp.MustCompile(`\.Arguments\.(\w+)`) + match := re.FindStringSubmatch(err.Error()) + if len(match) > 1 { + return true, match[1] + } + } + + return false, "" +} + +func parseTemplate(source string, data any) (string, error) { + t, err := tpl.Parse(source) + + if err != nil { + return "", err + } + + t = t.Option("missingkey=error") + var sb strings.Builder - err = t.Execute(&sb, &templateVariables) + err = t.Execute(&sb, data) if err != nil { log.WithFields(log.Fields{ - "source": source, - "err": err, - "currentEntity": ent, + "source": source, + "err": err, }).Errorf("Error executing template") - ret = fmt.Sprintf("tpl exec error: %v", err.Error()) + + return "", err } else { - ret = sb.String() + return sb.String(), nil } - - return ret } -func ParseTemplateWith(source string, ent *entities.Entity) string { - return ParseTemplateWithArgs(source, ent, nil) +func ParseTemplateOfActionBeforeExec(source string, ent *entities.Entity) string { + result, err := ParseTemplateWithActionContext(source, ent, nil) + if err != nil { + return "" + } + return result } +/* func ParseTemplateBoolWith(source string, ent *entities.Entity) bool { source = strings.TrimSpace(source) - tplBool := ParseTemplateWith(source, ent) + tplBool := ParseTemplateOfActionBeforeExec(source, ent) return tplBool == "true" } +*/ From 086a5ffd22b41766f5cbe2654a53b2771fe004b7 Mon Sep 17 00:00:00 2001 From: jamesread Date: Sat, 7 Feb 2026 00:31:24 +0000 Subject: [PATCH 2/2] chore: fix tpl coderabbit suggestions --- service/internal/tpl/templates.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/service/internal/tpl/templates.go b/service/internal/tpl/templates.go index 26f9925..bf0a242 100644 --- a/service/internal/tpl/templates.go +++ b/service/internal/tpl/templates.go @@ -12,7 +12,8 @@ import ( log "github.com/sirupsen/logrus" ) -var tpl = template.New("tpl") +var tpl = template.New("tpl"). + Option("missingkey=error") type olivetinInfo struct { Build *installationinfo.BuildInfo @@ -183,8 +184,6 @@ func parseTemplate(source string, data any) (string, error) { return "", err } - t = t.Option("missingkey=error") - var sb strings.Builder err = t.Execute(&sb, data) @@ -203,6 +202,10 @@ func parseTemplate(source string, data any) (string, error) { func ParseTemplateOfActionBeforeExec(source string, ent *entities.Entity) string { result, err := ParseTemplateWithActionContext(source, ent, nil) if err != nil { + log.WithFields(log.Fields{ + "source": source, + "err": err, + }).Errorf("Error parsing template of action before exec") return "" } return result