feat(apply): per-record selection + deletes-before-updates ordering
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwxdSt4reTm7Dj1oxRvpP3
This commit is contained in:
@@ -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()
|
a, m := newTestAPI()
|
||||||
router := NewRouter(a)
|
router := NewRouter(a)
|
||||||
|
|
||||||
did := uuid.New().String()
|
did := uuid.New().String()
|
||||||
body := `{"applyUpdates":true}` // applyPrunes отсутствует → false
|
body := `{"prunes":["A gitlocator.com."]}`
|
||||||
req := requestWithSessionCookie(http.MethodPost,
|
req := requestWithSessionCookie(http.MethodPost,
|
||||||
"/api/v1/projects/00000000-0000-0000-0000-000000000002/domains/"+did+"/apply",
|
"/api/v1/projects/00000000-0000-0000-0000-000000000002/domains/"+did+"/apply",
|
||||||
strings.NewReader(body))
|
strings.NewReader(body))
|
||||||
@@ -99,7 +102,7 @@ func TestApplyDefaultsPruneFalse(t *testing.T) {
|
|||||||
if w.Code != http.StatusOK {
|
if w.Code != http.StatusOK {
|
||||||
t.Fatalf("status %d body %s", w.Code, w.Body.String())
|
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)
|
t.Fatalf("apply request mismatch: %+v", m.lastReq)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -117,8 +120,8 @@ func TestApplyEmptyBodyOK(t *testing.T) {
|
|||||||
if w.Code != http.StatusOK {
|
if w.Code != http.StatusOK {
|
||||||
t.Fatalf("status %d body %s", w.Code, w.Body.String())
|
t.Fatalf("status %d body %s", w.Code, w.Body.String())
|
||||||
}
|
}
|
||||||
if m.lastReq.ApplyPrunes != false {
|
if len(m.lastReq.Updates) != 0 || len(m.lastReq.Prunes) != 0 {
|
||||||
t.Fatalf("expected ApplyPrunes=false for empty body, got %+v", m.lastReq)
|
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)
|
router := NewRouter(a)
|
||||||
|
|
||||||
did := uuid.New().String()
|
did := uuid.New().String()
|
||||||
body := `{"applyUpdates":`
|
body := `{"updates":`
|
||||||
req := requestWithSessionCookie(http.MethodPost,
|
req := requestWithSessionCookie(http.MethodPost,
|
||||||
"/api/v1/projects/00000000-0000-0000-0000-000000000002/domains/"+did+"/apply",
|
"/api/v1/projects/00000000-0000-0000-0000-000000000002/domains/"+did+"/apply",
|
||||||
strings.NewReader(body))
|
strings.NewReader(body))
|
||||||
|
|||||||
+4
-3
@@ -40,11 +40,12 @@ func toAuthResponse(u store.User, p store.Project) authResponse {
|
|||||||
}
|
}
|
||||||
|
|
||||||
type applyRequest struct {
|
type applyRequest struct {
|
||||||
ApplyUpdates bool `json:"applyUpdates"`
|
Updates []string `json:"updates"`
|
||||||
ApplyPrunes bool `json:"applyPrunes"`
|
Prunes []string `json:"prunes"`
|
||||||
}
|
}
|
||||||
|
|
||||||
type recordView struct {
|
type recordView struct {
|
||||||
|
Key string `json:"key"`
|
||||||
Kind string `json:"kind"`
|
Kind string `json:"kind"`
|
||||||
Type string `json:"type"`
|
Type string `json:"type"`
|
||||||
Name string `json:"name"`
|
Name string `json:"name"`
|
||||||
@@ -61,7 +62,7 @@ type changesetResponse struct {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func toRecordView(d diff.RecordDiff) recordView {
|
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 {
|
if d.Desired != nil {
|
||||||
rv.Desired = d.Desired.Values
|
rv.Desired = d.Desired.Values
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -66,15 +66,16 @@ func (a *API) handleApply(w http.ResponseWriter, r *http.Request) {
|
|||||||
var req applyRequest
|
var req applyRequest
|
||||||
if r.Body != nil {
|
if r.Body != nil {
|
||||||
r.Body = http.MaxBytesReader(w, r.Body, 1<<20) // 1 MiB
|
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) {
|
if err := json.NewDecoder(r.Body).Decode(&req); err != nil && !errors.Is(err, io.EOF) {
|
||||||
writeErr(w, http.StatusBadRequest, "invalid request body")
|
writeErr(w, http.StatusBadRequest, "invalid request body")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
cs, err := a.Svc.Apply(r.Context(), pid, did, service.ApplyRequest{
|
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 {
|
if err != nil {
|
||||||
log.Printf("api: apply failed: %v", err)
|
log.Printf("api: apply failed: %v", err)
|
||||||
|
|||||||
@@ -21,6 +21,14 @@ type RecordDiff struct {
|
|||||||
ReadOnly bool // NS/SOA — shown but never applied
|
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 {
|
type Changeset struct {
|
||||||
Diffs []RecordDiff
|
Diffs []RecordDiff
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -22,6 +22,16 @@ func find(cs Changeset, key string) *RecordDiff {
|
|||||||
return nil
|
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) {
|
func TestDiffAddUpdateDeleteInSync(t *testing.T) {
|
||||||
tmpl := []model.Record{
|
tmpl := []model.Record{
|
||||||
{Type: model.A, Name: "a.example.com.", TTL: 300, Values: []string{"1.1.1.1"}}, // in sync
|
{Type: model.A, Name: "a.example.com.", TTL: 300, Values: []string{"1.1.1.1"}}, // in sync
|
||||||
|
|||||||
@@ -50,8 +50,8 @@ type Recorder interface {
|
|||||||
}
|
}
|
||||||
|
|
||||||
type ApplyRequest struct {
|
type ApplyRequest struct {
|
||||||
ApplyUpdates bool
|
Updates []string // record keys (RecordDiff.Key) to add/update
|
||||||
ApplyPrunes bool
|
Prunes []string // record keys to delete
|
||||||
}
|
}
|
||||||
|
|
||||||
type DomainService struct {
|
type DomainService struct {
|
||||||
@@ -128,18 +128,29 @@ func (s *DomainService) ZoneRecords(ctx context.Context, projectID, domainID uui
|
|||||||
return recs, nil
|
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) {
|
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)
|
p, creds, ref, cs, err := s.resolve(ctx, projectID, domainID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return diff.Changeset{}, err
|
return diff.Changeset{}, err
|
||||||
}
|
}
|
||||||
|
selPrunes := toSet(req.Prunes)
|
||||||
|
selUpdates := toSet(req.Updates)
|
||||||
var toApply []diff.RecordDiff
|
var toApply []diff.RecordDiff
|
||||||
if req.ApplyUpdates {
|
for _, d := range cs.Prunes() {
|
||||||
toApply = append(toApply, cs.Updates()...)
|
if selPrunes[d.Key()] {
|
||||||
|
toApply = append(toApply, d)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
if req.ApplyPrunes {
|
for _, d := range cs.Updates() {
|
||||||
toApply = append(toApply, cs.Prunes()...)
|
if selUpdates[d.Key()] {
|
||||||
|
toApply = append(toApply, d)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
applied := diff.Changeset{Diffs: toApply}
|
applied := diff.Changeset{Diffs: toApply}
|
||||||
if len(toApply) > 0 {
|
if len(toApply) > 0 {
|
||||||
@@ -149,3 +160,11 @@ func (s *DomainService) Apply(ctx context.Context, projectID, domainID uuid.UUID
|
|||||||
}
|
}
|
||||||
return applied, nil
|
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
|
||||||
|
}
|
||||||
|
|||||||
@@ -125,39 +125,69 @@ func TestZoneRecordsReadsProviderDirectly(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestApplyRespectsPruneGuard(t *testing.T) {
|
// TestApplySelectsByKeyAndOrdersPrunesBeforeUpdates covers the two goals of
|
||||||
// зона содержит лишнюю запись b (нет в шаблоне) → Prune-кандидат
|
// 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{
|
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"}},
|
{Type: model.A, Name: "b.example.com.", TTL: 300, Values: []string{"2.2.2.2"}},
|
||||||
}
|
}
|
||||||
tmpl := dto.TemplateDoc{Records: []dto.RecordDTO{
|
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)
|
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)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
for _, d := range fp.applied.Diffs {
|
if len(fp.applied.Diffs) != 1 || fp.applied.Diffs[0].Kind != diff.Delete {
|
||||||
if d.Kind == diff.Delete {
|
t.Fatalf("expected only the selected prune applied, got %+v", fp.applied.Diffs)
|
||||||
t.Fatalf("prune must be skipped when ApplyPrunes=false, applied: %+v", fp.applied.Diffs)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// applyPrunes=true → удаление b применяется
|
// Only the update selected -> only the update is applied.
|
||||||
svc2, fp2 := setup(t, actual, tmpl)
|
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)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
var sawDelete bool
|
if len(fp2.applied.Diffs) != 1 || fp2.applied.Diffs[0].Kind != diff.Update {
|
||||||
for _, d := range fp2.applied.Diffs {
|
t.Fatalf("expected only the selected update applied, got %+v", fp2.applied.Diffs)
|
||||||
if d.Kind == diff.Delete && d.Name == "b.example.com." {
|
|
||||||
sawDelete = true
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
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)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user