carlos / platform Public

carlos ratelimit set: read settings from the record, not the counts (#358)

set read each host's current settings through the rate-limit GET, which
also sums every box's usage day objects and fails when any is
unreadable. A broken usage object, even one on an unrelated box,
therefore blocked a settings change.

set now reads the settings it overlays from the instance GET, whose
record carries rate_limit as stored (a record without it is the
defaults). show keeps the counts-bearing GET and still fails explicitly
on a statistics error. No new words: errors reuse the existing ones.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude 14 hours ago a4a4897a2611454369520150cd768bee7f9843fe parent 7af07df

4 files changed, +113 −16

apimode.go +26 −0
internal/console/apiinstances_ratelimit_test.go +28 −0
ratelimit_cli.go +5 −4
ratelimit_cli_test.go +54 −12

diff --git a/apimode.go b/apimode.go

index e156b5036..332a9ed8d 100644

--- a/apimode.go

+++ b/apimode.go

@@ -3657,6 +3657,32 @@ func (s *apiSession) instanceRateLimit(ctx context.Context, a apiAcct, app, host

return out, nil

}

+// instanceRateLimitSettings reads one instance's own rate-limit settings

+// from its record (GET …/instances/{host}), which carries rate_limit as

+// stored. It is what set overlays, so a change never waits on the counts

+// the rate-limit GET sums over every box's usage objects: an unreadable one

+// there must not block a settings change here. A record without the field

+// is the defaults.

+func (s *apiSession) instanceRateLimitSettings(ctx context.Context, a apiAcct, app, host string) (ratelimit.Settings, error) {

+ status, body, err := s.getJSON(ctx, apiPath(a, app, "/instances/"+url.PathEscape(host)))

+ if err != nil {

+ return ratelimit.Settings{}, err

+ }

+ if status != http.StatusOK {

+ return ratelimit.Settings{}, apiErrorFromBody(status, body)

+ }

+ var out struct {

+ RateLimit *ratelimit.Settings `json:"rate_limit"`

+ }

+ if err := json.Unmarshal(body, &out); err != nil {

+ return ratelimit.Settings{}, fmt.Errorf("rate limit: unreadable response: %w", err)

+ }

+ if out.RateLimit == nil {

+ return ratelimit.Settings{}, nil

+ }

+ return *out.RateLimit, nil

// instanceSetRateLimit replaces one instance's rate-limit settings whole;

// the zero Settings clears them. A console older than the field answers 400

// "nothing to update", which is reported as the approved older-console text.

diff --git a/internal/console/apiinstances_ratelimit_test.go b/internal/console/apiinstances_ratelimit_test.go

index c86222446..4a31652c6 100644

--- a/internal/console/apiinstances_ratelimit_test.go

+++ b/internal/console/apiinstances_ratelimit_test.go

@@ -341,6 +341,34 @@ func TestRateLimitGetSameHourOwnerChange(t *testing.T) {

}

+// The instance GET carries the record's rate_limit as stored, and reads no

+// usage: `carlos ratelimit set` overlays its flags on this, so a usage

+// object that does not decode (which fails the rate-limit GET) cannot block

+// a settings change.

+func TestRateLimitInstanceGetCarriesSettings(t *testing.T) {

+ fx, token := rateLimitFixture(t, "code-rl-record")

+ pu := updateURL(fx, "bab", "shop", rlHost)

+ if status, _, out := rawReq(t, http.MethodPatch, pu, token, `{"rate_limit":{"rate":500}}`); status != http.StatusOK {

+ t.Fatalf("patch = %d %s", status, out)

+ }

+ if _, err := fx.rel.Put(context.Background(), usage.EdgeDayKey("box-b", usage.DayOf(time.Now())), []byte("{not json"), ""); err != nil {

+ t.Fatal(err)

+ }

+ status, _, out := rawReq(t, http.MethodGet, pu, token, "")

+ if status != http.StatusOK {

+ t.Fatalf("instance GET = %d %s", status, out)

+ }

+ var rec struct {

+ RateLimit *ratelimit.Settings `json:"rate_limit"`

+ }

+ if err := json.Unmarshal([]byte(out), &rec); err != nil {

+ t.Fatal(err)

+ }

+ if rec.RateLimit == nil || !reflect.DeepEqual(*rec.RateLimit, ratelimit.Settings{Rule: ratelimit.Rule{Rate: 500}}) {

+ t.Fatalf("instance GET rate_limit = %+v, want rate 500 only: %s", rec.RateLimit, out)

+ }

// A box's day object that does not decode fails the GET with its usage 502

// rather than answering a short total that reads as fewer refusals.

func TestRateLimitGetMalformedDay502(t *testing.T) {

diff --git a/ratelimit_cli.go b/ratelimit_cli.go

index 23fa73daa..b2877c025 100644

--- a/ratelimit_cli.go

+++ b/ratelimit_cli.go

@@ -448,7 +448,8 @@ func ratelimitPickupLine(w io.Writer, hosts, failed int) {

// ratelimitApply is the set/clear loop: one line per host, and the number

// that failed, which the callers turn into the exit code. A failure on one

// host does not stop the others. change == nil is the clear; otherwise each

-// host's current settings are read and change returns what to store.

+// host's current settings are read from its record, not the counts-bearing

+// rate-limit GET that show uses, and change returns what to store.

func ratelimitApply(sess *apiSession, acct apiAcct, app string, hosts []string, change func(ratelimit.Settings) ratelimit.Settings, w io.Writer) (failed int) {

bad, ok := ratelimitSetBad, ratelimitSetOK

if change == nil {

@@ -459,9 +460,9 @@ func ratelimitApply(sess *apiSession, acct apiAcct, app string, hosts []string,

var next ratelimit.Settings

var err error

if change != nil {

- var cur apiRateLimit

- if cur, err = sess.instanceRateLimit(ctx, acct, app, h); err == nil {

- next = change(cur.Settings)

+ var cur ratelimit.Settings

+ if cur, err = sess.instanceRateLimitSettings(ctx, acct, app, h); err == nil {

+ next = change(cur)

}

if err == nil {

diff --git a/ratelimit_cli_test.go b/ratelimit_cli_test.go

index 89532c1a8..604947c37 100644

--- a/ratelimit_cli_test.go

+++ b/ratelimit_cli_test.go

@@ -14,21 +14,23 @@ import (

)

// rlConsole is a fake console for the ratelimit client: the instance list,

-// the per-host rate-limit GET and the instance PATCH, each driven by the

-// test, with every request recorded so a test can prove a door was not

-// knocked on.

+// the instance GET, the per-host rate-limit GET and the instance PATCH, each

+// driven by the test, with every request recorded so a test can prove a

+// door was not knocked on.

type rlConsole struct {

- ts *httptest.Server

- seen []string

- list string // body for GET .../instances

- current string // body for GET .../rate-limit

- patch func(host string) (int, string) // status and body for PATCH .../instances/<host>

- bodies map[string]string // PATCH body by host

+ ts *httptest.Server

+ seen []string

+ list string // body for GET .../instances

+ record string // body for GET .../instances/<host>

+ current string // body for GET .../rate-limit

+ currentStatus int // status for GET .../rate-limit; 0 is 200

+ patch func(host string) (int, string) // status and body for PATCH .../instances/<host>

+ bodies map[string]string // PATCH body by host

}

func newRLConsole(t *testing.T) *rlConsole {

t.Helper()

- fc := &rlConsole{bodies: map[string]string{}, current: `{"host":"h","settings":{}}`}

+ fc := &rlConsole{bodies: map[string]string{}, current: `{"host":"h","settings":{}}`, record: `{"host":"h"}`}

fc.ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

fc.seen = append(fc.seen, r.Method+" "+r.URL.Path)

switch {

@@ -43,7 +45,12 @@ func newRLConsole(t *testing.T) *rlConsole {

w.WriteHeader(status)

w.Write([]byte(body))

case strings.HasSuffix(r.URL.Path, "/rate-limit"):

+ if fc.currentStatus != 0 {

+ w.WriteHeader(fc.currentStatus)

+ }

w.Write([]byte(fc.current))

+ case strings.Contains(r.URL.Path, "/instances/"):

+ w.Write([]byte(fc.record))

default:

w.Write([]byte(fc.list))

}

@@ -81,7 +88,7 @@ func ratelimitSet(fc *rlConsole, hosts []string, change func(ratelimit.Settings)

func TestRatelimitSetOverlaysFlags(t *testing.T) {

fc := newRLConsole(t)

- fc.current = `{"host":"a.example.com","settings":{"rate":500}}`

+ fc.record = `{"host":"a.example.com","rate_limit":{"rate":500}}`

burst := 2000

failed, out := ratelimitSet(fc, []string{"a.example.com"}, func(cur ratelimit.Settings) ratelimit.Settings {

return overlayRatelimit(cur, nil, nil, nil, &burst, nil, nil)

@@ -151,6 +158,41 @@ func TestRatelimitMalformedPathExits2(t *testing.T) {

}

+// set reads the settings it overlays from the instance's record, never

+// through the rate-limit GET: that one sums every box's usage objects for

+// its counts, and an unreadable one must not stop a settings change. A

+// record with no rate_limit is the defaults.

+func TestRatelimitSetSkipsTheCounts(t *testing.T) {

+ fc := newRLConsole(t)

+ fc.currentStatus = http.StatusBadGateway

+ fc.current = `{"error":"could not read the instance's usage"}`

+ fc.record = `{"host":"a.example.com","rate_limit":{"rate":500}}`

+ burst := 2000

+ change := func(cur ratelimit.Settings) ratelimit.Settings {

+ return overlayRatelimit(cur, nil, nil, nil, &burst, nil, nil)

+ }

+ failed, out := ratelimitSet(fc, []string{"a.example.com"}, change)

+ if failed != 0 {

+ t.Fatalf("failed = %d with the counts unreadable: %s", failed, out)

+ }

+ if got := fc.bodies["a.example.com"]; got != `{"rate_limit":{"rate":500,"burst":2000}}` {

+ t.Errorf("PATCH body = %s, want the record's rate kept and the burst added", got)

+ }

+ for _, s := range fc.seen {

+ if strings.HasSuffix(s, "/rate-limit") {

+ t.Errorf("set read the counts: %v", fc.seen)

+ }

+ fc.record = `{"host":"b.example.com"}`

+ if failed, out := ratelimitSet(fc, []string{"b.example.com"}, change); failed != 0 {

+ t.Fatalf("failed = %d: %s", failed, out)

+ }

+ if got := fc.bodies["b.example.com"]; got != `{"rate_limit":{"burst":2000}}` {

+ t.Errorf("PATCH body = %s, want only the burst on a record with no settings", got)

+ }

func TestRatelimitClearSendsEmptyObject(t *testing.T) {

fc := newRLConsole(t)

failed, out := ratelimitSet(fc, []string{"a.example.com"}, nil)

@@ -347,7 +389,7 @@ func TestRatelimitTruncatedListIsNotSuccess(t *testing.T) {

// base rule. It is not the same as leaving the field out.

func TestRatelimitPathZeroResetsAField(t *testing.T) {

fc := newRLConsole(t)

- fc.current = `{"host":"a.example.com","settings":{"paths":[{"prefix":"/api/","client_share":32,"rate":1000}]}}`

+ fc.record = `{"host":"a.example.com","rate_limit":{"paths":[{"prefix":"/api/","client_share":32,"rate":1000}]}}`

edit := mustPath(t, "/api/:rate=0")

failed, out := ratelimitSet(fc, []string{"a.example.com"}, func(cur ratelimit.Settings) ratelimit.Settings {

return overlayRatelimit(cur, nil, nil, nil, nil, []pathEdit{edit}, nil)