From f2950344051277430b0b05ba6d3e52ce0606a2ec Mon Sep 17 00:00:00 2001 From: Philip Gough Date: Fri, 10 May 2024 16:34:31 +0100 Subject: [PATCH] Validate url for external remote write endpoint Signed-off-by: Philip Gough --- .../observatorium.go | 11 +++- .../observatorium_test.go | 3 +- .../pkg/util/remotewriteendpoint.go | 20 ++++++++ .../pkg/util/remotewriteendpoint_test.go | 51 +++++++++++++++++++ 4 files changed, 81 insertions(+), 4 deletions(-) diff --git a/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium.go b/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium.go index 9dfae3f85..98db22661 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/controllers/multiclusterobservability/observatorium_test.go b/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium_test.go index 993a9e006..7d07fa0c4 100644 --- a/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium_test.go +++ b/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium_test.go @@ -90,8 +90,7 @@ func TestNewDefaultObservatoriumSpec(t *testing.T) { }, Type: "Opaque", Data: map[string][]byte{ - "write_key": []byte(`url: http://remotewrite/endpoint -`), + "write_key": []byte(`url: http://remotewrite/endpoint`), }, } diff --git a/operators/multiclusterobservability/pkg/util/remotewriteendpoint.go b/operators/multiclusterobservability/pkg/util/remotewriteendpoint.go index 221391705..6280c09ac 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,24 @@ 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.URL.String() == "" { + return fmt.Errorf("url is required for remote write endpoint %s", res.Name) + } + + u, err := url.ParseRequestURI(res.URL.String()) + if err != nil { + return fmt.Errorf("url %s is invalid for remote write endpoint %s: %s", res.URL.String(), res.Name, err) + } + + if u.Scheme == "" || u.Scheme != "http" && u.Scheme != "https" { + return fmt.Errorf("url %s is invalid for remote write endpoint %s: scheme must be http or https", res.URL.String(), res.Name) + } + + 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 dc28d7c76..b6e7d9707 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 missing url", + endpoint: &RemoteWriteEndpointWithSecret{Name: "valid-name", URL: mustParseURL(t, "")}, + wantErr: true, + }, + { + name: "test invalid url no scheme", + endpoint: &RemoteWriteEndpointWithSecret{Name: "valid-name", URL: mustParseURL(t, "invalid-url")}, + wantErr: true, + }, + { + name: "test valid url invalid scheme", + endpoint: &RemoteWriteEndpointWithSecret{Name: "valid-name", URL: mustParseURL(t, "httttp://some-valid-host.com:8080/prometheus/api/v1/write")}, + 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} +}