Просмотр исходного кода

feat(config): warn on unknown onclick and dashboard type values

Unknown values for an action's onclick, defaultOnClick, and dashboard
component type previously fell back to defaults silently. They are now
reported as warnings on the Diagnostics page and in the log, while
keeping the existing fallback behaviour for backwards compatibility.

Also fix the k8s solution doc, which used the invalid
defaultOnClick value execution-dialog-output-only.

Refs #1145

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jamesread 2 дней назад
Родитель
Сommit
4a72a6c464

+ 1 - 1
docs/modules/ROOT/pages/solutions/k8s-control-panel-hosted/index.adoc

@@ -82,7 +82,7 @@ Add `kubectl` job to OliveTin config with `kubectl edit cm/olivetin-config -n ol
 apiVersion: v1
 data:
   config.yaml: |
-    defaultOnClick: execution-dialog-output-only
+    defaultOnClick: execution-dialog
 
     actions:
       - title: get pods

+ 56 - 13
service/internal/config/sanitize.go

@@ -263,6 +263,7 @@ func (action *Action) sanitize(cfg *Config) {
 	action.Icon = lookupHTMLIcon(action.Icon, cfg.DefaultIconForActions)
 	migrateActionOnClick(action)
 	action.sanitizeJustification()
+	action.reportUnknownOnClick(cfg)
 	action.OnClick = sanitizeOnClick(action.OnClick, cfg)
 	action.PopupOnStart = action.OnClick
 
@@ -454,22 +455,63 @@ func getActionID(action *Action) string {
 	return action.ID
 }
 
-//gocyclo:ignore
+// onClickModes are the onclick values that change how an action behaves.
+var onClickModes = []string{
+	"execution-dialog",
+	"execution-dialog-output-html",
+	"execution-dialog-stdout-only",
+	"execution-button",
+	"history",
+}
+
+// onClickDefaultAliases are accepted values that mean "use the default behaviour".
+var onClickDefaultAliases = []string{
+	"",
+	"default",
+	"nothing",
+}
+
+func isKnownOnClick(value string) bool {
+	return slices.Contains(onClickModes, value) || slices.Contains(onClickDefaultAliases, value)
+}
+
 func sanitizeOnClick(raw string, cfg *Config) string {
-	switch raw {
-	case "execution-dialog":
+	if slices.Contains(onClickModes, raw) {
 		return raw
-	case "execution-dialog-output-html":
-		return raw
-	case "execution-dialog-stdout-only":
-		return raw
-	case "execution-button":
-		return raw
-	case "history":
-		return raw
-	default:
-		return cfg.DefaultOnClick
 	}
+
+	return cfg.DefaultOnClick
+}
+
+func (action *Action) reportUnknownOnClick(cfg *Config) {
+	// On a second sanitize pass, an action's onclick may already hold the
+	// (separately reported) defaultOnClick value.
+	if isKnownOnClick(action.OnClick) || action.OnClick == cfg.DefaultOnClick {
+		return
+	}
+
+	configissues.ReportSticky(configissues.Issue{
+		Severity:    configissues.SeverityWarning,
+		Code:        configissues.CodeOnClickUnknown,
+		Message:     fmt.Sprintf("Unknown onclick value %q, using defaultOnClick (%q) instead", action.OnClick, cfg.DefaultOnClick),
+		ActionID:    action.ID,
+		ActionTitle: action.Title,
+		Source:      action.OnClick,
+		ConfigFile:  action.SourceFile,
+	})
+}
+
+func (cfg *Config) reportUnknownDefaultOnClick() {
+	if isKnownOnClick(cfg.DefaultOnClick) {
+		return
+	}
+
+	configissues.ReportSticky(configissues.Issue{
+		Severity: configissues.SeverityWarning,
+		Code:     configissues.CodeOnClickUnknown,
+		Message:  fmt.Sprintf("Unknown defaultOnClick value %q, actions will behave as if it were \"nothing\"", cfg.DefaultOnClick),
+		Source:   cfg.DefaultOnClick,
+	})
 }
 
 func migrateActionOnClick(action *Action) {
@@ -511,6 +553,7 @@ func (cfg *Config) sanitizeOnClickDefaults() {
 		cfg.DefaultOnClick = "nothing"
 	}
 
+	cfg.reportUnknownDefaultOnClick()
 	cfg.DefaultPopupOnStart = cfg.DefaultOnClick
 }
 

+ 50 - 0
service/internal/config/sanitize_test.go

@@ -3,6 +3,7 @@ package config
 import (
 	"testing"
 
+	"github.com/OliveTin/OliveTin/internal/configissues"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -293,3 +294,52 @@ func TestValidateChecklistChoiceValuesAllowsCommas(t *testing.T) {
 	err := c.validateChecklistChoiceValues()
 	require.NoError(t, err)
 }
+
+func stickyIssuesWithCode(code string) []configissues.Issue {
+	out := make([]configissues.Issue, 0)
+	for _, issue := range configissues.CopySticky() {
+		if issue.Code == code {
+			out = append(out, issue)
+		}
+	}
+	return out
+}
+
+func TestSanitizeReportsUnknownOnClick(t *testing.T) {
+	configissues.BeginConfigLoad()
+	c := DefaultConfig()
+	c.Actions = append(c.Actions, &Action{Title: "Ping", OnClick: "myexample"})
+	c.Sanitize()
+
+	issues := stickyIssuesWithCode(configissues.CodeOnClickUnknown)
+	require.Len(t, issues, 1)
+	assert.Equal(t, "Ping", issues[0].ActionTitle)
+	assert.Equal(t, "myexample", issues[0].Source)
+	assert.Equal(t, "nothing", c.findAction("Ping").OnClick, "Unknown onclick still falls back to the default")
+}
+
+func TestSanitizeAcceptsKnownOnClickValues(t *testing.T) {
+	configissues.BeginConfigLoad()
+	c := DefaultConfig()
+	for _, value := range []string{"", "default", "nothing", "execution-dialog", "execution-dialog-output-html", "execution-dialog-stdout-only", "execution-button", "history"} {
+		c.Actions = append(c.Actions, &Action{Title: "Action " + value, OnClick: value})
+	}
+	c.Sanitize()
+
+	assert.Empty(t, stickyIssuesWithCode(configissues.CodeOnClickUnknown))
+}
+
+func TestSanitizeReportsUnknownDefaultOnClickOnce(t *testing.T) {
+	configissues.BeginConfigLoad()
+	c := DefaultConfig()
+	c.DefaultOnClick = "myexample"
+	c.Actions = append(c.Actions, &Action{Title: "Ping"})
+	c.Sanitize()
+	c.Sanitize()
+
+	issues := stickyIssuesWithCode(configissues.CodeOnClickUnknown)
+	require.NotEmpty(t, issues)
+	for _, issue := range issues {
+		assert.Empty(t, issue.ActionTitle, "Only defaultOnClick is reported, not each action inheriting it")
+	}
+}

+ 61 - 0
service/internal/configcheck/dashboard_types.go

@@ -0,0 +1,61 @@
+package configcheck
+
+import (
+	"fmt"
+	"slices"
+
+	"github.com/OliveTin/OliveTin/internal/config"
+	"github.com/OliveTin/OliveTin/internal/configissues"
+)
+
+// knownDashboardComponentTypes are the dashboard component types the web UI
+// can render. An empty type lets OliveTin pick one from the component's contents.
+var knownDashboardComponentTypes = []string{
+	"",
+	"fieldset",
+	"directory",
+	"display",
+	"link",
+	"stdout-most-recent-execution",
+}
+
+func collectDashboardTypeIssues(cfg *config.Config) []configissues.Issue {
+	out := make([]configissues.Issue, 0)
+	visited := make(map[*config.DashboardComponent]bool)
+
+	for _, dashboard := range cfg.Dashboards {
+		out = append(out, dashboardTypeIssues(dashboard, dashboard, visited)...)
+	}
+
+	return out
+}
+
+func dashboardTypeIssues(dashboard, component *config.DashboardComponent, visited map[*config.DashboardComponent]bool) []configissues.Issue {
+	if component == nil || visited[component] {
+		return nil
+	}
+
+	visited[component] = true
+
+	out := unknownDashboardTypeIssue(dashboard, component)
+
+	for _, child := range component.Contents {
+		out = append(out, dashboardTypeIssues(dashboard, child, visited)...)
+	}
+
+	return out
+}
+
+func unknownDashboardTypeIssue(dashboard, component *config.DashboardComponent) []configissues.Issue {
+	if slices.Contains(knownDashboardComponentTypes, component.Type) {
+		return nil
+	}
+
+	return []configissues.Issue{{
+		Severity: configissues.SeverityWarning,
+		Code:     configissues.CodeDashboardTypeUnknown,
+		Message: fmt.Sprintf("Dashboard %q component %q has unknown type %q; a default type will be used instead",
+			dashboard.Title, component.Title, component.Type),
+		Source: component.Type,
+	}}
+}

+ 70 - 0
service/internal/configcheck/dashboard_types_test.go

@@ -0,0 +1,70 @@
+package configcheck_test
+
+import (
+	"testing"
+
+	"github.com/OliveTin/OliveTin/internal/config"
+	"github.com/OliveTin/OliveTin/internal/configcheck"
+	"github.com/OliveTin/OliveTin/internal/configissues"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestRebuildUnknownDashboardComponentType(t *testing.T) {
+	configissues.BeginConfigLoad()
+	cfg := config.DefaultConfig()
+	cfg.Dashboards = []*config.DashboardComponent{
+		{
+			Title: "Containers",
+			Contents: []*config.DashboardComponent{
+				{
+					Title:  "Container {{ .CurrentEntity.Names }}",
+					Entity: "container",
+					Type:   "myexample",
+					Contents: []*config.DashboardComponent{
+						{Title: "Start", Type: "display"},
+					},
+				},
+			},
+		},
+	}
+
+	configcheck.Rebuild(cfg)
+
+	issues := issuesWithCode(configissues.List(), configissues.CodeDashboardTypeUnknown)
+	require.Len(t, issues, 1)
+	assert.Equal(t, "myexample", issues[0].Source)
+	assert.Contains(t, issues[0].Message, "Containers")
+}
+
+func TestRebuildKnownDashboardComponentTypes(t *testing.T) {
+	configissues.BeginConfigLoad()
+	cfg := config.DefaultConfig()
+	cfg.Dashboards = []*config.DashboardComponent{
+		{
+			Title: "Everything",
+			Contents: []*config.DashboardComponent{
+				{Title: "a"},
+				{Title: "b", Type: "fieldset"},
+				{Title: "c", Type: "directory"},
+				{Title: "d", Type: "display"},
+				{Title: "e", Type: "link"},
+				{Title: "f", Type: "stdout-most-recent-execution"},
+			},
+		},
+	}
+
+	configcheck.Rebuild(cfg)
+
+	assert.False(t, hasCode(configissues.List(), configissues.CodeDashboardTypeUnknown))
+}
+
+func issuesWithCode(issues []configissues.Issue, code string) []configissues.Issue {
+	out := make([]configissues.Issue, 0)
+	for _, issue := range issues {
+		if issue.Code == code {
+			out = append(out, issue)
+		}
+	}
+	return out
+}

+ 1 - 0
service/internal/configcheck/rebuild.go

@@ -38,6 +38,7 @@ func Rebuild(cfg *config.Config, extra ...configissues.Issue) {
 	collected = append(collected, collectEntityEmptyIssues(cfg)...)
 	collected = append(collected, collectCronIssues(cfg)...)
 	collected = append(collected, collectWatcherPathIssues(cfg)...)
+	collected = append(collected, collectDashboardTypeIssues(cfg)...)
 	collected = append(collected, extra...)
 	configissues.Replace(collected)
 }

+ 2 - 0
service/internal/configissues/issue.go

@@ -23,6 +23,8 @@ const (
 	CodeWatcherPath            = "watcher_path"
 	CodeAclUnknown             = "acl_unknown"
 	CodeDeprecatedAPICall      = "deprecated_api_call"
+	CodeOnClickUnknown         = "onclick_unknown"
+	CodeDashboardTypeUnknown   = "dashboard_type_unknown"
 )
 
 // Issue is a configuration warning or error surfaced on Diagnostics.