feat(elasticsearch): remove hard-coded port 9200, validate URL input (#…

…941)

This PR enables a flexible ES endpoints by removing hard-coded ES port
in the endpoints section. This PR also sanitize/validate whether user
input a valid URL or not by utilizing `url.Parse`. I'm wondering whether
we do need to have a test or not since `url.Parse` function already have
their own
[test](https://cs.opensource.google/go/go/+/refs/tags/go1.21.6:src/net/url/url_test.go)

Please advise if we need more test cases

This PR will close #940

---------

Signed-off-by: clavinjune <24659468+clavinjune@users.noreply.github.com>

autoscaler/controllers/gateway/config/elasticsearch.go

@@ -1,7 +1,8 @@
 package config
 
 import (
-	"fmt"
+	"errors"
+	"net/url"
 
 	odigosv1 "github.com/keyval-dev/odigos/api/odigos/v1alpha1"
 	commonconf "github.com/keyval-dev/odigos/autoscaler/controllers/common"
@@ -15,19 +16,27 @@ const (
 	esLogsIndexKey      = "ES_LOGS_INDEX"
 )
 
+var _ Configer = (*Elasticsearch)(nil)
+
 type Elasticsearch struct{}
 
 func (e *Elasticsearch) DestType() common.DestinationType {
 	return common.ElasticsearchDestinationType
 }
 
 func (e *Elasticsearch) ModifyConfig(dest *odigosv1.Destination, currentConfig *commonconf.Config) {
-	url, exists := dest.Spec.Data[elasticsearchUrlKey]
+	rawURL, exists := dest.Spec.Data[elasticsearchUrlKey]
 	if !exists {
 		log.Log.V(0).Info("ElasticSearch url not specified, gateway will not be configured for ElasticSearch")
 		return
 	}
 
+	parsedURL, err := e.SanitizeURL(rawURL)
+	if err != nil {
+		log.Log.V(0).Error(err, "failed to sanitize URL", "elasticsearch-url", rawURL)
+		return
+	}
+
 	if isTracingEnabled(dest) {
 		esTraceExporterName := "elasticsearch/trace-" + dest.Name
 		traceIndexVal, exists := dest.Spec.Data[esTracesIndexKey]
@@ -36,7 +45,7 @@ func (e *Elasticsearch) ModifyConfig(dest *odigosv1.Destination, currentConfig *
 		}
 
 		currentConfig.Exporters[esTraceExporterName] = commonconf.GenericMap{
-			"endpoints":    []string{fmt.Sprintf("%s:9200", url)},
+			"endpoints":    []string{parsedURL},
 			"traces_index": traceIndexVal,
 		}
 
@@ -56,7 +65,7 @@ func (e *Elasticsearch) ModifyConfig(dest *odigosv1.Destination, currentConfig *
 		}
 
 		currentConfig.Exporters[esLogExporterName] = commonconf.GenericMap{
-			"endpoints":  []string{fmt.Sprintf("%s:9200", url)},
+			"endpoints":  []string{parsedURL},
 			"logs_index": logIndexVal,
 		}
 
@@ -68,3 +77,22 @@ func (e *Elasticsearch) ModifyConfig(dest *odigosv1.Destination, currentConfig *
 		}
 	}
 }
+
+// SanitizeURL will check whether URL is correct by utilizing url.ParseRequestURI
+// if the said URL has not defined any port, 9200 will be used in order to keep the backward compatibility with current configuration
+func (e *Elasticsearch) SanitizeURL(URL string) (string, error) {
+	parsedURL, err := url.ParseRequestURI(URL)
+	if err != nil {
+		return "", err
+	}
+
+	if parsedURL.Scheme == "" || parsedURL.Host == "" {
+		return "", errors.New("invalid URL")
+	}
+
+	if !urlHostContainsPort(parsedURL.Host) {
+		parsedURL.Host += ":9200"
+	}
+
+	return parsedURL.String(), nil
+}

autoscaler/controllers/gateway/config/elasticsearch_test.go

@@ -0,0 +1,93 @@
+package config_test
+
+import (
+	"testing"
+
+	"github.com/keyval-dev/odigos/autoscaler/controllers/gateway/config"
+	"github.com/stretchr/testify/require"
+)
+
+func TestElasticsearch_SanitizeURL(t *testing.T) {
+	tt := []struct {
+		_           struct{}
+		URL         string
+		ExpectedURL string
+		ExpectedErr string
+	}{
+		{
+			URL:         "http://localhost:9200/",
+			ExpectedURL: "http://localhost:9200/",
+			ExpectedErr: "",
+		},
+		{
+			URL:         "http://localhost",
+			ExpectedURL: "http://localhost:9200",
+			ExpectedErr: "",
+		},
+		{
+			URL:         "http:///localhost",
+			ExpectedURL: "",
+			ExpectedErr: "invalid URL",
+		},
+		{
+			URL:         "localhost",
+			ExpectedURL: "",
+			ExpectedErr: "invalid URI for request",
+		},
+		{
+			URL:         "http://user:pass@localhost:9200",
+			ExpectedURL: "http://user:pass@localhost:9200",
+			ExpectedErr: "",
+		},
+		{
+			URL:         "http://user:pass@localhost:80",
+			ExpectedURL: "http://user:pass@localhost:80",
+			ExpectedErr: "",
+		},
+		{
+			URL:         "https://foobar.com:8443",
+			ExpectedURL: "https://foobar.com:8443",
+			ExpectedErr: "",
+		},
+		// IPs
+		{
+			URL:         "127.0.0.1:8080",
+			ExpectedURL: "",
+			ExpectedErr: "invalid URI for request",
+		},
+		{
+			URL:         "http://127.0.0.1:8080",
+			ExpectedURL: "http://127.0.0.1:8080",
+			ExpectedErr: "",
+		},
+		{
+			URL:         "[::1]:8080",
+			ExpectedURL: "",
+			ExpectedErr: "invalid URI for request",
+		},
+		{
+			URL:         "http://[::1]:8080",
+			ExpectedURL: "http://[::1]:8080",
+			ExpectedErr: "",
+		},
+	}
+
+	var es config.Elasticsearch
+	for i := range tt {
+		tc := tt[i]
+		t.Run(tc.URL, func(t *testing.T) {
+			t.Parallel()
+			r := require.New(t)
+
+			actualURL, actualErr := es.SanitizeURL(tc.URL)
+			if tc.ExpectedErr != "" {
+				r.Error(actualErr)
+				r.Contains(actualErr.Error(), tc.ExpectedErr)
+			} else {
+				r.NoError(actualErr)
+				r.Equal(tc.ExpectedURL, actualURL)
+			}
+
+		})
+	}
+}