From c1594cb2dbe91268e35a3712363a1168cd8612bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Wed, 24 May 2023 19:31:05 +0200 Subject: [PATCH] move implicit cascade deletes of plugins to postActions --- tests/integration/test_utils.go | 1 - types/postProcess.go | 45 ++++++++++++++++++++++++++++++--- types/service.go | 31 ----------------------- 3 files changed, 42 insertions(+), 35 deletions(-) diff --git a/tests/integration/test_utils.go b/tests/integration/test_utils.go index b21dd5c50..228db1e28 100644 --- a/tests/integration/test_utils.go +++ b/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" diff --git a/types/postProcess.go b/types/postProcess.go index 2ec5e9f6e..453205973 100644 --- a/types/postProcess.go +++ b/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) { diff --git a/types/service.go b/types/service.go index 6c063b844..71f27f241 100644 --- a/types/service.go +++ b/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",