merge backlog: worktree-agent-aca0d858dec169a39

internal/provider/selectel/selectel.go

@@ -106,6 +106,10 @@ func (c *Client) ListZones(ctx context.Context, creds provider.Credentials) ([]p
 		if page.NextOffset == 0 || len(page.Result) == 0 {
 			break
 		}
+		if page.NextOffset <= offset {
+			// API returned a non-advancing offset; stop instead of looping forever.
+			break
+		}
 		offset = page.NextOffset
 	}
 	return zones, nil
@@ -136,6 +140,10 @@ func (c *Client) listRRSets(ctx context.Context, token, zoneID string) ([]apiRRS
 		if page.NextOffset == 0 || len(page.Result) == 0 {
 			break
 		}
+		if page.NextOffset <= offset {
+			// API returned a non-advancing offset; stop instead of looping forever.
+			break
+		}
 		offset = page.NextOffset
 	}
 	return all, nil
@@ -192,6 +200,12 @@ func (c *Client) ApplyChanges(ctx context.Context, creds provider.Credentials, z
 	return nil
 }
 
+// NOTE: disabled-записи не участвуют в diff и не сохраняются при apply, т.к. PATCH
+// заменяет весь набор records; поддержка отключённых записей — вне текущей фазы.
+// Selectel PATCH /rrset/{id} полностью заменяет набор records: он не мержит переданные
+// значения с уже существующими. toRecord отбрасывает disabled-записи при чтении, а
+// toRRSet никогда их не восстанавливает при записи — поэтому round-trip (read -> diff
+// -> apply) безвозвратно теряет ранее disabled-записи в rrset, если он был изменён.
 func toRecord(rr apiRRSet) model.Record {
 	vals := make([]string, 0, len(rr.Records))
 	for _, r := range rr.Records {
@@ -203,6 +217,10 @@ func toRecord(rr apiRRSet) model.Record {
 	return model.Record{Type: model.RecordType(rr.Type), Name: rr.Name, TTL: rr.TTL, Values: vals}
 }
 
+// NOTE: disabled-записи не участвуют в diff и не сохраняются при apply, т.к. PATCH
+// заменяет весь набор records; поддержка отключённых записей — вне текущей фазы.
+// toRRSet всегда шлёт записи без поля disabled, т.е. любой ранее disabled контент,
+// отсутствующий в model.Record.Values, будет молча потерян при следующем PATCH этого rrset.
 func toRRSet(rec model.Record) apiRRSet {
 	rs := apiRRSet{Name: rec.Name, Type: string(rec.Type), TTL: rec.TTL}
 	for _, v := range rec.Values {

internal/provider/selectel/selectel_test.go

@@ -7,6 +7,7 @@ import (
 	"net/http/httptest"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/vasyakrg/dns-autoresolver/internal/diff"
 	"github.com/vasyakrg/dns-autoresolver/internal/model"
@@ -344,6 +345,109 @@ func TestApplyChangesAddWithNilDesiredReturnsErrorNoPanic(t *testing.T) {
 	}
 }
 
+// Global Constraint: New() must configure the default base URL and a 30s HTTP timeout.
+func TestNewDefaults(t *testing.T) {
+	c := New()
+	if c.BaseURL != DefaultBaseURL {
+		t.Fatalf("BaseURL = %q, want %q", c.BaseURL, DefaultBaseURL)
+	}
+	if c.HTTP == nil {
+		t.Fatal("HTTP client must not be nil")
+	}
+	if c.HTTP.Timeout != 30*time.Second {
+		t.Fatalf("HTTP.Timeout = %v, want %v", c.HTTP.Timeout, 30*time.Second)
+	}
+}
+
+// Global Constraint: if the API returns a NextOffset that does not advance past the
+// current offset (stuck or regressing), even with a non-empty page, ListZones must
+// stop pagination instead of looping forever on the same/earlier offset.
+func TestListZonesStopsOnNonAdvancingNextOffset(t *testing.T) {
+	var requests int
+	c, srv := newTestClient(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		requests++
+		if requests > 5 {
+			t.Fatalf("too many requests, possible infinite pagination loop on stuck offset")
+		}
+		offset := r.URL.Query().Get("offset")
+		switch offset {
+		case "0":
+			json.NewEncoder(w).Encode(map[string]any{
+				"result":      []map[string]any{{"id": "z1", "name": "first.example.com."}},
+				"next_offset": 1000,
+			})
+		case "1000":
+			// Buggy/malicious API: next_offset does not advance past current offset,
+			// yet the page is non-empty.
+			json.NewEncoder(w).Encode(map[string]any{
+				"result":      []map[string]any{{"id": "z2", "name": "second.example.com."}},
+				"next_offset": 1000,
+			})
+		default:
+			t.Fatalf("unexpected offset %q", offset)
+		}
+	}))
+	defer srv.Close()
+
+	zs, err := c.ListZones(context.Background(), creds())
+	if err != nil {
+		t.Fatal(err)
+	}
+	if requests != 2 {
+		t.Fatalf("expected exactly 2 requests before stopping on stuck offset, got %d", requests)
+	}
+	if len(zs) != 2 || zs[0].ID != "z1" || zs[1].ID != "z2" {
+		t.Fatalf("expected zones accumulated from both pages before stopping, got %+v", zs)
+	}
+}
+
+// Global Constraint: same guard applies to listRRSets (exercised via GetRecords) —
+// a non-advancing NextOffset on a non-empty page must stop pagination, not loop forever.
+func TestGetRecordsStopsOnNonAdvancingNextOffset(t *testing.T) {
+	var requests int
+	c, srv := newTestClient(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		requests++
+		if requests > 5 {
+			t.Fatalf("too many requests, possible infinite pagination loop on stuck offset")
+		}
+		offset := r.URL.Query().Get("offset")
+		switch offset {
+		case "0":
+			json.NewEncoder(w).Encode(map[string]any{
+				"result": []map[string]any{
+					{"id": "r1", "name": "a.example.com.", "type": "A", "ttl": 300,
+						"records": []map[string]any{{"content": "1.1.1.1"}}},
+				},
+				"next_offset": 1000,
+			})
+		case "1000":
+			// Buggy/malicious API: next_offset regresses below current offset,
+			// yet the page is non-empty.
+			json.NewEncoder(w).Encode(map[string]any{
+				"result": []map[string]any{
+					{"id": "r2", "name": "b.example.com.", "type": "A", "ttl": 300,
+						"records": []map[string]any{{"content": "2.2.2.2"}}},
+				},
+				"next_offset": 500,
+			})
+		default:
+			t.Fatalf("unexpected offset %q", offset)
+		}
+	}))
+	defer srv.Close()
+
+	recs, err := c.GetRecords(context.Background(), creds(), "z1")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if requests != 2 {
+		t.Fatalf("expected exactly 2 requests before stopping on stuck/regressing offset, got %d", requests)
+	}
+	if len(recs) != 2 {
+		t.Fatalf("expected records accumulated from both pages before stopping, got %+v", recs)
+	}
+}
+
 // Global Constraint: HTTP errors (status >= 300) must surface a non-nil error whose text
 // includes the method/path/status (or response body) for diagnosability.
 func TestListZonesHTTPErrorIncludesMethodPathStatus(t *testing.T) {