fix(quota): 配额表单整体替换,留空即不限、0 即不给、负数和越界拒收,超配额的服务器仍可调小
This commit is contained in:
17 files changed
+346
-109
No files matched your search
@@ -1511,21 +1511,9 @@ func (f *fakeRepo) SetQuotas(_ context.Context, userID string, qi QuotaInput, _
|
||||
if f.fakeQuotas == nil {
|
||||
f.fakeQuotas = map[string]*QuotaView{}
|
||||
}
|
||||
if _, ok := f.fakeQuotas[userID]; !ok {
|
||||
f.fakeQuotas[userID] = &QuotaView{UserID: userID}
|
||||
}
|
||||
if qi.MaxServers != nil {
|
||||
f.fakeQuotas[userID].MaxServers = qi.MaxServers
|
||||
}
|
||||
if qi.MaxCPUMilli != nil {
|
||||
f.fakeQuotas[userID].MaxCPUMilli = qi.MaxCPUMilli
|
||||
}
|
||||
if qi.MaxMemoryMB != nil {
|
||||
f.fakeQuotas[userID].MaxMemoryMB = qi.MaxMemoryMB
|
||||
}
|
||||
if qi.MaxStorageGB != nil {
|
||||
f.fakeQuotas[userID].MaxStorageGB = qi.MaxStorageGB
|
||||
}
|
||||
// A full replacement, like the SQL upsert: nil is unlimited.
|
||||
f.fakeQuotas[userID] = &QuotaView{UserID: userID, MaxServers: qi.MaxServers,
|
||||
MaxCPUMilli: qi.MaxCPUMilli, MaxMemoryMB: qi.MaxMemoryMB, MaxStorageGB: qi.MaxStorageGB}
|
||||
return f.fakeQuotas[userID], nil
|
||||
}
|
||||
|
||||
|
||||
@@ -434,3 +434,43 @@ func TestPatchServerClearsDisplayName(t *testing.T) {
|
||||
t.Fatalf("patched displayName = %v, want an empty one", p.DisplayName)
|
||||
}
|
||||
}
|
||||
|
||||
// An owner over a cap an admin lowered (quota set below what they already use)
|
||||
// must still be brought back under it: only growth is held to the caps. Before,
|
||||
// every resource patch ran the quota check, so shrinking a server of an over-cap
|
||||
// owner got the same 403 as growing it.
|
||||
func TestPatchServerOverQuotaMayShrink(t *testing.T) {
|
||||
api, repo, cl, _ := newPatchAPI()
|
||||
seedResources(cl)
|
||||
repo.byName["survival"].OwnerID = "u1"
|
||||
repo.quota["u1"] = false // over every cap: any check refuses
|
||||
repo.serverResources["survival"] = ResourceSpec{CPUMilli: 1000, MemoryMB: 4096, StorageMB: 10240}
|
||||
|
||||
for _, body := range []string{`{"resources":{"cpu":"500m"}}`, `{"resources":{"cpu":"1"}}`} {
|
||||
delete(cl.patched, "survival")
|
||||
w := patchSurvival(api, body)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("%s: code = %d, want 200 (%s)", body, w.Code, w.Body.String())
|
||||
}
|
||||
if _, ok := cl.patched["survival"]; !ok {
|
||||
t.Fatalf("%s: the patch did not reach the cluster", body)
|
||||
}
|
||||
}
|
||||
if len(repo.quotaChecked) != 0 {
|
||||
t.Errorf("a patch that grows nothing was quota-checked: %+v", repo.quotaChecked)
|
||||
}
|
||||
if got := repo.resourceUpdates["survival"]; got != (ResourceSpec{CPUMilli: 1000, MemoryMB: 4096, StorageMB: 10240}) {
|
||||
t.Errorf("resource cache = %+v, want cpu 1000 / mem 4096 / storage kept", got)
|
||||
}
|
||||
|
||||
for _, body := range []string{`{"resources":{"cpu":"2"}}`, `{"resources":{"cpu":"500m","memory":"8Gi"}}`} {
|
||||
delete(cl.patched, "survival")
|
||||
w := patchSurvival(api, body)
|
||||
if w.Code != http.StatusForbidden || decodeErr(t, w) != "quota_exceeded" {
|
||||
t.Fatalf("growing an over-cap owner's server %s: code = %d body %s, want 403 quota_exceeded", body, w.Code, w.Body.String())
|
||||
}
|
||||
if _, ok := cl.patched["survival"]; ok {
|
||||
t.Fatalf("a refused growth %s reached the cluster", body)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1041,7 +1041,17 @@ func (a *API) handlePatchServer(w http.ResponseWriter, r *http.Request) {
|
||||
writeError(w, r, err)
|
||||
return
|
||||
}
|
||||
if rec != nil && rec.OwnerID != "" {
|
||||
var cur ResourceSpec
|
||||
if rec != nil {
|
||||
if cur, err = a.Repo.ServerResources(r.Context(), name); err != nil {
|
||||
writeError(w, r, err)
|
||||
return
|
||||
}
|
||||
}
|
||||
// Only growth is held to the caps. A change that grows neither CPU nor memory
|
||||
// cannot push the owner past one, and it is how an admin brings a server back
|
||||
// under a cap lowered below what the owner already uses.
|
||||
if rec != nil && rec.OwnerID != "" && (newCPU > cur.CPUMilli || newMemMB > cur.MemoryMB) {
|
||||
ok, err := a.Repo.QuotaCheck(r.Context(), rec.OwnerID, name,
|
||||
ResourceSpec{CPUMilli: newCPU, MemoryMB: newMemMB})
|
||||
if err != nil {
|
||||
@@ -1062,16 +1072,7 @@ func (a *API) handlePatchServer(w http.ResponseWriter, r *http.Request) {
|
||||
// A resource patch cannot change storage, so its cached contribution must
|
||||
// be preserved: passing 0 would silently zero the storage dimension of the
|
||||
// owner's four-cap aggregate (the cached columns are its only input).
|
||||
storMB := 0
|
||||
if rec != nil {
|
||||
cur, err := a.Repo.ServerResources(r.Context(), name)
|
||||
if err != nil {
|
||||
writeError(w, r, err)
|
||||
return
|
||||
}
|
||||
storMB = cur.StorageMB
|
||||
}
|
||||
_ = a.Repo.UpdateServerResources(r.Context(), name, newCPU, newMemMB, storMB)
|
||||
_ = a.Repo.UpdateServerResources(r.Context(), name, newCPU, newMemMB, cur.StorageMB)
|
||||
} else {
|
||||
if err := a.Cluster.PatchServerSpec(r.Context(), name, patch); err != nil {
|
||||
a.writeLookupError(w, r, err)
|
||||
|
||||
@@ -2,6 +2,7 @@ package api
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"math"
|
||||
"net/http"
|
||||
"strconv"
|
||||
"strings"
|
||||
@@ -317,11 +318,23 @@ func (a *API) handleSetQuotas(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
// Reject a body where every field is nil — a silent no-op is a client mistake.
|
||||
if body.MaxServers == nil && body.MaxCPUMilli == nil && body.MaxMemoryMB == nil && body.MaxStorageGB == nil {
|
||||
writeError(w, r, newError(http.StatusBadRequest, "bad_request",
|
||||
"at least one quota field must be set"))
|
||||
return
|
||||
// The body replaces all four caps; an empty one lifts every cap. A negative cap
|
||||
// would refuse every claim the way 0 does while reading like a mistake, and the
|
||||
// columns are 32-bit.
|
||||
for _, f := range []struct {
|
||||
name string
|
||||
v *int
|
||||
}{
|
||||
{"max_servers", body.MaxServers},
|
||||
{"max_cpu_milli", body.MaxCPUMilli},
|
||||
{"max_memory_mb", body.MaxMemoryMB},
|
||||
{"max_storage_gb", body.MaxStorageGB},
|
||||
} {
|
||||
if f.v != nil && (*f.v < 0 || *f.v > math.MaxInt32) {
|
||||
writeError(w, r, newError(http.StatusBadRequest, "invalid_quota",
|
||||
"%s must be a whole number from 0 to 2147483647, or null for unlimited", f.name))
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
v, err := a.Repo.SetQuotas(r.Context(), id, body, p.Email)
|
||||
|
||||
@@ -2,6 +2,7 @@ package api
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
@@ -127,3 +128,65 @@ func TestAdminSubresourcesRequireLiveUser(t *testing.T) {
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// The quotas form replaces all four caps at once, so an owner can lift a cap they
|
||||
// set: a missing or null field is unlimited, 0 grants none of it. Before, a null
|
||||
// field meant "leave it", the panel sent null for every emptied box, and a cap once
|
||||
// set could only be moved, never removed; a negative one was written as is.
|
||||
func TestSetQuotasReplacesAllCaps(t *testing.T) {
|
||||
owner := &Principal{UserID: "usr-root", Role: "owner", ViaAdminAccess: true}
|
||||
repo := newFakeRepo()
|
||||
repo.seedUser(UserView{ID: "usr-root", Username: "root", Role: "owner"})
|
||||
repo.seedUser(UserView{ID: "u2", Username: "alice", Role: "user"})
|
||||
api := newTestAPI(repo, newFakeCluster())
|
||||
api.External = staticExternal{p: owner}
|
||||
eh := api.ExternalHandler()
|
||||
|
||||
caps := func() string {
|
||||
t.Helper()
|
||||
w := do(eh, "GET", "/api/v1/users/u2/quotas", "", nil)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("get quotas: code = %d body %s", w.Code, w.Body.String())
|
||||
}
|
||||
return strings.TrimSpace(w.Body.String())
|
||||
}
|
||||
put := func(body string, want int) {
|
||||
t.Helper()
|
||||
w := do(eh, "PUT", "/api/v1/users/u2/quotas", body, jsonHeader)
|
||||
if w.Code != want {
|
||||
t.Fatalf("PUT %s: code = %d body %s, want %d", body, w.Code, w.Body.String(), want)
|
||||
}
|
||||
}
|
||||
|
||||
put(`{"max_servers":0,"max_cpu_milli":2000,"max_memory_mb":4096,"max_storage_gb":20}`, http.StatusOK)
|
||||
if got, want := caps(), `{"user_id":"u2","max_servers":0,"max_cpu_milli":2000,"max_memory_mb":4096,"max_storage_gb":20}`; got != want {
|
||||
t.Fatalf("after setting every cap: %s, want %s", got, want)
|
||||
}
|
||||
// What the panel sends after the owner empties two boxes.
|
||||
put(`{"max_servers":null,"max_cpu_milli":1000,"max_memory_mb":null,"max_storage_gb":20}`, http.StatusOK)
|
||||
if got, want := caps(), `{"user_id":"u2","max_cpu_milli":1000,"max_storage_gb":20}`; got != want {
|
||||
t.Fatalf("after emptying two boxes: %s, want %s", got, want)
|
||||
}
|
||||
put(`{}`, http.StatusOK)
|
||||
if got, want := caps(), `{"user_id":"u2"}`; got != want {
|
||||
t.Fatalf("after lifting every cap: %s, want %s", got, want)
|
||||
}
|
||||
|
||||
put(`{"max_servers":3}`, http.StatusOK)
|
||||
for _, body := range []string{
|
||||
`{"max_servers":-1}`,
|
||||
`{"max_servers":1,"max_storage_gb":-5}`,
|
||||
`{"max_memory_mb":2147483648}`,
|
||||
`{"max_cpu_milli":1.5}`,
|
||||
} {
|
||||
put(body, http.StatusBadRequest)
|
||||
}
|
||||
if got, want := caps(), `{"user_id":"u2","max_servers":3}`; got != want {
|
||||
t.Fatalf("a refused write changed the caps: %s, want %s", got, want)
|
||||
}
|
||||
w := do(eh, "PUT", "/api/v1/users/u2/quotas", `{"max_storage_gb":-5}`, jsonHeader)
|
||||
if !strings.Contains(w.Body.String(), `"invalid_quota"`) || !strings.Contains(w.Body.String(), "max_storage_gb") {
|
||||
t.Fatalf("a negative cap should name the field: %s", w.Body.String())
|
||||
}
|
||||
put(`{"max_memory_mb":2147483647}`, http.StatusOK)
|
||||
}
|
||||
+12
-56
@@ -2027,55 +2027,23 @@ func (p *PGRepo) requireLiveUser(ctx context.Context, userID string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// SetQuotas upserts a quotas row. Nil fields are left unchanged; a non-nil
|
||||
// zero-value field clears the cap.
|
||||
// SetQuotas replaces the user's quotas row. A nil field stores NULL, which every
|
||||
// quota gate reads as unlimited; any other value is the cap, 0 included. The form
|
||||
// always carries all four caps, so clearing one is a matter of leaving it out.
|
||||
func (p *PGRepo) SetQuotas(ctx context.Context, userID string, qi QuotaInput, setBy string) (*QuotaView, error) {
|
||||
if err := p.requireLiveUser(ctx, userID); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
type col struct {
|
||||
name string
|
||||
value *int
|
||||
}
|
||||
cols := []col{
|
||||
{"max_servers", qi.MaxServers},
|
||||
{"max_cpu_milli", qi.MaxCPUMilli},
|
||||
{"max_memory_mb", qi.MaxMemoryMB},
|
||||
{"max_storage_gb", qi.MaxStorageGB},
|
||||
}
|
||||
|
||||
// Build the ON CONFLICT upsert dynamically.
|
||||
var insCols, insVals []string
|
||||
var upd []string
|
||||
var args []any
|
||||
argn := 0
|
||||
args = append(args, userID) // $1 = user_id
|
||||
argn++
|
||||
args = append(args, setBy) // $2 = updated_by
|
||||
argn++
|
||||
insCols = append(insCols, "user_id", "updated_by")
|
||||
insVals = append(insVals, "$1", "$2")
|
||||
|
||||
for _, c := range cols {
|
||||
if c.value == nil {
|
||||
continue
|
||||
}
|
||||
argn++
|
||||
insCols = append(insCols, c.name)
|
||||
insVals = append(insVals, fmt.Sprintf("$%d", argn))
|
||||
args = append(args, *c.value)
|
||||
upd = append(upd, fmt.Sprintf("%s = EXCLUDED.%s", c.name, c.name))
|
||||
}
|
||||
|
||||
query := fmt.Sprintf(`INSERT INTO quotas (%s) VALUES (%s)
|
||||
ON CONFLICT (user_id) DO UPDATE SET %s, updated_by = $2
|
||||
RETURNING user_id, max_servers, max_cpu_milli, max_memory_mb, max_storage_gb`,
|
||||
joinStr(insCols), joinStr(insVals), joinStr(upd))
|
||||
|
||||
v := QuotaView{}
|
||||
switch err := p.db.QueryRowContext(ctx, query, args...).Scan(
|
||||
&v.UserID, &v.MaxServers, &v.MaxCPUMilli, &v.MaxMemoryMB, &v.MaxStorageGB); {
|
||||
case err != nil:
|
||||
if err := p.db.QueryRowContext(ctx,
|
||||
`INSERT INTO quotas (user_id, updated_by, max_servers, max_cpu_milli, max_memory_mb, max_storage_gb)
|
||||
VALUES ($1, $2, $3, $4, $5, $6)
|
||||
ON CONFLICT (user_id) DO UPDATE SET updated_by = EXCLUDED.updated_by,
|
||||
max_servers = EXCLUDED.max_servers, max_cpu_milli = EXCLUDED.max_cpu_milli,
|
||||
max_memory_mb = EXCLUDED.max_memory_mb, max_storage_gb = EXCLUDED.max_storage_gb
|
||||
RETURNING user_id, max_servers, max_cpu_milli, max_memory_mb, max_storage_gb`,
|
||||
userID, setBy, qi.MaxServers, qi.MaxCPUMilli, qi.MaxMemoryMB, qi.MaxStorageGB).Scan(
|
||||
&v.UserID, &v.MaxServers, &v.MaxCPUMilli, &v.MaxMemoryMB, &v.MaxStorageGB); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return &v, nil
|
||||
@@ -2735,18 +2703,6 @@ func (p *PGRepo) CreateSetupToken(ctx context.Context, tokenHash, userID string,
|
||||
return err
|
||||
}
|
||||
|
||||
// joinStr joins a slice of strings with ", ".
|
||||
func joinStr(vals []string) string {
|
||||
if len(vals) == 0 {
|
||||
return ""
|
||||
}
|
||||
s := vals[0]
|
||||
for _, v := range vals[1:] {
|
||||
s += ", " + v
|
||||
}
|
||||
return s
|
||||
}
|
||||
|
||||
// isUniqueViolation reports whether err is a Postgres unique-constraint
|
||||
// violation (code 23505).
|
||||
func isUniqueViolation(err error) bool {
|
||||
|
||||
@@ -723,8 +723,8 @@ type Repo interface {
|
||||
// GetQuotas returns the quotas row for a user, or a zero-value view when no
|
||||
// row exists (which means unlimited per spec §9.3).
|
||||
GetQuotas(ctx context.Context, userID string) (*QuotaView, error)
|
||||
// SetQuotas upserts a quotas row for userID. Nil fields leave the column
|
||||
// untouched; a zero-value (non-nil) field clears the cap (unlimited).
|
||||
// SetQuotas replaces the quotas row for userID with q: a nil field is stored as
|
||||
// NULL (unlimited), any other value is the cap, 0 included.
|
||||
SetQuotas(ctx context.Context, userID string, q QuotaInput, setBy string) (*QuotaView, error)
|
||||
|
||||
// ---- session admin (admin-only) ----
|
||||
@@ -854,8 +854,9 @@ type QuotaView struct {
|
||||
MaxStorageGB *int `json:"max_storage_gb,omitempty"`
|
||||
}
|
||||
|
||||
// QuotaInput is the admin set-quotas form. Nil fields are left unchanged;
|
||||
// a non-nil zero-value field clears the cap (unlimited).
|
||||
// QuotaInput is the admin set-quotas form. It replaces all four caps at once: an
|
||||
// absent or null field is unlimited, and 0 grants none of that resource, so every
|
||||
// claim that needs it is refused. The handler rejects a value outside 0..MaxInt32.
|
||||
type QuotaInput struct {
|
||||
MaxServers *int `json:"max_servers,omitempty"`
|
||||
MaxCPUMilli *int `json:"max_cpu_milli,omitempty"`
|
||||
|
||||
Reference in new issue
Block a user