feat(Pingdom): add requestHeadersEnvVar field (#599)

* feat(Pingdom): add requestHeadersEnvVar field

* debug gh action

* fix gh action

* add line

* update documentation

api/v1alpha1/endpointmonitor_types.go

@@ -244,10 +244,15 @@ type PingdomConfig struct {
 	// +optional
 	NotifyWhenBackUp bool `json:"notifyWhenBackUp,omitempty"`
 
-	// Custom pingdom request headers
+	// Custom request headers
 	// +optional
 	RequestHeaders string `json:"requestHeaders,omitempty"`
 
+	// Custom request headers that should be read from an environment variable as it possibly contains sensitive data.
+	// An example would be an API token.
+	// +optional
+	RequestHeadersEnvVar string `json:"requestHeadersEnvVar,omitempty"`
+
 	// Required for basic-authentication
 	// +optional
 	BasicAuthUser string `json:"basicAuthUser,omitempty"`
@@ -287,7 +292,7 @@ type PingdomConfig struct {
 
 	// Data that should be posted to the web page, for example submission data for a sign-up or login form.
 	// The data needs to be formatted in the same way as a web browser would send it to the web server.
-	// Because post data contains sensitive secret this field is only reference to a environment variable.
+	// Because post data contains sensitive secret this field is only a reference to an environment variable.
 	// +optional
 	PostDataEnvVar string `json:"postDataEnvVar,omitempty"`
 }

config/crd/bases/endpointmonitor.stakater.com_endpointmonitors.yaml

@@ -107,10 +107,15 @@ spec:
                       submission data for a sign-up or login form. The data needs
                       to be formatted in the same way as a web browser would send
                       it to the web server. Because post data contains sensitive secret
-                      this field is only reference to a environment variable.
+                      this field is only a reference to an environment variable.
                     type: string
                   requestHeaders:
-                    description: Custom pingdom request headers
+                    description: Custom request headers
+                    type: string
+                  requestHeadersEnvVar:
+                    description: Custom request headers that should be read from an
+                      environment variable as it possibly contains sensitive data.
+                      An example would be an API token.
                     type: string
                   resolution:
                     description: The pingdom check interval in minutes

docs/pingdom-configuration.md

@@ -1,9 +1,11 @@
 # Pingdom Configuration
 
 ## Note
+
 Currently we do not have access to Pingdom account that's why Tests are not verified. Community members having Pingdom account are welcome to contribute in Test Cases.
 
 ## Basic
+
 The following properties need to be configured for Pingdom, in addition to the general properties listed
 in the [Configuration section of the README](../README.md#configuration):
 
@@ -12,31 +14,35 @@ in the [Configuration section of the README](../README.md#configuration):
 | apiToken | Pingdom API Token generated inside My Pingdom |
 
 ## Optional
+
 The following optional properties can be included if you want to declare some default options, without re-declaring them for each EndpointMonitor.
 You are able to override any of them via EndpointMonitor specific options.
 
 | Key               | Description                                              |
 |-------------------|----------------------------------------------------------|
-| alertIntegrations | `-` separated list of integration ids                  |
-| teamAlertContacts | `-` separated list of teams ids                  |
-| alertContacts     | `-` separated list of alert contacts ids                  |
+| alertIntegrations | `-` separated list of integration ids                    |
+| teamAlertContacts | `-` separated list of teams ids                          |
+| alertContacts     | `-` separated list of alert contacts ids                 |
+
 ## Advanced
 
 Currently additional pingdom configurations can be added through these fields:
 
-|                        Fields                        |                    Description                   |
-|:--------------------------------------------------------:|:------------------------------------------------:|
-| Resolution                  | The pingdom check interval in minutes            |
-| SendNotificationWhenDown | How many failed check attempts before notifying  |
-| Paused                      | Set to "true" to pause checks                    |
-| NotifyWhenBackUp         | Set to "false" to disable recovery notifications |
-| RequestHeaders             | Custom pingdom request headers (e.g. {"Accept"="application/json"}) |
+| Fields                    |                    Description                   |
+|---------------------------|--------------------------------------------------|
+| Resolution                | The pingdom check interval in minutes            |
+| SendNotificationWhenDown  | How many failed check attempts before notifying  |
+| Paused                    | Set to "true" to pause checks                    |
+| NotifyWhenBackUp          | Set to "false" to disable recovery notifications |
+| PostDataEnvVar            | Send post data. - [see below](#post-data-checks) |
+| RequestHeaders            | Custom request headers (e.g. {"Accept"="application/json"}) |
+| RequestHeadersEnvVar      | Custom request headers that should be stored in an environemnt variable, e.g., authentication headers. Behaves the same as PostDataEnvVar - [see below](#post-data-checks) |
 | BasicAuthUser             | Required for basic-authentication checks - [see below](#basic-auth-checks) |
-| ShouldContain              | Set to text string that has to be present in the HTML code of the page (configures "Should contain") |
-| Tags                        | Comma separated set of tags to apply to check (e.g. "testing,aws") |
-| AlertIntegrations                | `-` separated set list of integrations ids (e.g. "91166-12168") |
-| AlertContacts                | `-` separated contact id's (e.g. "1234567_8_9-9876543_2_1") to override the [default alertContacts](https://github.com/stakater/IngressMonitorController/blob/master/README.md#usage)|
-| TeamAlertContacts            | Teams to alert.  `-` separated set list of teams ids (e.g. "1234567_8_9-9876543_2_1)|
+| ShouldContain             | Set to text string that has to be present in the HTML code of the page (configures "Should contain") |
+| Tags                      | Comma separated set of tags to apply to check (e.g. "testing,aws") |
+| AlertIntegrations         | `-` separated set list of integrations ids (e.g. "91166-12168") |
+| AlertContacts             | `-` separated contact id's (e.g. "1234567_8_9-9876543_2_1") to override the [default alertContacts](https://github.com/stakater/IngressMonitorController/blob/master/README.md#usage)|
+| TeamAlertContacts         | Teams to alert.  `-` separated set list of teams ids (e.g. "1234567_8_9-9876543_2_1)|
 
 ### Basic Auth checks
 
@@ -71,9 +77,9 @@ envFrom:
       name: stakater-post-data
 ```
 
-If you set postData the request method will be automatically POST.
+If you set postDataEnvVar the request method will be automatically POST.
 
-## Example: 
+## Example
 
 ```yaml
 apiVersion: endpointmonitor.stakater.com/v1alpha1
@@ -97,4 +103,3 @@ spec:
     teamAlertContacts: "1234567_8_9-9876543_2_1,1234567_8_9-9876543_2_2"
     postDataEnvVar: "monitor-user"
 ```
-

pkg/controllers/endpointmonitor_deleted.go

@@ -39,7 +39,7 @@ func (r *EndpointMonitorReconciler) removeMonitorIfExists(monitorService *monito
 	// Monitor Exists
 	if monitor != nil {
 		// Monitor Exists, remove the monitor
-		log.Info("Removing monitor: " + monitorName + " from provider provider: " + monitorService.GetType())
+		log.Info("Removing monitor " + monitorName + " from provider: " + monitorService.GetType())
 		monitorService.Remove(*monitor)
 	} else {
 		log.Info("Cannot find monitor with name: " + monitorName + " for provider: " + monitorService.GetType())

pkg/monitors/pingdom/pingdom-monitor.go

@@ -30,7 +30,7 @@ type PingdomMonitorService struct {
 	client            *pingdom.Client
 }
 
-func (monitor *PingdomMonitorService) Equal(oldMonitor models.Monitor, newMonitor models.Monitor) bool {
+func (service *PingdomMonitorService) Equal(oldMonitor models.Monitor, newMonitor models.Monitor) bool {
 	// TODO: Retrieve oldMonitor config and compare it here
 	return false
 }
@@ -48,7 +48,7 @@ func (service *PingdomMonitorService) Setup(p config.Provider) {
 		BaseURL:  service.url,
 	})
 	if err != nil {
-		log.Info("Error Seting Up Monitor Service: " + err.Error())
+		log.Info("Error setting up Monitor Service", "error", err)
 	}
 }
 
@@ -62,15 +62,15 @@ func (service *PingdomMonitorService) GetByName(name string) (*models.Monitor, e
 		}
 	}
 
-	return match, fmt.Errorf("Unable to locate monitor with name %v", name)
+	return match, fmt.Errorf("Unable to locate monitor with name '%v'", name)
 }
 
 func (service *PingdomMonitorService) GetAll() []models.Monitor {
 	var monitors []models.Monitor
 
 	checks, err := service.client.Checks.List()
 	if err != nil {
-		log.Info("Error received while listing checks: " + err.Error())
+		log.Info("Error received while listing checks", "error", err)
 		return nil
 	}
 	for _, mon := range checks {
@@ -90,9 +90,9 @@ func (service *PingdomMonitorService) Add(m models.Monitor) {
 
 	_, err := service.client.Checks.Create(&httpCheck)
 	if err != nil {
-		log.Info(fmt.Sprintf("Error Adding Monitor %s %v", m.Name, err.Error()))
+		log.Info(fmt.Sprintf("Error adding Monitor '%s': %v", m.Name, err.Error()))
 	} else {
-		log.Info("Added monitor for: " + m.Name)
+		log.Info("Successfully added Monitor " + m.Name)
 	}
 }
 
@@ -102,9 +102,9 @@ func (service *PingdomMonitorService) Update(m models.Monitor) {
 
 	resp, err := service.client.Checks.Update(monitorID, &httpCheck)
 	if err != nil {
-		log.Info(fmt.Sprintf("Error updating Monitor %s %v", m.Name, err.Error()))
+		log.Info(fmt.Sprintf("Error updating Monitor '%s': %v", m.Name, err.Error()))
 	} else {
-		log.Info("Sucessfully updated Monitor "+m.Name, "Response", resp.Message)
+		log.Info("Successfully updated Monitor "+m.Name, "response", resp.Message)
 	}
 }
 
@@ -113,18 +113,17 @@ func (service *PingdomMonitorService) Remove(m models.Monitor) {
 
 	resp, err := service.client.Checks.Delete(monitorID)
 	if err != nil {
-		log.Info(fmt.Sprintf("Error deleting Monitor %s %v", m.Name, err.Error()))
+		log.Info(fmt.Sprintf("Error deleting Monitor '%s': %v", m.Name, err.Error()))
 	} else {
-		log.Info("Sucessfully deleted Monitor "+m.Name, "Response", resp.Message)
+		log.Info("Successfully deleted Monitor "+m.Name, "response", resp.Message)
 	}
 }
 
 func (service *PingdomMonitorService) createHttpCheck(monitor models.Monitor) pingdom.HttpCheck {
 	httpCheck := pingdom.HttpCheck{}
 	url, err := url.Parse(monitor.URL)
 	if err != nil {
-		log.Info(fmt.Sprintf("Error parsing url '%s' of monitor %s", monitor.Name, service.url))
-		log.Info("Unable to parse the URL: " + service.url)
+		log.Info(fmt.Sprintf("Error parsing url '%s' of monitor %s", service.url, monitor.Name))
 	}
 
 	if url.Scheme == "https" {
@@ -184,7 +183,7 @@ func (service *PingdomMonitorService) addConfigToHttpCheck(httpCheck *pingdom.Ht
 		userIdsStringArray := strings.Split(providerConfig.AlertContacts, "-")
 
 		if userIds, err := util.SliceAtoi(userIdsStringArray); err != nil {
-			log.Info("Error decoding user alert contact IDs from config" + err.Error())
+			log.Info("Error decoding user alert contact IDs from config", "error", err)
 		} else {
 			httpCheck.UserIds = userIds
 		}
@@ -194,7 +193,7 @@ func (service *PingdomMonitorService) addConfigToHttpCheck(httpCheck *pingdom.Ht
 		integrationIdsStringArray := strings.Split(providerConfig.AlertIntegrations, "-")
 
 		if integrationIds, err := util.SliceAtoi(integrationIdsStringArray); err != nil {
-			log.Info("Error decoding integration ids into integers" + err.Error())
+			log.Info("Error decoding integration ids into integers", "error", err)
 		} else {
 			httpCheck.IntegrationIds = integrationIds
 		}
@@ -204,7 +203,7 @@ func (service *PingdomMonitorService) addConfigToHttpCheck(httpCheck *pingdom.Ht
 		integrationTeamIdsStringArray := strings.Split(providerConfig.TeamAlertContacts, "-")
 
 		if integrationTeamIdsStringArray, err := util.SliceAtoi(integrationTeamIdsStringArray); err != nil {
-			log.Info("Error decoding integration ids into integers" + err.Error())
+			log.Info("Error decoding integration ids into integers", "error", err)
 		} else {
 			httpCheck.TeamIds = integrationTeamIdsStringArray
 		}
@@ -226,7 +225,28 @@ func (service *PingdomMonitorService) addConfigToHttpCheck(httpCheck *pingdom.Ht
 		httpCheck.RequestHeaders = make(map[string]string)
 		err := json.Unmarshal([]byte(providerConfig.RequestHeaders), &httpCheck.RequestHeaders)
 		if err != nil {
-			log.Info("Error Converting from string to JSON object")
+			log.Info("Error converting request headers from string to JSON", "value", providerConfig.RequestHeaders, "error", err)
+		}
+	}
+	if providerConfig != nil && len(providerConfig.RequestHeadersEnvVar) > 0 {
+		requestHeaderEnvValue := os.Getenv(providerConfig.RequestHeadersEnvVar)
+		if requestHeaderEnvValue != "" {
+			requestHeadersValue := make(map[string]string)
+			err := json.Unmarshal([]byte(requestHeaderEnvValue), &requestHeadersValue)
+			if err != nil {
+				log.Info("Error converting request headers from environment from string to JSON", "envVar", providerConfig.RequestHeadersEnvVar, "error", err)
+			}
+
+			if httpCheck.RequestHeaders != nil {
+				for key, value := range requestHeadersValue {
+					httpCheck.RequestHeaders[key] = value
+				}
+			} else {
+				httpCheck.RequestHeaders = requestHeadersValue
+			}
+
+		} else {
+			log.Error(errors.New("error reading request headers from environment variable"), "Environment Variable does not exist", "envVar", providerConfig.RequestHeadersEnvVar)
 		}
 	}
 
@@ -240,7 +260,7 @@ func (service *PingdomMonitorService) addConfigToHttpCheck(httpCheck *pingdom.Ht
 			httpCheck.Username = providerConfig.BasicAuthUser
 			httpCheck.Password = passwordValue
 		} else {
-			log.Info("Error reading basic auth password from environment variable")
+			log.Error(errors.New("error reading basic auth password from environment variable"), "Environment Variable does not exist", "envVar", providerConfig.BasicAuthUser)
 		}
 	}
 
@@ -266,7 +286,7 @@ func (service *PingdomMonitorService) addConfigToHttpCheck(httpCheck *pingdom.Ht
 			if postDataValue != "" {
 				httpCheck.PostData = postDataValue
 			} else {
-				log.Error(errors.New("error reading post data from environment variable"), "Environment Variable %s does not exist", providerConfig.PostDataEnvVar)
+				log.Error(errors.New("error reading post data from environment variable"), "Environment Variable does not exist", "envVar", providerConfig.PostDataEnvVar)
 			}
 		}
 	}

pkg/monitors/pingdomtransaction/pingdom-transaction-monitor.go

@@ -40,7 +40,7 @@ type PingdomTransactionMonitorService struct {
 	namespace         string
 }
 
-func (monitor *PingdomTransactionMonitorService) Equal(oldMonitor models.Monitor, newMonitor models.Monitor) bool {
+func (service *PingdomTransactionMonitorService) Equal(oldMonitor models.Monitor, newMonitor models.Monitor) bool {
 	// TODO: Retrieve oldMonitor config and compare it here
 	return false
 }
@@ -105,7 +105,7 @@ func (service *PingdomTransactionMonitorService) GetAll() []models.Monitor {
 func (service *PingdomTransactionMonitorService) GetUrlFromSteps(id int64) string {
 	check, _, err := service.client.TMSChecksAPI.GetCheck(service.context, id).Execute()
 	if err != nil {
-		log.Error(err, "Error getting transaction check")
+		log.Error(err, "Error getting transaction check", "id", id)
 		return ""
 	}
 	if check == nil {
@@ -126,7 +126,7 @@ func (service *PingdomTransactionMonitorService) Add(m models.Monitor) {
 	}
 	_, resp, err := service.client.TMSChecksAPI.AddCheck(service.context).CheckWithoutID(*transactionCheck).Execute()
 	if err != nil {
-		log.Error(err, "Error Adding Pingdom Transaction Monitor "+m.Name, "Response", parseResponseBody(resp))
+		log.Error(err, "Error adding Pingdom Transaction Monitor "+m.Name, "response", parseResponseBody(resp))
 	} else {
 		log.Info("Successfully added Pingdom Transaction Monitor " + m.Name)
 	}
@@ -140,18 +140,18 @@ func (service *PingdomTransactionMonitorService) Update(m models.Monitor) {
 	monitorID := util.StrToInt64(m.ID)
 	_, resp, err := service.client.TMSChecksAPI.ModifyCheck(service.context, monitorID).CheckWithoutIDPUT(*transactionCheck.AsPut()).Execute()
 	if err != nil {
-		log.Error(err, "Error Updating Pingdom Transaction Monitor", "Response", parseResponseBody(resp))
+		log.Error(err, "Error updating Pingdom Transaction Monitor", "response", parseResponseBody(resp))
 		return
 	}
-	log.Info("Updated Pingdom Transaction Monitor Monitor " + m.Name)
+	log.Info("Successfully updated Pingdom Transaction Monitor " + m.Name)
 }
 
 func (service *PingdomTransactionMonitorService) Remove(m models.Monitor) {
 	_, resp, err := service.client.TMSChecksAPI.DeleteCheck(service.context, util.StrToInt64(m.ID)).Execute()
 	if err != nil {
-		log.Error(err, "Error Deleting Pingdom Transaction Monitor", "Response", parseResponseBody(resp))
+		log.Error(err, "Error deleting Pingdom Transaction Monitor", "response", parseResponseBody(resp))
 	} else {
-		log.Info("Deleted Pingdom Transaction Monitor Monitor " + m.Name)
+		log.Info("Successfully deleted Pingdom Transaction Monitor " + m.Name)
 	}
 }
 
@@ -177,19 +177,19 @@ func (service *PingdomTransactionMonitorService) createTransactionCheck(monitor
 	if teamAlertContacts != nil {
 		transactionCheck.TeamIds = teamAlertContacts
 	}
-	service.addConfigToTranscationCheck(transactionCheck, monitor)
+	service.addConfigToTransactionCheck(transactionCheck, monitor)
 
 	return transactionCheck
 }
 
-func (service *PingdomTransactionMonitorService) addConfigToTranscationCheck(transactionCheck *pingdomNew.CheckWithoutID, monitor models.Monitor) {
+func (service *PingdomTransactionMonitorService) addConfigToTransactionCheck(transactionCheck *pingdomNew.CheckWithoutID, monitor models.Monitor) {
 
 	// Retrieve provider configuration
 	config := monitor.Config
 	providerConfig, _ := config.(*endpointmonitorv1alpha1.PingdomTransactionConfig)
 
 	if providerConfig == nil {
-		// providerConfig is not set, we create a go_to transaction by default from url because its required by API
+		// providerConfig is not set, we create a go_to transaction by default from url because it's required by API
 		transactionCheck.Steps = append(transactionCheck.Steps, pingdomNew.Step{
 			Args: &pingdomNew.StepArgs{
 				Url: ptr.String(monitor.URL),
@@ -233,7 +233,7 @@ func (service *PingdomTransactionMonitorService) addConfigToTranscationCheck(tra
 		transactionCheck.SeverityLevel = ptr.String(providerConfig.SeverityLevel)
 	}
 	if providerConfig.Interval > 0 {
-		transactionCheck.Interval = ptr.Int64((int64(providerConfig.Interval)))
+		transactionCheck.Interval = ptr.Int64(int64(providerConfig.Interval))
 	}
 	for _, step := range providerConfig.Steps {
 		args := service.NewStepArgsByMap(step.Args)
@@ -369,7 +369,7 @@ func parseResponseBody(resp *http.Response) string {
 	// Attempt to unmarshal the response body into a map
 	var responseBodyMap map[string]map[string]interface{}
 	if err := json.Unmarshal(bodyBytes, &responseBodyMap); err != nil {
-		// If unmarshaling fails, return the whole body as a string.
+		// If unmarshalling fails, return the whole body as a string.
 		return string(bodyBytes)
 	}
 	// Check if "error" key exists in the map