From 6052958ece6eee4813da64075f62a6c6775d3756 Mon Sep 17 00:00:00 2001
From: jwierzbo <jakub.wierzbowski@arangodb.com>
Date: Mon, 26 Jun 2023 12:18:03 +0200
Subject: [PATCH] GT-452 Improve master endpoint validation (#1339)

---
 CHANGELOG.md                                  |  1 +
 .../v1/sync_external_access_spec.go           |  9 ++++-
 pkg/apis/deployment/v1/sync_spec_test.go      | 34 +++++++++++++++++++
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1d19d6aba..9a475b57c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## [master](https://github.com/arangodb/kube-arangodb/tree/master) (N/A)
 - (Improvement) Block traffic on the services if there is more than 1 active leader in ActiveFailover mode
+- (Improvement) Improve master endpoint validation.
 
 ## [1.2.30](https://github.com/arangodb/kube-arangodb/tree/1.2.30) (2023-06-16)
 - (Feature) AgencyCache Interface
diff --git a/pkg/apis/deployment/v1/sync_external_access_spec.go b/pkg/apis/deployment/v1/sync_external_access_spec.go
index 1e143c629..aafc60d78 100644
--- a/pkg/apis/deployment/v1/sync_external_access_spec.go
+++ b/pkg/apis/deployment/v1/sync_external_access_spec.go
@@ -63,8 +63,15 @@ func (s SyncExternalAccessSpec) Validate() error {
 		return errors.WithStack(err)
 	}
 	for _, ep := range s.MasterEndpoint {
-		if _, err := url.Parse(ep); err != nil {
+		if u, err := url.Parse(ep); err != nil {
 			return errors.WithStack(errors.Newf("Failed to parse master endpoint '%s': %s", ep, err))
+		} else {
+			if u.Scheme != "http" && u.Scheme != "https" {
+				return errors.WithStack(errors.Newf("Invalid scheme '%s' in master endpoint '%s'", u.Scheme, ep))
+			}
+			if u.Host == "" {
+				return errors.WithStack(errors.Newf("Missing host in master endpoint '%s'", ep))
+			}
 		}
 	}
 	for _, name := range s.AccessPackageSecretNames {
diff --git a/pkg/apis/deployment/v1/sync_spec_test.go b/pkg/apis/deployment/v1/sync_spec_test.go
index b4bb54226..22cc8c57b 100644
--- a/pkg/apis/deployment/v1/sync_spec_test.go
+++ b/pkg/apis/deployment/v1/sync_spec_test.go
@@ -102,3 +102,37 @@ func TestSyncSpecResetImmutableFields(t *testing.T) {
 		assert.Equal(t, test.Expected, test.Target)
 	}
 }
+
+func TestSyncSpecMasterEndpointValidate(t *testing.T) {
+	auth := SyncAuthenticationSpec{
+		JWTSecretName:      util.NewType[string]("foo"),
+		ClientCASecretName: util.NewType[string]("foo-client"),
+	}
+	tls := TLSSpec{
+		CASecretName: util.NewType[string]("None"),
+	}
+	t.Run("Valid MasterEndpoint", func(t *testing.T) {
+		err := SyncSpec{
+			Authentication: auth,
+			TLS:            tls,
+			ExternalAccess: SyncExternalAccessSpec{
+				MasterEndpoint: []string{"https://arangodb.xyz:8629"},
+			},
+			Enabled: util.NewType[bool](true),
+		}.Validate(DeploymentModeCluster)
+		assert.Nil(t, err)
+	})
+
+	t.Run("Invalid MasterEndpoint without protocol", func(t *testing.T) {
+		err := SyncSpec{
+			Authentication: auth,
+			TLS:            tls,
+			ExternalAccess: SyncExternalAccessSpec{
+				MasterEndpoint: []string{"example.com:8629"},
+			},
+			Enabled: util.NewType[bool](true),
+		}.Validate(DeploymentModeCluster)
+		assert.Error(t, err)
+		assert.Equal(t, "Invalid scheme 'example.com' in master endpoint 'example.com:8629'", err.Error())
+	})
+}