feat(deleter): org delete refuses while resources block it

core/deleter/service.go

@@ -616,7 +616,7 @@ func (d Service) ensureDeletable(ctx context.Context, id string, customers []cus
 	paidPlans := map[string]bool{}
 
 	// the delete is about to happen, so judge invoices on fresh provider data
-	blockers, err := d.collectBlockers(ctx, customers, true, balances, paidPlans)
+	blockers, err := d.collectBlockers(ctx, id, customers, true, balances, paidPlans)
 	if err != nil {
 		return err
 	}
@@ -707,16 +707,17 @@ func (d Service) CheckOrganizationDelete(ctx context.Context, id string) ([]Bloc
 	if err != nil {
 		return nil, err
 	}
-	return d.collectBlockers(ctx, customers, false, nil, map[string]bool{})
+	return d.collectBlockers(ctx, id, customers, false, nil, map[string]bool{})
 }
 
 // collectBlockers gathers the blockers across the org's billing accounts:
 // running subscriptions on a paid plan, invoices that still ask for money,
-// and token debt. fromProvider judges the invoices on data read straight
+// and token debt. It also adds the resources the deletion config says block
+// an org delete. fromProvider judges the invoices on data read straight
 // from the billing provider instead of the local rows. balances carries
 // account balances the caller already read, nil means read them here; the
 // plan lookups land in the caller's paidPlans cache.
-func (d Service) collectBlockers(ctx context.Context, customers []customer.Customer, fromProvider bool, balances map[string]int64, paidPlans map[string]bool) ([]Blocker, error) {
+func (d Service) collectBlockers(ctx context.Context, orgID string, customers []customer.Customer, fromProvider bool, balances map[string]int64, paidPlans map[string]bool) ([]Blocker, error) {
 	var blockers []Blocker
 	for _, c := range customers {
 		if !c.IsOffline() {
@@ -756,7 +757,12 @@ func (d Service) collectBlockers(ctx context.Context, customers []customer.Custo
 			})
 		}
 	}
-	return blockers, nil
+
+	resourceBlockers, err := d.OrganizationResourceBlockers(ctx, orgID)
+	if err != nil {
+		return nil, err
+	}
+	return append(blockers, resourceBlockers...), nil
 }
 
 // subscriptionBlockers returns a blocker for every running subscription on a

core/deleter/service_test.go

@@ -507,6 +507,8 @@ func TestDeleteOrganization(t *testing.T) {
 			Return([]customer.Customer{}, nil)
 		m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}).
 			Return([]project.Project{{ID: "proj-1", Name: "p1"}}, nil)
+		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-1"}).
+			Return([]resource.Resource{{ID: "m1", NamespaceID: "compute/machine", Name: "machine-1"}}, nil)
 		m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: "proj-1"}).
 			Return([]policy.Policy{}, nil)
 		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-1"}).
@@ -695,6 +697,57 @@ func TestDeleteOrganization(t *testing.T) {
 		}, types)
 	})
 
