From 14bd5aaad94a88be90233977c781c52af6ab6876 Mon Sep 17 00:00:00 2001 From: Juan Bustamante Date: Thu, 12 Feb 2026 08:09:44 -0500 Subject: [PATCH] Fix restorer to use selected mirror for Platform API 0.14 For Platform API 0.14+, the restorer should validate the already-selected run image from analyzed.toml without retrying the primary image first. This fixes issue #1590 where the restorer's runImageAccessCheck would call BestRunImageMirrorFor with the full entry (primary + mirrors), causing it to attempt accessing the inaccessible primary image before trying the already-selected mirror. Changes: - Add ResolveRunImageFromAnalyzed function to validate selected mirror - Update restorer to use new function for Platform API 0.14+ - Add comprehensive tests covering the fix and edge cases - Improve error messages to include image name for better debugging Fixes #1590 Co-Authored-By: Claude Sonnet 4.5 Signed-off-by: Juan Bustamante --- cmd/lifecycle/restorer.go | 8 +-- platform/run_image.go | 30 ++++++++++ platform/run_image_test.go | 116 +++++++++++++++++++++++++++++++++++++ 3 files changed, 149 insertions(+), 5 deletions(-) diff --git a/cmd/lifecycle/restorer.go b/cmd/lifecycle/restorer.go index 6616ce490..37682de78 100644 --- a/cmd/lifecycle/restorer.go +++ b/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 { diff --git a/platform/run_image.go b/platform/run_image.go index d66684788..06a4c2947 100644 --- a/platform/run_image.go +++ b/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) +} diff --git a/platform/run_image_test.go b/platform/run_image_test.go index 7fa41e8f9..f49dfc55d 100644 --- a/platform/run_image_test.go +++ b/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") + }) + }) + }) }