fix: enrich targets now accepts any scheme (#149)

* fix: enrich targets now accepts any scheme Signed-off-by: Niklas Treml <[email protected]> * chore: remove debug logs Signed-off-by: Niklas Treml <[email protected]> * chore: add test for new validation logic Signed-off-by: Niklas Treml <[email protected]> --------- Signed-off-by: Niklas Treml <[email protected]>
telekom · Jun 7, 2024 · 4d047e4 · 4d047e4
1 parent d55eefc
commit 4d047e4
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 11 deletions.
diff --git a/pkg/sparrow/run.go b/pkg/sparrow/run.go
@@ -21,6 +21,7 @@ package sparrow
 import (
 	"context"
 	"fmt"
+	"net/url"
 	"slices"
 	"strings"
 	"sync"
@@ -114,7 +115,7 @@ func (s *Sparrow) Run(ctx context.Context) error {
 	for {
 		select {
 		case cfg := <-s.cRuntime:
-			cfg = s.enrichTargets(cfg)
+			cfg = s.enrichTargets(ctx, cfg)
 			s.controller.Reconcile(ctx, cfg)
 		case <-ctx.Done():
 			s.shutdown(ctx)
@@ -131,24 +132,33 @@ func (s *Sparrow) Run(ctx context.Context) error {
 
 // enrichTargets updates the targets of the sparrow's checks with the
 // global targets. Per default, the two target lists are merged.
-func (s *Sparrow) enrichTargets(cfg runtime.Config) runtime.Config {
+func (s *Sparrow) enrichTargets(ctx context.Context, cfg runtime.Config) runtime.Config {
+	l := logger.FromContext(ctx)
 	if cfg.Empty() || s.tarMan == nil {
 		return cfg
 	}
 
 	for _, gt := range s.tarMan.GetTargets() {
-		if gt.Url == fmt.Sprintf("https://%s", s.config.SparrowName) {
+		u, err := url.Parse(gt.Url)
+		if err != nil {
+			l.Error("Failed to parse global target URL", "error", err, "url", gt.Url)
 			continue
 		}
-		if cfg.HasHealthCheck() && !slices.Contains(cfg.Health.Targets, gt.Url) {
-			cfg.Health.Targets = append(cfg.Health.Targets, gt.Url)
+
+		// split off hostWithoutPort because it could contain a port
+		hostWithoutPort := strings.Split(u.Host, ":")[0]
+		if hostWithoutPort == s.config.SparrowName {
+			continue
+		}
+
+		if cfg.HasHealthCheck() && !slices.Contains(cfg.Health.Targets, u.String()) {
+			cfg.Health.Targets = append(cfg.Health.Targets, u.String())
 		}
-		if cfg.HasLatencyCheck() && !slices.Contains(cfg.Latency.Targets, gt.Url) {
-			cfg.Latency.Targets = append(cfg.Latency.Targets, gt.Url)
+		if cfg.HasLatencyCheck() && !slices.Contains(cfg.Latency.Targets, u.String()) {
+			cfg.Latency.Targets = append(cfg.Latency.Targets, u.String())
 		}
-		if cfg.HasDNSCheck() && !slices.Contains(cfg.Dns.Targets, gt.Url) {
-			t, _ := strings.CutPrefix(gt.Url, "https://")
-			cfg.Dns.Targets = append(cfg.Dns.Targets, t)
+		if cfg.HasDNSCheck() && !slices.Contains(cfg.Dns.Targets, hostWithoutPort) {
+			cfg.Dns.Targets = append(cfg.Dns.Targets, hostWithoutPort)
 		}
 	}
 

diff --git a/pkg/sparrow/run_test.go b/pkg/sparrow/run_test.go
@@ -112,6 +112,7 @@ func TestSparrow_Run_ContextCancel(t *testing.T) {
 // TestSparrow_enrichTargets tests that the enrichTargets method
 // updates the targets of the configured checks.
 func TestSparrow_enrichTargets(t *testing.T) {
+	t.Parallel()
 	now := time.Now()
 	testTarget := "https://localhost.de"
 	gt := []checks.GlobalTarget{
@@ -237,6 +238,28 @@ func TestSparrow_enrichTargets(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "global targets contains http and https - dns validation still works does not fail and splits off scheme",
+			config: runtime.Config{
+				Dns: &dns.Config{
+					Targets: []string{},
+				},
+			},
+			globalTargets: []checks.GlobalTarget{
+				{
+					Url:      "http://az1.sparrow.com",
+					LastSeen: now,
+				},
+				{
+					Url: "https://az2.sparrow.com",
+				},
+			},
+			expected: runtime.Config{
+				Dns: &dns.Config{
+					Targets: []string{"az1.sparrow.com", "az2.sparrow.com"},
+				},
+			},
+		},
 	}
 
 	for _, tt := range tests {
@@ -249,7 +272,7 @@ func TestSparrow_enrichTargets(t *testing.T) {
 					SparrowName: "sparrow.com",
 				},
 			}
-			got := s.enrichTargets(tt.config)
+			got := s.enrichTargets(context.Background(), tt.config)
 			assert.Equal(t, tt.expected, got)
 		})
 	}