harden(selectel): защита пагинации от неподвижного offset, тест New, документирование disabled

This commit is contained in:
2026-07-03 13:13:24 +07:00
parent 9de5d4712c
commit 70f9bc6793
2 changed files with 122 additions and 0 deletions
+18
View File
@@ -106,6 +106,10 @@ func (c *Client) ListZones(ctx context.Context, creds provider.Credentials) ([]p
if page.NextOffset == 0 || len(page.Result) == 0 { if page.NextOffset == 0 || len(page.Result) == 0 {
break break
} }
if page.NextOffset <= offset {
// API returned a non-advancing offset; stop instead of looping forever.
break
}
offset = page.NextOffset offset = page.NextOffset
} }
return zones, nil 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 { if page.NextOffset == 0 || len(page.Result) == 0 {
break break
} }
if page.NextOffset <= offset {
// API returned a non-advancing offset; stop instead of looping forever.
break
}
offset = page.NextOffset offset = page.NextOffset
} }
return all, nil return all, nil
@@ -192,6 +200,12 @@ func (c *Client) ApplyChanges(ctx context.Context, creds provider.Credentials, z
return nil 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 { func toRecord(rr apiRRSet) model.Record {
vals := make([]string, 0, len(rr.Records)) vals := make([]string, 0, len(rr.Records))
for _, r := range 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} 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 { func toRRSet(rec model.Record) apiRRSet {
rs := apiRRSet{Name: rec.Name, Type: string(rec.Type), TTL: rec.TTL} rs := apiRRSet{Name: rec.Name, Type: string(rec.Type), TTL: rec.TTL}
for _, v := range rec.Values { for _, v := range rec.Values {
+104
View File
@@ -7,6 +7,7 @@ import (
"net/http/httptest" "net/http/httptest"
"strings" "strings"
"testing" "testing"
"time"
"github.com/vasyakrg/dns-autoresolver/internal/diff" "github.com/vasyakrg/dns-autoresolver/internal/diff"
"github.com/vasyakrg/dns-autoresolver/internal/model" "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 // Global Constraint: HTTP errors (status >= 300) must surface a non-nil error whose text
// includes the method/path/status (or response body) for diagnosability. // includes the method/path/status (or response body) for diagnosability.
func TestListZonesHTTPErrorIncludesMethodPathStatus(t *testing.T) { func TestListZonesHTTPErrorIncludesMethodPathStatus(t *testing.T) {