feat(admin): paginate/filter app listings, add deployment delete + purge#237
Merged
Merged
Conversation
The three admin app listings returned every row as a bare `{data: [...]}`
with no query parameters at all, and there was no way to delete an app
deployment or to remove a subscription that had ever been paid. Both
issues land together because they rewrite the same DB query paths.
Pagination and filters (#235)
`GET /api/admin/v1/apps`, `/app_clusters` and `/app-deployments` now take
limit/offset (default 50, max 100) and return the standard
ApiPaginatedData envelope every other admin list uses, ordered id DESC.
Filters are pushed into SQL via a new FilteredQuery helper that builds
the COUNT and the page from one set of conditions, rather than being
applied in the handler.
Deployment deletion is a soft delete, so a deleted deployment was
previously invisible to admins entirely. `include_deleted=true` now
surfaces it and AdminAppDeploymentInfo carries `deleted`. `app` and
`app_cluster` have no soft-delete, so the flag does not apply there -
`enabled` is their only visibility filter.
Delete and purge (#234)
New `DELETE /api/admin/v1/app-deployments/{id}`: deactivates billing and
soft-deletes, and - following the VM rule - removes a never-paid
deployment entirely. `{"purge": true}` hard-deletes the row with its
subscription, line items and payments in one transaction, super_admin
only, authorized before the lookup exactly like the VM purge. The
subscription is kept if it still bills another resource, so a mixed
subscription cannot take a VM's billing down with it.
Purging does not orphan Kubernetes resources: the operator GCs any
`app-{id}` namespace absent from its active set, and a purged row is
absent exactly like a soft-deleted one.
The same `purge` flag on `DELETE /api/admin/v1/subscriptions/{id}`
bypasses the paid-payments guard, refusing while a VM or deployment
still references one of the line items.
No migration needed - the app_deployment permission set including
`delete` was already seeded for super_admin.
Fixes #234
Fixes #235
Co-authored-by: Kieran <kieran@harkin.me>
Signed-off-by: Kieran <kieran@harkin.me>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #234
Fixes #235
Both issues land in one branch because they rewrite the same three DB query paths — doing them separately means rewriting
list_all_app_deploymentstwice.#235 — pagination, filters,
include_deletedGET /api/admin/v1/apps,/app_clustersand/app-deploymentspreviously returned every row as a bare{"data": [...]}with no query parameters at all. They now acceptlimit/offset(default 50, max 100) and return the standardApiPaginatedDataenvelope used by every other admin list, orderedid DESC.Filters are pushed into SQL rather than applied in the handler, via a new
FilteredQueryhelper (lnvps_db/src/mysql.rs) that builds theCOUNT(*)and the row page from one set of conditions:/app-deploymentsuser_id,app_id,cluster_id,region_id,status,desired_state,search,include_deleted/appsenabled,search/app_clustersenabled,region_id,searchregion_idon deployments resolves through the cluster.searchis a case-insensitive substring match with LIKE wildcards escaped so the term matches literally.Because deployment deletion is a soft delete, a deleted deployment was previously invisible to admins entirely.
include_deleted=true(defaultfalse) now surfaces it, andAdminAppDeploymentInfogains adeletedboolean.Breaking for admin clients. The response shape of all three listings changes from
{data}to{data, total, limit, offset}. The issue explicitly asks for the standard envelope so the dashboard's shared paginated table can drive these lists, so this takes the break rather than versioning the endpoints. Called out inAPI_CHANGELOG.md.#234 — delete and purge
New
DELETE /api/admin/v1/app-deployments/{id}(app_deployment::delete): deactivates billing and soft-deletes, and the operator tears down the namespace and volumes on its next reconcile. Following the rule VMs already use, a deployment whose first payment was never confirmed carries no billing history and is removed entirely instead.Optional
{"purge": true}hard-deletes the row together with itssubscription,subscription_line_itemand payment rows in one transaction.super_adminonly, rejected403for everyone else, authorized before the lookup — exactly like the VM purge. An already soft-deleted deployment can still be purged; a plain delete of one returns409.The same
purgeflag is added toDELETE /api/admin/v1/subscriptions/{id}, bypassing the "N paid payments exist" guard and cascading line items and payments. Regular deletes keep today's behaviour.Three decisions that differ from the issue text
1. The purge/teardown concern does not apply. #234 asks that a purge be "rejected (or deferred) until the operator has actually torn the namespace/PVCs down, so we don't orphan Kubernetes resources." It doesn't need to be.
gc_namespaces(lnvps_operator/src/app_deployments.rs:1167) collects anyapp-{id}namespace whose id is absent from the set of live deployments — and a hard-deleted row is absent from that set exactly like a soft-deleted one. The namespace and its PVCs are torn down on the next reconcile either way. No deferral, no orphans.2. No migration. #234 asks for the
app_deployment::deletepermission to be seeded.20260725150000_app_deployment_rbac_permissions.sqlalready grants actions 0–3 on resource 27 tosuper_admin.3.
include_deletedonly exists on deployments. #234 asks for it on all three listings, butappandapp_clusterhave nodeletedcolumn — onlyenabled, and the admin listings already returned disabled rows. A parameter that silently does nothing is worse than not having it, so those two getenabledand the docs say why.Guards worth reviewing
hard_delete_app_deploymentkeeps the subscription if it still bills another resource, so a mixed subscription cannot take a VM's billing (and its FK) down with the deployment.hard_delete_subscriptionrefuses while a VM or app deployment still references one of the subscription's line items, and names which resource is in the way.Adjacent pre-existing bug, deliberately not fixed here
admin_delete_appandadmin_delete_app_clusterguard onlist_all_app_deployments(), which excludes soft-deleted rows — but a soft-deleted deployment still holds the FK toapp/app_cluster. Deleting an app whose only deployments are soft-deleted passes the guard and then fails on the foreign key as a 500. Customers could already reach that state via the customer delete before this PR, so it is not new; left alone rather than widening scope. Happy to file it separately.Testing
734 unit tests pass (
cargo test --workspace --exclude lnvps_e2e -- --test-threads=1) atfce5788, working tree clean.cargo clippyis clean on every file touched.New coverage:
lnvps_api_common/src/mock.rs) — every filter on all three listings, pagination totals versus page size, blank-search handling,include_deleted, both hard-delete cascades, the mixed-subscription carve-out, and both purge guards.lnvps_e2e/src/admin_api.rs,rbac.rs) — the paginated envelope, filters andinclude_deletedover HTTP, the soft-delete → 409 → purge sequence with DB assertions, the never-paid purge, the subscription purge including the refusal paths, and 403 for a role that holdsapp_deployment::deletebut is notsuper_admin.docker compose up -dlocally) before this merges.Two build failures in that environment are pre-existing and reproduce on
master:lnvps_healthdoes not build on macOS (Linux-onlylibcMTU constants) andlnvps_api --all-featureshas aBitvoraNodetrait error.Opened from the
generalBuzz channel (f5894ea9-44c7-56fb-bd9b-6e3e671e4bc1).