stakater / IngressMonitorController

A Kubernetes controller to watch ingresses and create liveness alerts for your apps/microservices in UptimeRobot, StatusCake, Pingdom, etc. – [✩Star] if you're using it!
https://stakater.com
Apache License 2.0
578 stars 102 forks source link

Add Pingdom Transaction Checks #570

Closed karlderkaefer closed 2 months ago

karlderkaefer commented 7 months ago

Summary

This pull request addresses issue #553 #569 . The main changes are:

Detailed Feature List

Multi Provider Support:

It was not possible to add more than one provider. For example if you have provider config

providers:
  - name: PingdomTransaction
    apiURL: "https://api.pingdom.com/api/3.1"
    apiToken: <API_TOKEN>
  - name: Pingdom
    apiURL: "https://api.pingdom.com/api/3.1"
    apiToken: <API_TOKEN>
enableMonitorDeletion: true

If you add monitor regardless of the provider spec, the create and update events were fired to all available providers. This lead API errors and unwanted creation of monitors

We will now find the right service monitor proxy depending on the spec and sent create, update and delete events only to suitable providers. In case the provider could not be detected we use the first configured provider, which is default for endpoint monitors without specific provider spec.

Further more I added a enum for different providers

The existing spec does not assign a check to a service provider directly on the CRDs. We have check with either a provider config or without. This will lead to more code. It would be probably better to introduce a required field spec.monitorType to map each check explicitly to a provider. However that would introduce a breaking change (wanted to avoid that)

New Provider for Pingdom Transaction I created a new provider and package for Pingdom transaction provider since the fields are significantly different than Uptime checks. Also we need another client to do the API calls. So it's probably best if separate by packages

Using Pointer for Monitor Proxy The service monitor proxies are initialised on startup. It would make more sense for me if we use pointer here to avoid allocation of new objects. What do you think?

Refactor Logging Output

Secret Template to retrieve sensitive Data We have the requirement to set sensitive data such as password for some fields. We would like to avoid to store them in endpoint monitor CRD. Instead we just define a secret reference e.g. value: '{{secret:my-secret-name:my-secret-key}}'. The secret will be retrieved and is replacing the template before the post request is sent to Pingdom.

Minor fixes

Motivation

569

Testing

Pingdom-Transaction-Check

Additional Notes

Include any additional notes or comments you have about the changes made.

closes #569

github-actions[bot] commented 7 months ago

@karlderkaefer Image is available for testing. docker pull stakater/ingressmonitorcontroller:SNAPSHOT-PR-570-08fa1fdc

karlderkaefer commented 6 months ago

@karl-johan-grahn can you have look? I wanted to minimize changes but in the end still a lot. If splitting of the PR helps let me know

karlderkaefer commented 5 months ago

Hi @MuneebAijaz is there an update on the review status?

MuneebAijaz commented 5 months ago

@karlderkaefer apologies for the delay, this is scheduled for next week

karlderkaefer commented 2 months ago

@MuneebAijaz I have answered all comments, can we get an update on the review?

karlderkaefer commented 2 months ago

@MuneebAijaz I appreciate the approval very much. Although we have tested these changes intensively, If there any bug related I'm happy to fix. you can mention me or @dennis-ge