Consumer deletion

cmd/maestro/environments/service_types.go

@@ -49,6 +49,7 @@ func NewConsumerServiceLocator(env *Env) ConsumerServiceLocator {
 		return services.NewConsumerService(
 			db.NewAdvisoryLockFactory(env.Database.SessionFactory),
 			dao.NewConsumerDao(&env.Database.SessionFactory),
+			dao.NewResourceDao(&env.Database.SessionFactory),
 			env.Services.Events(),
 		)
 	}

cmd/maestro/server/routes.go

@@ -23,7 +23,7 @@ func (s *apiServer) routes() *mux.Router {
 	}
 
 	resourceHandler := handlers.NewResourceHandler(services.Resources(), services.Generic())
-	consumerHandler := handlers.NewConsumerHandler(services.Consumers(), services.Generic())
+	consumerHandler := handlers.NewConsumerHandler(services.Consumers(), services.Resources(), services.Generic())
 	errorsHandler := handlers.NewErrorsHandler()
 
 	var authMiddleware auth.JWTMiddleware

pkg/dao/consumer.go

@@ -13,7 +13,7 @@ type ConsumerDao interface {
 	Get(ctx context.Context, id string) (*api.Consumer, error)
 	Create(ctx context.Context, consumer *api.Consumer) (*api.Consumer, error)
 	Replace(ctx context.Context, consumer *api.Consumer) (*api.Consumer, error)
-	Delete(ctx context.Context, id string) error
+	Delete(ctx context.Context, id string, unscoped bool) error
 	FindByIDs(ctx context.Context, ids []string) (api.ConsumerList, error)
 	All(ctx context.Context) (api.ConsumerList, error)
 }
@@ -55,8 +55,12 @@ func (d *sqlConsumerDao) Replace(ctx context.Context, consumer *api.Consumer) (*
 	return consumer, nil
 }
 
