search

caas-team / sparrow

A monitoring tool to gather infrastructure network information
Apache License 2.0
6 stars 4 forks source link

Bug: DNS Check can never use targets supplied by target manager #148

Closed niklastreml closed 3 weeks ago

niklastreml commented 3 weeks ago

Is there an existing issue for this?

Current Behavior

Currently, targets supplied by targetmanager to the dns check fail the validation and cannot be registered. This is because the targets include a scheme like http or https, which the dns checks config validation will reject.

Expected Behavior

Sparrow should "just work", i.e. just parse the url and throwaway everything that is not the domain name

Steps To Reproduce

  1. Run sparrow with targetmanager enabled and configured.
  2. Configure a dns check
  3. Add a targetmanager remote target with a https scheme
  4. Check logs

Relevant logs and/or screenshots, environment information, etc.

{"time":"2024-06-06T13:19:38.257569916Z","level":"ERROR","source":{"function":"github.com/caas-team/sparrow/pkg/sparrow.(*ChecksController).Reconcile","file":"/home/runner/work/sparrow/sparrow/pkg/sparrow/controller.go","line":100},"msg":"Failed to create checks from config","error":"invalid configuration field \"targets\" in check \"dns\": target URLs must not start with 'https://' or 'http://'"}

Who can address the issue?

No response

Anything else?

No response

niklastreml commented 3 weeks ago

I'm going to create a temporary hacky fix for this, by just overwriting the name in the config validation. This will be very ugly, but we need this fixed now. Later on, this should be refactored and a more permanent solution should be implemented instead of this mess :/

niklastreml commented 3 weeks ago

Turns out I didn't have to do that, the issue was actually in the enrichTargets function. I'm writing a pull request right now, so you guys can have a look at it