From 19f24add0ae1e8e2b795ad0e679773d1c8be4b78 Mon Sep 17 00:00:00 2001 From: Vegard Engen Date: Sun, 29 Jun 2025 00:11:49 +0200 Subject: [PATCH 1/5] Check for Status field before checking for managed resources --- .../controller/firewallgroup_controller.go | 8 +- .../controller/firewallpolicy_controller.go | 102 +++++++++--------- 2 files changed, 56 insertions(+), 54 deletions(-) diff --git a/internal/controller/firewallgroup_controller.go b/internal/controller/firewallgroup_controller.go index f74117f..a312d32 100644 --- a/internal/controller/firewallgroup_controller.go +++ b/internal/controller/firewallgroup_controller.go @@ -377,7 +377,7 @@ func (r *FirewallGroupReconciler) Reconcile(ctx context.Context, req reconcile.R if err != nil { msg := strings.ToLower(err.Error()) log.Info(msg) - if strings.Contains(msg, "api.err.objectreferredby") || strings.Contains(msg,"invalid character") { + if strings.Contains(msg, "api.err.objectreferredby") || strings.Contains(msg, "invalid character") { log.Info("Firewall group is in use. Invoking workaround...!") firewall_group.GroupMembers = []string{"127.0.0.1"} firewall_group.Name = firewall_group.Name + "-deleted" @@ -417,7 +417,7 @@ func (r *FirewallGroupReconciler) Reconcile(ctx context.Context, req reconcile.R if err != nil { msg := strings.ToLower(err.Error()) log.Info(msg) - if strings.Contains(msg, "api.err.objectreferredby") || strings.Contains(msg,"invalid character") { + if strings.Contains(msg, "api.err.objectreferredby") || strings.Contains(msg, "invalid character") { log.Info("Firewall group is in use. Invoking workaround...!") firewall_group.GroupMembers = []string{"::1"} firewall_group.Name = firewall_group.Name + "-deleted" @@ -457,7 +457,7 @@ func (r *FirewallGroupReconciler) Reconcile(ctx context.Context, req reconcile.R if err != nil { msg := strings.ToLower(err.Error()) log.Info(msg) - if strings.Contains(msg, "api.err.objectreferredby") || strings.Contains(msg,"invalid character") { + if strings.Contains(msg, "api.err.objectreferredby") || strings.Contains(msg, "invalid character") { log.Info("Firewall group is in use. Invoking workaround...!") firewall_group.GroupMembers = []string{"0"} firewall_group.Name = firewall_group.Name + "-deleted" @@ -497,7 +497,7 @@ func (r *FirewallGroupReconciler) Reconcile(ctx context.Context, req reconcile.R if err != nil { msg := strings.ToLower(err.Error()) log.Info(msg) - if strings.Contains(msg, "api.err.objectreferredby") || strings.Contains(msg,"invalid character") { + if strings.Contains(msg, "api.err.objectreferredby") || strings.Contains(msg, "invalid character") { log.Info("Firewall group is in use. Invoking workaround...!") firewall_group.GroupMembers = []string{"127.0.0.1"} firewall_group.Name = firewall_group.Name + "-deleted" diff --git a/internal/controller/firewallpolicy_controller.go b/internal/controller/firewallpolicy_controller.go index b3565af..c608cd1 100644 --- a/internal/controller/firewallpolicy_controller.go +++ b/internal/controller/firewallpolicy_controller.go @@ -125,70 +125,72 @@ func (r *FirewallPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque } log.Info("Running finalizer logic for FirewallPolicy", "name", firewallPolicy.Name) - if len(firewallPolicy.Status.ResourcesManaged.UnifiFirewallPolicies) > 0 { - for i, UnifiFirewallPolicy := range firewallPolicy.Status.ResourcesManaged.UnifiFirewallPolicies { - log.Info(fmt.Sprintf("From: %s to: %s TcpIpv4: %s UdpIpv4: %s TcpIpv6: %s UdpIpv6: %s", UnifiFirewallPolicy.From, UnifiFirewallPolicy.To, UnifiFirewallPolicy.TcpIpv4ID, UnifiFirewallPolicy.UdpIpv4ID, UnifiFirewallPolicy.TcpIpv6ID, UnifiFirewallPolicy.UdpIpv6ID)) - if len(UnifiFirewallPolicy.TcpIpv4ID) > 0 { - err := r.UnifiClient.Client.DeleteFirewallPolicy(context.Background(), r.UnifiClient.SiteID, UnifiFirewallPolicy.TcpIpv4ID) - if err != nil && !strings.Contains(err.Error(), "not found") { - } else { - firewallPolicy.Status.ResourcesManaged.UnifiFirewallPolicies[i].TcpIpv4ID = "" - if err := r.Status().Update(ctx, &firewallPolicy); err != nil { - return ctrl.Result{RequeueAfter: 10 * time.Minute}, err + if len(firewallPolicy.Status) > 0 { + if len(firewallPolicy.Status.ResourcesManaged.UnifiFirewallPolicies) > 0 { + for i, UnifiFirewallPolicy := range firewallPolicy.Status.ResourcesManaged.UnifiFirewallPolicies { + log.Info(fmt.Sprintf("From: %s to: %s TcpIpv4: %s UdpIpv4: %s TcpIpv6: %s UdpIpv6: %s", UnifiFirewallPolicy.From, UnifiFirewallPolicy.To, UnifiFirewallPolicy.TcpIpv4ID, UnifiFirewallPolicy.UdpIpv4ID, UnifiFirewallPolicy.TcpIpv6ID, UnifiFirewallPolicy.UdpIpv6ID)) + if len(UnifiFirewallPolicy.TcpIpv4ID) > 0 { + err := r.UnifiClient.Client.DeleteFirewallPolicy(context.Background(), r.UnifiClient.SiteID, UnifiFirewallPolicy.TcpIpv4ID) + if err != nil && !strings.Contains(err.Error(), "not found") { + } else { + firewallPolicy.Status.ResourcesManaged.UnifiFirewallPolicies[i].TcpIpv4ID = "" + if err := r.Status().Update(ctx, &firewallPolicy); err != nil { + return ctrl.Result{RequeueAfter: 10 * time.Minute}, err + } } } - } - if len(UnifiFirewallPolicy.UdpIpv4ID) > 0 { - err := r.UnifiClient.Client.DeleteFirewallPolicy(context.Background(), r.UnifiClient.SiteID, UnifiFirewallPolicy.UdpIpv4ID) - if err != nil && !strings.Contains(err.Error(), "not found") { - return ctrl.Result{RequeueAfter: 10 * time.Minute}, err - } else { - firewallPolicy.Status.ResourcesManaged.UnifiFirewallPolicies[i].UdpIpv4ID = "" - if err := r.Status().Update(ctx, &firewallPolicy); err != nil { + if len(UnifiFirewallPolicy.UdpIpv4ID) > 0 { + err := r.UnifiClient.Client.DeleteFirewallPolicy(context.Background(), r.UnifiClient.SiteID, UnifiFirewallPolicy.UdpIpv4ID) + if err != nil && !strings.Contains(err.Error(), "not found") { return ctrl.Result{RequeueAfter: 10 * time.Minute}, err + } else { + firewallPolicy.Status.ResourcesManaged.UnifiFirewallPolicies[i].UdpIpv4ID = "" + if err := r.Status().Update(ctx, &firewallPolicy); err != nil { + return ctrl.Result{RequeueAfter: 10 * time.Minute}, err + } } } - } - if len(UnifiFirewallPolicy.TcpIpv6ID) > 0 { - err := r.UnifiClient.Client.DeleteFirewallPolicy(context.Background(), r.UnifiClient.SiteID, UnifiFirewallPolicy.TcpIpv6ID) - if err != nil && !strings.Contains(err.Error(), "not found") { - return ctrl.Result{RequeueAfter: 10 * time.Minute}, err - } else { - firewallPolicy.Status.ResourcesManaged.UnifiFirewallPolicies[i].TcpIpv6ID = "" - if err := r.Status().Update(ctx, &firewallPolicy); err != nil { + if len(UnifiFirewallPolicy.TcpIpv6ID) > 0 { + err := r.UnifiClient.Client.DeleteFirewallPolicy(context.Background(), r.UnifiClient.SiteID, UnifiFirewallPolicy.TcpIpv6ID) + if err != nil && !strings.Contains(err.Error(), "not found") { return ctrl.Result{RequeueAfter: 10 * time.Minute}, err + } else { + firewallPolicy.Status.ResourcesManaged.UnifiFirewallPolicies[i].TcpIpv6ID = "" + if err := r.Status().Update(ctx, &firewallPolicy); err != nil { + return ctrl.Result{RequeueAfter: 10 * time.Minute}, err + } } } - } - if len(UnifiFirewallPolicy.UdpIpv6ID) > 0 { - err := r.UnifiClient.Client.DeleteFirewallPolicy(context.Background(), r.UnifiClient.SiteID, UnifiFirewallPolicy.UdpIpv6ID) - if err != nil && !strings.Contains(err.Error(), "not found") { - return ctrl.Result{RequeueAfter: 10 * time.Minute}, err - } else { - firewallPolicy.Status.ResourcesManaged.UnifiFirewallPolicies[i].UdpIpv6ID = "" - if err := r.Status().Update(ctx, &firewallPolicy); err != nil { + if len(UnifiFirewallPolicy.UdpIpv6ID) > 0 { + err := r.UnifiClient.Client.DeleteFirewallPolicy(context.Background(), r.UnifiClient.SiteID, UnifiFirewallPolicy.UdpIpv6ID) + if err != nil && !strings.Contains(err.Error(), "not found") { return ctrl.Result{RequeueAfter: 10 * time.Minute}, err + } else { + firewallPolicy.Status.ResourcesManaged.UnifiFirewallPolicies[i].UdpIpv6ID = "" + if err := r.Status().Update(ctx, &firewallPolicy); err != nil { + return ctrl.Result{RequeueAfter: 10 * time.Minute}, err + } } } } } - } - if len(firewallPolicy.Status.ResourcesManaged.FirewallGroups) > 0 { - for i, firewallGroup := range firewallPolicy.Status.ResourcesManaged.FirewallGroups { - var firewallGroupCRD unifiv1beta1.FirewallGroup - if firewallGroup.Name != "" { - if err := r.Get(ctx, types.NamespacedName{Name: firewallGroup.Name, Namespace: firewallGroup.Namespace}, &firewallGroupCRD); err != nil { - return ctrl.Result{RequeueAfter: 10 * time.Minute}, err - } - if err := r.Delete(ctx, &firewallGroupCRD); err != nil { - log.Error(err, "Could not delete firewall group") - return ctrl.Result{RequeueAfter: 10 * time.Minute}, err - } - firewallPolicy.Status.ResourcesManaged.FirewallGroups[i].Name = "" - firewallPolicy.Status.ResourcesManaged.FirewallGroups[i].Namespace = "" - if err := r.Status().Update(ctx, &firewallPolicy); err != nil { - return ctrl.Result{RequeueAfter: 10 * time.Minute}, err + if len(firewallPolicy.Status.ResourcesManaged.FirewallGroups) > 0 { + for i, firewallGroup := range firewallPolicy.Status.ResourcesManaged.FirewallGroups { + var firewallGroupCRD unifiv1beta1.FirewallGroup + if firewallGroup.Name != "" { + if err := r.Get(ctx, types.NamespacedName{Name: firewallGroup.Name, Namespace: firewallGroup.Namespace}, &firewallGroupCRD); err != nil { + return ctrl.Result{RequeueAfter: 10 * time.Minute}, err + } + if err := r.Delete(ctx, &firewallGroupCRD); err != nil { + log.Error(err, "Could not delete firewall group") + return ctrl.Result{RequeueAfter: 10 * time.Minute}, err + } + firewallPolicy.Status.ResourcesManaged.FirewallGroups[i].Name = "" + firewallPolicy.Status.ResourcesManaged.FirewallGroups[i].Namespace = "" + if err := r.Status().Update(ctx, &firewallPolicy); err != nil { + return ctrl.Result{RequeueAfter: 10 * time.Minute}, err + } } } } From 44d89a5a5065faee6297c888c15d501bd0d3a473 Mon Sep 17 00:00:00 2001 From: Vegard Engen Date: Sun, 29 Jun 2025 00:17:44 +0200 Subject: [PATCH 2/5] Check for nil instead of length --- internal/controller/firewallpolicy_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/firewallpolicy_controller.go b/internal/controller/firewallpolicy_controller.go index c608cd1..eff07f8 100644 --- a/internal/controller/firewallpolicy_controller.go +++ b/internal/controller/firewallpolicy_controller.go @@ -125,7 +125,7 @@ func (r *FirewallPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque } log.Info("Running finalizer logic for FirewallPolicy", "name", firewallPolicy.Name) - if len(firewallPolicy.Status) > 0 { + if firewallPolicy.Status != nil { if len(firewallPolicy.Status.ResourcesManaged.UnifiFirewallPolicies) > 0 { for i, UnifiFirewallPolicy := range firewallPolicy.Status.ResourcesManaged.UnifiFirewallPolicies { log.Info(fmt.Sprintf("From: %s to: %s TcpIpv4: %s UdpIpv4: %s TcpIpv6: %s UdpIpv6: %s", UnifiFirewallPolicy.From, UnifiFirewallPolicy.To, UnifiFirewallPolicy.TcpIpv4ID, UnifiFirewallPolicy.UdpIpv4ID, UnifiFirewallPolicy.TcpIpv6ID, UnifiFirewallPolicy.UdpIpv6ID)) From 61606e8a7eb03a3bed3ec7e710a5def7bf4b8167 Mon Sep 17 00:00:00 2001 From: Vegard Engen Date: Sun, 29 Jun 2025 00:23:26 +0200 Subject: [PATCH 3/5] debug --- internal/controller/firewallpolicy_controller.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/controller/firewallpolicy_controller.go b/internal/controller/firewallpolicy_controller.go index eff07f8..bb5075d 100644 --- a/internal/controller/firewallpolicy_controller.go +++ b/internal/controller/firewallpolicy_controller.go @@ -125,6 +125,7 @@ func (r *FirewallPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque } log.Info("Running finalizer logic for FirewallPolicy", "name", firewallPolicy.Name) + log.Info(fmt.Sprintf("Deleting %+v", firewallPolicy)) if firewallPolicy.Status != nil { if len(firewallPolicy.Status.ResourcesManaged.UnifiFirewallPolicies) > 0 { for i, UnifiFirewallPolicy := range firewallPolicy.Status.ResourcesManaged.UnifiFirewallPolicies { From 8623d6cbc0320a65b964d4b38034528500bcabfd Mon Sep 17 00:00:00 2001 From: Vegard Engen Date: Sun, 29 Jun 2025 00:24:00 +0200 Subject: [PATCH 4/5] debug --- internal/controller/firewallpolicy_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/firewallpolicy_controller.go b/internal/controller/firewallpolicy_controller.go index bb5075d..6d7071f 100644 --- a/internal/controller/firewallpolicy_controller.go +++ b/internal/controller/firewallpolicy_controller.go @@ -126,7 +126,7 @@ func (r *FirewallPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque log.Info("Running finalizer logic for FirewallPolicy", "name", firewallPolicy.Name) log.Info(fmt.Sprintf("Deleting %+v", firewallPolicy)) - if firewallPolicy.Status != nil { + if true { if len(firewallPolicy.Status.ResourcesManaged.UnifiFirewallPolicies) > 0 { for i, UnifiFirewallPolicy := range firewallPolicy.Status.ResourcesManaged.UnifiFirewallPolicies { log.Info(fmt.Sprintf("From: %s to: %s TcpIpv4: %s UdpIpv4: %s TcpIpv6: %s UdpIpv6: %s", UnifiFirewallPolicy.From, UnifiFirewallPolicy.To, UnifiFirewallPolicy.TcpIpv4ID, UnifiFirewallPolicy.UdpIpv4ID, UnifiFirewallPolicy.TcpIpv6ID, UnifiFirewallPolicy.UdpIpv6ID)) From 37d806099565bec73302e97aeac057c462b928a3 Mon Sep 17 00:00:00 2001 From: Vegard Engen Date: Sun, 29 Jun 2025 00:44:49 +0200 Subject: [PATCH 5/5] Check for nil ResourcesManaged --- internal/controller/firewallpolicy_controller.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/controller/firewallpolicy_controller.go b/internal/controller/firewallpolicy_controller.go index 6d7071f..9730b59 100644 --- a/internal/controller/firewallpolicy_controller.go +++ b/internal/controller/firewallpolicy_controller.go @@ -125,8 +125,7 @@ func (r *FirewallPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque } log.Info("Running finalizer logic for FirewallPolicy", "name", firewallPolicy.Name) - log.Info(fmt.Sprintf("Deleting %+v", firewallPolicy)) - if true { + if firewallPolicy.Status.ResourcesManaged != nil { if len(firewallPolicy.Status.ResourcesManaged.UnifiFirewallPolicies) > 0 { for i, UnifiFirewallPolicy := range firewallPolicy.Status.ResourcesManaged.UnifiFirewallPolicies { log.Info(fmt.Sprintf("From: %s to: %s TcpIpv4: %s UdpIpv4: %s TcpIpv6: %s UdpIpv6: %s", UnifiFirewallPolicy.From, UnifiFirewallPolicy.To, UnifiFirewallPolicy.TcpIpv4ID, UnifiFirewallPolicy.UdpIpv4ID, UnifiFirewallPolicy.TcpIpv6ID, UnifiFirewallPolicy.UdpIpv6ID))