fix: broken tests after changing the way arguments are parsed

.gitignore

@@ -12,3 +12,4 @@ frontend/dist/
 frontend/node_modules
 custom-frontend
 integration-tests/screenshots/
+.vscode/

config.yaml

@@ -5,6 +5,12 @@
 # Listen on all addresses available, port 1337
 listenAddressSingleHTTPFrontend: 0.0.0.0:1337
 
+bannerMessage: "This is an early alpha version of OliveTin 3000. Many thanks are broken, many things will change."
+bannerCss: "background-color: #b2e4b2; color: black; font-size: small; text-align: center; padding: .6em; border-radius: 0.5em;"
+
+insecureAllowDumpSos: true
+insecureAllowDumpVars: true
+
 # Choose from INFO (default), WARN and DEBUG
 logLevel: "INFO"
 
@@ -99,7 +105,7 @@ actions:
   # Docs: https://docs.olivetin.app/solutions/container-control-panel/index.html
   - title: Restart Docker Container
     icon: restart
-    shell: docker restart {{ container }}
+    shell: docker restart {{ .CurrentEntity }}
     arguments:
       - name: container
         title: Container name
@@ -205,15 +211,15 @@ actions:
     shell: "echo 'Ping all servers'"
     icon: ping
 
-  - title: Start {{ container.Names }}
+  - title: Start {{ .CurrentEntity.Names }}
     icon: box
-    shell: docker start {{ container.Names }}
+    shell: docker start {{ .CurrentEntity.Names }}
     entity: container
     triggers: ["Update container entity file"]
 
-  - title: Stop {{ container.Names }}
+  - title: Stop {{ .CurrentEntity.Names }}
     icon: box
-    shell: docker stop {{ container.Names }}
+    shell: docker stop {{ .CurrentEntity.Names }}
     entity: container
     triggers: ["Update container entity file"]
 
@@ -287,7 +293,7 @@ dashboards:
       # actions grouped together without a folder.
       - type: fieldset
         entity: server
-        title: 'Server: {{ server.hostname }}'
+        title: 'Server: {{ .CurrentEntity.hostname }}'
         contents:
           # By default OliveTin will look for an action with a matching title
           # and put it on the dashboard.
@@ -306,7 +312,7 @@ dashboards:
   # This is the second dashboard.
   - title: My Containers
     contents:
-      - title: 'Container {{ container.Names }} ({{ container.Image }})'
+      - title: 'Container {{ .CurrentEntity.Names }} ({{ .CurrentEntity.Image }})'
         entity: container
         type: fieldset
         contents:
@@ -314,5 +320,5 @@ dashboards:
             title: |
               {{ container.RunningFor }} <br /><br /><strong>{{ container.State }}</strong>
 
-          - title: 'Start {{ container.Names }}'
-          - title: 'Stop {{ container.Names }}'
+          - title: 'Start {{ .CurrentEntity.Names }}'
+          - title: 'Stop {{ .CurrentEntity.Names }}'

service/internal/api/api_test.go

@@ -1,11 +1,12 @@
 package api
 
 import (
-	"connectrpc.com/connect"
 	"context"
-	"github.com/stretchr/testify/assert"
 	"testing"
 
+	"connectrpc.com/connect"
+	"github.com/stretchr/testify/assert"
+
 	log "github.com/sirupsen/logrus"
 
 	apiv1 "github.com/OliveTin/OliveTin/gen/olivetin/api/v1"
@@ -15,24 +16,27 @@ import (
 
 	"net/http"
 	"net/http/httptest"
+	"path"
 )
 
 func getNewTestServerAndClient(t *testing.T, injectedConfig *config.Config) (*httptest.Server, apiv1connect.OliveTinApiServiceClient) {
 	ex := executor.DefaultExecutor(injectedConfig)
 	ex.RebuildActionMap()
 
-	path, handler := GetNewHandler(ex)
-
-	path = "/api" + path
+	apiPath, apiHandler := GetNewHandler(ex)
 
 	mux := http.NewServeMux()
-	mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) {
+	mux.Handle("/api/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		log.Infof("HTTP Request: %s %s", r.Method, r.URL.Path)
 
-		http.StripPrefix("/api/", handler)
-	})
+		// Translate /api/<service>/<method> to <service>/<method>
+		fn := path.Base(r.URL.Path)
+		r.URL.Path = apiPath + fn
 
-	log.Infof("API path is %s", path)
+		apiHandler.ServeHTTP(w, r)
+	}))
+
+	log.Infof("API path is %s", apiPath)
 
 	httpclient := &http.Client{}
 
