fix: handle region-less S3 URLs

semaphoreci · Oct 3, 2024 · b3a2a32 · b3a2a32
1 parent 2f90de9
commit b3a2a32
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 3 deletions.
diff --git a/pkg/api/signed_url.go b/pkg/api/signed_url.go
@@ -239,10 +239,15 @@ func parseGoogleStorageURL(URL *url.URL) (string, error) {
 	return parsed[1], nil
 }
 
-// S3 URLs follow the format 'https://<bucket-name>.s3.<region>.amazonaws.com/<path>'
-// Note: S3 URLs use the project id as a prefix, so we take that into account here as well
+// S3 URLs can be in two formats:
+// 1. With a region: 'https://<bucket-name>.s3.<region>.amazonaws.com/<path>'
+// 2. Without a region: 'https://<bucket-name>.s3.amazonaws.com/<path>'
+// We accept both formats here.
+//
+// Note: the hub's S3 URLs use the Semaphore project ID as a prefix,
+// so we take that into account here as well.
 func parseS3URL(URL *url.URL) (string, error) {
-	re := regexp.MustCompile(`https:\/\/(.+)\.s3\.(.+)\.amazonaws\.com\/[a-z0-9\-]+\/([^?]+)\?`)
+	re := regexp.MustCompile(`https:\/\/(.+)\.s3\.?(.+)?\.amazonaws\.com\/[a-z0-9\-]+\/([^?]+)\?`)
 	parsed := re.FindStringSubmatch(URL.String())
 	if len(parsed) < 4 {
 		log.Warn("Failed to parse S3 URL.\n")

diff --git a/pkg/api/signed_url_test.go b/pkg/api/signed_url_test.go
@@ -28,13 +28,27 @@ func Test__GetObject(t *testing.T) {
 		assert.Equal(t, "artifacts/project/projectid/myfile.txt", obj)
 	})
 
+	t.Run("S3 with region-less URL - file", func(t *testing.T) {
+		signedURL := SignedURL{URL: "https://my-bucket1.s3.amazonaws.com/projectid/artifacts/project/projectid/myfile.txt?X-Amz-Whatever"}
+		obj, err := signedURL.GetObject()
+		assert.Nil(t, err)
+		assert.Equal(t, "artifacts/project/projectid/myfile.txt", obj)
+	})
+
 	t.Run("S3 - file inside directory", func(t *testing.T) {
 		signedURL := SignedURL{URL: "https://my-bucket1.s3.us-east-1.amazonaws.com/projectid/artifacts/project/projectid/mydir/myfile.txt?X-Amz-Whatever"}
 		obj, err := signedURL.GetObject()
 		assert.Nil(t, err)
 		assert.Equal(t, "artifacts/project/projectid/mydir/myfile.txt", obj)
 	})
 
+	t.Run("S3 with region-less URL - file inside directory", func(t *testing.T) {
+		signedURL := SignedURL{URL: "https://my-bucket1.s3.amazonaws.com/projectid/artifacts/project/projectid/mydir/myfile.txt?X-Amz-Whatever"}
+		obj, err := signedURL.GetObject()
+		assert.Nil(t, err)
+		assert.Equal(t, "artifacts/project/projectid/mydir/myfile.txt", obj)
+	})
+
 	t.Run("127.0.0.1 - file", func(t *testing.T) {
 		signedURL := SignedURL{URL: "http://127.0.0.1:8080/artifacts/project/projectid/myfile.txt"}
 		obj, err := signedURL.GetObject()