From e233997fc0fe5191d7364f2d6bb6d40d8c80bcdb Mon Sep 17 00:00:00 2001
From: Mustafa Bayar <mbayar@gitlab.com>
Date: Thu, 10 Oct 2024 12:09:25 +0300
Subject: [PATCH] config: Add awssdk query parameter to the s3 url

Go CDK v0.39.0 defaults to AWS SDK v2, introducing a token bucket
rate limiting mechanism that can cause unexpected behavior. This
commit adds 'awssdk=v1' to S3 URLs as a temporary workaround.
The upcoming Go CDK v0.40.0 will default to no rate limiting unless
explicitly defined, resolving this issue.
---
 internal/gitaly/config/config.go      | 20 ++++++++
 internal/gitaly/config/config_test.go | 68 +++++++++++++++++++++++++++
 2 files changed, 88 insertions(+)

diff --git a/internal/gitaly/config/config.go b/internal/gitaly/config/config.go
index 4f92c006f6..3870fa6cf5 100644
--- a/internal/gitaly/config/config.go
+++ b/internal/gitaly/config/config.go
@@ -918,6 +918,26 @@ func (cfg *Cfg) Sanitize() error {
 		cfg.Logging.Config.Level = "info"
 	}
 
+	// Go Cloud Development Kit (CDK) v0.39.0 introduced a default to AWS SDK v2,
+	// which includes a new token bucket rate limiting mechanism. This can cause
+	// unexpected behavior in some scenarios. To mitigate this, we use the 'awssdk=v1'
+	// query parameter to revert to v1 behavior. This workaround is temporary;
+	// Go CDK v0.40.0 will default to no rate limiting unless explicitly defined.
+	if cfg.Backup.GoCloudURL != "" {
+		parsedURL, err := url.Parse(cfg.Backup.GoCloudURL)
+		if err != nil {
+			return fmt.Errorf("parse GoCloudURL: %w", err)
+		}
+		if parsedURL.Scheme == "s3" {
+			query := parsedURL.Query()
+			if query.Get("awssdk") == "" {
+				query.Set("awssdk", "v1")
+				parsedURL.RawQuery = query.Encode()
+				cfg.Backup.GoCloudURL = parsedURL.String()
+			}
+		}
+	}
+
 	return nil
 }
 
diff --git a/internal/gitaly/config/config_test.go b/internal/gitaly/config/config_test.go
index dcc94004ad..3c3b048a43 100644
--- a/internal/gitaly/config/config_test.go
+++ b/internal/gitaly/config/config_test.go
@@ -2967,3 +2967,71 @@ initial_members = {1 = "localhost:4001", 2 = "localhost:4002", 3 = "localhost:40
 	require.NoError(t, expectedCfg.Sanitize())
 	require.Equal(t, expectedCfg.Raft, cfg.Raft)
 }
+
+func TestGoCloudURL(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		name        string
+		cfg         Cfg
+		expectedURL string
+		expectedErr error
+	}{
+		{
+			name: "invalid url provided",
+			cfg: Cfg{
+				Backup: BackupConfig{
+					GoCloudURL: "s3://my bucket",
+				},
+			},
+			expectedErr: fmt.Errorf(""),
+		},
+		{
+			name: "url is not s3",
+			cfg: Cfg{
+				Backup: BackupConfig{
+					GoCloudURL: "gs://my-bucket",
+				},
+			},
+			expectedURL: "gs://my-bucket",
+		},
+		{
+			name: "no query parameter provided",
+			cfg: Cfg{
+				Backup: BackupConfig{
+					GoCloudURL: "s3://my-bucket",
+				},
+			},
+			expectedURL: "s3://my-bucket?awssdk=v1",
+		},
+		{
+			name: "query parameter other than awssdk provided",
+			cfg: Cfg{
+				Backup: BackupConfig{
+					GoCloudURL: "s3://my-bucket?foo=bar",
+				},
+			},
+			expectedURL: "s3://my-bucket?awssdk=v1&foo=bar",
+		},
+		{
+			name: "awssdk query parameter provided",
+			cfg: Cfg{
+				Backup: BackupConfig{
+					GoCloudURL: "s3://my-bucket?awssdk=v2",
+				},
+			},
+			expectedURL: "s3://my-bucket?awssdk=v2",
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			err := tc.cfg.Sanitize()
+
+			if tc.expectedErr == nil {
+				require.NoError(t, err)
+				require.Equal(t, tc.expectedURL, tc.cfg.Backup.GoCloudURL)
+			} else {
+				require.Error(t, err, "parse GoCloudURL: parse \"s3://my bucket\": invalid character")
+			}
+		})
+	}
+}
-- 
GitLab