+	t.Run("resources that block an org delete stop it before the subscription is canceled", func(t *testing.T) {
+		m := newMocks(t)
+		m.deletion = blockerConfig()
+
+		c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"}
+		m.orgSvc.EXPECT().GetRaw(mock.Anything, "org-1").
+			Return(organization.Organization{ID: "org-1"}, nil)
+		m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}).
+			Return([]customer.Customer{c}, nil)
+		m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}).
+			Return([]subscription.Subscription{{ID: "sub-1", State: "active", PlanID: "plan-free"}}, nil)
+		m.invocSvc.EXPECT().ListPayableOnProvider(mock.Anything, c).
+			Return([]invoice.Invoice{}, nil)
+		m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(0, nil)
+		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{OrgID: "org-1"}).
+			Return([]resource.Resource{{ID: "t1", NamespaceID: "storage/transfer"}}, 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([]resource.Resource{
+				{ID: "m1", NamespaceID: "compute/machine"},
+				{ID: "o1", NamespaceID: "compute/order"},
+			}, nil)
+
+		err := m.build().DeleteOrganization(context.Background(), "org-1")
+
+		var blocked *deleter.BlockedError
+		assert.ErrorAs(t, err, &blocked)
+		assert.Equal(t, "org-1", blocked.OrgID)
+		assert.Equal(t, []deleter.Blocker{
+			resourcesExist("compute/order", 1),
+			resourcesExist("storage/transfer", 1),
+		}, blocked.Blockers)
+		assert.EqualError(t, err, "organization cannot be deleted yet")
+	})
+
+	t.Run("a failed resource check stops the delete before anything changes", func(t *testing.T) {
+		m := newMocks(t)
+		m.deletion = blockerConfig()
+
+		m.orgSvc.EXPECT().GetRaw(mock.Anything, "org-1").
+			Return(organization.Organization{ID: "org-1"}, nil)
+		m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}).
+			Return([]customer.Customer{}, nil)
+		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{OrgID: "org-1"}).
+			Return(nil, errors.New("db error"))
+
+		err := m.build().DeleteOrganization(context.Background(), "org-1")
+		assert.ErrorContains(t, err, "failed to list resources of organization[org-1]: db error")
+	})
+
 	t.Run("paid plan subscription blocks until the caller downgrades", func(t *testing.T) {
 		m := newMocks(t)
 
@@ -1411,6 +1464,41 @@ func TestCheckOrganizationDelete(t *testing.T) {
 		}, subjectTypes)
 	})
 
+	t.Run("reports the resource blockers with their counts after the billing ones", func(t *testing.T) {
+		m := newMocks(t)
+		m.deletion = blockerConfig()
+
+		c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"}
+		m.orgSvc.EXPECT().GetRaw(mock.Anything, "org-1").
+			Return(organization.Organization{ID: "org-1"}, nil)
+		m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}).
+			Return([]customer.Customer{c}, nil)
+		m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}).
+			Return([]subscription.Subscription{{ID: "sub-1", State: "active", PlanID: "plan-paid"}}, nil)
+		m.invocSvc.EXPECT().List(mock.Anything, invoice.Filter{CustomerID: "cust-1", NonZeroOnly: true}).
+			Return([]invoice.Invoice{}, nil)
+		m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(0, nil)
+		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{OrgID: "org-1"}).
+			Return([]resource.Resource{{ID: "t1", NamespaceID: "storage/transfer"}}, 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([]resource.Resource{
+				{ID: "o1", NamespaceID: "compute/order"},
+				{ID: "o2", NamespaceID: "compute/order"},
+			}, nil)
+
+		blockers, err := m.build().CheckOrganizationDelete(context.Background(), "org-1")
+		assert.NoError(t, err)
+		if assert.Len(t, blockers, 3) {
+			assert.Equal(t, deleter.BlockerActiveSubscription, blockers[0].Type)
+			assert.Equal(t, []deleter.Blocker{
+				resourcesExist("compute/order", 2),
+				resourcesExist("storage/transfer", 1),
+			}, blockers[1:])
+		}
+	})
+
 	t.Run("returns empty when there are no blockers", func(t *testing.T) {
 		m := newMocks(t)
 

internal/api/v1beta1connect/deleter_test.go

@@ -177,6 +177,7 @@ func TestHandler_DeleteOrganization(t *testing.T) {
 			Blockers: []deleter.Blocker{
 				{Type: deleter.BlockerUnpaidInvoice, Subject: "inv-1", Message: "invoice[inv-1] is unpaid: pay it via its hosted payment page, then retry the delete"},
 				{Type: deleter.BlockerNegativeTokenBalance, Subject: "cust-1", Message: "billing account[cust-1] owes 50 tokens: contact support to settle the balance, then retry the delete"},
+				{Type: deleter.BlockerResourcesExist, Subject: "compute/order", SubjectType: deleter.SubjectResourceNamespace, Count: 2},
 			},
 		}
 		mockDelOrg := new(mocks.CascadeDeleter)
@@ -191,19 +192,24 @@ func TestHandler_DeleteOrganization(t *testing.T) {
 		var connectErr *connect.Error
 		assert.ErrorAs(t, err, &connectErr)
 		assert.Equal(t, connect.CodeFailedPrecondition, connectErr.Code())
-		assert.Contains(t, connectErr.Message(), "pay it")
-		assert.Contains(t, connectErr.Message(), "contact support")
+		assert.Equal(t, "organization cannot be deleted yet: "+
+			"invoice[inv-1] is unpaid: pay it via its hosted payment page, then retry the delete; "+
+			"billing account[cust-1] owes 50 tokens: contact support to settle the balance, then retry the delete",
+			connectErr.Message())
 
 		assert.Len(t, connectErr.Details(), 1)
 		detail, detailErr := connectErr.Details()[0].Value()
 		assert.NoError(t, detailErr)
 		failure, ok := detail.(*errdetails.PreconditionFailure)
 		assert.True(t, ok)
-		assert.Len(t, failure.GetViolations(), 2)
+		assert.Len(t, failure.GetViolations(), 3)
 		assert.Equal(t, deleter.BlockerUnpaidInvoice, failure.GetViolations()[0].GetType())
 		assert.Equal(t, "inv-1", failure.GetViolations()[0].GetSubject())
 		assert.Equal(t, deleter.BlockerNegativeTokenBalance, failure.GetViolations()[1].GetType())
 		assert.Equal(t, "cust-1", failure.GetViolations()[1].GetSubject())
+		assert.Equal(t, deleter.BlockerResourcesExist, failure.GetViolations()[2].GetType())
+		assert.Equal(t, "compute/order", failure.GetViolations()[2].GetSubject())
+		assert.Empty(t, failure.GetViolations()[2].GetDescription())
 	})
 }
 
