diff --git a/activator/steplib/activate.go b/activator/steplib/activate.go index 9749f7d8..fc490098 100644 --- a/activator/steplib/activate.go +++ b/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) diff --git a/activator/steplib/activate_executable.go b/activator/steplib/activate_executable.go index 9672aebd..ca81a56f 100644 --- a/activator/steplib/activate_executable.go +++ b/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...)) } diff --git a/activator/steplib/activate_executable_test.go b/activator/steplib/activate_executable_test.go index 5bf9652e..40d7ec1c 100644 --- a/activator/steplib/activate_executable_test.go +++ b/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") + }) +}