diff --git a/cmd/simpledeploy/main.go b/cmd/simpledeploy/main.go index cf7a3b9d..2ae3f7fe 100644 --- a/cmd/simpledeploy/main.go +++ b/cmd/simpledeploy/main.go @@ -787,6 +787,8 @@ func runServe(cmd *cobra.Command, args []string) error { srv.SetWebhookDispatcher(dispatcher) auditRec := audit.NewRecorder(db) auditRec.SetBus(eventBus) + auditRec.SetSyncTracking(srv.GitSyncPushing) + reconcilerAudit.SetSyncTracking(srv.GitSyncPushing) srv.SetAudit(auditRec) srv.SetBus(eventBus) dep.SetAuditEmitter(&api.DeployerAuditAdapter{Rec: auditRec}) diff --git a/docs/architecture/auth.md b/docs/architecture/auth.md index bb12e219..1db1d77e 100644 --- a/docs/architecture/auth.md +++ b/docs/architecture/auth.md @@ -31,7 +31,7 @@ API keys inherit the creator's role and per-app grants at the moment of creation ## Rate limiting -A per-IP token bucket limits authentication attempts and dashboard requests. Defaults: 200 requests per 60 seconds with a burst of 50, keyed by IP. Configurable in `config.yaml`. Apps can override per-route via compose labels. +A per-IP token bucket limits authentication attempts and dashboard requests. Defaults: 200 requests per 60 seconds, keyed by IP. Configurable in `config.yaml` (`ratelimit.burst` and `ratelimit.by` are ignored here). Proxied app traffic has its own per-app limits from compose labels. A separate, stricter limit on `/api/auth/login` blocks credential stuffing. After repeated failures, the account is temporarily locked. diff --git a/docs/architecture/proxy.md b/docs/architecture/proxy.md index 46be6b7d..5ee9bcca 100644 --- a/docs/architecture/proxy.md +++ b/docs/architecture/proxy.md @@ -56,7 +56,7 @@ All three are registered in `init()` of their respective files in [/internal/pro ### `simpledeploy_ratelimit` -[/internal/proxy/ratelimit.go](https://github.com/vazra/simpledeploy/blob/main/internal/proxy/ratelimit.go). Per-domain configs registered via `RateLimiters.Set(domain, cfg)` from `SetRoutes`. Each domain has its own `domainLimiter` keyed by client IP (default), header value, or query param depending on `simpledeploy.ratelimit.by`. Each domain keeps at most 10,000 buckets: when full, buckets idle for a whole window are evicted first, then the least recently used. Keys longer than 128 bytes are stored as their SHA-256. +[/internal/proxy/ratelimit.go](https://github.com/vazra/simpledeploy/blob/main/internal/proxy/ratelimit.go). Per-domain configs registered via `RateLimiters.Set(domain, cfg)` from `SetRoutes`. Each domain has its own `domainLimiter` keyed by client IP (default), header value, or path depending on `simpledeploy.ratelimit.by`. Each bucket allows `requests` per window; a window starts at the bucket's first request after the previous one ended. With `burst` > 0, up to `burst` more requests pass once the window's `requests` are used up. The burst refills only after a whole window within `requests` (or with no requests), so a client that keeps going over is held to `requests` per window. `burst` 0 (label unset) is a plain fixed window. Each domain keeps at most 10,000 buckets: when full, buckets idle for a whole window are evicted first, then the least recently used. Keys longer than 128 bytes are stored as their SHA-256. ### `simpledeploy_ipaccess` diff --git a/docs/concepts/activity-log.md b/docs/concepts/activity-log.md index cafb3555..ad7f4f5b 100644 --- a/docs/concepts/activity-log.md +++ b/docs/concepts/activity-log.md @@ -19,7 +19,7 @@ Each entry stores: actor, source (UI / API / CLI / git sync / system), category, ## Sync eligibility -Entries for config-change categories are marked with a sync status when [git sync](/operations/git-sync/) is enabled. The git sync worker stamps each relevant entry with `synced`, `pending`, or `failed` after the push completes. Runtime events (deploys, auth, start/stop) are not git-tracked and carry no badge. +Entries for config-change categories are marked with a sync status when [git sync](/operations/git-sync/) is enabled and pushes changes (auto-push on); entries recorded while it is off carry no badge. The git sync worker stamps each relevant entry with `synced`, `pending`, or `failed` after the push completes. Runtime events (deploys, auth, start/stop) are not git-tracked and carry no badge. ## Retention diff --git a/docs/guides/rate-limiting.md b/docs/guides/rate-limiting.md index 2066b535..666cf246 100644 --- a/docs/guides/rate-limiting.md +++ b/docs/guides/rate-limiting.md @@ -9,12 +9,12 @@ Rate limiting is per-app, configured with `simpledeploy.ratelimit.*` labels on a | Label | Default | Description | |-------|---------|-------------| -| `simpledeploy.ratelimit.requests` | `200` (server config) | Allowed requests per window | +| `simpledeploy.ratelimit.requests` | none | Allowed requests per window. Rate limiting is on only when this is set | | `simpledeploy.ratelimit.window` | `60s` | Length of the window | -| `simpledeploy.ratelimit.burst` | `50` | Extra short-spike allowance over the steady rate | +| `simpledeploy.ratelimit.burst` | `0` | Extra requests allowed in a short spike, on top of `requests` | | `simpledeploy.ratelimit.by` | `ip` | Key to bucket by: `ip`, `header:NAME`, or `path` | -`requests`/`window` define the steady rate. `burst` lets a client briefly exceed it before getting throttled. Once the bucket is empty, clients get `429 Too Many Requests` with a `Retry-After: 60` header. +`requests`/`window` define the steady rate. `burst` lets a client briefly go over it: once a window's `requests` are used up, up to `burst` more get through. A client that keeps going over is held to `requests` per window; the burst comes back after it stays within `requests` (or sends nothing) for a whole window. Without `burst`, the limit is exactly `requests` per window. Once nothing is left, clients get `429 Too Many Requests` with a `Retry-After: 60` header. ## Per-IP (most common) @@ -31,7 +31,7 @@ services: simpledeploy.ratelimit.by: "ip" ``` -100 req/min/IP, with burst up to 120. +100 req/min/IP. A short spike can reach 120 in one minute; a client that keeps going over is held to 100 a minute. ## Per API key (header) diff --git a/docs/guides/small-vps-tuning.md b/docs/guides/small-vps-tuning.md index 80c2526b..d60fa6a1 100644 --- a/docs/guides/small-vps-tuning.md +++ b/docs/guides/small-vps-tuning.md @@ -45,10 +45,9 @@ Defaults assume a beefier host. On 1 vCPU: ratelimit: requests: 100 window: 60s - burst: 25 ``` -This blocks bursty scrapers that would otherwise pin CPU. +This limits requests to the dashboard and management API. To slow down scrapers hitting your apps, add [rate limit labels](/guides/rate-limiting/) to those apps. ## Prune Docker regularly diff --git a/docs/operations/security-audit.md b/docs/operations/security-audit.md index f8a4424b..e86efa98 100644 --- a/docs/operations/security-audit.md +++ b/docs/operations/security-audit.md @@ -41,7 +41,7 @@ Entries for config-change categories (`compose`, `endpoint`, `backup`, `alert`, | Pending | Waiting for the next sync cycle | | Sync failed | Commit or push failed; see git sync error details | -Runtime-only events (`deploy`, `auth`, `system`, `lifecycle` start/stop/scale) are never committed to git and show no badge. +Runtime-only events (`deploy`, `auth`, `system`, `lifecycle` start/stop/scale) are never committed to git and show no badge. Entries recorded while git sync is off, or while auto-push is off (pull-only), also show no badge. ## Retention diff --git a/docs/reference/api.md b/docs/reference/api.md index 4e89ebc2..be2691e4 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -135,9 +135,11 @@ Set the IP allowlist. Body: `{"allow": "1.2.3.4, 10.0.0.0/8"}` (empty string all ## App Actions +Stop, start, scale and rollback wait for `docker compose` to finish before responding. The operation keeps running if the client disconnects mid-request. + ### `POST /api/apps/{slug}/restart` -Restart all containers for the app. Returns `{"status": "ok"}`. +Restart the app's existing containers in place (`docker compose restart`). Containers are not recreated, so compose changes and newer images are not picked up (use pull or a deploy for that). Runs in the background and returns `202` with `{"status": "started"}`. ### `POST /api/apps/{slug}/stop` @@ -149,7 +151,7 @@ Start stopped containers. Returns `{"status": "ok"}`. ### `POST /api/apps/{slug}/pull` -Pull latest images and redeploy. Uses configured registry auth if available. Returns `{"status": "ok"}`. +Pull latest images and redeploy. Uses configured registry auth if available. Runs in the background and returns `202` with `{"status": "started"}`. ### `POST /api/apps/{slug}/scale` diff --git a/docs/reference/compose-labels.md b/docs/reference/compose-labels.md index d50a359f..035a5ab8 100644 --- a/docs/reference/compose-labels.md +++ b/docs/reference/compose-labels.md @@ -226,10 +226,10 @@ See [Rate limiting](/guides/rate-limiting/) for the full guide, including key ty | Label | Default | Description | |-------|---------|-------------| -| `simpledeploy.ratelimit.requests` | global default (200) | Max requests per window | -| `simpledeploy.ratelimit.window` | global default (60s) | Time window | -| `simpledeploy.ratelimit.burst` | global default (50) | Burst allowance above limit | -| `simpledeploy.ratelimit.by` | global default (ip) | Rate limit key | +| `simpledeploy.ratelimit.requests` | none (no limit) | Max requests per window. Required to turn rate limiting on | +| `simpledeploy.ratelimit.window` | `60s` | Time window | +| `simpledeploy.ratelimit.burst` | `0` | Extra requests allowed in a short spike, on top of `requests` | +| `simpledeploy.ratelimit.by` | `ip` | Rate limit key | ## Request Tracking Labels diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 707b03fb..0b75c89f 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -73,12 +73,11 @@ metrics: - name: 1h retention: 8760h # 1 year -# Global rate limit defaults (per-app labels override) +# Management API rate limit, per client IP. App traffic is limited only by +# per-app simpledeploy.ratelimit.* labels. ratelimit: requests: 200 window: 60s - burst: 50 - by: ip # ip | header:X-API-Key | path # Default registries applied to all apps (names reference stored registries) registries: @@ -121,10 +120,10 @@ git_sync: | `master_secret` | string | **required** | Encryption/signing key | | `audit_retention_days` | int | `365` | Activity log retention in days; `0` = keep forever | | `metrics.tiers` | list | see above | Metrics rollup tiers | -| `ratelimit.requests` | int | `200` | Default requests per window | -| `ratelimit.window` | string | `60s` | Rate limit time window | -| `ratelimit.burst` | int | `50` | Burst allowance | -| `ratelimit.by` | string | `ip` | Rate limit key | +| `ratelimit.requests` | int | `200` | Management API requests per window, per client IP | +| `ratelimit.window` | string | `60s` | Management API rate limit window | +| `ratelimit.burst` | int | `50` | Reserved, currently ignored. Per-app burst is the `simpledeploy.ratelimit.burst` label | +| `ratelimit.by` | string | `ip` | Reserved, currently ignored (the management API always limits by IP) | | `registries` | list | `[]` | Default registry names for all apps | | `allowed_bind_paths` | list of string | `[]` | Absolute host folders apps may bind-mount even though they sit under a protected system folder (e.g. `["/home/media"]`). SimpleDeploy's data folder and other apps' folders stay protected. Do not list parents of sensitive folders such as `/var` or `/etc`. See [compose security validation](/reference/compose-labels/#compose-security-validation). | | `git_sync.enabled` | bool | `false` | Enable two-way git sync | diff --git a/internal/api/actions.go b/internal/api/actions.go index e3a51fa4..17df1dcb 100644 --- a/internal/api/actions.go +++ b/internal/api/actions.go @@ -10,6 +10,7 @@ import ( "os" "path/filepath" "strconv" + "time" "github.com/vazra/simpledeploy/internal/audit" "github.com/vazra/simpledeploy/internal/compose" @@ -28,6 +29,18 @@ type serviceInfo struct { ScaleReason string `json:"scale_reason,omitempty"` } +// composeOpTimeout bounds a docker compose operation that runs detached +// from its request, so a hung daemon cannot hold it forever. +const composeOpTimeout = 30 * time.Minute + +// detachedCtx returns a context for a docker compose operation that must +// finish even if the client goes away. Closing the tab cancels r.Context(), +// which would kill the compose process mid-way (e.g. leave 1 of 2 replicas). +// Request values (actor for audit) are kept. +func detachedCtx(r *http.Request) (context.Context, context.CancelFunc) { + return context.WithTimeout(context.WithoutCancel(r.Context()), composeOpTimeout) +} + func (s *Server) handleRestart(w http.ResponseWriter, r *http.Request) { slug := r.PathValue("slug") if s.reconciler == nil { @@ -58,12 +71,14 @@ func (s *Server) handleStop(w http.ResponseWriter, r *http.Request) { http.Error(w, "reconciler not configured", http.StatusInternalServerError) return } - if err := s.reconciler.StopOne(r.Context(), slug); err != nil { + ctx, cancel := detachedCtx(r) + defer cancel() + if err := s.reconciler.StopOne(ctx, slug); err != nil { httpError(w, err, http.StatusInternalServerError) return } afterJSON, _ := json.Marshal(map[string]any{"name": slug, "status": "stopped"}) - _, _ = s.audit.Record(r.Context(), audit.RecordReq{ + _, _ = s.audit.Record(ctx, audit.RecordReq{ Category: "lifecycle", Action: "stopped", AppSlug: slug, @@ -79,12 +94,14 @@ func (s *Server) handleStart(w http.ResponseWriter, r *http.Request) { http.Error(w, "reconciler not configured", http.StatusInternalServerError) return } - if err := s.reconciler.StartOne(r.Context(), slug); err != nil { + ctx, cancel := detachedCtx(r) + defer cancel() + if err := s.reconciler.StartOne(ctx, slug); err != nil { httpError(w, err, http.StatusInternalServerError) return } afterJSON, _ := json.Marshal(map[string]any{"name": slug, "status": "running"}) - _, _ = s.audit.Record(r.Context(), audit.RecordReq{ + _, _ = s.audit.Record(ctx, audit.RecordReq{ Category: "lifecycle", Action: "started", AppSlug: slug, @@ -174,7 +191,9 @@ func (s *Server) handleScale(w http.ResponseWriter, r *http.Request) { } } - if err := s.reconciler.ScaleOne(r.Context(), slug, body.Scales); err != nil { + ctx, cancel := detachedCtx(r) + defer cancel() + if err := s.reconciler.ScaleOne(ctx, slug, body.Scales); err != nil { // Forward the actual error (which wraps docker compose stderr) so the // UI can show something more useful than "Internal Server Error". log.Printf("[api] scale %s: %v", slug, err) @@ -184,7 +203,7 @@ func (s *Server) handleScale(w http.ResponseWriter, r *http.Request) { beforeJSON, _ := json.Marshal(map[string]any{"name": slug, "replicas": beforeReplicas}) afterJSON, _ := json.Marshal(map[string]any{"name": slug, "replicas": afterReplicas}) - _, _ = s.audit.Record(r.Context(), audit.RecordReq{ + _, _ = s.audit.Record(ctx, audit.RecordReq{ Category: "lifecycle", Action: "scaled", AppSlug: slug, @@ -346,7 +365,9 @@ func (s *Server) handleRollback(w http.ResponseWriter, r *http.Request) { if _, ok := s.checkVersionForRestore(w, r, slug, filepath.Join(s.appsDir, slug, "docker-compose.yml"), ver); !ok { return } - if err := s.reconciler.RollbackOne(r.Context(), slug, body.VersionID); err != nil { + ctx, cancel := detachedCtx(r) + defer cancel() + if err := s.reconciler.RollbackOne(ctx, slug, body.VersionID); err != nil { if v, ok := violationsOf(err); ok { writeViolations(w, http.StatusBadRequest, versionRefused, v) return @@ -359,7 +380,7 @@ func (s *Server) handleRollback(w http.ResponseWriter, r *http.Request) { return } afterJSON, _ := json.Marshal(map[string]any{"version_id": body.VersionID}) - _, _ = s.audit.Record(r.Context(), audit.RecordReq{ + _, _ = s.audit.Record(ctx, audit.RecordReq{ Category: "lifecycle", Action: "rolled_back", AppSlug: slug, diff --git a/internal/api/actions_test.go b/internal/api/actions_test.go index d8faadaa..88654ca9 100644 --- a/internal/api/actions_test.go +++ b/internal/api/actions_test.go @@ -7,7 +7,10 @@ import ( "fmt" "net/http" "net/http/httptest" + "os" + "path/filepath" "testing" + "time" "github.com/vazra/simpledeploy/internal/compose" "github.com/vazra/simpledeploy/internal/deployer" @@ -215,3 +218,115 @@ func TestScaleEndpointMissingBody(t *testing.T) { t.Fatalf("status = %d, want 400", w.Code) } } + +// blockingRunner is a deployer.CommandRunner that hands each compose call's +// context to the test and blocks until released. Like exec.CommandContext, +// it fails if that context is canceled. +type blockingRunner struct { + ctxs chan context.Context + release chan struct{} +} + +func (b *blockingRunner) Run(ctx context.Context, _ string, args ...string) (string, string, error) { + if len(args) == 2 && args[1] == "version" { + return "", "", nil + } + b.ctxs <- ctx + select { + case <-b.release: + case <-ctx.Done(): + } + return "", "", ctx.Err() +} + +// runnerReconciler routes lifecycle actions through a real deployer so the +// context reaching the CommandRunner is the one the handler passed. +type runnerReconciler struct { + mockReconcilerFull + d *deployer.Deployer + runner deployer.CommandRunner + composePath string +} + +func (m *runnerReconciler) StopOne(ctx context.Context, slug string) error { + return m.d.Stop(ctx, slug) +} +func (m *runnerReconciler) StartOne(ctx context.Context, slug string) error { + return m.d.Start(ctx, slug) +} +func (m *runnerReconciler) ScaleOne(ctx context.Context, slug string, scales map[string]int) error { + return m.d.Scale(ctx, &compose.AppConfig{Name: slug, ComposePath: m.composePath}, scales) +} +func (m *runnerReconciler) RollbackOne(ctx context.Context, _ string, _ int64) error { + _, _, err := m.runner.Run(ctx, "docker", "compose", "up", "-d") + return err +} + +func TestLifecycleActionsSurviveClientDisconnect(t *testing.T) { + scale, _ := json.Marshal(map[string]any{"scales": map[string]int{"web": 2}}) + cases := []struct { + name, path string + body []byte + rollback bool + }{ + {"stop", "/api/apps/myapp/stop", nil, false}, + {"start", "/api/apps/myapp/start", nil, false}, + {"scale", "/api/apps/myapp/scale", scale, false}, + {"rollback", "/api/apps/myapp/rollback", nil, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + srv, _ := newTestServer(t) + srv.SetAppsDir(t.TempDir()) + runner := &blockingRunner{ctxs: make(chan context.Context, 1), release: make(chan struct{})} + d, err := deployer.New(runner) + if err != nil { + t.Fatal(err) + } + composePath := filepath.Join(t.TempDir(), "docker-compose.yml") + if err := os.WriteFile(composePath, []byte("services:\n web:\n image: nginx\n"), 0o644); err != nil { + t.Fatal(err) + } + srv.SetReconciler(&runnerReconciler{d: d, runner: runner, composePath: composePath}) + + body := tc.body + if tc.rollback { + body, _ = json.Marshal(map[string]any{"version_id": seedVersion(t, srv, "myapp")}) + } + reqCtx, closeTab := context.WithCancel(context.Background()) + defer closeTab() + req := httptest.NewRequest(http.MethodPost, tc.path, bytes.NewReader(body)).WithContext(reqCtx) + req.Header.Set("Content-Type", "application/json") + req.AddCookie(superAdminCookie(t, srv.jwt)) + w := httptest.NewRecorder() + done := make(chan struct{}) + go func() { + srv.Handler().ServeHTTP(w, req) + close(done) + }() + + var opCtx context.Context + select { + case opCtx = <-runner.ctxs: + case <-time.After(5 * time.Second): + t.Fatal("compose command never ran") + } + closeTab() + if err := opCtx.Err(); err != nil { + t.Fatalf("compose command context canceled with the request: %v", err) + } + if _, ok := opCtx.Deadline(); !ok { + t.Error("compose command context has no timeout") + } + close(runner.release) + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("handler did not return") + } + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body: %s", w.Code, w.Body.String()) + } + }) + } +} diff --git a/internal/api/gitsync_config_handler.go b/internal/api/gitsync_config_handler.go index 04597fba..1d3961ec 100644 --- a/internal/api/gitsync_config_handler.go +++ b/internal/api/gitsync_config_handler.go @@ -27,6 +27,19 @@ func (s *Server) SetReconcilerRef(r gitsync.Reconciler) { reconcilerRef = r } +// GitSyncPushing reports whether git sync is running and pushes changes. +// The audit recorder uses it to mark rows pending only when a push will +// stamp them. Lock-free, since audit rows may be recorded under gsMu. +func (s *Server) GitSyncPushing() bool { return s.gsPushing.Load() } + +func gitSyncPushes(gs *gitsync.Syncer) bool { + if gs == nil { + return false + } + st := gs.Status() + return st.Enabled && st.AutoPushEnabled +} + // ReloadGitSync stops the current syncer (if any), rebuilds config from DB+YAML, // and starts a new syncer. Returns the new status or an error. func (s *Server) ReloadGitSync(ctx context.Context) (*gitsync.Status, error) { @@ -39,6 +52,7 @@ func (s *Server) ReloadGitSync(ctx context.Context) (*gitsync.Status, error) { log.Printf("[gitsync] reload: stop existing: %v", err) } s.gs = nil + s.gsPushing.Store(false) } if s.cfg == nil || s.store == nil { @@ -66,11 +80,13 @@ func (s *Server) ReloadGitSync(ctx context.Context) (*gitsync.Status, error) { if err != nil { return nil, fmt.Errorf("gitsync.New: %w", err) } - if err := gs.Start(ctx); err != nil { + // ctx is usually the request's; the syncer must outlive it (Stop ends it). + if err := gs.Start(context.WithoutCancel(ctx)); err != nil { return nil, fmt.Errorf("gitsync.Start: %w", err) } s.gs = gs + s.gsPushing.Store(gitSyncPushes(gs)) // Re-wire configsync sidecar hook. if s.cs != nil { diff --git a/internal/api/gitsync_config_handler_test.go b/internal/api/gitsync_config_handler_test.go index 09f01a54..bf927d66 100644 --- a/internal/api/gitsync_config_handler_test.go +++ b/internal/api/gitsync_config_handler_test.go @@ -2,9 +2,12 @@ package api import ( "bytes" + "context" "encoding/json" "net/http" "net/http/httptest" + "os" + "path/filepath" "testing" "time" @@ -12,6 +15,7 @@ import ( "github.com/vazra/simpledeploy/internal/auth" "github.com/vazra/simpledeploy/internal/config" + "github.com/vazra/simpledeploy/internal/gitsync" ) func gitPlainInit(path string, bare bool) (*git.Repository, error) { @@ -312,3 +316,99 @@ func TestDisableGitSync(t *testing.T) { t.Errorf("expected empty DB after disable, got %v", kv) } } + +func TestGitSyncPushingFollowsSyncer(t *testing.T) { + srv, _ := newTestServer(t) + newSyncer := func(push bool) *gitsync.Syncer { + t.Helper() + gs, err := gitsync.New(gitsync.Config{ + Enabled: true, + Remote: "file://" + t.TempDir(), + Branch: "main", + AppsDir: t.TempDir(), + AutoPushEnabled: push, + }, srv.store, nil, nil) + if err != nil { + t.Fatalf("gitsync.New: %v", err) + } + return gs + } + + if srv.GitSyncPushing() { + t.Error("no syncer: want false") + } + srv.SetGitSync(newSyncer(false)) + if srv.GitSyncPushing() { + t.Error("pull-only syncer: want false") + } + srv.SetGitSync(newSyncer(true)) + if !srv.GitSyncPushing() { + t.Error("pushing syncer: want true") + } + + // Reloading with git sync turned off clears it. + srv.cfg = &config.Config{AppsDir: t.TempDir(), MasterSecret: "secret"} + srv.masterSecret = "secret" + if _, err := srv.ReloadGitSync(context.Background()); err != nil { + t.Fatalf("ReloadGitSync: %v", err) + } + if srv.GitSyncPushing() { + t.Error("after reload with git sync off: want false") + } + + // Reloading with git sync on sets it again. + bare := t.TempDir() + if _, err := gitPlainInit(bare, true); err != nil { + t.Fatal(err) + } + if err := srv.store.SetGitSyncConfig(map[string]string{"enabled": "true", "remote": "file://" + bare, "branch": "main"}); err != nil { + t.Fatal(err) + } + if _, err := srv.ReloadGitSync(context.Background()); err != nil { + t.Fatalf("ReloadGitSync: %v", err) + } + t.Cleanup(func() { _ = srv.gs.Stop() }) + if !srv.GitSyncPushing() { + t.Error("after reload with git sync on: want true") + } +} + +// The handlers reload with r.Context(), which ends with the response. The +// syncer started by the reload must keep running after that. +func TestReloadGitSyncOutlivesRequest(t *testing.T) { + srv, _ := newTestServer(t) + appsDir := t.TempDir() + srv.cfg = &config.Config{AppsDir: appsDir, MasterSecret: "secret"} + srv.masterSecret = "secret" + bare := t.TempDir() + if _, err := gitPlainInit(bare, true); err != nil { + t.Fatal(err) + } + if err := srv.store.SetGitSyncConfig(map[string]string{"enabled": "true", "remote": "file://" + bare, "branch": "main"}); err != nil { + t.Fatal(err) + } + + reqCtx, endRequest := context.WithCancel(context.Background()) + if _, err := srv.ReloadGitSync(reqCtx); err != nil { + t.Fatalf("ReloadGitSync: %v", err) + } + t.Cleanup(func() { _ = srv.gs.Stop() }) + endRequest() + time.Sleep(100 * time.Millisecond) // let a request-bound worker exit + + before := srv.gs.Status().HeadSHA + if err := os.MkdirAll(filepath.Join(appsDir, "foo"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(appsDir, "foo", "docker-compose.yml"), []byte("services: {}\n"), 0o644); err != nil { + t.Fatal(err) + } + srv.gs.EnqueueCommit(nil, "test edit") + deadline := time.Now().Add(10 * time.Second) + for srv.gs.Status().HeadSHA == before { + if time.Now().After(deadline) { + t.Fatal("syncer stopped committing once the reload request ended") + } + time.Sleep(50 * time.Millisecond) + } +} diff --git a/internal/api/server.go b/internal/api/server.go index 87a805be..090c7d45 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -13,6 +13,7 @@ import ( "strconv" "strings" "sync" + "sync/atomic" "time" "github.com/robfig/cron/v3" @@ -79,6 +80,7 @@ type Server struct { cfgPath string deploymentMode deployment.Mode gs *gitsync.Syncer + gsPushing atomic.Bool // gs is set and pushes; see GitSyncPushing cs *configsync.Syncer recipesCache *recipes.Cache bus *events.Bus @@ -164,7 +166,10 @@ func (s *Server) SetConfig(cfg *config.Config, path string) { func (s *Server) SetDocker(dc docker.Client) { s.docker = dc } // SetGitSync sets the gitsync.Syncer (nil disables git endpoints). -func (s *Server) SetGitSync(gs *gitsync.Syncer) { s.gs = gs } +func (s *Server) SetGitSync(gs *gitsync.Syncer) { + s.gs = gs + s.gsPushing.Store(gitSyncPushes(gs)) +} // SetConfigSync sets the configsync.Syncer (nil disables sidecar cleanup on delete). func (s *Server) SetConfigSync(cs *configsync.Syncer) { s.cs = cs } diff --git a/internal/audit/recorder.go b/internal/audit/recorder.go index 2371aebb..77c7a26a 100644 --- a/internal/audit/recorder.go +++ b/internal/audit/recorder.go @@ -2,6 +2,7 @@ package audit import ( "context" + "sync/atomic" "github.com/vazra/simpledeploy/internal/audit/render" "github.com/vazra/simpledeploy/internal/events" @@ -17,8 +18,9 @@ type Publisher interface { // Recorder writes audit events to the DB-backed audit_log table. type Recorder struct { - s *store.Store - bus Publisher + s *store.Store + bus Publisher + syncOn atomic.Pointer[func() bool] } // NewRecorder creates a Recorder backed by the given store. @@ -33,6 +35,21 @@ func (r *Recorder) SetBus(b Publisher) { r.bus = b } +// SetSyncTracking sets how the recorder learns whether git sync is pushing +// changes. Sync-eligible rows are marked pending only while on returns true, +// so Activity does not show "pending" forever when git sync is off. nil (the +// default) marks every eligible row pending. Safe to call while recording. +func (r *Recorder) SetSyncTracking(on func() bool) { + if r == nil { + return + } + if on == nil { + r.syncOn.Store(nil) + return + } + r.syncOn.Store(&on) +} + // RecordReq is the input to Recorder.Record. type RecordReq struct { AppID *int64 @@ -73,6 +90,9 @@ func (r *Recorder) Record(ctx context.Context, req RecordReq) (int64, error) { if req.Category == "lifecycle" && syncEligibleLifecycleActions[req.Action] { eligible = true } + if on := r.syncOn.Load(); eligible && on != nil && !(*on)() { + eligible = false + } id, err := r.s.RecordAudit(ctx, store.AuditEntry{ AppID: req.AppID, AppSlug: req.AppSlug, diff --git a/internal/audit/recorder_test.go b/internal/audit/recorder_test.go index 656c27f5..1f55e62d 100644 --- a/internal/audit/recorder_test.go +++ b/internal/audit/recorder_test.go @@ -134,3 +134,44 @@ func TestRecorderNilSafe(t *testing.T) { t.Fatalf("nil Recorder.Record returned non-zero id: %d", id) } } + +func TestRecorderSyncPendingOnlyWhileGitSyncPushes(t *testing.T) { + s := openTestStore(t) + rec := NewRecorder(s) + ctx := context.Background() + status := func(id int64) string { + t.Helper() + got, err := s.GetActivity(ctx, id) + if err != nil { + t.Fatalf("GetActivity: %v", err) + } + if got.SyncStatus == nil { + return "" + } + return *got.SyncStatus + } + + pushing := false + rec.SetSyncTracking(func() bool { return pushing }) + + id, _ := rec.Record(ctx, RecordReq{Category: "compose", Action: "changed"}) + if got := status(id); got != "" { + t.Errorf("git sync off: sync_status = %q, want none", got) + } + + pushing = true + id, _ = rec.Record(ctx, RecordReq{Category: "compose", Action: "changed"}) + if got := status(id); got != "pending" { + t.Errorf("git sync on: sync_status = %q, want pending", got) + } + id, _ = rec.Record(ctx, RecordReq{Category: "lifecycle", Action: "stopped"}) + if got := status(id); got != "" { + t.Errorf("non-synced action: sync_status = %q, want none", got) + } + + // The pending row is what git sync stamps after a push. + ids, err := s.PendingSyncAuditIDs(ctx) + if err != nil || len(ids) != 1 { + t.Fatalf("PendingSyncAuditIDs = %v, %v; want one row", ids, err) + } +} diff --git a/internal/proxy/ratelimit.go b/internal/proxy/ratelimit.go index fc1d2dbf..4534770a 100644 --- a/internal/proxy/ratelimit.go +++ b/internal/proxy/ratelimit.go @@ -128,6 +128,8 @@ type domainLimiter struct { type rlBucket struct { key string tokens int + spare int // burst allowance left, used once tokens run out + over bool // a request went past tokens this window lastReset time.Time lastAccess time.Time } @@ -171,28 +173,40 @@ func (d *domainLimiter) allow(key string) bool { } else { if len(d.buckets) >= d.maxBuckets { // Full. Buckets idle for a whole window would restart with full - // tokens anyway, so dropping them loses nothing; if none are, - // drop the least recently used. + // tokens anyway, so dropping them loses little (at most a spent + // burst); if none are, drop the least recently used. d.evictIdle(now.Add(-d.window)) for len(d.buckets) >= d.maxBuckets && d.lru.Len() > 0 { d.removeElement(d.lru.Back()) } } - b = &rlBucket{key: key, tokens: d.requests, lastReset: now} + b = &rlBucket{key: key, tokens: d.requests, spare: d.burst, lastReset: now} d.buckets[key] = d.lru.PushFront(b) } b.lastAccess = now - if now.Sub(b.lastReset) >= d.window { + if elapsed := now.Sub(b.lastReset); elapsed >= d.window { + // The burst comes back only after a whole window within requests + // (or with no requests at all), so a client that keeps going over + // is held to the steady rate. + if !b.over || elapsed >= 2*d.window { + b.spare = d.burst + } + b.over = false b.tokens = d.requests b.lastReset = now } - if b.tokens <= 0 { - return false + if b.tokens > 0 { + b.tokens-- + return true + } + b.over = true + if b.spare > 0 { + b.spare-- + return true } - b.tokens-- - return true + return false } // evictIdle removes buckets last used before cutoff, oldest first. diff --git a/internal/proxy/ratelimit_test.go b/internal/proxy/ratelimit_test.go index 550dadd5..af1bbd40 100644 --- a/internal/proxy/ratelimit_test.go +++ b/internal/proxy/ratelimit_test.go @@ -291,3 +291,55 @@ func TestRateLimitHandlerHostWithPortAndCase(t *testing.T) { } } } + +// allowN sends n requests for key and returns how many were allowed. +func allowN(d *domainLimiter, key string, n int) int { + ok := 0 + for i := 0; i < n; i++ { + if d.allow(key) { + ok++ + } + } + return ok +} + +func TestDomainLimiterBurst(t *testing.T) { + now := time.Unix(1000, 0) + d := newDomainLimiter(&RateLimitConfig{Requests: 3, Window: time.Minute, Burst: 2, By: "ip"}) + d.now = func() time.Time { return now } + + steps := []struct { + name string + send, got int + }{ + {"spike uses burst", 10, 5}, + {"burst spent: held to requests", 10, 3}, + {"still over: burst stays spent", 10, 3}, + {"calm window", 2, 2}, + {"burst back after a calm window", 10, 5}, + } + for _, s := range steps { + if got := allowN(d, "a", s.send); got != s.got { + t.Fatalf("%s: allowed %d of %d, want %d", s.name, got, s.send, s.got) + } + now = now.Add(time.Minute) + } + + // A whole window with no requests also brings the burst back. + now = now.Add(time.Minute) + if got := allowN(d, "a", 10); got != 5 { + t.Fatalf("after idle window: allowed %d, want 5", got) + } +} + +func TestDomainLimiterNoBurstIsFixedWindow(t *testing.T) { + now := time.Unix(1000, 0) + d := newDomainLimiter(&RateLimitConfig{Requests: 3, Window: time.Minute, By: "ip"}) + d.now = func() time.Time { return now } + for i := 0; i < 4; i++ { + if got := allowN(d, "a", 10); got != 3 { + t.Fatalf("window %d: allowed %d, want 3", i, got) + } + now = now.Add(time.Minute) + } +} diff --git a/ui/src/components/SettingsTab.svelte b/ui/src/components/SettingsTab.svelte index b56e7284..f58f5ab9 100644 --- a/ui/src/components/SettingsTab.svelte +++ b/ui/src/components/SettingsTab.svelte @@ -53,6 +53,7 @@ let showDanger = $state(false) let showDeleteModal = $state(false) let deleting = $state(false) + let deleteError = $state('') // Export let exporting = $state(false) @@ -190,9 +191,16 @@ async function confirmDelete() { deleting = true - await api.removeApp(slug) + deleteError = '' + const res = await api.removeApp(slug) deleting = false showDeleteModal = false + // On failure the API call already toasted the reason; stay on the page + // so it is clear the app still exists. + if (res?.error) { + deleteError = res.error + return + } push('/') } @@ -480,6 +488,11 @@ Delete App + {#if deleteError} + + {/if} {/if} diff --git a/ui/src/components/__tests__/SettingsTab.test.js b/ui/src/components/__tests__/SettingsTab.test.js index f0c9387b..e488bd7f 100644 --- a/ui/src/components/__tests__/SettingsTab.test.js +++ b/ui/src/components/__tests__/SettingsTab.test.js @@ -9,6 +9,7 @@ vi.mock('svelte-spa-router', () => ({ default: null, })); +import { push } from 'svelte-spa-router'; import SettingsTab from '../SettingsTab.svelte'; import { toasts } from '../../lib/stores/toast.js'; @@ -82,6 +83,7 @@ describe('SettingsTab', () => { routes = {}; installFetch(); for (const t of get(toasts)) toasts.remove(t.id); + push.mockClear(); }); afterEach(() => { @@ -194,6 +196,32 @@ describe('SettingsTab', () => { expect(getByLabelText('IP Allowlist').value).toBe('10.0.0.0/8'); }); + async function deleteApp() { + const r = renderTab(); + await fireEvent.click(r.getByRole('button', { name: /Danger Zone/i })); + await fireEvent.click(r.getByRole('button', { name: /Delete App/i })); + await fireEvent.click(r.getByRole('button', { name: 'Confirm' })); + return r; + } + + it('stays on the page and shows the error when deleting the app fails', async () => { + routes['DELETE /api/apps/foo'] = () => json({ error: 'compose down failed: daemon unreachable' }, 500); + const { findByTestId } = await deleteApp(); + + await waitFor(() => expect(calls('DELETE', '/api/apps/foo')).toHaveLength(1)); + expect((await findByTestId('delete-error')).textContent).toMatch(/daemon unreachable/); + expect(get(toasts).some((t) => t.type === 'error' && /daemon unreachable/.test(t.message))).toBe(true); + expect(push).not.toHaveBeenCalled(); + }); + + it('goes back to the app list after the app is deleted', async () => { + routes['DELETE /api/apps/foo'] = () => json({ status: 'ok' }); + const { queryByTestId } = await deleteApp(); + + await waitFor(() => expect(push).toHaveBeenCalledWith('/')); + expect(queryByTestId('delete-error')).toBeNull(); + }); + it('gives viewers a read-only environment editor', async () => { const { findByTestId, queryByText } = renderTab({ canMutate: false }); expect(await findByTestId('env-readonly-hint')).toBeInTheDocument(); diff --git a/ui/src/routes/AppDetail.svelte b/ui/src/routes/AppDetail.svelte index 632c22e7..35ae069d 100644 --- a/ui/src/routes/AppDetail.svelte +++ b/ui/src/routes/AppDetail.svelte @@ -452,7 +452,7 @@

Restart App

-

This will force-recreate all containers for {app.Name}. Continue?

+

Each service in {app.Name} will be stopped and started again, keeping its current settings and data. The app may be unavailable for a few seconds. To download newer images, use Pull & Update instead.