@@ -213,6 +219,7 @@ func TestHandler_CheckOrganizationDelete(t *testing.T) {
 		mockDel.EXPECT().CheckOrganizationDelete(mock.Anything, "some-id").Return([]deleter.Blocker{
 			{Type: deleter.BlockerActiveSubscription, Subject: "sub-1", SubjectType: deleter.SubjectSubscription, Message: "subscription[sub-1] is active on a paid plan: downgrade to the standard plan, then retry the delete"},
 			{Type: deleter.BlockerUnpaidInvoice, Subject: "inv-1", SubjectType: deleter.SubjectInvoice, Message: "invoice[inv-1] is unpaid: pay it via its hosted payment page, then retry the delete"},
+			{Type: deleter.BlockerResourcesExist, Subject: "compute/order", SubjectType: deleter.SubjectResourceNamespace, Count: 2},
 		}, nil)
 		mockDep := &ConnectHandler{deleterService: mockDel}
 
@@ -221,12 +228,16 @@ func TestHandler_CheckOrganizationDelete(t *testing.T) {
 		}))
 		assert.NoError(t, err)
 		assert.False(t, resp.Msg.GetCanDelete())
-		assert.Len(t, resp.Msg.GetBlockers(), 2)
+		assert.Len(t, resp.Msg.GetBlockers(), 3)
 		assert.Equal(t, deleter.BlockerActiveSubscription, resp.Msg.GetBlockers()[0].GetType())
 		assert.Equal(t, "sub-1", resp.Msg.GetBlockers()[0].GetSubject())
 		assert.Equal(t, deleter.SubjectSubscription, resp.Msg.GetBlockers()[0].GetSubjectType())
 		assert.Equal(t, deleter.BlockerUnpaidInvoice, resp.Msg.GetBlockers()[1].GetType())
 		assert.Equal(t, deleter.SubjectInvoice, resp.Msg.GetBlockers()[1].GetSubjectType())
