Skip to content

Commit

Permalink
Actually wait for the CFOrg to be deleted in job endpoint
Browse files Browse the repository at this point in the history
Before this change, the job endpoint would say that an org was deleted
as soon as the CFOrg finalizer removed their role binding from the Org
namespace. We now fetch the CFOrg directly to determine if it's done
deleting. If it isn't deleting, then we will use GetOrg to determine if
the user has access to the org

[#2605]
  • Loading branch information
matt-royal committed Jul 17, 2023
1 parent 82680ee commit 008dc5e
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 5 deletions.
20 changes: 17 additions & 3 deletions api/repositories/org_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,11 +243,25 @@ func (r *OrgRepo) PatchOrgMetadata(ctx context.Context, authInfo authorization.I
}

func (r *OrgRepo) GetDeletedAt(ctx context.Context, authInfo authorization.Info, orgGUID string) (*time.Time, error) {
org, err := r.GetOrg(ctx, authInfo, orgGUID)
userClient, err := r.userClientFactory.BuildClient(authInfo)
if err != nil {
return nil, err
return nil, fmt.Errorf("get-deleted-at failed to build user client: %w", err)
}
return org.DeletedAt, nil

cfOrg := new(korifiv1alpha1.CFOrg)
err = userClient.Get(ctx, client.ObjectKey{Namespace: r.rootNamespace, Name: orgGUID}, cfOrg)
if err != nil {
return nil, apierrors.FromK8sError(err, OrgResourceType)
}

record := cfOrgToOrgRecord(*cfOrg)

if record.DeletedAt != nil {
return record.DeletedAt, nil
}

_, err = r.GetOrg(ctx, authInfo, orgGUID)
return nil, err
}

func cfOrgToOrgRecord(cfOrg korifiv1alpha1.CFOrg) OrgRecord {
Expand Down
28 changes: 26 additions & 2 deletions api/repositories/org_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,9 @@ var _ = Describe("OrgRepository", func() {
})

When("the org is being deleted", func() {
// This case occurs briefly between the CFOrg starting to delete and the finalizer deleting
// the roles in the org namespace. Once the finalizer deletes the roles, we'll be in the
// "the user does not have a role binding in the org" case below
BeforeEach(func() {
Expect(k8s.PatchResource(ctx, k8sClient, cfOrg, func() {
cfOrg.Finalizers = append(cfOrg.Finalizers, "foo")
Expand All @@ -396,8 +399,29 @@ var _ = Describe("OrgRepository", func() {
})

When("the user does not have a role binding in the org", func() {
It("errors", func() {
Expect(getErr).To(matchers.WrapErrorAssignableToTypeOf(apierrors.NotFoundError{}))
When("the org is not being deleted", func() {
It("errors", func() {
Expect(getErr).To(matchers.WrapErrorAssignableToTypeOf(apierrors.NotFoundError{}))
})
})

When("the org is being deleted", func() {
// This case occurs in 2 situations:
// 1. The user never has access to the Org, but another user deleted it
// 2. The user had access to the Org, deleted it, but the CFOrg finalizer has already
// deleted their role bindings
BeforeEach(func() {
Expect(k8s.PatchResource(ctx, k8sClient, cfOrg, func() {
cfOrg.Finalizers = append(cfOrg.Finalizers, "foo")
})).To(Succeed())

Expect(k8sClient.Delete(ctx, cfOrg)).To(Succeed())
})

It("returns the deletion time", func() {
Expect(getErr).NotTo(HaveOccurred())
Expect(deletionTime).To(PointTo(BeTemporally("~", time.Now(), time.Minute)))
})
})
})

Expand Down

0 comments on commit 008dc5e

Please sign in to comment.