The rollback endpoint took a deployment ID and did nothing with it

Project Overview

Staxa is a multi-tenant deployment platform I am building solo under Stackforge Labs. The backend is a single Go binary (staxad) using the chi router, with about 60 API endpoints, running on K3s on a Hetzner CAX21 ARM64 server that costs around $11/month. Each tenant gets an isolated Kubernetes namespace with their own app container, a PostgreSQL 16 or MySQL 8 database, a subdomain with automatic SSL, and resource quotas. Container builds run through Buildah, and the frontend is Next.js (App Router) with shadcn/ui and Clerk for auth.

Bug Fix or Performance Improvement

The symptom: POST /api/v1/tenants/{id}/deployments/{depId}/rollback accepted a deployment ID in the URL path and then completely ignored it. Whatever version you asked for, you got the most recent successful deployment instead. The route was wired up correctly in internal/api/router.go:149:

r.Post("/tenants/{id}/deployments/{depId}/rollback", srv.handleRollbackDeployment)

But handleRollbackDeployment never called chi.URLParam(r, "depId"). It read {id} for the tenant and stopped there.

How I Found It

I was auditing my published API docs against the actual handlers, endpoint by endpoint. When I got to the rollback entry I went to write down what {depId} did, went to the handler to confirm, and found nothing reading it. The docs described an ID that the code never looked at.

The worst part is that it returned 202 Accepted and then performed a real, successful rollback. Just not the one you asked for. There was no error to notice, no failed request in any log. The frontend had been passing the deployment ID into the URL since it was written (src/lib/api.ts), so the UI always believed the parameter was honored.

Root Cause

The handler created a rollback deployment row with no reference to any target, and the worker independently decided what to restore. In internal/worker/pipeline.go, runRollbackPipeline did this:

prevDep, err := cfg.Store.GetPreviousDeployment(ctx, tenant.ID)
if err != nil {
    return failPipeline(ctx, cfg, dep, "deploy",
        fmt.Errorf("no previous successful deployment to roll back to: %w", err))
}

GetPreviousDeployment was a single query: most recent deployment for this tenant where status='success' AND is_current=FALSE, ordered by version descending, limit 1. It is a perfectly good "roll back one version" implementation. It just is not "roll back to the version the user named," and nothing in between the HTTP layer and the worker was carrying that information. (That query has since changed again, for reasons in the last section.)

Why It Mattered

Rollback is the button you press when production is broken. If v9 is bad you might roll back to v8, find that also bad, and go for v5. On Staxa you would click v5 and get v8 again, three times in a row, while your tenant stayed down. Meanwhile the deployment history would show three successful rollbacks. The row it wrote was misleading too. It hardcoded ImageTag: "rollback", a placeholder that is not a real image reference, so the deployment history could not tell you what had actually been deployed even after the fact.

Then fixing it exposed a second, older bug underneath. More on that below.

Code

Three PRs on staxa-api-v2:

• #21 fix(deployments): roll back to the deployment named in the path (merged) - the fix itself, 332 insertions across 4 files
• #22 fix(deployments): scope is_current to the service, not the tenant - the schema bug the fix exposed
• #24 feat(deployments): snapshot deployment config so rollback replays it - the design problem the fix exposed

The Handler, Before and After (internal/api/deployments.go)

Before:

func (s *Server) handleRollbackDeployment(w http.ResponseWriter, r *http.Request) {
    provider := middleware.GetProvider(r.Context())
    tenantID := chi.URLParam(r, "id")
    // depId is never read
    if err := s.assertTenantOwnership(r, provider.ID, tenantID); err != nil {
        pkgapi.ErrNotFound(w, r, "tenant not found")
        return
    }
    version, err := s.store.GetNextDeploymentVersion(r.Context(), tenantID, "")
    if err != nil {
        pkgapi.ErrInternal(w, r)
        return
    }
    d := &store.Deployment{
        ID:          id.Generate("dep"),
        TenantID:    tenantID,
        ProviderID:  provider.ID,
        DeployType:  "rollback",
        Trigger:     "api",
        Status:      "pending",
        ImageTag:    "rollback",
        Version:     version,
        Metadata:    map[string]any{},
        CreatedAt:   time.Now(),
    }
    // ... create + enqueue
}

After:

