Fallback download URL logic for steps

activator/steplib/activate.go

@@ -15,8 +15,13 @@ import (
 )
 
 const precompiledStepsEnv = "BITRISE_EXPERIMENT_PRECOMPILED_STEPS"
-const precompiledStepsDefaultStorage = "https://storage.googleapis.com/bitrise-steplib-storage"
-const precompiledStepsPrimaryStorageEnv = "BITRISE_PRECOMPILED_STEPS_PRIMARY_STORAGE"
+const precompiledStepsStorageURLsEnv = "BITRISE_PRECOMPILED_STEPS_STORAGE_URLS"
+const precompiledStepsPrimaryStorageEnvDeprecated = "BITRISE_PRECOMPILED_STEPS_PRIMARY_STORAGE"
+
+var precompiledStepsDefaultStorageURLs = []string{
+	"https://storage.googleapis.com/bitrise-steplib-storage",
+	"https://storage-gateway.services.bitrise.io",
+}
 
 func ActivateStep(stepLibURI, id, version, destination, destinationStepYML string, log stepman.Logger, isOfflineMode bool) (string, error) {
 	stepCollection, err := stepman.ReadStepSpec(stepLibURI)

activator/steplib/activate_executable.go

@@ -3,12 +3,13 @@ package steplib
 import (
 	"crypto/sha256"
 	"encoding/hex"
+	"errors"
 	"fmt"
 	"io"
 	"os"
 	"path/filepath"
 	"strings"
-	
+
 	"github.com/bitrise-io/go-utils/log"
 	"github.com/bitrise-io/stepman/models"
 	"github.com/hashicorp/go-retryablehttp"
@@ -22,19 +23,12 @@ func activateStepExecutable(
 	destinationDir string,
 	destinationStepYML string,
 ) (string, error) {
-	url := downloadURL(executable)
-
-	if strings.HasPrefix(url, "http://") {
-		return "", fmt.Errorf("http URL is unsupported, please use https: %s", url)
-	}
-
-	resp, err := retryablehttp.Get(url)
+	body, err := downloadExecutable(executable)
 	if err != nil {
-		return "", fmt.Errorf("fetch from %s: %w", url, err)
+		return "", err
 	}
 	defer func() {
-		err := resp.Body.Close()
-		if err != nil {
+		if err := body.Close(); err != nil {
 			log.Warnf("Failed to close response body: %s\n", err)
 		}
 	}()
@@ -56,9 +50,9 @@ func activateStepExecutable(
 		}
 	}()
 
-	_, err = io.Copy(file, resp.Body)
+	_, err = io.Copy(file, body)
 	if err != nil {
-		return "", fmt.Errorf("download %s to %s: %w", url, path, err)
+		return "", fmt.Errorf("download to %s: %w", path, err)
 	}
 
 	err = validateHash(path, executable.Hash)
@@ -106,11 +100,61 @@ func validateHash(filePath string, expectedHash string) error {
 	return nil
 }
 
-func downloadURL(executable models.Executable) string {
-	baseURL := os.Getenv(precompiledStepsPrimaryStorageEnv)
-	if baseURL == "" {
-		baseURL = precompiledStepsDefaultStorage
+func buildDownloadURLs(executable models.Executable) ([]string, error) {
+	bases := precompiledStepsDefaultStorageURLs
+	if override := os.Getenv(precompiledStepsStorageURLsEnv); override != "" {
+		bases = strings.Split(override, ",")
+	} else if legacy := os.Getenv(precompiledStepsPrimaryStorageEnvDeprecated); legacy != "" {
+		log.Warnf("%s is deprecated, use %s (comma-separated list) instead\n", precompiledStepsPrimaryStorageEnvDeprecated, precompiledStepsStorageURLsEnv)
+		bases = []string{legacy}
+	}
+
+	uri := strings.TrimLeft(executable.StorageURI, "/")
+	var urls []string
+	for _, base := range bases {
+		base = strings.TrimRight(strings.TrimSpace(base), "/")
+		if base == "" {
+			continue
+		}
+		url := fmt.Sprintf("%s/%s", base, uri)
+		if strings.HasPrefix(url, "http://") {
+			return nil, fmt.Errorf("http URL is unsupported, please use https: %s", url)
+		}
+		urls = append(urls, url)
+	}
+
+	if len(urls) == 0 {
+		return nil, fmt.Errorf("no storage URLs configured")
+	}
+	return urls, nil
+}
+
+func downloadExecutable(executable models.Executable) (io.ReadCloser, error) {
+	urls, err := buildDownloadURLs(executable)
+	if err != nil {
+		return nil, err
+	}
+	return downloadFromURLs(urls)
+}
+
+func downloadFromURLs(urls []string) (io.ReadCloser, error) {
+	var errs []error
+	for _, url := range urls {
+		resp, err := retryablehttp.Get(url)
+		if err == nil && resp.StatusCode < 400 {
+			return resp.Body, nil
+		}
+
+		if err != nil {
+			log.Warnf("Failed to download from %s: %s\n", url, err)
+			errs = append(errs, fmt.Errorf("%s: %w", url, err))
+		} else {
+			if closeErr := resp.Body.Close(); closeErr != nil {
+				log.Warnf("Failed to close response body: %s\n", closeErr)
+			}
+			log.Warnf("Storage returned status %d for %s\n", resp.StatusCode, url)
+			errs = append(errs, fmt.Errorf("%s: status %d", url, resp.StatusCode))
+		}
 	}
-	baseURL = strings.TrimRight(baseURL, "/")
-	return fmt.Sprintf("%s/%s", baseURL, strings.TrimLeft(executable.StorageURI, "/"))
+	return nil, fmt.Errorf("failed to download executable: %w", errors.Join(errs...))
 }

activator/steplib/activate_executable_test.go

@@ -2,6 +2,9 @@ package steplib
 
 import (
 	"fmt"
+	"io"
+	"net/http"
+	"net/http/httptest"
 	"testing"
 
 	"github.com/bitrise-io/stepman/models"
@@ -60,80 +63,166 @@ func TestValidateHash(t *testing.T) {
 	}
 }
 
-func TestDownloadURL(t *testing.T) {
+func TestBuildDownloadURLs(t *testing.T) {
 	tests := []struct {
-		name        string
-		env         map[string]string
-		executable  models.Executable
-		expectedURL string
+		name         string
+		storageURLs  string
+		executable   models.Executable
+		expectedURLs []string
+		expectedErr  error
 	}{
 		{
-			name: "With custom base URL",
-			env: map[string]string{
-				precompiledStepsPrimaryStorageEnv: "https://custom.example.com/storage",
-			},
+			name: "Default list: GCS first, gateway second",
 			executable: models.Executable{
 				StorageURI: "steps/step1.tar.gz",
 			},
-			expectedURL: "https://custom.example.com/storage/steps/step1.tar.gz",
-		},
-		{
-			name: "With custom base URL with trailing slash",
-			env: map[string]string{
-				precompiledStepsPrimaryStorageEnv: "https://custom.example.com/storage/",
+			expectedURLs: []string{
+				"https://storage.googleapis.com/bitrise-steplib-storage/steps/step1.tar.gz",
+				"https://storage-gateway.services.bitrise.io/steps/step1.tar.gz",
 			},
-			executable: models.Executable{
-				StorageURI: "steps/step1.tar.gz",
-			},
-			expectedURL: "https://custom.example.com/storage/steps/step1.tar.gz",
 		},
 		{
-			name: "With default base URL",
-			env:  map[string]string{},
+			name:        "Override list via env var",
+			storageURLs: "https://a.example.com,https://b.example.com",
 			executable: models.Executable{
 				StorageURI: "steps/step2.tar.gz",
 			},
-			expectedURL: "https://storage.googleapis.com/bitrise-steplib-storage/steps/step2.tar.gz",
+			expectedURLs: []string{
+				"https://a.example.com/steps/step2.tar.gz",
+				"https://b.example.com/steps/step2.tar.gz",
+			},
 		},
 		{
-			name: "With leading slash in storage URI",
-			env: map[string]string{
-				precompiledStepsPrimaryStorageEnv: "https://custom.example.com/storage",
-			},
+			name:        "URL normalization: trailing slashes and leading StorageURI slash",
+			storageURLs: "https://a.example.com/// , https://b.example.com///",
 			executable: models.Executable{
 				StorageURI: "/steps/step3.tar.gz",
 			},
-			expectedURL: "https://custom.example.com/storage/steps/step3.tar.gz",
+			expectedURLs: []string{
+				"https://a.example.com/steps/step3.tar.gz",
+				"https://b.example.com/steps/step3.tar.gz",
+			},
 		},
 		{
-			name: "With multiple slashes in base URL",
-			env: map[string]string{
-				precompiledStepsPrimaryStorageEnv: "https://custom.example.com/storage///",
-			},
+			name:        "Input parsing: spaces and empty entries",
+			storageURLs: ", https://a.example.com , , https://b.example.com ,",
 			executable: models.Executable{
 				StorageURI: "steps/step4.tar.gz",
 			},
-			expectedURL: "https://custom.example.com/storage/steps/step4.tar.gz",
+			expectedURLs: []string{
+				"https://a.example.com/steps/step4.tar.gz",
+				"https://b.example.com/steps/step4.tar.gz",
+			},
 		},
 		{
-			name: "Empty storage URI",
-			env: map[string]string{
-				precompiledStepsPrimaryStorageEnv: "https://custom.example.com/storage",
+			name:        "http URL is rejected",
+			storageURLs: "http://a.example.com",
+			executable: models.Executable{
+				StorageURI: "steps/step5.tar.gz",
 			},
+			expectedErr: fmt.Errorf("http URL is unsupported, please use https: http://a.example.com/steps/step5.tar.gz"),
+		},
+		{
+			name:        "All-empty list yields a configuration error",
+			storageURLs: ",,",
 			executable: models.Executable{
-				StorageURI: "",
+				StorageURI: "steps/step6.tar.gz",
 			},
-			expectedURL: "https://custom.example.com/storage/",
+			expectedErr: fmt.Errorf("no storage URLs configured"),
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if val, exists := tt.env[precompiledStepsPrimaryStorageEnv]; exists {
-				t.Setenv(precompiledStepsPrimaryStorageEnv, val)
+			t.Setenv(precompiledStepsStorageURLsEnv, tt.storageURLs)
+			t.Setenv(precompiledStepsPrimaryStorageEnvDeprecated, "")
+
+			got, err := buildDownloadURLs(tt.executable)
+			if tt.expectedErr != nil {
+				require.EqualError(t, err, tt.expectedErr.Error())
+			} else {
+				require.NoError(t, err)
+				require.Equal(t, tt.expectedURLs, got)
 			}
-			url := downloadURL(tt.executable)
-			require.Equal(t, tt.expectedURL, url)
 		})
 	}
 }
+
+func TestBuildDownloadURLs_DeprecatedEnvVar(t *testing.T) {
+	t.Setenv(precompiledStepsStorageURLsEnv, "")
+	t.Setenv(precompiledStepsPrimaryStorageEnvDeprecated, "https://legacy.example.com")
+
+	got, err := buildDownloadURLs(models.Executable{StorageURI: "steps/step.tar.gz"})
+	require.NoError(t, err)
+	require.Equal(t, []string{"https://legacy.example.com/steps/step.tar.gz"}, got)
+}
+
+func TestBuildDownloadURLs_NewEnvVarWinsOverDeprecated(t *testing.T) {
+	t.Setenv(precompiledStepsStorageURLsEnv, "https://new.example.com")
+	t.Setenv(precompiledStepsPrimaryStorageEnvDeprecated, "https://legacy.example.com")
+
+	got, err := buildDownloadURLs(models.Executable{StorageURI: "steps/step.tar.gz"})
+	require.NoError(t, err)
+	require.Equal(t, []string{"https://new.example.com/steps/step.tar.gz"}, got)
+}
+
+func TestDownloadFromURLs(t *testing.T) {
+	t.Run("primary succeeds, secondary is not called", func(t *testing.T) {
+		secondaryHits := 0
+		primary := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			_, _ = w.Write([]byte("from primary"))
+		}))
+		defer primary.Close()
+		secondary := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			secondaryHits++
+		}))
+		defer secondary.Close()
+
+		body, err := downloadFromURLs([]string{primary.URL, secondary.URL})
+		require.NoError(t, err)
+		defer func() { _ = body.Close() }()
+
+		b, err := io.ReadAll(body)
+		require.NoError(t, err)
+		require.Equal(t, "from primary", string(b))
+		require.Equal(t, 0, secondaryHits)
+	})
+
+	t.Run("primary 404 falls back to secondary", func(t *testing.T) {
+		primary := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusNotFound)
+		}))
+		defer primary.Close()
+		secondary := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			_, _ = w.Write([]byte("from secondary"))
+		}))
+		defer secondary.Close()
+
+		body, err := downloadFromURLs([]string{primary.URL, secondary.URL})
+		require.NoError(t, err)
+		defer func() { _ = body.Close() }()
+
+		b, err := io.ReadAll(body)
+		require.NoError(t, err)
+		require.Equal(t, "from secondary", string(b))
+	})
+
+	t.Run("all URLs fail and the error lists each one", func(t *testing.T) {
+		primary := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusNotFound)
+		}))
+		defer primary.Close()
+		secondary := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusForbidden)
+		}))
+		defer secondary.Close()
+
+		_, err := downloadFromURLs([]string{primary.URL, secondary.URL})
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "failed to download executable")
+		require.Contains(t, err.Error(), primary.URL)
+		require.Contains(t, err.Error(), "status 404")
+		require.Contains(t, err.Error(), secondary.URL)
+		require.Contains(t, err.Error(), "status 403")
+	})
+}