Merge pull request #1606 from buildpacks/jjbustamante/fix/issue-1590-restorer-mirror-selection

Fix restorer to use selected mirror for Platform API 0.14

Author: jabrown85

cmd/lifecycle/restorer.go

@@ -214,11 +214,9 @@ func (r *restoreCmd) runImageAccessCheck(runImageName string) (string, error) {
 		return "", err
 	}
 
-	if !runToml.Contains(runImageName) {
-		return runImageName, nil
-	}
-
-	return platform.BestRunImageMirrorFor("", runToml.FindByRef(runImageName), r.AccessChecker())
+	// For Platform API 0.14+, use the new function that validates the already-selected
+	// run image from analyzed.toml without retrying the primary image first (issue #1590)
+	return platform.ResolveRunImageFromAnalyzed(runImageName, runToml, r.AccessChecker())
 }
 
 func (r *restoreCmd) needsUpdating(runImage *files.RunImage, group buildpack.Group) bool {

platform/run_image.go

@@ -126,3 +126,33 @@ func GetRunImageFromMetadata(inputs LifecycleInputs, md files.LayersMetadata) (f
 		return files.RunImageForExport{}, errors.New("no run image metadata available")
 	}
 }
+
+// ResolveRunImageFromAnalyzed resolves the run image for Platform API 0.14+ restorer.
+// It takes the already-selected run image from analyzed.toml and validates it's accessible.
+// For issue #1590: This should NOT retry with the primary image if a mirror was already selected.
+func ResolveRunImageFromAnalyzed(runImageFromAnalyzed string, runToml files.Run, checkReadAccess CheckReadAccess) (string, error) {
+	// If the run image is not in run.toml, return it as-is
+	// (e.g., extensions switched the run image)
+	if !runToml.Contains(runImageFromAnalyzed) {
+		return runImageFromAnalyzed, nil
+	}
+
+	// FIX for issue #1590:
+	// The analyzer already selected an accessible run image and wrote it to analyzed.toml.
+	// For Platform API 0.14, the restorer should just validate that THIS specific image
+	// is still accessible, not redo the entire selection process.
+	//
+	// We must create a keychain for the selected image to check access
+	keychain, err := auth.DefaultKeychain(runImageFromAnalyzed)
+	if err != nil {
+		return "", fmt.Errorf("unable to create keychain: %w", err)
+	}
+
+	// Check if the already-selected run image is accessible
+	if ok, _ := checkReadAccess(runImageFromAnalyzed, keychain); ok {
+		return runImageFromAnalyzed, nil
+	}
+
+	// If the selected image is not accessible, return an error with the image name
+	return "", fmt.Errorf("selected run image '%s' is not accessible", runImageFromAnalyzed)
+}

platform/run_image_test.go

@@ -1,6 +1,7 @@
 package platform_test
 
 import (
+	"fmt"
 	"path/filepath"
 	"testing"
 
@@ -277,4 +278,119 @@ func testRunImage(t *testing.T, when spec.G, it spec.S) {
 			})
 		})
 	})