func (s *Server) handleRollbackDeployment(w http.ResponseWriter, r *http.Request) {
    provider := middleware.GetProvider(r.Context())
    tenantID := chi.URLParam(r, "id")
    depID := chi.URLParam(r, "depId")
    if err := s.assertTenantOwnership(r, provider.ID, tenantID); err != nil {
        pkgapi.ErrNotFound(w, r, "tenant not found")
        return
    }
    target, err := s.store.GetDeployment(r.Context(), depID)
    if err != nil || target.TenantID != tenantID {
        pkgapi.ErrNotFound(w, r, "deployment not found")
        return
    }
    if err := validateRollbackTarget(target); err != nil {
        pkgapi.ErrUnprocessable(w, r, err.Error())
        return
    }
    version, err := s.store.GetNextDeploymentVersion(r.Context(), tenantID, "")
    if err != nil {
        pkgapi.ErrInternal(w, r)
        return
    }
    // The new deployment carries the target's image and source so the row is an
    // honest record of what ends up running; the pipeline reads the target back
    // out of metadata to decide what to deploy.
    d := &store.Deployment{
        ID:          id.Generate("dep"),
        TenantID:    tenantID,
        ProviderID:  provider.ID,
        ImageTag:    target.ImageTag,
        SourceRef:   target.SourceRef,
        SourceType:  target.SourceType,
        DeployType:  "rollback",
        Trigger:     "api",
        Status:      "pending",
        Version:     version,
        Metadata: map[string]any{
            "rollback_to":         target.ID,
            "rollback_to_version": target.Version,
        },
        CreatedAt:   time.Now(),
    }
    // ... create + enqueue
}

Validation Function

The validation rules are their own function so both the handler and the worker can apply them:

// validateRollbackTarget reports whether a deployment can be rolled back to.
// Only a deployment that finished successfully has an image known to run, and
// only a tenant-level one can be restored by the rollback pipeline (that
// pipeline redeploys the tenant's primary workload, so restoring one service's
// image through it would deploy that image tenant-wide).
func validateRollbackTarget(d *store.Deployment) error {
    if d.Status != "success" {
        return errors.New("can only roll back to a deployment that completed successfully")
    }
    if d.ServiceID != "" {
        return errors.New("cannot roll back to a service-scoped deployment, redeploy the service instead")
    }
    // "pending" and "rollback" are placeholders written at create time; a
    // deployment carrying one never had a real image assigned to it.
    switch d.ImageTag {
    case "", "pending", "rollback":
        return errors.New("deployment has no image to roll back to")
    }
    return nil
}

The Worker (internal/worker/pipeline.go)

The GetPreviousDeployment call became:

// resolveRollbackTarget returns the deployment whose image this rollback should
// restore. The API records the caller's chosen target under "rollback_to" in the
// rollback deployment's metadata; jobs enqueued before that field existed carry
// no target, so those fall back to the most recent successful deployment.
func resolveRollbackTarget(ctx context.Context, cfg WorkerConfig, dep *store.Deployment, tenantID string) (*store.Deployment, error) {
    targetID, _ := dep.Metadata["rollback_to"].(string)
    if targetID == "" {
        prev, err := cfg.Store.GetPreviousDeployment(ctx, tenantID)
        if err != nil {
            return nil, fmt.Errorf("no previous successful deployment to roll back to: %w", err)
        }
        return prev, nil
    }
    target, err := cfg.Store.GetDeployment(ctx, targetID)
    if err != nil {
        return nil, fmt.Errorf("rollback target %s not found: %w", targetID, err)
    }
    // Re-check what the handler checked. The target could have been deleted or
    // rewritten between enqueue and execution, and deploying the wrong image is
    // worse than failing the rollback.
    if target.TenantID != tenantID {
        return nil, fmt.Errorf("rollback target %s belongs to another tenant", targetID)
    }
    // Mirrors validateRollbackTarget: this pipeline deploys the result as the
    // tenant's primary workload, so a service-scoped row would be pushed
    // tenant-wide and then marked current in the service's is_current scope.
    if target.ServiceID != "" {
        return nil, fmt.Errorf("rollback target %s is service-scoped and cannot be restored tenant-wide", targetID)
    }
    switch target.ImageTag {
    case "", "pending", "rollback":
        return nil, fmt.Errorf("rollback target %s has no image to restore", targetID)
    }
    return target, nil
}

That ServiceID check is a late addition. It came out of code review, and the story behind it is below.

