From b7ad5cdb124f3251f9b4740d6f6be861d49700b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Susa=20T=C3=BCnker?= Date: Thu, 21 Aug 2025 16:32:26 +0200 Subject: [PATCH] fix: deletion blocked --- .../checkly/alertchannel_controller.go | 24 ++++-- .../controller/checkly/apicheck_controller.go | 24 ++++-- .../controller/checkly/group_controller.go | 24 ++++-- internal/controller/checkly/utils.go | 31 +++++++ internal/controller/checkly/utils_test.go | 85 +++++++++++++++++++ 5 files changed, 167 insertions(+), 21 deletions(-) create mode 100644 internal/controller/checkly/utils.go create mode 100644 internal/controller/checkly/utils_test.go diff --git a/internal/controller/checkly/alertchannel_controller.go b/internal/controller/checkly/alertchannel_controller.go index 2521159..2017704 100644 --- a/internal/controller/checkly/alertchannel_controller.go +++ b/internal/controller/checkly/alertchannel_controller.go @@ -84,15 +84,25 @@ func (r *AlertChannelReconciler) Reconcile(ctx context.Context, req ctrl.Request if ac.GetDeletionTimestamp() != nil { if controllerutil.ContainsFinalizer(ac, acFinalizer) { - logger.V(1).Info("Finalizer is present, trying to delete Checkly AlertChannel", "ID", ac.Status.ID) - err := external.DeleteAlertChannel(ac, r.ApiClient) - if err != nil { - logger.Error(err, "Failed to delete checkly AlertChannel") - return ctrl.Result{}, err + logger.V(1).Info("Finalizer is present, processing deletion", "ID", ac.Status.ID) + + // Only attempt deletion if the resource was actually created in Checkly + if ac.Status.ID != 0 { + err := external.DeleteAlertChannel(ac, r.ApiClient) + if err != nil { + if isNotFoundError(err) { + logger.Info("Checkly AlertChannel already deleted or doesn't exist", "ID", ac.Status.ID) + } else { + logger.Error(err, "Failed to delete checkly AlertChannel") + return ctrl.Result{}, err + } + } else { + logger.V(1).Info("Successfully deleted checkly AlertChannel", "ID", ac.Status.ID) + } + } else { + logger.Info("Skipping Checkly deletion - resource was never created", "name", ac.Name) } - logger.V(1).Info("Successfully deleted checkly AlertChannel", "ID", ac.Status.ID) - controllerutil.RemoveFinalizer(ac, acFinalizer) err = r.Update(ctx, ac) if err != nil { diff --git a/internal/controller/checkly/apicheck_controller.go b/internal/controller/checkly/apicheck_controller.go index 47099fe..124ad20 100644 --- a/internal/controller/checkly/apicheck_controller.go +++ b/internal/controller/checkly/apicheck_controller.go @@ -80,15 +80,25 @@ func (r *ApiCheckReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c if apiCheck.GetDeletionTimestamp() != nil { if controllerutil.ContainsFinalizer(apiCheck, apiCheckFinalizer) { - logger.V(1).Info("Finalizer is present, trying to delete Checkly check", "checkly ID", apiCheck.Status.ID) - err := external.Delete(apiCheck.Status.ID, r.ApiClient) - if err != nil { - logger.Error(err, "Failed to delete checkly API check") - return ctrl.Result{}, err + logger.V(1).Info("Finalizer is present, processing deletion", "checkly ID", apiCheck.Status.ID) + + // Only attempt deletion if the resource was actually created in Checkly + if apiCheck.Status.ID != "" { + err := external.Delete(apiCheck.Status.ID, r.ApiClient) + if err != nil { + if isNotFoundError(err) { + logger.Info("Checkly resource already deleted or doesn't exist", "checkly ID", apiCheck.Status.ID) + } else { + logger.Error(err, "Failed to delete checkly API check") + return ctrl.Result{}, err + } + } else { + logger.Info("Successfully deleted checkly API check", "checkly ID", apiCheck.Status.ID) + } + } else { + logger.Info("Skipping Checkly deletion - resource was never created", "name", apiCheck.Name) } - logger.Info("Successfully deleted checkly API check", "checkly ID", apiCheck.Status.ID) - controllerutil.RemoveFinalizer(apiCheck, apiCheckFinalizer) err = r.Update(ctx, apiCheck) if err != nil { diff --git a/internal/controller/checkly/group_controller.go b/internal/controller/checkly/group_controller.go index 5b8385a..ae04d91 100644 --- a/internal/controller/checkly/group_controller.go +++ b/internal/controller/checkly/group_controller.go @@ -78,15 +78,25 @@ func (r *GroupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl // If DeletionTimestamp is present, the object is marked for deletion, we need to remove the finalizer if group.GetDeletionTimestamp() != nil { if controllerutil.ContainsFinalizer(group, groupFinalizer) { - logger.V(1).Info("Finalizer is present, trying to delete Checkly group", "checkly group ID", group.Status.ID) - err := external.GroupDelete(group.Status.ID, r.ApiClient) - if err != nil { - logger.Error(err, "Failed to delete checkly group") - return ctrl.Result{}, err + logger.V(1).Info("Finalizer is present, processing deletion", "checkly group ID", group.Status.ID) + + // Only attempt deletion if the resource was actually created in Checkly + if group.Status.ID != 0 { + err := external.GroupDelete(group.Status.ID, r.ApiClient) + if err != nil { + if isNotFoundError(err) { + logger.Info("Checkly group already deleted or doesn't exist", "checkly group ID", group.Status.ID) + } else { + logger.Error(err, "Failed to delete checkly group") + return ctrl.Result{}, err + } + } else { + logger.Info("Successfully deleted checkly group", "checkly group ID", group.Status.ID) + } + } else { + logger.Info("Skipping Checkly deletion - resource was never created", "name", group.Name) } - logger.Info("Successfully deleted checkly group", "checkly group ID", group.Status.ID) - controllerutil.RemoveFinalizer(group, groupFinalizer) err = r.Update(ctx, group) if err != nil { diff --git a/internal/controller/checkly/utils.go b/internal/controller/checkly/utils.go new file mode 100644 index 0000000..73f5524 --- /dev/null +++ b/internal/controller/checkly/utils.go @@ -0,0 +1,31 @@ +/* +Copyright 2022. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package checkly + +import "strings" + +// isNotFoundError checks if an error represents a resource not found (404) error +func isNotFoundError(err error) bool { + if err == nil { + return false + } + + errMsg := strings.ToLower(err.Error()) + return strings.Contains(errMsg, "404") || + strings.Contains(errMsg, "not found") || + strings.Contains(errMsg, "does not exist") +} diff --git a/internal/controller/checkly/utils_test.go b/internal/controller/checkly/utils_test.go new file mode 100644 index 0000000..3d38cd7 --- /dev/null +++ b/internal/controller/checkly/utils_test.go @@ -0,0 +1,85 @@ +/* +Copyright 2022. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package checkly + +import ( + "errors" + "testing" +) + +func TestIsNotFoundError(t *testing.T) { + tests := []struct { + name string + err error + expected bool + }{ + { + name: "nil error", + err: nil, + expected: false, + }, + { + name: "404 error", + err: errors.New("404 Not Found"), + expected: true, + }, + { + name: "not found error", + err: errors.New("resource not found"), + expected: true, + }, + { + name: "does not exist error", + err: errors.New("check does not exist"), + expected: true, + }, + { + name: "mixed case 404", + err: errors.New("Error: 404 - Resource Not Found"), + expected: true, + }, + { + name: "mixed case not found", + err: errors.New("Item NOT FOUND in database"), + expected: true, + }, + { + name: "other error", + err: errors.New("internal server error"), + expected: false, + }, + { + name: "500 error", + err: errors.New("500 Internal Server Error"), + expected: false, + }, + { + name: "authorization error", + err: errors.New("401 Unauthorized"), + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isNotFoundError(tt.err) + if result != tt.expected { + t.Errorf("isNotFoundError(%v) = %v, expected %v", tt.err, result, tt.expected) + } + }) + } +}