From 3831ac757d10ef4c59244686301f29cc64d1b30d Mon Sep 17 00:00:00 2001 From: Aravind Warrier Date: Tue, 4 Aug 2026 11:34:06 -0400 Subject: [PATCH 1/5] feat(operator): Support multiple triage actions --- pkg/operator/triage.go | 246 ++++++++++++++++++++++++++++++------ pkg/operator/triage_test.go | 95 +++++++++++--- 2 files changed, 288 insertions(+), 53 deletions(-) diff --git a/pkg/operator/triage.go b/pkg/operator/triage.go index 236e71d..8f3e6c2 100644 --- a/pkg/operator/triage.go +++ b/pkg/operator/triage.go @@ -36,12 +36,22 @@ var triageRunsV2GVR = schema.GroupVersionResource{ Resource: "triageruns", } +var applicationsV2GVR = schema.GroupVersionResource{ + Group: "apps.wandb.com", + Version: "v2", + Resource: "applications", +} + // TriageRunRequest describes one immutable request to diagnose an Application. -// Namespace and ApplicationName are required; Action defaults to "default". +// Namespace and ApplicationName are required; Actions defaults to ["default"]. type TriageRunRequest struct { - Namespace string `json:"namespace"` - ApplicationName string `json:"applicationName"` - Action string `json:"action,omitempty"` + Namespace string `json:"namespace"` + ApplicationName string `json:"applicationName"` + Actions []string `json:"actions,omitempty"` + + // Action is retained for source compatibility with clients of the initial + // single-action SDK. New callers should use Actions; setting both is invalid. + Action string `json:"action,omitempty"` } // TriageRunRef identifies the newly created TriageRun. It is intentionally @@ -84,19 +94,35 @@ type TriageCheckResult struct { DurationMilliseconds int64 `json:"durationMs,omitempty"` } +// TriageActionStatus contains the execution and results for one selected +// Application action. +type TriageActionStatus struct { + Action string `json:"action"` + Phase string `json:"phase,omitempty"` + JobName string `json:"jobName,omitempty"` + StartedAt string `json:"startedAt,omitempty"` + CompletedAt string `json:"completedAt,omitempty"` + Summary *TriageRunSummary `json:"summary,omitempty"` + Results []TriageCheckResult `json:"results,omitempty"` +} + // TriageRun is the caller-facing view of an immutable TriageRun and its latest // operator-reported status. type TriageRun struct { - Namespace string `json:"namespace"` - Name string `json:"name"` - ApplicationName string `json:"applicationName"` - Action string `json:"action"` - Phase string `json:"phase,omitempty"` - CreatedAt string `json:"createdAt,omitempty"` - StartedAt string `json:"startedAt,omitempty"` - CompletedAt string `json:"completedAt,omitempty"` - Summary *TriageRunSummary `json:"summary,omitempty"` - Results []TriageCheckResult `json:"results,omitempty"` + Namespace string `json:"namespace"` + Name string `json:"name"` + ApplicationName string `json:"applicationName"` + Actions []string `json:"actions"` + // Action mirrors the selected action for legacy single-action consumers. + Action string `json:"action,omitempty"` + Phase string `json:"phase,omitempty"` + CreatedAt string `json:"createdAt,omitempty"` + StartedAt string `json:"startedAt,omitempty"` + CompletedAt string `json:"completedAt,omitempty"` + Summary *TriageRunSummary `json:"summary,omitempty"` + ActionStatuses []TriageActionStatus `json:"actionStatuses,omitempty"` + // Results is a flattened compatibility view of all action results. + Results []TriageCheckResult `json:"results,omitempty"` } // CreateTriageRun creates a fresh TriageRun through wsm's configured dynamic @@ -110,6 +136,15 @@ func CreateTriageRun(ctx context.Context, request TriageRunRequest) (TriageRunRe return createTriageRun(ctx, dynamicClient, request) } +// ListTriageActions returns the sorted action names declared by one Application. +func ListTriageActions(ctx context.Context, namespace, applicationName string) ([]string, error) { + _, dynamicClient, err := kubectl.GetDynamicClientset() + if err != nil { + return nil, err + } + return listTriageActions(ctx, dynamicClient, namespace, applicationName) +} + // ListTriageRuns returns newest-first run history in one namespace. Supplying // ApplicationName filters the history without excluding runs created outside // wsm. @@ -149,9 +184,7 @@ func createTriageRun( if err := validateTriageRunRequest(request); err != nil { return TriageRunRef{}, err } - if request.Action == "" { - request.Action = DefaultTriageAction - } + request.Actions = normalizedTriageActions(request) created, err := dynamicClient.Resource(triageRunsV2GVR).Namespace(request.Namespace).Create( ctx, @@ -173,6 +206,43 @@ func createTriageRun( }, nil } +func listTriageActions( + ctx context.Context, + dynamicClient dynamic.Interface, + namespace string, + applicationName string, +) ([]string, error) { + if err := validateTriageRunNamespace(namespace); err != nil { + return nil, err + } + if err := validateTriageApplicationName(applicationName); err != nil { + return nil, err + } + application, err := dynamicClient.Resource(applicationsV2GVR).Namespace(namespace).Get( + ctx, + applicationName, + metav1.GetOptions{}, + ) + if err != nil { + return nil, fmt.Errorf("failed to get Application %s/%s: %w", namespace, applicationName, err) + } + actionMap, found, err := unstructured.NestedMap( + application.Object, "spec", "triage", "actions") + if err != nil { + return nil, fmt.Errorf( + "failed to read Application %s/%s triage actions: %w", namespace, applicationName, err) + } + if !found { + return []string{}, nil + } + actions := make([]string, 0, len(actionMap)) + for action := range actionMap { + actions = append(actions, action) + } + sort.Strings(actions) + return actions, nil +} + func listTriageRuns( ctx context.Context, dynamicClient dynamic.Interface, @@ -286,12 +356,20 @@ func parseTriageRun(obj *unstructured.Unstructured) (TriageRun, error) { if err != nil { return TriageRun{}, fmt.Errorf("failed to read TriageRun %s/%s application: %w", obj.GetNamespace(), obj.GetName(), err) } - action, _, err := unstructured.NestedString(obj.Object, "spec", "action") + actions, foundActions, err := unstructured.NestedStringSlice(obj.Object, "spec", "actions") if err != nil { - return TriageRun{}, fmt.Errorf("failed to read TriageRun %s/%s action: %w", obj.GetNamespace(), obj.GetName(), err) + return TriageRun{}, fmt.Errorf("failed to read TriageRun %s/%s actions: %w", obj.GetNamespace(), obj.GetName(), err) } - if action == "" { - action = DefaultTriageAction + if !foundActions { + legacyAction, _, legacyErr := unstructured.NestedString(obj.Object, "spec", "action") + if legacyErr != nil { + return TriageRun{}, fmt.Errorf( + "failed to read TriageRun %s/%s legacy action: %w", obj.GetNamespace(), obj.GetName(), legacyErr) + } + if legacyAction == "" { + legacyAction = DefaultTriageAction + } + actions = []string{legacyAction} } phase, _, err := unstructured.NestedString(obj.Object, "status", "phase") if err != nil { @@ -314,12 +392,15 @@ func parseTriageRun(obj *unstructured.Unstructured) (TriageRun, error) { Namespace: obj.GetNamespace(), Name: obj.GetName(), ApplicationName: applicationName, - Action: action, + Actions: actions, Phase: phase, CreatedAt: creationTimestamp.UTC().Format("2006-01-02T15:04:05Z07:00"), StartedAt: startedAt, CompletedAt: completedAt, } + if len(actions) == 1 { + run.Action = actions[0] + } if creationTimestamp.IsZero() { run.CreatedAt = "" } @@ -335,27 +416,93 @@ func parseTriageRun(obj *unstructured.Unstructured) (TriageRun, error) { } } - resultItems, found, err := unstructured.NestedSlice(obj.Object, "status", "results") + actionStatusItems, found, err := unstructured.NestedSlice(obj.Object, "status", "actionStatuses") if err != nil { - return TriageRun{}, fmt.Errorf("failed to read TriageRun %s/%s results: %w", obj.GetNamespace(), obj.GetName(), err) + return TriageRun{}, fmt.Errorf("failed to read TriageRun %s/%s action statuses: %w", obj.GetNamespace(), obj.GetName(), err) } if found { - run.Results = make([]TriageCheckResult, 0, len(resultItems)) - for i, item := range resultItems { - resultMap, ok := item.(map[string]any) + run.ActionStatuses = make([]TriageActionStatus, 0, len(actionStatusItems)) + for i, item := range actionStatusItems { + statusMap, ok := item.(map[string]any) if !ok { - return TriageRun{}, fmt.Errorf("TriageRun %s/%s result %d is %T, want object", obj.GetNamespace(), obj.GetName(), i, item) + return TriageRun{}, fmt.Errorf( + "TriageRun %s/%s action status %d is %T, want object", + obj.GetNamespace(), obj.GetName(), i, item) } - result, err := parseTriageCheckResult(resultMap) + status, err := parseTriageActionStatus(statusMap) if err != nil { - return TriageRun{}, fmt.Errorf("failed to parse TriageRun %s/%s result %d: %w", obj.GetNamespace(), obj.GetName(), i, err) + return TriageRun{}, fmt.Errorf( + "failed to parse TriageRun %s/%s action status %d: %w", + obj.GetNamespace(), obj.GetName(), i, err) } - run.Results = append(run.Results, result) + run.ActionStatuses = append(run.ActionStatuses, status) + run.Results = append(run.Results, status.Results...) + } + } else { + legacyResults, resultErr := parseTriageCheckResults(obj.Object, "status", "results") + if resultErr != nil { + return TriageRun{}, fmt.Errorf( + "failed to parse TriageRun %s/%s legacy results: %w", + obj.GetNamespace(), obj.GetName(), resultErr) } + run.Results = legacyResults } return run, nil } +func parseTriageActionStatus(item map[string]any) (TriageActionStatus, error) { + status := TriageActionStatus{} + var err error + if status.Action, _, err = unstructured.NestedString(item, "action"); err != nil { + return TriageActionStatus{}, err + } + if status.Phase, _, err = unstructured.NestedString(item, "phase"); err != nil { + return TriageActionStatus{}, err + } + if status.JobName, _, err = unstructured.NestedString(item, "jobRef", "name"); err != nil { + return TriageActionStatus{}, err + } + if status.StartedAt, _, err = unstructured.NestedString(item, "startedAt"); err != nil { + return TriageActionStatus{}, err + } + if status.CompletedAt, _, err = unstructured.NestedString(item, "completedAt"); err != nil { + return TriageActionStatus{}, err + } + if summary, found, summaryErr := unstructured.NestedMap(item, "summary"); summaryErr != nil { + return TriageActionStatus{}, summaryErr + } else if found { + status.Summary, err = parseTriageRunSummary(summary) + if err != nil { + return TriageActionStatus{}, err + } + } + status.Results, err = parseTriageCheckResults(item, "results") + if err != nil { + return TriageActionStatus{}, err + } + return status, nil +} + +func parseTriageCheckResults(object map[string]any, fields ...string) ([]TriageCheckResult, error) { + items, found, err := unstructured.NestedSlice(object, fields...) + if err != nil || !found { + return nil, err + } + results := make([]TriageCheckResult, 0, len(items)) + for i, item := range items { + resultMap, ok := item.(map[string]any) + if !ok { + return nil, fmt.Errorf("result %d is %T, want object", i, item) + } + result, err := parseTriageCheckResult(resultMap) + if err != nil { + return nil, fmt.Errorf("result %d: %w", i, err) + } + results = append(results, result) + } + return results, nil +} + func parseTriageRunSummary(summary map[string]any) (*TriageRunSummary, error) { result := &TriageRunSummary{} var err error @@ -417,7 +564,33 @@ func validateTriageRunRequest(request TriageRunRequest) error { if err := validateTriageRunNamespace(request.Namespace); err != nil { return err } - return validateTriageApplicationName(request.ApplicationName) + if err := validateTriageApplicationName(request.ApplicationName); err != nil { + return err + } + if request.Action != "" && len(request.Actions) > 0 { + return errors.New("set either action or actions, not both") + } + seen := make(map[string]struct{}) + for i, action := range normalizedTriageActions(request) { + if strings.TrimSpace(action) == "" { + return fmt.Errorf("triage action %d must not be empty", i) + } + if _, exists := seen[action]; exists { + return fmt.Errorf("triage action %q is selected more than once", action) + } + seen[action] = struct{}{} + } + return nil +} + +func normalizedTriageActions(request TriageRunRequest) []string { + if len(request.Actions) > 0 { + return append([]string(nil), request.Actions...) + } + if request.Action != "" { + return []string{request.Action} + } + return []string{DefaultTriageAction} } func validateTriageRunIdentity(namespace, name string) error { @@ -454,9 +627,10 @@ func validateTriageApplicationName(applicationName string) error { } func newTriageRun(request TriageRunRequest) *unstructured.Unstructured { - action := request.Action - if action == "" { - action = DefaultTriageAction + actions := normalizedTriageActions(request) + unstructuredActions := make([]any, len(actions)) + for i := range actions { + unstructuredActions[i] = actions[i] } return &unstructured.Unstructured{Object: map[string]any{ @@ -473,7 +647,7 @@ func newTriageRun(request TriageRunRequest) *unstructured.Unstructured { "applicationRef": map[string]any{ "name": request.ApplicationName, }, - "action": action, + "actions": unstructuredActions, }, }} } diff --git a/pkg/operator/triage_test.go b/pkg/operator/triage_test.go index d840ceb..5dc71f4 100644 --- a/pkg/operator/triage_test.go +++ b/pkg/operator/triage_test.go @@ -39,8 +39,8 @@ func TestCreateTriageRun(t *testing.T) { ); got != "weave-trace" { t.Fatalf("applicationRef.name = %q, want weave-trace", got) } - if got, _, _ := unstructured.NestedString(obj.Object, "spec", "action"); got != "default" { - t.Fatalf("action = %q, want default", got) + if got, _, _ := unstructured.NestedStringSlice(obj.Object, "spec", "actions"); len(got) != 1 || got[0] != "default" { + t.Fatalf("actions = %#v, want [default]", got) } obj.SetName(obj.GetGenerateName() + "abcde") @@ -59,15 +59,15 @@ func TestCreateTriageRun(t *testing.T) { } } -func TestCreateTriageRunPreservesExplicitAction(t *testing.T) { +func TestCreateTriageRunPreservesExplicitActions(t *testing.T) { t.Parallel() client := fake.NewSimpleDynamicClient(runtime.NewScheme()) client.PrependReactor("create", "triageruns", func(action k8stesting.Action) (bool, runtime.Object, error) { obj := action.(k8stesting.CreateAction).GetObject().(*unstructured.Unstructured).DeepCopy() - got, _, _ := unstructured.NestedString(obj.Object, "spec", "action") - if got != "dependencies" { - t.Fatalf("action = %q, want dependencies", got) + got, _, _ := unstructured.NestedStringSlice(obj.Object, "spec", "actions") + if len(got) != 2 || got[0] != "dependencies" || got[1] != "deep" { + t.Fatalf("actions = %#v, want [dependencies deep]", got) } obj.SetName("weave-trace-triage-explicit") return true, obj, nil @@ -76,7 +76,7 @@ func TestCreateTriageRunPreservesExplicitAction(t *testing.T) { _, err := createTriageRun(context.Background(), client, TriageRunRequest{ Namespace: "wandb", ApplicationName: "weave-trace", - Action: "dependencies", + Actions: []string{"dependencies", "deep"}, }) if err != nil { t.Fatalf("create TriageRun: %v", err) @@ -111,6 +111,22 @@ func TestCreateTriageRunValidatesRequestBeforeCallingCluster(t *testing.T) { request: TriageRunRequest{Namespace: "wandb", ApplicationName: "Not Valid"}, want: "invalid Application name", }, + { + name: "action and actions conflict", + request: TriageRunRequest{ + Namespace: "wandb", ApplicationName: "weave-trace", + Action: "default", Actions: []string{"deep"}, + }, + want: "either action or actions", + }, + { + name: "duplicate actions", + request: TriageRunRequest{ + Namespace: "wandb", ApplicationName: "weave-trace", + Actions: []string{"default", "default"}, + }, + want: "selected more than once", + }, } for _, test := range tests { @@ -169,13 +185,46 @@ func TestTriageRunGVR(t *testing.T) { } } +func TestListTriageActionsReturnsSortedApplicationActions(t *testing.T) { + t.Parallel() + + application := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "apps.wandb.com/v2", + "kind": "Application", + "metadata": map[string]any{ + "name": "weave-trace", + "namespace": "wandb", + }, + "spec": map[string]any{ + "triage": map[string]any{ + "actions": map[string]any{ + "deep": map[string]any{"args": []any{"deep"}}, + "default": map[string]any{"args": []any{"run-all"}}, + }, + }, + }, + }} + application.SetGroupVersionKind(schema.GroupVersionKind{ + Group: applicationsV2GVR.Group, Version: applicationsV2GVR.Version, Kind: "Application", + }) + client := fake.NewSimpleDynamicClient(runtime.NewScheme(), application) + + actions, err := listTriageActions(context.Background(), client, "wandb", "weave-trace") + if err != nil { + t.Fatalf("list triage actions: %v", err) + } + if len(actions) != 2 || actions[0] != "deep" || actions[1] != "default" { + t.Fatalf("actions = %#v, want [deep default]", actions) + } +} + func TestNewTriageRunHasNamespacedMetadata(t *testing.T) { t.Parallel() obj := newTriageRun(TriageRunRequest{ Namespace: "wandb", ApplicationName: "weave-trace", - Action: "default", + Actions: []string{"default"}, }) if obj.GetNamespace() != "wandb" || obj.GetCreationTimestamp() != (metav1.Time{}) { t.Fatalf("unexpected metadata: %#v", obj.Object["metadata"]) @@ -216,15 +265,23 @@ func TestListTriageRunsFiltersSortsAndParsesStatus(t *testing.T) { "error": int64(0), "overallSeverity": "fail", }, - "results": []any{ + "actionStatuses": []any{ map[string]any{ - "name": "clickhouse-reachable", - "umbrella": "clickhouse", - "severity": "pass", - "message": "connected", - "durationMs": int64(12), - "evidence": map[string]any{"host": "clickhouse"}, - "remediation": "", + "action": "default", + "phase": "Succeeded", + "jobRef": map[string]any{"name": "weave-trace-triage-newer-triage-0"}, + "summary": map[string]any{"total": int64(1), "pass": int64(1)}, + "results": []any{ + map[string]any{ + "name": "clickhouse-reachable", + "umbrella": "clickhouse", + "severity": "pass", + "message": "connected", + "durationMs": int64(12), + "evidence": map[string]any{"host": "clickhouse"}, + "remediation": "", + }, + }, }, }, }, "status"); err != nil { @@ -251,6 +308,10 @@ func TestListTriageRunsFiltersSortsAndParsesStatus(t *testing.T) { if len(runs[0].Results) != 1 || runs[0].Results[0].Name != "clickhouse-reachable" { t.Fatalf("results = %#v", runs[0].Results) } + if len(runs[0].ActionStatuses) != 1 || + runs[0].ActionStatuses[0].JobName != "weave-trace-triage-newer-triage-0" { + t.Fatalf("action statuses = %#v", runs[0].ActionStatuses) + } evidence, ok := runs[0].Results[0].Evidence.(map[string]any) if !ok || evidence["host"] != "clickhouse" { t.Fatalf("evidence = %#v", runs[0].Results[0].Evidence) @@ -359,7 +420,7 @@ func testTriageRun( obj := newTriageRun(TriageRunRequest{ Namespace: "wandb", ApplicationName: applicationName, - Action: "default", + Actions: []string{"default"}, }) obj.SetName(name) obj.SetGenerateName("") From d5b587e64daf981027747a294a896472a421f4fa Mon Sep 17 00:00:00 2001 From: Aravind Warrier Date: Tue, 4 Aug 2026 12:32:13 -0400 Subject: [PATCH 2/5] refactor(operator): expose structured triage actions --- pkg/operator/triage.go | 140 ++++++++++++++++++++++++++++-------- pkg/operator/triage_test.go | 29 ++++---- 2 files changed, 127 insertions(+), 42 deletions(-) diff --git a/pkg/operator/triage.go b/pkg/operator/triage.go index 8f3e6c2..af98222 100644 --- a/pkg/operator/triage.go +++ b/pkg/operator/triage.go @@ -42,12 +42,24 @@ var applicationsV2GVR = schema.GroupVersionResource{ Resource: "applications", } +// TriageAction is one action advertised by an Application. +type TriageAction struct { + Name string `json:"name"` + Description string `json:"description,omitempty"` +} + +// TriageActionReference selects an advertised Application action by name. +type TriageActionReference struct { + Name string `json:"name"` +} + // TriageRunRequest describes one immutable request to diagnose an Application. -// Namespace and ApplicationName are required; Actions defaults to ["default"]. +// Namespace and ApplicationName are required; Actions defaults to [{name: +// "default"}]. type TriageRunRequest struct { - Namespace string `json:"namespace"` - ApplicationName string `json:"applicationName"` - Actions []string `json:"actions,omitempty"` + Namespace string `json:"namespace"` + ApplicationName string `json:"applicationName"` + Actions []TriageActionReference `json:"actions,omitempty"` // Action is retained for source compatibility with clients of the initial // single-action SDK. New callers should use Actions; setting both is invalid. @@ -109,10 +121,10 @@ type TriageActionStatus struct { // TriageRun is the caller-facing view of an immutable TriageRun and its latest // operator-reported status. type TriageRun struct { - Namespace string `json:"namespace"` - Name string `json:"name"` - ApplicationName string `json:"applicationName"` - Actions []string `json:"actions"` + Namespace string `json:"namespace"` + Name string `json:"name"` + ApplicationName string `json:"applicationName"` + Actions []TriageActionReference `json:"actions"` // Action mirrors the selected action for legacy single-action consumers. Action string `json:"action,omitempty"` Phase string `json:"phase,omitempty"` @@ -136,8 +148,9 @@ func CreateTriageRun(ctx context.Context, request TriageRunRequest) (TriageRunRe return createTriageRun(ctx, dynamicClient, request) } -// ListTriageActions returns the sorted action names declared by one Application. -func ListTriageActions(ctx context.Context, namespace, applicationName string) ([]string, error) { +// ListTriageActions returns the sorted action metadata declared by one +// Application. +func ListTriageActions(ctx context.Context, namespace, applicationName string) ([]TriageAction, error) { _, dynamicClient, err := kubectl.GetDynamicClientset() if err != nil { return nil, err @@ -211,7 +224,7 @@ func listTriageActions( dynamicClient dynamic.Interface, namespace string, applicationName string, -) ([]string, error) { +) ([]TriageAction, error) { if err := validateTriageRunNamespace(namespace); err != nil { return nil, err } @@ -226,20 +239,60 @@ func listTriageActions( if err != nil { return nil, fmt.Errorf("failed to get Application %s/%s: %w", namespace, applicationName, err) } - actionMap, found, err := unstructured.NestedMap( + rawActions, found, err := unstructured.NestedFieldNoCopy( application.Object, "spec", "triage", "actions") if err != nil { return nil, fmt.Errorf( "failed to read Application %s/%s triage actions: %w", namespace, applicationName, err) } if !found { - return []string{}, nil - } - actions := make([]string, 0, len(actionMap)) - for action := range actionMap { - actions = append(actions, action) + return []TriageAction{}, nil + } + actions := []TriageAction{} + switch value := rawActions.(type) { + case []any: + actions = make([]TriageAction, 0, len(value)) + for i, item := range value { + actionMap, ok := item.(map[string]any) + if !ok { + return nil, fmt.Errorf( + "Application %s/%s triage action %d is %T, want object", + namespace, applicationName, i, item) + } + name, _, nameErr := unstructured.NestedString(actionMap, "name") + if nameErr != nil { + return nil, fmt.Errorf( + "Application %s/%s triage action %d has invalid name: %w", + namespace, applicationName, i, nameErr) + } + if strings.TrimSpace(name) == "" { + return nil, fmt.Errorf( + "Application %s/%s triage action %d has an empty name", + namespace, applicationName, i) + } + description, _, descriptionErr := unstructured.NestedString(actionMap, "description") + if descriptionErr != nil { + return nil, fmt.Errorf( + "Application %s/%s triage action %q has invalid description: %w", + namespace, applicationName, name, descriptionErr) + } + actions = append(actions, TriageAction{Name: name, Description: description}) + } + case map[string]any: + // Read the original map-shaped catalog during rolling upgrades. + actions = make([]TriageAction, 0, len(value)) + for name, item := range value { + description := "" + if actionMap, ok := item.(map[string]any); ok { + description, _, _ = unstructured.NestedString(actionMap, "description") + } + actions = append(actions, TriageAction{Name: name, Description: description}) + } + default: + return nil, fmt.Errorf( + "Application %s/%s triage actions are %T, want array", namespace, applicationName, rawActions) } - sort.Strings(actions) + sort.Slice(actions, func(i, j int) bool { return actions[i].Name < actions[j].Name }) return actions, nil } @@ -356,7 +409,7 @@ func parseTriageRun(obj *unstructured.Unstructured) (TriageRun, error) { if err != nil { return TriageRun{}, fmt.Errorf("failed to read TriageRun %s/%s application: %w", obj.GetNamespace(), obj.GetName(), err) } - actions, foundActions, err := unstructured.NestedStringSlice(obj.Object, "spec", "actions") + actions, foundActions, err := parseTriageActionReferences(obj.Object, "spec", "actions") if err != nil { return TriageRun{}, fmt.Errorf("failed to read TriageRun %s/%s actions: %w", obj.GetNamespace(), obj.GetName(), err) } @@ -369,7 +422,7 @@ func parseTriageRun(obj *unstructured.Unstructured) (TriageRun, error) { if legacyAction == "" { legacyAction = DefaultTriageAction } - actions = []string{legacyAction} + actions = []TriageActionReference{{Name: legacyAction}} } phase, _, err := unstructured.NestedString(obj.Object, "status", "phase") if err != nil { @@ -399,7 +452,7 @@ func parseTriageRun(obj *unstructured.Unstructured) (TriageRun, error) { CompletedAt: completedAt, } if len(actions) == 1 { - run.Action = actions[0] + run.Action = actions[0].Name } if creationTimestamp.IsZero() { run.CreatedAt = "" @@ -450,6 +503,33 @@ func parseTriageRun(obj *unstructured.Unstructured) (TriageRun, error) { return run, nil } +func parseTriageActionReferences( + object map[string]any, + fields ...string, +) ([]TriageActionReference, bool, error) { + items, found, err := unstructured.NestedSlice(object, fields...) + if err != nil || !found { + return nil, found, err + } + actions := make([]TriageActionReference, 0, len(items)) + for i, item := range items { + switch value := item.(type) { + case map[string]any: + name, _, nameErr := unstructured.NestedString(value, "name") + if nameErr != nil { + return nil, true, fmt.Errorf("action %d name: %w", i, nameErr) + } + actions = append(actions, TriageActionReference{Name: name}) + case string: + // Read the original string-shaped selection during rolling upgrades. + actions = append(actions, TriageActionReference{Name: value}) + default: + return nil, true, fmt.Errorf("action %d is %T, want object", i, item) + } + } + return actions, true, nil +} + func parseTriageActionStatus(item map[string]any) (TriageActionStatus, error) { status := TriageActionStatus{} var err error @@ -572,25 +652,25 @@ func validateTriageRunRequest(request TriageRunRequest) error { } seen := make(map[string]struct{}) for i, action := range normalizedTriageActions(request) { - if strings.TrimSpace(action) == "" { + if strings.TrimSpace(action.Name) == "" { return fmt.Errorf("triage action %d must not be empty", i) } - if _, exists := seen[action]; exists { - return fmt.Errorf("triage action %q is selected more than once", action) + if _, exists := seen[action.Name]; exists { + return fmt.Errorf("triage action %q is selected more than once", action.Name) } - seen[action] = struct{}{} + seen[action.Name] = struct{}{} } return nil } -func normalizedTriageActions(request TriageRunRequest) []string { +func normalizedTriageActions(request TriageRunRequest) []TriageActionReference { if len(request.Actions) > 0 { - return append([]string(nil), request.Actions...) + return append([]TriageActionReference(nil), request.Actions...) } if request.Action != "" { - return []string{request.Action} + return []TriageActionReference{{Name: request.Action}} } - return []string{DefaultTriageAction} + return []TriageActionReference{{Name: DefaultTriageAction}} } func validateTriageRunIdentity(namespace, name string) error { @@ -630,7 +710,7 @@ func newTriageRun(request TriageRunRequest) *unstructured.Unstructured { actions := normalizedTriageActions(request) unstructuredActions := make([]any, len(actions)) for i := range actions { - unstructuredActions[i] = actions[i] + unstructuredActions[i] = map[string]any{"name": actions[i].Name} } return &unstructured.Unstructured{Object: map[string]any{ diff --git a/pkg/operator/triage_test.go b/pkg/operator/triage_test.go index 5dc71f4..efcaf45 100644 --- a/pkg/operator/triage_test.go +++ b/pkg/operator/triage_test.go @@ -39,7 +39,8 @@ func TestCreateTriageRun(t *testing.T) { ); got != "weave-trace" { t.Fatalf("applicationRef.name = %q, want weave-trace", got) } - if got, _, _ := unstructured.NestedStringSlice(obj.Object, "spec", "actions"); len(got) != 1 || got[0] != "default" { + got, _, _ := parseTriageActionReferences(obj.Object, "spec", "actions") + if len(got) != 1 || got[0].Name != "default" { t.Fatalf("actions = %#v, want [default]", got) } @@ -65,8 +66,8 @@ func TestCreateTriageRunPreservesExplicitActions(t *testing.T) { client := fake.NewSimpleDynamicClient(runtime.NewScheme()) client.PrependReactor("create", "triageruns", func(action k8stesting.Action) (bool, runtime.Object, error) { obj := action.(k8stesting.CreateAction).GetObject().(*unstructured.Unstructured).DeepCopy() - got, _, _ := unstructured.NestedStringSlice(obj.Object, "spec", "actions") - if len(got) != 2 || got[0] != "dependencies" || got[1] != "deep" { + got, _, _ := parseTriageActionReferences(obj.Object, "spec", "actions") + if len(got) != 2 || got[0].Name != "dependencies" || got[1].Name != "deep" { t.Fatalf("actions = %#v, want [dependencies deep]", got) } obj.SetName("weave-trace-triage-explicit") @@ -76,7 +77,10 @@ func TestCreateTriageRunPreservesExplicitActions(t *testing.T) { _, err := createTriageRun(context.Background(), client, TriageRunRequest{ Namespace: "wandb", ApplicationName: "weave-trace", - Actions: []string{"dependencies", "deep"}, + Actions: []TriageActionReference{ + {Name: "dependencies"}, + {Name: "deep"}, + }, }) if err != nil { t.Fatalf("create TriageRun: %v", err) @@ -115,7 +119,7 @@ func TestCreateTriageRunValidatesRequestBeforeCallingCluster(t *testing.T) { name: "action and actions conflict", request: TriageRunRequest{ Namespace: "wandb", ApplicationName: "weave-trace", - Action: "default", Actions: []string{"deep"}, + Action: "default", Actions: []TriageActionReference{{Name: "deep"}}, }, want: "either action or actions", }, @@ -123,7 +127,7 @@ func TestCreateTriageRunValidatesRequestBeforeCallingCluster(t *testing.T) { name: "duplicate actions", request: TriageRunRequest{ Namespace: "wandb", ApplicationName: "weave-trace", - Actions: []string{"default", "default"}, + Actions: []TriageActionReference{{Name: "default"}, {Name: "default"}}, }, want: "selected more than once", }, @@ -197,9 +201,9 @@ func TestListTriageActionsReturnsSortedApplicationActions(t *testing.T) { }, "spec": map[string]any{ "triage": map[string]any{ - "actions": map[string]any{ - "deep": map[string]any{"args": []any{"deep"}}, - "default": map[string]any{"args": []any{"run-all"}}, + "actions": []any{ + map[string]any{"name": "default", "description": "Run standard diagnostics"}, + map[string]any{"name": "deep", "description": "Run deeper diagnostics"}, }, }, }, @@ -213,7 +217,8 @@ func TestListTriageActionsReturnsSortedApplicationActions(t *testing.T) { if err != nil { t.Fatalf("list triage actions: %v", err) } - if len(actions) != 2 || actions[0] != "deep" || actions[1] != "default" { + if len(actions) != 2 || actions[0].Name != "deep" || actions[1].Name != "default" || + actions[1].Description != "Run standard diagnostics" { t.Fatalf("actions = %#v, want [deep default]", actions) } } @@ -224,7 +229,7 @@ func TestNewTriageRunHasNamespacedMetadata(t *testing.T) { obj := newTriageRun(TriageRunRequest{ Namespace: "wandb", ApplicationName: "weave-trace", - Actions: []string{"default"}, + Actions: []TriageActionReference{{Name: "default"}}, }) if obj.GetNamespace() != "wandb" || obj.GetCreationTimestamp() != (metav1.Time{}) { t.Fatalf("unexpected metadata: %#v", obj.Object["metadata"]) @@ -420,7 +425,7 @@ func testTriageRun( obj := newTriageRun(TriageRunRequest{ Namespace: "wandb", ApplicationName: applicationName, - Actions: []string{"default"}, + Actions: []TriageActionReference{{Name: "default"}}, }) obj.SetName(name) obj.SetGenerateName("") From 308df61a30ee2e38722e2ddc0a65c5ec5820b125 Mon Sep 17 00:00:00 2001 From: Aravind Warrier Date: Tue, 4 Aug 2026 12:39:16 -0400 Subject: [PATCH 3/5] fix(operator): normalize triage action errors --- pkg/operator/triage.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/operator/triage.go b/pkg/operator/triage.go index af98222..a89b14b 100644 --- a/pkg/operator/triage.go +++ b/pkg/operator/triage.go @@ -256,24 +256,24 @@ func listTriageActions( actionMap, ok := item.(map[string]any) if !ok { return nil, fmt.Errorf( - "Application %s/%s triage action %d is %T, want object", + "application %s/%s triage action %d is %T, want object", namespace, applicationName, i, item) } name, _, nameErr := unstructured.NestedString(actionMap, "name") if nameErr != nil { return nil, fmt.Errorf( - "Application %s/%s triage action %d has invalid name: %w", + "application %s/%s triage action %d has invalid name: %w", namespace, applicationName, i, nameErr) } if strings.TrimSpace(name) == "" { return nil, fmt.Errorf( - "Application %s/%s triage action %d has an empty name", + "application %s/%s triage action %d has an empty name", namespace, applicationName, i) } description, _, descriptionErr := unstructured.NestedString(actionMap, "description") if descriptionErr != nil { return nil, fmt.Errorf( - "Application %s/%s triage action %q has invalid description: %w", + "application %s/%s triage action %q has invalid description: %w", namespace, applicationName, name, descriptionErr) } actions = append(actions, TriageAction{Name: name, Description: description}) @@ -290,7 +290,7 @@ func listTriageActions( } default: return nil, fmt.Errorf( - "Application %s/%s triage actions are %T, want array", namespace, applicationName, rawActions) + "application %s/%s triage actions are %T, want array", namespace, applicationName, rawActions) } sort.Slice(actions, func(i, j int) bool { return actions[i].Name < actions[j].Name }) return actions, nil From d29db415a4affb1eaf8a0eec36bdeae1d2922981 Mon Sep 17 00:00:00 2001 From: Aravind Warrier Date: Thu, 6 Aug 2026 12:31:54 -0400 Subject: [PATCH 4/5] test(operator): cover triage action compatibility --- pkg/operator/triage_test.go | 59 +++++++++++++++++++++++++++++++++++-- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/pkg/operator/triage_test.go b/pkg/operator/triage_test.go index efcaf45..ee23a04 100644 --- a/pkg/operator/triage_test.go +++ b/pkg/operator/triage_test.go @@ -87,6 +87,30 @@ func TestCreateTriageRunPreservesExplicitActions(t *testing.T) { } } +func TestCreateTriageRunPreservesLegacyAction(t *testing.T) { + t.Parallel() + + client := fake.NewSimpleDynamicClient(runtime.NewScheme()) + client.PrependReactor("create", "triageruns", func(action k8stesting.Action) (bool, runtime.Object, error) { + obj := action.(k8stesting.CreateAction).GetObject().(*unstructured.Unstructured).DeepCopy() + got, _, _ := parseTriageActionReferences(obj.Object, "spec", "actions") + if len(got) != 1 || got[0].Name != "dependencies" { + t.Fatalf("actions = %#v, want [dependencies]", got) + } + obj.SetName("weave-trace-triage-legacy") + return true, obj, nil + }) + + _, err := createTriageRun(context.Background(), client, TriageRunRequest{ + Namespace: "wandb", + ApplicationName: "weave-trace", + Action: "dependencies", + }) + if err != nil { + t.Fatalf("create TriageRun: %v", err) + } +} + func TestCreateTriageRunValidatesRequestBeforeCallingCluster(t *testing.T) { t.Parallel() @@ -258,6 +282,12 @@ func TestListTriageRunsFiltersSortsAndParsesStatus(t *testing.T) { "Succeeded", "2026-07-28T20:00:00Z", ) + if err := unstructured.SetNestedSlice(newer.Object, []any{ + map[string]any{"name": "default"}, + map[string]any{"name": "deep"}, + }, "spec", "actions"); err != nil { + t.Fatalf("set actions: %v", err) + } if err := unstructured.SetNestedMap(newer.Object, map[string]any{ "phase": "Failed", "startedAt": "2026-07-28T19:00:01Z", @@ -288,6 +318,21 @@ func TestListTriageRunsFiltersSortsAndParsesStatus(t *testing.T) { }, }, }, + map[string]any{ + "action": "deep", + "phase": "Failed", + "jobRef": map[string]any{"name": "weave-trace-triage-newer-triage-1"}, + "summary": map[string]any{"total": int64(1), "fail": int64(1)}, + "results": []any{ + map[string]any{ + "name": "kafka-reachable", + "umbrella": "kafka", + "severity": "fail", + "message": "connection refused", + "durationMs": int64(8), + }, + }, + }, }, }, "status"); err != nil { t.Fatalf("set status: %v", err) @@ -310,11 +355,19 @@ func TestListTriageRunsFiltersSortsAndParsesStatus(t *testing.T) { if runs[0].Summary == nil || runs[0].Summary.OverallSeverity != "fail" || runs[0].Summary.Total != 2 { t.Fatalf("summary = %#v", runs[0].Summary) } - if len(runs[0].Results) != 1 || runs[0].Results[0].Name != "clickhouse-reachable" { + if len(runs[0].Actions) != 2 || runs[0].Actions[0].Name != "default" || + runs[0].Actions[1].Name != "deep" || runs[0].Action != "" { + t.Fatalf("actions = %#v, legacy action = %q", runs[0].Actions, runs[0].Action) + } + if len(runs[0].Results) != 2 || runs[0].Results[0].Name != "clickhouse-reachable" || + runs[0].Results[1].Name != "kafka-reachable" { t.Fatalf("results = %#v", runs[0].Results) } - if len(runs[0].ActionStatuses) != 1 || - runs[0].ActionStatuses[0].JobName != "weave-trace-triage-newer-triage-0" { + if len(runs[0].ActionStatuses) != 2 || + runs[0].ActionStatuses[0].Action != "default" || + runs[0].ActionStatuses[0].JobName != "weave-trace-triage-newer-triage-0" || + runs[0].ActionStatuses[1].Action != "deep" || + runs[0].ActionStatuses[1].JobName != "weave-trace-triage-newer-triage-1" { t.Fatalf("action statuses = %#v", runs[0].ActionStatuses) } evidence, ok := runs[0].Results[0].Evidence.(map[string]any) From 14b555e66a68531092521d2cf1882485159bfdbd Mon Sep 17 00:00:00 2001 From: Aravind Warrier Date: Mon, 10 Aug 2026 11:08:53 -0400 Subject: [PATCH 5/5] refactor(operator): remove singular triage action API --- pkg/operator/triage.go | 146 +++++++++++++++--------------------- pkg/operator/triage_test.go | 124 ++++++++++++++++++++++-------- 2 files changed, 153 insertions(+), 117 deletions(-) diff --git a/pkg/operator/triage.go b/pkg/operator/triage.go index a89b14b..90ee0e1 100644 --- a/pkg/operator/triage.go +++ b/pkg/operator/triage.go @@ -60,10 +60,6 @@ type TriageRunRequest struct { Namespace string `json:"namespace"` ApplicationName string `json:"applicationName"` Actions []TriageActionReference `json:"actions,omitempty"` - - // Action is retained for source compatibility with clients of the initial - // single-action SDK. New callers should use Actions; setting both is invalid. - Action string `json:"action,omitempty"` } // TriageRunRef identifies the newly created TriageRun. It is intentionally @@ -125,14 +121,12 @@ type TriageRun struct { Name string `json:"name"` ApplicationName string `json:"applicationName"` Actions []TriageActionReference `json:"actions"` - // Action mirrors the selected action for legacy single-action consumers. - Action string `json:"action,omitempty"` - Phase string `json:"phase,omitempty"` - CreatedAt string `json:"createdAt,omitempty"` - StartedAt string `json:"startedAt,omitempty"` - CompletedAt string `json:"completedAt,omitempty"` - Summary *TriageRunSummary `json:"summary,omitempty"` - ActionStatuses []TriageActionStatus `json:"actionStatuses,omitempty"` + Phase string `json:"phase,omitempty"` + CreatedAt string `json:"createdAt,omitempty"` + StartedAt string `json:"startedAt,omitempty"` + CompletedAt string `json:"completedAt,omitempty"` + Summary *TriageRunSummary `json:"summary,omitempty"` + ActionStatuses []TriageActionStatus `json:"actionStatuses,omitempty"` // Results is a flattened compatibility view of all action results. Results []TriageCheckResult `json:"results,omitempty"` } @@ -149,13 +143,13 @@ func CreateTriageRun(ctx context.Context, request TriageRunRequest) (TriageRunRe } // ListTriageActions returns the sorted action metadata declared by one -// Application. +// Application for Watchtower and other SDK consumers. func ListTriageActions(ctx context.Context, namespace, applicationName string) ([]TriageAction, error) { _, dynamicClient, err := kubectl.GetDynamicClientset() if err != nil { return nil, err } - return listTriageActions(ctx, dynamicClient, namespace, applicationName) + return listTriageActionsWithClient(ctx, dynamicClient, namespace, applicationName) } // ListTriageRuns returns newest-first run history in one namespace. Supplying @@ -219,7 +213,7 @@ func createTriageRun( }, nil } -func listTriageActions( +func listTriageActionsWithClient( ctx context.Context, dynamicClient dynamic.Interface, namespace string, @@ -248,50 +242,38 @@ func listTriageActions( if !found { return []TriageAction{}, nil } - actions := []TriageAction{} - switch value := rawActions.(type) { - case []any: - actions = make([]TriageAction, 0, len(value)) - for i, item := range value { - actionMap, ok := item.(map[string]any) - if !ok { - return nil, fmt.Errorf( - "application %s/%s triage action %d is %T, want object", - namespace, applicationName, i, item) - } - name, _, nameErr := unstructured.NestedString(actionMap, "name") - if nameErr != nil { - return nil, fmt.Errorf( - "application %s/%s triage action %d has invalid name: %w", - namespace, applicationName, i, nameErr) - } - if strings.TrimSpace(name) == "" { - return nil, fmt.Errorf( - "application %s/%s triage action %d has an empty name", - namespace, applicationName, i) - } - description, _, descriptionErr := unstructured.NestedString(actionMap, "description") - if descriptionErr != nil { - return nil, fmt.Errorf( - "application %s/%s triage action %q has invalid description: %w", - namespace, applicationName, name, descriptionErr) - } - actions = append(actions, TriageAction{Name: name, Description: description}) - } - case map[string]any: - // Read the original map-shaped catalog during rolling upgrades. - actions = make([]TriageAction, 0, len(value)) - for name, item := range value { - description := "" - if actionMap, ok := item.(map[string]any); ok { - description, _, _ = unstructured.NestedString(actionMap, "description") - } - actions = append(actions, TriageAction{Name: name, Description: description}) - } - default: + actionItems, ok := rawActions.([]any) + if !ok { return nil, fmt.Errorf( "application %s/%s triage actions are %T, want array", namespace, applicationName, rawActions) } + actions := make([]TriageAction, 0, len(actionItems)) + for i, item := range actionItems { + actionMap, ok := item.(map[string]any) + if !ok { + return nil, fmt.Errorf( + "application %s/%s triage action %d is %T, want object", + namespace, applicationName, i, item) + } + name, _, nameErr := unstructured.NestedString(actionMap, "name") + if nameErr != nil { + return nil, fmt.Errorf( + "application %s/%s triage action %d has invalid name: %w", + namespace, applicationName, i, nameErr) + } + if strings.TrimSpace(name) == "" { + return nil, fmt.Errorf( + "application %s/%s triage action %d has an empty name", + namespace, applicationName, i) + } + description, _, descriptionErr := unstructured.NestedString(actionMap, "description") + if descriptionErr != nil { + return nil, fmt.Errorf( + "application %s/%s triage action %q has invalid description: %w", + namespace, applicationName, name, descriptionErr) + } + actions = append(actions, TriageAction{Name: name, Description: description}) + } sort.Slice(actions, func(i, j int) bool { return actions[i].Name < actions[j].Name }) return actions, nil } @@ -414,15 +396,9 @@ func parseTriageRun(obj *unstructured.Unstructured) (TriageRun, error) { return TriageRun{}, fmt.Errorf("failed to read TriageRun %s/%s actions: %w", obj.GetNamespace(), obj.GetName(), err) } if !foundActions { - legacyAction, _, legacyErr := unstructured.NestedString(obj.Object, "spec", "action") - if legacyErr != nil { - return TriageRun{}, fmt.Errorf( - "failed to read TriageRun %s/%s legacy action: %w", obj.GetNamespace(), obj.GetName(), legacyErr) - } - if legacyAction == "" { - legacyAction = DefaultTriageAction - } - actions = []TriageActionReference{{Name: legacyAction}} + return TriageRun{}, fmt.Errorf( + "failed to read TriageRun %s/%s actions: field is required", + obj.GetNamespace(), obj.GetName()) } phase, _, err := unstructured.NestedString(obj.Object, "status", "phase") if err != nil { @@ -451,9 +427,6 @@ func parseTriageRun(obj *unstructured.Unstructured) (TriageRun, error) { StartedAt: startedAt, CompletedAt: completedAt, } - if len(actions) == 1 { - run.Action = actions[0].Name - } if creationTimestamp.IsZero() { run.CreatedAt = "" } @@ -511,21 +484,23 @@ func parseTriageActionReferences( if err != nil || !found { return nil, found, err } + if len(items) == 0 { + return nil, true, errors.New("actions must contain at least one action") + } actions := make([]TriageActionReference, 0, len(items)) for i, item := range items { - switch value := item.(type) { - case map[string]any: - name, _, nameErr := unstructured.NestedString(value, "name") - if nameErr != nil { - return nil, true, fmt.Errorf("action %d name: %w", i, nameErr) - } - actions = append(actions, TriageActionReference{Name: name}) - case string: - // Read the original string-shaped selection during rolling upgrades. - actions = append(actions, TriageActionReference{Name: value}) - default: + actionMap, ok := item.(map[string]any) + if !ok { return nil, true, fmt.Errorf("action %d is %T, want object", i, item) } + name, _, nameErr := unstructured.NestedString(actionMap, "name") + if nameErr != nil { + return nil, true, fmt.Errorf("action %d name: %w", i, nameErr) + } + if strings.TrimSpace(name) == "" { + return nil, true, fmt.Errorf("action %d name must not be empty", i) + } + actions = append(actions, TriageActionReference{Name: name}) } return actions, true, nil } @@ -647,8 +622,8 @@ func validateTriageRunRequest(request TriageRunRequest) error { if err := validateTriageApplicationName(request.ApplicationName); err != nil { return err } - if request.Action != "" && len(request.Actions) > 0 { - return errors.New("set either action or actions, not both") + if request.Actions != nil && len(request.Actions) == 0 { + return errors.New("actions must contain at least one action") } seen := make(map[string]struct{}) for i, action := range normalizedTriageActions(request) { @@ -664,13 +639,10 @@ func validateTriageRunRequest(request TriageRunRequest) error { } func normalizedTriageActions(request TriageRunRequest) []TriageActionReference { - if len(request.Actions) > 0 { - return append([]TriageActionReference(nil), request.Actions...) - } - if request.Action != "" { - return []TriageActionReference{{Name: request.Action}} + if request.Actions == nil { + return []TriageActionReference{{Name: DefaultTriageAction}} } - return []TriageActionReference{{Name: DefaultTriageAction}} + return append([]TriageActionReference(nil), request.Actions...) } func validateTriageRunIdentity(namespace, name string) error { diff --git a/pkg/operator/triage_test.go b/pkg/operator/triage_test.go index ee23a04..133a57a 100644 --- a/pkg/operator/triage_test.go +++ b/pkg/operator/triage_test.go @@ -87,30 +87,6 @@ func TestCreateTriageRunPreservesExplicitActions(t *testing.T) { } } -func TestCreateTriageRunPreservesLegacyAction(t *testing.T) { - t.Parallel() - - client := fake.NewSimpleDynamicClient(runtime.NewScheme()) - client.PrependReactor("create", "triageruns", func(action k8stesting.Action) (bool, runtime.Object, error) { - obj := action.(k8stesting.CreateAction).GetObject().(*unstructured.Unstructured).DeepCopy() - got, _, _ := parseTriageActionReferences(obj.Object, "spec", "actions") - if len(got) != 1 || got[0].Name != "dependencies" { - t.Fatalf("actions = %#v, want [dependencies]", got) - } - obj.SetName("weave-trace-triage-legacy") - return true, obj, nil - }) - - _, err := createTriageRun(context.Background(), client, TriageRunRequest{ - Namespace: "wandb", - ApplicationName: "weave-trace", - Action: "dependencies", - }) - if err != nil { - t.Fatalf("create TriageRun: %v", err) - } -} - func TestCreateTriageRunValidatesRequestBeforeCallingCluster(t *testing.T) { t.Parallel() @@ -140,12 +116,20 @@ func TestCreateTriageRunValidatesRequestBeforeCallingCluster(t *testing.T) { want: "invalid Application name", }, { - name: "action and actions conflict", + name: "explicit empty actions", + request: TriageRunRequest{ + Namespace: "wandb", ApplicationName: "weave-trace", + Actions: []TriageActionReference{}, + }, + want: "at least one action", + }, + { + name: "whitespace action name", request: TriageRunRequest{ Namespace: "wandb", ApplicationName: "weave-trace", - Action: "default", Actions: []TriageActionReference{{Name: "deep"}}, + Actions: []TriageActionReference{{Name: " "}}, }, - want: "either action or actions", + want: "must not be empty", }, { name: "duplicate actions", @@ -237,7 +221,7 @@ func TestListTriageActionsReturnsSortedApplicationActions(t *testing.T) { }) client := fake.NewSimpleDynamicClient(runtime.NewScheme(), application) - actions, err := listTriageActions(context.Background(), client, "wandb", "weave-trace") + actions, err := listTriageActionsWithClient(context.Background(), client, "wandb", "weave-trace") if err != nil { t.Fatalf("list triage actions: %v", err) } @@ -247,6 +231,86 @@ func TestListTriageActionsReturnsSortedApplicationActions(t *testing.T) { } } +func TestListTriageActionsRejectsInvalidCatalogs(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + rawActions any + want string + }{ + { + name: "map shaped catalog", + rawActions: map[string]any{ + "default": map[string]any{"description": "Run standard diagnostics"}, + }, + want: "want array", + }, + { + name: "invalid description", + rawActions: []any{ + map[string]any{"name": "default", "description": int64(1)}, + }, + want: "invalid description", + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + application := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "apps.wandb.com/v2", + "kind": "Application", + "metadata": map[string]any{ + "name": "weave-trace", + "namespace": "wandb", + }, + "spec": map[string]any{ + "triage": map[string]any{"actions": test.rawActions}, + }, + }} + application.SetGroupVersionKind(schema.GroupVersionKind{ + Group: applicationsV2GVR.Group, Version: applicationsV2GVR.Version, Kind: "Application", + }) + client := fake.NewSimpleDynamicClient(runtime.NewScheme(), application) + + _, err := listTriageActionsWithClient(context.Background(), client, "wandb", "weave-trace") + if err == nil || !strings.Contains(err.Error(), test.want) { + t.Fatalf("error = %v, want containing %q", err, test.want) + } + }) + } +} + +func TestParseTriageActionReferencesRejectsInvalidSelections(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + rawActions any + }{ + {name: "empty array", rawActions: []any{}}, + {name: "empty object", rawActions: []any{map[string]any{}}}, + {name: "empty string", rawActions: ""}, + {name: "string item", rawActions: []any{"default"}}, + {name: "whitespace name", rawActions: []any{map[string]any{"name": " "}}}, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + object := map[string]any{ + "spec": map[string]any{"actions": test.rawActions}, + } + if _, _, err := parseTriageActionReferences(object, "spec", "actions"); err == nil { + t.Fatalf("parse actions %#v: expected error", test.rawActions) + } + }) + } +} + func TestNewTriageRunHasNamespacedMetadata(t *testing.T) { t.Parallel() @@ -356,8 +420,8 @@ func TestListTriageRunsFiltersSortsAndParsesStatus(t *testing.T) { t.Fatalf("summary = %#v", runs[0].Summary) } if len(runs[0].Actions) != 2 || runs[0].Actions[0].Name != "default" || - runs[0].Actions[1].Name != "deep" || runs[0].Action != "" { - t.Fatalf("actions = %#v, legacy action = %q", runs[0].Actions, runs[0].Action) + runs[0].Actions[1].Name != "deep" { + t.Fatalf("actions = %#v", runs[0].Actions) } if len(runs[0].Results) != 2 || runs[0].Results[0].Name != "clickhouse-reachable" || runs[0].Results[1].Name != "kafka-reachable" {