+		assert.Equal(t, deleter.BlockerResourcesExist, resp.Msg.GetBlockers()[2].GetType())
+		assert.Equal(t, "compute/order", resp.Msg.GetBlockers()[2].GetSubject())
+		assert.Equal(t, deleter.SubjectResourceNamespace, resp.Msg.GetBlockers()[2].GetSubjectType())
+		assert.Equal(t, int32(2), resp.Msg.GetBlockers()[2].GetCount())
 	})
 
 	t.Run("should report can_delete true when nothing blocks", func(t *testing.T) {

test/e2e/regression/api_test.go

@@ -107,6 +107,7 @@ func (s *APIRegressionTestSuite) SetupSuite() {
 				Enabled: true,
 				Resources: map[string]deleter.Rules{
 					computeOrderNamespace:  {deleter.ParentProject: deleter.RuleBlock, deleter.ParentOrganization: deleter.RuleAllow},
+					computeDiskNamespace:   {deleter.ParentProject: deleter.RuleBlock, deleter.ParentOrganization: deleter.RuleBlockUnlessDeletable},
 					computeVolumeNamespace: {deleter.ParentProject: deleter.RuleAllow, deleter.ParentOrganization: deleter.RuleAllow},
 				},
 			},
@@ -4174,6 +4175,126 @@ func (s *APIRegressionTestSuite) TestProjectDeleteRefusedWhileResourcesBlock() {
 	})
 }
 