-func (d *sqlConsumerDao) Delete(ctx context.Context, id string) error {
+func (d *sqlConsumerDao) Delete(ctx context.Context, id string, unscoped bool) error {
 	g2 := (*d.sessionFactory).New(ctx)
+	if unscoped {
+		// Unscoped is used to permanently delete the record
+		g2 = g2.Unscoped()
+	}
 	if err := g2.Omit(clause.Associations).Delete(&api.Consumer{Meta: api.Meta{ID: id}}).Error; err != nil {
 		db.MarkForRollback(ctx, err)
 		return err

pkg/dao/mocks/consumer.go

@@ -38,7 +38,7 @@ func (d *consumerDaoMock) Replace(ctx context.Context, consumer *api.Consumer) (
 	return nil, errors.NotImplemented("Consumer").AsError()
 }
 
-func (d *consumerDaoMock) Delete(ctx context.Context, id string) error {
+func (d *consumerDaoMock) Delete(ctx context.Context, id string, unscoped bool) error {
 	return errors.NotImplemented("Consumer").AsError()
 }
 

pkg/dao/mocks/resource.go

@@ -81,3 +81,7 @@ func (d *resourceDaoMock) FindBySource(ctx context.Context, source string) (api.
 func (d *resourceDaoMock) All(ctx context.Context) (api.ResourceList, error) {
 	return d.resources, nil
 }
+
+func (d *resourceDaoMock) FirstByConsumerName(ctx context.Context, consumerName string, unscoped bool) (api.Resource, error) {
+	return *d.resources[0], errors.NotImplemented("Resource").AsError()
+}

pkg/dao/resource.go

@@ -19,6 +19,7 @@ type ResourceDao interface {
 	FindBySource(ctx context.Context, source string) (api.ResourceList, error)
 	FindByConsumerName(ctx context.Context, consumerName string) (api.ResourceList, error)
 	All(ctx context.Context) (api.ResourceList, error)
+	FirstByConsumerName(ctx context.Context, name string, unscoped bool) (api.Resource, error)
 }
 
 var _ ResourceDao = &sqlResourceDao{}
@@ -115,3 +116,15 @@ func (d *sqlResourceDao) All(ctx context.Context) (api.ResourceList, error) {
 	}
 	return resources, nil
 }
+
+// FirstByConsumerName will take the first item of the resources. Currently leverage it to determine whether the result exists in the table.
+func (d *sqlResourceDao) FirstByConsumerName(ctx context.Context, consumerName string, unscoped bool) (api.Resource, error) {
+	g2 := (*d.sessionFactory).New(ctx)
+	if unscoped {
+		// Unscoped is used to find the deleting resources
+		g2 = g2.Unscoped()
+	}
+	resource := api.Resource{}
+	err := g2.Where("consumer_name = ?", consumerName).First(&resource).Error
+	return resource, err
+}

pkg/handlers/consumer.go

@@ -17,12 +17,14 @@ var _ RestHandler = consumerHandler{}
 
 type consumerHandler struct {
 	consumer services.ConsumerService
+	resource services.ResourceService
 	generic  services.GenericService
 }
 
-func NewConsumerHandler(consumer services.ConsumerService, generic services.GenericService) *consumerHandler {
+func NewConsumerHandler(consumer services.ConsumerService, resource services.ResourceService, generic services.GenericService) *consumerHandler {
 	return &consumerHandler{
 		consumer: consumer,
+		resource: resource,
 		generic:  generic,
 	}
 }
@@ -84,7 +86,7 @@ func (h consumerHandler) List(w http.ResponseWriter, r *http.Request) {
 			ctx := r.Context()
 
 			listArgs := services.NewListArguments(r.URL.Query())
-			var consumers = []api.Consumer{}
+			consumers := []api.Consumer{}
 			paging, err := h.generic.List(ctx, "username", listArgs, &consumers)
 			if err != nil {
 				return nil, err
@@ -135,7 +137,11 @@ func (h consumerHandler) Get(w http.ResponseWriter, r *http.Request) {
 func (h consumerHandler) Delete(w http.ResponseWriter, r *http.Request) {
 	cfg := &handlerConfig{
 		Action: func() (interface{}, *errors.ServiceError) {
-			return nil, errors.NotImplemented("delete")
+			id := mux.Vars(r)["id"]
+			if err := h.consumer.Delete(r.Context(), id); err != nil {
+				return nil, err
+			}
+			return nil, nil
 		},
 	}
 	handleDelete(w, r, cfg, http.StatusNoContent)

pkg/services/consumer.go

@@ -20,16 +20,18 @@ type ConsumerService interface {
 	FindByIDs(ctx context.Context, ids []string) (api.ConsumerList, *errors.ServiceError)
 }
 
-func NewConsumerService(lockFactory db.LockFactory, consumerDao dao.ConsumerDao, events EventService) ConsumerService {
+func NewConsumerService(lockFactory db.LockFactory, consumerDao dao.ConsumerDao, resourceDao dao.ResourceDao, events EventService) ConsumerService {
 	return &sqlConsumerService{
 		consumerDao: consumerDao,
+		resourceDao: resourceDao,
 	}
 }
 
 var _ ConsumerService = &sqlConsumerService{}
 
 type sqlConsumerService struct {
 	consumerDao dao.ConsumerDao
+	resourceDao dao.ResourceDao
 }
 
 func (s *sqlConsumerService) Get(ctx context.Context, id string) (*api.Consumer, *errors.ServiceError) {
@@ -62,9 +64,28 @@ func (s *sqlConsumerService) Replace(ctx context.Context, consumer *api.Consumer
 	return consumer, nil
 }
 
+// Delete will remove the consumer from the storage. Currently, it will:
+// 1. Perform a hard delete on the consumer, the resource creation will be blocked after it.
+// 2. Forbid consumer deletion if there are associated resources.
+// 3. The deleting resources(marked as deleted) will still block the consumer deletion.
+// TODO: Additional deletion options or strategies may be added in the future.
 func (s *sqlConsumerService) Delete(ctx context.Context, id string) *errors.ServiceError {
-	if err := s.consumerDao.Delete(ctx, id); err != nil {
-		return handleDeleteError("Consumer", errors.GeneralError("Unable to delete consumer: %s", err))
+	// TODO: The following code snippet is for soft deleting a consumer
+	// consumer, err := s.Get(ctx, id)
+	// if err != nil {
+	// 	return err
+	// }
+	// _, e := s.resourceDao.FirstByConsumerName(ctx, consumer.Name, true)
+	// if e == nil {
+	// 	return errors.Forbidden("Resources associated with the consumer: %s", consumer.Name)
+	// }
+	// if e != gorm.ErrRecordNotFound {
+	// 	return handleDeleteError("Consumer", e)
+	// }
+	// then resource is not found for the consumer
+
+	if err := s.consumerDao.Delete(ctx, id, true); err != nil {
+		return handleDeleteError("Consumer", err)
 	}
 	return nil
 }

pkg/services/util.go

@@ -48,6 +48,9 @@ func handleUpdateError(resourceType string, err error) *errors.ServiceError {
 }
 
 func handleDeleteError(resourceType string, err error) *errors.ServiceError {
+	if strings.Contains(err.Error(), "violates foreign key constraint") {
+		return errors.Forbidden("Unable to delete %s: %s", resourceType, err)
+	}
 	return errors.GeneralError("Unable to delete %s: %s", resourceType, err.Error())
 }
 

test/e2e/pkg/consumer_test.go

@@ -11,25 +11,42 @@ import (
 
 // go test -v ./test/e2e/pkg -args -api-server=$api_server -consumer-name=$consumer_name -consumer-kubeconfig=$consumer_kubeconfig -ginkgo.focus "Consumer"
 var _ = Describe("Consumer", Ordered, func() {
-	var testConsumerName string
-	var testConsumerID string
+	var consumer openapi.Consumer
+	var resourceConsumer openapi.Consumer
+	var resource openapi.Resource
 	BeforeAll(func() {
-		testConsumerName = "test-consumer"
+		consumer = openapi.Consumer{Name: openapi.PtrString("linda")}
+		resourceConsumer = openapi.Consumer{Name: openapi.PtrString("susan")}
+		resource = helper.NewAPIResource(*resourceConsumer.Name, 1)
 	})
 
 	Context("Consumer CRUD Tests", func() {
 		It("create consumer", func() {
-			consumer := openapi.Consumer{Name: &testConsumerName}
+			// create a consumer without resource
 			created, resp, err := apiClient.DefaultApi.ApiMaestroV1ConsumersPost(ctx).Consumer(consumer).Execute()
 			Expect(err).To(Succeed())
 			Expect(resp.StatusCode).To(Equal(http.StatusCreated))
 			Expect(*created.Id).NotTo(BeEmpty())
-			testConsumerID = *created.Id
+			consumer = *created
 
-			got, resp, err := apiClient.DefaultApi.ApiMaestroV1ConsumersIdGet(ctx, testConsumerID).Execute()
+			got, resp, err := apiClient.DefaultApi.ApiMaestroV1ConsumersIdGet(ctx, *consumer.Id).Execute()
 			Expect(err).To(Succeed())
 			Expect(resp.StatusCode).To(Equal(http.StatusOK))
 			Expect(got).NotTo(BeNil())
+
+			// create a consumer associates with resource
+			created, resp, err = apiClient.DefaultApi.ApiMaestroV1ConsumersPost(ctx).Consumer(resourceConsumer).Execute()
+			Expect(err).To(Succeed())
+			Expect(resp.StatusCode).To(Equal(http.StatusCreated))
+			Expect(*created.Id).NotTo(BeEmpty())
+			resourceConsumer = *created
+
+			res, resp, err := apiClient.DefaultApi.ApiMaestroV1ResourcesPost(ctx).Resource(resource).Execute()
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(resp.StatusCode).To(Equal(http.StatusCreated))
+			Expect(*res.Id).ShouldNot(BeEmpty())
+			Expect(*res.Version).To(Equal(int32(1)))
+			resource = *res
 		})
 
 		It("list consumer", func() {
@@ -41,7 +58,7 @@ var _ = Describe("Consumer", Ordered, func() {
 
 			got := false
 			for _, c := range consumerList.Items {
-				if *c.Name == testConsumerName {
+				if *c.Name == *consumer.Name {
 					got = true
 				}
 			}
@@ -50,14 +67,14 @@ var _ = Describe("Consumer", Ordered, func() {
 
 		It("patch consumer", func() {
 			labels := &map[string]string{"hello": "world"}
-			patched, resp, err := apiClient.DefaultApi.ApiMaestroV1ConsumersIdPatch(ctx, testConsumerID).
+			patched, resp, err := apiClient.DefaultApi.ApiMaestroV1ConsumersIdPatch(ctx, *consumer.Id).
 				ConsumerPatchRequest(openapi.ConsumerPatchRequest{Labels: labels}).Execute()
 			Expect(err).To(Succeed())
 			Expect(resp.StatusCode).To(Equal(http.StatusOK))
 			_, ok := patched.GetLabelsOk()
 			Expect(ok).To(BeTrue())
 
-			got, resp, err := apiClient.DefaultApi.ApiMaestroV1ConsumersIdGet(ctx, testConsumerID).Execute()
+			got, resp, err := apiClient.DefaultApi.ApiMaestroV1ConsumersIdGet(ctx, *consumer.Id).Execute()
 			Expect(err).To(Succeed())
 			Expect(resp.StatusCode).To(Equal(http.StatusOK))
 			Expect(got).NotTo(BeNil())
@@ -66,7 +83,30 @@ var _ = Describe("Consumer", Ordered, func() {
 		})
 
 		AfterAll(func() {
-			// TODO: add the consumer deletion
+			// delete the consumer
+			resp, err := apiClient.DefaultApi.ApiMaestroV1ConsumersIdDelete(ctx, *consumer.Id).Execute()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(resp.StatusCode).To(Equal(http.StatusNoContent))
+
+			_, resp, err = apiClient.DefaultApi.ApiMaestroV1ConsumersIdGet(ctx, *consumer.Id).Execute()
+			Expect(err.Error()).To(ContainSubstring("Not Found"))
+			Expect(resp.StatusCode).To(Equal(http.StatusNotFound))
+
+			// delete the consumer associated with resource
+			resp, err = apiClient.DefaultApi.ApiMaestroV1ConsumersIdDelete(ctx, *resourceConsumer.Id).Execute()
+			Expect(err).To(HaveOccurred())
+			Expect(resp.StatusCode).To(Equal(http.StatusForbidden)) // 403 forbid deletion
+
+			// delete the resource
+			resp, err = apiClient.DefaultApi.ApiMaestroV1ResourcesIdDelete(ctx, *resource.Id).Execute()
+			Expect(err).To(Succeed())
+			Expect(resp.StatusCode).To(Equal(http.StatusNoContent))
+
+			// only permanently delete the resources, the consumer can be deleted
+			// deleting resource still block the consumer deletion
+			resp, err = apiClient.DefaultApi.ApiMaestroV1ConsumersIdDelete(ctx, *resourceConsumer.Id).Execute()
+			Expect(err).To(HaveOccurred())
+			Expect(resp.StatusCode).To(Equal(http.StatusForbidden)) // 403 forbid deletion
 		})
 	})
 })