fix(api): manual check persists last_check_status (was stale unknown)
Manual domain checks (Recheck button / diff page load) never wrote domains.last_check_status - only the scheduler did, leaving a newly-templated domain stuck at "unknown" until the next scheduled run. Extract status derivation into internal/service (single source of truth): StatusUnknown/InSync/Drift/Error constants and DeriveStatus(diff.Changeset). The scheduler now aliases these constants instead of duplicating them. handleCheck persists the derived status (or StatusError on failure) via TenantStore.SetDomainStatus after every manual check - status/history only, no notification, which remains the scheduler's job. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BwxdSt4reTm7Dj1oxRvpP3
This commit is contained in:
@@ -4,6 +4,7 @@ import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
@@ -20,9 +21,20 @@ type mockCheckApplier struct {
|
||||
lastReq service.ApplyRequest
|
||||
zoneRecords []model.Record
|
||||
zoneErr error
|
||||
|
||||
// checkCS/checkErr, when set, override Check's default fixed changeset —
|
||||
// used by handleCheck status-persistence tests (drift/in_sync/error).
|
||||
checkCS *diff.Changeset
|
||||
checkErr error
|
||||
}
|
||||
|
||||
func (m *mockCheckApplier) Check(context.Context, uuid.UUID, uuid.UUID) (diff.Changeset, error) {
|
||||
if m.checkErr != nil {
|
||||
return diff.Changeset{}, m.checkErr
|
||||
}
|
||||
if m.checkCS != nil {
|
||||
return *m.checkCS, nil
|
||||
}
|
||||
d := model.Record{Type: model.A, Name: "a.example.com.", TTL: 300, Values: []string{"1.1.1.1"}}
|
||||
return diff.Changeset{Diffs: []diff.RecordDiff{{Kind: diff.Add, Type: d.Type, Name: d.Name, Desired: &d}}}, nil
|
||||
}
|
||||
@@ -44,7 +56,10 @@ func (m *mockCheckApplier) ZoneRecords(context.Context, uuid.UUID, uuid.UUID) ([
|
||||
// middleware_test.go's own tests and the IDOR regression.
|
||||
func newTestAPI() (*API, *mockCheckApplier) {
|
||||
m := &mockCheckApplier{}
|
||||
return &API{Svc: m, Auth: alwaysOwnedAuthStore(), Sessions: alwaysValidSessions(uuid.New())}, m
|
||||
return &API{
|
||||
Svc: m, Store: &mockTenantStore{},
|
||||
Auth: alwaysOwnedAuthStore(), Sessions: alwaysValidSessions(uuid.New()),
|
||||
}, m
|
||||
}
|
||||
|
||||
func TestCheckEndpoint(t *testing.T) {
|
||||
@@ -137,6 +152,77 @@ func TestApplyBadUUID(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestCheckEndpoint_PersistsDriftStatus covers the core bug fix: a manual
|
||||
// check (GET .../check) whose changeset has an actionable prune must persist
|
||||
// "drift" via Store.SetDomainStatus — previously only the scheduler wrote
|
||||
// last_check_status, leaving a manually-checked domain stuck at "unknown".
|
||||
func TestCheckEndpoint_PersistsDriftStatus(t *testing.T) {
|
||||
a, m := newTestAPI()
|
||||
ts := a.Store.(*mockTenantStore)
|
||||
rec := model.Record{Type: model.A, Name: "x.example.com.", Values: []string{"1.1.1.1"}}
|
||||
m.checkCS = &diff.Changeset{Diffs: []diff.RecordDiff{{Kind: diff.Delete, Type: rec.Type, Name: rec.Name, Actual: &rec}}}
|
||||
router := NewRouter(a)
|
||||
|
||||
did := uuid.New()
|
||||
req := requestWithSessionCookie(http.MethodGet,
|
||||
"/api/v1/projects/00000000-0000-0000-0000-000000000002/domains/"+did.String()+"/check", nil)
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("status %d body %s", w.Code, w.Body.String())
|
||||
}
|
||||
if len(ts.statusCalls) != 1 || ts.statusCalls[0].domainID != did || ts.statusCalls[0].status != service.StatusDrift {
|
||||
t.Fatalf("expected SetDomainStatus(%s, drift), got %+v", did, ts.statusCalls)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCheckEndpoint_PersistsInSyncStatus covers the no-drift case: a
|
||||
// changeset with no actionable diffs persists "in_sync".
|
||||
func TestCheckEndpoint_PersistsInSyncStatus(t *testing.T) {
|
||||
a, m := newTestAPI()
|
||||
ts := a.Store.(*mockTenantStore)
|
||||
m.checkCS = &diff.Changeset{}
|
||||
router := NewRouter(a)
|
||||
|
||||
did := uuid.New()
|
||||
req := requestWithSessionCookie(http.MethodGet,
|
||||
"/api/v1/projects/00000000-0000-0000-0000-000000000002/domains/"+did.String()+"/check", nil)
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("status %d body %s", w.Code, w.Body.String())
|
||||
}
|
||||
if len(ts.statusCalls) != 1 || ts.statusCalls[0].status != service.StatusInSync {
|
||||
t.Fatalf("expected SetDomainStatus(_, in_sync), got %+v", ts.statusCalls)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCheckEndpoint_ErrorPersistsErrorStatus covers the failure path: when
|
||||
// Svc.Check itself fails (provider/loader error), the handler must persist
|
||||
// "error" before returning 500 — a write failure of the status itself must
|
||||
// not mask the original 500.
|
||||
func TestCheckEndpoint_ErrorPersistsErrorStatus(t *testing.T) {
|
||||
a, m := newTestAPI()
|
||||
ts := a.Store.(*mockTenantStore)
|
||||
m.checkErr = errors.New("boom: provider unreachable")
|
||||
router := NewRouter(a)
|
||||
|
||||
did := uuid.New()
|
||||
req := requestWithSessionCookie(http.MethodGet,
|
||||
"/api/v1/projects/00000000-0000-0000-0000-000000000002/domains/"+did.String()+"/check", nil)
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusInternalServerError {
|
||||
t.Fatalf("expected 500, got %d body %s", w.Code, w.Body.String())
|
||||
}
|
||||
if len(ts.statusCalls) != 1 || ts.statusCalls[0].domainID != did || ts.statusCalls[0].status != service.StatusError {
|
||||
t.Fatalf("expected SetDomainStatus(%s, error), got %+v", did, ts.statusCalls)
|
||||
}
|
||||
}
|
||||
|
||||
// TestChangesetResponseEmptyMarshalsToArrays guards the белый-экран bug: an
|
||||
// empty changeset (zone matches its template exactly, e.g. right after a
|
||||
// snapshot) must marshal updates/prunes/readOnly as [] not null — a nil slice
|
||||
|
||||
Reference in New Issue
Block a user