move implicit cascade deletes of plugins to postActions

tests/integration/test_utils.go

@@ -10,7 +10,6 @@ import (
 	"github.com/fatih/color"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
-
 	"github.com/kong/deck/cmd"
 	deckDump "github.com/kong/deck/dump"
 	"github.com/kong/deck/utils"

types/postProcess.go

@@ -2,6 +2,7 @@ package types
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/kong/deck/crud"
 	"github.com/kong/deck/state"
@@ -16,7 +17,20 @@ func (crud *servicePostAction) Create(_ context.Context, args ...crud.Arg) (crud
 }
 
 func (crud *servicePostAction) Delete(_ context.Context, args ...crud.Arg) (crud.Arg, error) {
-	return nil, crud.currentState.Services.Delete(*((args[0].(*state.Service)).ID))
+	serviceID := *args[0].(*state.Service).ID
+
+	// Delete all plugins associated with this service as that's the implicit behavior of Kong (cascade delete).
+	plugins, err := crud.currentState.Plugins.GetAllByServiceID(serviceID)
+	if err != nil {
+		return nil, fmt.Errorf("error looking up plugins for service '%v': %v", serviceID, err)
+	}
+	for _, plugin := range plugins {
+		err = crud.currentState.Plugins.Delete(*plugin.ID)
+		if err != nil {
+			return nil, fmt.Errorf("error deleting plugin '%v' for service '%v': %v", *plugin.ID, serviceID, err)
+		}
+	}
+	return nil, crud.currentState.Services.Delete(serviceID)
 }
 
 func (crud *servicePostAction) Update(_ context.Context, args ...crud.Arg) (crud.Arg, error) {
@@ -32,7 +46,20 @@ func (crud *routePostAction) Create(_ context.Context, args ...crud.Arg) (crud.A
 }
 
 func (crud *routePostAction) Delete(_ context.Context, args ...crud.Arg) (crud.Arg, error) {
-	return nil, crud.currentState.Routes.Delete(*((args[0].(*state.Route)).ID))
+	routeID := *args[0].(*state.Route).ID
+
+	// Delete all plugins associated with this route as that's the implicit behavior of Kong (cascade delete).
+	plugins, err := crud.currentState.Plugins.GetAllByRouteID(routeID)
+	if err != nil {
+		return nil, fmt.Errorf("error looking up plugins for route '%v': %v", routeID, err)
+	}
+	for _, plugin := range plugins {
+		err = crud.currentState.Plugins.Delete(*plugin.ID)
+		if err != nil {
+			return nil, fmt.Errorf("error deleting plugin '%v' for route '%v': %v", *plugin.ID, routeID, err)
+		}
+	}
+	return nil, crud.currentState.Routes.Delete(routeID)
 }
 
 func (crud *routePostAction) Update(_ context.Context, args ...crud.Arg) (crud.Arg, error) {
@@ -146,7 +173,19 @@ func (crud *consumerPostAction) Create(_ context.Context, args ...crud.Arg) (cru
 }
 
 func (crud *consumerPostAction) Delete(_ context.Context, args ...crud.Arg) (crud.Arg, error) {
-	return nil, crud.currentState.Consumers.Delete(*((args[0].(*state.Consumer)).ID))
+	consumerID := *args[0].(*state.Consumer).ID
+
+	// Delete all plugins associated with this consumer as that's the implicit behavior of Kong (cascade delete).
+	plugins, err := crud.currentState.Plugins.GetAllByConsumerID(consumerID)
+	if err != nil {
+		return nil, fmt.Errorf("error looking up plugins for consumer '%v': %v", consumerID, err)
+	}
+	for _, plugin := range plugins {
+		if err := crud.currentState.Plugins.Delete(*plugin.ID); err != nil {
+			return nil, fmt.Errorf("error deleting plugin '%v' for consumer '%v': %v", *plugin.ID, consumerID, err)
+		}
+	}
+	return nil, crud.currentState.Consumers.Delete(consumerID)
 }
 
 func (crud *consumerPostAction) Update(_ context.Context, args ...crud.Arg) (crud.Arg, error) {

types/service.go

@@ -186,7 +186,6 @@ func (d *serviceDiffer) deleteDuplicateService(targetService *state.Service) ([]
 	}
 
 	if *currentService.ID != *targetService.ID {
-		// Found a duplicate, delete it along with all routes and plugins associated with it.
 		var events []crud.Event
 
 		// We have to delete all routes beforehand as otherwise we will get a foreign key error when deleting the service
@@ -199,36 +198,6 @@ func (d *serviceDiffer) deleteDuplicateService(targetService *state.Service) ([]
 		}
 
 		for _, route := range routesToDelete {
-			// We have to delete all plugins associated with the route to make sure they'll be recreated eventually.
-			// Plugins are deleted by the cascading delete of the route and without us generating a delete event manually,
-			// they could not be later recreated in createUpdates stage of the diff.
-			// By generating a delete event for each plugin, we make sure that the implicit deletion of plugins is handled
-			// in the local state and createUpdate stage can recreate them.
-			pluginsToDelete, err := d.currentState.Plugins.GetAllByRouteID(*route.ID)
-			if err != nil {
-				return nil, fmt.Errorf("error looking up plugins for route %q: %w",
-					*route.Name, err)
-			}
-
-			for _, plugin := range pluginsToDelete {
-				if err := d.currentState.Plugins.Delete(*plugin.ID); err != nil {
-					return nil, fmt.Errorf("error deleting plugin %q for route %q: %w",
-						*route.Name, *plugin.Name, err)
-				}
-				// REVIEW: Should we generate a delete event for the plugin? It will always result in a DELETE
-				// call for this plugin while it could be just a local state change as we already know it's going
-				// to be deleted by the cascading delete of the route.
-				//
-				// It's also problematic when syncing with Konnect as Koko returns 404 when trying to delete a plugin
-				// that doesn't exist. It's not an issue when syncing with Kong as Kong returns 204.
-				//
-				//events = append(events, crud.Event{
-				//	Op:   crud.Delete,
-				//	Kind: "plugin",
-				//	Obj:  plugin,
-				//})
-			}
-
 			events = append(events, crud.Event{
 				Op:   crud.Delete,
 				Kind: "route",