Report correct deletion status in route deletion job

Issue: cloudfoundry#2607
Co-authored-by: Kieron Browne <[email protected]>

api/handlers/job.go

@@ -2,6 +2,7 @@ package handlers
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/url"
@@ -70,7 +71,7 @@ func (h *Job) get(r *http.Request) (*routing.Response, error) {
 	switch job.Type {
 	case syncSpaceJobType:
 		jobResponse = presenter.ForManifestApplyJob(job, h.serverURL)
-	case RouteDeleteJobType, DomainDeleteJobType, RoleDeleteJobType:
+	case DomainDeleteJobType, RoleDeleteJobType:
 		jobResponse = presenter.ForJob(job, []presenter.JobResponseError{}, presenter.StateComplete, h.serverURL)
 	default:
 		repository, ok := h.repositories[job.Type]
@@ -96,21 +97,20 @@ func (h *Job) handleDeleteJob(ctx context.Context, repository DeletionRepository
 
 	deletedAt, err := h.retryGetDeletedAt(ctx, repository, job)
 	if err != nil {
-		switch err.(type) {
-		case apierrors.NotFoundError, apierrors.ForbiddenError:
+		if errors.As(err, &apierrors.NotFoundError{}) || errors.As(err, &apierrors.ForbiddenError{}) {
 			return presenter.ForJob(job,
 				[]presenter.JobResponseError{},
 				presenter.StateComplete,
 				h.serverURL,
 			), nil
-		default:
-			return presenter.JobResponse{}, apierrors.LogAndReturn(
-				log,
-				err,
-				"failed to fetch "+job.ResourceType+" from Kubernetes",
-				job.ResourceType+"GUID", job.ResourceGUID,
-			)
 		}
+
+		return presenter.JobResponse{}, apierrors.LogAndReturn(
+			log,
+			err,
+			"failed to fetch "+job.ResourceType+" from Kubernetes",
+			job.ResourceType+"GUID", job.ResourceGUID,
+		)
 	}
 
 	if deletedAt == nil {

api/handlers/job_test.go

@@ -1,6 +1,7 @@
 package handlers_test
 
 import (
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"time"
@@ -40,7 +41,6 @@ var _ = Describe("Job", func() {
 
 			Expect(rr).To(HaveHTTPBody(SatisfyAll(bodyMatchers...)))
 		},
-			Entry("route delete", "route.delete", "cf-route-guid"),
 			Entry("domain delete", "domain.delete", "cf-domain-guid"),
 			Entry("role delete", "role.delete", "cf-role-guid"),
 			Entry("apply manifest", "space.apply_manifest", "cf-space-guid"),
@@ -86,7 +86,7 @@ var _ = Describe("Job", func() {
 
 		When("the resource does not exist", func() {
 			BeforeEach(func() {
-				deletionRepo.GetDeletedAtReturns(nil, apierrors.NewNotFoundError(nil, "foo"))
+				deletionRepo.GetDeletedAtReturns(nil, fmt.Errorf("wrapped error: %w", apierrors.NewNotFoundError(nil, "foo")))
 			})
 
 			It("returns a complete status", func() {
@@ -120,7 +120,7 @@ var _ = Describe("Job", func() {
 
 		When("the user does not have permission to see the resource", func() {
 			BeforeEach(func() {
-				deletionRepo.GetDeletedAtReturns(nil, apierrors.NewForbiddenError(nil, "foo"))
+				deletionRepo.GetDeletedAtReturns(nil, fmt.Errorf("wrapped err: %w", apierrors.NewForbiddenError(nil, "foo")))
 			})
 
 			It("returns a complete status", func() {

api/main.go

@@ -331,6 +331,7 @@ func main() {
 				handlers.OrgDeleteJobType:   orgRepo,
 				handlers.SpaceDeleteJobType: spaceRepo,
 				handlers.AppDeleteJobType:   appRepo,
+				handlers.RouteDeleteJobType: routeRepo,
 			},
 			500*time.Millisecond,
 		),

api/repositories/route_repository.go

@@ -57,6 +57,7 @@ type RouteRecord struct {
 	Annotations  map[string]string
 	CreatedAt    time.Time
 	UpdatedAt    *time.Time
+	DeletedAt    *time.Time
 }
 
 type AddDestinationsToRouteMessage struct {
@@ -146,13 +147,13 @@ func (m CreateRouteMessage) toCFRoute() korifiv1alpha1.CFRoute {
 	}
 }
 
-func (f *RouteRepo) GetRoute(ctx context.Context, authInfo authorization.Info, routeGUID string) (RouteRecord, error) {
-	ns, err := f.namespaceRetriever.NamespaceFor(ctx, routeGUID, RouteResourceType)
+func (r *RouteRepo) GetRoute(ctx context.Context, authInfo authorization.Info, routeGUID string) (RouteRecord, error) {
+	ns, err := r.namespaceRetriever.NamespaceFor(ctx, routeGUID, RouteResourceType)
 	if err != nil {
 		return RouteRecord{}, fmt.Errorf("failed to get namespace for route: %w", err)
 	}
 
-	userClient, err := f.userClientFactory.BuildClient(authInfo)
+	userClient, err := r.userClientFactory.BuildClient(authInfo)
 	if err != nil {
 		return RouteRecord{}, fmt.Errorf("failed to build user client: %w", err)
 	}
@@ -166,13 +167,13 @@ func (f *RouteRepo) GetRoute(ctx context.Context, authInfo authorization.Info, r
 	return cfRouteToRouteRecord(route), nil
 }
 
-func (f *RouteRepo) ListRoutes(ctx context.Context, authInfo authorization.Info, message ListRoutesMessage) ([]RouteRecord, error) {
-	nsList, err := f.namespacePermissions.GetAuthorizedSpaceNamespaces(ctx, authInfo)
+func (r *RouteRepo) ListRoutes(ctx context.Context, authInfo authorization.Info, message ListRoutesMessage) ([]RouteRecord, error) {
+	nsList, err := r.namespacePermissions.GetAuthorizedSpaceNamespaces(ctx, authInfo)
 	if err != nil {
 		return nil, fmt.Errorf("failed to list namespaces for spaces with user role bindings: %w", err)
 	}
 
-	userClient, err := f.userClientFactory.BuildClient(authInfo)
+	userClient, err := r.userClientFactory.BuildClient(authInfo)
 	if err != nil {
 		return []RouteRecord{}, fmt.Errorf("failed to build user client: %w", err)
 	}
@@ -215,8 +216,8 @@ func (f *RouteRepo) ListRoutes(ctx context.Context, authInfo authorization.Info,
 	return returnRouteList(filteredRoutes), nil
 }
 
-func (f *RouteRepo) ListRoutesForApp(ctx context.Context, authInfo authorization.Info, appGUID string, spaceGUID string) ([]RouteRecord, error) {
-	userClient, err := f.userClientFactory.BuildClient(authInfo)
+func (r *RouteRepo) ListRoutesForApp(ctx context.Context, authInfo authorization.Info, appGUID string, spaceGUID string) ([]RouteRecord, error) {
+	userClient, err := r.userClientFactory.BuildClient(authInfo)
 	if err != nil {
 		return []RouteRecord{}, fmt.Errorf("failed to build user client: %w", err)
 	}
@@ -275,6 +276,7 @@ func cfRouteToRouteRecord(cfRoute korifiv1alpha1.CFRoute) RouteRecord {
 		Destinations: destinations,
 		CreatedAt:    cfRoute.CreationTimestamp.Time,
 		UpdatedAt:    getLastUpdatedTime(&cfRoute),
+		DeletedAt:    golangTime(cfRoute.DeletionTimestamp),
 		Labels:       cfRoute.Labels,
 		Annotations:  cfRoute.Annotations,
 	}
@@ -290,9 +292,9 @@ func cfRouteDestinationToDestination(cfRouteDestination korifiv1alpha1.Destinati
 	}
 }
 
-func (f *RouteRepo) CreateRoute(ctx context.Context, authInfo authorization.Info, message CreateRouteMessage) (RouteRecord, error) {
+func (r *RouteRepo) CreateRoute(ctx context.Context, authInfo authorization.Info, message CreateRouteMessage) (RouteRecord, error) {
 	cfRoute := message.toCFRoute()
-	userClient, err := f.userClientFactory.BuildClient(authInfo)
+	userClient, err := r.userClientFactory.BuildClient(authInfo)
 	if err != nil {
 		return RouteRecord{}, fmt.Errorf("failed to build user client: %w", err)
 	}
@@ -305,8 +307,8 @@ func (f *RouteRepo) CreateRoute(ctx context.Context, authInfo authorization.Info
 	return cfRouteToRouteRecord(cfRoute), nil
 }
 
-func (f *RouteRepo) DeleteRoute(ctx context.Context, authInfo authorization.Info, message DeleteRouteMessage) error {
-	userClient, err := f.userClientFactory.BuildClient(authInfo)
+func (r *RouteRepo) DeleteRoute(ctx context.Context, authInfo authorization.Info, message DeleteRouteMessage) error {
+	userClient, err := r.userClientFactory.BuildClient(authInfo)
 	if err != nil {
 		return fmt.Errorf("failed to build user client: %w", err)
 	}
@@ -320,8 +322,8 @@ func (f *RouteRepo) DeleteRoute(ctx context.Context, authInfo authorization.Info
 	return apierrors.FromK8sError(err, RouteResourceType)
 }
 
-func (f *RouteRepo) GetOrCreateRoute(ctx context.Context, authInfo authorization.Info, message CreateRouteMessage) (RouteRecord, error) {
-	existingRecord, exists, err := f.fetchRouteByFields(ctx, authInfo, message)
+func (r *RouteRepo) GetOrCreateRoute(ctx context.Context, authInfo authorization.Info, message CreateRouteMessage) (RouteRecord, error) {
+	existingRecord, exists, err := r.fetchRouteByFields(ctx, authInfo, message)
 	if err != nil {
 		return RouteRecord{}, fmt.Errorf("GetOrCreateRoute: %w", err)
 	}
@@ -330,11 +332,11 @@ func (f *RouteRepo) GetOrCreateRoute(ctx context.Context, authInfo authorization
 		return existingRecord, nil
 	}
 
-	return f.CreateRoute(ctx, authInfo, message)
+	return r.CreateRoute(ctx, authInfo, message)
 }
 
-func (f *RouteRepo) AddDestinationsToRoute(ctx context.Context, authInfo authorization.Info, message AddDestinationsToRouteMessage) (RouteRecord, error) {
-	userClient, err := f.userClientFactory.BuildClient(authInfo)
+func (r *RouteRepo) AddDestinationsToRoute(ctx context.Context, authInfo authorization.Info, message AddDestinationsToRouteMessage) (RouteRecord, error) {
+	userClient, err := r.userClientFactory.BuildClient(authInfo)
 	if err != nil {
 		return RouteRecord{}, fmt.Errorf("failed to build user client: %w", err)
 	}
@@ -355,8 +357,8 @@ func (f *RouteRepo) AddDestinationsToRoute(ctx context.Context, authInfo authori
 	return cfRouteToRouteRecord(*cfRoute), err
 }
 
-func (f *RouteRepo) RemoveDestinationFromRoute(ctx context.Context, authInfo authorization.Info, message RemoveDestinationFromRouteMessage) (RouteRecord, error) {
-	userClient, err := f.userClientFactory.BuildClient(authInfo)
+func (r *RouteRepo) RemoveDestinationFromRoute(ctx context.Context, authInfo authorization.Info, message RemoveDestinationFromRouteMessage) (RouteRecord, error) {
+	userClient, err := r.userClientFactory.BuildClient(authInfo)
 	if err != nil {
 		return RouteRecord{}, fmt.Errorf("failed to build user client: %w", err)
 	}
@@ -413,8 +415,8 @@ outer:
 	return result
 }
 
-func (f *RouteRepo) fetchRouteByFields(ctx context.Context, authInfo authorization.Info, message CreateRouteMessage) (RouteRecord, bool, error) {
-	matches, err := f.ListRoutes(ctx, authInfo, ListRoutesMessage{
+func (r *RouteRepo) fetchRouteByFields(ctx context.Context, authInfo authorization.Info, message CreateRouteMessage) (RouteRecord, bool, error) {
+	matches, err := r.ListRoutes(ctx, authInfo, ListRoutesMessage{
 		SpaceGUIDs:  []string{message.SpaceGUID},
 		DomainGUIDs: []string{message.DomainGUID},
 		Hosts:       []string{message.Host},
@@ -448,8 +450,8 @@ func destinationRecordsToCFDestinations(destinationRecords []DestinationRecord)
 	return destinations
 }
 
-func (f *RouteRepo) PatchRouteMetadata(ctx context.Context, authInfo authorization.Info, message PatchRouteMetadataMessage) (RouteRecord, error) {
-	userClient, err := f.userClientFactory.BuildClient(authInfo)
+func (r *RouteRepo) PatchRouteMetadata(ctx context.Context, authInfo authorization.Info, message PatchRouteMetadataMessage) (RouteRecord, error) {
+	userClient, err := r.userClientFactory.BuildClient(authInfo)
 	if err != nil {
 		return RouteRecord{}, fmt.Errorf("failed to build user client: %w", err)
 	}
@@ -469,3 +471,8 @@ func (f *RouteRepo) PatchRouteMetadata(ctx context.Context, authInfo authorizati
 
 	return cfRouteToRouteRecord(*route), nil
 }
+
+func (r *RouteRepo) GetDeletedAt(ctx context.Context, authInfo authorization.Info, routeGUID string) (*time.Time, error) {
+	route, err := r.GetRoute(ctx, authInfo, routeGUID)
+	return route.DeletedAt, err
+}

api/repositories/route_repository_test.go

@@ -184,9 +184,10 @@ var _ = Describe("RouteRepository", func() {
 					Expect(destinationRecord.Protocol).To(Equal(cfRoute1.Spec.Destinations[0].Protocol))
 				})
 
-				By("returning a record where the CreatedAt and UpdatedAt match the CR creation time", func() {
+				By("returning a record with timestamps", func() {
 					Expect(route.CreatedAt).To(BeTemporally("~", time.Now(), timeCheckThreshold))
 					Expect(route.UpdatedAt).To(PointTo(BeTemporally("~", time.Now(), timeCheckThreshold)))
+					Expect(route.DeletedAt).To(BeNil())
 				})
 
 				Expect(route.Domain).To(Equal(DomainRecord{GUID: domainGUID}))
@@ -1518,6 +1519,80 @@ var _ = Describe("RouteRepository", func() {
 			})
 		})
 	})
+
+	Describe("GetDeletedAt", func() {
+		var (
+			cfRoute   *korifiv1alpha1.CFRoute
+			deletedAt *time.Time
+			getErr    error
+		)
+
+		BeforeEach(func() {
+			createRoleBinding(testCtx, userName, spaceDeveloperRole.Name, space.Name)
+
+			cfRoute = &korifiv1alpha1.CFRoute{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      route1GUID,
+					Namespace: space.Name,
+				},
+				Spec: korifiv1alpha1.CFRouteSpec{
+					Host:     "my-subdomain-1",
+					Path:     "",
+					Protocol: "http",
+					DomainRef: corev1.ObjectReference{
+						Name:      domainGUID,
+						Namespace: rootNamespace,
+					},
+					Destinations: []korifiv1alpha1.Destination{
+						{
+							GUID: "destination-guid",
+							Port: 8080,
+							AppRef: corev1.LocalObjectReference{
+								Name: "some-app-guid",
+							},
+							ProcessType: "web",
+							Protocol:    "http1",
+						},
+					},
+				},
+			}
+			Expect(k8sClient.Create(testCtx, cfRoute)).To(Succeed())
+		})
+
+		JustBeforeEach(func() {
+			deletedAt, getErr = routeRepo.GetDeletedAt(testCtx, authInfo, route1GUID)
+		})
+
+		It("returns nil", func() {
+			Expect(getErr).NotTo(HaveOccurred())
+			Expect(deletedAt).To(BeNil())
+		})
+
+		When("the route is being deleted", func() {
+			BeforeEach(func() {
+				Expect(k8s.PatchResource(ctx, k8sClient, cfRoute, func() {
+					cfRoute.Finalizers = append(cfRoute.Finalizers, "foo")
+				})).To(Succeed())
+
+				Expect(k8sClient.Delete(ctx, cfRoute)).To(Succeed())
+			})
+
+			It("returns the deletion time", func() {
+				Expect(getErr).NotTo(HaveOccurred())
+				Expect(deletedAt).To(PointTo(BeTemporally("~", time.Now(), time.Minute)))
+			})
+		})
+
+		When("the route isn't found", func() {
+			BeforeEach(func() {
+				Expect(k8sClient.Delete(ctx, cfRoute)).To(Succeed())
+			})
+
+			It("errors", func() {
+				Expect(getErr).To(matchers.WrapErrorAssignableToTypeOf(apierrors.NotFoundError{}))
+			})
+		})
+	})
 })
 
 func createRoute(guid, namespace, host, path, domainGUID, appGUID string) *korifiv1alpha1.CFRoute {

tests/e2e/routes_test.go

@@ -315,15 +315,13 @@ var _ = Describe("Routes", func() {
 				g.Expect(string(jobResp.Body())).To(ContainSubstring("COMPLETE"))
 			}).Should(Succeed())
 
-			Eventually(func(g Gomega) {
-				getRouteResp, err := client.R().Get("/v3/routes/" + routeGUID)
-				g.Expect(err).NotTo(HaveOccurred())
-				g.Expect(getRouteResp).To(HaveRestyStatusCode(http.StatusNotFound))
-			}).Should(Succeed())
-		})
+			getRouteResp, err := client.R().Get("/v3/routes/" + routeGUID)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(getRouteResp).To(HaveRestyStatusCode(http.StatusNotFound))
 
-		It("frees up the deleted route's name for reuse", func() {
-			createRoute(host, path, spaceGUID, domainGUID)
+			By("freeing up the deleted route's name for reuse", func() {
+				createRoute(host, path, spaceGUID, domainGUID)
+			})
 		})
 	})