From 0b26923586fc99da9195bb3e9396378528715ba1 Mon Sep 17 00:00:00 2001 From: Vassiliy Yegorov Date: Sun, 5 Jul 2026 15:10:01 +0700 Subject: [PATCH] feat(apply): per-record selection + deletes-before-updates ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RecordDiff.Key() gives a stable normalized identifier ("TYPE name.") for every diff kind, exposed as recordView.Key. ApplyRequest now takes Updates/Prunes key lists instead of two booleans, so callers can apply a subset of records. service.Apply builds the applied set with selected prunes (Delete) added before selected updates (Add/Update) — an invariant, not an option — since the provider rejects an Add/Update whose name still conflicts with an existing record (e.g. a CNAME cannot be created while an A on the same name still exists). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BwxdSt4reTm7Dj1oxRvpP3 --- internal/api/api_test.go | 15 ++++--- internal/api/dto.go | 7 ++-- internal/api/handlers.go | 7 ++-- internal/diff/diff.go | 8 ++++ internal/diff/diff_test.go | 10 +++++ internal/service/service.go | 33 ++++++++++++---- internal/service/service_test.go | 68 +++++++++++++++++++++++--------- 7 files changed, 110 insertions(+), 38 deletions(-) diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 75f5e9b..89b3e27 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -84,12 +84,15 @@ func TestCheckEndpoint(t *testing.T) { } } -func TestApplyDefaultsPruneFalse(t *testing.T) { +// TestApplySendsSelectedKeys covers the per-record selection request shape: +// POST /apply with only "prunes" set must reach the service with that key in +// Prunes and an empty Updates. +func TestApplySendsSelectedKeys(t *testing.T) { a, m := newTestAPI() router := NewRouter(a) did := uuid.New().String() - body := `{"applyUpdates":true}` // applyPrunes отсутствует → false + body := `{"prunes":["A gitlocator.com."]}` req := requestWithSessionCookie(http.MethodPost, "/api/v1/projects/00000000-0000-0000-0000-000000000002/domains/"+did+"/apply", strings.NewReader(body)) @@ -99,7 +102,7 @@ func TestApplyDefaultsPruneFalse(t *testing.T) { if w.Code != http.StatusOK { t.Fatalf("status %d body %s", w.Code, w.Body.String()) } - if m.lastReq.ApplyPrunes != false || m.lastReq.ApplyUpdates != true { + if len(m.lastReq.Prunes) != 1 || m.lastReq.Prunes[0] != "A gitlocator.com." || len(m.lastReq.Updates) != 0 { t.Fatalf("apply request mismatch: %+v", m.lastReq) } } @@ -117,8 +120,8 @@ func TestApplyEmptyBodyOK(t *testing.T) { if w.Code != http.StatusOK { t.Fatalf("status %d body %s", w.Code, w.Body.String()) } - if m.lastReq.ApplyPrunes != false { - t.Fatalf("expected ApplyPrunes=false for empty body, got %+v", m.lastReq) + if len(m.lastReq.Updates) != 0 || len(m.lastReq.Prunes) != 0 { + t.Fatalf("expected empty Updates/Prunes for empty body, got %+v", m.lastReq) } } @@ -127,7 +130,7 @@ func TestApplyMalformedBody(t *testing.T) { router := NewRouter(a) did := uuid.New().String() - body := `{"applyUpdates":` + body := `{"updates":` req := requestWithSessionCookie(http.MethodPost, "/api/v1/projects/00000000-0000-0000-0000-000000000002/domains/"+did+"/apply", strings.NewReader(body)) diff --git a/internal/api/dto.go b/internal/api/dto.go index ddef2dd..6d5fa9f 100644 --- a/internal/api/dto.go +++ b/internal/api/dto.go @@ -40,11 +40,12 @@ func toAuthResponse(u store.User, p store.Project) authResponse { } type applyRequest struct { - ApplyUpdates bool `json:"applyUpdates"` - ApplyPrunes bool `json:"applyPrunes"` + Updates []string `json:"updates"` + Prunes []string `json:"prunes"` } type recordView struct { + Key string `json:"key"` Kind string `json:"kind"` Type string `json:"type"` Name string `json:"name"` @@ -61,7 +62,7 @@ type changesetResponse struct { } func toRecordView(d diff.RecordDiff) recordView { - rv := recordView{Kind: string(d.Kind), Type: string(d.Type), Name: d.Name, ReadOnly: d.ReadOnly} + rv := recordView{Key: d.Key(), Kind: string(d.Kind), Type: string(d.Type), Name: d.Name, ReadOnly: d.ReadOnly} if d.Desired != nil { rv.Desired = d.Desired.Values } diff --git a/internal/api/handlers.go b/internal/api/handlers.go index adcc435..a8491de 100644 --- a/internal/api/handlers.go +++ b/internal/api/handlers.go @@ -66,15 +66,16 @@ func (a *API) handleApply(w http.ResponseWriter, r *http.Request) { var req applyRequest if r.Body != nil { r.Body = http.MaxBytesReader(w, r.Body, 1<<20) // 1 MiB - // пустое тело допустимо → значения по умолчанию (prune=false); - // любая другая ошибка decode (битый JSON, неверные типы) → 400 + // пустое тело допустимо → значения по умолчанию (пустые списки, ничего + // не применяется); любая другая ошибка decode (битый JSON, неверные + // типы) → 400 if err := json.NewDecoder(r.Body).Decode(&req); err != nil && !errors.Is(err, io.EOF) { writeErr(w, http.StatusBadRequest, "invalid request body") return } } cs, err := a.Svc.Apply(r.Context(), pid, did, service.ApplyRequest{ - ApplyUpdates: req.ApplyUpdates, ApplyPrunes: req.ApplyPrunes, + Updates: req.Updates, Prunes: req.Prunes, }) if err != nil { log.Printf("api: apply failed: %v", err) diff --git a/internal/diff/diff.go b/internal/diff/diff.go index 78df864..e5dd15c 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -21,6 +21,14 @@ type RecordDiff struct { ReadOnly bool // NS/SOA — shown but never applied } +// Key is the stable identifier of the RRset this diff targets, normalised the +// same way as model.Record.Key ("TYPE name."). Used to select individual diffs +// for a partial apply. Works for every Kind (Delete has no Desired, Add has no +// Actual) because Type/Name are always populated. +func (d RecordDiff) Key() string { + return model.Record{Type: d.Type, Name: d.Name}.Key() +} + type Changeset struct { Diffs []RecordDiff } diff --git a/internal/diff/diff_test.go b/internal/diff/diff_test.go index 6a50200..baf90ac 100644 --- a/internal/diff/diff_test.go +++ b/internal/diff/diff_test.go @@ -22,6 +22,16 @@ func find(cs Changeset, key string) *RecordDiff { return nil } +// TestRecordDiffKeyNormalizes pins RecordDiff.Key() down to the same +// normalization as model.Record.Key() (lowercase + trailing dot), for a +// Delete diff (which has no Desired, only Type/Name populated directly). +func TestRecordDiffKeyNormalizes(t *testing.T) { + d := RecordDiff{Kind: Delete, Type: model.A, Name: "Mail.Example.COM"} + if got := d.Key(); got != "A mail.example.com." { + t.Fatalf("key: %q", got) + } +} + func TestDiffAddUpdateDeleteInSync(t *testing.T) { tmpl := []model.Record{ {Type: model.A, Name: "a.example.com.", TTL: 300, Values: []string{"1.1.1.1"}}, // in sync diff --git a/internal/service/service.go b/internal/service/service.go index a4662fa..80d8ca8 100644 --- a/internal/service/service.go +++ b/internal/service/service.go @@ -50,8 +50,8 @@ type Recorder interface { } type ApplyRequest struct { - ApplyUpdates bool - ApplyPrunes bool + Updates []string // record keys (RecordDiff.Key) to add/update + Prunes []string // record keys to delete } type DomainService struct { @@ -128,18 +128,29 @@ func (s *DomainService) ZoneRecords(ctx context.Context, projectID, domainID uui return recs, nil } -// Apply applies updates always (when ApplyUpdates) and prunes only when ApplyPrunes. +// Apply applies exactly the diffs whose keys are selected in req.Updates and +// req.Prunes. Selected prunes are added to the applied set BEFORE selected +// updates: deletes first is an invariant, not an option — the provider +// rejects an Add/Update whose name still has a conflicting record (e.g. a +// CNAME cannot be created while an A on the same name exists), so pruning the +// old records before applying updates avoids that. func (s *DomainService) Apply(ctx context.Context, projectID, domainID uuid.UUID, req ApplyRequest) (diff.Changeset, error) { p, creds, ref, cs, err := s.resolve(ctx, projectID, domainID) if err != nil { return diff.Changeset{}, err } + selPrunes := toSet(req.Prunes) + selUpdates := toSet(req.Updates) var toApply []diff.RecordDiff - if req.ApplyUpdates { - toApply = append(toApply, cs.Updates()...) + for _, d := range cs.Prunes() { + if selPrunes[d.Key()] { + toApply = append(toApply, d) + } } - if req.ApplyPrunes { - toApply = append(toApply, cs.Prunes()...) + for _, d := range cs.Updates() { + if selUpdates[d.Key()] { + toApply = append(toApply, d) + } } applied := diff.Changeset{Diffs: toApply} if len(toApply) > 0 { @@ -149,3 +160,11 @@ func (s *DomainService) Apply(ctx context.Context, projectID, domainID uuid.UUID } return applied, nil } + +func toSet(keys []string) map[string]bool { + m := make(map[string]bool, len(keys)) + for _, k := range keys { + m[k] = true + } + return m +} diff --git a/internal/service/service_test.go b/internal/service/service_test.go index 4bcea46..69f080a 100644 --- a/internal/service/service_test.go +++ b/internal/service/service_test.go @@ -125,39 +125,69 @@ func TestZoneRecordsReadsProviderDirectly(t *testing.T) { } } -func TestApplyRespectsPruneGuard(t *testing.T) { - // зона содержит лишнюю запись b (нет в шаблоне) → Prune-кандидат +// TestApplySelectsByKeyAndOrdersPrunesBeforeUpdates covers the two goals of +// selective apply: (1) only diffs whose key is present in the request are +// applied, and (2) when both an update and a prune are selected, the prune +// (Delete) must land BEFORE the update in the applied Changeset — this is the +// regression guard for the provider rejecting an Add/Update whose name still +// conflicts with an existing record (e.g. a CNAME cannot be created while an +// A on the same name still exists). +func TestApplySelectsByKeyAndOrdersPrunesBeforeUpdates(t *testing.T) { + // zone: a needs updating (9.9.9.9 -> 1.1.1.1), b is an extra record not in + // the template (prune candidate). actual := []model.Record{ - {Type: model.A, Name: "a.example.com.", TTL: 300, Values: []string{"1.1.1.1"}}, + {Type: model.A, Name: "a.example.com.", TTL: 300, Values: []string{"9.9.9.9"}}, {Type: model.A, Name: "b.example.com.", TTL: 300, Values: []string{"2.2.2.2"}}, } tmpl := dto.TemplateDoc{Records: []dto.RecordDTO{ - {Type: "A", Name: "a.example.com.", TTL: 300, Values: []string{"1.1.1.1"}}, // in sync + {Type: "A", Name: "a.example.com.", TTL: 300, Values: []string{"1.1.1.1"}}, // update }} - // applyPrunes=false → удаление b НЕ применяется + const updKey = "A a.example.com." + const pruneKey = "A b.example.com." + + // Only the prune selected -> only the delete is applied. svc, fp := setup(t, actual, tmpl) - if _, err := svc.Apply(context.Background(), uuid.New(), uuid.New(), ApplyRequest{ApplyUpdates: true, ApplyPrunes: false}); err != nil { + if _, err := svc.Apply(context.Background(), uuid.New(), uuid.New(), ApplyRequest{Prunes: []string{pruneKey}}); err != nil { t.Fatal(err) } - for _, d := range fp.applied.Diffs { - if d.Kind == diff.Delete { - t.Fatalf("prune must be skipped when ApplyPrunes=false, applied: %+v", fp.applied.Diffs) - } + if len(fp.applied.Diffs) != 1 || fp.applied.Diffs[0].Kind != diff.Delete { + t.Fatalf("expected only the selected prune applied, got %+v", fp.applied.Diffs) } - // applyPrunes=true → удаление b применяется + // Only the update selected -> only the update is applied. svc2, fp2 := setup(t, actual, tmpl) - if _, err := svc2.Apply(context.Background(), uuid.New(), uuid.New(), ApplyRequest{ApplyUpdates: true, ApplyPrunes: true}); err != nil { + if _, err := svc2.Apply(context.Background(), uuid.New(), uuid.New(), ApplyRequest{Updates: []string{updKey}}); err != nil { t.Fatal(err) } - var sawDelete bool - for _, d := range fp2.applied.Diffs { - if d.Kind == diff.Delete && d.Name == "b.example.com." { - sawDelete = true - } + if len(fp2.applied.Diffs) != 1 || fp2.applied.Diffs[0].Kind != diff.Update { + t.Fatalf("expected only the selected update applied, got %+v", fp2.applied.Diffs) } - if !sawDelete { - t.Fatalf("prune must be applied when ApplyPrunes=true, applied: %+v", fp2.applied.Diffs) + + // Nothing selected -> nothing applied. + svc3, fp3 := setup(t, actual, tmpl) + if _, err := svc3.Apply(context.Background(), uuid.New(), uuid.New(), ApplyRequest{}); err != nil { + t.Fatal(err) + } + if len(fp3.applied.Diffs) != 0 { + t.Fatalf("expected nothing applied when nothing is selected, got %+v", fp3.applied.Diffs) + } + + // CRITICAL: both selected -> the prune (Delete) must be applied FIRST, + // the update SECOND. Regressing this order reintroduces the + // CNAME/A-conflict bug where the provider rejects the update because the + // stale conflicting record hasn't been deleted yet. + svc4, fp4 := setup(t, actual, tmpl) + if _, err := svc4.Apply(context.Background(), uuid.New(), uuid.New(), ApplyRequest{Updates: []string{updKey}, Prunes: []string{pruneKey}}); err != nil { + t.Fatal(err) + } + if len(fp4.applied.Diffs) != 2 { + t.Fatalf("expected both selected diffs applied, got %+v", fp4.applied.Diffs) + } + if fp4.applied.Diffs[0].Kind != diff.Delete { + t.Fatalf("expected prune (Delete) FIRST in applied order, got %+v", fp4.applied.Diffs) + } + if fp4.applied.Diffs[1].Kind != diff.Update { + t.Fatalf("expected update SECOND in applied order, got %+v", fp4.applied.Diffs) } }