Browse Source

fix: Massive cleanup of template parsing (#861)

James Read 4 months ago
parent
commit
e0da041b67

+ 8 - 8
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),
 		})
 	}
 

+ 5 - 5
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) {

+ 1 - 1
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{

+ 21 - 40
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

+ 93 - 47
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")
 }

+ 3 - 3
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"

+ 16 - 12
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)

+ 75 - 37
service/internal/tpl/templates.go

@@ -12,15 +12,16 @@ 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
 	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 +107,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 +148,75 @@ 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
+	}
+
 	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 {
+		log.WithFields(log.Fields{
+			"source": source,
+			"err":    err,
+		}).Errorf("Error parsing template of action before exec")
+		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"
 }
+*/