From 21ac5beaa40c54545a2ad59c46f8bf4672ba1cc7 Mon Sep 17 00:00:00 2001 From: aman Date: Sun, 18 Oct 2026 14:31:31 +0530 Subject: [PATCH 1/2] feat(deleter): count live resources that block a project or org delete ProjectResourceBlockers and OrganizationResourceBlockers return one RESOURCES_EXIST blocker per namespace, with the number of live resources that the app.deletion rules say block the delete. The org check judges the resources of its projects by the organization rules. Nothing calls them yet. --- core/deleter/deleter.go | 20 ++-- core/deleter/resource_blockers.go | 79 +++++++++++++ core/deleter/service_test.go | 182 +++++++++++++++++++++++++++++- 3 files changed, 273 insertions(+), 8 deletions(-) create mode 100644 core/deleter/resource_blockers.go diff --git a/core/deleter/deleter.go b/core/deleter/deleter.go index cf968ce66..824b9b82e 100644 --- a/core/deleter/deleter.go +++ b/core/deleter/deleter.go @@ -2,32 +2,38 @@ package deleter import "strings" -// Reasons an organization delete can be blocked. The API error carries +// Reasons a delete can be blocked. The API error carries // them as violation types, so a client can tell the reasons apart // without reading the message text. const ( BlockerActiveSubscription = "ACTIVE_SUBSCRIPTION" BlockerUnpaidInvoice = "UNPAID_INVOICE" BlockerNegativeTokenBalance = "NEGATIVE_TOKEN_BALANCE" + BlockerResourcesExist = "RESOURCES_EXIST" ) // Kinds of entity a Blocker's Subject id refers to. const ( - SubjectSubscription = "billing_subscription" - SubjectInvoice = "billing_invoice" - SubjectBillingAccount = "billing_account" + SubjectSubscription = "billing_subscription" + SubjectInvoice = "billing_invoice" + SubjectBillingAccount = "billing_account" + SubjectResourceNamespace = "resource_namespace" ) -// Blocker is one reason an organization cannot be deleted right now. +// Blocker is one reason a project or an organization cannot be deleted right now. type Blocker struct { // Type is one of the Blocker* constants. Type string - // Subject is the id of the entity behind the reason. + // Subject is the id of the entity behind the reason, or the namespace + // for BlockerResourcesExist. Subject string // SubjectType is one of the Subject* constants and says what kind of // entity the Subject id refers to. SubjectType string - // Message says what blocks the delete and how to clear it. + // Count is the number of blocking resources, for BlockerResourcesExist. + Count int + // Message says what blocks the delete and how to clear it. Empty for + // BlockerResourcesExist. Message string } diff --git a/core/deleter/resource_blockers.go b/core/deleter/resource_blockers.go new file mode 100644 index 000000000..6b845ab3b --- /dev/null +++ b/core/deleter/resource_blockers.go @@ -0,0 +1,79 @@ +package deleter + +import ( + "context" + "maps" + "slices" + + "github.com/raystack/frontier/core/resource" +) + +const metadataDeletable = "deletable" + +func (d Service) ProjectResourceBlockers(ctx context.Context, projectID string) ([]Blocker, error) { + if !d.deletionConfig.Enabled { + return nil, nil + } + resources, err := d.resService.List(ctx, resource.Filter{ProjectID: projectID}) + if err != nil { + return nil, err + } + return d.deletionConfig.blockers(ParentProject, resources), nil +} + +// OrganizationResourceBlockers judges the org's own resources and those of +// all its projects by the organization rules. +func (d Service) OrganizationResourceBlockers(ctx context.Context, orgID string) ([]Blocker, error) { + if !d.deletionConfig.Enabled { + return nil, nil + } + // the org's own resources only; project resources are added below + resources, err := d.resService.List(ctx, resource.Filter{OrgID: orgID}) + if err != nil { + return nil, err + } + projects, err := d.listOrgProjects(ctx, orgID) + if err != nil { + return nil, err + } + for _, p := range projects { + projectResources, err := d.resService.List(ctx, resource.Filter{ProjectID: p.ID}) + if err != nil { + return nil, err + } + resources = append(resources, projectResources...) + } + return d.deletionConfig.blockers(ParentOrganization, resources), nil +} + +func (c Config) blockers(parent Parent, resources []resource.Resource) []Blocker { + counts := map[string]int{} + for _, r := range resources { + if c.blocks(parent, r) { + counts[r.NamespaceID]++ + } + } + var blockers []Blocker + for _, namespace := range slices.Sorted(maps.Keys(counts)) { + blockers = append(blockers, Blocker{ + Type: BlockerResourcesExist, + Subject: namespace, + SubjectType: SubjectResourceNamespace, + Count: counts[namespace], + }) + } + return blockers +} + +func (c Config) blocks(parent Parent, r resource.Resource) bool { + switch c.Resources[r.NamespaceID][parent] { + case RuleAllow: + return false + case RuleBlockUnlessDeletable: + deletable, _ := r.Metadata[metadataDeletable].(bool) + return !deletable + default: + // block, or no rule + return true + } +} diff --git a/core/deleter/service_test.go b/core/deleter/service_test.go index 94af4ca09..dbd6f81f3 100644 --- a/core/deleter/service_test.go +++ b/core/deleter/service_test.go @@ -60,6 +60,7 @@ type deleterMocks struct { kycSvc *mocks.KycService planSvc *mocks.PlanService dialer *mailermocks.Dialer + deletion deleter.Config } func newMocks(t *testing.T) deleterMocks { @@ -107,7 +108,7 @@ func (m deleterMocks) build() *deleter.Service { return deleter.NewCascadeDeleter(m.orgSvc, m.projSvc, m.resSvc, m.grpSvc, m.mbrSvc, m.polSvc, m.roleSvc, m.invSvc, m.usrSvc, m.patSvc, m.domainSvc, m.suSvc, m.custSvc, m.subSvc, m.invocSvc, m.checkoutSvc, m.creditSvc, m.kycSvc, - m.planSvc, m.dialer, billing.OrgDeleteNoticeConfig{}, deleter.Config{}) + m.planSvc, m.dialer, billing.OrgDeleteNoticeConfig{}, m.deletion) } func TestDeleteProject(t *testing.T) { @@ -1514,3 +1515,182 @@ func TestDeleteUser(t *testing.T) { assert.ErrorContains(t, err, "pat cleanup boom") }) } + +func blockerConfig() deleter.Config { + return deleter.Config{ + Enabled: true, + Resources: map[string]deleter.Rules{ + "compute/machine": { + deleter.ParentProject: deleter.RuleBlock, + deleter.ParentOrganization: deleter.RuleAllow, + }, + "compute/order": { + deleter.ParentProject: deleter.RuleBlockUnlessDeletable, + deleter.ParentOrganization: deleter.RuleBlockUnlessDeletable, + }, + "storage/file": { + deleter.ParentProject: deleter.RuleAllow, + deleter.ParentOrganization: deleter.RuleAllow, + }, + "storage/transfer": { // no project rule + deleter.ParentOrganization: deleter.RuleBlock, + }, + }, + } +} + +func resourcesExist(namespace string, count int) deleter.Blocker { + return deleter.Blocker{ + Type: deleter.BlockerResourcesExist, + Subject: namespace, + SubjectType: deleter.SubjectResourceNamespace, + Count: count, + } +} + +func TestProjectResourceBlockers(t *testing.T) { + t.Run("returns no blockers and reads nothing while disabled", func(t *testing.T) { + m := newMocks(t) + + blockers, err := m.build().ProjectResourceBlockers(context.Background(), "proj-1") + + assert.NoError(t, err) + assert.Empty(t, blockers) + }) + + t.Run("counts blocking resources per namespace", func(t *testing.T) { + m := newMocks(t) + m.deletion = blockerConfig() + m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-1"}). + Return([]resource.Resource{ + {ID: "m1", NamespaceID: "compute/machine"}, + {ID: "m2", NamespaceID: "compute/machine"}, + {ID: "o1", NamespaceID: "compute/order", Metadata: map[string]any{"deletable": true}}, + {ID: "o2", NamespaceID: "compute/order", Metadata: map[string]any{"deletable": false}}, + {ID: "o3", NamespaceID: "compute/order", Metadata: map[string]any{"deletable": "true"}}, + {ID: "o4", NamespaceID: "compute/order"}, + {ID: "f1", NamespaceID: "storage/file"}, + {ID: "t1", NamespaceID: "storage/transfer"}, + {ID: "r1", NamespaceID: "compute/report"}, + }, nil) + + blockers, err := m.build().ProjectResourceBlockers(context.Background(), "proj-1") + + assert.NoError(t, err) + assert.Equal(t, []deleter.Blocker{ + resourcesExist("compute/machine", 2), + resourcesExist("compute/order", 3), + resourcesExist("compute/report", 1), + resourcesExist("storage/transfer", 1), + }, blockers) + }) + + t.Run("returns no blockers when nothing blocks", func(t *testing.T) { + m := newMocks(t) + m.deletion = blockerConfig() + m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-1"}). + Return([]resource.Resource{ + {ID: "o1", NamespaceID: "compute/order", Metadata: map[string]any{"deletable": true}}, + {ID: "f1", NamespaceID: "storage/file"}, + }, nil) + + blockers, err := m.build().ProjectResourceBlockers(context.Background(), "proj-1") + + assert.NoError(t, err) + assert.Empty(t, blockers) + }) + + t.Run("returns error when resource list fails", func(t *testing.T) { + m := newMocks(t) + m.deletion = blockerConfig() + m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-1"}). + Return(nil, errors.New("db error")) + + _, err := m.build().ProjectResourceBlockers(context.Background(), "proj-1") + + assert.ErrorContains(t, err, "db error") + }) +} + +func TestOrganizationResourceBlockers(t *testing.T) { + t.Run("returns no blockers and reads nothing while disabled", func(t *testing.T) { + m := newMocks(t) + + blockers, err := m.build().OrganizationResourceBlockers(context.Background(), "org-1") + + assert.NoError(t, err) + assert.Empty(t, blockers) + }) + + t.Run("judges the org's own and its projects' resources by the organization rules", func(t *testing.T) { + m := newMocks(t) + m.deletion = blockerConfig() + m.resSvc.EXPECT().List(mock.Anything, resource.Filter{OrgID: "org-1"}). + Return([]resource.Resource{ + {ID: "t1", NamespaceID: "storage/transfer"}, + {ID: "f1", NamespaceID: "storage/file"}, + }, nil) + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). + Return([]project.Project{{ID: "proj-1"}, {ID: "proj-2"}}, nil) + m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-1"}). + Return([]resource.Resource{ + {ID: "m1", NamespaceID: "compute/machine"}, + {ID: "m2", NamespaceID: "compute/machine"}, + {ID: "o1", NamespaceID: "compute/order", Metadata: map[string]any{"deletable": false}}, + }, nil) + m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-2"}). + Return([]resource.Resource{ + {ID: "o2", NamespaceID: "compute/order"}, + {ID: "o3", NamespaceID: "compute/order", Metadata: map[string]any{"deletable": true}}, + {ID: "r1", NamespaceID: "compute/report"}, + }, nil) + + blockers, err := m.build().OrganizationResourceBlockers(context.Background(), "org-1") + + assert.NoError(t, err) + assert.Equal(t, []deleter.Blocker{ + resourcesExist("compute/order", 2), + resourcesExist("compute/report", 1), + resourcesExist("storage/transfer", 1), + }, blockers) + }) + + t.Run("returns error when the org's own resource list fails", func(t *testing.T) { + m := newMocks(t) + m.deletion = blockerConfig() + m.resSvc.EXPECT().List(mock.Anything, resource.Filter{OrgID: "org-1"}). + Return(nil, errors.New("db error")) + + _, err := m.build().OrganizationResourceBlockers(context.Background(), "org-1") + + assert.ErrorContains(t, err, "db error") + }) + + t.Run("returns error when project list fails", func(t *testing.T) { + m := newMocks(t) + m.deletion = blockerConfig() + m.resSvc.EXPECT().List(mock.Anything, resource.Filter{OrgID: "org-1"}). + Return([]resource.Resource{}, nil) + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). + Return(nil, errors.New("db error")) + + _, err := m.build().OrganizationResourceBlockers(context.Background(), "org-1") + + assert.ErrorContains(t, err, "db error") + }) + + t.Run("returns error when a project's resource list fails", func(t *testing.T) { + m := newMocks(t) + m.deletion = blockerConfig() + m.resSvc.EXPECT().List(mock.Anything, resource.Filter{OrgID: "org-1"}). + Return([]resource.Resource{}, nil) + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). + Return([]project.Project{{ID: "proj-1"}}, nil) + m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-1"}). + Return(nil, errors.New("db error")) + + _, err := m.build().OrganizationResourceBlockers(context.Background(), "org-1") + + assert.ErrorContains(t, err, "db error") + }) +} From 2f3504894a44abbccaca19eb1c6398e99abd198a Mon Sep 17 00:00:00 2001 From: aman Date: Tue, 20 Oct 2026 13:35:34 +0530 Subject: [PATCH 2/2] refactor(deleter): name the project or org in resource list errors Also drops comments the names already cover, and adds tests for an org with no projects and a project with no resources. --- core/deleter/resource_blockers.go | 13 ++++------- core/deleter/service_test.go | 39 +++++++++++++++++++++++++++---- 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/core/deleter/resource_blockers.go b/core/deleter/resource_blockers.go index 6b845ab3b..e0feda598 100644 --- a/core/deleter/resource_blockers.go +++ b/core/deleter/resource_blockers.go @@ -2,6 +2,7 @@ package deleter import ( "context" + "fmt" "maps" "slices" @@ -16,30 +17,27 @@ func (d Service) ProjectResourceBlockers(ctx context.Context, projectID string) } resources, err := d.resService.List(ctx, resource.Filter{ProjectID: projectID}) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to list resources of project[%s]: %w", projectID, err) } return d.deletionConfig.blockers(ParentProject, resources), nil } -// OrganizationResourceBlockers judges the org's own resources and those of -// all its projects by the organization rules. func (d Service) OrganizationResourceBlockers(ctx context.Context, orgID string) ([]Blocker, error) { if !d.deletionConfig.Enabled { return nil, nil } - // the org's own resources only; project resources are added below resources, err := d.resService.List(ctx, resource.Filter{OrgID: orgID}) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to list resources of organization[%s]: %w", orgID, err) } projects, err := d.listOrgProjects(ctx, orgID) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to list projects of organization[%s]: %w", orgID, err) } for _, p := range projects { projectResources, err := d.resService.List(ctx, resource.Filter{ProjectID: p.ID}) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to list resources of project[%s]: %w", p.ID, err) } resources = append(resources, projectResources...) } @@ -73,7 +71,6 @@ func (c Config) blocks(parent Parent, r resource.Resource) bool { deletable, _ := r.Metadata[metadataDeletable].(bool) return !deletable default: - // block, or no rule return true } } diff --git a/core/deleter/service_test.go b/core/deleter/service_test.go index dbd6f81f3..b6514c2d2 100644 --- a/core/deleter/service_test.go +++ b/core/deleter/service_test.go @@ -1532,7 +1532,7 @@ func blockerConfig() deleter.Config { deleter.ParentProject: deleter.RuleAllow, deleter.ParentOrganization: deleter.RuleAllow, }, - "storage/transfer": { // no project rule + "storage/transfer": { deleter.ParentOrganization: deleter.RuleBlock, }, }, @@ -1600,6 +1600,18 @@ func TestProjectResourceBlockers(t *testing.T) { assert.Empty(t, blockers) }) + t.Run("returns no blockers for a project with no resources", func(t *testing.T) { + m := newMocks(t) + m.deletion = blockerConfig() + m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-1"}). + Return([]resource.Resource{}, nil) + + blockers, err := m.build().ProjectResourceBlockers(context.Background(), "proj-1") + + assert.NoError(t, err) + assert.Empty(t, blockers) + }) + t.Run("returns error when resource list fails", func(t *testing.T) { m := newMocks(t) m.deletion = blockerConfig() @@ -1608,7 +1620,7 @@ func TestProjectResourceBlockers(t *testing.T) { _, err := m.build().ProjectResourceBlockers(context.Background(), "proj-1") - assert.ErrorContains(t, err, "db error") + assert.ErrorContains(t, err, "failed to list resources of project[proj-1]: db error") }) } @@ -1655,6 +1667,23 @@ func TestOrganizationResourceBlockers(t *testing.T) { }, blockers) }) + t.Run("judges the org's own resources when it has no projects", func(t *testing.T) { + m := newMocks(t) + m.deletion = blockerConfig() + m.resSvc.EXPECT().List(mock.Anything, resource.Filter{OrgID: "org-1"}). + Return([]resource.Resource{ + {ID: "t1", NamespaceID: "storage/transfer"}, + {ID: "f1", NamespaceID: "storage/file"}, + }, nil) + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). + Return([]project.Project{}, nil) + + blockers, err := m.build().OrganizationResourceBlockers(context.Background(), "org-1") + + assert.NoError(t, err) + assert.Equal(t, []deleter.Blocker{resourcesExist("storage/transfer", 1)}, blockers) + }) + t.Run("returns error when the org's own resource list fails", func(t *testing.T) { m := newMocks(t) m.deletion = blockerConfig() @@ -1663,7 +1692,7 @@ func TestOrganizationResourceBlockers(t *testing.T) { _, err := m.build().OrganizationResourceBlockers(context.Background(), "org-1") - assert.ErrorContains(t, err, "db error") + assert.ErrorContains(t, err, "failed to list resources of organization[org-1]: db error") }) t.Run("returns error when project list fails", func(t *testing.T) { @@ -1676,7 +1705,7 @@ func TestOrganizationResourceBlockers(t *testing.T) { _, err := m.build().OrganizationResourceBlockers(context.Background(), "org-1") - assert.ErrorContains(t, err, "db error") + assert.ErrorContains(t, err, "failed to list projects of organization[org-1]: db error") }) t.Run("returns error when a project's resource list fails", func(t *testing.T) { @@ -1691,6 +1720,6 @@ func TestOrganizationResourceBlockers(t *testing.T) { _, err := m.build().OrganizationResourceBlockers(context.Background(), "org-1") - assert.ErrorContains(t, err, "db error") + assert.ErrorContains(t, err, "failed to list resources of project[proj-1]: db error") }) }