feat(feature): add OCI pull retry with URL sanitization

cmd/up.go

@@ -66,6 +66,34 @@ func NewUpCmd(f *flags.GlobalFlags) *cobra.Command {
 	return upCmd
 }
 
+// Run runs the command logic.
+func (cmd *UpCmd) Run(
+	ctx context.Context,
+	devsyConfig *config.Config,
+	client client2.BaseWorkspaceClient,
+	args []string,
+) error {
+	cmd.prepareWorkspace(client)
+
+	wctx, err := cmd.executeDevsyUp(ctx, devsyConfig, client)
+	if err != nil {
+		return err
+	}
+	if wctx == nil {
+		return nil // Platform mode
+	}
+
+	if cmd.Prebuild {
+		return nil
+	}
+
+	if err := cmd.configureWorkspace(devsyConfig, client, wctx); err != nil {
+		return err
+	}
+
+	return cmd.openIDE(ctx, devsyConfig, client, wctx)
+}
+
 func (cmd *UpCmd) execute(cobraCmd *cobra.Command, args []string) error {
 	if err := cmd.validate(); err != nil {
 		return err
@@ -255,34 +283,6 @@ func (cmd *UpCmd) registerTestingFlags(upCmd *cobra.Command) {
 	_ = upCmd.Flags().MarkHidden("force-dockerless")
 }
 
-// Run runs the command logic.
-func (cmd *UpCmd) Run(
-	ctx context.Context,
-	devsyConfig *config.Config,
-	client client2.BaseWorkspaceClient,
-	args []string,
-) error {
-	cmd.prepareWorkspace(client)
-
-	wctx, err := cmd.executeDevsyUp(ctx, devsyConfig, client)
-	if err != nil {
-		return err
-	}
-	if wctx == nil {
-		return nil // Platform mode
-	}
-
-	if cmd.Prebuild {
-		return nil
-	}
-
-	if err := cmd.configureWorkspace(devsyConfig, client, wctx); err != nil {
-		return err
-	}
-
-	return cmd.openIDE(ctx, devsyConfig, client, wctx)
-}
-
 // workspaceContext holds the result of workspace preparation.
 type workspaceContext struct {
 	result  *config2.Result

pkg/devcontainer/build/options_test.go

@@ -9,8 +9,9 @@ import (
 )
 
 const (
-	testCacheImage    = "myregistry.io/cache:latest"
-	testCLICacheImage = "cli-override:v1"
+	testCacheImage      = "myregistry.io/cache:latest"
+	testCLICacheImage   = "cli-override:v1"
+	testOtherCacheImage = "other:tag"
 )
 
 func substitutedConfig(cfg *config.DevContainerConfig) *config.SubstitutedConfig {
@@ -22,7 +23,7 @@ func TestNewOptions_CacheFrom_ConfigOnly(t *testing.T) {
 		ParsedConfig: substitutedConfig(&config.DevContainerConfig{
 			DockerfileContainer: config.DockerfileContainer{
 				Build: &config.ConfigBuildOptions{
-					CacheFrom: types.StrArray{testCacheImage, "other:tag"},
+					CacheFrom: types.StrArray{testCacheImage, testOtherCacheImage},
 				},
 			},
 		}),
@@ -35,7 +36,7 @@ func TestNewOptions_CacheFrom_ConfigOnly(t *testing.T) {
 	if len(opts.CacheFrom) != 2 {
 		t.Fatalf("expected 2 CacheFrom entries, got %d: %v", len(opts.CacheFrom), opts.CacheFrom)
 	}
-	if opts.CacheFrom[0] != testCacheImage || opts.CacheFrom[1] != "other:tag" {
+	if opts.CacheFrom[0] != testCacheImage || opts.CacheFrom[1] != testOtherCacheImage {
 		t.Fatalf("unexpected CacheFrom: %v", opts.CacheFrom)
 	}
 	if _, ok := opts.BuildArgs["BUILDKIT_INLINE_CACHE"]; ok {
@@ -100,7 +101,7 @@ func TestNewOptions_CacheFrom_CLIOverridesConfig(t *testing.T) {
 		ParsedConfig: substitutedConfig(&config.DevContainerConfig{
 			DockerfileContainer: config.DockerfileContainer{
 				Build: &config.ConfigBuildOptions{
-					CacheFrom: types.StrArray{testCacheImage, "other:tag"},
+					CacheFrom: types.StrArray{testCacheImage, testOtherCacheImage},
 				},
 			},
 		}),

pkg/devcontainer/feature/features.go

@@ -147,6 +147,23 @@ func checkFeatureCache(id string) (string, bool) {
 	return "", false
 }
 
+func pullOCIImage(ref name.Reference) (v1.Image, error) {
+	var img v1.Image
+	err := retryOCIPull(func() error {
+		log.Debugf("fetching OCI image: reference=%s", ref.String())
+		var fetchErr error
+		img, fetchErr = remote.Image(ref, remote.WithAuthFromKeychain(authn.DefaultKeychain))
+		return fetchErr
+	})
+	if err != nil {
+		err = image.SanitizeRegistryError(err)
+		registry := sanitizeURL(ref.Context().RegistryStr())
+		log.Debugf("failed to fetch OCI image: error=%v, registry=%s", err, registry)
+		return nil, fmt.Errorf("pull from %s: %w", registry, err)
+	}
+	return img, nil
+}
+
 func processOCIFeature(id string) (string, error) {
 	log.Debugf("processing OCI feature: featureId=%s", id)
 
@@ -167,45 +184,47 @@ func processOCIFeature(id string) (string, error) {
 		return "", err
 	}
 
-	log.Debugf("fetching OCI image: reference=%s", ref.String())
-	img, err := remote.Image(ref, remote.WithAuthFromKeychain(authn.DefaultKeychain))
-	if err != nil {
-		err = image.SanitizeRegistryError(err)
-		log.Debugf("failed to fetch OCI image: error=%v, reference=%s", err, ref.String())
+	if err := pullAndExtractOCIFeature(ref, id, featureFolder, featureExtractedFolder); err != nil {
 		return "", err
 	}
 
+	log.Infof(
+		"OCI feature processed successfully: featureId=%s, path=%s",
+		id,
+		featureExtractedFolder,
+	)
+	return featureExtractedFolder, nil
+}
+
+func pullAndExtractOCIFeature(ref name.Reference, id, featureFolder, destDir string) error {
+	img, err := pullOCIImage(ref)
+	if err != nil {
+		return err
+	}
+
 	destFile := filepath.Join(featureFolder, "feature.tgz")
+	registry := sanitizeURL(ref.Context().RegistryStr())
 	err = downloadLayer(img, id, destFile)
 	if err != nil {
 		log.Debugf("failed to download feature layer: error=%v, featureId=%s", err, id)
-		return "", err
+		return fmt.Errorf("download layer from %s: %w", registry, err)
 	}
 
 	file, err := os.Open(destFile)
 	if err != nil {
-		return "", err
+		return err
 	}
 	defer func() { _ = file.Close() }()
 
-	log.Debugf("extract feature: destination=%s", featureExtractedFolder)
-	err = extract.Extract(file, featureExtractedFolder)
+	log.Debugf("extract feature: destination=%s", destDir)
+	err = extract.Extract(file, destDir)
 	if err != nil {
-		log.Debugf(
-			"failed to extract feature: error=%v, destination=%s",
-			err,
-			featureExtractedFolder,
-		)
-		_ = os.RemoveAll(featureExtractedFolder)
-		return "", err
+		log.Debugf("failed to extract feature: error=%v, destination=%s", err, destDir)
+		_ = os.RemoveAll(destDir)
+		return err
 	}
 
-	log.Infof(
-		"OCI feature processed successfully: featureId=%s, path=%s",
-		id,
-		featureExtractedFolder,
-	)
-	return featureExtractedFolder, nil
+	return nil
 }
 
 func validateImageManifest(img v1.Image) (*v1.Manifest, error) {

pkg/devcontainer/feature/features_oci_test.go

@@ -0,0 +1,36 @@
+package feature
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/suite"
+)
+
+type OCIFeatureTestSuite struct {
+	suite.Suite
+}
+
+func TestOCIFeatureTestSuite(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping OCI pull test in short mode")
+	}
+	suite.Run(t, new(OCIFeatureTestSuite))
+}
+
+func (s *OCIFeatureTestSuite) TestProcessOCIFeature_HappyPath() {
+	dir := s.T().TempDir()
+	orig := os.Getenv("XDG_CACHE_HOME")
+	s.T().Setenv("XDG_CACHE_HOME", dir)
+	defer func() {
+		if orig != "" {
+			_ = os.Setenv("XDG_CACHE_HOME", orig)
+		}
+	}()
+
+	result, err := processOCIFeature("ghcr.io/devcontainers/features/go:1")
+	s.Require().NoError(err)
+	s.DirExists(result)
+	s.FileExists(filepath.Join(result, "devcontainer-feature.json"))
+}

pkg/devcontainer/feature/oci_retry.go

@@ -0,0 +1,59 @@
+package feature
+
+import (
+	"errors"
+	"net/http"
+	"time"
+
+	"github.com/devsy-org/devsy/pkg/log"
+	"github.com/google/go-containerregistry/pkg/v1/remote/transport"
+)
+
+const (
+	ociMaxRetries    = 3
+	ociBaseDelay     = 1 * time.Second
+	ociRetryExponent = 2
+)
+
+// isTransientError returns true for errors that may resolve on retry:
+// network timeouts, connection resets, and 5xx server errors.
+func isTransientError(err error) bool {
+	if err == nil {
+		return false
+	}
+
+	var terr *transport.Error
+	if errors.As(err, &terr) {
+		return terr.StatusCode >= http.StatusInternalServerError
+	}
+
+	return true
+}
+
+// retryOCIPull executes fn up to ociMaxRetries times with exponential backoff.
+// It only retries when isTransientError returns true.
+func retryOCIPull(fn func() error) error {
+	var lastErr error
+	delay := ociBaseDelay
+
+	for attempt := range ociMaxRetries {
+		if attempt > 0 {
+			log.Debugf("OCI pull retry: attempt=%d, delay=%v", attempt+1, delay)
+			time.Sleep(delay)
+			delay *= ociRetryExponent
+		}
+
+		lastErr = fn()
+		if lastErr == nil {
+			return nil
+		}
+
+		if !isTransientError(lastErr) {
+			return lastErr
+		}
+
+		log.Debugf("OCI pull transient failure: attempt=%d, error=%v", attempt+1, lastErr)
+	}
+
+	return lastErr
+}