Repository: buildpacks/lifecycle
Pull request #1227 (2023-10-27 08:31:46)

Do not obscure error message
<!-- 🎉🎉🎉 Thank you for the PR!!! 🎉🎉🎉 -->

### Summary
<!-- Please describe your changes at a high level. -->
When the lifecycle failed because there was no readable run image, the error was rather misleading:

`-run-image is required when there is no stack metadata available`

This was caused by hiding the underlying error.

#### Release notes
<!-- Please provide 1-2 sentences for release notes. -->
<!-- Example: When using platform API `0.7` or greater, the `creator` logs the expected phase header for the analyze phase -->

Improve error message in case run-image is not readable.

---

### Related
<!-- If this PR addresses an issue, please provide the issue number below. -->

Resolves #___

---

### Context
<!-- Add any other context that may help reviewers (e.g., code that requires special attention, etc.). -->

When the run image is on a protected registry, the `auth` data must be provided to the lifecycle.


natalieparellano: Aye, now the linter is unhappy because the spacing for our exported message vars has changed and they were uncommented. You could update the commit sha here: https://github.com/buildpacks/lifecycle/blob/367b451eb708dff1aa6e5ba179e1c88c4b9917e2/golangci.yaml#L38 if you don't feel like adding comments. 
c0d1ngm0nk3y: > Aye, now the linter is unhappy...

I have made some changes. IMHO, not the most useful linter warning though :) Please have a look.

Patch:
diff --git a/platform/resolve_inputs.go b/platform/resolve_inputs.go
index 743784ee..68d633be 100644
--- a/platform/resolve_inputs.go
+++ b/platform/resolve_inputs.go
@@ -11,13 +11,18 @@ import (
 )
 
 var (
-	ErrOutputImageRequired           = "image argument is required"
-	ErrRunImageRequiredWhenNoStackMD = "-run-image is required when there is no stack metadata available"
-	ErrRunImageRequiredWhenNoRunMD   = "-run-image is required when there is no run metadata available"
-	ErrSupplyOnlyOneRunImage         = "supply only one of -run-image or (deprecated) -image"
-	ErrRunImageUnsupported           = "-run-image is unsupported"
-	ErrImageUnsupported              = "-image is unsupported"
-	MsgIgnoringLaunchCache           = "Ignoring -launch-cache, only intended for use with -daemon"
+	// ErrOutputImageRequired user facing error message
+	ErrOutputImageRequired = "image argument is required"
+	// ErrRunImageRequiredWhenNoRunMD user facing error message
+	ErrRunImageRequiredWhenNoRunMD = "-run-image is required when there is no run metadata available"
+	// ErrSupplyOnlyOneRunImage user facing error message
+	ErrSupplyOnlyOneRunImage = "supply only one of -run-image or (deprecated) -image"
+	// ErrRunImageUnsupported user facing error message
+	ErrRunImageUnsupported = "-run-image is unsupported"
+	// ErrImageUnsupported user facing error message
+	ErrImageUnsupported = "-image is unsupported"
+	// MsgIgnoringLaunchCache user facing error message
+	MsgIgnoringLaunchCache = "Ignoring -launch-cache, only intended for use with -daemon"
 )
 
 func ResolveInputs(phase LifecyclePhase, i *LifecycleInputs, logger log.Logger) error {
@@ -178,7 +183,7 @@ func fillRunImageFromStackTOMLIfNeeded(i *LifecycleInputs, logger log.Logger) er
 	}
 	i.RunImageRef, err = BestRunImageMirrorFor(targetRegistry, stackMD.RunImage, i.AccessChecker())
 	if err != nil {
-		return errors.New(ErrRunImageRequiredWhenNoStackMD)
+		return err
 	}
 	return nil
 }