The Schema Bug Underneath (PR #22)

internal/store/migrations/004_deployments.sql:28, untouched since I wrote it:

CREATE UNIQUE INDEX idx_deployments_current ON deployments (tenant_id, is_current) WHERE is_current = TRUE;

At most one current deployment per tenant. Migration 027 replaces it:

DROP INDEX IF EXISTS idx_deployments_current;
CREATE UNIQUE INDEX idx_deployments_current ON deployments (tenant_id, COALESCE(service_id, '')) WHERE is_current = TRUE;

And SetCurrentDeployment in internal/store/deployments.go went from clearing the flag across the whole tenant:

// before
tx.Exec(ctx, `UPDATE deployments SET is_current=FALSE WHERE tenant_id=$1 AND is_current=TRUE`, tenantID)
tx.Exec(ctx, `UPDATE deployments SET is_current=TRUE WHERE id=$1`, deploymentID)

to deriving its scope from the target row, in one statement:

// after
_, err := s.pool.Exec(ctx, `
    UPDATE deployments d
    SET is_current = (d.id = $2)
    FROM deployments t
    WHERE t.id = $2
      AND d.tenant_id = $1
      AND COALESCE(d.service_id, '') = COALESCE(t.service_id, '')
      AND (d.is_current OR d.id = $2)
`, tenantID, deploymentID)

The signature is unchanged, so all five existing call sites got the correct scoping without edits.

My Improvements

Diagnosing It

The diagnosis was trivial once I looked. What is worth writing down is why I had not looked for four months. Rollback worked. I had tested it by deploying twice, clicking rollback, and watching the previous version come back up. With exactly two deployments, "the target you named" and "the most recent successful one" are the same row. My manual test could not distinguish the correct behavior from the bug, and I never went back with three versions.

Tracing it took two greps: grep -rn "depId" internal/api/ showed handleGetDeployment reading it and handleRollbackDeployment not, and reading runRollbackPipeline showed where the decision was actually being made.

How to Get the Target to the Worker

Jobs go through a Redis list as queue.Job structs. The obvious move was adding a TargetDeploymentID field to queue.Job. I put it in the deployment's Metadata map instead, for three reasons:

• Jobs are transient and the deployments table is not, so the metadata answers "what did that rollback restore?" months later, when the job is long gone.
• It survives a requeue, since the pipeline re-reads the deployment row by ID and the target comes back with it.
• It is already serialized into the API response, so the caller sees rollback_to and rollback_to_version without me adding response fields.

The cost is that it is a map[string]any, so the read is an unchecked type assertion (dep.Metadata["rollback_to"].(string)) instead of a typed field. I decided the durability was worth losing the compile-time check, and resolveRollbackTarget treats a missing or non-string value as "no target given" and falls back, which is also the migration path for jobs already sitting in Redis when the binary is replaced.

Validating Twice on Purpose

validateRollbackTarget runs in the handler, and resolveRollbackTarget re-checks tenant ownership and the image tag in the worker. That is deliberate duplication. Between the enqueue and the execution the target row can be deleted or rewritten, and jobs can retry minutes later. Deploying the wrong image is worse than failing the rollback, so the worker does not trust the ID it was handed.

The Bug the Fix Exposed

Rejecting service-scoped targets was correct for the pipeline as written, since runRollbackPipeline calls Orchestrator.DeployApp, which redeploys the tenant's primary workload. Restoring one service's image through it would push that image tenant-wide. But it meant multi-service tenants now had no rollback at all, because every deployment after tenant creation is service-scoped.

Before my fix the same call "worked" only by accident: GetPreviousDeployment filtered on tenant_id alone, so it could pick up a service-scoped row and deploy that service's image as the entire tenant. I had turned a silent corruption into a loud 422, which is an improvement, but it is still a regression from the user's side.

Chasing why a service row could surface there at all led to the index above. is_current was tenant-scoped at the schema level, left over from before Staxa supported multiple services per tenant. Migration 011 had already made the sibling version index service-aware, so this one was just missed.

The masking was almost funny: runServiceScopedRedeploy never called SetCurrentDeployment at all, so no service deployment was ever marked current, so the unique index was never violated and nobody noticed it was wrong. Two bugs cancelling out.

Adding the missing call without fixing the index would have started throwing SQLSTATE 23505 on the second service deploy. I confirmed the tests actually catch it by deleting migration 027 and re-running them:

--- FAIL: TestDeploymentsCurrentPerService/two_services_in_one_tenant_can_both_be_cu