From 8375b985e864cf47b565d47cf9871720c234cee1 Mon Sep 17 00:00:00 2001
From: Joachim Bauch <bauch@struktur.de>
Date: Thu, 17 Jul 2025 12:51:04 +0200
Subject: [PATCH] Improve detecting duplicate backend URLs in etcd
 configuration.

---
 api_backend.go      |  22 +++++++--
 api_backend_test.go | 107 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 125 insertions(+), 4 deletions(-)

diff --git a/api_backend.go b/api_backend.go
index 1a17ed8..e20d5bf 100644
--- a/api_backend.go
+++ b/api_backend.go
@@ -454,7 +454,7 @@ type BackendInformationEtcd struct {
 	SessionLimit uint64 `json:"sessionlimit,omitempty"`
 }
 
-func (p *BackendInformationEtcd) CheckValid() error {
+func (p *BackendInformationEtcd) CheckValid() (err error) {
 	if p.Secret == "" {
 		return fmt.Errorf("secret missing")
 	}
@@ -462,17 +462,31 @@ func (p *BackendInformationEtcd) CheckValid() error {
 	if len(p.Urls) > 0 {
 		slices.Sort(p.Urls)
 		p.Urls = slices.Compact(p.Urls)
-		for idx, u := range p.Urls {
+		seen := make(map[string]bool)
+		outIdx := 0
+		for _, u := range p.Urls {
 			parsedUrl, err := url.Parse(u)
 			if err != nil {
 				return fmt.Errorf("invalid url %s: %w", u, err)
 			}
+
 			if strings.Contains(parsedUrl.Host, ":") && hasStandardPort(parsedUrl) {
 				parsedUrl.Host = parsedUrl.Hostname()
-				p.Urls[idx] = parsedUrl.String()
+				u = parsedUrl.String()
+				p.Urls[outIdx] = u
+			} else {
+				p.Urls[outIdx] = u
 			}
-
+			if seen[u] {
+				continue
+			}
+			seen[u] = true
 			p.parsedUrls = append(p.parsedUrls, parsedUrl)
+			outIdx++
+		}
+		if len(p.Urls) != outIdx {
+			clear(p.Urls[outIdx:])
+			p.Urls = p.Urls[:outIdx]
 		}
 	} else if p.Url != "" {
 		parsedUrl, err := url.Parse(p.Url)
diff --git a/api_backend_test.go b/api_backend_test.go
index 724075d..1a46212 100644
--- a/api_backend_test.go
+++ b/api_backend_test.go
@@ -72,3 +72,110 @@ func TestValidNumbers(t *testing.T) {
 		assert.False(isValidNumber(number), "number %s should not be valid", number)
 	}
 }
+
+func TestValidateBackendInformationEtcd(t *testing.T) {
+	t.Parallel()
+	assert := assert.New(t)
+
+	testcases := []struct {
+		b             BackendInformationEtcd
+		expectedError string
+		expectedUrls  []string
+	}{
+		{
+			b:             BackendInformationEtcd{},
+			expectedError: "secret missing",
+		},
+		{
+			b: BackendInformationEtcd{
+				Secret: "verysecret",
+			},
+			expectedError: "urls missing",
+		},
+		{
+			b: BackendInformationEtcd{
+				Secret: "verysecret",
+				Url:    "https://foo\n",
+			},
+			expectedError: "invalid url",
+		},
+		{
+			b: BackendInformationEtcd{
+				Secret: "verysecret",
+				Urls:   []string{"https://foo\n"},
+			},
+			expectedError: "invalid url",
+		},
+		{
+			b: BackendInformationEtcd{
+				Secret: "verysecret",
+				Urls:   []string{"https://foo", "https://foo\n"},
+			},
+			expectedError: "invalid url",
+		},
+		{
+			b: BackendInformationEtcd{
+				Secret: "verysecret",
+				Url:    "https://foo:443",
+			},
+			expectedUrls: []string{"https://foo"},
+		},
+		{
+			b: BackendInformationEtcd{
+				Secret: "verysecret",
+				Urls:   []string{"https://foo:443"},
+			},
+			expectedUrls: []string{"https://foo"},
+		},
+		{
+			b: BackendInformationEtcd{
+				Secret: "verysecret",
+				Url:    "https://foo:8443",
+			},
+			expectedUrls: []string{"https://foo:8443"},
+		},
+		{
+			b: BackendInformationEtcd{
+				Secret: "verysecret",
+				Urls:   []string{"https://foo:8443"},
+			},
+			expectedUrls: []string{"https://foo:8443"},
+		},
+		{
+			b: BackendInformationEtcd{
+				Secret: "verysecret",
+				Urls:   []string{"https://foo", "https://bar", "https://foo"},
+			},
+			expectedUrls: []string{"https://bar", "https://foo"},
+		},
+		{
+			b: BackendInformationEtcd{
+				Secret: "verysecret",
+				Urls:   []string{"https://foo", "https://bar", "https://foo:443", "https://zaz"},
+			},
+			expectedUrls: []string{"https://bar", "https://foo", "https://zaz"},
+		},
+		{
+			b: BackendInformationEtcd{
+				Secret: "verysecret",
+				Urls:   []string{"https://foo:443", "https://bar", "https://foo", "https://zaz"},
+			},
+			expectedUrls: []string{"https://bar", "https://foo", "https://zaz"},
+		},
+	}
+
+	for idx, tc := range testcases {
+		if tc.expectedError == "" {
+			if assert.NoError(tc.b.CheckValid(), "failed for testcase %d", idx) {
+				assert.Equal(tc.expectedUrls, tc.b.Urls, "failed for testcase %d", idx)
+				var urls []string
+				for _, u := range tc.b.parsedUrls {
+					urls = append(urls, u.String())
+				}
+				assert.Equal(tc.expectedUrls, urls, "failed for testcase %d", idx)
+			}
+		} else {
+			assert.ErrorContains(tc.b.CheckValid(), tc.expectedError, "failed for testcase %d, got %+v", idx, tc.b.parsedUrls)
+		}
+	}
+}