@@ -40,7 +44,7 @@ func getNewTestServerAndClient(t *testing.T, injectedConfig *config.Config) (*ht
 
 	client := apiv1connect.NewOliveTinApiServiceClient(httpclient, ts.URL+"/api")
 
-	log.Infof("Test server URL is %s", ts.URL+path)
+	log.Infof("Test server URL is %s", ts.URL+"/api"+apiPath)
 
 	return ts, client
 }
@@ -59,11 +63,16 @@ func TestGetActionsAndStart(t *testing.T) {
 
 	conn, client := getNewTestServerAndClient(t, cfg)
 
-	respInit, err := client.Init(context.Background(), connect.NewRequest(&apiv1.InitRequest{}))
-	respGetReady, err := client.GetReadyz(context.Background(), connect.NewRequest(&apiv1.GetReadyzRequest{}))
+	respInit, errInit := client.Init(context.Background(), connect.NewRequest(&apiv1.InitRequest{}))
+	respGetReady, errReady := client.GetReadyz(context.Background(), connect.NewRequest(&apiv1.GetReadyzRequest{}))
 
-	if err != nil {
-		t.Errorf("GetDashboardComponentsRequest: %v", err)
+	if errInit != nil {
+		t.Errorf("Init request failed: %v", errInit)
+		return
+	}
+
+	if errReady != nil {
+		t.Errorf("GetReadyz request failed: %v", errReady)
 		return
 	}
 

service/internal/entities/templates.go

@@ -11,10 +11,11 @@ import (
 
 var tpl = template.New("tpl")
 
-var legacyEntityRegex = regexp.MustCompile(`{{ ([a-zA-Z0-9_]+)\.*?([a-zA-Z0-9_\.]+) }}`)
+var legacyArgumentRegex = regexp.MustCompile(`{{ ([a-zA-Z0-9_]+) }}`)
+var legacyEntityPropertiesRegex = regexp.MustCompile(`{{ ([a-zA-Z0-9_]+)\.([a-zA-Z0-9_\.]+) }}`)
 
-func migrateLegacyArgumentNames(rawShellCommand string) string {
-	foundArgumentNames := legacyEntityRegex.FindAllStringSubmatch(rawShellCommand, -1)
+func migrateLegacyEntityProperties(rawShellCommand string) string {
+	foundArgumentNames := legacyEntityPropertiesRegex.FindAllStringSubmatch(rawShellCommand, -1)
 
 	for _, match := range foundArgumentNames {
 		entityName := match[1]
@@ -45,8 +46,28 @@ func migrateLegacyArgumentNames(rawShellCommand string) string {
 	return rawShellCommand
 }
 
+func migrateLegacyArgumentNames(rawShellCommand string) string {
+	foundArgumentNames := legacyArgumentRegex.FindAllStringSubmatch(rawShellCommand, -1)
+
+	for _, match := range foundArgumentNames {
+		argName := match[1]
+
+		if !strings.HasPrefix(argName, ".Arguments.") {
+			log.WithFields(log.Fields{
+				"old": argName,
+				"new": ".Arguments." + argName,
+			}).Warnf("Legacy variable name found, changing to Argument")
+
+			rawShellCommand = strings.ReplaceAll(rawShellCommand, argName, ".Arguments."+argName)
+		}
+	}
+
+	return rawShellCommand
+}
+
 func ParseTemplateWithArgs(source string, ent *Entity, args map[string]string) string {
 	source = migrateLegacyArgumentNames(source)
+	source = migrateLegacyEntityProperties(source)
 
 	ret := ""
 

service/internal/executor/arguments.go

@@ -25,7 +25,7 @@ var (
 )
 
 func parseCommandForReplacements(shellCommand string, values map[string]string, entity any) (string, error) {
-	r := regexp.MustCompile(`{{ *?\.Arguments\.([a-zA-Z0-9_]+?) *?}}`)
+	r := regexp.MustCompile(`{{ *?([a-zA-Z0-9_]+?) *?}}`)
 	foundArgumentNames := r.FindAllStringSubmatch(shellCommand, -1)
 
 	for _, match := range foundArgumentNames {
@@ -66,7 +66,7 @@ func parseActionArguments(values map[string]string, action *config.Action, entit
 		}).Debugf("Arg assigned")
 	}
 
-	parsedShellCommand := entities.ParseTemplateWith(rawShellCommand, entity)
+	parsedShellCommand := entities.ParseTemplateWithArgs(rawShellCommand, entity, values)
 	redactedShellCommand := redactShellCommand(parsedShellCommand, action.Arguments, values)
 
 	if err != nil {

service/internal/executor/arguments_test.go

@@ -257,26 +257,35 @@ func TestTypeSafetyCheckUnicodeIdentifier(t *testing.T) {
 		name     string
 		field    string
 		value    string
-		hasError bool
+		expectsError bool
 	}{
 		{"Valid unicode identifier", "name", "hello_world", false},
 		{"Valid with numbers", "name", "test123", false},
-		{"Valid with spaces", "name", "hello world", false},
-		{"Valid with path separators", "name", "path/to/file", false},
-		{"Valid with backslashes", "name", "path\\to\\file", false},
 		{"Valid with dots", "name", "file.txt", false},
 		{"Valid with underscores", "name", "my_file_name", false},
 		{"Invalid with special chars", "name", "hello@world", true},
 		{"Invalid with brackets", "name", "hello[world]", true},
+		{"Invalid with spaces", "name", "hello world", true},
+		{"Invalid with path separators", "name", "path/to/file", true},
+		{"Invalid with backslashes", "name", "path\\to\\file", true},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			err := TypeSafetyCheck(tt.field, tt.value, "unicode_identifier")
-			if tt.hasError {
-				assert.NotNil(t, err, "Expected error for value '%s'", tt.value)
+
+			if tt.expectsError {
+				if err == nil {
+					t.Errorf("Expected error for value '%s', but got none", tt.value)
+				} else {
+					t.Logf("Received expected error for value '%s': %v", tt.value, err)
+				}
 			} else {
-				assert.Nil(t, err, "Expected no error for value '%s', but got: %v", tt.value, err)
+				if err != nil {
+					t.Errorf("Expected no error for value '%s', but got: %v", tt.value, err)
+				} else {
+					t.Logf("No error for valid value '%s' as expected", tt.value)
+				}
 			}
 		})
 	}

service/internal/executor/executor.go

@@ -447,6 +447,13 @@ func stepParseArgs(req *ExecutionRequest) bool {
 func stepRequestAction(req *ExecutionRequest) bool {
 	metricActionsRequested.Inc()
 
+	// If there is no binding or action, do not proceed. Leave default
+	// log entry values (icon/title/id) and stop execution gracefully.
+	if req.Binding == nil || req.Binding.Action == nil {
+		log.Warnf("Action request has no binding/action; skipping execution")
+		return false
+	}
+
 	req.logEntry.ActionConfigTitle = req.Binding.Action.Title
 	req.logEntry.ActionTitle = entities.ParseTemplateWith(req.Binding.Action.Title, req.Binding.Entity)
 	req.logEntry.ActionIcon = req.Binding.Action.Icon

service/internal/executor/executor_test.go

@@ -1,9 +1,10 @@
 package executor
 
 import (
-	"github.com/stretchr/testify/assert"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+
 	acl "github.com/OliveTin/OliveTin/internal/acl"
 	config "github.com/OliveTin/OliveTin/internal/config"
 )
@@ -41,6 +42,12 @@ func TestCreateExecutorAndExec(t *testing.T) {
 		},
 	}
 
+	// Ensure bindings are available and set the binding to the only configured action
+	e.RebuildActionMap()
+	if len(cfg.Actions) > 0 {
+		req.Binding = e.FindBindingWithNoEntity(cfg.Actions[0])
+	}
+
 	assert.NotNil(t, e, "Create an executor")
 
 	wg, _ := e.ExecRequest(&req)
@@ -114,11 +121,11 @@ func TestGetLogsEmpty(t *testing.T) {
 
 	assert.Equal(t, int64(10), cfg.LogHistoryPageSize, "Logs page size should be 10")
 
-	logs, remaining := e.GetLogTrackingIds(0, 10)
+	logs, paging := e.GetLogTrackingIds(0, 10)
 
 	assert.NotNil(t, logs, "Logs should not be nil")
 	assert.Equal(t, 0, len(logs), "No logs yet")
-	assert.Equal(t, int64(0), remaining, "There should be no remaining logs")
+	assert.Equal(t, int64(0), paging.CountRemaining, "There should be no remaining logs")
 }
 
 func TestGetLogsLessThanPageSize(t *testing.T) {
@@ -130,12 +137,15 @@ func TestGetLogsLessThanPageSize(t *testing.T) {
 	})
 	cfg.Sanitize()
 
+	// Rebuild action map to include newly added action
+	e.RebuildActionMap()
+
 	assert.Equal(t, int64(10), cfg.LogHistoryPageSize, "Logs page size should be 10")
 
-	logEntries, remaining := e.GetLogTrackingIds(0, 10)
+	logEntries, paging := e.GetLogTrackingIds(0, 10)
 
 	assert.Equal(t, 0, len(logEntries), "There should be 0 logs")
-	assert.Zero(t, remaining, "There should be no remaining logs")
+	assert.Zero(t, paging.CountRemaining, "There should be no remaining logs")
 
 	execNewReqAndWait(e, "blat", cfg)
 	execNewReqAndWait(e, "blat", cfg)
@@ -145,10 +155,10 @@ func TestGetLogsLessThanPageSize(t *testing.T) {
 	execNewReqAndWait(e, "blat", cfg)
 	execNewReqAndWait(e, "blat", cfg)
 
-	logEntries, remaining = e.GetLogTrackingIds(0, 10)
+	logEntries, paging = e.GetLogTrackingIds(0, 10)
 
 	assert.Equal(t, 7, len(logEntries), "There should be 7 logs")
-	assert.Zero(t, remaining, "There should be no remaining logs")
+	assert.Zero(t, paging.CountRemaining, "There should be no remaining logs")
 
 	execNewReqAndWait(e, "blat", cfg)
 	execNewReqAndWait(e, "blat", cfg)
@@ -156,10 +166,10 @@ func TestGetLogsLessThanPageSize(t *testing.T) {
 	execNewReqAndWait(e, "blat", cfg)
 	execNewReqAndWait(e, "blat", cfg)
 
-	logEntries, remaining = e.GetLogTrackingIds(0, 10)
+	logEntries, paging = e.GetLogTrackingIds(0, 10)
 
 	assert.Equal(t, 10, len(logEntries), "There should be 10 logs")
-	assert.Equal(t, int64(2), remaining, "There should be 1 remaining logs")
+	assert.Equal(t, int64(2), paging.CountRemaining, "There should be 1 remaining logs")
 }
 
 func execNewReqAndWait(e *Executor, title string, cfg *config.Config) {
@@ -168,6 +178,19 @@ func execNewReqAndWait(e *Executor, title string, cfg *config.Config) {
 		Cfg: cfg,
 	}
 
+	// Ensure we have a binding for the requested title
+	e.RebuildActionMap()
+	var action *config.Action
+	for _, a := range cfg.Actions {
+		if a.Title == title {
+			action = a
+			break
+		}
+	}
+	if action != nil {
+		req.Binding = e.FindBindingWithNoEntity(action)
+	}
+
 	wg, _ := e.ExecRequest(req)
 	wg.Wait()
 }
@@ -245,6 +268,9 @@ func TestMangleInvalidArgumentValues(t *testing.T) {
 	cfg.Actions = append(cfg.Actions, a1)
 	cfg.Sanitize()
 
+	// Build bindings for newly added action
+	e.RebuildActionMap()
+
 	req := ExecutionRequest{
 		//		Action:            a1,
 		AuthenticatedUser: acl.UserFromSystem(cfg, "testuser"),
@@ -254,6 +280,9 @@ func TestMangleInvalidArgumentValues(t *testing.T) {
 		},
 	}
 
+	// Set binding to our appended action
+	req.Binding = e.FindBindingWithNoEntity(a1)
+
 	wg, _ := e.ExecRequest(&req)
 	wg.Wait()
 

service/internal/httpservers/singleFrontend.go

@@ -48,6 +48,9 @@ func StartSingleHTTPFrontend(cfg *config.Config, ex *executor.Executor) {
 	mux.Handle("/api/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		fn := path.Base(r.URL.Path)
 
+		// Translate /api/foo/bar to /api/bar - this preserves compatibility
+		// with OliveTin 2k.
+
 		r.URL.Path = apiPath + fn
 
 		log.Debugf("SingleFrontend HTTP API Req URL after rewrite: %v", r.URL.Path)