diff --git a/platform/run_image.go b/platform/run_image.go
index c0273e18..12cc283b 100644
--- a/platform/run_image.go
+++ b/platform/run_image.go
@@ -25,7 +25,7 @@ const (
 func BestRunImageMirrorFor(targetRegistry string, runImageMD files.RunImageForExport, checkReadAccess CheckReadAccess) (string, error) {
 	var runImageMirrors []string
 	if runImageMD.Image == "" {
-		return "", errors.New("missing run image metadata")
+		return "", errors.New("missing run image metadata (-run-image)")
 	}
 	runImageMirrors = append(runImageMirrors, runImageMD.Image)
 	runImageMirrors = append(runImageMirrors, runImageMD.Mirrors...)


Test patch:
diff --git a/log/timelog_test.go b/log/timelog_test.go
index fbcedf92..4cac702b 100644
--- a/log/timelog_test.go
+++ b/log/timelog_test.go
@@ -24,28 +24,28 @@ func (m mockLog) incr(key string) {
 	}
 }
 
-func (m mockLog) Debug(msg string) {
+func (m mockLog) Debug(_ string) {
 	m.incr("Debug")
 }
-func (m mockLog) Debugf(fmt string, v ...interface{}) {
+func (m mockLog) Debugf(_ string, _ ...interface{}) {
 	m.incr("Debug")
 }
-func (m mockLog) Info(msg string) {
+func (m mockLog) Info(_ string) {
 	m.incr("Info")
 }
-func (m mockLog) Infof(fmt string, v ...interface{}) {
+func (m mockLog) Infof(_ string, _ ...interface{}) {
 	m.incr("Info")
 }
-func (m mockLog) Warn(msg string) {
+func (m mockLog) Warn(_ string) {
 	m.incr("Warn")
 }
-func (m mockLog) Warnf(fmt string, v ...interface{}) {
+func (m mockLog) Warnf(_ string, _ ...interface{}) {
 	m.incr("Warn")
 }
-func (m mockLog) Error(msg string) {
+func (m mockLog) Error(_ string) {
 	m.incr("Error")
 }
-func (m mockLog) Errorf(fmt string, v ...interface{}) {
+func (m mockLog) Errorf(_ string, _ ...interface{}) {
 	m.incr("Error")
 }
 
diff --git a/platform/resolve_analyze_inputs_test.go b/platform/resolve_analyze_inputs_test.go
index 4f4b560f..5e738cf4 100644
--- a/platform/resolve_analyze_inputs_test.go
+++ b/platform/resolve_analyze_inputs_test.go
@@ -111,7 +111,7 @@ func testResolveAnalyzeInputs(platformAPI string) func(t *testing.T, when spec.G
 								inputs.StackPath = "not-exist-stack.toml"
 								err := platform.ResolveInputs(platform.Analyze, inputs, logger)
 								h.AssertNotNil(t, err)
-								expected := "-run-image is required when there is no stack metadata available"
+								expected := "missing run image metadata (-run-image)"
 								h.AssertStringContains(t, err.Error(), expected)
 							})
 						})
diff --git a/platform/resolve_create_inputs_test.go b/platform/resolve_create_inputs_test.go
index 51d8fbc0..105afcad 100644
--- a/platform/resolve_create_inputs_test.go
+++ b/platform/resolve_create_inputs_test.go
@@ -110,7 +110,7 @@ func testResolveCreateInputs(platformAPI string) func(t *testing.T, when spec.G,
 								inputs.StackPath = "not-exist-stack.toml"
 								err := platform.ResolveInputs(platform.Create, inputs, logger)
 								h.AssertNotNil(t, err)
-								expected := "-run-image is required when there is no stack metadata available"
+								expected := "missing run image metadata (-run-image)"
 								h.AssertStringContains(t, err.Error(), expected)
 							})
 						})
diff --git a/platform/run_image_test.go b/platform/run_image_test.go
index 0025ddfb..bc7a6b83 100644
--- a/platform/run_image_test.go
+++ b/platform/run_image_test.go
@@ -235,5 +235,18 @@ func testRunImage(t *testing.T, when spec.G, it spec.S) {
 				h.AssertEq(t, name, "gcr.io/myorg/myrepo")
 			})
 		})
+
+		when("there is no read access", func() {
+			it("fails", func() {
+				noReadAccess := func(_ string, _ authn.Keychain) (bool, error) {
+					return false, nil
+				}
+
+				_, err := platform.BestRunImageMirrorFor("gcr.io", stackMD.RunImage, noReadAccess)
+				h.AssertNotNil(t, err)
+				expected := "failed to find accessible run image"
+				h.AssertStringContains(t, err.Error(), expected)
+			})
+		})
 	})
 }