+
+	when(".ResolveRunImageFromAnalyzed", func() {
+		when("Platform API 0.14+", func() {
+			it("should use already-selected mirror from analyzed.toml", func() {
+				// Reproduces issue #1590
+				// The restorer should validate the already-selected mirror from analyzed.toml
+				// without retrying the primary image first
+				//
+				// The scenario:
+				//   1. Analyzer already selected "localhost:5000/pack-test/run" (accessible mirror)
+				//   2. Wrote it to analyzed.toml as run-image.image
+				//   3. Restorer reads this value: runImageName = "localhost:5000/pack-test/run"
+				//   4. Restorer has run.toml with primary + mirrors
+				//   5. Restorer should validate the already-selected mirror
+
+				// STEP 1: The run image name from analyzed.toml (already selected by analyzer)
+				runImageNameFromAnalyzed := "localhost:5000/pack-test/run"
+
+				// STEP 2: The run.toml with full entry (primary + mirrors)
+				runToml := files.Run{
+					Images: []files.RunImageForExport{
+						{
+							Image:   "pack-test/run",                          // Primary - INACCESSIBLE
+							Mirrors: []string{"localhost:5000/pack-test/run"}, // Mirror - accessible
+						},
+					},
+				}
+
+				// STEP 3: Access checker - simulates the real scenario
+				callCount := make(map[string]int)
+				checkAccess := func(image string, _ authn.Keychain) (bool, error) {
+					callCount[image]++
+					if image == "pack-test/run" {
+						// Primary is NOT accessible (401 error)
+						return false, fmt.Errorf("failed to get remote image: unauthorized")
+					}
+					if image == "localhost:5000/pack-test/run" {
+						// Mirror IS accessible
+						return true, nil
+					}
+					return false, fmt.Errorf("unknown image: %s", image)
+				}
+
+				// STEP 4: Call the function (simulates what restorer does)
+				result, err := platform.ResolveRunImageFromAnalyzed(runImageNameFromAnalyzed, runToml, checkAccess)
+
+				// EXPECTATIONS:
+				// 1. Should succeed
+				h.AssertNil(t, err)
+				h.AssertEq(t, result, "localhost:5000/pack-test/run")
+
+				// 2. CRITICAL: Should NOT have checked the primary image!
+				//    Since we already know the mirror was selected, we shouldn't retry primary
+				if callCount["pack-test/run"] > 0 {
+					t.Fatalf("FAIL: Should not check primary 'pack-test/run' when mirror was already selected. Call count: %v", callCount)
+				}
+
+				// 3. Should have checked the selected mirror
+				h.AssertEq(t, callCount["localhost:5000/pack-test/run"], 1)
+			})
+
+			it("returns the run image as-is when not in run.toml", func() {
+				// If the run image from analyzed.toml is not found in run.toml
+				// (e.g., extensions switched the run image), just return it
+				runImageNameFromAnalyzed := "some-extension-switched-image:latest"
+
+				runToml := files.Run{
+					Images: []files.RunImageForExport{
+						{
+							Image:   "pack-test/run",
+							Mirrors: []string{"localhost:5000/pack-test/run"},
+						},
+					},
+				}
+
+				checkAccess := func(_ string, _ authn.Keychain) (bool, error) {
+					return true, nil
+				}
+
+				result, err := platform.ResolveRunImageFromAnalyzed(runImageNameFromAnalyzed, runToml, checkAccess)
+
+				h.AssertNil(t, err)
+				h.AssertEq(t, result, "some-extension-switched-image:latest")
+			})
+
+			it("fails with helpful message when selected mirror is not accessible", func() {
+				// If the environment changed between analyzer and restorer phases,
+				// and the previously-selected mirror is no longer accessible,
+				// we should fail with a clear error message that includes the image name
+				runImageNameFromAnalyzed := "localhost:5000/pack-test/run"
+
+				runToml := files.Run{
+					Images: []files.RunImageForExport{
+						{
+							Image:   "pack-test/run",
+							Mirrors: []string{"localhost:5000/pack-test/run"},
+						},
+					},
+				}
+
+				checkAccess := func(_ string, _ authn.Keychain) (bool, error) {
+					// Mirror is no longer accessible
+					return false, fmt.Errorf("failed to get remote image: connection refused")
+				}
+
+				_, err := platform.ResolveRunImageFromAnalyzed(runImageNameFromAnalyzed, runToml, checkAccess)
+
+				// Should fail with clear message including the image name
+				h.AssertNotNil(t, err)
+				h.AssertStringContains(t, err.Error(), "selected run image")
+				h.AssertStringContains(t, err.Error(), "localhost:5000/pack-test/run")
+				h.AssertStringContains(t, err.Error(), "not accessible")
+			})
+		})
+	})
 }