From d387cd629b37766d2212d0c2320d1dd88f8d16ef Mon Sep 17 00:00:00 2001 From: Pierre-Olivier Mercier Date: Fri, 29 May 2026 21:07:45 +0800 Subject: [PATCH] checker: add CheckEnabler interface for data-driven eligibility Add an optional CheckEnabler interface that lets a provider decide, from the actual target data, whether running the checker is meaningful at all (e.g. reverse-zone outside in-addr.arpa, delegation without DNSSEC). The result is folded into the POST /definition response via new Eligible and EligibilityReason fields, and the handler now tracks load since IsEligible may perform I/O. --- checker/server/server.go | 19 ++++++- checker/server/server_test.go | 104 ++++++++++++++++++++++++++++++++++ checker/types.go | 37 ++++++++++++ 3 files changed, 157 insertions(+), 3 deletions(-) diff --git a/checker/server/server.go b/checker/server/server.go index 5075b04..c6c43f2 100644 --- a/checker/server/server.go +++ b/checker/server/server.go @@ -133,7 +133,7 @@ func New(provider checker.ObservationProvider) *Server { s.definition = def s.definition.BuildRulesInfo() s.mux.HandleFunc("GET /definition", s.handleDefinition) - s.mux.HandleFunc("POST /definition", s.handlePrecheck) + s.mux.Handle("POST /definition", s.TrackWork(http.HandlerFunc(s.handlePrecheck))) s.mux.Handle("POST /evaluate", s.TrackWork(http.HandlerFunc(s.handleEvaluate))) } } @@ -343,10 +343,23 @@ func (s *Server) handlePrecheck(w http.ResponseWriter, r *http.Request) { failures[rule.Name()] = err.Error() } } - writeJSON(w, http.StatusOK, checker.RulePrecheckResponse{ + resp := checker.RulePrecheckResponse{ CheckerDefinition: s.definition, PrecheckFailures: failures, - }) + } + if en, ok := s.provider.(checker.CheckEnabler); ok { + eligible, reason, err := en.IsEligible(r.Context(), req.Options) + if err != nil { + // Eligibility undetermined: leave Eligible nil so the host fails + // open (shows the checker), but surface the error for diagnostics. + log.Printf("IsEligible failed: %v", err) + resp.EligibilityReason = err.Error() + } else { + resp.Eligible = &eligible + resp.EligibilityReason = reason + } + } + writeJSON(w, http.StatusOK, resp) } func (s *Server) handleCollect(w http.ResponseWriter, r *http.Request) { diff --git a/checker/server/server_test.go b/checker/server/server_test.go index 17bbd12..d6feefa 100644 --- a/checker/server/server_test.go +++ b/checker/server/server_test.go @@ -687,6 +687,110 @@ func (r *prereqRule) Precheck(ctx context.Context, opts checker.CheckerOptions) return nil } +// enablerProvider is a minimal ObservationProvider + CheckerDefinitionProvider +// that also implements CheckEnabler, returning whatever isEligibleFn yields. +type enablerProvider struct { + key checker.ObservationKey + definition *checker.CheckerDefinition + isEligibleFn func(ctx context.Context, opts checker.CheckerOptions) (bool, string, error) +} + +func (p *enablerProvider) Key() checker.ObservationKey { return p.key } +func (p *enablerProvider) Collect(ctx context.Context, opts checker.CheckerOptions) (any, error) { + return map[string]string{"result": "ok"}, nil +} +func (p *enablerProvider) Definition() *checker.CheckerDefinition { return p.definition } +func (p *enablerProvider) IsEligible(ctx context.Context, opts checker.CheckerOptions) (bool, string, error) { + return p.isEligibleFn(ctx, opts) +} + +func TestServer_Precheck_Eligibility(t *testing.T) { + tests := []struct { + name string + fn func(ctx context.Context, opts checker.CheckerOptions) (bool, string, error) + wantNil bool // expect Eligible == nil + wantElig bool // value of *Eligible when not nil + wantReason string // expected EligibilityReason + }{ + { + name: "eligible true", + fn: func(context.Context, checker.CheckerOptions) (bool, string, error) { return true, "", nil }, + wantElig: true, + }, + { + name: "eligible false with reason", + fn: func(context.Context, checker.CheckerOptions) (bool, string, error) { return false, "not a reverse zone", nil }, + wantElig: false, + wantReason: "not a reverse zone", + }, + { + name: "error fails open", + fn: func(context.Context, checker.CheckerOptions) (bool, string, error) { return false, "", errors.New("lookup timeout") }, + wantNil: true, + wantReason: "lookup timeout", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + p := &enablerProvider{ + key: "test", + definition: &checker.CheckerDefinition{ID: "test", Rules: []checker.CheckRule{}}, + isEligibleFn: tc.fn, + } + srv := New(p) + defer srv.Close() + + rec := doRequest(srv.Handler(), "POST", "/definition", checker.RulePrecheckRequest{Options: checker.CheckerOptions{}}, nil) + if rec.Code != http.StatusOK { + t.Fatalf("POST /definition = %d, want %d", rec.Code, http.StatusOK) + } + var resp checker.RulePrecheckResponse + if err := json.NewDecoder(rec.Body).Decode(&resp); err != nil { + t.Fatalf("decode: %v", err) + } + if tc.wantNil { + if resp.Eligible != nil { + t.Errorf("Eligible = %v, want nil", *resp.Eligible) + } + } else { + if resp.Eligible == nil { + t.Fatalf("Eligible = nil, want %v", tc.wantElig) + } + if *resp.Eligible != tc.wantElig { + t.Errorf("Eligible = %v, want %v", *resp.Eligible, tc.wantElig) + } + } + if resp.EligibilityReason != tc.wantReason { + t.Errorf("EligibilityReason = %q, want %q", resp.EligibilityReason, tc.wantReason) + } + }) + } +} + +// TestServer_Precheck_NoEnabler verifies that a provider not implementing +// CheckEnabler yields no eligibility fields (Eligible nil, reason empty). +func TestServer_Precheck_NoEnabler(t *testing.T) { + p := &testProvider{key: "test", definition: &checker.CheckerDefinition{ID: "test", Rules: []checker.CheckRule{}}} + srv := newTestServer(p) + defer srv.Close() + + rec := doRequest(srv.Handler(), "POST", "/definition", checker.RulePrecheckRequest{Options: checker.CheckerOptions{}}, nil) + if rec.Code != http.StatusOK { + t.Fatalf("POST /definition = %d, want %d", rec.Code, http.StatusOK) + } + if bytes.Contains(rec.Body.Bytes(), []byte("eligible")) { + t.Errorf("response leaked eligible field for non-enabler provider: %s", rec.Body.String()) + } + var resp checker.RulePrecheckResponse + if err := json.NewDecoder(rec.Body).Decode(&resp); err != nil { + t.Fatalf("decode: %v", err) + } + if resp.Eligible != nil { + t.Errorf("Eligible = %v, want nil for non-enabler provider", *resp.Eligible) + } +} + func TestServer_Precheck(t *testing.T) { gated := &prereqRule{name: "gated", optKey: "api_key", msg: "missing API key"} open := &dummyRule{name: "open", desc: "no prereq"} diff --git a/checker/types.go b/checker/types.go index f32d8d8..ed4a9fc 100644 --- a/checker/types.go +++ b/checker/types.go @@ -262,6 +262,31 @@ type RulePrecheck interface { Precheck(ctx context.Context, opts CheckerOptions) error } +// CheckEnabler is an optional interface an ObservationProvider can implement +// to declare, from the actual target data, whether running this checker is +// meaningful at all. +// +// It complements the two existing gates: +// - CheckerAvailability is a static, registration-time scope/service-type +// filter; it never sees the target's data. +// - RulePrecheck is a per-rule, options-only check ("missing API key"). +// +// CheckEnabler is whole-checker and data-driven. IsEligible receives the same +// CheckerOptions as Collect, including the autofilled domain_name / zone / +// service payloads (read them with GetOption), and may perform light I/O +// (e.g. a DNSKEY lookup) to decide. +// +// Return (true, "", nil) to run the checker, or (false, reason, nil) with a +// short human-readable reason ("not a reverse zone", "DNSSEC not enabled") +// to skip it. Return a non-nil error only when eligibility could not be +// determined (transient I/O failure); the host treats that as "unknown" and +// fails open (shows the checker) rather than as a definitive skip. +// +// Detect support with a type assertion: _, ok := provider.(CheckEnabler) +type CheckEnabler interface { + IsEligible(ctx context.Context, opts CheckerOptions) (eligible bool, reason string, err error) +} + // RulePrecheckRequest is the body accepted by POST /definition. type RulePrecheckRequest struct { Options CheckerOptions `json:"options"` @@ -276,6 +301,18 @@ type RulePrecheckRequest struct { type RulePrecheckResponse struct { *CheckerDefinition PrecheckFailures map[string]string `json:"precheck_failures"` + + // Eligible reports whether this checker is meaningful for the submitted + // target, as decided by the provider's CheckEnabler (if implemented). It + // is nil when the checker does not implement CheckEnabler, or when + // IsEligible could not determine eligibility (its error was non-nil). A + // non-nil false means the checker is definitively not applicable to this + // target; the host should hide it unless Eligible != nil && !*Eligible. + Eligible *bool `json:"eligible,omitempty"` + + // EligibilityReason explains a false Eligible, or carries the lookup error + // message when eligibility could not be determined. Empty otherwise. + EligibilityReason string `json:"eligibility_reason,omitempty"` } // ObservationGetter provides access to observation data (used by CheckRule).