From 3ae00da15d50a64a93bf5d83a830b4f1f794dff0 Mon Sep 17 00:00:00 2001
From: Philip Gough <philip.p.gough@gmail.com>
Date: Fri, 10 May 2024 16:34:31 +0100
Subject: [PATCH] Validate url for external remote write endpoint

Signed-off-by: Philip Gough <philip.p.gough@gmail.com>
---
 .../observatorium.go                          | 11 +++-
 .../pkg/util/remotewriteendpoint.go           | 19 +++++++
 .../pkg/util/remotewriteendpoint_test.go      | 51 +++++++++++++++++++
 3 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium.go b/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium.go
index 9dfae3f859..98db226617 100644
--- a/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium.go
+++ b/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium.go
@@ -533,8 +533,8 @@ func newAPISpec(c client.Client, mco *mcov1beta2.MultiClusterObservability) (obs
 	apiSpec.ImagePullPolicy = mcoconfig.GetImagePullPolicy(mco.Spec)
 	apiSpec.ServiceMonitor = true
 	if mco.Spec.StorageConfig.WriteStorage != nil {
-		eps := []mcoutil.RemoteWriteEndpointWithSecret{}
-		mountSecrets := []string{}
+		var eps []mcoutil.RemoteWriteEndpointWithSecret
+		var mountSecrets []string
 		for _, storageConfig := range mco.Spec.StorageConfig.WriteStorage {
 			storageSecret := &v1.Secret{}
 			err := c.Get(context.TODO(), types.NamespacedName{Name: storageConfig.Name,
@@ -560,6 +560,13 @@ func newAPISpec(c client.Client, mco *mcov1beta2.MultiClusterObservability) (obs
 					log.Error(err, "Failed to unmarshal data in secret", "name", storageConfig.Name)
 					return apiSpec, err
 				}
+
+				err = ep.Validate()
+				if err != nil {
+					log.Error(err, "Failed to validate data in secret", "name", storageConfig.Name)
+					return apiSpec, err
+				}
+
 				newEp := &mcoutil.RemoteWriteEndpointWithSecret{
 					Name: storageConfig.Name,
 					URL:  ep.URL,
diff --git a/operators/multiclusterobservability/pkg/util/remotewriteendpoint.go b/operators/multiclusterobservability/pkg/util/remotewriteendpoint.go
index 2213917052..397ce8706b 100644
--- a/operators/multiclusterobservability/pkg/util/remotewriteendpoint.go
+++ b/operators/multiclusterobservability/pkg/util/remotewriteendpoint.go
@@ -5,6 +5,8 @@
 package util
 
 import (
+	"fmt"
+	"net/url"
 	"path"
 
 	"github.com/prometheus/common/config"
@@ -95,6 +97,23 @@ type RemoteWriteEndpointWithSecret struct {
 	HttpClientConfig *HTTPClientConfigWithSecret `yaml:"http_client_config,omitempty" json:"http_client_config,omitempty"`
 }
 
+// Validate validates the remote write endpoint
+func (res *RemoteWriteEndpointWithSecret) Validate() error {
+	if res.Name == "" {
+		return fmt.Errorf("name is required for remote write endpoint")
+	}
+
+	if res.URL.String() == "" {
+		return fmt.Errorf("url is required for remote write endpoint %s", res.Name)
+	}
+
+	if _, err := url.ParseRequestURI(res.URL.String()); err != nil {
+		return fmt.Errorf("url %s is invalid for remote write endpoint %s: %s", res.URL.String(), res.Name, err)
+	}
+
+	return nil
+}
+
 func getMountPath(secretName, key string) string {
 	return path.Join(MountPath, secretName, key)
 }
diff --git a/operators/multiclusterobservability/pkg/util/remotewriteendpoint_test.go b/operators/multiclusterobservability/pkg/util/remotewriteendpoint_test.go
index dc28d7c768..fe4d71fc08 100644
--- a/operators/multiclusterobservability/pkg/util/remotewriteendpoint_test.go
+++ b/operators/multiclusterobservability/pkg/util/remotewriteendpoint_test.go
@@ -5,8 +5,11 @@
 package util
 
 import (
+	"net/url"
 	"path"
 	"testing"
+
+	"github.com/prometheus/common/config"
 )
 
 const (
@@ -84,3 +87,51 @@ func TestTransform(t *testing.T) {
 		t.Fatalf("Wrong number of mount secrets: expect 5, get %d", len(names))
 	}
 }
+
+func TestValidateRemoteWriteEndpointWithSecret(t *testing.T) {
+	testCases := []struct {
+		name     string
+		endpoint *RemoteWriteEndpointWithSecret
+		wantErr  bool
+	}{
+		{
+			name:     "test empty name",
+			endpoint: &RemoteWriteEndpointWithSecret{Name: "", URL: mustParseURL(t, "https://example.com")},
+			wantErr:  true,
+		},
+		{
+			name:     "test missing url",
+			endpoint: &RemoteWriteEndpointWithSecret{Name: "valid-name", URL: mustParseURL(t, "")},
+			wantErr:  true,
+		},
+		{
+			name:     "test invalid url",
+			endpoint: &RemoteWriteEndpointWithSecret{Name: "valid-name", URL: mustParseURL(t, "invalid-url")},
+			wantErr:  true,
+		},
+		{
+			name:     "test happy path",
+			endpoint: &RemoteWriteEndpointWithSecret{Name: "valid-name", URL: mustParseURL(t, "https://example.com")},
+			wantErr:  false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := tc.endpoint.Validate()
+			if (err != nil) != tc.wantErr {
+				t.Errorf("Validate() error = %v, wantErr %v", err, tc.wantErr)
+				return
+			}
+		})
+	}
+}
+
+func mustParseURL(t *testing.T, s string) config.URL {
+	u, err := url.Parse(s)
+	if err != nil {
+		t.Fatalf(err.Error())
+	}
+
+	return config.URL{URL: u}
+}