From 3298b7be5a50242da392a2e3f5f9d98ef4d97f74 Mon Sep 17 00:00:00 2001 From: Pascal Bourdier Date: Wed, 3 Sep 2025 12:35:08 +0200 Subject: [PATCH] fix: avoid nil exception return early in error case to simplify code --- pkg/monitors/gcloud/gcloud-monitor.go | 11 ++-- pkg/monitors/pingdom/pingdom-monitor.go | 12 ++-- pkg/monitors/updown/updown-monitor.go | 84 +++++++++++++------------ 3 files changed, 56 insertions(+), 51 deletions(-) diff --git a/pkg/monitors/gcloud/gcloud-monitor.go b/pkg/monitors/gcloud/gcloud-monitor.go index 3cc7c421..6ec1291d 100644 --- a/pkg/monitors/gcloud/gcloud-monitor.go +++ b/pkg/monitors/gcloud/gcloud-monitor.go @@ -9,10 +9,10 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" monitoring "cloud.google.com/go/monitoring/apiv3/v2" - monitoringpb "cloud.google.com/go/monitoring/apiv3/v2/monitoringpb" + "cloud.google.com/go/monitoring/apiv3/v2/monitoringpb" "google.golang.org/api/iterator" "google.golang.org/api/option" - monitoredres "google.golang.org/genproto/googleapis/api/monitoredres" + "google.golang.org/genproto/googleapis/api/monitoredres" endpointmonitorv1alpha1 "github.com/stakater/IngressMonitorController/v2/api/v1alpha1" "github.com/stakater/IngressMonitorController/v2/pkg/config" @@ -98,11 +98,12 @@ func (service *MonitorService) Add(monitor models.Monitor) { portString := url.Port() var port int if portString == "" { - if url.Scheme == "http" { + switch url.Scheme { + case "http": port = 80 - } else if url.Scheme == "https" { + case "https": port = 443 - } else { + default: log.Info("Error Adding Monitor: unknown protocol " + url.Scheme) return } diff --git a/pkg/monitors/pingdom/pingdom-monitor.go b/pkg/monitors/pingdom/pingdom-monitor.go index d5f75aa8..984b0471 100644 --- a/pkg/monitors/pingdom/pingdom-monitor.go +++ b/pkg/monitors/pingdom/pingdom-monitor.go @@ -126,14 +126,16 @@ func (service *PingdomMonitorService) createHttpCheck(monitor models.Monitor) pi log.Info(fmt.Sprintf("Error parsing url '%s' of monitor %s", service.url, monitor.Name)) } - if url.Scheme == "https" { - httpCheck.Encryption = true - } else { + if url != nil { httpCheck.Encryption = false + if url.Scheme == "https" { + httpCheck.Encryption = true + } + + httpCheck.Hostname = url.Host + httpCheck.Url = url.Path } - httpCheck.Hostname = url.Host - httpCheck.Url = url.Path httpCheck.Name = monitor.Name // Set the default values if they are present in provider config // all of them can be overridden via EndpointMonitor specific options diff --git a/pkg/monitors/updown/updown-monitor.go b/pkg/monitors/updown/updown-monitor.go index 01eddff9..22a53a2c 100644 --- a/pkg/monitors/updown/updown-monitor.go +++ b/pkg/monitors/updown/updown-monitor.go @@ -29,7 +29,7 @@ type UpdownMonitorService struct { client *updown.Client } -func (monitor *UpdownMonitorService) Equal(oldMonitor models.Monitor, newMonitor models.Monitor) bool { +func (updownService *UpdownMonitorService) Equal(oldMonitor models.Monitor, newMonitor models.Monitor) bool { // TODO: Retrieve oldMonitor config and compare it here return false } @@ -59,26 +59,25 @@ func (updownService *UpdownMonitorService) GetAll() []models.Monitor { updownChecks, httpResponse, err := updownService.client.Check.List() log.Info("Monitors (updown checks) object list has been pulled") - if (httpResponse.StatusCode == http.StatusOK) && (err == nil) { - log.Info("Populating monitors list using the updownChecks object given in updownChecks list") - - // populating a monitors slice using the updownChecks objects given in updownChecks slice - for _, updownCheck := range updownChecks { - newMonitor := models.Monitor{ - URL: updownCheck.URL, - Name: updownCheck.Alias, - ID: updownCheck.Token, - } - monitors = append(monitors, newMonitor) - } - return monitors - - } else { + if err != nil || httpResponse.StatusCode != http.StatusOK { log.Info("Unable to get updown provider checks(monitor) list") + return nil + } + + log.Info("Populating monitors list using the updownChecks object given in updownChecks list") + // populating a monitors slice using the updownChecks objects given in updownChecks slice + for _, updownCheck := range updownChecks { + newMonitor := models.Monitor{ + URL: updownCheck.URL, + Name: updownCheck.Alias, + ID: updownCheck.Token, + } + monitors = append(monitors, newMonitor) } + return monitors } // GetByName function will return a monitor(updown check) object based on the name provided @@ -101,26 +100,25 @@ func (updownService *UpdownMonitorService) GetByName(monitorName string) (*model } // Add function method will add a monitor (updown check) -func (service *UpdownMonitorService) Add(updownMonitor models.Monitor) { +func (updownService *UpdownMonitorService) Add(updownMonitor models.Monitor) { log.Info("Updown monitor's Add method has been called") - updownCheckItemObj := service.createHttpCheck(updownMonitor) + updownCheckItemObj := updownService.createHttpCheck(updownMonitor) - _, httpResponse, err := service.client.Check.Add(updownCheckItemObj) + _, httpResponse, err := updownService.client.Check.Add(updownCheckItemObj) log.Info("Monitor addition request has been completed") - if (httpResponse.StatusCode == http.StatusCreated) && (err == nil) { - log.Info(fmt.Sprintf("Monitor %s has been added.", updownMonitor.Name)) - - } else if (httpResponse.StatusCode == http.StatusBadRequest) && (err != nil) { - log.Info(fmt.Sprintf("Monitor %s is not created because of invalid parameters or it exists.", updownMonitor.Name)) - - } else { + if err != nil { + if httpResponse != nil && httpResponse.StatusCode == http.StatusBadRequest { + log.Info(fmt.Sprintf("Monitor %s is not created because of invalid parameters or it exists.", updownMonitor.Name)) + return + } log.Info(fmt.Sprintf("Unable to create monitor %s ", updownMonitor.Name)) - + return } + log.Info(fmt.Sprintf("Monitor %s has been added.", updownMonitor.Name)) } // createHttpCheck method it will populate updown CheckItem object using updownMonitor's attributes @@ -152,7 +150,7 @@ func (updownService *UpdownMonitorService) createHttpCheck(updownMonitor models. } // addConfigToHttpCheck method will populate Updown's CheckItem object attributes using provider config -func (service *UpdownMonitorService) addConfigToHttpCheck(updownCheckItemObj *updown.CheckItem, config interface{}) { +func (updownService *UpdownMonitorService) addConfigToHttpCheck(updownCheckItemObj *updown.CheckItem, config interface{}) { // Read provider config, try to map them to updown check configs // set some default values if we can't find them @@ -182,22 +180,25 @@ func (service *UpdownMonitorService) addConfigToHttpCheck(updownCheckItemObj *up } // Update method will update a monitor (updown check) -func (service *UpdownMonitorService) Update(updownMonitor models.Monitor) { +func (updownService *UpdownMonitorService) Update(updownMonitor models.Monitor) { log.Info("Updown's Update method has been called") - httpCheckItemObj := service.createHttpCheck(updownMonitor) - _, httpResponse, err := service.client.Check.Update(updownMonitor.ID, httpCheckItemObj) + httpCheckItemObj := updownService.createHttpCheck(updownMonitor) + _, httpResponse, err := updownService.client.Check.Update(updownMonitor.ID, httpCheckItemObj) log.Info("Updown's check Update request has been completed") - if (httpResponse.StatusCode == http.StatusOK) && (err == nil) { - log.Info(fmt.Sprintf("Monitor %s has been updated with following parameters", updownMonitor.Name)) - - } else { + if err != nil { log.Info(fmt.Sprintf("Monitor %s is not updated because of %s", updownMonitor.Name, err.Error())) + return + } + if httpResponse.StatusCode != http.StatusOK { + log.Info(fmt.Sprintf("Unable to update monitor %s, status code is %s", updownMonitor.Name, httpResponse.Status)) + return } + log.Info(fmt.Sprintf("Monitor %s has been updated with following parameters", updownMonitor.Name)) } // Remove method will remove a monitor (updown check) @@ -208,14 +209,15 @@ func (updownService *UpdownMonitorService) Remove(updownMonitor models.Monitor) _, httpResponse, err := updownService.client.Check.Remove(updownMonitor.ID) log.Info("Updown's check Remove request has been completed") - if (httpResponse.StatusCode == http.StatusOK) && (err == nil) { - log.Info(fmt.Sprintf("Monitor %v has been deleted.", updownMonitor.Name)) + if err != nil { + log.Info(fmt.Sprintf("Unable to delete %v monitor: %s", updownMonitor.Name, err.Error())) + return + } - } else if (httpResponse.StatusCode == http.StatusNotFound) && (err != nil) { + if httpResponse.StatusCode == http.StatusNotFound { log.Info(fmt.Sprintf("Monitor %v is not found.", updownMonitor.Name)) - - } else { - log.Info("Unable to delete %v monitor: " + updownMonitor.Name) + return } + log.Info(fmt.Sprintf("Monitor %v has been deleted.", updownMonitor.Name)) }