Repository navigation
feat(deleter): org delete refuses while resources block it - #2017
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 51 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check |  |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Coverage Report for CI Build 39895381862Coverage increased (+0.009%) to 55.729%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
2e14945 to
8f2fc33
Compare
3997c73 to
0adf05a
Compare
8f2fc33 to
fefd216
Compare
8670b16 to
8884f71
Compare
DeleteOrganization and CheckOrganizationDelete also report the resources the deletion config says block an org delete, in the same list as the billing blockers, judged by the organization rules over the org's own resources and every project's. The delete checks them before anything is canceled or deleted.
The e2e test adds a resource created with CreateOrganizationResource and checks that it blocks the org delete on its own. A test comment that the subtest name already says is removed.
8884f71 to
71275d9
Compare
Summary
DeleteOrganizationandCheckOrganizationDeletenow also report the resources that the deletion config says block an organization delete, asRESOURCES_EXISTblockers in the same list as the billing blockers. The org's own resources and the resources of every project in it, disabled projects included, are judged by theorganizationrules. The delete runs this check before anything is canceled or deleted. No delete is refused for resources whileapp.deletionis disabled. Stacked on #2011.Changes
core/deleter/service.go:collectBlockerstakes the org id and addsOrganizationResourceBlockersafter the billing blockers.ensureDeletablecalls it in its first pass, before a free-plan subscription is canceled;CheckOrganizationDeletecalls it too.test/e2e/regression/api_test.go: the API suite's deletion config gains a rule forcompute/disk(project: block,organization: block_unless_deletable).Technical Details
organization cannot be deleted yet; resource blockers have no message.failed_preconditionis HTTP 400.Caveats
internalat the permission step. This is pre-existing. The org delete now shows it, because a disabled project's resources count.CheckOrganizationDeleteresponse. A refused delete lists each blocker's type and namespace only, becausegoogle.rpc.PreconditionFailurehas no field for a count. This is pre-existing and the same for the project delete.can_deleteis left out of the JSON response when it is false (proto3 default), so a client must read a missing value as false. This is pre-existing and the same forCheckProjectDelete.Test Plan
core/deleter/service_test.goTestDeleteOrganization: resources that block an org delete stop it before a free-plan subscription is canceled, and nothing is removed; a failed resource check stops the delete before anything changes; an org whose project holds a resource that only blocks a project delete is deleted.TestCheckOrganizationDelete: the resource blockers come after the billing blocker, with their counts.internal/api/v1beta1connect/deleter_test.go:TestHandler_CheckOrganizationDeletecopiescountfor a resource blocker;TestHandler_DeleteOrganizationanswers a mixed refusal with one violation per blocker, the resource violation without a description, and the billing messages without an empty piece.TestOrganizationDeleteRefusedWhileResourcesBlock: the project holds onecompute/diskresource without the flag and one withdeletable: true, and the org holds one morecompute/diskresource of its own, made withCreateOrganizationResource. The check reports one blocker with count 2, the delete is refused with that violation, and the org, the project and both blocking resources stay. Once the project's blocking resource is deleted, the org's own resource still blocks with count 1. Once that one is deleted too, the project delete is still refused by the project rule, the check reportscan_delete, and the org delete succeeds.go test ./core/... ./internal/api/... ./pkg/server/...passes.go test ./test/e2e/...passes.golangci-lint run ./core/deleter/... ./internal/api/v1beta1connect/... ./test/e2e/regression/...reports no issues.Sandbox results
A binary built from this branch, run against Postgres and SpiceDB, with this deletion config:
Each scenario uses its own new org, and the org owner makes the calls. 49 of 49 checks passed.
CheckOrganizationDeleteDeleteOrganizationcan_deletetruedeleted_at; a second delete or check answerspermission_denied(pre-existing)can_deletetrueresource/orderitem(1)permission_deniedon both RPCs (pre-existing)deletable: truecan_deletetruedeletable: falseresource/orderitem(1)deletable: "true"(a string)resource/orderitem(1)truecountsresource/report, which has no ruleresource/report(1)resource/orderitem(1)internal(pre-existing); after enabling the project and deleting it, the check reportscan_deleteand the delete succeedsresource/transfer(1)UpdateOrganizationResourcesetsdeletable: true, the check reportscan_deleteand the delete succeedsresource/orderitem(2),resource/report(1),resource/transfer(1)NEGATIVE_TOKEN_BALANCE, thenresource/orderitem(1)organization cannot be deleted yet: billing account[<id>] owes 50 tokens: contact support to settle the balance, then retry the deletepermission_denied, for a platform superuser tooenabled: falsecan_deletetrueThe sandbox has no billing provider. The billing account in S11 is an offline one, its debt is one debit row written with SQL, and its delete was not run past the refusal.
ACTIVE_SUBSCRIPTIONandUNPAID_INVOICEneed a billing provider and are covered by the unit tests.Refused delete in S10:
{ "code": "failed_precondition", "message": "organization cannot be deleted yet", "details": [{ "type": "google.rpc.PreconditionFailure", "debug": { "violations": [ { "type": "RESOURCES_EXIST", "subject": "resource/orderitem" }, { "type": "RESOURCES_EXIST", "subject": "resource/report" }, { "type": "RESOURCES_EXIST", "subject": "resource/transfer" } ] } }] }All 49 checks
UpdateOrganizationResource