From 3382f62f5735706ee07535fd4198d36eaa5b754d Mon Sep 17 00:00:00 2001 From: Pierre-Olivier Mercier Date: Fri, 24 Apr 2026 16:58:16 +0700 Subject: [PATCH] checker: thread rule states into ReportContext Reporters can now read rule output via ctx.States() instead of re-deriving severity/hints from the raw payload, keeping the rules screen and the HTML report aligned on a single source of truth. --- checker/server/interactive.go | 7 +- checker/server/interactive_test.go | 59 +++++++++++++++++ checker/server/server.go | 4 +- checker/server/server_test.go | 101 +++++++++++++++++++++++++++++ checker/types.go | 57 ++++++++++------ checker/types_test.go | 49 ++++++++++++++ 6 files changed, 252 insertions(+), 25 deletions(-) diff --git a/checker/server/interactive.go b/checker/server/interactive.go index 2ac133c..27cc260 100644 --- a/checker/server/interactive.go +++ b/checker/server/interactive.go @@ -96,14 +96,15 @@ func (s *Server) handleCheckForm(w http.ResponseWriter, r *http.Request) { } func (s *Server) handleCheckSubmit(w http.ResponseWriter, r *http.Request) { + fields := s.interactive.RenderForm() if err := r.ParseForm(); err != nil { - s.renderCheckForm(w, s.interactive.RenderForm(), fmt.Sprintf("invalid form: %v", err)) + s.renderCheckForm(w, fields, fmt.Sprintf("invalid form: %v", err)) return } opts, err := s.interactive.ParseForm(r) if err != nil { - s.renderCheckForm(w, s.interactive.RenderForm(), err.Error()) + s.renderCheckForm(w, fields, err.Error()) return } @@ -135,7 +136,7 @@ func (s *Server) handleCheckSubmit(w http.ResponseWriter, r *http.Request) { result.States = s.evaluateRules(r.Context(), obs, opts, nil) } - ctx := checker.NewReportContext(raw, related) + ctx := checker.NewReportContext(raw, related, result.States) if reporter, ok := s.provider.(checker.CheckerHTMLReporter); ok { html, rerr := reporter.GetHTMLReport(ctx) diff --git a/checker/server/interactive_test.go b/checker/server/interactive_test.go index bc1d338..4ce7ade 100644 --- a/checker/server/interactive_test.go +++ b/checker/server/interactive_test.go @@ -355,6 +355,65 @@ func TestCheck_Submit_RunsSiblingAndExposesRelated(t *testing.T) { } } +// interactiveStatesPeekingProvider implements Interactive + HTMLReporter +// and captures the ReportContext.States() seen at GetHTMLReport time. +type interactiveStatesPeekingProvider struct { + key checker.ObservationKey + def *checker.CheckerDefinition + seen *[]checker.CheckState +} + +func (p *interactiveStatesPeekingProvider) Key() checker.ObservationKey { return p.key } +func (p *interactiveStatesPeekingProvider) Collect(ctx context.Context, opts checker.CheckerOptions) (any, error) { + return map[string]string{"ok": "1"}, nil +} +func (p *interactiveStatesPeekingProvider) Definition() *checker.CheckerDefinition { return p.def } +func (p *interactiveStatesPeekingProvider) RenderForm() []checker.CheckerOptionField { + return []checker.CheckerOptionField{{Id: "domain", Type: "string"}} +} +func (p *interactiveStatesPeekingProvider) ParseForm(r *http.Request) (checker.CheckerOptions, error) { + return checker.CheckerOptions{"domain": r.FormValue("domain")}, nil +} +func (p *interactiveStatesPeekingProvider) GetHTMLReport(ctx checker.ReportContext) (string, error) { + if p.seen != nil { + *p.seen = ctx.States() + } + return "

ok

", nil +} + +// TestCheck_Submit_ThreadsStatesIntoReport verifies that CheckStates +// produced by evaluateRules during POST /check are threaded into the +// ReportContext handed to GetHTMLReport. Without this wiring, the /check +// UI can show states in its own section but the embedded report would +// have to re-derive severity/hints from Data. +func TestCheck_Submit_ThreadsStatesIntoReport(t *testing.T) { + var seen []checker.CheckState + p := &interactiveStatesPeekingProvider{ + key: "test", + def: &checker.CheckerDefinition{ + ID: "test", + Rules: []checker.CheckRule{&dummyRule{name: "rule1", desc: "first"}}, + }, + seen: &seen, + } + srv := New(p) + defer srv.Close() + + rec := postForm(srv.Handler(), "/check", url.Values{"domain": {"example.com"}}) + if rec.Code != http.StatusOK { + t.Fatalf("POST /check = %d, want 200", rec.Code) + } + if len(seen) != 1 { + t.Fatalf("reporter saw %d states, want 1", len(seen)) + } + if seen[0].RuleName != "rule1" { + t.Errorf("state RuleName = %q, want %q", seen[0].RuleName, "rule1") + } + if seen[0].Status != checker.StatusOK { + t.Errorf("state Status = %v, want %v", seen[0].Status, checker.StatusOK) + } +} + func TestCheck_Submit_NoSibling_LeavesRelatedEmpty(t *testing.T) { p := &interactiveProvider{ testProvider: &testProvider{ diff --git a/checker/server/server.go b/checker/server/server.go index d875dc2..26f50ee 100644 --- a/checker/server/server.go +++ b/checker/server/server.go @@ -357,7 +357,7 @@ func (s *Server) handleReport(w http.ResponseWriter, r *http.Request) { return } - html, err := reporter.GetHTMLReport(checker.NewReportContext(req.Data, req.Related)) + html, err := reporter.GetHTMLReport(checker.NewReportContext(req.Data, req.Related, req.States)) if err != nil { http.Error(w, fmt.Sprintf("failed to generate HTML report: %v", err), http.StatusInternalServerError) return @@ -375,7 +375,7 @@ func (s *Server) handleReport(w http.ResponseWriter, r *http.Request) { return } - metrics, err := reporter.ExtractMetrics(checker.NewReportContext(req.Data, req.Related), time.Now()) + metrics, err := reporter.ExtractMetrics(checker.NewReportContext(req.Data, req.Related, req.States), time.Now()) if err != nil { writeJSON(w, http.StatusInternalServerError, map[string]string{ "error": fmt.Sprintf("failed to extract metrics: %v", err), diff --git a/checker/server/server_test.go b/checker/server/server_test.go index 03aa58f..9e0adc4 100644 --- a/checker/server/server_test.go +++ b/checker/server/server_test.go @@ -542,6 +542,107 @@ func (p *relatedPeekingProvider) GetHTMLReport(ctx checker.ReportContext) (strin return "

ok

", nil } +// statesPeekingProvider captures the ReportContext's States slice at +// GetHTMLReport / ExtractMetrics time. +type statesPeekingProvider struct { + base *testProvider + htmlSeen *[]checker.CheckState + metricSeen *[]checker.CheckState +} + +func (p *statesPeekingProvider) Key() checker.ObservationKey { return p.base.Key() } +func (p *statesPeekingProvider) Collect(ctx context.Context, opts checker.CheckerOptions) (any, error) { + return p.base.Collect(ctx, opts) +} +func (p *statesPeekingProvider) Definition() *checker.CheckerDefinition { return p.base.definition } +func (p *statesPeekingProvider) GetHTMLReport(ctx checker.ReportContext) (string, error) { + if p.htmlSeen != nil { + *p.htmlSeen = ctx.States() + } + return "

ok

", nil +} +func (p *statesPeekingProvider) ExtractMetrics(ctx checker.ReportContext, t time.Time) ([]checker.CheckMetric, error) { + if p.metricSeen != nil { + *p.metricSeen = ctx.States() + } + return []checker.CheckMetric{{Name: "m1", Value: 1.0, Timestamp: t}}, nil +} + +// TestServer_Report_States_HTML verifies ExternalReportRequest.States is +// threaded into the ReportContext seen by the HTML reporter. +func TestServer_Report_States_HTML(t *testing.T) { + var seen []checker.CheckState + base := &testProvider{ + key: "test", + definition: &checker.CheckerDefinition{ID: "test-checker", Rules: []checker.CheckRule{}}, + } + srv := New(&statesPeekingProvider{base: base, htmlSeen: &seen}) + defer srv.Close() + + states := []checker.CheckState{ + {Status: checker.StatusCrit, Message: "broken", RuleName: "r1", Code: "bad", Subject: "host.example"}, + } + req := checker.ExternalReportRequest{ + Key: "test", + Data: json.RawMessage(`{}`), + States: states, + } + rec := doRequest(srv.Handler(), "POST", "/report", req, map[string]string{"Accept": "text/html"}) + if rec.Code != http.StatusOK { + t.Fatalf("POST /report = %d, want 200", rec.Code) + } + if len(seen) != 1 || seen[0].RuleName != "r1" || seen[0].Code != "bad" || seen[0].Subject != "host.example" { + t.Errorf("reporter saw states = %+v, want single state {RuleName:r1, Code:bad, Subject:host.example}", seen) + } +} + +// TestServer_Report_States_Metrics verifies the States passthrough on the +// metrics path as well. +func TestServer_Report_States_Metrics(t *testing.T) { + var seen []checker.CheckState + base := &testProvider{ + key: "test", + definition: &checker.CheckerDefinition{ID: "test-checker", Rules: []checker.CheckRule{}}, + } + srv := New(&statesPeekingProvider{base: base, metricSeen: &seen}) + defer srv.Close() + + req := checker.ExternalReportRequest{ + Key: "test", + Data: json.RawMessage(`{}`), + States: []checker.CheckState{{Status: checker.StatusWarn, RuleName: "r1"}}, + } + rec := doRequest(srv.Handler(), "POST", "/report", req, map[string]string{"Accept": "application/json"}) + if rec.Code != http.StatusOK { + t.Fatalf("POST /report = %d, want 200", rec.Code) + } + if len(seen) != 1 || seen[0].RuleName != "r1" { + t.Errorf("reporter saw states = %+v, want single state with RuleName=r1", seen) + } +} + +// TestServer_Report_States_Absent verifies that omitting States in the +// request yields a nil States() slice on the reporter side (graceful +// degradation for hosts that don't thread evaluate→report yet). +func TestServer_Report_States_Absent(t *testing.T) { + seen := []checker.CheckState{{Status: checker.StatusOK}} // non-nil sentinel + base := &testProvider{ + key: "test", + definition: &checker.CheckerDefinition{ID: "test-checker", Rules: []checker.CheckRule{}}, + } + srv := New(&statesPeekingProvider{base: base, htmlSeen: &seen}) + defer srv.Close() + + req := checker.ExternalReportRequest{Key: "test", Data: json.RawMessage(`{}`)} + rec := doRequest(srv.Handler(), "POST", "/report", req, map[string]string{"Accept": "text/html"}) + if rec.Code != http.StatusOK { + t.Fatalf("POST /report = %d, want 200", rec.Code) + } + if seen != nil { + t.Errorf("States() = %+v, want nil when ExternalReportRequest.States is absent", seen) + } +} + func TestServer_Report_BadBody(t *testing.T) { p := &testProvider{ key: "test", diff --git a/checker/types.go b/checker/types.go index c218dbb..7256766 100644 --- a/checker/types.go +++ b/checker/types.go @@ -298,32 +298,40 @@ type CheckAggregator interface { Aggregate(states []CheckState) CheckState } -// ReportContext carries both the primary observation payload and any -// observations produced by other checkers that cover the same discovery -// entries. Hosts build a ReportContext and hand it to reporter methods. +// ReportContext carries the primary observation payload, any observations +// produced by other checkers that cover the same discovery entries, and the +// CheckStates produced by this checker's rules for the same observation. +// Hosts build a ReportContext and hand it to reporter methods. // -// The method set is deliberately tiny: a single primary payload (Data) and -// a query for related observations by key (Related). Hosts return nil from -// Related when there is nothing to relate; reporters must tolerate that. +// Reporters use States() to render rule-driven sections (for example a +// "fix these first" list) without re-deriving severity or hints from the +// raw payload. Hosts that have not yet threaded rule output into the +// report pipeline return nil; reporters must treat a nil or empty slice +// as "not provided" and fall back to a data-only rendering. The same +// nil-tolerance applies to Related(key). type ReportContext interface { Data() json.RawMessage Related(key ObservationKey) []RelatedObservation + States() []CheckState } -// NewReportContext returns a ReportContext backed by a primary payload and -// a pre-resolved map of related observations by key. The SDK's /report HTTP -// handler uses this to wrap ExternalReportRequest contents; hosts and tests -// can use it whenever they already have the related observations in memory. +// NewReportContext returns a ReportContext backed by a primary payload, a +// pre-resolved map of related observations by key, and the CheckStates +// produced by the checker's rules on this observation. The SDK's /report +// HTTP handler uses this to wrap ExternalReportRequest contents; hosts and +// tests can use it whenever they already have that material in memory. // -// Passing a nil or empty related map is fine; Related(key) will then return -// nil, just like StaticReportContext. -func NewReportContext(data json.RawMessage, related map[ObservationKey][]RelatedObservation) ReportContext { - return fixedReportContext{data: data, related: related} +// Passing a nil related map or a nil states slice is fine; Related(key) +// and States() will then return nil respectively. Use StaticReportContext +// as a shorthand when both are absent. +func NewReportContext(data json.RawMessage, related map[ObservationKey][]RelatedObservation, states []CheckState) ReportContext { + return fixedReportContext{data: data, related: related, states: states} } -// StaticReportContext is a shorthand for NewReportContext(data, nil): a -// ReportContext with a primary payload and no related observations. -// Intended for tests and ad-hoc callers that have no lineage to supply. +// StaticReportContext is a shorthand for NewReportContext(data, nil, nil): +// a ReportContext with a primary payload, no related observations, and no +// rule states. Intended for tests and ad-hoc callers that have no lineage +// or rule output to supply. func StaticReportContext(data json.RawMessage) ReportContext { return fixedReportContext{data: data} } @@ -331,6 +339,7 @@ func StaticReportContext(data json.RawMessage) ReportContext { type fixedReportContext struct { data json.RawMessage related map[ObservationKey][]RelatedObservation + states []CheckState } func (f fixedReportContext) Data() json.RawMessage { return f.data } @@ -340,6 +349,7 @@ func (f fixedReportContext) Related(key ObservationKey) []RelatedObservation { } return f.related[key] } +func (f fixedReportContext) States() []CheckState { return f.states } // CheckerHTMLReporter is an optional interface that observation providers can // implement to render their stored data as a full HTML document (for iframe embedding). @@ -486,13 +496,20 @@ type ExternalEvaluateResponse struct { // Related carries observations produced by other checkers on DiscoveryEntry // records originally published by the target of this report, that is, the // cross-checker lineage that ObservationGetter.GetRelated would expose in -// the in-process path. The host composes it before making the HTTP request; -// when absent, the remote checker receives a context that reports no -// related observations (equivalent to StaticReportContext). +// the in-process path. States carries the CheckStates the host produced by +// evaluating this checker's rules against the same observation, letting +// reporters render rule-driven sections (for example a "fix these first" +// list) without re-deriving severity or hints from Data. +// +// The host composes both fields before making the HTTP request. When both +// are absent, the remote checker receives a context equivalent to +// StaticReportContext (no related observations and no states); the +// reporter then falls back to a data-only rendering. type ExternalReportRequest struct { Key ObservationKey `json:"key"` Data json.RawMessage `json:"data"` Related map[ObservationKey][]RelatedObservation `json:"related,omitempty"` + States []CheckState `json:"states,omitempty"` } // HealthResponse is returned by GET /health on a remote checker endpoint. diff --git a/checker/types_test.go b/checker/types_test.go index 8ba89f2..87022ee 100644 --- a/checker/types_test.go +++ b/checker/types_test.go @@ -17,6 +17,7 @@ package checker import ( "context" "encoding/json" + "reflect" "testing" ) @@ -156,6 +157,54 @@ func TestCheckerDefinition_BuildRulesInfo(t *testing.T) { } } +// Compile-time check that fixedReportContext implements ReportContext. +var _ ReportContext = fixedReportContext{} + +func TestStaticReportContext_NoExtras(t *testing.T) { + ctx := StaticReportContext(json.RawMessage(`{"k":"v"}`)) + if string(ctx.Data()) != `{"k":"v"}` { + t.Errorf("Data() = %s, want %s", ctx.Data(), `{"k":"v"}`) + } + if ctx.Related("any") != nil { + t.Error("Related(any) should be nil for StaticReportContext") + } + if ctx.States() != nil { + t.Error("States() should be nil for StaticReportContext") + } +} + +func TestNewReportContext_NilStates(t *testing.T) { + ctx := NewReportContext(json.RawMessage(`{}`), nil, nil) + if ctx.States() != nil { + t.Errorf("States() = %v, want nil", ctx.States()) + } +} + +func TestNewReportContext_PassesStates(t *testing.T) { + states := []CheckState{ + {Status: StatusWarn, Message: "heads up", RuleName: "r1"}, + {Status: StatusCrit, Message: "fix me", RuleName: "r2", Subject: "host.example"}, + } + ctx := NewReportContext(json.RawMessage(`{}`), nil, states) + got := ctx.States() + if !reflect.DeepEqual(got, states) { + t.Errorf("States() = %+v, want %+v", got, states) + } +} + +func TestNewReportContext_PassesRelated(t *testing.T) { + rel := map[ObservationKey][]RelatedObservation{ + "other.key": {{CheckerID: "other", Key: "other.key", Ref: "r1"}}, + } + ctx := NewReportContext(json.RawMessage(`{}`), rel, nil) + if got := ctx.Related("other.key"); len(got) != 1 || got[0].CheckerID != "other" { + t.Errorf("Related(other.key) = %+v, want one entry with CheckerID=other", got) + } + if ctx.Related("missing") != nil { + t.Error("Related(missing) should be nil") + } +} + func TestRegisterChecker_EmptyIDRejected(t *testing.T) { resetRegistries() RegisterChecker(&CheckerDefinition{ID: "", Name: "bad"})