+func (s *APIRegressionTestSuite) TestOrganizationDeleteRefusedWhileResourcesBlock() {
+	ctxOrgAdminAuth := testbench.ContextWithAuth(context.Background(), s.adminCookie)
+
+	createOrgResp, err := s.testBench.Client.CreateOrganization(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{
+		Body: &frontierv1beta1.OrganizationRequestBody{
+			Title: "org delete blockers",
+			Name:  "org-delete-blockers",
+		},
+	}))
+	s.Require().NoError(err)
+	orgID := createOrgResp.Msg.GetOrganization().GetId()
+
+	projectResp, err := s.testBench.Client.CreateProject(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateProjectRequest{
+		Body: &frontierv1beta1.ProjectRequestBody{Name: "org-delete-blockers-project", OrgId: orgID},
+	}))
+	s.Require().NoError(err)
+	projectID := projectResp.Msg.GetProject().GetId()
+
+	createDisk := func(name string, metadata *structpb.Struct) string {
+		resp, err := s.testBench.Client.CreateProjectResource(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateProjectResourceRequest{
+			ProjectId: projectID,
+			Body:      &frontierv1beta1.ResourceRequestBody{Name: name, Namespace: computeDiskNamespace, Metadata: metadata},
+		}))
+		s.Require().NoError(err)
+		return resp.Msg.GetResource().GetId()
+	}
+	blockingDiskID := createDisk("disk-in-use", nil)
+	createDisk("disk-deletable", &structpb.Struct{Fields: map[string]*structpb.Value{
+		"deletable": structpb.NewBoolValue(true),
+	}})
+
+	orgDiskResp, err := s.testBench.Client.CreateOrganizationResource(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateOrganizationResourceRequest{
+		OrgId: orgID,
+		Body:  &frontierv1beta1.ResourceRequestBody{Name: "org-disk-in-use", Namespace: computeDiskNamespace},
+	}))
+	s.Require().NoError(err)
+	orgDiskID := orgDiskResp.Msg.GetResource().GetId()
+
+	deleteOrg := func() error {
+		_, err := s.testBench.Client.DeleteOrganization(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.DeleteOrganizationRequest{Id: orgID}))
+		return err
+	}
+	checkOrg := func() *frontierv1beta1.CheckOrganizationDeleteResponse {
+		resp, err := s.testBench.Client.CheckOrganizationDelete(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CheckOrganizationDeleteRequest{Id: orgID}))
+		s.Require().NoError(err)
+		return resp.Msg
+	}
+
+	s.Run("the check counts the project's and the org's own resources not marked deletable", func() {
+		resp := checkOrg()
+		s.Assert().False(resp.GetCanDelete())
+		s.Require().Len(resp.GetBlockers(), 1)
+		s.Assert().Equal(deleter.BlockerResourcesExist, resp.GetBlockers()[0].GetType())
+		s.Assert().Equal(computeDiskNamespace, resp.GetBlockers()[0].GetSubject())
+		s.Assert().Equal(deleter.SubjectResourceNamespace, resp.GetBlockers()[0].GetSubjectType())
+		s.Assert().Equal(int32(2), resp.GetBlockers()[0].GetCount())
+	})
+
+	s.Run("the org delete is refused with the blocker and nothing is removed", func() {
+		err := deleteOrg()
+		var connectErr *connect.Error
+		s.Require().ErrorAs(err, &connectErr)
+		s.Assert().Equal(connect.CodeFailedPrecondition, connectErr.Code())
+		s.Assert().Equal("organization cannot be deleted yet", connectErr.Message())
+
+		s.Require().Len(connectErr.Details(), 1)
+		detail, err := connectErr.Details()[0].Value()
+		s.Require().NoError(err)
+		failure, ok := detail.(*errdetails.PreconditionFailure)
+		s.Require().True(ok)
+		s.Require().Len(failure.GetViolations(), 1)
+		s.Assert().Equal(deleter.BlockerResourcesExist, failure.GetViolations()[0].GetType())
+		s.Assert().Equal(computeDiskNamespace, failure.GetViolations()[0].GetSubject())
+
+		_, err = s.testBench.Client.GetOrganization(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.GetOrganizationRequest{Id: orgID}))
+		s.Assert().NoError(err)
+		_, err = s.testBench.Client.GetProject(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.GetProjectRequest{Id: projectID}))
+		s.Assert().NoError(err)
+		_, err = s.testBench.Client.GetProjectResource(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.GetProjectResourceRequest{
+			ProjectId: projectID,
+			Id:        blockingDiskID,
+		}))
+		s.Assert().NoError(err)
+
+		var liveOrgDisks int
+		s.Require().NoError(s.dbc.GetContext(context.Background(), &liveOrgDisks,
+			"SELECT count(*) FROM resources WHERE id = $1 AND deleted_at IS NULL", orgDiskID))
+		s.Assert().Equal(1, liveOrgDisks)
+	})
+
+	s.Run("the org's own resource still blocks once the project's blocking resource is gone", func() {
+		_, err := s.testBench.Client.DeleteProjectResource(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.DeleteProjectResourceRequest{
+			ProjectId: projectID,
+			Id:        blockingDiskID,
+		}))
+		s.Require().NoError(err)
+
+		resp := checkOrg()
+		s.Assert().False(resp.GetCanDelete())
+		s.Require().Len(resp.GetBlockers(), 1)
+		s.Assert().Equal(computeDiskNamespace, resp.GetBlockers()[0].GetSubject())
+		s.Assert().Equal(int32(1), resp.GetBlockers()[0].GetCount())
+		s.Assert().Equal(connect.CodeFailedPrecondition, connect.CodeOf(deleteOrg()))
+	})
+
+	s.Run("the org can be deleted once no blocking resource is left, while the project rule still refuses", func() {
+		_, err := s.testBench.Client.DeleteOrganizationResource(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.DeleteOrganizationResourceRequest{
+			Id:    orgDiskID,
+			OrgId: orgID,
+		}))
+		s.Require().NoError(err)
+
+		_, err = s.testBench.Client.DeleteProject(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.DeleteProjectRequest{Id: projectID}))
+		s.Assert().Equal(connect.CodeFailedPrecondition, connect.CodeOf(err))
+
+		s.Assert().True(checkOrg().GetCanDelete())
+		s.Assert().NoError(deleteOrg())
+	})
+}
+
 func TestEndToEndAPIRegressionTestSuite(t *testing.T) {
 	suite.Run(t, new(APIRegressionTestSuite))
 }