From ef34e0f1dd8e4d9fdd4870008f24fc04ece0b15d Mon Sep 17 00:00:00 2001 From: Andrew Costa Date: Mon, 26 Jun 2023 17:19:04 -0700 Subject: [PATCH] Implement org deletion job [#2605] - Add check for org deletion timeout with message - Calculate status for deletion based on presence of CRD and its timestamp Co-authored-by: Dave Walter --- api/handlers/job.go | 38 +++++++- api/handlers/job_test.go | 106 ++++++++++++++++++++++- api/main.go | 1 + api/presenter/job.go | 34 +++++--- api/presenter/job_test.go | 16 +++- api/repositories/org_repository.go | 1 + api/repositories/task_repository_test.go | 2 +- 7 files changed, 176 insertions(+), 22 deletions(-) diff --git a/api/handlers/job.go b/api/handlers/job.go index 4eb22ca5e..f30067826 100644 --- a/api/handlers/job.go +++ b/api/handlers/job.go @@ -5,6 +5,9 @@ import ( "net/http" "net/url" "regexp" + "time" + + "code.cloudfoundry.org/korifi/api/authorization" apierrors "code.cloudfoundry.org/korifi/api/errors" "code.cloudfoundry.org/korifi/api/presenter" @@ -22,21 +25,26 @@ const ( spaceDeletePrefix = "space.delete" domainDeletePrefix = "domain.delete" roleDeletePrefix = "role.delete" + + JobTimeoutDuration = 120.0 ) const JobResourceType = "Job" type Job struct { serverURL url.URL + orgRepo CFOrgRepository } -func NewJob(serverURL url.URL) *Job { +func NewJob(serverURL url.URL, orgRepo CFOrgRepository) *Job { return &Job{ serverURL: serverURL, + orgRepo: orgRepo, } } func (h *Job) get(r *http.Request) (*routing.Response, error) { + authInfo, _ := authorization.InfoFromContext(r.Context()) logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.job.get") jobGUID := routing.URLParam(r, "guid") @@ -56,8 +64,32 @@ func (h *Job) get(r *http.Request) (*routing.Response, error) { switch jobType { case syncSpacePrefix: jobResponse = presenter.ForManifestApplyJob(jobGUID, resourceGUID, h.serverURL) - case appDeletePrefix, orgDeletePrefix, spaceDeletePrefix, routeDeletePrefix, domainDeletePrefix, roleDeletePrefix: - jobResponse = presenter.ForJob(jobGUID, jobType, h.serverURL) + case appDeletePrefix, spaceDeletePrefix, routeDeletePrefix, domainDeletePrefix, roleDeletePrefix: + jobResponse = presenter.ForJob(jobGUID, []presenter.JobResponseError{}, presenter.StateComplete, jobType, h.serverURL) + case orgDeletePrefix: + org, err := h.orgRepo.GetOrg(r.Context(), authInfo, resourceGUID) + if err != nil { + if _, ok := err.(apierrors.NotFoundError); !ok { + return nil, apierrors.LogAndReturn(logger, apierrors.ForbiddenAsNotFound(err), "Failed to fetch org from Kubernetes", "OrgGUID", resourceGUID) + } + jobResponse = presenter.ForJob(jobGUID, []presenter.JobResponseError{}, presenter.StateComplete, jobType, h.serverURL) + break + } + + deletionTime, err := time.Parse(time.RFC3339Nano, org.DeletedAt) + if err != nil { + return nil, apierrors.LogAndReturn(logger, err, "failed to parse org deletion time", "name", org.Name, "timestamp", org.DeletedAt) + } + + if time.Since(deletionTime).Seconds() < JobTimeoutDuration { + jobResponse = presenter.ForJob(jobGUID, []presenter.JobResponseError{}, presenter.StateProcessing, jobType, h.serverURL) + } else { + jobResponse = presenter.ForJob(jobGUID, []presenter.JobResponseError{{ + Detail: fmt.Sprintf("CFOrg deletion timed out. Check for lingering resources in the %q namespace", org.GUID), + Title: "CF-UnprocessableEntity", + Code: 10008, + }}, presenter.StateFailed, jobType, h.serverURL) + } default: return nil, apierrors.LogAndReturn( logger, diff --git a/api/handlers/job_test.go b/api/handlers/job_test.go index 9ce219e60..88f7864dc 100644 --- a/api/handlers/job_test.go +++ b/api/handlers/job_test.go @@ -1,8 +1,16 @@ package handlers_test import ( + "fmt" "net/http" "net/http/httptest" + "time" + + apierrors "code.cloudfoundry.org/korifi/api/errors" + + "code.cloudfoundry.org/korifi/api/repositories" + + "code.cloudfoundry.org/korifi/api/handlers/fake" "code.cloudfoundry.org/korifi/api/handlers" . "code.cloudfoundry.org/korifi/tests/matchers" @@ -17,11 +25,14 @@ var _ = Describe("Job", func() { spaceGUID string jobGUID string req *http.Request + orgRepo *fake.OrgRepository ) BeforeEach(func() { spaceGUID = uuid.NewString() - apiHandler := handlers.NewJob(*serverURL) + + orgRepo = new(fake.OrgRepository) + apiHandler := handlers.NewJob(*serverURL, orgRepo) routerBuilder.LoadRoutes(apiHandler) }) @@ -63,9 +74,7 @@ var _ = Describe("Job", func() { MatchJSONPath("$.operation", operation), ))) }, - Entry("app delete", "app.delete", "cf-app-guid"), - Entry("org delete", "org.delete", "cf-org-guid"), Entry("space delete", "space.delete", "cf-space-guid"), Entry("route delete", "route.delete", "cf-route-guid"), Entry("domain delete", "domain.delete", "cf-domain-guid"), @@ -81,5 +90,96 @@ var _ = Describe("Job", func() { expectNotFoundError("Job") }) }) + + Describe("org delete", func() { + var ( + operation string + resourceGUID string + ) + + BeforeEach(func() { + operation = "org.delete" + resourceGUID = "cf-org-guid" + }) + + When("org delete is in progress", func() { + BeforeEach(func() { + orgRepo.GetOrgReturns(repositories.OrgRecord{ + GUID: "cf-org-guid", + DeletedAt: time.Now().Format(time.RFC3339Nano), + }, nil) + }) + + It("returns a processing status", func() { + guid := operation + "~" + resourceGUID + req, err := http.NewRequestWithContext(ctx, "GET", "/v3/jobs/"+guid, nil) + Expect(err).NotTo(HaveOccurred()) + + rr = httptest.NewRecorder() + routerBuilder.Build().ServeHTTP(rr, req) + + Expect(rr).To(HaveHTTPBody(SatisfyAll( + MatchJSONPath("$.guid", guid), + MatchJSONPath("$.links.self.href", defaultServerURL+"/v3/jobs/"+guid), + MatchJSONPath("$.operation", operation), + MatchJSONPath("$.state", "PROCESSING"), + MatchJSONPath("$.errors", BeEmpty()), + ))) + }) + }) + + When("org delete completed", func() { + BeforeEach(func() { + orgRepo.GetOrgReturns(repositories.OrgRecord{}, apierrors.NewNotFoundError(nil, repositories.OrgResourceType)) + }) + + It("returns a complete status", func() { + guid := operation + "~" + resourceGUID + req, err := http.NewRequestWithContext(ctx, "GET", "/v3/jobs/"+guid, nil) + Expect(err).NotTo(HaveOccurred()) + + rr = httptest.NewRecorder() + routerBuilder.Build().ServeHTTP(rr, req) + + Expect(rr).To(HaveHTTPBody(SatisfyAll( + MatchJSONPath("$.guid", guid), + MatchJSONPath("$.links.self.href", defaultServerURL+"/v3/jobs/"+guid), + MatchJSONPath("$.operation", operation), + MatchJSONPath("$.state", "COMPLETE"), + MatchJSONPath("$.errors", BeEmpty()), + ))) + }) + }) + + When("org delete times out", func() { + BeforeEach(func() { + orgRepo.GetOrgReturns(repositories.OrgRecord{ + GUID: "cf-org-guid", + DeletedAt: (time.Now().Add(-180 * time.Second)).Format(time.RFC3339Nano), + }, nil) + }) + + It("returns a failed status", func() { + guid := operation + "~" + resourceGUID + req, err := http.NewRequestWithContext(ctx, "GET", "/v3/jobs/"+guid, nil) + Expect(err).NotTo(HaveOccurred()) + + rr = httptest.NewRecorder() + routerBuilder.Build().ServeHTTP(rr, req) + + Expect(rr).To(HaveHTTPBody(SatisfyAll( + MatchJSONPath("$.guid", guid), + MatchJSONPath("$.links.self.href", defaultServerURL+"/v3/jobs/"+guid), + MatchJSONPath("$.operation", operation), + MatchJSONPath("$.state", "FAILED"), + MatchJSONPath("$.errors", ConsistOf(map[string]interface{}{ + "code": float64(10008), + "detail": fmt.Sprintf("CFOrg deletion timed out. Check for lingering resources in the %q namespace", resourceGUID), + "title": "CF-UnprocessableEntity", + })), + ))) + }) + }) + }) }) }) diff --git a/api/main.go b/api/main.go index 97e4fa766..722a8ed0c 100644 --- a/api/main.go +++ b/api/main.go @@ -329,6 +329,7 @@ func main() { ), handlers.NewJob( *serverURL, + orgRepo, ), handlers.NewLogCache( appRepo, diff --git a/api/presenter/job.go b/api/presenter/job.go index 952c5628c..1c7759d23 100644 --- a/api/presenter/job.go +++ b/api/presenter/job.go @@ -8,6 +8,10 @@ import ( const ( JobGUIDDelimiter = "~" + StateComplete = "COMPLETE" + StateFailed = "FAILED" + StateProcessing = "PROCESSING" + AppDeleteOperation = "app.delete" OrgDeleteOperation = "org.delete" RouteDeleteOperation = "route.delete" @@ -17,15 +21,21 @@ const ( RoleDeleteOperation = "role.delete" ) +type JobResponseError struct { + Detail string `json:"detail"` + Title string `json:"title"` + Code int `json:"code"` +} + type JobResponse struct { - GUID string `json:"guid"` - Errors *string `json:"errors"` - Warnings *string `json:"warnings"` - Operation string `json:"operation"` - State string `json:"state"` - CreatedAt string `json:"created_at"` - UpdatedAt string `json:"updated_at"` - Links JobLinks `json:"links"` + GUID string `json:"guid"` + Errors []JobResponseError `json:"errors"` + Warnings *string `json:"warnings"` + Operation string `json:"operation"` + State string `json:"state"` + CreatedAt string `json:"created_at"` + UpdatedAt string `json:"updated_at"` + Links JobLinks `json:"links"` } type JobLinks struct { @@ -34,20 +44,20 @@ type JobLinks struct { } func ForManifestApplyJob(jobGUID string, spaceGUID string, baseURL url.URL) JobResponse { - response := ForJob(jobGUID, SpaceApplyManifestOperation, baseURL) + response := ForJob(jobGUID, []JobResponseError{}, StateComplete, SpaceApplyManifestOperation, baseURL) response.Links.Space = &Link{ HRef: buildURL(baseURL).appendPath("/v3/spaces", spaceGUID).build(), } return response } -func ForJob(jobGUID string, operation string, baseURL url.URL) JobResponse { +func ForJob(jobGUID string, errors []JobResponseError, state string, operation string, baseURL url.URL) JobResponse { return JobResponse{ GUID: jobGUID, - Errors: nil, + Errors: errors, Warnings: nil, Operation: operation, - State: "COMPLETE", + State: state, CreatedAt: "", UpdatedAt: "", Links: JobLinks{ diff --git a/api/presenter/job_test.go b/api/presenter/job_test.go index 070b5ffeb..910210d17 100644 --- a/api/presenter/job_test.go +++ b/api/presenter/job_test.go @@ -32,7 +32,7 @@ var _ = Describe("", func() { It("renders the job", func() { Expect(output).To(MatchJSON(`{ "created_at": "", - "errors": null, + "errors": [], "guid": "the-job-guid", "links": { "self": { @@ -52,7 +52,11 @@ var _ = Describe("", func() { Describe("ForDeleteJob", func() { JustBeforeEach(func() { - response := presenter.ForJob("the-job-guid", "the.operation", *baseURL) + response := presenter.ForJob("the-job-guid", []presenter.JobResponseError{{ + Detail: "error detail", + Title: "CF-JobErrorTitle", + Code: 12345, + }}, "COMPLETE", "the.operation", *baseURL) var err error output, err = json.Marshal(response) Expect(err).NotTo(HaveOccurred()) @@ -61,7 +65,13 @@ var _ = Describe("", func() { It("renders the job", func() { Expect(output).To(MatchJSON(`{ "created_at": "", - "errors": null, + "errors": [ + { + "code": 12345, + "detail": "error detail", + "title": "CF-JobErrorTitle" + } + ], "guid": "the-job-guid", "links": { "self": { diff --git a/api/repositories/org_repository.go b/api/repositories/org_repository.go index 6a9958637..6423f70bc 100644 --- a/api/repositories/org_repository.go +++ b/api/repositories/org_repository.go @@ -51,6 +51,7 @@ type OrgRecord struct { Annotations map[string]string CreatedAt string UpdatedAt string + DeletedAt string } type OrgRepo struct { diff --git a/api/repositories/task_repository_test.go b/api/repositories/task_repository_test.go index f4203fa26..d4d5cf250 100644 --- a/api/repositories/task_repository_test.go +++ b/api/repositories/task_repository_test.go @@ -6,13 +6,13 @@ import ( "sync" "time" - "code.cloudfoundry.org/korifi/tools/k8s" "code.cloudfoundry.org/korifi/api/authorization" apierrors "code.cloudfoundry.org/korifi/api/errors" "code.cloudfoundry.org/korifi/api/repositories" "code.cloudfoundry.org/korifi/api/repositories/conditions" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" "code.cloudfoundry.org/korifi/tests/matchers" + "code.cloudfoundry.org/korifi/tools/k8s" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega"